diff --git a/.claude/rules/02-quality-gates.md b/.claude/rules/02-quality-gates.md index 20abcef73..72c3dd311 100644 --- a/.claude/rules/02-quality-gates.md +++ b/.claude/rules/02-quality-gates.md @@ -95,7 +95,7 @@ Ask: "What test, if it existed before the breaking change was introduced, would - **If the bug involves cancellation through a timeout, retry, poll, or multi-step resource boundary**, the regression must prove the caller signal is composed with (not replaced by) internal timeouts, the exact caller reason survives provider-specific error handling, listeners/timers are cleaned up, and no later retry, poll, cleanup, cache write, or resource mutation starts after cancellation. Include a cancellation reason shaped like an otherwise retryable or idempotent domain error so classification catches cannot swallow it. - **If the bug involves streamed UI data that is later reconstructed from durable storage**, write a parity regression test for the persisted representation, not only the live stream. The test MUST include a partial/status-only update event and assert omitted fields do not clear previously visible metadata. See the retained incident lesson in this rule. - **If the bug involves lifecycle control across a runtime boundary** (agent/session/workspace/node stop, cancel, retry, replacement, suspend, or resume), the regression test MUST assert the runtime command is invoked before accepting the terminal state or dispatching replacement work. Database state changes and successful JSON responses are insufficient; the test must prove the external agent/node/workspace control side effect. -- **If runtime liveness is represented in more than one control plane** (for example D1, a session Durable Object, and a runtime Durable Object), timeout and cleanup tests MUST cross those boundaries with deliberately stale secondary state. A heartbeat timeout or sweep may terminalize work only after the runtime owner reports a conclusively terminal lifecycle; sleep, wake, restore, replacement, probe failure, and unknown state are inconclusive. Use one shared lifecycle classifier for every cleanup path so a stale replica cannot strand recoverable work. +- **If runtime liveness is represented in more than one control plane** (for example D1, a session Durable Object, and a runtime Durable Object), timeout and cleanup tests MUST cross those boundaries with deliberately stale secondary state. A heartbeat timeout or sweep may terminalize work only after the runtime owner reports a conclusively terminal lifecycle; sleep, wake, restore, replacement, probe failure, and unknown state are inconclusive. Use one shared lifecycle classifier for every cleanup path so a stale replica cannot strand recoverable work. A shared classifier is necessary but NOT sufficient: it must also derive its verdict from the record the recovery path actually reads, or it will confidently declare restorable work dead while every cleanup path agrees with it. See `.claude/rules/58-terminal-verdicts-must-match-the-resumer.md`. - **Inactivity is never successful completion evidence.** An idle, timeout, or cleanup sweep MUST NOT write `completed`; success requires an explicit successful task/runtime transition. If a shared liveness classifier proves the runtime conclusively dead, the sweep may write `failed` only with diagnostic context, a system `task_status_events` row, and failed trigger-execution synchronization. Gate workspace deletion on that same conclusive-death result; live and inconclusive tasks and workspaces remain intact. - **If the bug involves shell or process execution lifecycle** (process groups, child processes, cancellation, timeout, or cleanup after command completion), the regression test MUST cover the success path as well as failure/cancellation paths and prove spawned children are not left alive after the tool or command returns. - **If the bug involves a utility LLM call through a provider-compatible API**, the regression test MUST assert the exact provider payload controls that make the response contract reliable, not just the returned parsed text. For reasoning-capable models, this includes any explicit thinking/reasoning-disable parameters or response-format controls required for the utility to receive text in the field it reads. diff --git a/.claude/rules/58-terminal-verdicts-must-match-the-resumer.md b/.claude/rules/58-terminal-verdicts-must-match-the-resumer.md new file mode 100644 index 000000000..623b331bf --- /dev/null +++ b/.claude/rules/58-terminal-verdicts-must-match-the-resumer.md @@ -0,0 +1,143 @@ +# A "Work Is Unrecoverable" Verdict Must Read The Same Record The Resumer Reads + +## When This Applies + +Any code that writes a **terminal verdict about recoverability** — "this runtime is +conclusively gone", "this session is dead", "this job cannot be retried", "this workspace +is unreachable" — while a **separate code path elsewhere can still restore that work** +from a snapshot, checkpoint, replica, backup, or queued replay. + +The canonical pair in this repo: + +| Role | Function | Signal it reads | +| --------- | -------------------------------------------------------------------------------- | ------------------------------- | +| Destroyer | `classifyTaskRuntimeLiveness` (`apps/api/src/services/task-runtime-liveness.ts`) | `workspaces.status` | +| Resumer | `loadRecoveryContext` (`apps/api/src/services/session-recovery.ts`) | `session_snapshots.sleeping_at` | + +**Find the whole resumer before you mirror it.** The restore path is usually more than one +function, and the one that reads most naturally as "the resumer" is often not the one that +actually authorizes the restore. Here `loadRecoveryContext` merely assembles context; the real +gate is `claimSessionSnapshotRecovery` +(`apps/api/src/services/session-snapshot-recovery-lifecycle.ts`), whose `WHERE` clause adds a +restorable `status`/`degradation` pair and `recovery_attempts < max`. Mirroring only the +first function leaves the destroyer *looser* than the resumer — the opposite failure to the +original bug, and just as real: work the resumer will never wake is preserved anyway, so the +task hangs until the artifact's TTV expires instead of failing promptly. Enumerate every +predicate on the path from "candidate" to "restored" and mirror the union. + +## Why This Rule Exists + +On 2026-08-16, 31+ production tasks (`2026-08-06` onward, still firing on `2026-08-17`) +were terminalized as +`"Task runtime is conclusively gone after reconciliation grace (workspace_deleted)."` +while their sessions were asleep, unexpired, and fully restorable for another seven days. + +The chain was entirely composed of individually-correct steps: + +1. An agent's ACP turn ended normally (`end_turn`). The task stayed `in_progress` / + `awaiting_followup`. +2. The session-sleep cron slept it after the idle interval — **correct**, and exactly what + the "aggressively sleep idle sessions" policy asks for. A snapshot was captured; + `workspaces.status` became `sleeping`; `session_snapshots.sleep_status='sleeping'` with + an `expires_at` seven days out. +3. Five minutes later `NodeLifecycle` ran + `UPDATE workspaces SET status='deleted' ... WHERE status IN ('stopped','sleeping')` + (`node-lifecycle.ts:570-573`) — **rewriting the inconclusive `sleeping` marker into the + conclusive `deleted` marker.** +4. The classifier read `workspaces.status`, saw `deleted`, and returned + `conclusive: true`. The stuck-task sweep wrote `failed`. + +Every step was locally defensible. The system was still wrong, because **the destroying +side and the restoring side were reading different records.** `loadRecoveryContext` never +reads `workspaces.status` at all — a `deleted` workspace row was, and remains, perfectly +wakeable. Nothing forced the two to agree, and nothing tested them as a pair. + +Note that a shared classifier already existed, as `.claude/rules/02` requires. A single +shared classifier is necessary but **not sufficient**: it prevented the _cleanup paths_ +from disagreeing with each other, while leaving the classifier free to disagree with the +resumer. + +## Class of Bug + +**Destroyer/resumer signal divergence.** Two subsystems answer the same question — "is this +work still recoverable?" — from different columns, and a third path (here, a TTL sweep) +mutates the column only one of them reads. The failure is invisible in isolation: the +classifier's logic is correct given its inputs, the resumer's logic is correct given its +inputs, and the TTL sweep is doing its documented job. + +Tells: + +- A terminal verdict derived from a **status/lifecycle enum** rather than from the artifact + that actually enables recovery. +- A TTL, GC, or retention sweep whose predicate spans an inconclusive state + (`WHERE status IN ('stopped','sleeping')`) and collapses it into a terminal one. +- A recovery path whose precondition set is _narrower_ than the destroy path's — i.e. it can + restore things the destroyer already declared dead. + +## Hard Requirements + +1. **Derive the terminal verdict from the recovery precondition, not from a status enum.** + Before writing "unrecoverable", read the same record the resumer requires. If the resumer + would accept it, the verdict must be **inconclusive**. + +2. **Mirror the resumer's predicate explicitly, and say so in a comment naming the + function.** When the destroyer's predicate is deliberately _stricter_ than the resumer's, + the extra condition must be justified in that comment (in the canonical fix the only + addition is an expiry bound; see requirement 3). + +3. **Every "preserve" verdict needs a bounded escape** (`.claude/rules/47`). Preserving + recoverable work must not create an immortal task. Bound it on the artifact's own + retention (`expires_at`), and treat an **absent or unparseable** bound as _not_ + recoverable so a malformed row cannot pin work open forever. The bound must be + env-configurable with a `DEFAULT_*` constant. + +4. **A failed recoverability lookup withholds the terminal verdict.** The destructive action + is the irreversible one, so an unknown answer must not resolve to "destroy". + +5. **Do not add a `*_reason` / `*_cause` column just to tell the causes apart** when an + existing artifact already discriminates. In the canonical fix, snapshot _presence_ is the + discriminator: a user-initiated delete destroys the snapshot row + (`session-snapshot-persistence.ts:deleteSessionSnapshotState`), an idle sleep keeps it. + Prefer the record that already exists over new schema. + +6. **Keep the lookup off the hot path** (`.claude/rules/47`). Probe only for candidates that + would otherwise be terminalized, so a control loop pays the extra read only when it is + about to take the destructive action. + +## Required Tests + +- **The incident, reproduced**: the artifact is recoverable, the status enum says dead → + assert **inconclusive**. Must FAIL against the pre-fix code; verify that once. +- **The discriminating control**: same status enum, artifact genuinely absent → assert the + terminal verdict still fires. Without this, a test suite passes equally well if + terminalization were disabled outright. +- **The bound**: expired artifact → terminal. Absent/unparseable bound → terminal. +- **Scoping predicates against a real SQL engine** (`.claude/rules/28`): cross-tenant and + cross-resource fixtures, each proven discriminating by deleting the predicate. +- **Every adapter** that feeds the classifier supplies the new signal + (`.claude/rules/44` — enumerate them; a signal wired into one adapter and not another + reintroduces the bug on the unwired path). + +## Quick Compliance Check + +- [ ] The terminal verdict reads the resumer's own record, not just a status enum +- [ ] A comment names the resumer function the predicate mirrors +- [ ] Any extra strictness vs. the resumer is justified in that comment +- [ ] Preserve verdicts are bounded by an env-configurable retention; absent bound → terminal +- [ ] A failed recoverability lookup withholds the terminal verdict +- [ ] The probe fires only for otherwise-doomed candidates +- [ ] Incident reproduction + discriminating control both exist, and the reproduction was + verified to fail pre-fix + +## References + +- Task: `tasks/archive/2026-08-17-fix-slept-session-classified-as-dead.md` +- `.claude/rules/02-quality-gates.md` — "sleep, wake, restore, replacement, probe failure, + and unknown state are inconclusive"; one shared lifecycle classifier +- `.claude/rules/47-control-loop-io-budget.md` — bounded escape paths, I/O budget +- `.claude/rules/53-scheduled-handler-isolation-and-liveness-signals.md` — a signal that + cannot answer the question being asked of it +- `.claude/rules/57-write-only-cross-boundary-state.md` — reconcile, don't just report +- `.claude/rules/44-dual-write-migration-enumerate-writers.md` — enumerate every adapter +- `.claude/rules/28-credential-resolution-fallback-tests.md` — SQL predicates need a real + SQL engine diff --git a/apps/api/src/durable-objects/project-data/task-runtime-liveness.ts b/apps/api/src/durable-objects/project-data/task-runtime-liveness.ts index ff5649877..cde3c7673 100644 --- a/apps/api/src/durable-objects/project-data/task-runtime-liveness.ts +++ b/apps/api/src/durable-objects/project-data/task-runtime-liveness.ts @@ -6,9 +6,12 @@ import { import type { Env as WorkerEnv } from '../../env'; import { createModuleLogger } from '../../lib/logger'; +import { DEFAULT_SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS } from '../../services/session-snapshot-artifacts'; import { classifyTaskRuntimeLiveness, loadRuntimeWorkspaceSnapshot, + loadSessionResumabilitySnapshot, + needsSessionResumabilityProbe, type RuntimeAcpSessionSnapshot, type TaskRuntimeLiveness, type TaskRuntimeLivenessSignals, @@ -55,7 +58,32 @@ export async function getLocalTaskRuntimeLiveness( } } + // Only probed for a workspace that would otherwise be declared conclusively + // dead, keeping this off the alarm's hot path (`.claude/rules/47`). + let resumabilityProbeOutcome: TaskRuntimeLivenessSignals['resumabilityProbeOutcome'] = 'not_run'; + let sessionResumability: TaskRuntimeLivenessSignals['sessionResumability'] = null; + if (needsSessionResumabilityProbe(workspace, workspaceProbeOutcome)) { + try { + sessionResumability = await loadSessionResumabilitySnapshot( + env.DATABASE, + task.projectId, + workspace.id, + workspace.chatSessionId + ); + resumabilityProbeOutcome = 'ok'; + } catch (err) { + resumabilityProbeOutcome = 'error'; + log.warn('session_resumability_query_failed', { + projectId: task.projectId, + workspaceId: task.workspaceId, + action: 'preserved', + error: err instanceof Error ? err.message : String(err), + }); + } + } + const baseSignals: TaskRuntimeLivenessSignals = { + projectId: task.projectId, taskWorkspaceId: task.workspaceId, workspace, workspaceProbeOutcome, @@ -65,6 +93,12 @@ export async function getLocalTaskRuntimeLiveness( acpSessions: [], containerProbeOutcome: 'not_run', containerLifecycle: null, + resumabilityProbeOutcome, + sessionResumability, + resumabilityMaxRecoveryAttempts: positiveInt( + env.SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS, + DEFAULT_SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS + ), }; const initialClassification = classifyTaskRuntimeLiveness(baseSignals); if ( diff --git a/apps/api/src/durable-objects/project-data/types.ts b/apps/api/src/durable-objects/project-data/types.ts index db494b022..312c41b97 100644 --- a/apps/api/src/durable-objects/project-data/types.ts +++ b/apps/api/src/durable-objects/project-data/types.ts @@ -47,6 +47,7 @@ export type Env = { SESSION_ACTIVITY_PROBE_TIMEOUT_MS?: string; SESSION_ACTIVITY_PROBE_MAX_ATTEMPTS?: string; SESSION_ACTIVITY_PROBE_MAX_CANDIDATES?: string; + SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS?: string; DO_ALARMS_ENABLED_KV_KEY?: string; CONTROL_LOOP_KILL_SWITCH_CACHE_MS?: string; CONTROL_LOOP_DISABLED_ALARM_RETRY_MS?: string; diff --git a/apps/api/src/scheduled/stuck-tasks.ts b/apps/api/src/scheduled/stuck-tasks.ts index 088c9c590..14e4f2ee4 100644 --- a/apps/api/src/scheduled/stuck-tasks.ts +++ b/apps/api/src/scheduled/stuck-tasks.ts @@ -43,14 +43,18 @@ import * as schema from '../db/schema'; import type { TaskRunner } from '../durable-objects/task-runner'; import type { Env } from '../env'; import { log } from '../lib/logger'; +import { parsePositiveInt } from '../lib/route-helpers'; import { maybeJsonRecord } from '../lib/runtime-validation'; import { ulid } from '../lib/ulid'; import { persistError } from '../services/observability'; import * as projectDataService from '../services/project-data'; +import { DEFAULT_SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS } from '../services/session-snapshot-artifacts'; import { cleanupTaskRun } from '../services/task-runner'; import { classifyTaskRuntimeLiveness, loadRuntimeWorkspaceSnapshot, + loadSessionResumabilitySnapshot, + needsSessionResumabilityProbe, type RuntimeAcpSessionSnapshot, type TaskRuntimeLiveness, type TaskRuntimeLivenessSignals, @@ -437,7 +441,33 @@ export async function getTaskRuntimeLiveness( } } + // Only probed for a workspace that would otherwise be declared conclusively + // dead, so the sweep pays one extra point lookup only when it is about to + // terminalize a task (`.claude/rules/47`). + let resumabilityProbeOutcome: TaskRuntimeLivenessSignals['resumabilityProbeOutcome'] = 'not_run'; + let sessionResumability: TaskRuntimeLivenessSignals['sessionResumability'] = null; + if (needsSessionResumabilityProbe(workspace, workspaceProbeOutcome)) { + try { + sessionResumability = await loadSessionResumabilitySnapshot( + env.DATABASE, + task.project_id, + workspace.id, + workspace.chatSessionId + ); + resumabilityProbeOutcome = 'ok'; + } catch (err) { + resumabilityProbeOutcome = 'error'; + log.warn('stuck_task.session_resumability_query_failed', { + workspaceId: task.workspace_id, + projectId: task.project_id, + action: 'preserved', + error: err instanceof Error ? err.message : String(err), + }); + } + } + const baseSignals: TaskRuntimeLivenessSignals = { + projectId: task.project_id, taskWorkspaceId: task.workspace_id, workspace, workspaceProbeOutcome, @@ -447,6 +477,12 @@ export async function getTaskRuntimeLiveness( acpSessions: [], containerProbeOutcome: 'not_run', containerLifecycle: null, + resumabilityProbeOutcome, + sessionResumability, + resumabilityMaxRecoveryAttempts: parsePositiveInt( + env.SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS, + DEFAULT_SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS + ), }; const initialClassification = classifyTaskRuntimeLiveness(baseSignals); if ( diff --git a/apps/api/src/services/session-snapshot-artifacts.ts b/apps/api/src/services/session-snapshot-artifacts.ts index 094a60ba2..ad09edbef 100644 --- a/apps/api/src/services/session-snapshot-artifacts.ts +++ b/apps/api/src/services/session-snapshot-artifacts.ts @@ -22,6 +22,23 @@ export const DEFAULT_SESSION_SLEEP_RETRY_DELAY_MS = 5 * 60 * 1000; export const DEFAULT_SESSION_SLEEP_MAX_ATTEMPTS = 9; export const DEFAULT_SESSION_SNAPSHOT_RECOVERY_CLAIM_LEASE_MS = 10 * 60 * 1000; +/** + * Whether a snapshot's `status`/`degradation` pair still permits a restore. + * The in-memory twin of `restorableSnapshotCondition()` in + * `session-snapshot-recovery-lifecycle.ts`, which is the SQL predicate + * `claimSessionSnapshotRecovery` uses to authorize a wake. + * + * Lives here (rather than beside the SQL) so the task-runtime liveness + * classifier can mirror the real resume gate without duplicating the rule + * (`.claude/rules/58-terminal-verdicts-must-match-the-resumer.md`). + */ +export function isRestorableSnapshot(status: string | null, degradation: string | null): boolean { + return ( + (status === 'available' && degradation === 'none') || + (status === 'degraded' && Boolean(degradation) && degradation !== 'none') + ); +} + type SnapshotLeaseEnv = Env & { SESSION_SLEEP_CLAIM_LEASE_MS?: string; SESSION_SNAPSHOT_RECOVERY_CLAIM_LEASE_MS?: string; diff --git a/apps/api/src/services/session-snapshot-recovery-lifecycle.ts b/apps/api/src/services/session-snapshot-recovery-lifecycle.ts index c3fbeb8b2..7f9e8f2d8 100644 --- a/apps/api/src/services/session-snapshot-recovery-lifecycle.ts +++ b/apps/api/src/services/session-snapshot-recovery-lifecycle.ts @@ -7,6 +7,7 @@ import { parsePositiveInt } from '../lib/route-helpers'; import { DEFAULT_SESSION_SNAPSHOT_RECOVERY_CLAIM_LEASE_MS, DEFAULT_SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS, + isRestorableSnapshot, sessionLifecycleError, type SessionSnapshotRecoveryClaim, } from './session-snapshot-artifacts'; @@ -27,13 +28,6 @@ function restorableSnapshotCondition() { ); } -function isRestorableSnapshot(status: string | null, degradation: string | null): boolean { - return ( - (status === 'available' && degradation === 'none') || - (status === 'degraded' && Boolean(degradation) && degradation !== 'none') - ); -} - function sessionRecoveryClaimLeaseMs(env: Env): number { return parsePositiveInt( env.SESSION_SNAPSHOT_RECOVERY_CLAIM_LEASE_MS, @@ -244,10 +238,7 @@ export async function markSessionSnapshotAwakeInPlace( updatedAt: now, }) .where( - and( - eq(schema.sessionSnapshots.chatSessionId, chatSessionId), - restorableSnapshotCondition() - ) + and(eq(schema.sessionSnapshots.chatSessionId, chatSessionId), restorableSnapshotCondition()) ); } diff --git a/apps/api/src/services/task-runtime-liveness.ts b/apps/api/src/services/task-runtime-liveness.ts index c387e86dc..c888dc0ac 100644 --- a/apps/api/src/services/task-runtime-liveness.ts +++ b/apps/api/src/services/task-runtime-liveness.ts @@ -1,5 +1,7 @@ import type { AcpSessionStatus } from '@simple-agent-manager/shared'; +import { isRestorableSnapshot } from './session-snapshot-artifacts'; + export interface TaskRuntimeLiveness { live: boolean; conclusive: boolean; @@ -37,7 +39,39 @@ export interface ContainerLifecycleSnapshot { activeWorkStatus: string | null; } +/** + * The `session_snapshots` sleep record — the authoritative answer to "can this + * session still be restored?". Deliberately mirrors the gate the resumer + * actually applies, so the classifier and the resumer cannot disagree about + * what "gone" means (`.claude/rules/58-terminal-verdicts-must-match-the-resumer.md`). + * + * The resume path is two functions, and this type carries the inputs to both: + * - `session-recovery.ts:loadRecoveryContext` — requires `workspaceId`, + * a matching `projectId`, and `sleepingAt`. + * - `session-snapshot-recovery-lifecycle.ts:claimSessionSnapshotRecovery` — + * the function that actually authorizes a wake. It additionally requires a + * restorable `status`/`degradation` pair, an unexpired `expires_at`, and + * `recovery_attempts < SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS`. + */ +export interface SessionResumabilitySnapshot { + chatSessionId: string; + projectId: string | null; + workspaceId: string | null; + /** ms epoch; null when the session was never slept. */ + sleepingAt: number | null; + sleepStatus: string | null; + /** ms epoch; null when absent or unparseable. */ + expiresAtMs: number | null; + status: string | null; + degradation: string | null; + recoveryAttempts: number; +} + +export type ResumabilityProbeOutcome = 'ok' | 'error' | 'not_run'; + export interface TaskRuntimeLivenessSignals { + /** The task's project — re-checked against the snapshot row in memory. */ + projectId: string; taskWorkspaceId: string | null; workspace: RuntimeWorkspaceSnapshot | null; workspaceProbeOutcome: 'ok' | 'error' | 'unknown'; @@ -47,11 +81,90 @@ export interface TaskRuntimeLivenessSignals { acpSessions: RuntimeAcpSessionSnapshot[]; containerProbeOutcome: RuntimeProbeOutcome; containerLifecycle: ContainerLifecycleSnapshot | null; + /** + * `not_run` preserves the pre-resumability behaviour for callers that cannot + * reach D1; `error` withholds a conclusive-death verdict because the + * alternative is terminalizing a session that may still be restorable. + */ + resumabilityProbeOutcome: ResumabilityProbeOutcome; + sessionResumability: SessionResumabilitySnapshot | null; + /** + * `SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS` as the resumer resolves it, so the + * classifier applies the same wake-attempt ceiling the claim does. + */ + resumabilityMaxRecoveryAttempts: number; } const ACTIVE_ACP_STATUSES = new Set(['assigned', 'running']); const INCONCLUSIVE_WORKSPACE_STATUSES = new Set(['creating', 'sleeping', 'recovery']); const TERMINAL_CONTAINER_STATUSES = new Set(['stopping', 'stopped', 'expired', 'error']); +/** `session_snapshots.sleep_status` value meaning "asleep right now". */ +const RESUMABLE_SLEEP_STATUS = 'sleeping'; + +/** + * True when the session is currently asleep with a restorable, unexpired + * snapshot. `.claude/rules/02` requires sleep to be classified inconclusive: + * `NodeLifecycle` rewrites a slept workspace's `sleeping` status to `deleted` + * five minutes after sleep, so workspace status alone cannot distinguish + * "slept and restorable" from "destroyed". + * + * A user-initiated delete destroys the snapshot row entirely + * (`session-snapshot-persistence.ts:deleteSessionSnapshotState`), so snapshot + * presence — not a deletion-cause column — is the discriminator. + * + * Every condition below mirrors one the resumer already enforces, so this + * predicate can never be looser than the gate that authorizes a real wake + * (`.claude/rules/58`). Being *equal* rather than merely safe matters: a + * snapshot the resumer would refuse must terminalize, or the task waits out the + * full snapshot TTL for a wake that can never happen. + */ +function isSessionResumable( + snapshot: SessionResumabilitySnapshot | null, + projectId: string, + workspaceId: string, + maxRecoveryAttempts: number, + nowMs: number +): boolean { + if (!snapshot) return false; + // Defence in depth: the loader is already project+workspace scoped, so these + // two re-checks are the in-memory half of the pair `.claude/rules/28` wants. + if (snapshot.projectId !== projectId) return false; + if (snapshot.workspaceId !== workspaceId) return false; + if (snapshot.sleepingAt === null) return false; + // A session that already woke clears both `sleeping_at` and `sleep_status` + // (`markSessionSnapshotAwakeInPlace`, `completeSessionSnapshotRecovery`); + // this is the belt-and-braces half of that pair. + if (snapshot.sleepStatus !== RESUMABLE_SLEEP_STATUS) return false; + // Mirrors `restorableSnapshotCondition()` in the claim's WHERE clause. + if (!isRestorableSnapshot(snapshot.status, snapshot.degradation)) return false; + // Mirrors `recovery_attempts < maxAttempts`. Once wake attempts are spent the + // resumer refuses the claim, so preserving the task would strand it until the + // snapshot TTL — the second bounded escape (`.claude/rules/47`). + if (snapshot.recoveryAttempts >= maxRecoveryAttempts) return false; + // An absent or unparseable expiry is treated as NOT resumable so a snapshot + // can never make a task immortal (`.claude/rules/47` bounded escape path). + if (snapshot.expiresAtMs === null) return false; + return snapshot.expiresAtMs > nowMs; +} + +/** + * Whether a resumability lookup can still change the verdict. Adapters use this + * to keep the extra D1 read off the hot path: it only fires for a workspace + * that would otherwise be declared conclusively dead + * (`.claude/rules/47` control-loop I/O budget). + */ +export function needsSessionResumabilityProbe( + workspace: RuntimeWorkspaceSnapshot | null, + workspaceProbeOutcome: TaskRuntimeLivenessSignals['workspaceProbeOutcome'] +): workspace is RuntimeWorkspaceSnapshot & { chatSessionId: string } { + return ( + workspaceProbeOutcome === 'ok' && + workspace !== null && + workspace.chatSessionId !== null && + workspace.status !== 'running' && + !INCONCLUSIVE_WORKSPACE_STATUSES.has(workspace.status) + ); +} function result( workspace: RuntimeWorkspaceSnapshot | null, @@ -100,6 +213,33 @@ export function classifyTaskRuntimeLiveness( } if (workspace.status !== 'running') { + // Sleep is not death. A slept session keeps a restorable `session_snapshots` + // row that `session-recovery.ts` can wake even when the workspace row reads + // `deleted`, so terminalizing here would destroy recoverable work. + if (signals.resumabilityProbeOutcome === 'error') { + return result(workspace, { + live: false, + conclusive: false, + reason: `workspace_${workspace.status}_resumability_unknown`, + activeAcpSessionId: null, + }); + } + if ( + isSessionResumable( + signals.sessionResumability, + signals.projectId, + workspace.id, + signals.resumabilityMaxRecoveryAttempts, + signals.nowMs + ) + ) { + return result(workspace, { + live: false, + conclusive: false, + reason: `workspace_${workspace.status}_snapshot_resumable`, + activeAcpSessionId: null, + }); + } return result(workspace, { live: false, conclusive: true, @@ -257,3 +397,57 @@ export async function loadRuntimeWorkspaceSnapshot( nodeHeartbeatAt: Number.isFinite(heartbeatAt) ? heartbeatAt : null, }; } + +function parseTimestamp(value: string | null): number | null { + if (!value) return null; + const parsed = Date.parse(value); + return Number.isFinite(parsed) ? parsed : null; +} + +/** + * Load the session sleep record used to tell "slept and restorable" apart from + * "destroyed". Project- and workspace-scoped per `.claude/rules/11`; + * `chat_session_id` is uniquely indexed so this is a point lookup. + */ +export async function loadSessionResumabilitySnapshot( + db: D1Database, + projectId: string, + workspaceId: string, + chatSessionId: string +): Promise { + const row = await db + .prepare( + `SELECT chat_session_id, project_id, workspace_id, sleeping_at, sleep_status, expires_at, + status, degradation, recovery_attempts + FROM session_snapshots + WHERE chat_session_id = ? AND project_id = ? AND workspace_id = ? + LIMIT 1` + ) + .bind(chatSessionId, projectId, workspaceId) + .first<{ + chat_session_id: string; + project_id: string | null; + workspace_id: string | null; + sleeping_at: string | null; + sleep_status: string | null; + expires_at: string | null; + status: string | null; + degradation: string | null; + recovery_attempts: number | null; + }>(); + if (!row) return null; + + return { + chatSessionId: row.chat_session_id, + projectId: row.project_id, + workspaceId: row.workspace_id, + sleepingAt: parseTimestamp(row.sleeping_at), + sleepStatus: row.sleep_status, + expiresAtMs: parseTimestamp(row.expires_at), + status: row.status, + degradation: row.degradation, + // NOT NULL DEFAULT 0 in schema; coalesce defensively so a null can never + // read as "attempts remaining" via NaN comparison. + recoveryAttempts: row.recovery_attempts ?? 0, + }; +} diff --git a/apps/api/tests/unit/services/task-runtime-liveness.test.ts b/apps/api/tests/unit/services/task-runtime-liveness.test.ts index c67d33d9d..32241f59a 100644 --- a/apps/api/tests/unit/services/task-runtime-liveness.test.ts +++ b/apps/api/tests/unit/services/task-runtime-liveness.test.ts @@ -2,14 +2,19 @@ import { describe, expect, it } from 'vitest'; import { classifyTaskRuntimeLiveness, + needsSessionResumabilityProbe, + type SessionResumabilitySnapshot, type TaskRuntimeLivenessSignals, } from '../../../src/services/task-runtime-liveness'; const NOW = Date.parse('2026-08-06T12:00:00.000Z'); const STALE_MS = 5 * 60 * 1000; +/** Mirrors `DEFAULT_SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS`. */ +const MAX_RECOVERY_ATTEMPTS = 3; function signals(overrides: Partial = {}): TaskRuntimeLivenessSignals { return { + projectId: 'project-1', taskWorkspaceId: 'workspace-1', workspaceProbeOutcome: 'ok', workspace: { @@ -38,6 +43,11 @@ function signals(overrides: Partial = {}): TaskRunti ], containerProbeOutcome: 'not_run', containerLifecycle: null, + // Default `not_run` keeps every pre-existing expectation in this file + // unchanged, which is the back-compat proof for callers that cannot probe. + resumabilityProbeOutcome: 'not_run', + sessionResumability: null, + resumabilityMaxRecoveryAttempts: MAX_RECOVERY_ATTEMPTS, ...overrides, }; } @@ -182,3 +192,216 @@ describe('classifyTaskRuntimeLiveness', () => { }); }); }); + +/** + * Regression suite for the 2026-08-16 production incident: two task sessions + * (`da90b7c4`, `8bd22a42`) were terminalized as + * "Task runtime is conclusively gone after reconciliation grace (workspace_deleted)" + * while their `session_snapshots` rows were asleep, unexpired and restorable. + * + * `NodeLifecycle` rewrites a slept workspace's `sleeping` status to `deleted` + * five minutes after sleep, so workspace status alone cannot tell "slept and + * restorable" apart from "destroyed". Fixture values below are the real + * production rows. + */ +describe('classifyTaskRuntimeLiveness — slept sessions are not dead', () => { + const SLEEPING_AT = NOW - 9 * 60 * 1000; + const EXPIRES_AT = NOW + 7 * 24 * 60 * 60 * 1000; + + function resumable(overrides: Partial = {}) { + return { + chatSessionId: 'chat-1', + projectId: 'project-1', + workspaceId: 'workspace-1', + sleepingAt: SLEEPING_AT, + sleepStatus: 'sleeping', + expiresAtMs: EXPIRES_AT, + status: 'available', + degradation: 'none', + recoveryAttempts: 0, + ...overrides, + } satisfies SessionResumabilitySnapshot; + } + + /** Workspace as NodeLifecycle leaves it 5 min after an idle sleep. */ + function sleptWorkspace(base: TaskRuntimeLivenessSignals) { + return { ...workspaceFrom(base), status: 'deleted' }; + } + + it('does not terminalize a slept session with a live snapshot (incident 8bd22a42)', () => { + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: sleptWorkspace(base), + resumabilityProbeOutcome: 'ok', + sessionResumability: resumable(), + }) + ) + ).toMatchObject({ + live: false, + conclusive: false, + reason: 'workspace_deleted_snapshot_resumable', + workspaceStatus: 'deleted', + }); + }); + + it('treats a degraded-but-restorable snapshot as resumable (incident da90b7c4)', () => { + // Real row: status='degraded', degradation='entries-skipped', home R2 key + // present. `restorableSnapshotCondition()` accepts that pair, so the + // classifier must too. These fields are genuinely read by + // `isRestorableSnapshot`, so this case is not a duplicate of the one above. + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: sleptWorkspace(base), + resumabilityProbeOutcome: 'ok', + sessionResumability: resumable({ + status: 'degraded', + degradation: 'entries-skipped', + }), + }) + ) + ).toMatchObject({ conclusive: false, reason: 'workspace_deleted_snapshot_resumable' }); + }); + + it('still terminalizes a user-deleted workspace that has no snapshot row', () => { + // Discriminating control: a user delete destroys the snapshot row, so this + // must keep failing exactly as before. Without it, the test above would + // also pass if terminalization were disabled outright. + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: sleptWorkspace(base), + resumabilityProbeOutcome: 'ok', + sessionResumability: null, + }) + ) + ).toMatchObject({ + live: false, + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it.each([ + ['expired snapshot', { expiresAtMs: NOW - 1 }], + ['expiry exactly now', { expiresAtMs: NOW }], + ['unparseable/absent expiry', { expiresAtMs: null }], + ['never slept', { sleepingAt: null }], + // Every real wake path clears BOTH fields; this guards the half-cleared + // shape defensively rather than reproducing an observed transition. + ['already woke (sleep_status cleared)', { sleepStatus: null }], + ['snapshot for another workspace', { workspaceId: 'workspace-2' }], + ['snapshot for another project', { projectId: 'project-2' }], + // Parity with `claimSessionSnapshotRecovery`: the resumer refuses these, so + // preserving the task would strand it until the snapshot TTL. + ['unrestorable status/degradation pair', { status: 'failed', degradation: 'none' }], + ['degraded but degradation cleared', { status: 'degraded', degradation: 'none' }], + ['available but degradation set', { status: 'available', degradation: 'entries-skipped' }], + ['wake attempts exhausted', { recoveryAttempts: MAX_RECOVERY_ATTEMPTS }], + ['wake attempts over budget', { recoveryAttempts: MAX_RECOVERY_ATTEMPTS + 1 }], + ])('terminalizes when the snapshot is not restorable: %s', (_label, overrides) => { + // Bounded escape path (`.claude/rules/47`): a snapshot must never be able + // to keep a task alive forever. + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: sleptWorkspace(base), + resumabilityProbeOutcome: 'ok', + sessionResumability: resumable(overrides), + }) + ) + ).toMatchObject({ + live: false, + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('still preserves on the last remaining wake attempt', () => { + // Boundary control for the `recoveryAttempts` escape: one attempt left is + // still resumable, so the guard must be `>=`, not `>`. + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: sleptWorkspace(base), + resumabilityProbeOutcome: 'ok', + sessionResumability: resumable({ recoveryAttempts: MAX_RECOVERY_ATTEMPTS - 1 }), + }) + ) + ).toMatchObject({ conclusive: false, reason: 'workspace_deleted_snapshot_resumable' }); + }); + + it('withholds a death verdict when the resumability probe failed', () => { + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: sleptWorkspace(base), + resumabilityProbeOutcome: 'error', + sessionResumability: null, + }) + ) + ).toMatchObject({ + live: false, + conclusive: false, + reason: 'workspace_deleted_resumability_unknown', + }); + }); + + it('keeps a missing workspace row conclusively dead even with a snapshot', () => { + // `loadRecoveryContext` requires the workspace row to exist, so a + // hard-deleted workspace genuinely cannot be resumed. + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: null, + resumabilityProbeOutcome: 'ok', + sessionResumability: resumable(), + }) + ) + ).toMatchObject({ + live: false, + conclusive: true, + reason: 'workspace_missing', + }); + }); + + it('leaves the already-inconclusive sleeping status untouched', () => { + const base = signals(); + expect( + classifyTaskRuntimeLiveness( + signals({ + workspace: { ...workspaceFrom(base), status: 'sleeping' }, + resumabilityProbeOutcome: 'not_run', + }) + ) + ).toMatchObject({ conclusive: false, reason: 'workspace_sleeping_resumable' }); + }); + + it('does not probe resumability for a workspace that is not about to be failed', () => { + expect(needsSessionResumabilityProbe(signals().workspace, 'ok')).toBe(false); + }); + + it.each(['deleted', 'stopped', 'error', 'pending'])( + 'probes resumability before failing a %s workspace', + (status) => { + const base = signals(); + expect(needsSessionResumabilityProbe({ ...workspaceFrom(base), status }, 'ok')).toBe(true); + } + ); + + it('skips the probe when workspace identity or the workspace read is unusable', () => { + const base = signals(); + const deleted = { ...workspaceFrom(base), status: 'deleted' }; + expect(needsSessionResumabilityProbe(deleted, 'error')).toBe(false); + expect(needsSessionResumabilityProbe({ ...deleted, chatSessionId: null }, 'ok')).toBe(false); + expect(needsSessionResumabilityProbe(null, 'ok')).toBe(false); + expect(needsSessionResumabilityProbe({ ...deleted, status: 'sleeping' }, 'ok')).toBe(false); + }); +}); diff --git a/apps/api/tests/unit/stuck-task-slept-session-liveness.test.ts b/apps/api/tests/unit/stuck-task-slept-session-liveness.test.ts new file mode 100644 index 000000000..3fcb3d53c --- /dev/null +++ b/apps/api/tests/unit/stuck-task-slept-session-liveness.test.ts @@ -0,0 +1,338 @@ +import Database from 'better-sqlite3'; +import { beforeEach, describe, expect, it } from 'vitest'; + +import * as schema from '../../src/db/schema'; +import { getLocalTaskRuntimeLiveness } from '../../src/durable-objects/project-data/task-runtime-liveness'; +import type { Env as ProjectDataEnv } from '../../src/durable-objects/project-data/types'; +import type { Env } from '../../src/env'; +import { getTaskRuntimeLiveness } from '../../src/scheduled/stuck-tasks'; +import { needsSessionResumabilityProbe } from '../../src/services/task-runtime-liveness'; +import { createSchemaTables, createSqliteD1 } from '../helpers/sqlite-d1'; +import { createSqlStorage } from './durable-objects/sql-storage-test-utils'; + +/** + * Vertical slice (`.claude/rules/35`) for the 2026-08-16 production incident, + * exercising the real cron adapter against a real SQL engine rather than a + * `.where()`-ignoring mock — the resumability guard IS a SQL predicate + * (project + workspace scoped), so `.claude/rules/28` requires a real engine. + * + * Row values are the production shapes recovered from `sam-prod`: + * workspace 01M06502R3MW9JY75M7WK68B42 / session 8bd22a42-cf37-41fa-9947-30e78a0b6ece + * which was terminalized with "conclusively gone ... (workspace_deleted)" while + * its snapshot was asleep and unexpired for another seven days. + */ + +const PROJECT_ID = 'project-1'; +const WORKSPACE_ID = '01M06502R3MW9JY75M7WK68B42'; +const CHAT_SESSION_ID = '8bd22a42-cf37-41fa-9947-30e78a0b6ece'; +const NODE_ID = '01M064TG56ECJW1D127H32BRVJ'; + +const task = { project_id: PROJECT_ID, workspace_id: WORKSPACE_ID }; + +let sqlite: Database.Database; +let env: Env; + +function iso(offsetMs: number): string { + return new Date(Date.now() + offsetMs).toISOString(); +} + +function seedWorkspace(status: string): void { + sqlite + .prepare( + `INSERT INTO workspaces (id, user_id, name, repository, branch, status, vm_size, vm_location, + project_id, chat_session_id, node_id, created_at, updated_at) + VALUES (?, 'user-1', 'ws', 'org/repo', 'main', ?, 'cpx21', 'nbg1', ?, ?, ?, ?, ?)` + ) + .run(WORKSPACE_ID, status, PROJECT_ID, CHAT_SESSION_ID, NODE_ID, iso(-3_600_000), iso(0)); +} + +/** A node that is still healthy — the incident's node was `running`/`healthy`. */ +function seedNode(): void { + sqlite + .prepare( + `INSERT INTO nodes (id, user_id, name, status, health_status, last_heartbeat_at, + vm_size, vm_location, cloud_provider, created_at, updated_at) + VALUES (?, 'user-1', 'node', 'running', 'healthy', ?, 'cpx21', 'nbg1', 'hetzner', ?, ?)` + ) + .run(NODE_ID, iso(0), iso(-3_600_000), iso(0)); +} + +function seedSnapshot( + overrides: { + projectId?: string; + workspaceId?: string; + chatSessionId?: string; + sleepStatus?: string | null; + sleepingAt?: string | null; + expiresAt?: string; + status?: string; + degradation?: string; + recoveryAttempts?: number; + } = {} +): void { + sqlite + .prepare( + `INSERT INTO session_snapshots (id, project_id, workspace_id, node_id, user_id, chat_session_id, + runtime, status, degradation, manifest_r2_key, home_r2_key, + expires_at, sleeping_at, sleep_status, recovery_attempts, + sleep_attempts, created_at, updated_at) + VALUES (?, ?, ?, ?, 'user-1', ?, 'vm', ?, ?, 'manifest-key', 'home-key', ?, ?, ?, ?, 0, ?, ?)` + ) + .run( + 'snapshot-1', + overrides.projectId ?? PROJECT_ID, + overrides.workspaceId ?? WORKSPACE_ID, + NODE_ID, + overrides.chatSessionId ?? CHAT_SESSION_ID, + overrides.status ?? 'available', + overrides.degradation ?? 'none', + overrides.expiresAt ?? iso(7 * 24 * 60 * 60 * 1000), + overrides.sleepingAt === undefined ? iso(-9 * 60 * 1000) : overrides.sleepingAt, + overrides.sleepStatus === undefined ? 'sleeping' : overrides.sleepStatus, + overrides.recoveryAttempts ?? 0, + iso(-3_600_000), + iso(0) + ); +} + +/** A D1 binding whose `session_snapshots` reads always throw. */ +function brokenSnapshotDb(): { DATABASE: unknown } { + return { + DATABASE: { + prepare: (query: string) => + query.includes('session_snapshots') + ? { bind: () => ({ first: () => Promise.reject(new Error('D1 unavailable')) }) } + : createSqliteD1(sqlite).prepare(query), + }, + }; +} + +beforeEach(() => { + sqlite = new Database(':memory:'); + createSchemaTables(sqlite, [schema.workspaces, schema.nodes, schema.sessionSnapshots]); + env = { DATABASE: createSqliteD1(sqlite) } as Env; + seedNode(); +}); + +describe('stuck-task liveness for a slept session', () => { + it('does not declare a slept, restorable session conclusively dead', async () => { + // NodeLifecycle rewrote 'sleeping' -> 'deleted' five minutes after the sleep. + seedWorkspace('deleted'); + seedSnapshot(); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + live: false, + conclusive: false, + reason: 'workspace_deleted_snapshot_resumable', + workspaceStatus: 'deleted', + }); + }); + + it('preserves a degraded snapshot the recovery path would still restore', async () => { + seedWorkspace('deleted'); + seedSnapshot({ status: 'degraded', degradation: 'entries-skipped' }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ conclusive: false }); + }); + + it('still fails a user-deleted workspace whose snapshot row is gone', async () => { + // Discriminating control: a user delete destroys the snapshot row, so this + // must terminalize exactly as before. Without this case, the tests above + // would also pass if terminalization had simply been disabled. + seedWorkspace('deleted'); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + live: false, + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('fails once the snapshot has expired', async () => { + // Bounded escape path (`.claude/rules/47`): resumability cannot outlive the + // snapshot, so a task can never be preserved indefinitely. + seedWorkspace('deleted'); + seedSnapshot({ expiresAt: iso(-1_000) }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('fails when the session already woke', async () => { + // Every real wake path (`markSessionSnapshotAwakeInPlace`, + // `completeSessionSnapshotRecovery`) clears sleep_status back to NULL. + seedWorkspace('deleted'); + seedSnapshot({ sleepStatus: null }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('fails when expires_at is stored unparseable', async () => { + // Exercises `parseTimestamp`'s NaN -> null path through the REAL loader, + // not just the pure classifier: a corrupt bound must terminalize, never + // pin the task open (`.claude/rules/47`). + seedWorkspace('deleted'); + seedSnapshot({ expiresAt: 'not-a-timestamp' }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('fails once the snapshot has exhausted its wake attempts', async () => { + // Parity with `claimSessionSnapshotRecovery`, which refuses to claim once + // recovery_attempts reaches the max. Preserving here would strand the task + // for the full snapshot TTL waiting on a wake that can never happen. + seedWorkspace('deleted'); + seedSnapshot({ recoveryAttempts: 3 }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('still preserves while a wake attempt remains', async () => { + seedWorkspace('deleted'); + seedSnapshot({ recoveryAttempts: 2 }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: false, + reason: 'workspace_deleted_snapshot_resumable', + }); + }); + + it('withholds a death verdict when the cron adapter snapshot read fails', async () => { + // Rule 44 symmetry: the cron adapter has its own try/catch, so it needs its + // own error-path proof rather than inheriting the DO adapter's. + seedWorkspace('deleted'); + + await expect( + getTaskRuntimeLiveness(brokenSnapshotDb() as unknown as Env, task) + ).resolves.toMatchObject({ + live: false, + conclusive: false, + reason: 'workspace_deleted_resumability_unknown', + }); + }); + + it('ignores a snapshot belonging to a different project', async () => { + // Proves the `project_id` predicate is evaluated. Deleting it from the + // query makes this case return `conclusive: false` instead. + seedWorkspace('deleted'); + seedSnapshot({ projectId: 'project-2' }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('ignores a snapshot belonging to a different workspace', async () => { + // Proves the `workspace_id` predicate is evaluated. + seedWorkspace('deleted'); + seedSnapshot({ workspaceId: 'workspace-other' }); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('applies the same protection to a stopped workspace', async () => { + seedWorkspace('stopped'); + seedSnapshot(); + + await expect(getTaskRuntimeLiveness(env, task)).resolves.toMatchObject({ + conclusive: false, + reason: 'workspace_stopped_snapshot_resumable', + }); + }); + + it('leaves a running workspace on the normal ACP-liveness path', async () => { + // Owner-path control: a healthy workspace must never be diverted onto the + // resumability branch, even with a live snapshot row present. Asserting the + // absence of the resumability reasons is the load-bearing part — asserting + // only `workspaceStatus` would pass even if the branch had swallowed it. + seedWorkspace('running'); + seedSnapshot(); + + const result = await getTaskRuntimeLiveness(env, task); + expect(result.workspaceStatus).toBe('running'); + expect(result.reason).not.toContain('snapshot_resumable'); + expect(result.reason).not.toContain('resumability_unknown'); + // Proves the gate itself refuses to probe a running workspace, independent + // of whatever the downstream ACP probe concludes in this harness. + expect( + needsSessionResumabilityProbe( + { + id: WORKSPACE_ID, + status: 'running', + chatSessionId: CHAT_SESSION_ID, + nodeId: NODE_ID, + nodeRuntime: 'vm', + nodeStatus: 'running', + nodeHealthStatus: 'healthy', + nodeHeartbeatAt: Date.now(), + }, + 'ok' + ) + ).toBe(false); + }); +}); + +/** + * `.claude/rules/44` — the classifier gate reads `session_snapshots`, so EVERY + * adapter that feeds it must supply the signal. There are exactly two in + * production; this covers the ProjectData one that backs both DO idle sweeps + * (`processExpiredCleanups` and `checkWorkspaceIdleTimeouts`), which reach the + * classifier via `terminalizeIdleTaskInD1`. + */ +describe('ProjectData idle-cleanup liveness for a slept session', () => { + function doEnv(): ProjectDataEnv { + return { DATABASE: createSqliteD1(sqlite) } as unknown as ProjectDataEnv; + } + + const doTask = { projectId: PROJECT_ID, workspaceId: WORKSPACE_ID }; + + it('preserves a slept, restorable session instead of terminalizing it', async () => { + seedWorkspace('deleted'); + seedSnapshot(); + const sql = createSqlStorage(new Database(':memory:')); + + await expect(getLocalTaskRuntimeLiveness(sql, doEnv(), doTask)).resolves.toMatchObject({ + live: false, + conclusive: false, + reason: 'workspace_deleted_snapshot_resumable', + }); + }); + + it('still terminalizes when no snapshot row exists', async () => { + seedWorkspace('deleted'); + const sql = createSqlStorage(new Database(':memory:')); + + await expect(getLocalTaskRuntimeLiveness(sql, doEnv(), doTask)).resolves.toMatchObject({ + live: false, + conclusive: true, + reason: 'workspace_deleted', + }); + }); + + it('withholds a death verdict when the snapshot read fails', async () => { + seedWorkspace('deleted'); + const sql = createSqlStorage(new Database(':memory:')); + const broken = brokenSnapshotDb() as unknown as ProjectDataEnv; + + await expect(getLocalTaskRuntimeLiveness(sql, broken, doTask)).resolves.toMatchObject({ + live: false, + conclusive: false, + reason: 'workspace_deleted_resumability_unknown', + }); + }); +}); diff --git a/tasks/archive/2026-08-17-fix-slept-session-classified-as-dead.md b/tasks/archive/2026-08-17-fix-slept-session-classified-as-dead.md new file mode 100644 index 000000000..d5a1adaef --- /dev/null +++ b/tasks/archive/2026-08-17-fix-slept-session-classified-as-dead.md @@ -0,0 +1,393 @@ +# Fix: a slept (resumable) session is classified as conclusive runtime death + +**SAM task**: `01M074T96YJWVYCJWX6Z0T2E0E` +**Branch**: `sam/fix-production-workspace-reaping-0t2e0e` (PR #1844) +**Status**: archived (landed via PR #1844) + +## Problem + +Production task-mode sessions are being terminalized as `failed` with +`"Task runtime is conclusively gone after reconciliation grace (workspace_deleted)."` +while their session snapshot is intact, unexpired, and fully resumable. + +Two reported kills on 2026-08-16 (21:21Z and 21:36Z), but the audit found this is +**not** a two-off: **31 tasks** have been terminalized with a `workspace_deleted` +reason since 2026-08-06, and it is still occurring (1 on 2026-08-17). + +## Audit trail (production D1, evidence-backed — no inference) + +Account `e2eb9a8d5b560cce006fdd03ad6f2e49`, DB `sam-prod` / `sam-observability-prod`. + +| Session | Task | in_progress | ACP prompt ended | slept at | ws → `deleted` | task → `failed` | +| ---------- | ---------------------------- | ----------- | ------------------------------ | ------------- | -------------- | --------------- | +| `da90b7c4` | `01M064S8K7C13GRHCJJ13EJ6E7` | 20:48:02Z | 20:52:47Z (`end_turn`, 4m28s) | 21:11:57.499Z | 21:17:14.512Z | 21:21:05.825Z | +| `8bd22a42` | `01M064TAJ7B0AX8G115CY85RXM` | 20:43:32Z | 21:09:57Z (`end_turn`, 26m11s) | 21:26:51.529Z | ~21:31:51Z | 21:36:06.166Z | + +`session_snapshots` state **at the moment each task was declared "conclusively gone"**: + +| Session | `status` | `degradation` | `sleep_status` | `sleeping_at` | `expires_at` | home R2 key | +| ---------- | ----------- | ----------------- | -------------- | ------------- | -------------- | ----------- | +| `8bd22a42` | `available` | `none` | `sleeping` | 21:26:51.529Z | **2026-08-23** | present | +| `da90b7c4` | `degraded` | `entries-skipped` | `sleeping` | 21:11:57.499Z | **2026-08-23** | present | + +Both sessions were resumable for another **7 days** when SAM wrote `failed`. + +### Proven causal chain + +1. The agent's ACP prompt completes with `end_turn` + (`vm-agent` → `recordTurnEnd` → `session_state.activity='idle'`). The task stays + `in_progress` / `execution_step='awaiting_followup'` because the agent has not + called `complete_task()` yet. +2. The **session-sleep cron** (`runSessionSleepSweep`, `apps/api/src/scheduled/session-sleep.ts:221`) + finds the session eligible: `isActivitySafeForSleep` + (`apps/api/src/services/session-sleep.ts:195`) returns `true` on `activity === 'idle'`, + and `SESSION_SLEEP_AFTER_MS` (default 15 min) has elapsed. **This is intended, + policy-sanctioned behaviour** ("aggressively sleep idle sessions"). +3. `sleepWorkspaceSession` captures the snapshot, sets `workspaces.status='sleeping'`, + writes `session_snapshots.sleep_status='sleeping'` + `sleeping_at` + `expires_at`, + then arms `NodeLifecycle.scheduleWorkspaceDeletion` (`session-sleep.ts:610-621`). +4. **5 minutes later** (`WORKSPACE_STOPPED_TTL_MS`, + `packages/shared/src/constants/node-pooling.ts:100`), the NodeLifecycle alarm runs + `UPDATE workspaces SET status='deleted' ... WHERE status IN ('stopped','sleeping')` + (`apps/api/src/durable-objects/node-lifecycle.ts:570-573`). + **This rewrites the inconclusive `sleeping` marker into the conclusive `deleted` marker.** + (Timing confirms: `sleeping_at` + 5 min == the workspace row's `updated_at`.) +5. The stuck-task cron classifies the runtime. + `classifyTaskRuntimeLiveness` (`apps/api/src/services/task-runtime-liveness.ts:102-109`) + sees `status !== 'running'` and `'deleted'` is not in `INCONCLUSIVE_WORKSPACE_STATUSES` + (`:53`), so it returns `{ live:false, conclusive:true, reason:'workspace_deleted' }`. +6. `apps/api/src/scheduled/stuck-tasks.ts:988` sees `conclusive && !live` → writes `failed`. + +### The core defect + +**The classifier and the resumer disagree about what "gone" means.** + +`loadRecoveryContext` (`apps/api/src/services/session-recovery.ts:85`) — the code that +actually wakes a slept session — accepts a session as resumable when: + +```ts +snapshot.workspaceId && snapshot.projectId === projectId && snapshot.sleepingAt; +``` + +It **never reads `workspaces.status`**. A workspace row with `status='deleted'` is still +fully wakeable. Meanwhile the classifier's only workspace signal _is_ `workspaces.status`, +and it never reads `session_snapshots`. So the classifier declares conclusive death for +sessions the recovery path would happily restore. + +This is precisely the contract in `.claude/rules/02-quality-gates.md`: + +> "sleep, wake, restore, replacement, probe failure, and unknown state are inconclusive" + +and the class of bug in `.claude/rules/53`: a _turn-level_ signal (`activity='idle'`, +meaning "the ACP prompt ended") and a _status_ signal (`workspaces.status`) being used as +proxies for a question they cannot answer ("is this session's work unrecoverable?"). + +### Why the original brief's framing needed correcting + +The brief attributed the kill to `idle-cleanup.ts` treating chat-silence as idleness, and +proposed making that sweep consult `session_state.activity`. The evidence does not support +that as the proximate cause: + +- Both agents had **finished** their ACP turn (`end_turn`) 17–19 min before the sleep. They + were not "chat-quiet but working" at kill time; they were genuinely between turns. +- Both DO idle sweeps (`processExpiredCleanups` `idle-cleanup.ts:320`, + `checkWorkspaceIdleTimeouts` `idle-cleanup.ts:548`) route through + `terminalizeIdleTaskInD1` → `getLocalTaskRuntimeLiveness`, which **preserves** unless the + shared classifier says conclusively dead. They did not kill these sessions. +- The sleep cron is already activity-aware and will not sleep a `prompting` session. + +Every terminalization path — the stuck-task cron _and_ both DO idle sweeps — funnels through +the single shared `classifyTaskRuntimeLiveness`. Fixing it there fixes all three at once +(DRY), instead of bolting separate guards into each sweep. + +## Fix + +Teach the shared classifier the one thing it is missing: **whether the session is currently +asleep and restorable.** + +Add a session-resumability signal to `TaskRuntimeLivenessSignals`. When the workspace row +exists but is not `running`, and the session has a live sleep record, classify as +**inconclusive** (`workspace__snapshot_resumable`) instead of conclusive death. + +Resumability predicate (mirrors the resumer, plus a bound the resumer lacks): + +- a `session_snapshots` row exists for this workspace's `chat_session_id`, scoped to the + same `project_id` **and** `workspace_id` (rule 11: project-scoped reads) +- `sleeping_at IS NOT NULL` — the session was genuinely slept. User deletes destroy the + snapshot row entirely (`session-snapshot-persistence.ts:42`), so this discriminates + idle-sleep from user deletion **without needing a new `deleted_reason` column** +- `sleep_status = 'sleeping'` — asleep _now_, not a stale marker from a session that + already woke +- `expires_at` parses and is in the future — **the bounded escape** (rule 47). Once the + snapshot expires the session is genuinely unrecoverable and the task fails normally. + An absent/unparseable expiry counts as NOT resumable, so no task can become immortal. + +`workspace_missing` (row absent) stays **conclusive**: `loadRecoveryContext` requires the +workspace row to exist (`session-recovery.ts:91-94`), so a hard-deleted workspace really is +unrecoverable. This boundary is chosen to match the resumer exactly. + +Probe-outcome handling: `'not_run'` preserves today's behaviour (no evidence → unchanged); +`'error'` yields inconclusive for the non-running branch only, because the alternative is +destroying a possibly-recoverable session. + +## Research findings → checklist mapping + +Every finding below has a checklist item or an explicit deferral. + +| Finding | Disposition | +| ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------- | +| Classifier reads only `workspaces.status`; never `session_snapshots` | Item 1, 2 | +| `node-lifecycle.ts:571` predicate rewrites `sleeping` → `deleted` | Covered by items 1–2 (classifier no longer trusts status alone) | +| `workspaces` has no deletion-cause column | Not needed — snapshot presence is the discriminator (documented above) | +| Two adapters feed the classifier; both must supply the signal | Item 3 (rule 44 enumeration) | +| `expires_at` is `NOT NULL` in schema and is the natural escape bound | Item 1 | +| `loadRecoveryContext` ignores `expires_at` — an expired snapshot would still be "restored" | **Deferred** → SAM Idea `01M076660EE6YENQFDK40S4N9P` | +| `cancelStalledPrompt` bypasses `recordTurnEnd`/`publishTurnEnd` | Already tracked in `tasks/backlog/2026-08-17-migrate-cancel-stalled-prompt-to-record-turn-end.md` | +| Snapshot for `da90b7c4` was `degraded` (`entries-skipped`) | Not gated on — matches resumer; noted in tests | + +## Implementation checklist + +- [x] 1. `apps/api/src/services/task-runtime-liveness.ts`: add + `SessionResumabilitySnapshot`, `resumabilityProbeOutcome` + + `sessionResumability` to `TaskRuntimeLivenessSignals`, and an + `isSessionResumable()` helper enforcing the four-part predicate above. +- [x] 2. Insert the resumability branch into `classifyTaskRuntimeLiveness` between the + `INCONCLUSIVE_WORKSPACE_STATUSES` check and the `status !== 'running'` conclusive + branch. Keep `workspace_missing` conclusive. +- [x] 3. Add `loadSessionResumabilitySnapshot()` next to `loadRuntimeWorkspaceSnapshot()` + and wire **every** adapter (rule 44 — enumerate all callers of + `classifyTaskRuntimeLiveness`): + `apps/api/src/scheduled/stuck-tasks.ts` (`getTaskRuntimeLiveness`) and + `apps/api/src/durable-objects/project-data/task-runtime-liveness.ts` + (`getLocalTaskRuntimeLiveness`). +- [x] 4. Regression tests (see below). +- [x] 5. Post-mortem + process fix (rule 02 mandates both). +- [x] 6. File the deferred `loadRecoveryContext` expiry gap as a SAM Idea (`01M076660EE6YENQFDK40S4N9P`). + +## Required tests (rule 02 — must be discriminating) + +- [x] **Reproduces the incident**: workspace `status='deleted'` + snapshot + `sleep_status='sleeping'`, `sleeping_at` set, `expires_at` 7 days out + → `conclusive === false`. Must FAIL on pre-fix code. +- [x] **Degraded snapshot still resumable** (the real `da90b7c4` shape: + `status='degraded'`, `degradation='entries-skipped'`) → inconclusive. +- [x] **Discriminating control — user delete**: workspace `deleted`, **no** snapshot row + → still `conclusive: true, reason:'workspace_deleted'`. Proves the fix does not + blanket-disable terminalization. +- [x] **Bounded escape (rule 47)**: `expires_at` in the past → conclusive dead. + Plus unparseable/absent expiry → conclusive dead (no immortal tasks). +- [x] **Already woke**: `sleeping_at` set but `sleep_status != 'sleeping'` → conclusive. +- [x] **`workspace_missing` unchanged**: row absent + snapshot present → conclusive + (matches `loadRecoveryContext`'s workspace-row requirement). +- [x] **Probe failure preserves**: `resumabilityProbeOutcome='error'` on a non-running + workspace → inconclusive. +- [x] **`not_run` back-compat**: existing signals shape → unchanged verdicts. +- [x] **Both adapters wired**: a vertical-slice test per adapter (rule 35) with realistic + D1 rows proving the snapshot is actually queried and reaches the classifier. +- [x] **Existing pins updated**: `apps/api/tests/workers/scheduled-stuck-tasks.test.ts:160,198` + and `apps/api/tests/unit/stuck-tasks.test.ts:1163,1207,1248` assert + `workspace_deleted` failures — confirm they use no-snapshot fixtures (correct) or + update them. + +## Acceptance criteria + +- [x] A slept, unexpired session is never terminalized as conclusive runtime death by any + of the three paths (stuck-task cron, `processExpiredCleanups`, `checkWorkspaceIdleTimeouts`). +- [x] A user-deleted workspace still terminalizes exactly as before. +- [x] An expired snapshot terminalizes (bounded escape proven by test). +- [x] `pnpm lint && pnpm typecheck && pnpm test && pnpm build` green. +- [ ] Staging deploy green + verified. +- [x] Post-mortem + process fix included in the PR. + +## References + +- `.claude/rules/02-quality-gates.md` — "sleep… is inconclusive"; regression + process fix +- `.claude/rules/53-scheduled-handler-isolation-and-liveness-signals.md` — liveness ≠ idleness +- `.claude/rules/57-write-only-cross-boundary-state.md` — reconcile, don't just report +- `.claude/rules/47-control-loop-io-budget.md` — bounded escape path +- `.claude/rules/44-dual-write-migration-enumerate-writers.md` — enumerate every adapter +- `tasks/active/2026-08-16-session-activity-state-machine.md` — PR #1840 (ancestor) + +## Verification record (2026-08-17, landing run) + +Branch rebased onto `origin/main` (`1b89bf598`) — **clean, no conflicts**. PR #1840's +ProjectData DO migration `029-session-activity-reconciliation` was neither renumbered nor +altered. + +### Suite result + +`apps/api`: 93 passed across `tests/unit/services/task-runtime-liveness.test.ts`, +`tests/unit/stuck-task-slept-session-liveness.test.ts`, and the pre-existing +`tests/unit/stuck-tasks.test.ts` pins (which use no-snapshot fixtures and therefore still +correctly assert `workspace_deleted` terminalization). + +### Discrimination proof (rule 58 requires verifying this once) + +Neutralizing the resumability branch in `classifyTaskRuntimeLiveness` reproduces the pre-fix +verdicts. Result: **8 failed | 39 passed**. The 8 failures are exactly the incident and +preserve assertions: + +- `does not terminalize a slept session with a live snapshot (incident 8bd22a42)` +- `treats a degraded-but-restorable snapshot as resumable (incident da90b7c4)` +- `withholds a death verdict when the resumability probe failed` +- `does not declare a slept, restorable session conclusively dead` +- `preserves a degraded snapshot the recovery path would still restore` +- `applies the same protection to a stopped workspace` +- `preserves a slept, restorable session instead of terminalizing it` +- `withholds a death verdict when the snapshot read fails` + +All 39 controls stayed green — user-delete-no-snapshot, expired snapshot, already-woke, +`workspace_missing`, cross-project, cross-workspace, and running-workspace. That proves the +suite is not merely asserting "terminalization is disabled". + +### SQL scoping predicates (rule 28 / rule 58) + +Deleting `AND project_id = ?` from `loadSessionResumabilitySnapshot` turns +`ignores a snapshot belonging to a different project` red — the predicate is proven +discriminating against a real SQL engine (`better-sqlite3` + `createSqliteD1` + +`createSchemaTables`, not a `.where()`-ignoring mock). + +**Nuance worth recording:** deleting `AND workspace_id = ?` does *not* redden the +cross-workspace test, because `isSessionResumable()` also enforces +`snapshot.workspaceId !== workspaceId` in memory. That is intentional defence-in-depth +(rule 28), and `session_snapshots.chat_session_id` is uniquely indexed so only one row can +match the session in the first place. Both layers are exercised; the in-memory guard is what +the cross-workspace assertion discriminates on. + +## Specialist review round (2026-08-17) + +Three local reviewers ran against the rebased branch. **No CRITICAL or HIGH code findings.** +Everything below was fixed in the branch rather than deferred. + +### cloudflare-specialist — ADDRESSED + +- **D1 query correctness, index coverage, I/O budget, error handling, DO concurrency: PASS.** + Independently confirmed `NodeLifecycle.deleteWorkspace` only rewrites `status` and never nulls + `chat_session_id`, so the probe genuinely engages for the real incident shape. +- **[MEDIUM] The predicate mirrored only half the resumer.** `loadRecoveryContext` assembles + context, but the function that actually *authorizes* a wake is + `claimSessionSnapshotRecovery`, whose `WHERE` also requires a restorable + `status`/`degradation` pair and `recovery_attempts < max`. The original predicate checked + neither, making the classifier **looser** than the resumer: a snapshot with exhausted wake + attempts would be preserved for the full 7-day TTL waiting on a wake that can never happen. + **Fixed** — `isSessionResumable` now mirrors the claim exactly (shared + `isRestorableSnapshot` helper, plus an env-configurable + `SESSION_SNAPSHOT_RECOVERY_MAX_ATTEMPTS` ceiling threaded through both adapters). This adds a + second bounded escape alongside `expires_at`. + +### test-engineer — ADDRESSED + +- **[MEDIUM] The two "degraded snapshot" tests were non-discriminating duplicates** — they + seeded `status`/`degradation` that the code never read. **Fixed** by the parity change above: + those fields are now genuinely read by `isRestorableSnapshot`, and the test asserts the exact + resumable reason. Verified discriminating. +- **[MEDIUM] `leaves a running workspace on the normal ACP-liveness path` proved nothing** — it + asserted only `workspaceStatus: 'running'`, which passes even if the resumability branch had + swallowed the request. **Fixed**: now asserts the reason is neither resumability reason and + directly asserts `needsSessionResumabilityProbe(...) === false` for a running workspace. +- **[MEDIUM] The cron adapter's own resumability-error path was untested** (only the DO + adapter's was). **Fixed** — added + `withholds a death verdict when the cron adapter snapshot read fails`, sharing a + `brokenSnapshotDb()` helper with the DO test (rule 44 symmetry). +- **[LOW] No in-memory `projectId` re-check** (asymmetric defence-in-depth vs `workspaceId`). + **Fixed** — `SessionResumabilitySnapshot` now carries `projectId` and `isSessionResumable` + re-checks it, so both scoping predicates have the SQL + in-memory pair rule 28 asks for. +- **[LOW] Unparseable `expires_at` was only covered at the pure-function boundary.** **Fixed** — + added a vertical-slice case seeding a literally unparseable value through the real loader. +- **Two-sweep zombie test (rule 47): agreed not required.** `loadSessionResumabilitySnapshot` + is read-only and mutates no counter, so repeated probing cannot exhaust a destructive budget; + the bounded escapes (`expires_at`, `recovery_attempts`) are proven directly. + +### task-completion-validator — ADDRESSED + +- Reproduced the discrimination proof independently (8 red / 39 green) and additionally showed + that deleting the in-memory `workspaceId` guard reddens the pure-unit case while the SQL + predicate still covers the vertical slice — confirming both defence layers are real and + independently load-bearing. +- Confirmed scope boundary respected: no `session-sleep.ts`, no `isActivitySafeForSleep`, no + `packages/vm-agent/` paths in the diff. +- **[LOW] The "already woke" fixture used `sleep_status='completed'`, a value never written.** + **Fixed** — the fixture now uses `null`, which is what every real wake path + (`markSessionSnapshotAwakeInPlace`, `completeSessionSnapshotRecovery`) actually writes, and + the comment marks the guard as defensive rather than an observed transition. +- **[MEDIUM] Rule 58 referenced `tasks/archive/…` while the file was still in `tasks/active/`.** + Resolved by archiving the task file in this PR. + +### Rule 58 amended + +Added a "find the whole resumer before you mirror it" section: the function that reads as the +resumer is often not the one that authorizes the restore, and mirroring only the first leaves +the destroyer *looser* than the resumer — the inverse failure, where unwakeable work is +preserved until its TTL instead of failing promptly. + +--- + +## Post-mortem + +### What broke + +Task-mode sessions were terminalized as `failed` with +`"Task runtime is conclusively gone after reconciliation grace (workspace_deleted)."` +while their work was intact and restorable. Users saw a red "Task failed" on work that had +merely gone to sleep. **31 tasks** since 2026-08-06, still firing on 2026-08-17. + +### Root cause + +`classifyTaskRuntimeLiveness` decided recoverability from `workspaces.status` alone, while +`loadRecoveryContext` — the code that actually wakes a slept session — decides from +`session_snapshots.sleeping_at` and never reads `workspaces.status` at all. + +`NodeLifecycle` rewrites a slept workspace's `sleeping` status to `deleted` five minutes +after sleep (`node-lifecycle.ts:570-573`, predicate `status IN ('stopped','sleeping')`), +collapsing the one inconclusive marker the classifier understood into a conclusive one. +From that moment the classifier and the resumer disagreed, and the classifier won. + +### Timeline + +- **2026-08-06** — first occurrence in production `task_status_events`. +- **2026-08-16 20:39–21:36Z** — the two reported kills. Both agents finished their ACP turn + normally (`end_turn`, 4m28s and 26m11s), slept 17–19 min later, and were failed ~9 min + after the workspace status flipped. `sleeping_at` + 5 min matches the workspace row's + `updated_at` in both cases. +- **2026-08-17** — reported and fixed. + +### Why it was not caught + +- **Every component was individually correct.** The sleep cron is activity-aware and refused + to sleep a `prompting` session. Both DO idle sweeps route through the shared classifier + and preserve unless it says dead. The classifier's logic is sound given its inputs. There + was no single wrong line to find in review. +- **The one shared classifier gave false confidence.** `.claude/rules/02` already required a + single shared lifecycle classifier, and SAM had one. That guaranteed the cleanup paths + agreed _with each other_ — and said nothing about whether they agreed with the resumer. +- **Sleep-then-classify was never tested as a sequence.** Tests covered sleep, and covered + classification, but no test slept a session and then asked the classifier about it. The + existing `workspace_deleted` assertions all used no-snapshot fixtures, so they encoded the + buggy verdict as expected behaviour. +- **The symptom read as a different bug.** "Agent was working and got reaped" points at the + idle detector. The idle detector was innocent; only the D1 audit trail + (`session_snapshots` rows still `sleeping`/unexpired at kill time) disproved that framing. + +### Class of bug + +**Destroyer/resumer signal divergence** — two subsystems answering "is this work still +recoverable?" from different records, with a third path (a TTL sweep) mutating only the one +the destroyer reads. Not covered by the existing rules, which addressed _cleanup paths +disagreeing with each other_ rather than _cleanup disagreeing with recovery_. + +### Process fix + +New rule **`.claude/rules/58-terminal-verdicts-must-match-the-resumer.md`**: any verdict that +work is unrecoverable must be derived from the record the resumer requires, must name that +resumer function in a comment, must be bounded by the artifact's own env-configurable +retention (absent bound → terminal, so nothing becomes immortal), must withhold the verdict +when the lookup fails, and must ship both an incident reproduction _and_ a discriminating +control proving terminalization still fires. + +`.claude/rules/02-quality-gates.md:98` amended to state that a shared lifecycle classifier is +necessary but not sufficient, pointing at rule 58. + +### What this fix deliberately does NOT change + +Sleeping an idle session stays aggressive and unchanged — that behaviour is correct and is +explicit project policy. The fix only stops SAM from mistaking its own sleep for death.