Fix remote-only workspace base refs and pre-adapter retry loops (#11892)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Execution workspaces give each run an isolated directory and a
selected base ref
> - A remote-only base ref can fail before `git worktree add` when the
ref is not local
> - A setup failure before adapter dispatch must block the run without
an agent-only retry
> - This pull request resolves both remote-tracking ref forms and bounds
recovery for the same unresolved ref
> - The benefit is correct workspace setup and no repeated pre-adapter
recovery loop

## Linked Issues or Issue Description

This PR has no existing public issue. It addresses a workspace setup
bug.

**What happened?**

A remote-only base ref could fail before `git worktree add`. A setup
failure before adapter dispatch could also queue an agent-only
missing-comment retry.

**Expected behavior**

Paperclip must resolve `fix/foo` and `origin/fix/foo` before it creates
a worktree. An unresolved ref must create a human-owned configuration
blocker. Paperclip must not queue an agent-only retry when the adapter
never starts.

**Steps to reproduce**

1. Configure an execution workspace with a base ref that exists only on
the remote.
2. Start a run that creates a fresh worktree.
3. Repeat the run with the same unresolved ref.
4. Observe one configuration blocker and no repeated agent-only recovery
action.

**Paperclip version or commit**

`7664e323189bc219d8cbe00433b2e82b682b0504`

**Deployment mode**

Built from source with `pnpm dev`.

**Agent adapter(s) involved**

Not adapter-specific. The failure occurs before adapter dispatch.

**Database mode**

Not database-related.

**Access context**

Both board and agent execution paths can use execution workspaces.

Related public pull request: `Refs #11123`.

## What Changed

- Resolve remote-only base refs with the authenticated fetch helper
before `git worktree add`.
- Support both unqualified refs and remote-tracking refs.
- Raise a `configuration_incomplete` blocker when the requested ref
remains unresolved.
- Suppress missing-comment retries when setup fails before adapter
dispatch.
- Add the requested ref to the recovery fingerprint to bound identical
recovery actions.
- Add focused tests and update the execution semantics document.

## Verification

- `tsc --noEmit` passed for the changed server code.
- Focused Vitest suites passed, including four base-ref tests,
fingerprint deduplication, and pre-adapter retry suppression.
- GitHub Actions must run the full pull request gate.

## Risks

Low risk. The change affects workspace setup before adapter dispatch.
Existing worktree reuse remains unchanged. An unresolved ref now creates
a clear configuration blocker instead of starting an adapter run.

## Model Used

OpenAI GPT-5; exact model ID `gpt-5`; agentic tool use and repository
review.

## 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:
Nicky Leach 2026-08-21 13:09:44 -07:00 committed by GitHub
parent adfbe2d4b9
commit 69590890d4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 673 additions and 47 deletions

View File

@ -152,6 +152,8 @@ Pre-dispatch configuration validation is a distinct gate that runs after ownersh
A configuration-incomplete result is a gate outcome, not a runtime failure. It is one of the active gates that a checkout-time or dispatch-time check can surface instead of starting a run, and it leaves the issue in an explicit waiting state that names the missing binding. Surfacing the blocker keeps the issue healthy under the liveness contract while preventing a run that is guaranteed to fail once it cannot resolve its required secret/env bindings. A dispatched-then-failed run is the wrong shape for missing configuration: the missing binding is a known pre-dispatch condition, so the control plane must surface it as a configuration-incomplete blocker rather than letting the run start and then fail.
An unresolved workspace base ref is another configuration-incomplete condition. A `git_worktree` workspace bases a fresh worktree on a configured base ref. Paperclip first fetches a remote-only ref before dispatch: it maps an unqualified name (for example `fix/foo`) or a remote-tracking name (for example `origin/fix/foo`) to `origin/<branch>`, runs the authenticated fetch, and re-checks the commit. A ref that resolves lets work continue on the resolved commit. A ref that is still unresolvable after the fetch produces a configuration-incomplete blocker that names the requested ref, rather than a dispatched-then-failed run. Because the adapter never started, Paperclip queues no missing-comment retry. The recovery action dedupes by the canonical remote ref (`origin/<branch>`), not the operator spelling. Two equivalent spellings of one remote branch, for example `fix/foo` and `origin/fix/foo`, share one recovery identity, so a repeated failure reuses the active action and does not reset the attempt count or post a second notice. A different remote branch is a distinct blocker. Paperclip resolves the prior recovery action, creates a new action for the new ref, and notifies the operator with the new ref instead of overwriting the active action of the prior ref.
## 6. Parent/Sub-Issue vs Blockers
Paperclip uses two different relationships for different jobs.

View File

@ -3516,6 +3516,19 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
errorCode: "workspace_validation_failed",
});
expect(failedRun?.error).toContain("linked to a project workspace but has no project id");
// The adapter process never started, so no agent could post an issue
// comment. The comment policy is not_applicable and no missing-comment
// retry is queued, which stops a pre-adapter setup failure from looping.
expect(failedRun?.processStartedAt).toBeNull();
expect(failedRun?.issueCommentStatus).toBe("not_applicable");
const missingCommentWakeups = await db
.select()
.from(agentWakeupRequests)
.where(and(
eq(agentWakeupRequests.companyId, companyId),
eq(agentWakeupRequests.reason, "missing_issue_comment"),
));
expect(missingCommentWakeups).toHaveLength(0);
expect(failedRun?.resultJson).toMatchObject({
workspaceValidation: {
reason: "missing_project_id",

View File

@ -331,6 +331,180 @@ describeEmbeddedPostgres("issue recovery actions", () => {
});
});
// Model the production payload: `requestedRef` keeps the operator spelling,
// and the fingerprint carries the canonical remote ref. Two equivalent
// spellings of one remote branch share `identityRef`, so they share one
// fingerprint. A different branch gets a different `identityRef`.
const makeUnresolvedBaseRefRun = (agentId: string, issueId: string) =>
(requestedRef: string, identityRef: string) =>
({
id: randomUUID(),
agentId,
status: "failed",
error: `Configured workspace base ref "${requestedRef}" did not resolve to a commit on origin after an authenticated fetch.`,
errorCode: "configuration_incomplete",
contextSnapshot: { issueId },
livenessState: "needs_followup",
resultJson: {
configurationIncomplete: {
reason: "workspace_base_ref_unresolved",
requestedRef,
attemptedRefs: [identityRef],
fingerprint: `workspace_base_ref:${identityRef}`,
},
},
}) as const;
it("bounds configuration-incomplete recovery by the unresolved base ref fingerprint", async () => {
const { coderId, sourceIssue } = await seedCompany();
const enqueueWakeup = vi.fn(async () => null);
const recovery = recoveryService(db, { enqueueWakeup });
const makeRun = makeUnresolvedBaseRefRun(coderId, sourceIssue.id);
// Two reconciliations with the same unresolved ref reuse one active action.
await recovery.escalateStrandedAssignedIssue({
issue: sourceIssue,
previousStatus: "in_progress",
latestRun: makeRun("fix/foo", "origin/fix/foo"),
recoveryCause: "configuration_incomplete",
});
await recovery.escalateStrandedAssignedIssue({
issue: sourceIssue,
previousStatus: "in_progress",
latestRun: makeRun("fix/foo", "origin/fix/foo"),
recoveryCause: "configuration_incomplete",
});
const actions = await db
.select()
.from(issueRecoveryActions)
.where(eq(issueRecoveryActions.sourceIssueId, sourceIssue.id));
expect(actions).toHaveLength(1);
expect(actions[0]).toMatchObject({
cause: "configuration_incomplete",
status: "active",
attemptCount: 2,
});
// The fingerprint carries the canonical remote ref, so the same branch stays
// one action and a different branch would make a distinct fingerprint.
expect(actions[0]?.fingerprint).toBe(
`source_scoped_recovery:${sourceIssue.companyId}:${sourceIssue.id}:configuration_incomplete:workspace_base_ref:origin/fix/foo`,
);
});
it("keeps equivalent spellings of one unresolved base ref under one recovery identity", async () => {
const { coderId, sourceIssue } = await seedCompany();
const enqueueWakeup = vi.fn(async () => null);
const recovery = recoveryService(db, { enqueueWakeup });
const makeRun = makeUnresolvedBaseRefRun(coderId, sourceIssue.id);
// The operator retries the same remote branch under two spellings. Both map
// to the canonical `origin/fix/foo` identity, so recovery must not reset the
// attempt count or post a second notice.
await recovery.escalateStrandedAssignedIssue({
issue: sourceIssue,
previousStatus: "in_progress",
latestRun: makeRun("fix/foo", "origin/fix/foo"),
recoveryCause: "configuration_incomplete",
});
await recovery.escalateStrandedAssignedIssue({
issue: sourceIssue,
previousStatus: "in_progress",
latestRun: makeRun("origin/fix/foo", "origin/fix/foo"),
recoveryCause: "configuration_incomplete",
});
const actions = await db
.select()
.from(issueRecoveryActions)
.where(eq(issueRecoveryActions.sourceIssueId, sourceIssue.id));
// One identity, one active action, the attempt count advances.
expect(actions).toHaveLength(1);
expect(actions[0]).toMatchObject({
cause: "configuration_incomplete",
status: "active",
attemptCount: 2,
});
expect(actions[0]?.fingerprint).toBe(
`source_scoped_recovery:${sourceIssue.companyId}:${sourceIssue.id}:configuration_incomplete:workspace_base_ref:origin/fix/foo`,
);
// The operator gets one notice, bound to the one action.
const notices = await db
.select({ metadata: issueComments.metadata })
.from(issueComments)
.where(
and(
eq(issueComments.issueId, sourceIssue.id),
eq(issueComments.authorType, "system"),
),
);
expect(
notices.filter((row) =>
noticeMetadataReferencesRecoveryAction(row.metadata, actions[0]!.id),
),
).toHaveLength(1);
});
it("gives a distinct recovery identity and a new operator notice when the unresolved base ref changes", async () => {
const { coderId, sourceIssue } = await seedCompany();
const enqueueWakeup = vi.fn(async () => null);
const recovery = recoveryService(db, { enqueueWakeup });
const makeRun = makeUnresolvedBaseRefRun(coderId, sourceIssue.id);
await recovery.escalateStrandedAssignedIssue({
issue: sourceIssue,
previousStatus: "in_progress",
latestRun: makeRun("fix/foo", "origin/fix/foo"),
recoveryCause: "configuration_incomplete",
});
await recovery.escalateStrandedAssignedIssue({
issue: sourceIssue,
previousStatus: "in_progress",
latestRun: makeRun("fix/bar", "origin/fix/bar"),
recoveryCause: "configuration_incomplete",
});
const actions = await db
.select()
.from(issueRecoveryActions)
.where(eq(issueRecoveryActions.sourceIssueId, sourceIssue.id));
// The prior ref keeps its own record and the new ref gets a fresh identity.
expect(actions).toHaveLength(2);
const priorAction = actions.find((row) =>
row.fingerprint.endsWith("workspace_base_ref:origin/fix/foo"),
);
const newAction = actions.find((row) =>
row.fingerprint.endsWith("workspace_base_ref:origin/fix/bar"),
);
expect(priorAction?.status).toBe("cancelled");
expect(priorAction?.outcome).toBe("cancelled");
expect(newAction?.status).toBe("active");
expect(newAction?.attemptCount).toBe(1);
expect(newAction?.id).not.toBe(priorAction?.id);
// The operator gets one notice per distinct ref, each bound to its action.
const systemComments = await db
.select({ metadata: issueComments.metadata })
.from(issueComments)
.where(
and(
eq(issueComments.issueId, sourceIssue.id),
eq(issueComments.authorType, "system"),
),
);
expect(
systemComments.some((row) =>
noticeMetadataReferencesRecoveryAction(row.metadata, priorAction!.id),
),
).toBe(true);
expect(
systemComments.some((row) =>
noticeMetadataReferencesRecoveryAction(row.metadata, newAction!.id),
),
).toBe(true);
});
it.each([
["process_lost", undefined, "coder"],
["adapter_failed", "successful_run_missing_state", "coder"],

View File

@ -262,7 +262,10 @@ describe("issue update comment wakeups", () => {
});
expect(res.status).toBe(200);
expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1);
// The route dispatches the wake after it sends the response, so wait for
// the fire-and-forget dispatch to settle. This keeps the wake inside this
// test and stops it from leaking into the next test as an extra call.
await vi.waitFor(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(1));
expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith(
ASSIGNEE_AGENT_ID,
expect.objectContaining({

View File

@ -36,6 +36,7 @@ import {
realizeExecutionWorkspace,
refreshRemoteTrackingBaseRef,
releaseRuntimeServicesForRun,
UnresolvedWorkspaceBaseRefError,
resetRuntimeServicesForTests,
resolveRuntimeProvisionCommand,
resolveWorkspaceRuntimeReadinessTimeoutSec,
@ -251,6 +252,26 @@ async function advanceRemoteMaster(sourceRepo: string, remotePath: string, fileN
return readGit(sourceRepo, ["rev-parse", "master"]);
}
// Push a branch to the bare remote without leaving a local ref or a
// remote-tracking ref in `repoRoot`. This reproduces a remote-only feature
// branch: the clone was made before the push, so `repoRoot` learns the branch
// only after an authenticated fetch of `origin/<branch>`.
async function pushRemoteOnlyBranch(
sourceRepo: string,
remotePath: string,
branch: string,
fileName: string,
) {
await runGit(sourceRepo, ["checkout", "-B", branch]);
await fs.writeFile(path.join(sourceRepo, fileName), `${fileName}\n`, "utf8");
await runGit(sourceRepo, ["add", fileName]);
await runGit(sourceRepo, ["commit", "-m", `Add ${fileName}`]);
await runGit(sourceRepo, ["push", remotePath, branch]);
const sha = await readGit(sourceRepo, ["rev-parse", branch]);
await runGit(sourceRepo, ["checkout", "master"]);
return sha;
}
function realizeWorktreeForTest(repoRoot: string, repoRef: string | null) {
return realizeExecutionWorkspace({
base: {
@ -1018,6 +1039,111 @@ describe("realizeExecutionWorkspace", () => {
]);
});
it("bases a fresh worktree on a remote-only branch supplied as fix/foo", async () => {
const { sourceRepo, remotePath, repoRoot } = await createClonedRepoWithRemote();
const remoteSha = await pushRemoteOnlyBranch(sourceRepo, remotePath, "fix/foo", "remote-only.txt");
// The clone never learned the branch: no local ref and no remote-tracking ref.
await expect(readGit(repoRoot, ["rev-parse", "--verify", "fix/foo"])).rejects.toThrow();
await expect(readGit(repoRoot, ["rev-parse", "--verify", "origin/fix/foo"])).rejects.toThrow();
const workspace = await realizeWorktreeForTest(repoRoot, "fix/foo");
expect(workspace.created).toBe(true);
expect(workspace.repoRef).toBe("origin/fix/foo");
expect(workspace.baseRefSha).toBe(remoteSha);
expect(await readGit(workspace.cwd, ["rev-parse", "HEAD"])).toBe(remoteSha);
});
it("bases a fresh worktree on a remote-only branch supplied as origin/fix/foo", async () => {
const { sourceRepo, remotePath, repoRoot } = await createClonedRepoWithRemote();
const remoteSha = await pushRemoteOnlyBranch(sourceRepo, remotePath, "fix/bar", "remote-only.txt");
await expect(readGit(repoRoot, ["rev-parse", "--verify", "origin/fix/bar"])).rejects.toThrow();
const workspace = await realizeWorktreeForTest(repoRoot, "origin/fix/bar");
expect(workspace.created).toBe(true);
expect(workspace.repoRef).toBe("origin/fix/bar");
expect(workspace.baseRefSha).toBe(remoteSha);
expect(await readGit(workspace.cwd, ["rev-parse", "HEAD"])).toBe(remoteSha);
});
it("stops before git worktree add when the base ref is absent on origin", async () => {
const { repoRoot } = await createClonedRepoWithRemote();
const error = await realizeWorktreeForTest(repoRoot, "fix/does-not-exist").then(
() => null,
(caught: unknown) => caught,
);
expect(error).toBeInstanceOf(UnresolvedWorkspaceBaseRefError);
const unresolved = error as UnresolvedWorkspaceBaseRefError;
expect(unresolved.requestedRef).toBe("fix/does-not-exist");
expect(unresolved.recoveryIdentityRef).toBe("origin/fix/does-not-exist");
expect(unresolved.attemptedRefs).toEqual(["origin/fix/does-not-exist"]);
// No worktree directory was created for the fresh-create path.
await expect(
fs.stat(path.join(repoRoot, ".paperclip", "worktrees", "PAP-447-add-worktree-support")),
).rejects.toThrow();
});
it("gives equivalent spellings of one absent remote ref the same recovery identity", async () => {
const { repoRoot } = await createClonedRepoWithRemote();
// The unqualified form and the remote-tracking form name the same remote
// branch. Both must map to one `recoveryIdentityRef`, so recovery does not
// treat a spelling change as a new blocker.
const unqualified = await realizeWorktreeForTest(repoRoot, "fix/absent").then(
() => null,
(caught: unknown) => caught,
);
const remoteTracking = await realizeWorktreeForTest(repoRoot, "origin/fix/absent").then(
() => null,
(caught: unknown) => caught,
);
// The full remote-tracking spelling names the same branch as well. It must
// map to the same canonical recovery identity, not to its raw spelling.
const fullRemoteTracking = await realizeWorktreeForTest(repoRoot, "refs/remotes/origin/fix/absent").then(
() => null,
(caught: unknown) => caught,
);
expect(unqualified).toBeInstanceOf(UnresolvedWorkspaceBaseRefError);
expect(remoteTracking).toBeInstanceOf(UnresolvedWorkspaceBaseRefError);
expect(fullRemoteTracking).toBeInstanceOf(UnresolvedWorkspaceBaseRefError);
const unqualifiedError = unqualified as UnresolvedWorkspaceBaseRefError;
const remoteTrackingError = remoteTracking as UnresolvedWorkspaceBaseRefError;
const fullRemoteTrackingError = fullRemoteTracking as UnresolvedWorkspaceBaseRefError;
// Each error keeps its own operator spelling for the human notice.
expect(unqualifiedError.requestedRef).toBe("fix/absent");
expect(remoteTrackingError.requestedRef).toBe("origin/fix/absent");
expect(fullRemoteTrackingError.requestedRef).toBe("refs/remotes/origin/fix/absent");
// All three share one canonical recovery identity.
expect(unqualifiedError.recoveryIdentityRef).toBe("origin/fix/absent");
expect(remoteTrackingError.recoveryIdentityRef).toBe("origin/fix/absent");
expect(fullRemoteTrackingError.recoveryIdentityRef).toBe("origin/fix/absent");
});
it("surfaces an authenticated fetch failure as an unresolved base ref, not a crash", async () => {
const { repoRoot } = await createClonedRepoWithRemote();
// Point origin at a path that no repository backs. The authenticated fetch
// fails, so the ref never resolves and the resolver reports the fetch error.
await runGit(repoRoot, ["remote", "set-url", "origin", path.join(os.tmpdir(), "paperclip-missing-remote.git")]);
const error = await realizeWorktreeForTest(repoRoot, "fix/unreachable").then(
() => null,
(caught: unknown) => caught,
);
expect(error).toBeInstanceOf(UnresolvedWorkspaceBaseRefError);
const unresolved = error as UnresolvedWorkspaceBaseRefError;
expect(unresolved.requestedRef).toBe("fix/unreachable");
expect(unresolved.fetchError).toEqual(
expect.stringContaining("Could not refresh base ref origin/fix/unreachable"),
);
});
it("rejects reusing an empty directory that only looks like a worktree because it sits inside the repo", async () => {
const repoRoot = await createTempRepo();
const branchName = "PAP-447-add-worktree-support";
@ -7477,14 +7603,26 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => {
process.env.PAPERCLIP_HOME = paperclipHome;
process.env.PAPERCLIP_INSTANCE_ID = `runtime-pnpm-reconcile-${randomUUID()}`;
const portProbe = net.createServer();
await new Promise<void>((resolve) => portProbe.listen(0, "127.0.0.1", resolve));
const address = portProbe.address();
const port = typeof address === "object" && address ? address.port : null;
await new Promise<void>((resolve, reject) => {
portProbe.close((error) => error ? reject(error) : resolve());
});
if (!port) throw new Error("Failed to reserve pnpm reconciliation test port");
// Reserve a port outside the runtime exposure app-port range (42000-42999).
// The reconciler stores this port on the row. A port inside that range makes
// the reconciler treat the row as an exposure reservation and report drift,
// so the live service never reaches the adoption path this test verifies.
const reservePort = async () => {
for (let attempt = 0; attempt < 100; attempt += 1) {
const probe = net.createServer();
await new Promise<void>((resolve) => probe.listen(0, "127.0.0.1", resolve));
const address = probe.address();
const candidate = typeof address === "object" && address ? address.port : null;
await new Promise<void>((resolve, reject) => {
probe.close((error) => error ? reject(error) : resolve());
});
if (candidate && candidate <= 55_535 && (candidate < 42_000 || candidate > 42_999)) {
return candidate;
}
}
throw new Error("Failed to reserve pnpm reconciliation test port outside the broker range");
};
const port = await reservePort();
const companyId = randomUUID();
const projectId = randomUUID();

View File

@ -142,9 +142,11 @@ import {
persistAdapterManagedRuntimeServices,
realizeExecutionWorkspace,
releaseRuntimeServicesForRun,
isUnresolvedWorkspaceBaseRefError,
type ExecutionWorkspaceInput,
type RealizedExecutionWorkspace,
type RuntimeServiceRef,
type UnresolvedWorkspaceBaseRefError,
sanitizeRuntimeServiceBaseEnv,
} from "./workspace-runtime.js";
import {
@ -459,6 +461,14 @@ const WORKSPACE_VALIDATION_FAILURE_CODE = "workspace_validation_failed";
const WORKSPACE_VALIDATION_RECOVERY_CAUSE = "workspace_validation_failed";
const CONFIGURATION_INCOMPLETE_FAILURE_CODE = "configuration_incomplete";
const CONFIGURATION_INCOMPLETE_RECOVERY_CAUSE = "configuration_incomplete";
// Error codes that mark a pre-dispatch setup failure. The adapter process never
// started, so no agent could post an issue comment. The setup catch writes one
// of these codes when a failure happens before `adapter.execute`.
const PRE_ADAPTER_SETUP_FAILURE_CODES = new Set<string>([
"setup_failed",
CONFIGURATION_INCOMPLETE_FAILURE_CODE,
WORKSPACE_VALIDATION_FAILURE_CODE,
]);
const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_RETRY_REASON = "execution_review_participant_recovery";
const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_WAKE_REASON = "execution_review_participant_recovery";
const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_CAUSE = "execution_review_participant_recovery";
@ -542,6 +552,35 @@ export class ConfigurationIncompleteFailure extends Error {
}
}
// Build the configuration-incomplete result payload for a workspace base ref
// that never resolved to a commit. The setup catch maps this to errorCode
// `configuration_incomplete`, so the recovery path routes it to a human owner
// instead of a dispatched-then-failed run. The `fingerprint` uses the canonical
// remote ref, not the operator spelling. Two equivalent spellings of one remote
// branch (`fix/foo` and `origin/fix/foo`) share one fingerprint, so a repeated
// failure reuses one active recovery action and does not reset the attempt
// count or post a duplicate notice. A different branch makes a new action.
function buildUnresolvedWorkspaceBaseRefResultJson(
run: typeof heartbeatRuns.$inferSelect,
error: UnresolvedWorkspaceBaseRefError,
): Record<string, unknown> {
const context = parseObject(run.contextSnapshot);
return {
configurationIncomplete: {
reason: "workspace_base_ref_unresolved",
companyId: run.companyId,
agentId: run.agentId,
issueId: readNonEmptyString(context.issueId) ?? null,
projectId: readNonEmptyString(context.projectId) ?? null,
requestedRef: error.requestedRef,
attemptedRefs: error.attemptedRefs,
fetchError: error.fetchError,
fingerprint: `workspace_base_ref:${error.recoveryIdentityRef}`,
missingBindings: [],
},
};
}
export interface SharedWorkspaceHolder {
runId: string;
agentId: string;
@ -10074,6 +10113,22 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
return { outcome: "not_applicable" as const, queuedRun: null };
}
// A pre-dispatch setup failure means the adapter process never started (for
// example an unresolved workspace base ref). No agent could run, so no agent
// could post an issue comment. A missing-comment retry cannot help and would
// loop the identical pre-adapter failure, so mark the policy not_applicable
// and queue nothing.
if (run.errorCode != null && PRE_ADAPTER_SETUP_FAILURE_CODES.has(run.errorCode)) {
if (run.issueCommentStatus !== "not_applicable") {
await patchRunIssueCommentStatus(run.id, {
issueCommentStatus: "not_applicable",
issueCommentSatisfiedByCommentId: null,
issueCommentRetryQueuedAt: null,
});
}
return { outcome: "not_applicable" as const, queuedRun: null };
}
const postedComment = await findRunIssueComment(run.id, run.companyId, issueId);
if (postedComment) {
await patchRunIssueCommentStatus(run.id, {
@ -16677,11 +16732,17 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
// recovery path routes it to a human owner instead of looping retries.
const workspaceValidationSetupFailure = isWorkspaceValidationFailure(outerErr) ? outerErr : null;
const configurationIncompleteSetupFailure = isConfigurationIncompleteFailure(outerErr) ? outerErr : null;
// A remote-only base ref that never resolved is a known pre-dispatch
// configuration gap, not an opaque setup crash. Map it to the same
// configuration-incomplete code so the recovery path routes it to a
// human owner and bounds the repeat by its per-ref fingerprint.
const unresolvedBaseRefSetupFailure = isUnresolvedWorkspaceBaseRefError(outerErr) ? outerErr : null;
const recordedResponsibleUserDenialCode =
normalizeResponsibleUserDenialCode((await getRun(runId).catch(() => null))?.errorCode);
const setupFailureErrorCode =
workspaceValidationSetupFailure?.code ??
configurationIncompleteSetupFailure?.code ??
(unresolvedBaseRefSetupFailure ? CONFIGURATION_INCOMPLETE_FAILURE_CODE : null) ??
recordedResponsibleUserDenialCode ??
"setup_failed";
logger.error({ err: outerErr, runId }, "heartbeat execution setup failed");
@ -16695,7 +16756,11 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {})
errorCode: setupFailureErrorCode,
errorMessage: message,
resultJson:
workspaceValidationSetupFailure?.resultJson ?? configurationIncompleteSetupFailure?.resultJson ?? null,
workspaceValidationSetupFailure?.resultJson ??
configurationIncompleteSetupFailure?.resultJson ??
(unresolvedBaseRefSetupFailure
? buildUnresolvedWorkspaceBaseRefResultJson(run, unresolvedBaseRefSetupFailure)
: null),
}),
} : {}),
}).catch(() => ({ run: null, updated: false as const }));

View File

@ -36,6 +36,11 @@ export type UpsertIssueRecoveryActionInput = {
timeoutAt?: Date | null;
lastAttemptAt?: Date | null;
attemptCount?: number;
// When true, a change of (cause, fingerprint) does not overwrite the active
// action in place. The service resolves the prior action and inserts a new
// one. The new failure then gets a distinct recovery identity and a fresh
// operator notice, and the prior identity stays as a resolved record.
supersedeOnIdentityChange?: boolean;
};
export type ResolveIssueRecoveryActionInput = {
@ -179,6 +184,81 @@ export function issueRecoveryActionService(db: Db) {
return upsertSourceScopedUnlocked(input, retryCount + 1);
}
function buildInsertValues(
input: UpsertIssueRecoveryActionInput,
ownerType: IssueRecoveryActionOwnerType,
now: Date,
) {
return {
companyId: input.companyId,
sourceIssueId: input.sourceIssueId,
recoveryIssueId: input.recoveryIssueId ?? null,
kind: input.kind,
status: "active" as const,
ownerType,
ownerAgentId: input.ownerAgentId ?? null,
ownerUserId: input.ownerUserId ?? null,
previousOwnerAgentId: input.previousOwnerAgentId ?? null,
returnOwnerAgentId: input.returnOwnerAgentId ?? null,
cause: input.cause,
fingerprint: input.fingerprint,
evidence: input.evidence ?? {},
nextAction: input.nextAction,
wakePolicy: input.wakePolicy ?? null,
monitorPolicy: input.monitorPolicy ?? null,
attemptCount: input.attemptCount ?? 1,
maxAttempts: input.maxAttempts ?? null,
timeoutAt: input.timeoutAt ?? null,
lastAttemptAt: input.lastAttemptAt ?? now,
};
}
// Resolve the prior active action, then insert a new one in one transaction.
// The prior identity stays as a cancelled record and the new failure gets a
// fresh action row with its own id. The partial unique index on the active
// status stays satisfied because only the new row is active at commit.
async function supersedePriorAndInsert(
input: UpsertIssueRecoveryActionInput,
priorActionId: string,
ownerType: IssueRecoveryActionOwnerType,
now: Date,
retryCount: number,
): Promise<IssueRecoveryAction> {
try {
const created = await db.transaction(async (tx) => {
const [superseded] = await tx
.update(issueRecoveryActions)
.set({
status: "cancelled",
outcome: "cancelled",
resolutionNote: "A new failure with a different identity superseded this recovery action.",
resolvedAt: now,
updatedAt: now,
})
.where(
and(
eq(issueRecoveryActions.id, priorActionId),
inArray(issueRecoveryActions.status, [...ACTIVE_RECOVERY_ACTION_STATUSES]),
),
)
.returning();
// Another writer resolved the prior action first. Abort and retry the
// whole upsert so the retry reads the current active state.
if (!superseded) return null;
const [row] = await tx
.insert(issueRecoveryActions)
.values(buildInsertValues(input, ownerType, now))
.returning();
return row ?? null;
});
if (!created) return retryUpsertSourceScoped(input, retryCount);
return toReadModel(created);
} catch (error) {
if (!isUniqueRecoveryActionConflict(error)) throw error;
return retryUpsertSourceScoped(input, retryCount, error);
}
}
async function upsertSourceScopedUnlocked(
input: UpsertIssueRecoveryActionInput,
retryCount = 0,
@ -187,6 +267,15 @@ export function issueRecoveryActionService(db: Db) {
const now = new Date();
const ownerType = input.ownerType ?? (input.ownerAgentId ? "agent" : "board");
if (existing) {
// A distinct failure identity must not overwrite the active action of a
// prior identity. Resolve the prior action and insert a new one, so the
// operator gets a new notice for the new failure.
if (
input.supersedeOnIdentityChange &&
(existing.cause !== input.cause || existing.fingerprint !== input.fingerprint)
) {
return supersedePriorAndInsert(input, existing.id, ownerType, now, retryCount);
}
const [updated] = await db
.update(issueRecoveryActions)
.set({
@ -229,28 +318,7 @@ export function issueRecoveryActionService(db: Db) {
try {
const [created] = await db
.insert(issueRecoveryActions)
.values({
companyId: input.companyId,
sourceIssueId: input.sourceIssueId,
recoveryIssueId: input.recoveryIssueId ?? null,
kind: input.kind,
status: "active",
ownerType,
ownerAgentId: input.ownerAgentId ?? null,
ownerUserId: input.ownerUserId ?? null,
previousOwnerAgentId: input.previousOwnerAgentId ?? null,
returnOwnerAgentId: input.returnOwnerAgentId ?? null,
cause: input.cause,
fingerprint: input.fingerprint,
evidence: input.evidence ?? {},
nextAction: input.nextAction,
wakePolicy: input.wakePolicy ?? null,
monitorPolicy: input.monitorPolicy ?? null,
attemptCount: input.attemptCount ?? 1,
maxAttempts: input.maxAttempts ?? null,
timeoutAt: input.timeoutAt ?? null,
lastAttemptAt: input.lastAttemptAt ?? now,
})
.values(buildInsertValues(input, ownerType, now))
.returning();
return toReadModel(created!);
} catch (error) {

View File

@ -300,6 +300,11 @@ function readWorkspaceValidationFingerprint(latestRun: LatestIssueRun): string |
return readNonEmptyString(payload?.fingerprint);
}
function readConfigurationIncompleteFingerprint(latestRun: LatestIssueRun): string | null {
const payload = parseObject(parseObject(latestRun?.resultJson).configurationIncomplete);
return readNonEmptyString(payload?.fingerprint);
}
type WatchdogDecisionActor =
| { type: "board"; userId?: string | null; runId?: string | null }
| { type: "agent"; agentId?: string | null; runId?: string | null }
@ -2867,6 +2872,23 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
].join(":");
}
}
// A configuration-incomplete failure that carries a stable identity (for
// example an unresolved workspace base ref) dedupes per that identity, so a
// different requested ref makes a new recovery action while the same ref
// reuses one. Configuration gaps with no fingerprint fall back to the
// issue-and-cause scope below.
if (input.recoveryCause === "configuration_incomplete") {
const configurationFingerprint = readConfigurationIncompleteFingerprint(input.latestRun);
if (configurationFingerprint) {
return [
"source_scoped_recovery",
input.issue.companyId,
input.issue.id,
input.recoveryCause,
configurationFingerprint,
].join(":");
}
}
return [
"source_scoped_recovery",
input.issue.companyId,
@ -2925,6 +2947,11 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
const action = await recoveryActionsSvc.upsertSourceScoped({
companyId: input.issue.companyId,
sourceIssueId: input.issue.id,
// A configuration-incomplete failure carries a per-identity fingerprint
// (for example the unresolved workspace base ref). A different ref is a
// distinct blocker, so it must get a new recovery action and notify the
// operator, not overwrite the active action of the prior ref.
supersedeOnIdentityChange: recoveryCause === "configuration_incomplete",
kind: strandedRecoveryActionKind(recoveryCause),
ownerType: recoveryCause === "provider_quota" && !ownerAgentId ? "system" : ownerAgentId ? "agent" : "board",
ownerAgentId,

View File

@ -2364,26 +2364,116 @@ export async function ensureGitWorktreeBranchCoherent(input: {
};
}
// A configured base ref that does not resolve to a commit, even after an
// authenticated fetch of its `origin/<branch>` counterpart. The caller must
// stop before `git worktree add` and raise a pre-dispatch configuration
// failure. `requestedRef` keeps the operator spelling for the human notice.
// `recoveryIdentityRef` is the canonical remote ref the resolver probed, so two
// equivalent spellings of one remote branch map to one recovery identity.
// `attemptedRefs` names each ref the resolver tried, and `fetchError` carries
// the first fetch warning (masked) when the fetch itself failed.
export class UnresolvedWorkspaceBaseRefError extends Error {
requestedRef: string;
recoveryIdentityRef: string;
attemptedRefs: string[];
fetchError: string | null;
constructor(input: {
requestedRef: string;
recoveryIdentityRef: string;
attemptedRefs: string[];
fetchError?: string | null;
}) {
super(
`Configured workspace base ref "${input.requestedRef}" did not resolve to a commit on origin after an authenticated fetch.`,
);
this.name = "UnresolvedWorkspaceBaseRefError";
this.requestedRef = input.requestedRef;
this.recoveryIdentityRef = input.recoveryIdentityRef;
this.attemptedRefs = input.attemptedRefs;
this.fetchError = input.fetchError ?? null;
}
}
export function isUnresolvedWorkspaceBaseRefError(error: unknown): error is UnresolvedWorkspaceBaseRefError {
return error instanceof UnresolvedWorkspaceBaseRefError;
}
// A resolved base ref that the caller can pass to `git worktree add`, or an
// unresolved outcome that must stop the caller before it creates the worktree.
type AuthoritativeBaseRefResolution =
| { resolved: true; baseRef: string; warnings: string[]; refreshed: boolean }
| {
resolved: false;
requestedRef: string;
// The canonical remote ref the resolver probed for this branch, for
// example `origin/fix/foo`. Two equivalent spellings of one remote branch
// (`fix/foo` and `origin/fix/foo`) share this value, so recovery treats
// them as one identity. Two different branches get different values.
recoveryIdentityRef: string;
attemptedRefs: string[];
warnings: string[];
fetchError: string | null;
};
// Resolve the authoritative base ref for a fresh worktree. A configured local
// branch is mapped to its `origin/<branch>` counterpart so unpushed local
// divergence never leaks into the task branch; remote-tracking refs, SHAs, and
// tags are used verbatim, and an unset/`HEAD` base falls back to the detected
// default branch (which already prefers `origin/master`).
// divergence never leaks into the task branch; SHAs and tags are used verbatim,
// and an unset/`HEAD` base falls back to the detected default branch (which
// already prefers `origin/master`).
//
// A remote-only feature branch never has a local ref or a remote-tracking ref
// yet. The resolver fetches `origin/<branch>` with the authenticated helper,
// then re-checks the commit. This covers both the unqualified form (`fix/foo`)
// and the remote-tracking form (`origin/fix/foo`). A ref that still does not
// resolve returns `resolved: false`, so the caller stops before the worktree
// add instead of passing an invalid reference to git.
async function resolveAuthoritativeBaseRef(
repoRoot: string,
configuredBaseRef: string | null,
resolveGitAuth?: GitRemoteAuthProvider | null,
): Promise<{ baseRef: string; warnings: string[]; refreshed: boolean }> {
): Promise<AuthoritativeBaseRefResolution> {
const warnings: string[] = [];
const detectOrHead = async () => (await detectDefaultBranch(repoRoot, resolveGitAuth)) ?? "HEAD";
const configured = configuredBaseRef?.trim();
if (!configured || configured === "HEAD") {
return { baseRef: await detectOrHead(), warnings, refreshed: false };
return { resolved: true, baseRef: await detectOrHead(), warnings, refreshed: false };
}
if (parseRemoteTrackingRef(configured)) {
return { baseRef: configured, warnings, refreshed: false };
// A remote-tracking ref supplied directly (for example `origin/fix/foo`).
// Use it verbatim when it already resolves. When it does not, fetch it once
// and re-check, then stop if it is still absent on the remote.
//
// `parseRemoteTrackingRef` only checks the `remote/branch` shape. An
// unqualified branch name that contains a slash (for example `fix/foo`) has
// the same shape but names no real remote, so it is not a remote-tracking
// ref. Gate this branch on the first segment naming an existing remote, and
// let a name like `fix/foo` fall through to the remote-only branch handling
// below.
const remoteTracking = parseRemoteTrackingRef(configured);
if (remoteTracking && await resolveBaseRefSha(repoRoot, configured)) {
return { resolved: true, baseRef: configured, warnings, refreshed: false };
}
if (remoteTracking && await remoteExists(repoRoot, remoteTracking.remote)) {
const fetchWarnings = await refreshRemoteTrackingBaseRef(repoRoot, configured, resolveGitAuth);
warnings.push(...fetchWarnings);
if (await resolveBaseRefSha(repoRoot, configured)) {
return { resolved: true, baseRef: configured, warnings, refreshed: true };
}
// Build the recovery identity from the parsed remote and branch. The raw
// ref and its remote-tracking spelling (`origin/fix/foo` and
// `refs/remotes/origin/fix/foo`) then share one recovery fingerprint, so
// recovery treats them as one identity instead of two.
const canonicalRemoteRef = `${remoteTracking.remote}/${remoteTracking.branch}`;
return {
resolved: false,
requestedRef: configured,
recoveryIdentityRef: canonicalRemoteRef,
attemptedRefs: [configured],
warnings,
fetchError: fetchWarnings[0] ?? null,
};
}
if (await localBranchExists(repoRoot, configured)) {
@ -2392,17 +2482,37 @@ async function resolveAuthoritativeBaseRef(
// the returned ref (see `refreshed`) so we never fetch the same ref twice.
warnings.push(...await refreshRemoteTrackingBaseRef(repoRoot, remoteCandidate, resolveGitAuth));
if (await resolveBaseRefSha(repoRoot, remoteCandidate)) {
return { baseRef: remoteCandidate, warnings, refreshed: true };
return { resolved: true, baseRef: remoteCandidate, warnings, refreshed: true };
}
if (await remoteExists(repoRoot, "origin")) {
warnings.push(
`Configured base ref "${configured}" is a local branch with no matching origin/${configured}; basing the execution workspace on the local ref, which may include unpushed commits.`,
);
}
return { baseRef: configured, warnings, refreshed: false };
return { resolved: true, baseRef: configured, warnings, refreshed: false };
}
return { baseRef: configured, warnings, refreshed: false };
// Fall-through: an unqualified ref (for example `fix/foo`) that is not `HEAD`,
// not a remote-tracking ref, and not a local branch. A full SHA or a tag that
// already resolves stays verbatim. Otherwise treat it as a remote-only branch
// name: fetch `origin/<ref>` and base the worktree on the remote counterpart.
if (await resolveBaseRefSha(repoRoot, configured)) {
return { resolved: true, baseRef: configured, warnings, refreshed: false };
}
const remoteCandidate = `origin/${configured}`;
const fetchWarnings = await refreshRemoteTrackingBaseRef(repoRoot, remoteCandidate, resolveGitAuth);
warnings.push(...fetchWarnings);
if (await resolveBaseRefSha(repoRoot, remoteCandidate)) {
return { resolved: true, baseRef: remoteCandidate, warnings, refreshed: true };
}
return {
resolved: false,
requestedRef: configured,
recoveryIdentityRef: remoteCandidate,
attemptedRefs: [remoteCandidate],
warnings,
fetchError: fetchWarnings[0] ?? null,
};
}
// Auto-refresh a reused worktree to the latest base only when it is provably
@ -3117,11 +3227,15 @@ export async function realizeExecutionWorkspace(input: {
const configuredBaseRef = typeof rawStrategy.baseRef === "string" && rawStrategy.baseRef.length > 0
? rawStrategy.baseRef
: input.base.repoRef ?? null;
const {
baseRef,
warnings: baseRefResolutionWarnings,
refreshed: baseRefAlreadyRefreshed,
} = await resolveAuthoritativeBaseRef(repoRoot, configuredBaseRef, input.resolveGitAuth);
const baseRefResolution = await resolveAuthoritativeBaseRef(repoRoot, configuredBaseRef, input.resolveGitAuth);
// Keep a usable base ref for the reuse and drift paths even when the ref is
// unresolved: those paths tolerate a null base-ref SHA and never run
// `git worktree add -b <branch> <baseRef>`. Only the fresh-create path below
// stops on an unresolved ref. `baseRefAlreadyRefreshed` is true for the
// unresolved case because the resolver already attempted the fetch.
const baseRef = baseRefResolution.resolved ? baseRefResolution.baseRef : baseRefResolution.requestedRef;
const baseRefResolutionWarnings = baseRefResolution.warnings;
const baseRefAlreadyRefreshed = baseRefResolution.resolved ? baseRefResolution.refreshed : true;
const baseRefreshWarnings = [
...baseRefResolutionWarnings,
...(baseRefAlreadyRefreshed ? [] : await refreshRemoteTrackingBaseRef(repoRoot, baseRef, input.resolveGitAuth)),
@ -3257,6 +3371,19 @@ export async function realizeExecutionWorkspace(input: {
throw new Error(`Registered worktree for branch "${branchName}" at "${registeredBranchWorktree}" is not reusable${reason}.`);
}
// No reusable worktree exists, so a fresh `git worktree add -b <branch> <baseRef>`
// must run next. An unresolved base ref would make git fail with
// `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({
requestedRef: baseRefResolution.requestedRef,
recoveryIdentityRef: baseRefResolution.recoveryIdentityRef,
attemptedRefs: baseRefResolution.attemptedRefs,
fetchError: baseRefResolution.fetchError,
});
}
try {
await recordGitOperation(input.recorder, {
phase: "worktree_prepare",

View File

@ -13,6 +13,15 @@ export default defineConfig({
// mirrors it for the same reason.
hookTimeout: 30000,
teardownTimeout: 30000,
// The route/authz suites import very large modules (for example
// src/routes/issues.ts and its dependency graph). The first test in each
// file pays the one-time transform cost inside its own timeout budget. On
// the loaded serial shard (maxWorkers=1) that cost can cross vitest's
// default 5s testTimeout and fail the first test, which also lets its
// fire-and-forget wake leak into the next test. Give each test generous
// headroom; 15s is far above the observed module-load cost yet still
// catches a genuinely hung test well inside the 20 minute job limit.
testTimeout: 15000,
isolate: true,
maxConcurrency: 1,
maxWorkers: 1,