Record VM ownership only after the launched process execs

[artemnikitin]

Fixes the VM ownership recovery defects in #41.

Design reference

There is no #41 design doc. firework-vm-adoption.md (issue #22) governs this code, and this PR brings the implementation back in line with its §5 step 6: "Wait boundedly for the launcher PID, then read its boot ID, start ticks, and executable device/inode identity." The shipped code inspected immediately and discarded the error. This is a divergence fix, not new design.

Root cause

systemd-run reports a transient unit's MainPID at fork, before the child has exec'd into Firecracker. Manager.Start inspected that PID immediately, so the recorded identity described either nothing (inspect failed) or systemd itself (inspect succeeded, raced). Both persist a running manifest that validateOwnedProcess can never accept again, so the VM is quarantined into recovery_pending; Manager.Remove refuses a quarantined instance, the planned ActionUpdate fails identically every 30s, applied_revision freezes, and every service on that node projects unknown.

Both variants were confirmed live (2026-07-26 and 2026-08-08). The recorded process_start_ticks matched the live values exactly, which proves the right PID was inspected too early rather than the wrong PID resolved — so the fix belongs at when identity is recorded.

Changes

Manager.Start waits for the exec before recording identity. The signal is the command line, not the executable: the launched process must present this instance's unique --id together with its socket and config paths. The identity is taken from that same inspection, so the persisted manifest validates by construction. Argv rather than executable equality also keeps the shell-script fake Firecracker in smoke-local working.

A launch whose identity cannot be confirmed is abandoned, not recorded. It is killed (unit-scoped for systemd; an unreaped child for direct), and its state directory is removed only once the process is proven gone — leaving it in place otherwise, per the design's rule that a manifest is deleted only after its process is. A launch that failed before owning any process no longer blocks every later start of that service for the life of the agent process.

internal/vm/recovery.go's starting-manifest path had the identical swallowed Inspect and is fixed the same way. It is not in the issue's file list.

Recovery's once-per-process guard is armed even when the VM state directory is absent. On a fresh node it appears only after this process creates its first VM, so the single real pass landed in the process that created them. Services already tracked in memory are skipped as well. The guard is deliberately left unarmed on a non-ErrNotExist read failure, so a transient error retries rather than disabling recovery permanently.

A manifest that fails validation is re-proved from the live command line before being quarantined. The instance ID is 128 bits from crypto/rand, generated once per launch and passed as --id; a process presenting it with this instance's socket and config paths is provably the described process, whatever the recorded identity says. The identity is then repaired and persisted. A disagreeing process_start_ticks is never repaired — that is the PID-reuse signal the manifest exists to catch. This is what unsticks nodes already stalled in production: they recover on the next agent restart.

Unrecorded and mismatched identity are now distinct errors, per field. An absent field proves nothing about the process, so it stays quarantined; a host boot ID mismatch is now classified as a process that did not survive — the boot ID is host-global and read fresh, so a mismatch proves the recorded PID cannot be the recorded process — and its state is cleaned instead of quarantined.

Projection no longer erases a whole node. A node that has observed but not applied the current revision keeps agent_status_revision_mismatch on every service placed on it, but its fresh per-service observations are projected instead of stamped unknown. A service whose own vm_state is outside the published vocabulary (recovery_pending) still reports unknown, so the service that actually cannot converge is the one that stands out. nodeSummary's RunningServices is counted from the same fresh status so the node list cannot contradict the service list. Service detail keeps the stricter rule: runtime fields still require an applied revision.

Acceptance criteria

#	Criterion	Status
1	Recovery never adopts or quarantines a VM this process launched	done
2	Post-launch inspection failure surfaces as an error, no unvalidatable manifest	done
3	Unrecorded and mismatched identity produce distinct errors	done
4	A host reboot cleans up instead of quarantining	done
5	recovery_pending clearable through a supported interface	not delivered
6	A quarantined service does not erase fresh status for others on the node	done
7	Tests	see below

Criterion 5 is deliberately deferred

Every available mechanism is a bad fit for a bug fix and needs its own design decision:

• the agent API (internal/api/server.go) is GET-only and unauthenticated, so a mutating endpoint is new attack surface;
• fireworkctl talks to the control plane, and the architecture is pull-based — there is no push channel to agents;
• a firework-agent subcommand editing the state directory would not clear the running agent's in-memory recovery_pending.

What this PR does instead is make quarantine much rarer (the race is fixed) and self-healing for the provable case (ownership repair on the next agent restart). The manual procedure — stop the agent, remove <state_dir>/vms/<service>/, start it — is now documented in DESIGN.md, along with the fact that persistent volumes live outside that directory. Happy to split criterion 5 into its own issue.

Tests

• exec race: a launcher reporting a pre-exec PID plus an inspector returning systemd's identity until the exec completes — asserts the manifest holds Firecracker's identity, never systemd's, and validates immediately;
• an unprovable identity is abandoned, killed, and cleaned up; a surviving process instead retains its state;
• a failed launch does not block a later start;
• recovery on a fresh node where the directory appears mid-process;
• an empty identity set produces the distinct "never recorded" error and stays quarantined;
• a boot-ID mismatch after a simulated reboot cleans the directory;
• ownership repair for both live variants, and no repair for a recycled PID;
• the reconcile stall, as "an update against a repaired survivor no longer stalls". Note the narrower framing: Remove still refuses a genuinely quarantined instance, which is unchanged and still covered by TestRecoverQuarantinesPIDReuseAndNeverSignalsIt. What changed is that the raced VMs are no longer quarantined in the first place;
• projection keeps a healthy service visible on a stalled node while the quarantined one stays unknown, and the node summary agrees.

Notes for review

• Not fixed here: design §7 says recovery should run at agent startup, before the first reconcile; it is still invoked from Reconciler.Reconcile. That placement is the underlying reason defect 1 existed. The guard fix satisfies the acceptance criterion; moving the call is a larger refactor and is left out deliberately.
• Timing: the identity wait (3s) and the abandon-exit wait (2s) are spent holding the manager lock on the reconcile path. Worst case per failed service is ~5s, inside the 30s reconcile interval and the API server's 10s write timeout. Overrunning either budget is benign — the launch is killed, cleaned up, and retried on the next tick. Observed warm launches are ~110ms and a cold systemd-run ~1.2s.
• Behavior change worth knowing: a Firecracker that exits before it runs (a bad config, a missing kernel) is now reported as a start error by the reconciler rather than as a VM that started and then failed. That surfaces the failure synchronously instead of via the monitor, and it is why the fake VM binary in TestTick_StatusReportsVMProcessFailureWithoutExitedPID now has to survive its identity confirmation to still be testing the monitor.
• Non-Linux hosts have no /proc, so ownership is unprovable there by construction. Only the real inspector on such a platform falls open with a warning, which preserves the local dev loop (make smoke-local passes); tests and Linux always take the strict path.

Validation

make fmt, make test, make lint (incl. staticcheck), make test-race, and make smoke-local all pass.

🤖 Generated with Claude Code