mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
Capture bounded workspace base-ref failure diagnostics (#15719)
## Thinking Path > - Paperclip manages AI agents and their work. > - A run can prepare a Git worktree before an adapter starts. > - That preparation must resolve the configured base ref. > - The same unresolved-ref error can follow a missing ref, failed fetch, or failed credential lookup. > - Current failure reports do not distinguish these paths. > - This PR records bounded outcomes from the operations that already run. > - Maintainers can use this evidence without changing task recovery or hiding failures. ## Linked Issues or Issue Description Related: #11892 established remote-only base-ref resolution and pre-adapter recovery. I found no open PR for these diagnostics. **What happened?** A fresh worktree can fail with `UnresolvedWorkspaceBaseRefError`. Its failure report has no closed fields that distinguish remote lookup, credential lookup, fetch, and local ref-probe outcomes. An empty warning list does not prove that a fetch ran or that a remote ref is absent. **Expected behavior** Keep the task-visible failure and owner recovery action. Report bounded observations from the existing operations. Keep unknown causes reportable. **Steps to reproduce** Use a local Git repository with a configured base branch that is absent. Repeat with no origin, an inaccessible local origin, or a rejected credential resolver. Start fresh workspace preparation. Before this change, each path lacks the new typed observations in the failure report. Tests use local repositories and intercepted fetch commands; they require no external Git service. **Paperclip version or commit** Reproduced against source commit `1602512820`, built from source. This affects core workspace preparation before adapter dispatch. ## What Changed - Record closed remote lookup, credential lookup, fetch, and local ref-probe outcomes. Limit new output parsing to 8 KiB. - Attach evidence privately to the resolver's own error. Reject copied, forged, inherited, accessor, and malformed evidence. - Persist allowed values in the existing setup result and export them through run-failure diagnostics. - Preserve Git commands, credential fallback, warning text, recovery actions, and Sentry reporting. - Add real Git, heartbeat, reporter, and real Sentry envelope tests. Document the fields and their limits. ## Verification - Current head: `015b2737df004e971eeeb134ebc9a1024e244494`, based on `be1bc5d494d`. The only rebase conflict was in heartbeat test imports. All three imports were retained; `git range-diff` confirms no other patch change. - Full `pnpm -r typecheck` and `pnpm build` passed on the current head. - Fresh focused tests passed: 13 real-Git producer cases, 279 complete collector/reporter cases, two actual heartbeat recovery cases, and all nine real Sentry SDK cases. The required-SDK gate used version 10.71.0. Actual envelopes contain only allowed fields, with no private fixture text or cross-event leakage. - Independent review passed all 16 combined heartbeat cases from both integrated changes and five real-Git probes with identical baseline command sequences and outcomes. The original producer negative control failed on the absent receipt; the candidate passed. No review findings remain. - Current-head CI is complete: 54 successful checks and two intentional Storybook skips. Greptile is 5/5 on this exact head, with no unresolved threads or merge conflicts. - Local aggregate limitation: the earlier full run reported one failure in an unchanged tool-access case and was stopped with SIGINT (exit 130) to isolate it. Vitest did not print the deferred assertion summary. The unchanged complete file passed 420/420 alone. That aggregate is not claimed as a pass. Before the mechanical rebase, the complete workspace-runtime file also passed 172/172. The fresh full CI result covers the integrated head. - An initial independent combined reporter run hit its database setup timeout; the unchanged complete reporter file then passed 186/186 alone. Earlier-head CI lost four runners and recovered on one verification retry; current-head CI passed separately. ## Risks - These observations do not prove that a remote ref is absent. All affected failures remain reportable, including resumed and provider-started runs. - The new fields include no repository URL, ref text, customer path, raw command output, or credential. Existing task-visible details are unchanged. - No migration, dependency, authorization, retry, or deployment change. ## Model Used OpenAI GPT-6 through Codex, with tool execution and parallel independent review. The exact serving variant and context-window size are not exposed in this session. ## 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:
1 parent
be1bc5d494
commit
7333e4eba8
10 files changed
+390
-19
No files matched your search
@@ -67,6 +67,8 @@ import { isRuntimeOwnedGitBranch } from "./execution-workspace-branch-ownership.
|
||||
import { logActivity } from "./activity-log.js";
|
||||
import { readProjectWorkspaceRuntimeConfig } from "./project-workspace-runtime-config.js";
|
||||
import { workspaceGitOperationScheduler } from "./workspace-git-operation-scheduler.js";
|
||||
import { classifyGitCloneFailure } from "./git-connection-failure.js";
|
||||
import { readWorkspaceBaseRefDiagnostic, type WorkspaceBaseRefDiagnostic } from "./workspace-base-ref-diagnostics.js";
|
||||
import {
|
||||
MANAGED_GIT_WORKTREE_REASON_CODES,
|
||||
readManagedGitInspectionDiagnostic,
|
||||
@@ -914,13 +916,17 @@ async function executeProcess(input: {
|
||||
};
|
||||
}
|
||||
|
||||
async function runGit(args: string[], cwd: string, opts?: { env?: NodeJS.ProcessEnv }): Promise<string> {
|
||||
async function runGit(args: string[], cwd: string, opts?: {
|
||||
env?: NodeJS.ProcessEnv;
|
||||
observe?: (result: Awaited<ReturnType<typeof executeProcess>>) => void;
|
||||
}): Promise<string> {
|
||||
const proc = await executeProcess({
|
||||
command: "git",
|
||||
args,
|
||||
cwd,
|
||||
env: opts?.env,
|
||||
});
|
||||
opts?.observe?.(proc);
|
||||
if (proc.code !== 0) {
|
||||
throw new Error(proc.stderr.trim() || proc.stdout.trim() || `git ${args.join(" ")} failed`);
|
||||
}
|
||||
@@ -971,15 +977,32 @@ export async function refreshRemoteTrackingBaseRef(
|
||||
baseRef: string,
|
||||
resolveGitAuth?: GitRemoteAuthProvider | null,
|
||||
): Promise<string[]> {
|
||||
return (await refreshRemoteTrackingBaseRefWithDiagnostic(repoRoot, baseRef, resolveGitAuth)).warnings;
|
||||
}
|
||||
|
||||
async function refreshRemoteTrackingBaseRefWithDiagnostic(
|
||||
repoRoot: string,
|
||||
baseRef: string,
|
||||
resolveGitAuth?: GitRemoteAuthProvider | null,
|
||||
): Promise<{ warnings: string[]; diagnostic: WorkspaceBaseRefDiagnostic }> {
|
||||
const diagnostic: WorkspaceBaseRefDiagnostic = {
|
||||
schemaVersion: 1, remoteLookup: "not_attempted", authLookup: "not_requested", fetch: "not_attempted",
|
||||
};
|
||||
const remoteTracking = parseRemoteTrackingRef(baseRef);
|
||||
if (!remoteTracking) return [];
|
||||
if (!remoteTracking) return { warnings: [], diagnostic };
|
||||
|
||||
const remoteUrl = await runGit(["remote", "get-url", remoteTracking.remote], repoRoot)
|
||||
.then((value) => value.trim() || null)
|
||||
.catch(() => null);
|
||||
if (!remoteUrl) return [];
|
||||
.then((value) => {
|
||||
diagnostic.remoteLookup = value.trim() ? "resolved" : "empty";
|
||||
return value.trim() || null;
|
||||
})
|
||||
.catch(() => { diagnostic.remoteLookup = "failed"; return null; });
|
||||
if (!remoteUrl) return { warnings: [], diagnostic };
|
||||
|
||||
const auth = resolveGitAuth ? await resolveGitAuth(remoteUrl).catch(() => null) : null;
|
||||
const auth = resolveGitAuth ? await resolveGitAuth(remoteUrl).then((value) => {
|
||||
diagnostic.authLookup = value ? "resolved" : "unavailable";
|
||||
return value;
|
||||
}).catch(() => { diagnostic.authLookup = "failed"; return null; }) : null;
|
||||
try {
|
||||
await runGit([
|
||||
...(auth?.configArgs ?? []),
|
||||
@@ -987,24 +1010,52 @@ export async function refreshRemoteTrackingBaseRef(
|
||||
"--prune",
|
||||
remoteTracking.remote,
|
||||
`+refs/heads/${remoteTracking.branch}:refs/remotes/${remoteTracking.remote}/${remoteTracking.branch}`,
|
||||
], repoRoot, auth ? { env: { ...process.env, ...auth.env } } : undefined);
|
||||
return [];
|
||||
], repoRoot, {
|
||||
...(auth ? { env: { ...process.env, ...auth.env } } : {}),
|
||||
observe: (proc) => {
|
||||
diagnostic.fetch = proc.code === 0 ? "succeeded" : "failed";
|
||||
if (proc.code !== null) diagnostic.fetchExitCode = proc.code;
|
||||
if (proc.code === 0) return;
|
||||
diagnostic.fetchFailureKind = "unknown";
|
||||
// Diagnostic parsing must not add unbounded work to an already failed
|
||||
// fetch. Keep the existing captured warning/output limits unchanged.
|
||||
if (proc.stdoutTruncated || proc.stderrTruncated || proc.stderrBytes > 8192 || proc.stdoutBytes > 8192) return;
|
||||
// Reuse only the closed transport diagnostic parser. This is not a
|
||||
// clone failure marker and never changes reporting or ref validation.
|
||||
const failure = Object.assign(new Error(), { code: proc.code, stderr: proc.stderr });
|
||||
diagnostic.fetchFailureKind = classifyGitCloneFailure(remoteUrl, failure)?.reason ?? "unknown";
|
||||
if (proc.code === 128 && !proc.stdout.trim() &&
|
||||
proc.stderr.trim() === `fatal: couldn't find remote ref refs/heads/${remoteTracking.branch}`) {
|
||||
diagnostic.fetchFailureKind = "remote_ref_not_found";
|
||||
}
|
||||
},
|
||||
});
|
||||
return { warnings: [], diagnostic };
|
||||
} catch (error) {
|
||||
diagnostic.fetch = "failed";
|
||||
diagnostic.fetchFailureKind ??= "unknown";
|
||||
const rawMessage = error instanceof Error ? error.message : String(error);
|
||||
// Mask URL userinfo (any scheme) and whole URL query strings before the message rides
|
||||
// warnings that reach run logs.
|
||||
// Keep the existing warning contract; new diagnostics never retain this text.
|
||||
const message = rawMessage
|
||||
.replace(/([a-z][a-z0-9+.-]*:\/\/)[^/@\s]+@/gi, "$1***@")
|
||||
.replace(/([a-z][a-z0-9+.-]*:\/\/[^\s"'?]*)\?[^\s"']*/gi, "$1?***");
|
||||
const authNote = auth
|
||||
? ` The fetch authenticated with ${auth.secretName ? `the ${auth.secretName} company-secret GitHub credential` : "the server-environment GitHub credential"}, which may have been rejected.`
|
||||
: "";
|
||||
return [`Could not refresh base ref ${baseRef} before preparing the execution workspace: ${message}${authNote}`];
|
||||
return { warnings: [`Could not refresh base ref ${baseRef} before preparing the execution workspace: ${message}${authNote}`], diagnostic };
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveBaseRefSha(repoRoot: string, baseRef: string): Promise<string | null> {
|
||||
return await runGit(["rev-parse", "--verify", `${baseRef}^{commit}`], repoRoot).catch(() => null);
|
||||
async function resolveBaseRefSha(repoRoot: string, baseRef: string, diagnostic?: WorkspaceBaseRefDiagnostic): Promise<string | null> {
|
||||
return await runGit(["rev-parse", "--verify", `${baseRef}^{commit}`], repoRoot, diagnostic ? {
|
||||
observe: (proc) => {
|
||||
diagnostic.refResolution = proc.code === 0 ? "succeeded" : "failed";
|
||||
if (proc.code !== null) diagnostic.refExitCode = proc.code;
|
||||
},
|
||||
} : undefined).catch(() => {
|
||||
if (diagnostic && !diagnostic.refResolution) diagnostic.refResolution = "spawn_failed";
|
||||
return null;
|
||||
});
|
||||
}
|
||||
|
||||
function readRecordedBaseRefSha(metadata: Record<string, unknown> | null | undefined): string | null {
|
||||
@@ -2413,6 +2464,13 @@ export class UnresolvedWorkspaceBaseRefError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
const unresolvedBaseRefDiagnostics = new WeakMap<Error, WorkspaceBaseRefDiagnostic>();
|
||||
|
||||
/** Reads only the actual resolver's private receipt, not similarly named fields. */
|
||||
export function readUnresolvedWorkspaceBaseRefDiagnostic(error: unknown): WorkspaceBaseRefDiagnostic | null {
|
||||
return error instanceof Error ? readWorkspaceBaseRefDiagnostic(unresolvedBaseRefDiagnostics.get(error)) : null;
|
||||
}
|
||||
|
||||
export function isUnresolvedWorkspaceBaseRefError(error: unknown): error is UnresolvedWorkspaceBaseRefError {
|
||||
return error instanceof UnresolvedWorkspaceBaseRefError;
|
||||
}
|
||||
@@ -2432,6 +2490,7 @@ type AuthoritativeBaseRefResolution =
|
||||
attemptedRefs: string[];
|
||||
warnings: string[];
|
||||
fetchError: string | null;
|
||||
diagnostic: WorkspaceBaseRefDiagnostic;
|
||||
};
|
||||
|
||||
// Resolve the authoritative base ref for a fresh worktree. A configured local
|
||||
@@ -2474,9 +2533,9 @@ async function resolveAuthoritativeBaseRef(
|
||||
return { resolved: true, baseRef: configured, warnings, refreshed: false };
|
||||
}
|
||||
if (remoteTracking && await remoteExists(repoRoot, remoteTracking.remote)) {
|
||||
const fetchWarnings = await refreshRemoteTrackingBaseRef(repoRoot, configured, resolveGitAuth);
|
||||
const { warnings: fetchWarnings, diagnostic } = await refreshRemoteTrackingBaseRefWithDiagnostic(repoRoot, configured, resolveGitAuth);
|
||||
warnings.push(...fetchWarnings);
|
||||
if (await resolveBaseRefSha(repoRoot, configured)) {
|
||||
if (await resolveBaseRefSha(repoRoot, configured, diagnostic)) {
|
||||
return { resolved: true, baseRef: configured, warnings, refreshed: true };
|
||||
}
|
||||
// Build the recovery identity from the parsed remote and branch. The raw
|
||||
@@ -2491,6 +2550,7 @@ async function resolveAuthoritativeBaseRef(
|
||||
attemptedRefs: [configured],
|
||||
warnings,
|
||||
fetchError: fetchWarnings[0] ?? null,
|
||||
diagnostic,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2518,9 +2578,9 @@ async function resolveAuthoritativeBaseRef(
|
||||
return { resolved: true, baseRef: configured, warnings, refreshed: false };
|
||||
}
|
||||
const remoteCandidate = `origin/${configured}`;
|
||||
const fetchWarnings = await refreshRemoteTrackingBaseRef(repoRoot, remoteCandidate, resolveGitAuth);
|
||||
const { warnings: fetchWarnings, diagnostic } = await refreshRemoteTrackingBaseRefWithDiagnostic(repoRoot, remoteCandidate, resolveGitAuth);
|
||||
warnings.push(...fetchWarnings);
|
||||
if (await resolveBaseRefSha(repoRoot, remoteCandidate)) {
|
||||
if (await resolveBaseRefSha(repoRoot, remoteCandidate, diagnostic)) {
|
||||
return { resolved: true, baseRef: remoteCandidate, warnings, refreshed: true };
|
||||
}
|
||||
return {
|
||||
@@ -2530,6 +2590,7 @@ async function resolveAuthoritativeBaseRef(
|
||||
attemptedRefs: [remoteCandidate],
|
||||
warnings,
|
||||
fetchError: fetchWarnings[0] ?? null,
|
||||
diagnostic,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -3574,12 +3635,14 @@ export async function realizeExecutionWorkspace(input: {
|
||||
// `fatal: invalid reference`. Stop here instead and raise a pre-dispatch
|
||||
// configuration failure that the setup catch routes to a human owner.
|
||||
if (!baseRefResolution.resolved) {
|
||||
throw new UnresolvedWorkspaceBaseRefError({
|
||||
const error = new UnresolvedWorkspaceBaseRefError({
|
||||
requestedRef: baseRefResolution.requestedRef,
|
||||
recoveryIdentityRef: baseRefResolution.recoveryIdentityRef,
|
||||
attemptedRefs: baseRefResolution.attemptedRefs,
|
||||
fetchError: baseRefResolution.fetchError,
|
||||
});
|
||||
unresolvedBaseRefDiagnostics.set(error, readWorkspaceBaseRefDiagnostic(baseRefResolution.diagnostic)!);
|
||||
throw error;
|
||||
}
|
||||
|
||||
let branchCreatedByRuntime = true;
|
||||
|
||||
Reference in new issue
Block a user