Merge remote-tracking branch 'origin/farm/fa17b3a4/scope-package-changelog-lookup'
This commit is contained in:
can1357
@@ -25,6 +25,10 @@
|
||||
|
||||
- Fixed `createAgentSession()` dropping the hidden `resolve` tool from the registry when no active tool sets `deferrable: true`, even though plan mode dispatches the plan-approval `resolve { action: "apply", ... }` call through a standing handler. Read-only plan-mode toolsets (e.g. `read`, `search`, `find`, `web_search`) silently activated plan mode without `resolve`, leaving the agent unable to submit the finalized plan and forcing the user to exit plan mode manually. `resolve` is now kept whenever `plan.enabled` is true, so the standing handler always has a callable tool ([#1428](https://github.com/can1357/oh-my-pi/issues/1428))
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `omp` startup and `/changelog` reading the host project's `CHANGELOG.md` as omp's — `getPackageDir()` no longer falls back to the user's `cwd` when no owning `package.json` is locatable, preventing spurious `lastChangelogVersion` writes ([#1423](https://github.com/can1357/oh-my-pi/issues/1423))
|
||||
|
||||
## [15.5.3] - 2026-05-27
|
||||
### Breaking Changes
|
||||
|
||||
|
||||
@@ -18,30 +18,57 @@ const priorityList = [
|
||||
// =============================================================================
|
||||
|
||||
/**
|
||||
* Get the base directory for resolving optional package assets (docs, examples).
|
||||
* Walk up from import.meta.dir until we find package.json, or fall back to cwd.
|
||||
* Walk up from `startDir` looking for a `package.json`. Returns the directory
|
||||
* containing the marker, or `undefined` when the walk hits the filesystem root
|
||||
* without finding one.
|
||||
*
|
||||
* Exported for unit-testing the resolution contract from arbitrary start
|
||||
* directories (notably the `bun --compile` case where `import.meta.dir`
|
||||
* resolves to `/$bunfs/root` and no owning package is locatable — issue
|
||||
* #1423). Production callers should use {@link getPackageDir} instead.
|
||||
*/
|
||||
export function getPackageDir(): string {
|
||||
// Allow override via environment variable (useful for Nix/Guix where store paths tokenize poorly)
|
||||
const envDir = process.env.PI_PACKAGE_DIR;
|
||||
if (envDir) {
|
||||
return expandTilde(envDir);
|
||||
}
|
||||
|
||||
let dir = import.meta.dir;
|
||||
export function walkUpForPackageDir(startDir: string): string | undefined {
|
||||
let dir = startDir;
|
||||
while (dir !== path.dirname(dir)) {
|
||||
if (fs.existsSync(path.join(dir, "package.json"))) {
|
||||
return dir;
|
||||
}
|
||||
dir = path.dirname(dir);
|
||||
}
|
||||
// Fallback to project dir (docs/examples won't be found, but that's fine)
|
||||
return getProjectDir();
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/** Get path to CHANGELOG.md (optional, may not exist in binary) */
|
||||
export function getChangelogPath(): string {
|
||||
return path.resolve(path.join(getPackageDir(), "CHANGELOG.md"));
|
||||
/**
|
||||
* Get the base directory for resolving optional package assets (docs, examples, CHANGELOG.md).
|
||||
*
|
||||
* Honors the `PI_PACKAGE_DIR` override (useful for Nix/Guix store paths);
|
||||
* otherwise walks up from `import.meta.dir` looking for a `package.json`.
|
||||
* Returns `undefined` when no owning package is locatable — notably inside
|
||||
* `bun --compile` binaries where `import.meta.dir` resolves to `/$bunfs/root`
|
||||
* and the walk hits the filesystem root with nothing found.
|
||||
*
|
||||
* Callers MUST treat `undefined` as "no package assets available" and skip the
|
||||
* lookup. NEVER fall back to the user's `cwd` here: that conflates the host
|
||||
* project with omp's own assets and was the source of issue #1423 (the host
|
||||
* project's `CHANGELOG.md` rendered as omp's startup changelog).
|
||||
*/
|
||||
export function getPackageDir(): string | undefined {
|
||||
const envDir = process.env.PI_PACKAGE_DIR;
|
||||
if (envDir) {
|
||||
return expandTilde(envDir);
|
||||
}
|
||||
return walkUpForPackageDir(import.meta.dir);
|
||||
}
|
||||
|
||||
/**
|
||||
* Path to omp's own `CHANGELOG.md`, or `undefined` when the package directory
|
||||
* cannot be resolved (e.g. inside `bun --compile` binaries that don't bundle
|
||||
* package assets). Callers MUST skip changelog parsing when this is undefined;
|
||||
* see issue #1423.
|
||||
*/
|
||||
export function getChangelogPath(): string | undefined {
|
||||
const packageDir = getPackageDir();
|
||||
return packageDir ? path.resolve(packageDir, "CHANGELOG.md") : undefined;
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
||||
@@ -8,10 +8,18 @@ export interface ChangelogEntry {
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse changelog entries from CHANGELOG.md
|
||||
* Scans for ## lines and collects content until next ## or EOF
|
||||
* Parse changelog entries from the file at `changelogPath`. Scans for `## [x.y.z]`
|
||||
* headings and collects each block until the next heading or EOF.
|
||||
*
|
||||
* Returns `[]` when `changelogPath` is `undefined` (package directory not
|
||||
* resolvable — see `getChangelogPath`) or the file is missing. Callers MUST NOT
|
||||
* synthesize a fallback path from the host project's cwd; doing so caused issue
|
||||
* #1423 (the host project's `CHANGELOG.md` was rendered as omp's).
|
||||
*/
|
||||
export async function parseChangelog(changelogPath: string): Promise<ChangelogEntry[]> {
|
||||
export async function parseChangelog(changelogPath: string | undefined): Promise<ChangelogEntry[]> {
|
||||
if (!changelogPath) {
|
||||
return [];
|
||||
}
|
||||
try {
|
||||
const content = await Bun.file(changelogPath).text();
|
||||
const lines = content.split("\n");
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { getChangelogPath, getPackageDir, walkUpForPackageDir } from "../src/config";
|
||||
import { parseChangelog } from "../src/utils/changelog";
|
||||
|
||||
/**
|
||||
* Regression: omp startup parsed the host project's `CHANGELOG.md` as if it
|
||||
* were omp's, then persisted `lastChangelogVersion` to the global config. Root
|
||||
* cause was `getPackageDir()` falling back to `getProjectDir()` (the user's
|
||||
* cwd) when the walk-up from `import.meta.dir` couldn't find a `package.json`
|
||||
* — which happens in `bun --compile` binaries where `import.meta.dir`
|
||||
* resolves to `/$bunfs/root`.
|
||||
*
|
||||
* Contract this test defends:
|
||||
* 1. `walkUpForPackageDir(<isolated dir>)` returns `undefined` when no
|
||||
* ancestor contains `package.json`. NEVER falls back to `process.cwd()`.
|
||||
* 2. `parseChangelog(undefined)` returns `[]` so the compiled-binary path
|
||||
* skips startup display and never mutates `lastChangelogVersion`.
|
||||
* 3. `getChangelogPath()` (when defined) never points under the host
|
||||
* project's `cwd`.
|
||||
* 4. `PI_PACKAGE_DIR` overrides anchor exactly to the override directory and
|
||||
* do not fall back to `cwd`.
|
||||
*/
|
||||
describe("issue #1423 — package-dir lookup must not fall back to cwd", () => {
|
||||
let projectDir: string;
|
||||
let originalCwd: string;
|
||||
let originalEnv: string | undefined;
|
||||
|
||||
beforeAll(() => {
|
||||
originalEnv = process.env.PI_PACKAGE_DIR;
|
||||
delete process.env.PI_PACKAGE_DIR;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (originalEnv === undefined) {
|
||||
delete process.env.PI_PACKAGE_DIR;
|
||||
} else {
|
||||
process.env.PI_PACKAGE_DIR = originalEnv;
|
||||
}
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
originalCwd = process.cwd();
|
||||
projectDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-issue-1423-"));
|
||||
fs.writeFileSync(
|
||||
path.join(projectDir, "CHANGELOG.md"),
|
||||
"# Changelog\n\n## [99.0.0] - 2099-01-01\n\n### Added\n\n- Host project entry, NOT omp's.\n",
|
||||
);
|
||||
// Critical: no package.json in projectDir — only CHANGELOG.md.
|
||||
process.chdir(projectDir);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.chdir(originalCwd);
|
||||
fs.rmSync(projectDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("walkUpForPackageDir returns undefined when no ancestor owns a package.json", () => {
|
||||
// Find a tmpdir whose ancestors (up to /) have no package.json. Assuming
|
||||
// /tmp/{deeply,nested} on Linux/macOS, this is the realistic bunfs case.
|
||||
const isolated = fs.mkdtempSync(path.join(os.tmpdir(), "omp-issue-1423-isolated-"));
|
||||
const deep = path.join(isolated, "a", "b", "c");
|
||||
fs.mkdirSync(deep, { recursive: true });
|
||||
try {
|
||||
// Sanity: confirm no ancestor of `deep` has a package.json. If a
|
||||
// host machine happens to ship `/package.json`, this assertion
|
||||
// surfaces it instead of letting the test silently pass.
|
||||
for (let dir = deep; dir !== path.dirname(dir); dir = path.dirname(dir)) {
|
||||
expect(fs.existsSync(path.join(dir, "package.json"))).toBe(false);
|
||||
}
|
||||
expect(walkUpForPackageDir(deep)).toBeUndefined();
|
||||
} finally {
|
||||
fs.rmSync(isolated, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("walkUpForPackageDir from cwd-with-CHANGELOG-only returns undefined, not cwd", () => {
|
||||
// Before the fix, `getPackageDir()` would have returned `getProjectDir()`
|
||||
// (≈ `process.cwd()` = `projectDir`) once the walk-up failed. The pure
|
||||
// helper exercises the exact resolution shape from an arbitrary start
|
||||
// directory, proving the fallback was removed at the source.
|
||||
expect(walkUpForPackageDir(projectDir)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("parseChangelog(undefined) yields no entries", async () => {
|
||||
const entries = await parseChangelog(undefined);
|
||||
expect(entries).toEqual([]);
|
||||
});
|
||||
|
||||
it("getChangelogPath()'s real-tree result never points under host cwd", () => {
|
||||
const changelogPath = getChangelogPath();
|
||||
// In dev/test runs from the workspace, walk-up succeeds and resolves
|
||||
// to omp's own CHANGELOG.md. The host project's `cwd` MUST not bleed
|
||||
// into the resolution either way.
|
||||
if (changelogPath !== undefined) {
|
||||
expect(changelogPath.startsWith(projectDir)).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("host project's CHANGELOG.md is never parsed as omp's", async () => {
|
||||
const changelogPath = getChangelogPath();
|
||||
const entries = await parseChangelog(changelogPath);
|
||||
const hostEntry = entries.find(e => e.major === 99 && e.minor === 0 && e.patch === 0);
|
||||
expect(hostEntry).toBeUndefined();
|
||||
});
|
||||
|
||||
it("PI_PACKAGE_DIR override stays put and does not fall back to cwd", () => {
|
||||
const bogusDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-issue-1423-override-"));
|
||||
try {
|
||||
process.env.PI_PACKAGE_DIR = bogusDir;
|
||||
expect(getPackageDir()).toBe(bogusDir);
|
||||
expect(getChangelogPath()).toBe(path.join(bogusDir, "CHANGELOG.md"));
|
||||
} finally {
|
||||
delete process.env.PI_PACKAGE_DIR;
|
||||
fs.rmSync(bogusDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user