fix(server): keep a successor's issue lock when a finalizing native run releases its checkout (#19156)

`release_checkout` now clears `checkoutRunId` and the execution lock only when they belong to the committing run, so a successor that claimed the issue while its predecessor was finalizing keeps its claim and passes reviewed-chat attestation.

`attestReviewedExternalChatRun` reports the first failed check as a closed label. Setup failures store it in `resultJson.reviewedChatBinding.denialReason`, and Sentry run-failure diagnostics export it as `execution.reviewedChatBindingDenialReason` (no IDs or text). The error message is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Devin FoleyandClaude Opus 5.5 authored and GitHub committed 2026-11-04 17:49:03 -08:00
1 parent 6fa54d5577
commit 7edbc1fc07
9 files changed
+271 -34

No files matched your search

@@ -223,6 +223,22 @@ describe("run failure diagnostics", () => {
expect(collectRunFailureDiagnostics(run({ errorCode: "adapter_failed" }), {}).execution).toEqual({});
});
it("exports only the closed reviewed-chat denial label of a setup failure", () => {
const result = sanitizeRunFailureDiagnostics(collectRunFailureDiagnostics(run({
errorCode: "setup_failed", error: "reviewed_chat_execution_binding_not_authorized",
resultJson: { reviewedChatBinding: { denialReason: "owner_mismatch", runId: "private-run", commentId: "private-comment" } },
}), {}));
expect(result.execution).toEqual({ reviewedChatBindingDenialReason: "owner_mismatch" });
expect(JSON.stringify(result)).not.toContain("private");
for (const value of [null, [], { denialReason: "private-reason" },
Object.defineProperty({}, "denialReason", { get() { throw new Error("private"); } })]) {
expect(collectRunFailureDiagnostics(run({ errorCode: "setup_failed",
resultJson: { reviewedChatBinding: value } }), {}).execution).toEqual({});
}
expect(collectRunFailureDiagnostics(run({ errorCode: "adapter_failed",
resultJson: { reviewedChatBinding: { denialReason: "owner_mismatch" } } }), {}).execution).toEqual({});
});
it("exports the remote recovery stage and reason from an error thrown before the result was saved", () => {
const thrown = Object.assign(new RemoteRunnerRecoveryError({ stage: "process_verification", reason: "process_not_running" }),
{ path: "private-path", pid: 123 });
+9 -2
View File
@@ -407,6 +407,10 @@ import {
emitAgentTaskRunById,
} from "./agent-task-run-telemetry.js";
import { reportRunFailure } from "./run-failure-report.js";
import {
ReviewedChatBindingDeniedError,
type ReviewedChatBindingDenialReason,
} from "./reviewed-chat-binding-diagnostics.js";
import { collectRunFailureSecretValues, type RunFailureReportOptions } from "./run-failure-diagnostics.js";
import { companySkillService } from "./company-skills.js";
@@ -2241,6 +2245,7 @@ export function heartbeatService(
context.interactionKind === "ask_user_questions" &&
["in_progress", "in_review"].includes(issueContext?.status ?? "")))
) {
let denialReason: ReviewedChatBindingDenialReason | null = null;
const attested = await attestReviewedExternalChatRun({
db,
companyId: agent.companyId,
@@ -2251,9 +2256,11 @@ export function heartbeatService(
onQuestionResponseAttested: (answeredAtMs) => {
attestedQuestionResponseAtMs = answeredAtMs;
},
onDenied: (reason) => {
denialReason = reason;
},
});
if (!attested)
throw new Error("reviewed_chat_execution_binding_not_authorized");
if (!attested) throw new ReviewedChatBindingDeniedError(denialReason);
context[PAPERCLIP_EXTERNAL_CHAT_EXECUTION_BOUND_KEY] = true;
}
const wakeCommentId = deriveCommentId(context, null);
@@ -16,6 +16,7 @@ import { isWorkspaceValidationFailedRun, isWorkspaceValidationFailure, attachPap
import { preserveWorkspaceRestoreRecoveryMetadataSql } from "../legacy-workspace-restore-recovery.js";
import { cancellationResultJson, controllerLeaseLossCancellation, readRunCancellation } from "../run-cancellation.js";
import { CONTROLLER_LEASE_LOST_FAILURE_CODE, LegacyControllerLeaseLostError } from "../legacy-controller-lease.js";
import { reviewedChatBindingDenialResultJson } from "../reviewed-chat-binding-diagnostics.js";
import { accountRunCost } from "../run-cost-accounting.js";
import { isConversation, settleConversationTurn } from "../agent-conversations.js";
import { legacyExecutionNeedsReconciliationWithEvidence } from "../legacy-execution-recovery.js";
@@ -1409,6 +1410,7 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC
// the signal's reason when a setup step threw a different error after it.
...controllerLeaseLossEvidence(outerErr),
...controllerLeaseLossResultJson(signal),
...reviewedChatBindingDenialResultJson(outerErr),
...(!providerDispatchStarted
? { executionRecovery: { kind: "bootstrap", providerWorkStarted: false } }
: {}),
@@ -69,6 +69,7 @@ import {
toolProfiles,
} from "@paperclipai/db";
import { HttpError } from "../../errors.js";
import type { ReviewedChatBindingDenialReason } from "../reviewed-chat-binding-diagnostics.js";
import { filterResolvedGitHubConnectionsForRun } from "../git-credentials.js";
import type {
AdapterRuntimeMcpAccess,
@@ -2007,6 +2008,15 @@ export async function resolveExternalChatWakeProvider(input: {
}
/** Bind an already-claimed chat/reply turn without checking out or approving it. */
const CHAT_ATTACHMENT_DENIAL_REASONS: ReadonlyMap<
string,
ReviewedChatBindingDenialReason
> = new Map<string, ReviewedChatBindingDenialReason>([
["paperclip_runner_chat_attachment_binding_denied", "binding_denied"],
["paperclip_runner_chat_attachment_destination_denied", "destination_denied"],
["paperclip_runner_chat_attachment_principal_denied", "principal_denied"],
]);
export async function attestReviewedExternalChatRun(input: {
db: Db;
companyId: string;
@@ -2016,12 +2026,19 @@ export async function attestReviewedExternalChatRun(input: {
contextSnapshot: Record<string, unknown>;
/** Diagnostic-only value from the committed, authorized answer; never a wake field. */
onQuestionResponseAttested?: (answeredAtMs: number) => void;
/** Receives the closed label of the first failed check when attestation is denied. */
onDenied?: (reason: ReviewedChatBindingDenialReason) => void;
}): Promise<boolean> {
const attempt = () =>
input.db.transaction(
async (
transaction,
): Promise<boolean | "pending_delivery" | { answeredAtMs: number }> => {
): Promise<
| true
| { denied: ReviewedChatBindingDenialReason }
| "pending_delivery"
| { answeredAtMs: number }
> => {
const tx = transaction as unknown as Db;
const [issue] = await tx
.select()
@@ -2057,25 +2074,27 @@ export async function attestReviewedExternalChatRun(input: {
)
.for("update", { noWait: true })
.limit(1);
if (!issue || !run || !actor) return { denied: "binding_missing" };
if (
!issue ||
!run ||
!actor ||
!(
issue.status === "in_review" ||
(input.contextSnapshot.source === "issue.interaction.respond" &&
issue.status === "in_progress")
) ||
issue.assigneeAgentId !== input.agentId ||
issue.executionRunId !== input.runId ||
run.status !== "running" ||
(run.nativeIssueId !== null && run.nativeIssueId !== input.issueId) ||
// This attestation precedes execution-start's transition to running.
// A prior run's error projection is invokable, unlike an operator
// pause, termination, or pending approval; all binding checks remain.
DIRECT_NON_INVOKABLE_STATUSES.has(actor.status)
)
)
return false;
return { denied: "issue_status" };
if (issue.assigneeAgentId !== input.agentId)
return { denied: "assignee_mismatch" };
if (issue.executionRunId !== input.runId)
return { denied: "owner_mismatch" };
if (run.status !== "running") return { denied: "run_not_running" };
if (run.nativeIssueId !== null && run.nativeIssueId !== input.issueId)
return { denied: "native_issue_mismatch" };
// This attestation precedes execution-start's transition to running.
// A prior run's error projection is invokable, unlike an operator
// pause, termination, or pending approval; all binding checks remain.
if (DIRECT_NON_INVOKABLE_STATUSES.has(actor.status))
return { denied: "agent_not_invokable" };
const admittedContext = parseObject(run.contextSnapshot);
const admittedIds = extractWakeCommentIds(admittedContext);
const suppliedIds = extractWakeCommentIds(input.contextSnapshot);
@@ -2087,7 +2106,7 @@ export async function attestReviewedExternalChatRun(input: {
admittedIds.length !== suppliedIds.length ||
admittedIds.some((id, index) => id !== suppliedIds[index])
)
return false;
return { denied: "context_mismatch" };
try {
const answer =
admittedContext.source === "issue.interaction.respond"
@@ -2100,7 +2119,7 @@ export async function attestReviewedExternalChatRun(input: {
)
: null;
if (admittedContext.source === "issue.interaction.respond" && !answer)
return false;
return { denied: "question_response_missing" };
// The marker is built here only after proving the real execution owner.
// The shared boundary then verifies current provider/resource/principal
// access for every admitted message; no lifecycle state is mutated.
@@ -2164,15 +2183,11 @@ export async function attestReviewedExternalChatRun(input: {
return "pending_delivery";
}
}
if (
error instanceof Error &&
[
"paperclip_runner_chat_attachment_binding_denied",
"paperclip_runner_chat_attachment_destination_denied",
"paperclip_runner_chat_attachment_principal_denied",
].includes(error.message)
)
return false;
const denied =
error instanceof Error
? CHAT_ATTACHMENT_DENIAL_REASONS.get(error.message)
: undefined;
if (denied) return { denied };
throw error;
}
},
@@ -2183,6 +2198,10 @@ export async function attestReviewedExternalChatRun(input: {
for (let attemptNumber = 0; attemptNumber < 51; attemptNumber += 1) {
try {
const result = await attempt();
if (typeof result === "object" && "denied" in result) {
input.onDenied?.(result.denied);
return false;
}
if (typeof result === "object") {
// The transaction (including COMMIT) must succeed before any attempt-
// local timing is accepted. A retry/rollback cannot publish this value.
@@ -1540,13 +1540,17 @@ async function materializeDecisionEffect(input: {
}
if (effect.kind === "release_checkout") {
await notifyUnsafeWorkspaceExportRecovery(input.tx, { companyId: input.companyId, issueId: input.issue.id });
// Release only this run's own locks. A successor can claim execution
// without changing statusVersion, so a preserve decision committed after
// that claim must not clear it. Each SET reads the pre-update row.
const ownsExecution = sql`${issues.executionRunId} = ${input.runId}::uuid`;
const [released] = await input.tx
.update(issues)
.set({
checkoutRunId: null,
executionRunId: null,
executionAgentNameKey: null,
executionLockedAt: null,
checkoutRunId: sql`case when ${issues.checkoutRunId} = ${input.runId}::uuid then null else ${issues.checkoutRunId} end`,
executionRunId: sql`case when ${ownsExecution} then null else ${issues.executionRunId} end`,
executionAgentNameKey: sql`case when ${ownsExecution} then null else ${issues.executionAgentNameKey} end`,
executionLockedAt: sql`case when ${ownsExecution} then null else ${issues.executionLockedAt} end`,
updatedAt: new Date(),
})
.where(
@@ -1555,13 +1559,20 @@ async function materializeDecisionEffect(input: {
eq(issues.companyId, input.companyId),
),
)
.returning({ id: issues.id });
.returning({
id: issues.id,
checkoutRunId: issues.checkoutRunId,
executionRunId: issues.executionRunId,
});
if (!released) throw new Error("native_checkout_not_released");
return {
effectKind: effect.kind,
targetType: "issue_checkout",
targetId: released.id,
payload: { checkoutRunId: null, executionRunId: null },
payload: {
checkoutRunId: released.checkoutRunId,
executionRunId: released.executionRunId,
},
};
}
if (effect.kind === "record_recovery") {
@@ -0,0 +1,43 @@
export const REVIEWED_CHAT_BINDING_NOT_AUTHORIZED = "reviewed_chat_execution_binding_not_authorized";
/**
* The first reviewed-chat attestation check that denied a run. Closed labels
* only: the run result and its failure report never carry IDs or message text.
*/
const reasons = [
"binding_missing", "issue_status", "assignee_mismatch", "owner_mismatch",
"run_not_running", "native_issue_mismatch", "agent_not_invokable",
"context_mismatch", "question_response_missing",
"binding_denied", "destination_denied", "principal_denied",
] as const;
export type ReviewedChatBindingDenialReason = typeof reasons[number];
export function isReviewedChatBindingDenialReason(value: unknown): value is ReviewedChatBindingDenialReason {
return reasons.some((reason) => reason === value);
}
/** Keeps the established run error text while carrying the denied check. */
export class ReviewedChatBindingDeniedError extends Error {
readonly code = REVIEWED_CHAT_BINDING_NOT_AUTHORIZED;
constructor(readonly denialReason: ReviewedChatBindingDenialReason | null) {
super(REVIEWED_CHAT_BINDING_NOT_AUTHORIZED);
}
}
export function reviewedChatBindingDenialResultJson(error: unknown) {
return error instanceof ReviewedChatBindingDeniedError && error.denialReason
? { reviewedChatBinding: { denialReason: error.denialReason } }
: {};
}
export function readReviewedChatBindingDenialReason(value: unknown): ReviewedChatBindingDenialReason | null {
if (!value || typeof value !== "object") return null;
try {
const reason = (value as Record<string, unknown>).denialReason;
return isReviewedChatBindingDenialReason(reason) ? reason : null;
} catch {
return null;
}
}
@@ -3,6 +3,7 @@ import { getEnvironmentAcquisitionDiagnostic } from "./environment-acquisition-d
import type { heartbeatRuns } from "@paperclipai/db";
import { readControllerLeaseLoss, readControllerLeaseLossCause, readRunCancellation } from "./run-cancellation.js";
import { readProcessLossDiagnostic } from "./process-loss-diagnostics.js";
import { readReviewedChatBindingDenialReason } from "./reviewed-chat-binding-diagnostics.js";
import { WORKSPACE_RESTORE_FAILURE_CODES } from "@paperclipai/shared";
import { redactDiagnosticText } from "@paperclipai/adapter-utils/command-redaction";
import { sanitizeWorkspaceRestoreDiagnostic } from "@paperclipai/adapter-utils/workspace-restore-diagnostics";
@@ -327,6 +328,9 @@ export function collectRunFailureDiagnostics(run: Run, options: RunFailureReport
if (leaseLoss.deadlineLateMs !== undefined) execution.controllerLeaseDeadlineLateMs = leaseLoss.deadlineLateMs;
if (leaseLoss.renewalCount !== undefined) execution.controllerLeaseRenewalCount = leaseLoss.renewalCount;
}
const reviewedChatDenial = run.errorCode === "setup_failed"
? readReviewedChatBindingDenialReason(read(result, "reviewedChatBinding")) : null;
if (reviewedChatDenial) execution.reviewedChatBindingDenialReason = reviewedChatDenial;
Object.assign(execution, scalars(result, [
"mode", "stopReason", "timeoutFired", "timeoutSource", "timeoutConfigured",
"effectiveTimeoutSec", "errorFamily",