Repository navigation
[E02] Persist isolated changes and immutable attempts #400
Description
Activity
Unblocked — with one scope boundary pinned before you start
E01 (#399) is closed; its contract is merged at
b868fd9. This issue is now eligible.Do not design or implement a protection-continuity mechanism here. That question — how a host establishes and attests
CurrentStorage.protection: continuous-since-initialization, where the initialization marker and lapse history live so agent tools cannot write them, and how a lapse is detected — is D3, and it now has its own issue: #418 (E12). It does not block this one.What E02 owns: the store persists records. It reads
protectionfrom host-supplied configuration outside the namespace and reports it faithfully, including reporting it as unknown or absent — which fails closed tocooperativeinclassifyAcceptance. That is the correct behaviour today and needs no new mechanism.What E02 must not do:
- invent a continuity mechanism of its own;
- write or derive a
protectionvalue from anything inside.codecarto/engineering/, or from any value a caller could author; - treat "the boundary is enforced right now" as evidence that it was enforced before the namespace's first write.
If E12 later lands a marker format, the store consumes it as an additive change.
Consequence to expect and not to work around: with D3 unresolved, every acceptance on every host currently classifies
cooperative. That is the contract working as designed. Do not add a bypass, a default, or a "trusted" fallback to make averifiedreading reachable.Two lessons from E01 that apply directly to this issue
- A trust rule that lives in a helper nothing calls is not a rule. E01 shipped
acceptanceTtlWithinas an exported predicate an adapter was trusted to call; nothing called it, and the shipped fixture violated it while still readingverified. Enforce rules at the point they gate a decision. - A degradation must lower trust, never raise it. The first fix for the presentation-disclosure gap made the bound records an optional context field defaulting to
[]— so a reader that simply omitted them skipped the check entirely and gotverifiedback. An optional input that gates a trust decision is a bypass waiting to happen. For this issue that means: a store that cannot determineprotectionreports that it cannot, and the reading degrades; it does not assume the favourable value.
Merged as
c78d3b2(PR #426). Closing E02.What shipped
core/engineering/store.ts— file-backed storage for engineering records, plustests/engineering-store.test.mjs(16) andtests/engineering-distribution.test.mjs(7).Two storage classes, because they are not the same kind of thing:
changeandsliceare mutable projections, replaced under compare-and-swap on the integerrevisionfield the contract already defines.- Everything under
attempts/— snapshot, proof, review, approval — is create-only. Editing a recorded observation is rewriting history, not recording it, so no revision the caller offers can replace one.
Publication is atomic (
atomicWriteFile, temp + rename), serialized per change byacquireLock, idempotent under a caller-supplied key, and contained: no write or read may resolve outside.codecarto/engineering/.listChanges()reports corrupt records alongside healthy ones with a reason rather than skipping them.Defects found and closed before merge
Three came out of adversarial review; each passed on the first implementation.
- Revision could move backwards (blocking). The check refused an equal revision but let a lower one through, turning CAS into an ABA race: roll back to 1 and every writer still holding
ifRevision: 1— including one stalled since before the intervening updates — passes its check and silently overwrites work it never saw.put(rev:1, ifRevision:3)was accepted, thenput(rev:2, ifRevision:1)won and v3 was gone. A CAS token that is not monotonic is not a CAS token. - Reads escaped the namespace while writes were contained. The containment check realpath'd
dirnamebut never the leaf, so achange.jsonthat was itself a symlink out of the namespace was read straight through —get()returned the outside contents andlistChanges()called the change healthy. Writes survived only becauserenamereplaces a link rather than following it, which is exactly why checking the write path missed it. - One idempotency key could serve two changes. The key is global; the lock was per-change. Concurrent puts to different changes sharing a key never serialized, both wrote, no conflict was reported, and the surviving receipt named only one of them — so retrying the other re-executed it.
Windows CI caught two more that Linux hid:
engineeringPathsspeaks POSIX while callers compose withpath.join, so every locked path resolved toengineering\engineering\...(eleven store tests failed on that alone); andnpmis a.cmdshim there, soexecFile("npm", …)died withspawn npm ENOENT.One was mine in the test rather than the code: the pack control required
.codecarto/engineering/to already exist, which is true on a machine that has done engineering work and false in every fresh checkout, since the directory is gitignored. It now plants a synthetic record, re-runs the pack, and removes it in afinally.What review confirmed holds
- Cross-process CAS is genuinely safe — 8 separate node processes racing one change, exactly 1 winner, 7
stale-revision.acquireLockis a real filesystem bakery lock. - A stale ticket from a killed process is broken in ~2 ms. No hang, no residue.
listChanges()does not see.lockfiles: the lock lives atchanges/<id>.lockbut the loop filters to directories.- Distribution isolation is real and the two mechanisms are independently sufficient. Removing either alone still yields a clean tarball; removing both leaks. Because the end-to-end pack test can only catch losing both, each mechanism is pinned separately. No fourth leak path was found.
Verification: 16/16 store, 7/7 distribution, 1032/1032 full suite, build exit 0, nine mutation checks bite individually.
Note for the next worker: E01's module-graph test pins the exact file list and import layering of
core/engineering/. The purity rule was split rather than relaxed — the store may touchnode:fsbecause that is its job; every other module in the namespace still may not, and that is verified by injecting anode:fsimport intovalidation.tsand confirming the guard fails.Next: #402 (E04) is the first task that uses this store rather than adding to the contract. #395 goes first — it is the same POSIX-path class that cost two CI rounds here.
Context and scope
Part of the incremental CodeCartographer engineering evolution. Planned, not shipped. Read the agent handoff, vision/decisions, record contract, and implementation plan. Documentation baseline PR #410 is merged at
bbdf6b1a8b3bc348aa9e8f20a409c66df087af13. Follow this issue's prerequisite gates before starting.Host-executed/framework-tracked; preserve analysis state ABI, both Pi analysis guards, and current synthesis confirmation gates. Ordinary in-place changes accept zero external references. No implicit source execution, provider spend, GitHub write, release/deployment, or private-data publication. E01 owns schema/API/authority decisions; dependent workers consume its merged contract rather than inventing another.
Tracking: #398
Blocked by: #399. Prerequisites must be merged, not merely started.
Objective: preserve one change's history while creating/resuming another, with retry-safe framework ownership.
Depends on: E01. Files: create
core/engineering/store.ts,tests/engineering-store.test.mjs,tests/engineering-distribution.test.mjs; update the engineering barrel,core/workspace.tstemplate-copy exclusion sets,package.jsonfiles exclusions,.codecarto/.gitignore, and.codecarto/templates/gitignore. Extendtests/init-workspace-isolation.test.mjswhere appropriate. Reuse appropriate primitives fromcore/utils.ts/core/status.tsafter inspecting their actual contracts.Steps:
workflow/status.yamlor another change..codecarto/engineering/runtime state from template copying, default Git tracking, and npm packaging. Keep this exclusion distinct from distributable templates and schemas; deliberate shareable exports belong outside the private runtime namespace. Add synthetic-source initialization tests and a packed-file inventory test proving histories, approvals, and artifacts reach neither a fresh workspace nor the tarball. Never use real private records as fixtures.Acceptance: second change and retry do not overwrite history; duplicate ingestion is idempotent; stale revisions fail clearly; interruption leaves a recoverable state; existing analysis files remain untouched. Synthetic engineering records are ignored by default and absent from fresh-workspace copies and actual npm tarballs, while distributable guidance still arrives. Storage is not complete until these distribution-isolation checks pass.
Out of scope: distributed scheduling, database migration, automatic worktree management, or filesystem reorganization of legacy analyses.
Required verification and handoff
npm run build,npm test, andgit diff --check; record actual results, not historical counts.