Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Fix the failing snapshot test and add tombstone assertions; clarify the LoopList documentation.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Documents runtime continuity boundaries and safe recovery behavior without changing production runtime implementation.
Changes:
- Adds continuity guidance and recovery steps.
- Clarifies orchestration wake retention and retirement.
- Adds continuity tests and package inclusion checks.
| File | Summary |
|---|---|
test/runtime-continuity.test.ts |
Adds continuity invariant tests. Critical: snapshot comparison fails when .prev is absent. Moderate: does not verify process-local tombstones. |
test/orchestration-runtime.test.ts |
Tests orchestration wake expiry retirement. |
scripts/package-smoke.mjs |
Verifies the guide is included in packages. |
README.md |
Links to continuity guidance. |
package.json |
Packages the new documentation. |
docs/RUNTIME_CONTINUITY.md |
Defines continuity boundaries and recovery steps. Nit: distinguish LoopList from the authoritative lifecycle field view. |
docs/REFERENCE.md |
Updates runtime continuity references. |
AGENTS.md |
Aligns guidance with wake-retention behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function bytes(path: string) { | ||
| return [readFileSync(path, "utf8"), readFileSync(`${path}.prev`, "utf8")]; | ||
| } |
There was a problem hiding this comment.
Published follow-up: 7cb92ab1632ac04816e56eb986f68854d5f59fc9 (signed; normal push hooks also pass the 1,225-test suite).
Verified against the reviewed head aa3524947112302911179568c95c0f4d1e83ee9a: all 11 continuity tests pass unchanged, as do its Linux/Windows CI checks.
The first-write observation is correct, but these callers do not read .prev immediately after create:
- Ordinary byte checks follow
store.fire(), which performs a second save and writes the prior primary snapshot to.prev. - Orchestration byte checks follow two applied mutations, which also create the recovery snapshot.
Keeping the strict read is intentional: these byte-preservation checks require an actual recovery snapshot. Making missing backups optional would weaken that invariant. The follow-up adds explicit first-write absence/strict-helper failure and second-write recovery-content assertions.
Also strengthened deletion/fire-limit/expiry assertions to the exact current serialized shape { nextId: 2, loops: [] }, excluding additional tombstone/history fields, and corrected a separate documentation overstatement: LoopList includes current controller data and workflow execution/outcome guidance, but resolved storage and the generic action menu belong to /loop inspection.
Validation: 13 continuity tests; 127 focused tests; final 72-file/1,225-test coverage suite, 22 overlapping properties, typechecks, build/package smoke124/public types, audit0, fresh LSP and diff checks pass. Two established lint warnings remain. The documentation assertion was red before the copy correction; no missing-snapshot product defect was reproduced.

Stack
Depends on #142 (
test/workflow-first-attempt-metrics), then #140 → #139. No merge, auto-merge, release, or history rewrite requested.Change
Evidence
Expectation-first missing-matrix assertion:1 failure /9 passing behavioral controls, then green. Independent review caught an overbroad retention sentence; added explicit copy expectation plus cancellation/expiry controls before correction. Follow-up focused7 files /244 tests pass; fresh LSP clean. Two earlier fixture assumptions were corrected separately—not classified as product defects: raw
Store.fireis accounting rather than runtime one-shot cleanup; orchestration CAS preserves primary state but does not inherit the workflow-only recovery-byte no-rotation policy.Actual tarball inclusion assertion fails before adding the new guide to
package.json.files; final package smoke must pass that same check. Mock managers use explicit POSIX platform so Windows runs cannot invoke realtaskkillon a fixture PID.Final gate:72 files /1,223 coverage tests;22 overlapping properties; lint with two established warnings; production/development typechecks, build, actual package smoke124/public types, audit0, diff checks, and fresh LSP3 paths pass. Coverage:90.60% statements,84.95% branches,96.43% functions,95.28% lines. Normal push hooks will rerun the gate.
Main read-only review found the retention qualification above; narrow replacement follow-up found no remaining scoped factual/invariant blocker (turn-limited, no independent test rerun). No current continuity defect or measured operator benefit is claimed.
Limits
No production runtime implementation changed. No daemon, ordinary wake outbox, scheduler election, or recoverable-monitor implementation. Tests use controlled file-backed state, fake timers/notifications, and mock handles; they do not simulate OS death, prove orphan cleanup/duplicate execution, or measure operator comprehension. Recovery snapshots are not a removal journal.
.vitest/remains preserved and excluded.