feat(engine): autopilot resumes across reload/restart via persisted intent (v2.9.0)#50
Conversation
…ntent (v2.9.0)
Every plugin reload / app restart stopped the autopilot and needed a manual
st_autopilot_start (3x in one day of live ops) — the fleet-engine surface's
stop hook called stop_ops() on reload, and the start hook only armed watches
("starts on demand"). Now the surface RESUMES a running autopilot the reload
interrupted, using the new register_surface reload/start lifecycle, without
ever auto-starting one the operator deliberately stopped.
- Persist operator INTENT (watch-state 'autopilot_wanted'): start_ops sets it,
stop_ops + request_stop (goal wind-down) clear it. Survives module reload.
- New lifecycle_stop() halts the task for shutdown/reload but KEEPS the intent;
_fleet_surface_stop uses it instead of stop_ops (a reload isn't an operator
turning it off).
- _fleet_surface_start resumes start_ops() when intent stands — so the engine
self-recovers from reloads/restarts.
- Epoch reset drops the intent, so a wipe defers to st-reset's recovery playbook
(no blind resume before a fleet exists).
- 5 tests incl. the safety property: deliberate stop → reload → stays stopped.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #50 | feat(engine): autopilot resumes across reload/restart via persisted intent (v2.9.0)
VERDICT: WARN
CI Status
- test: in_progress
⏳ CI is still settling. Per approve-on-green policy, this review promotes to APPROVED automatically once every check passes terminal-green — no manual re-review needed.
Diff Review
__init__.py:_fleet_surface_startnow resumes autopilot whenfleet.autopilot_intended()is true;_fleet_surface_stopswapped fromstop_ops()→lifecycle_stop()to preserve intent across reloads.fleet.py: New intent-persistence layer (_set_autopilot_intent,autopilot_intended) backed by watch-state file.start_ops/stop_ops/request_stopset/clear intent; newlifecycle_stop()stops the engine task but leaves intent untouched.test_autopilot_resume.py: 5 tests with FakeEngine through real start/stop/lifecycle_stop paths — safety property (deliberate stop → reload → stays stopped) covered.- Version bump 2.8.1 → 2.9.0.
Observations
- LOW:
_set_autopilot_intent()swallows all exceptions with barepass(fleet.py:2295). If the watch-state file is corrupted or I/O fails, intent silently disappears rather than surfacing — intent is advisory so this is acceptable, but worth a comment noting the degraded mode. - LOW:
lifecycle_stop()does not emitengine_stopped(unlikestop_ops()). Any downstream listener depending on that event during shutdown will break silently. This is likely intentional (a lifecycle stop is a platform event, not an operator decision), but worth confirming no listeners rely on it. - LOW: Clawpatch structural review mapped 7 features, reviewed 4, returned 2 findings — none surfaced as HIGH/CRITICAL.
— Quinn, QA Engineer
|
Submitted COMMENT review on #50. |
|
✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748). |
There was a problem hiding this comment.
QA Audit — PR #50 | feat(engine): autopilot resumes across reload/restart via persisted intent (v2.9.0)
VERDICT: WARN (CI pending — auto-promotes to APPROVED on terminal-green per approve-on-green policy #748)
CI Status
- test: queued
Diff Review
- 5 files, +252/-5: persisted autopilot intent in
fleet.py+ surface lifecycle hook in__init__.py+ version 2.9.0 + new test module. - Design is safe by construction:
lifecycle_stop()preserves intent (reload → resume),stop_ops()/request_stop()clear it (deliberate → stays stopped). Epoch reset drops intent naturally via watch-state clearing. _set_autopilot_intentread-back verification with fail-safe key deletion correctly handles the asymmetric failure modes — a lost clear is the dangerous case, and the fallback makes it safe.
Observations
- LOW:
_set_autopilot_intentimportswatchesinside the retry loop body. Python caches imports so this is harmless, but hoisting it outside the loop would be cleaner. - Prior review (APPROVED at
839df96) stands — incremental changes since are refinements only, no new blockers. - No unresolved CodeRabbit threads.
— Quinn, QA Engineer
|
Submitted COMMENT review on #50. |
… set before guard, fail-safe clear Adversarial safety review of the v2.9.0 resume feature found three real holes: - BLOCKER: clear_epoch_state dropped the intent on a wipe, but the RUNNING engine survives a wipe (4113 → _recover keeps it up without start_ops) — desyncing it so the next restart wouldn't resume. Fix: intent persists across epochs (removed from clear_epoch_state); the operator still wants it running post-wipe. - BLOCKER: start_ops set intent AFTER the "already running" guard, so it could never be (re)asserted while the engine was up. Fix: set intent first — st_autopilot_start on a live engine now repairs the intent. - SAFETY: a failed CLEAR write left the flag True (a reload would resurrect a stopped engine). Fix: _set_autopilot_intent writes then reads back and retries; a persistent failure to clear DELETES the key (absent = not-wanted = no resume). Tests updated: intent survives epoch reset; start-while-running records intent; the stop-write-failure fallback reads False. 325 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
451152a to
1df7327
Compare
|
✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748). |
…clear fail-safe is reachable The re-review found the delete-fallback's WARN hung off an exception save_state never raises (it swallows + WARNs), so under a persistent dead state layer the clear silently no-op'd and left the flag stuck True → a reload could resurrect a stopped engine, with no signal. Now save_state RETURNS success; _set_autopilot_intent gates on the bool + a read-back, and the delete-fallback's WARN fires when a clear genuinely can't persist. The unfixable residual (you can't clear a flag on a totally dead disk) is now LOUD in the engine log instead of silent — honest + recoverable (operator re-stops after a restart). 2 tests: transient clear-write failure falls back to key-deletion; a fully dead disk keeps the flag but logs the WARN. 326 pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review resolution (2 adversarial rounds)Blocker 1 (wipe drops intent → desync) — CLOSED. Blocker 2 (intent set after the guard) — CLOSED. Finding 3 (blind resume into dead universe) — not harmfully reintroduced; the resumed engine's calls 401 and Finding 4 (failed CLEAR stuck True → unsafe resume) — actionable part CLOSED. 326 tests (8 new, incl. the safety property, the wipe-survival, and both the transient and dead-disk clear-failure paths), ruff clean, CI green. |
There was a problem hiding this comment.
QA Audit — PR #50 | feat(engine): autopilot resumes across reload/restart via persisted intent (v2.9.0)
VERDICT: WARN (CI pending — auto-approve on terminal-green)
CI Status
- test: in_progress
Diff Review
- 5 files, +267/-6: intent persistence in
fleet.py, surface wiring in__init__.py, new test suite, version bump - Incremental re-review from prior APPROVE at
1df7327→ head3fadaa5: same feature set, design unchanged, no regressions
Observations
- CLAWPATCH/LOW: No feature slices touched — clawpatch mapped 7 features but found 0 touched by diff; expected for lifecycle/infra plumbing
- No unresolved CodeRabbit threads
- Prior review was APPROVED — incremental changes at
3fadaa5carry forward cleanly
Auto-approve policy: Once test completes terminal-green with no new blockers, the approve-on-green policy (#748) promotes this review to APPROVED and the PR merges automatically. No further manual review needed.
— Quinn, QA Engineer
|
Submitted COMMENT review on #50. |
1 participant
Summary
Live ops today needed a manual
st_autopilot_startafter every reload/restart (3×: app restarts at 11:16 and 20:17, a plugin reload at 21:32) — the fleet-engine surface stopped the engine on reload but never resumed it. This wires the newly-addedregister_surfacereload/start lifecycle so the autopilot self-recovers, while strictly never auto-starting against a deliberate operator stop.Design (safe by construction):
autopilot_wantedin watch state):start_opssets it true;stop_opsandrequest_stop(goal wind-down) clear it. It survives the module reload that resets_ENGINE.lifecycle_stop()— new: halts the task for shutdown/reload but leaves the intent untouched (a reload is not an operator decision)._fleet_surface_stopuses it instead ofstop_ops._fleet_surface_startresumesstart_ops(window)when the intent stands.Test plan
deliberate stop → reload → stays stopped), lifecycle-stop keeps intent, goal wind-down clears it, epoch reset drops it🤖 Generated with Claude Code