[codex] Quiet packaged version fallback diagnostics (#9207)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server startup path reports the product version from package metadata and, in source checkouts, Git metadata. > - Packaged installs can run from `node_modules`, where Git metadata is normally unavailable and that absence is expected. > - The fallback path was still attempting Git metadata probing in packaged contexts, which could print scary diagnostic noise during onboarding even though the package version fallback was working. > - This pull request makes the packaged path skip Git probing only when the package does not look like a source checkout, and keeps fallback diagnostics opt-in. > - The benefit is a quieter first-run experience without weakening source-checkout version detection or debug diagnostics. ## Linked Issues or Issue Description No public GitHub issue exists. ### Bug Report #### Pre-submission checklist - [x] I have searched existing open and closed issues and this is not a duplicate. - [x] I am on the latest released version of Paperclip or can reproduce on `master`. - [x] I have confirmed the error originates in Paperclip itself, not in an agent adapter, API provider, or local configuration. #### What happened? When Paperclip starts from a packaged install, server version resolution can fall back from Git metadata to package metadata. That expected fallback path could emit scary Git diagnostic noise during onboarding even though startup could continue normally. #### Expected behavior Packaged Paperclip startup should use package metadata quietly when Git metadata is unavailable. Source checkouts should still use Git-derived versions, and operators who explicitly opt into version-resolution diagnostics should still receive useful Git failure details. #### Steps to reproduce 1. Run Paperclip from a packaged install where the server package is under `node_modules` and does not include package-local Git metadata. 2. Start the server in an environment where `git describe` cannot resolve repository metadata for that package. 3. Observe that version fallback can produce Git diagnostic noise during startup even though the package version fallback is expected. #### Paperclip version or commit Reproduced against the pre-fix server version resolution behavior on `master`-derived builds. #### Deployment mode Self-hosted server / packaged local install. #### Installation method npm / pnpm package install. #### Agent adapter(s) involved Not adapter-specific; this is core server startup/version behavior. #### Database mode Not database-related. #### Access context Unclear / not applicable. #### Relevant logs or output Git fallback diagnostics from `git describe` could appear during packaged startup. The exact path and Git output depend on the operator environment. #### Additional context The fix keeps diagnostics available behind `PAPERCLIP_DEBUG_VERSION_RESOLUTION=1` and preserves source-checkout Git version detection, including source paths that happen to contain a `node_modules` segment. #### Privacy checklist - [x] I have reviewed all pasted output for PII and redacted where necessary. ## What Changed - Skip Git metadata probing for packaged installs under `node_modules` only when no package-local Git metadata is present. - Preserve Git-derived version detection for source or linked workspace checkouts, even when their path contains a `node_modules` segment. - Keep fallback diagnostics behind the existing debug/diagnostic opt-in path. - Include useful Git failure details such as stderr/stdout/stack/cause when diagnostics are enabled. - Add version tests covering packaged fallback behavior, source-checkout detection, richer diagnostics, and quiet default output. ## Verification - `pnpm vitest run server/src/__tests__/version.test.ts` passed after the Greptile follow-up changes. - `pnpm --filter @paperclipai/server typecheck` passed. - `git diff --check` passed. - Greptile completed with confidence score 5/5 and no blocking issues on the latest reviewed commit. ## Risks Low risk. The change is scoped to version fallback behavior. Source-checkout Git version detection remains covered, while packaged `node_modules` contexts intentionally rely on package metadata instead of Git probing. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5-class coding agent with shell/tool use in the Paperclip workspace. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
49d1abc458
commit
e0f1905222
|
|
@ -42,19 +42,98 @@ describe("resolveServerVersion", () => {
|
|||
|
||||
it("falls back to package version without throwing when git is unavailable", () => {
|
||||
const debugLog = vi.fn();
|
||||
const cause = new Error("spawn git ENOENT");
|
||||
const err = Object.assign(new Error("fatal: not a git repository"), {
|
||||
code: 128,
|
||||
stderr: Buffer.from("fatal: not a git repository\n"),
|
||||
stdout: "",
|
||||
cause,
|
||||
});
|
||||
|
||||
expect(
|
||||
resolveServerVersion({
|
||||
packageVersion: "2026.706.0",
|
||||
gitDescribeCommand: () => {
|
||||
throw new Error("fatal: not a git repository");
|
||||
throw err;
|
||||
},
|
||||
debugLog,
|
||||
}),
|
||||
).toBe("2026.706.0");
|
||||
expect(debugLog).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ reason: "git_describe_unavailable" }),
|
||||
expect.objectContaining({
|
||||
err: expect.objectContaining({
|
||||
cause: expect.objectContaining({ message: "spawn git ENOENT" }),
|
||||
code: 128,
|
||||
message: "fatal: not a git repository",
|
||||
stderr: "fatal: not a git repository\n",
|
||||
stdout: "",
|
||||
stack: expect.any(String),
|
||||
}),
|
||||
reason: "git_describe_unavailable",
|
||||
}),
|
||||
"falling back to package version for server version",
|
||||
);
|
||||
});
|
||||
|
||||
it("skips git metadata probing for packaged installs under node_modules", () => {
|
||||
const debugLog = vi.fn();
|
||||
|
||||
expect(
|
||||
resolveServerVersion({
|
||||
packageVersion: "2026.707.0-canary.12",
|
||||
debugLog,
|
||||
packageRoot: "/tmp/npm/_npx/example/node_modules/@paperclipai/server",
|
||||
}),
|
||||
).toBe("2026.707.0-canary.12");
|
||||
|
||||
expect(debugLog).toHaveBeenCalledWith(
|
||||
{ reason: "packaged_install" },
|
||||
"falling back to package version for server version",
|
||||
);
|
||||
});
|
||||
|
||||
it("uses git metadata for source checkouts whose path contains node_modules", () => {
|
||||
const debugLog = vi.fn();
|
||||
const gitDescribeCommand = vi.fn(() => "v2026.626.0-58-g518fc71ce\n");
|
||||
|
||||
expect(
|
||||
resolveServerVersion({
|
||||
packageVersion: "2026.707.0-canary.12",
|
||||
debugLog,
|
||||
gitDescribeCommand,
|
||||
packageRoot: "/tmp/node_modules/source/paperclip/server",
|
||||
pathExists: (path) => path === "/tmp/node_modules/source/paperclip/.git",
|
||||
realpath: (path) => path,
|
||||
}),
|
||||
).toBe("2026.626.0+58.git.518fc71ce");
|
||||
|
||||
expect(gitDescribeCommand).toHaveBeenCalledOnce();
|
||||
expect(debugLog).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps fallback diagnostics quiet by default", () => {
|
||||
const previousDebugFlag = process.env.PAPERCLIP_DEBUG_VERSION_RESOLUTION;
|
||||
delete process.env.PAPERCLIP_DEBUG_VERSION_RESOLUTION;
|
||||
const consoleDebug = vi.spyOn(console, "debug").mockImplementation(() => {});
|
||||
|
||||
try {
|
||||
expect(
|
||||
resolveServerVersion({
|
||||
packageVersion: "2026.706.0",
|
||||
gitDescribeCommand: () => {
|
||||
throw new Error("fatal: not a git repository");
|
||||
},
|
||||
}),
|
||||
).toBe("2026.706.0");
|
||||
|
||||
expect(consoleDebug).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
consoleDebug.mockRestore();
|
||||
if (previousDebugFlag === undefined) {
|
||||
delete process.env.PAPERCLIP_DEBUG_VERSION_RESOLUTION;
|
||||
} else {
|
||||
process.env.PAPERCLIP_DEBUG_VERSION_RESOLUTION = previousDebugFlag;
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
import { createRequire } from "node:module";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { existsSync, realpathSync } from "node:fs";
|
||||
import { basename, dirname, join } from "node:path";
|
||||
|
||||
type PackageJson = {
|
||||
version?: string;
|
||||
|
|
@ -7,14 +9,19 @@ type PackageJson = {
|
|||
|
||||
type GitDescribeCommand = () => string;
|
||||
type DebugLog = (fields: Record<string, unknown>, message: string) => void;
|
||||
type PathExists = (path: string) => boolean;
|
||||
type Realpath = (path: string) => string;
|
||||
|
||||
const requirePackage = createRequire(import.meta.url);
|
||||
const packageRoot = dirname(requirePackage.resolve("../package.json"));
|
||||
const pkg = requirePackage("../package.json") as PackageJson;
|
||||
|
||||
const GIT_DESCRIBE_RE =
|
||||
/^v(?<publicVersion>\d+\.\d+\.\d+)-(?<commitsSinceTag>\d+)-g(?<sha>[0-9a-f]{7,40})(?<dirty>-dirty)?$/i;
|
||||
|
||||
function defaultDebugLog(fields: Record<string, unknown>, message: string): void {
|
||||
if (process.env.PAPERCLIP_DEBUG_VERSION_RESOLUTION !== "1") return;
|
||||
|
||||
console.debug(message, fields);
|
||||
}
|
||||
|
||||
|
|
@ -23,6 +30,7 @@ function defaultGitDescribeCommand(): string {
|
|||
"git",
|
||||
["describe", "--tags", "--match", "v*", "--long", "--dirty"],
|
||||
{
|
||||
cwd: packageRoot,
|
||||
encoding: "utf8",
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
timeout: 1500,
|
||||
|
|
@ -30,6 +38,97 @@ function defaultGitDescribeCommand(): string {
|
|||
);
|
||||
}
|
||||
|
||||
function hasPathSegment(path: string, segment: string): boolean {
|
||||
return path.split(/[\\/]+/).includes(segment);
|
||||
}
|
||||
|
||||
function safeRealpath(path: string, realpath: Realpath): string {
|
||||
try {
|
||||
return realpath(path);
|
||||
} catch {
|
||||
return path;
|
||||
}
|
||||
}
|
||||
|
||||
function hasGitMetadataBeforeNodeModulesBoundary(
|
||||
path: string,
|
||||
pathExists: PathExists,
|
||||
): boolean {
|
||||
let current = path;
|
||||
|
||||
while (true) {
|
||||
if (pathExists(join(current, ".git"))) return true;
|
||||
|
||||
const parent = dirname(current);
|
||||
if (parent === current || basename(current) === "node_modules") return false;
|
||||
|
||||
current = parent;
|
||||
}
|
||||
}
|
||||
|
||||
function isPackagedInstall(
|
||||
path: string,
|
||||
{
|
||||
pathExists = existsSync,
|
||||
realpath = realpathSync,
|
||||
}: { pathExists?: PathExists; realpath?: Realpath } = {},
|
||||
): boolean {
|
||||
const realPackageRoot = safeRealpath(path, realpath);
|
||||
const candidateRoots = Array.from(new Set([path, realPackageRoot]));
|
||||
const hasNodeModulesSegment = candidateRoots.some((candidate) =>
|
||||
hasPathSegment(candidate, "node_modules"),
|
||||
);
|
||||
|
||||
if (!hasNodeModulesSegment) return false;
|
||||
|
||||
return !candidateRoots.some((candidate) =>
|
||||
hasGitMetadataBeforeNodeModulesBoundary(candidate, pathExists),
|
||||
);
|
||||
}
|
||||
|
||||
function normalizeErrorField(value: unknown): unknown {
|
||||
if (Buffer.isBuffer(value)) return value.toString("utf8");
|
||||
if (value instanceof Uint8Array) return Buffer.from(value).toString("utf8");
|
||||
return value;
|
||||
}
|
||||
|
||||
function compactRecord(fields: Record<string, unknown>): Record<string, unknown> {
|
||||
return Object.fromEntries(
|
||||
Object.entries(fields).filter(([, value]) => value !== undefined),
|
||||
);
|
||||
}
|
||||
|
||||
function summarizeError(err: unknown): Record<string, unknown> {
|
||||
if (err && typeof err === "object") {
|
||||
const errorLike = err as {
|
||||
name?: unknown;
|
||||
message?: unknown;
|
||||
status?: unknown;
|
||||
signal?: unknown;
|
||||
code?: unknown;
|
||||
stdout?: unknown;
|
||||
stderr?: unknown;
|
||||
stack?: unknown;
|
||||
cause?: unknown;
|
||||
};
|
||||
|
||||
return compactRecord({
|
||||
name: errorLike.name,
|
||||
message: errorLike.message,
|
||||
status: errorLike.status,
|
||||
signal: errorLike.signal,
|
||||
code: errorLike.code,
|
||||
stdout: normalizeErrorField(errorLike.stdout),
|
||||
stderr: normalizeErrorField(errorLike.stderr),
|
||||
stack: errorLike.stack,
|
||||
cause:
|
||||
errorLike.cause === undefined ? undefined : summarizeError(errorLike.cause),
|
||||
});
|
||||
}
|
||||
|
||||
return { message: String(err) };
|
||||
}
|
||||
|
||||
export function parseGitDescribeVersion(output: string): string | null {
|
||||
const match = output.trim().match(GIT_DESCRIBE_RE);
|
||||
if (!match?.groups) return null;
|
||||
|
|
@ -51,11 +150,28 @@ export function resolveServerVersion(
|
|||
gitDescribeCommand?: GitDescribeCommand;
|
||||
packageVersion?: string;
|
||||
debugLog?: DebugLog;
|
||||
packageRoot?: string;
|
||||
pathExists?: PathExists;
|
||||
realpath?: Realpath;
|
||||
} = {},
|
||||
): string {
|
||||
const packageVersion = opts.packageVersion ?? pkg.version ?? "0.0.0";
|
||||
const gitDescribeCommand = opts.gitDescribeCommand ?? defaultGitDescribeCommand;
|
||||
const debugLog = opts.debugLog ?? defaultDebugLog;
|
||||
const resolvedPackageRoot = opts.packageRoot ?? packageRoot;
|
||||
|
||||
if (
|
||||
isPackagedInstall(resolvedPackageRoot, {
|
||||
pathExists: opts.pathExists,
|
||||
realpath: opts.realpath,
|
||||
})
|
||||
) {
|
||||
debugLog(
|
||||
{ reason: "packaged_install" },
|
||||
"falling back to package version for server version",
|
||||
);
|
||||
return packageVersion;
|
||||
}
|
||||
|
||||
try {
|
||||
const parsedVersion = parseGitDescribeVersion(gitDescribeCommand());
|
||||
|
|
@ -67,7 +183,7 @@ export function resolveServerVersion(
|
|||
);
|
||||
} catch (err) {
|
||||
debugLog(
|
||||
{ err, reason: "git_describe_unavailable" },
|
||||
{ err: summarizeError(err), reason: "git_describe_unavailable" },
|
||||
"falling back to package version for server version",
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue