diff --git a/doc/grok-bot-agent.md b/doc/grok-bot-agent.md index 54a9cf49f2a..ffd37093d98 100644 --- a/doc/grok-bot-agent.md +++ b/doc/grok-bot-agent.md @@ -89,6 +89,8 @@ routine is healthy. After inspecting Grok, use Clear recovery hold and verify the connection again. Paperclip retires the false file obligation only when the original admission denied workspace access, the assignment is fenced, the coordinator has settled, and the exact managed Runner leases have stop receipts. +Connection readiness is invalidated before task inspection, so a partial failure +cannot retain an old verified connection while removing a task hold. The inspection preserves the old result, action uncertainty, and no-replay hold. Send a new task message to request fresh work. Inspection does not certify global Grok termination or delete any previously retained allocation. Such allocations diff --git a/doc/plans/2026-10-13-grok-bot-production-readiness.md b/doc/plans/2026-10-13-grok-bot-production-readiness.md index b80579ba894..438cfca7607 100644 --- a/doc/plans/2026-10-13-grok-bot-production-readiness.md +++ b/doc/plans/2026-10-13-grok-bot-production-readiness.md @@ -280,3 +280,12 @@ negative assertion saw the earlier readiness gate instead of the revoked-session gate; the corrected test checks both before and after re-verification. Server typecheck passes after narrowing the native workspace union. Final current-head CI, review, deployed historical recovery and fresh Stop are still release gates. + +A second review caught a partial-transaction ordering gap. Routine readiness now +commits as invalid before the separate task inspection transactions. New tests +hold the task lock to prove readiness is already gone and verification remains +blocked; an unrelated active assignment cannot cause a task hold to disappear. +All 43 Grok routine tests pass. The first follow-up CI attempt also timed out in +one unchanged run-history case at 15 seconds; all 91 tests in that file pass +locally. The timeout remains a recorded failure with cause unproven. The new +source head requires fresh CI and review. diff --git a/server/src/__tests__/grok-bot-routine.test.ts b/server/src/__tests__/grok-bot-routine.test.ts index 454e8909653..c49b846a6d8 100644 --- a/server/src/__tests__/grok-bot-routine.test.ts +++ b/server/src/__tests__/grok-bot-routine.test.ts @@ -563,6 +563,39 @@ describe("Grok inspection of historical workspace-less Stop", () => { expect(await db.select().from(activityLog).where(and(eq(activityLog.companyId, f.company.id), eq(activityLog.action, "grok.workspace_obligation_inspected")))).toHaveLength(1); }); + it("does not retire task holds if another assignment prevents recovery", async () => { + const f = await historicalHold(); + const [other] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id, + status: "running", invocationSource: "assignment", triggerDetail: "system" }).returning(); + await db.insert(dotRunnerAssignments).values({ ...f.work, id: randomUUID(), runId: other!.id, + normalizedSessionId: randomUUID(), turnId: randomUUID(), status: "accepted" }); + await expect(f.service.recover(f.company.id, f.agent.id, f.principal.grant.userId)).rejects.toThrow("current Paperclip work session"); + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.run.id)); + expect(hasRequiredWorkspaceRecovery(run!.resultJson)).toBe(true); + expect(await f.service.recoveryNeeded(f.company.id, f.agent.id)).toBe(true); + }); + + it("commits verification invalidation before waiting for historical task inspection", async () => { + const f = await historicalHold(); + let recovery: Promise | undefined; + try { + await db.transaction(async tx => { + await tx.select().from(issues).where(eq(issues.id, f.task.id)).for("update"); + recovery = f.service.recover(f.company.id, f.agent.id, f.principal.grant.userId); + void recovery.catch(() => {}); + await vi.waitFor(async () => { + expect(await f.service.state(f.company.id, f.agent.id, f.pairing.bindingId)).toMatchObject({ verified: false, recoveryNeeded: true }); + }); + await expect(verification(f)).rejects.toThrow("recover"); + const [binding] = await db.select().from(dotAgentBindings).where(eq(dotAgentBindings.id, f.pairing.bindingId)); + expect(binding).toMatchObject({ status: "connected", readyAt: null }); + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.run.id)); + expect(hasRequiredWorkspaceRecovery(run!.resultJson)).toBe(true); + }); + } finally { await recovery; } + expect(await f.service.state(f.company.id, f.agent.id, f.pairing.bindingId)).toMatchObject({ verified: false, recoveryNeeded: false }); + }); + it.each(["cleanup_pending", "receipt_mismatch", "controller_owned", "assignment_active", "running", "operator_revoked", "foreign_company"])("does not retire an old obligation when %s", async condition => { const f = await historicalHold(); if (condition === "cleanup_pending") await db.update(environmentLeases).set({ cleanupStatus: "failed" }).where(eq(environmentLeases.id, f.lease.id)); @@ -581,6 +614,9 @@ describe("Grok inspection of historical workspace-less Stop", () => { } else await expect(f.service.recover(f.company.id, f.agent.id, f.principal.grant.userId)).rejects.toThrow(); const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.run.id)); expect(hasRequiredWorkspaceRecovery(run!.resultJson)).toBe(true); + if (["cleanup_pending", "receipt_mismatch", "controller_owned"].includes(condition)) { + expect(await f.service.state(f.company.id, f.agent.id, f.pairing.bindingId)).toMatchObject({ verified: false, recoveryNeeded: true }); + } expect(await f.continueWork()).toBeNull(); }); }); diff --git a/server/src/services/grok-bot/routine.ts b/server/src/services/grok-bot/routine.ts index 5a06c82c55d..6e6acb5ac62 100644 --- a/server/src/services/grok-bot/routine.ts +++ b/server/src/services/grok-bot/routine.ts @@ -51,7 +51,6 @@ export function grokRoutineService(db: Db, deliver: Delivery = deliverGrokRoutin async function recover(companyId: string, agentId: string, operatorId: string) { await authorizeExternalBotOperator(db, companyId, operatorId); - await inspectGrokWorkspaceObligations(db, companyId, agentId, operatorId); await db.transaction(async tx => { // All routine writers serialize on the binding before dispatch/routine rows. // Include retired bindings so recovery cannot bypass an old activity hold. @@ -68,6 +67,10 @@ export function grokRoutineService(db: Db, deliver: Delivery = deliverGrokRoutin await logActivity(tx as unknown as Db, { companyId, actorType: "user", actorId: operatorId, action: "grok.recovery_inspected", entityType: "agent", entityId: agentId, details: { inspectedRemoteWork: true, remoteTerminationConfirmed: false, requiresVerification: true } }); }); + // Commit loss of readiness before retiring task holds. If inspection fails + // or waits for a task lock, the old verification cannot admit new work. + // The remaining task hold also prevents verification during this gap. + await inspectGrokWorkspaceObligations(db, companyId, agentId, operatorId); return { recovered: true, requiresVerification: true, remoteTerminationConfirmed: false }; }