docs(plan): P5F-2 writer + resolver step-tables#147
Open
lantisprime wants to merge 3 commits into
Open
Conversation
added 3 commits
July 8, 2026 09:45
Authors Appendix B (P5F-2) post-P5F-1-merge: additive writeProjectTrustStore + atomic-write helper + resolveDefaultBackend, anchored against the shipped reader at agents/lib/bg-trust.ts@08a9be6. Three import edits to bg-trust.ts (renameSync, ./bg-terminal.ts, append) + append tests to test-bg-trust.mjs. Tests: 3 verbatim load-bearing bodies (testWrite_atomicFailureLeavesFinalUntouched discriminating, testReadTrustStore_rejectsAfterKeyRotation red-then-green, testResolveDefaultBackend_fallsBackWhenBackendUnregistered discriminating) + 4 contract-driven tests (high-capability executor scope, per P5F-1 step 1.2 precedent). P5F-2 target: 20/20 green (13 P5F-1 + 7 P5F-2). All ANCHORs verified verbatim against shipped bg-trust.ts / test-bg-trust.mjs / bg-terminal.ts / bg-state.ts. Plan-only slice; pending codex 5-pass plan review (mirrors PR #143 flow). P5F-3 step-tables remain deferred by design.
…RefError Pass-1 codex review (PR #147) returned changes-requested with 2 findings, both legit: P1_BLOCKER (Group-4 contract row): testResolveDefaultBackend_fallsBackWhenStoreForged was mislabeled — it did an ABSENT store, not a FORGED one, so the absent-vs-forged negative controls collapsed and REQ-5's forged fail-closed path went unexercised. Fix: promoted it to a 4th verbatim body that registers a stub backend, writes a valid store, then key-rotates (.trust.mac rm + readOrCreateProjectTrustKey mints key2) so readProjectTrustStore returns {ok:false,reason:forged} (State G MAC mismatch); asserts resolveDefaultBackend -> null EVEN THOUGH the backend is registered (discriminating: isolates the !read.ok fail-closed path from the unregistered-backend null path). P2_NIT (Group-3 contract row testWrite_atomicTempRename): referenced trustFilePath which is module-private/unexported -> a literal executor hits ReferenceError. Fix: spell the path with path.join(d,'.pi/trust/default-backend.json'); use lstatSync(.).mode for the 0600 check. Drive-by: dropped now-unused statSync from the (a.1) node:fs import edit; intro + executor note recount (3->4 verbatim, 4->3 contract-driven); added registry-reset guard note for the 2 resolver tests that touch the global backend registry.
… tests
Pass-2 re-review confirmed FIX 1 + FIX 2 (Pass-1 P1/P2) resolved, but raised a
NEW P1: the two resolver tests that register the __p5f-stub stub
(testResolveDefaultBackend_returnsTrustedName + testResolveDefaultBackend_fallsBackWhenStoreForged)
both __resetBgTerminalBackend() (which wipes the WHOLE registry) under the concurrent
fire-and-forget test() shim — one's reset can clear the other's stub mid-resolve, flaking
returnsTrustedName. try/finally-before/after is not real isolation under concurrency.
Fix: serialize both on a shared async mutex (withRegistryLock) + try/finally cleanup.
- Added registryLock/withRegistryLock helper at the top of the appended verbatim block.
- Wrapped the forged verbatim body in await withRegistryLock(async()=>{ try{...}finally{reset} }).
- returnsTrustedName contract row now specifies the same wrap.
- Updated the standalone registry note + executor note to reference the lock.
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.
What
Authors Appendix B (P5F-2) — the mechanical execution spec for the P5F-2 slice (writer + default-backend resolver), appended post-P5F-1-merge and anchored against the shipped reader at
agents/lib/bg-trust.ts(08a9be6).Plan-only PR (mirrors the #143 P5F plan-doc precedent). No code changes.
Scope of the P5F-2 slice (additive, no P5F-1 body rewrites)
renameSynctonode:fs; add./bg-terminal.tsimport (getBgTerminalBackendByName); APPENDwriteProjectTrustStore(REQ-6, sole writer, temp+rename atomic 0600) +atomicWriteTrustStoreSync+resolveDefaultBackend(REQ-5/8, three-state fail-closed). Zero production callers (P5F-3 wires the caller).testWrite_atomicFailureLeavesFinalUntouched— discriminating (injected throwingnow()proves full-store-before-write atomicity).testReadTrustStore_rejectsAfterKeyRotation— red-then-green (MAC-key binding State G).testResolveDefaultBackend_fallsBackWhenBackendUnregistered— discriminating (non-null REQUIRES a registered backend, not merely a valid store).Review status
All ANCHORs verified verbatim against shipped
bg-trust.ts/test-bg-trust.mjs/bg-terminal.ts/bg-state.ts. Pending codex 5-pass plan review (the established cmux consensus loop) before implementation.Out of scope
index.tsL713 +bg-preflight.ts) — deferred by design, authored after P5F-2 review.feat/p5f-2branch after plan acceptance).