Fix node reaping and gate VM agent rollout by build version - #1758
Merged
simple-agent-manager[bot] merged 16 commits intoAug 6, 2026
Merged
Conversation
Root-cause analysis of production node reaping failure. Documents a live production outage: the 5-minute cron has no error isolation, so a throw in runNodeCleanupSweep silently kills all 8 subsequent sweeps including user cron triggers (21 active, 5h overdue). Direct-to-main push for the task file is blocked by branch protection (required status checks), so it ships in this branch. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…provider orphans
PRIMARY FIX — stops a live production outage.
The 5-minute cron ran 13 sweeps as a flat sequence of bare awaits with no error
isolation. A throw in runNodeCleanupSweep silently skipped every sweep after it.
In production this began 2026-08-05T20:25Z and was still active ~13h later:
node reaping, observability purge, trigger-execution cleanup, session-task repair,
setup-session sweep, compose-artifact cleanup, compute-usage cleanup and trial
expiry all stopped, along with runCronTriggerSweep -- so 21 active USER cron
triggers stopped firing (earliest next_fire_at was 5h overdue). Nodes accumulated
until the shared Hetzner account hit its 10-server limit and staging deploys 403'd.
Each sweep now runs inside sweep-isolation.ts: failures are contained, logged, and
persisted to observability, and cron.completed reports failedSweeps so the next
occurrence is visible instead of showing up only as an absence of effects.
SECONDARY FIXES
- Idle orphan reaping actually reaps. The phase previously only wrote an
observability row. Its predicate was also unsatisfiable: it compared
nodes.updated_at, which every heartbeat bumps (verified in production --
updated_at is byte-identical to last_heartbeat_at on every running node), so a
healthy idle node never aged into the window. recoveryType 'orphaned_node' had
zero events across the sweep's entire lifetime. Idleness is now measured from
COALESCE(MAX(workspaces.updated_at), nodes.created_at).
- Max lifetime is a real backstop. The 4h ceiling skips any node with an active
workspace, so a workspace row wedged in 'running' made a node immortal -- two
production nodes survived 1932h and 2135h and were cleared manually. A 24h
absolute ceiling now applies when no workspace has reported ACTIVITY within the
idle window, which distinguishes a busy node from one holding a stuck row.
- Rule 47: every candidate query is bounded by a configurable LIMIT (phases 1-5
were unbounded), and sweep VM-agent calls use a new 5s background timeout
instead of the interactive 30s -- unbounded candidates x 30s dead-node timeouts
is the most likely cause of the original throw.
- Rule 51: deployment-environments.ts DELETE claim was missing
node_class != 'user-owned' (its twin in deployment-environment-lifecycle.ts has
it). Workspace phases 4 and 6 mutated workspaces with no join to nodes at all;
both now exclude user-owned nodes.
- Phase 1's live-workspace guard counted only status='running' while phases 2/3
counted ('running','creating','recovery'), so a node with a 'creating' workspace
could be destroyed. Widened to match.
SAFETY
Deployment nodes are explicitly protected. node_role='deployment' machines host
live user applications and legitimately hold zero workspaces forever, so every
'running with no workspaces' heuristic matches them perfectly. Three such nodes
backing ACTIVE production deployment_environments were live during this work --
reaping on the zero-workspaces signal alone would have destroyed three users'
production applications. Every destroy query carries node_role='workspace'.
PROVIDER-SIDE ORPHAN RECONCILIATION
New bounded sweep reclaims servers that exist at the provider but which no D1 row
claims (provisionNode writes provider_instance_id only after createVM returns, and
deletes the node row outright on transient capacity failure).
This is the only code in SAM that destroys infrastructure based on absence rather
than presence, so it is fail-closed throughout: no ENVIRONMENT identity skips the
run; servers missing or mismatching the env label are skipped; servers younger
than a 1h floor are skipped; any D1 or provider read failure aborts without
destroying anything; a claiming row in any non-terminal state preserves the server.
A new 'env' provider label makes this possible at all -- SAM's staging and
production share one Hetzner project, so without it the two deployments' servers
are indistinguishable and a staging sweep could delete production servers.
Pre-existing servers carry no env label and are therefore permanently out of scope.
Rule 18: node-cleanup.ts (646 lines) split into node-cleanup/ modules.
Co-Authored-By: Claude <noreply@anthropic.com>
…tion New behavioral suites (all run the real code against real SQLite where relevant): - node-cleanup-deployment-node-exemption.test.ts: the safety gate. Verified DISCRIMINATING by deleting node_role='workspace' from all four phases -- all 5 tests fail, and the lone-deployment-node case shows deleteCalls=['deployment-only'], reproducing in a test the exact scenario that would have destroyed three users' live production applications. Each test carries a workspace-role control node so 'nothing was destroyed' cannot pass by nothing matching. - node-cleanup-idle-signal.test.ts: proves the reaper is not defeated by heartbeat activity (node idle 7h with updated_at 2s old is reaped), does not touch busy nodes, respects the idle window and its env override, never reaps a freshly-provisioned node with no workspace yet, and includes the rule-47 two-sweep zombie check for a permanently failing candidate. - sweep-isolation.test.ts: a throwing sweep does not prevent later sweeps; failures are named and persisted; a failure while RECORDING a failure still cannot abort the cron; a crashed sweep yields undefined rather than a zero result so it stays distinguishable from one that ran and found nothing. - provider-orphan-reconciliation.test.ts: 19 tests weighted toward proving nothing is destroyed on incomplete evidence -- unlabeled, foreign-env, too-young, malformed node label, unparseable creation time, non-terminal claiming row, failed D1 claim lookup, failed provider list, missing platform credential, no ENVIRONMENT identity. Verified DISCRIMINATING by removing the env re-check: the foreign-environment server is then destroyed. - node-provider-labels.test.ts: pins the pre-existing label contract and proves the env label is OMITTED rather than defaulted when the environment is unknown. Updated existing suites for intentional behavior changes rather than weakening them: - node-role-exemption.test.ts: replaced four brittle source-string assertions (they sliced the sweep source by index and broke on any edit) with a pointer to the new behavioral suite. Source strings prove a filter is PRESENT, not that it WORKS. - recovery-resilience.test.ts: the 'runs stuck-task recovery first' ordering assertion is replaced by one asserting real isolation. Ordering was only ever a workaround -- it protects whatever runs earliest and nothing else, which is precisely how stuck-task recovery kept working while everything downstream stayed dead for 13 hours. The orphan-node assertions now require the heartbeat-immune signal and explicitly assert 'AND n.updated_at < ?' is ABSENT. - node-cleanup-user-owned-zombie.test.ts: the orphan case now asserts DESTRUCTION rather than flagging. Also added health_status to the harness schema -- without it every destroy's D1 write threw, and because the provider mock records the call before that write, asserting only on deleteCalls hid the failure entirely. Full API suite: 485 files, 6589 tests, all green. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Verified against production D1: nodes.id is stored UPPERCASE (every live row satisfies id = upper(id)), while the provider label is written lowercased (node.id.toLowerCase() in services/nodes.ts). The claim lookup must normalise. If it ever compares raw, the lookup returns zero rows for EVERY node, every live server reads as unclaimed, and the reconciler deletes the entire fleet -- at the time of writing all 9 production servers, three of them backing live customer deployments. The sibling suite stubs D1 by hand and ignores the SQL text, so it passes with or without lower(). This suite runs against real SQLite with real uppercase ULIDs. Verified DISCRIMINATING: replacing lower(id) IN (...) with id IN (...) makes the live-node test fail with deleteVM called for 'srv-live'. Also covers a mixed batch (live + deleted + unknown) to prove the batched IN-clause maps each server to its own row rather than applying one verdict to all. Co-Authored-By: Claude <noreply@anthropic.com>
Doc sync (rule 01): all nine new env vars were undocumented. Adds two sections to the configuration reference -- idle/orphan node reaping and provider-side orphan reconciliation -- including why idleness is measured from workspace activity rather than nodes.updated_at, and why deployment nodes are never reaped. Also corrects the MAX_AUTO_NODE_LIFETIME_MS description, which claimed to be an absolute ceiling while actually skipping any node with an active workspace. Process fix (rule 02 requires one per bug fix, targeting the CLASS of bug): .claude/rules/53-scheduled-handler-isolation-and-liveness-signals.md. The class is 'a control loop that fails silently, where the symptom is an absence rather than an error', with two sub-classes both present in this incident: unisolated sequential steps, and a liveness timestamp used as an idleness proxy. Rule 53 also records that the previous mitigation for sub-class 1 was to REORDER one sweep rather than isolate them -- and that reordering protected only the sweep that ran first, which is precisely how stuck-task recovery kept working while everything downstream stayed dead for 13 hours. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer verdict was PASS on Principle XI (0 violations; all 9 new env vars have a DEFAULT_* constant, an env override, and an Env declaration). These are its four improvement recommendations, all applied: - Add PROVIDER_ORPHAN_RECONCILE_LAST_RUN_KV_KEY. The interval-gate KV key was the one value in the new sweep with no override, while its direct sibling compose-image-artifact-cleanup.ts has exactly that override for the same class of value in the same directory. - Import parseMs/parsePositiveInt from node-cleanup/shared instead of re-declaring identical private copies (DRY). - Direct unit tests for both parsers across undefined/empty/whitespace/non-numeric/ negative/zero/Infinity/NaN. Previously they were only exercised indirectly through phase behaviour, where a broken fallback would be masked by whatever the phase did with the bad value. - Warn when env overrides invert the intended threshold ordering. This one is more than tidiness: an inverted override does not throw, it silently makes a phase unreachable or trivially satisfiable -- the exact 'a guard that never fires looks identical to a guard with nothing to do' failure this whole change exists to fix (rule 53). Deliberately a warning rather than a hard failure, since degraded reaping precision beats refusing to sweep at all. The reviewer also confirmed the earlier doc-sync commit resolved the missing-env-var documentation gap, and judged the curated static data (label keys, terminal status set, ULID pattern, Hetzner's 63-char limit) acceptable rather than hardcoded config. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
simple-agent-manager
Bot
deleted the
sam/recover-finish-failed-child-ebra1p
branch
August 6, 2026 13:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
01KZBEJT3JJHH1WENA8PSV5S3Yand rebase it onto currentmainafter Integrate strict CTO remediation fixes #1697/fix(vm-agent): disable Codex bwrap across all runtimes #1757.agentVersion, API persistsnodes.agent_version, and all reusable-node selection paths reject stale or missing builds whenVM_AGENT_REQUIRED_VERSIONis set.github.sha;skip_agentleaves the requirement unset.Validation
pnpm --filter @simple-agent-manager/api test -- --run tests/unit/durable-objects/task-runner-readiness.test.ts tests/unit/durable-objects/task-runner-node-selection.test.ts tests/unit/services/node-cleanup-idle-signal.test.ts tests/unit/routes/node-lifecycle-byo.test.tspnpm exec vitest run scripts/quality/deploy-reusable-workflow.test.ts scripts/quality/sync-wrangler-config.test.tscd packages/vm-agent && go test ./...pnpm --filter @simple-agent-manager/shared build && pnpm --filter @simple-agent-manager/providers build && pnpm --filter @simple-agent-manager/api typecheckpnpm --filter @simple-agent-manager/api lint(0 errors; existing warnings remain)git diff --check origin/main...HEADStaging verification
31104130685succeeded for commitb71a2601d2094da6eb8317c75edadda6221ef219.01KZBKK6V6SAHQA76B59HJ5ZQHon node01KZBKK6DM2WBXB3VR36EGBCZS; live D1 showedagent_version='b71a2601d2094da6eb8317c75edadda6221ef219'andagent_ready_at='2026-08-06T13:21:31.761Z'.End-to-end verification
VM_AGENT_REQUIRED_VERSION=${github.sha}for normal deploys and uploads VM-agent artifacts before the Worker deploys.scripts/deploy/sync-wrangler-config.tswrites the derived Worker var into config.packages/vm-agent/internal/server/health.gopostsagentVersionon/readyand/heartbeat.apps/api/src/routes/node-lifecycle.tspersists callback build identity tonodes.agent_version.UI Compliance
N/A: no UI surfaces, layout, styling, or user interaction flows changed in this PR.
Post-mortem
The production rollout hole was that #1757 deployed Worker behavior before the fleet was forced onto matching VM-agent binaries; current workspace
01KZBE2BBHG4014XR022RA53N7could therefore land on old node01KZA098X9R3ZXX85XKKDAN0M3withoutCODEX_CONFIG/INITIAL_AGENT_MODE. The archived task postmortem intasks/archive/2026-08-06-fix-node-reaping-orphan-reconciliation.mdrecords the original node reaping failure, the rollout addendum, and the validation evidence.Specialist Review Evidence
go test ./...passed inpackages/vm-agent.VM_AGENT_REQUIRED_VERSIONis an optional Worker var, deployment-derived, documented, and not a GH_/GITHUB_ secret mapping.github.sha, with no manual hardcoded rollout SHA; cleanup thresholds remain configurable..env.example, rule 54, and archived task postmortem.Agent Preflight (Required)
Classification
External References
N/A: recovery used repository code, failed child transcript/session
72f25472-3701-4880-a935-93d9b397c3c1, SAM task evidence, and live staging/Cloudflare state; no external API contract changed.Codebase Impact Analysis
Impacted areas are
apps/apiD1 schema/migration, node lifecycle routes, scheduler cleanup, TaskRunner/trial/manual workspace selectors;packages/vm-agenthealth callbacks;.github/workflows/deploy-reusable.yml;scripts/deploy;apps/wwwdocs; and.claude/.agentsoperational rules and references.Documentation & Specs
Updated
apps/www/src/content/docs/docs/reference/configuration.md,apps/api/.env.example,.claude/skills/env-reference/SKILL.md,.agents/skills/env-reference/SKILL.md,.claude/rules/54-vm-agent-rollout-compatibility.md, and archived task postmortemtasks/archive/2026-08-06-fix-node-reaping-orphan-reconciliation.md.Constitution & Risk Check
Checked Principle XI / no hardcoded values: rollout required version is generated from
github.shaand can be unset forskip_agentor local deploys. Main operational risk was stranding legacy busy nodes, handled by drain-not-destroy logic and bounded cleanup.