From 7edbc1fc07d15bcbf0a8718565df42a59a65ce81 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 4 Nov 2026 17:49:03 -0800 Subject: [PATCH] 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 --- .../heartbeat-comment-wake-batching.test.ts | 3 + ...-reviewed-chat-binding.integration.test.ts | 134 +++++++++++++++++- .../__tests__/run-failure-diagnostics.test.ts | 16 +++ server/src/services/heartbeat.ts | 11 +- .../src/services/heartbeat/run-completion.ts | 2 + .../src/services/heartbeat/run-preparation.ts | 69 +++++---- .../status-decision-committer.ts | 23 ++- .../reviewed-chat-binding-diagnostics.ts | 43 ++++++ .../src/services/run-failure-diagnostics.ts | 4 + 9 files changed, 271 insertions(+), 34 deletions(-) create mode 100644 server/src/services/reviewed-chat-binding-diagnostics.ts diff --git a/server/src/__tests__/heartbeat-comment-wake-batching.test.ts b/server/src/__tests__/heartbeat-comment-wake-batching.test.ts index 4410e199f0c..a062a868858 100644 --- a/server/src/__tests__/heartbeat-comment-wake-batching.test.ts +++ b/server/src/__tests__/heartbeat-comment-wake-batching.test.ts @@ -1876,6 +1876,9 @@ describeEmbeddedPostgres("heartbeat comment wake batching", () => { expect(deniedRun).toMatchObject({ status: "failed", error: "reviewed_chat_execution_binding_not_authorized", + resultJson: expect.objectContaining({ + reviewedChatBinding: { denialReason: "question_response_missing" }, + }), contextSnapshot: expect.objectContaining({ interactionId: answered.id, sourceRunId, diff --git a/server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts b/server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts index bdb3c06c70a..7f1e4dd2295 100644 --- a/server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts +++ b/server/src/__tests__/heartbeat-reviewed-chat-binding.integration.test.ts @@ -1,6 +1,6 @@ import { randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; import { agents, companies, @@ -19,6 +19,11 @@ import { companyMemberships, chatDeliveries, chatMessageLinks, + completionContracts, + nativeRunFinalizations, + nativeRunResults, + statusDecisionEffects, + workAssessments, } from "@paperclipai/db"; import { renderPaperclipWakePrompt } from "@paperclipai/adapter-utils/server-utils"; import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js"; @@ -32,6 +37,9 @@ import { } from "../services/native-runtime/chat-attachment-reuse.js"; import { decideIssueReviewPathRecovery } from "../services/recovery/review-path-recovery.js"; import { resolveCurrentWakeCommentsBinding } from "../services/native-runtime/current-wake-comments.js"; +import { commitNativeStatusDecision } from "../services/native-runtime/status-decision-committer.js"; +import { NATIVE_STATUS_ARBITER_POLICY_VERSION } from "../services/native-runtime/status-arbiter.js"; +import { recoveryService } from "../services/recovery/service.js"; describe.each(["slack", "discord"] as const)("reviewed %s execution binding", (provider) => { let temporary: Awaited>; @@ -432,6 +440,33 @@ describe.each(["slack", "discord"] as const)("reviewed %s execution binding", (p } }); + it("names the first failed check with a closed label", async () => { + const denial = async (overrides: Partial[0]> = {}) => { + const reasons: string[] = []; + const attested = await attestReviewedExternalChatRun({ + db, ...binding, contextSnapshot: context, ...overrides, onDenied: (reason) => reasons.push(reason), + }); + return { attested, reasons }; + }; + await expect(denial()).resolves.toEqual({ attested: true, reasons: [] }); + await expect(denial({ issueId: otherIssueId })).resolves.toEqual({ attested: false, reasons: ["owner_mismatch"] }); + await expect(denial({ runId: randomUUID() })).resolves.toEqual({ attested: false, reasons: ["binding_missing"] }); + await expect(denial({ contextSnapshot: { ...context, wakeCommentIds: [randomUUID()] } })) + .resolves.toEqual({ attested: false, reasons: ["context_mismatch"] }); + await db.update(companyMemberships).set({ status: "suspended" }).where(eq(companyMemberships.principalId, userId)); + try { + await expect(denial()).resolves.toEqual({ attested: false, reasons: ["principal_denied"] }); + } finally { + await db.update(companyMemberships).set({ status: "active" }).where(eq(companyMemberships.principalId, userId)); + } + await db.update(heartbeatRuns).set({ status: "succeeded" }).where(eq(heartbeatRuns.id, runId)); + try { + await expect(denial()).resolves.toEqual({ attested: false, reasons: ["run_not_running"] }); + } finally { + await db.update(heartbeatRuns).set({ status: "running" }).where(eq(heartbeatRuns.id, runId)); + } + }); + it("rejects wrong or revoked principals and disabled provider reach", async () => { await db .update(chatExternalPrincipals) @@ -519,6 +554,103 @@ describe.each(["slack", "discord"] as const)("reviewed %s execution binding", (p } }); + // The prior native run commits its status decision before its run row is + // terminal. A preserve decision does not bump statusVersion, and neither does + // the successor's claim, so its version check passes after the claim. + let predecessorContractRevision = 0; + async function finalizingPredecessor() { + const predecessorRunId = randomUUID(), contractId = randomUUID(), resultId = randomUUID(), assessmentId = randomUUID(); + await db.insert(completionContracts).values({ id: contractId, companyId, issueId, revision: ++predecessorContractRevision, + schemaVersion: "paperclip.completion-contract.v1", policyVersion: "test", risk: "standard", + completionAuthority: "server_arbiter", incompleteCriteriaPolicy: "preserve_non_terminal", + contractJson: {}, canonicalSha256: contractId, createdByActorType: "system", createdByActorId: "test" }); + await db.insert(heartbeatRuns).values({ id: predecessorRunId, companyId, agentId, status: "running", + runtimeMode: "native", nativeIssueId: issueId, invocationSource: "automation", triggerDetail: "system", + completionContractId: contractId, contextSnapshot: { issueId, source: "github.pull_request_review" } }); + await db.insert(nativeRunResults).values({ id: resultId, companyId, issueId, runId: predecessorRunId, + completionContractId: contractId, serverFingerprint: resultId, schemaStatus: "accepted", resultJson: {}, canonicalSha256: resultId }); + const [snapshot] = await db.select().from(issues).where(eq(issues.id, issueId)); + await db.insert(workAssessments).values({ id: assessmentId, companyId, issueId, runId: predecessorRunId, + contractId, resultId, triggerKind: "native_result", triggerActorCompanyId: companyId, + priorIssueStatus: snapshot.status, priorStatusVersion: snapshot.statusVersion, + policyVersion: NATIVE_STATUS_ARBITER_POLICY_VERSION, assessmentJson: {}, inputDigest: assessmentId }); + await db.insert(nativeRunFinalizations).values({ runId: predecessorRunId, companyId, issueId, phase: "arbitrating", resultId, assessmentId }); + const commit = () => commitNativeStatusDecision({ + db, companyId, issueId, runId: predecessorRunId, assessmentId, + priorStatus: snapshot.status, priorStatusVersion: Number(snapshot.statusVersion), priorDecisionId: snapshot.lastStatusDecisionId, + decision: { policyVersion: NATIVE_STATUS_ARBITER_POLICY_VERSION, statusAction: "preserve", toStatus: "in_review", + reasonCode: "native_review_action_finished", unblockDescriptor: null, effects: [{ kind: "release_checkout" }] }, + }); + return { predecessorRunId, commit }; + } + + async function restoreSuccessorLock(previous: typeof issues.$inferSelect) { + await db.update(issues).set({ + checkoutRunId: previous.checkoutRunId, executionRunId: previous.executionRunId, + executionAgentNameKey: previous.executionAgentNameKey, executionLockedAt: previous.executionLockedAt, + lastStatusDecisionId: previous.lastStatusDecisionId, + }).where(eq(issues.id, issueId)); + } + + it("keeps the successor's claim when the finalizing predecessor releases its checkout", async () => { + const [before] = await db.select().from(issues).where(eq(issues.id, issueId)); + const { predecessorRunId, commit } = await finalizingPredecessor(); + const lockedAt = new Date(); + // The successor holds execution; the predecessor still holds its own checkout. + await db.update(issues).set({ checkoutRunId: predecessorRunId, executionRunId: runId, + executionAgentNameKey: "chat runner", executionLockedAt: lockedAt }).where(eq(issues.id, issueId)); + try { + const committed = await commit(); + const [after] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(after).toMatchObject({ status: "in_review", statusVersion: before.statusVersion, + checkoutRunId: null, executionRunId: runId, executionAgentNameKey: "chat runner" }); + expect(after.executionLockedAt?.getTime()).toBe(lockedAt.getTime()); + const [effect] = await db.select().from(statusDecisionEffects) + .where(eq(statusDecisionEffects.decisionId, committed.decision.id)); + expect(effect).toMatchObject({ effectKind: "release_checkout", targetType: "issue_checkout", + payload: { checkoutRunId: null, executionRunId: runId } }); + await expect(attest()).resolves.toBe(true); + } finally { + await restoreSuccessorLock(before); + } + }); + + it("still releases the predecessor's own execution lock and checkout", async () => { + const [before] = await db.select().from(issues).where(eq(issues.id, issueId)); + const { predecessorRunId, commit } = await finalizingPredecessor(); + await db.update(issues).set({ checkoutRunId: predecessorRunId, executionRunId: predecessorRunId, + executionAgentNameKey: "chat runner", executionLockedAt: new Date() }).where(eq(issues.id, issueId)); + try { + await commit(); + const [after] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(after).toMatchObject({ status: "in_review", checkoutRunId: null, executionRunId: null, + executionAgentNameKey: null, executionLockedAt: null }); + } finally { + await restoreSuccessorLock(before); + } + }); + + it("leaves a cancelled successor's lock to the stale-lock sweep", async () => { + const [before] = await db.select().from(issues).where(eq(issues.id, issueId)); + const { predecessorRunId, commit } = await finalizingPredecessor(); + const cancelledRunId = randomUUID(); + await db.insert(heartbeatRuns).values({ id: cancelledRunId, companyId, agentId, status: "cancelled", + finishedAt: new Date(), invocationSource: "automation", triggerDetail: "system", contextSnapshot: context }); + await db.update(issues).set({ checkoutRunId: predecessorRunId, executionRunId: cancelledRunId, + executionAgentNameKey: "chat runner", executionLockedAt: new Date() }).where(eq(issues.id, issueId)); + try { + await commit(); + expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]) + .toMatchObject({ checkoutRunId: null, executionRunId: cancelledRunId }); + const swept = await recoveryService(db, { enqueueWakeup: vi.fn() }).sweepStaleIssueLocks(); + expect(swept.issueIds).toEqual([issueId]); + expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]) + .toMatchObject({ checkoutRunId: null, executionRunId: null, executionAgentNameKey: null, executionLockedAt: null }); + } finally { + await restoreSuccessorLock(before); + } + }); + it("waits for exact inbound delivery processing to commit before attesting", async () => { await db .update(chatDeliveries) diff --git a/server/src/services/__tests__/run-failure-diagnostics.test.ts b/server/src/services/__tests__/run-failure-diagnostics.test.ts index 5c6734a4003..a11b3f6dfd2 100644 --- a/server/src/services/__tests__/run-failure-diagnostics.test.ts +++ b/server/src/services/__tests__/run-failure-diagnostics.test.ts @@ -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 }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 41a6e101079..2f68698ef1f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -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); diff --git a/server/src/services/heartbeat/run-completion.ts b/server/src/services/heartbeat/run-completion.ts index 720fe69d8ac..8ef5afb87fb 100644 --- a/server/src/services/heartbeat/run-completion.ts +++ b/server/src/services/heartbeat/run-completion.ts @@ -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 } } : {}), diff --git a/server/src/services/heartbeat/run-preparation.ts b/server/src/services/heartbeat/run-preparation.ts index bfbe6c862c1..ea292d8f4e9 100644 --- a/server/src/services/heartbeat/run-preparation.ts +++ b/server/src/services/heartbeat/run-preparation.ts @@ -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([ + ["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; /** 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 { const attempt = () => input.db.transaction( async ( transaction, - ): Promise => { + ): 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. diff --git a/server/src/services/native-runtime/status-decision-committer.ts b/server/src/services/native-runtime/status-decision-committer.ts index ba4413f61ad..2697d029fe8 100644 --- a/server/src/services/native-runtime/status-decision-committer.ts +++ b/server/src/services/native-runtime/status-decision-committer.ts @@ -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") { diff --git a/server/src/services/reviewed-chat-binding-diagnostics.ts b/server/src/services/reviewed-chat-binding-diagnostics.ts new file mode 100644 index 00000000000..f1a4727e4df --- /dev/null +++ b/server/src/services/reviewed-chat-binding-diagnostics.ts @@ -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).denialReason; + return isReviewedChatBindingDenialReason(reason) ? reason : null; + } catch { + return null; + } +} diff --git a/server/src/services/run-failure-diagnostics.ts b/server/src/services/run-failure-diagnostics.ts index a7864795c94..f540ee30921 100644 --- a/server/src/services/run-failure-diagnostics.ts +++ b/server/src/services/run-failure-diagnostics.ts @@ -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",