docs(plan): P5F disk-backed per-project trust reader#143
Merged
Conversation
added 7 commits
July 4, 2026 20:57
Adds agents/docs/P5F_DISK_BACKED_TRUST_READER_PLAN.md — a planning-only document for the disk-backed per-project trust reader that unblocks both the persistent per-project default backend and P4R-PROJ. Composes with the existing P4R MAC lifecycle (bg-state.ts) and the P5-NL-bg projectTrusted manifest snapshot rather than introducing a parallel signing scheme. 3 slices: P5F-1 (pure reader extraction), P5F-2 (writer + resolver), P5F-3 (read-side wiring). 11 REQs, 16 unit tests + 2 static + 1 UNGUARDED-IN-CI smoke. Planning only — pending plan review + adversarial review.
Adds agents/docs/P5F_REVIEW.md — codex (gpt-5.5 high) plan review of
P5F_DISK_BACKED_TRUST_READER_PLAN.md via cmux (surface:48, workspace:31).
Verdict: changes-requested. 6 blockers, 3 non-blocking, 6 missing tests.
Key factual findings (independently ground-checked):
- INV-2 wrong global MAC path: actual ~/.pi/agent/bg/.session.mac
(bg-state.ts:134,138), NOT ~/.episodic-memory/.session.mac
- REQ-9 wrong: projectTrusted is boolean (bg-state.ts:55), not a
{projectRootSha256, keyGenId} snapshot
- index.ts hook citation: select call at L695, not L685-691
- REQ-1/2/3 contract: return-type/throw-semantics inconsistent
Plan architecturally sound; needs factual + test-design corrections
before re-review pass 2.
…+ audit)
Resolves all 6 changes-requested blockers from P5F_REVIEW.md:
1. REQ-9 corrected: projectTrusted is boolean (bg-state.ts:55), not a
{projectRootSha256,keyGenId} snapshot. Plan now sources only the
boolean from disk; snapshot enrichment deferred to P4R-PROJ.
2. REQ-4/EC2 discriminating fixture: inject A's .trust.mac into B so
MAC passes, assert A/B roots differ first, then assert cross-read
returns forged. Test renamed _withSharedKeySentinel.
3. REQ-1/2/3 contract unified on ProjectTrustReadResult union. Trust-file
symlink -> {ok:false,reason:'symlink'} (not throw); key-file symlink
throws via assertNoSymlink (now explicit in contract error table).
4. INV-2 global MAC path corrected: ~/.pi/agent/bg/.session.mac
(bg-state.ts:134/138), NOT ~/.episodic-memory/.session.mac. INV-2
grep guard now also blocks getBgStateDir/getBgSessionMacPath imports.
5. 9-state test coverage complete: added C/D/E + REQ-10 mac-ordering
(automated, not UNGUARDED-IN-CI). Group 1 expanded 6->10 tests.
6. Appendix B P5F-1 executor-ready: OD-1 resolved in-plan (no 'Decide');
step 1.1 provides verbatim bg-trust.ts source; step 1.2 test import
path fixed to ../lib/bg-state.ts.
Audit fix: index.ts hook citation corrected L685-691 -> L685 parse /
L695 explicit / L713 selectBgTerminalBackend fallback.
Review Consensus table: Pass 1 row filled (codex changes-requested, 6
blockers); Resolved blockers table added. Test catalog 16->23 unit +
2 static + 1 smoke.
…Pass 2)
Pass 2 (codex changes-requested) verified Pass-1 blockers 1-5 + audit
RESOLVED. Only remaining issues were execution defects in the verbatim
bg-trust.ts + test-bg-trust.mjs source blocks (same root cause):
2.1 Private-helper imports: bg-trust.ts imported PRIVATE assertNoSymlink
+ readUtf8FileNoSymlink (bg-state.ts:619,742 — not exported). Inlined
local equivalents (assertNoSymlinkLocal/readUtf8FileNoSymlinkLocal);
import block now imports only exported signBgPayload/verifyBgPayloadMac/
keyGenIdFromKey.
2.2 Missing await on async readUtf8FileNoSymlink (raw was a Promise ->
States D-I bypassed). Added await -> States C-I execute.
2.3 State E wrong side-effect: readProjectTrustStore called
readOrCreateProjectTrustKey (CREATES missing key). Added read-only
readProjectTrustKey; readProjectTrustStore uses it. State E: ENOENT
-> forged (no create). readOrCreateProjectTrustKey now delegates to
readProjectTrustKey + reserved for the P5F-2 writer.
2.4 Key-symlink throw contract: readProjectTrustStore now catches ONLY
ENOENT (-> forged); symlink throw from assertNoSymlinkLocal propagates
per Contract error-codes table.
2.5 Test excerpt: added mkdirSync/readFileSync imports; mintStore is now
async + awaits readOrCreateProjectTrustKey; all 3 call sites await it.
Review Consensus + Resolved-blockers tables updated with Pass 2 entry.
Status: Pass 1+2 addressed, awaiting Pass 3.
…cutor (Pass 3) Pass 3 (codex changes-requested): all 5 Pass-2 defects RESOLVED; only remaining blocker was test block being an excerpt, not full verbatim 12-test source as the (false) 'Full verbatim test source' claim stated. Resolution: re-scope P5F-1 step 1.2 to high-capability-executor scope (PLAN_TEMPLATE-sanctioned — Appendix B's executor-ready gate may be scoped to high-capability-only when the plan won't be implemented by a low-capability model; P5F-1 will be run by the orchestrator's default pi model per cmux-orchestrator skill, not a cheap sub-agent). - Step 1.2 table row: dropped false 'Full verbatim' claim; states 4 verbatim + 8 named-test contracts + high-capability scope. - Appendix B: added explicit Scope clause distinguishing step 1.1 (full verbatim, low-capability-ready) from step 1.2 (high-capability). - Step 1.2 header + preamble: honest re-scope. - Closing note: replaced 'fill in 8 tests' stub with an explicit 8-test contract table (name, asserted read-state, red-then-green/discriminating flag) — the executor authors these to match exactly. Review Consensus + Status updated with Pass 3 entry; awaiting Pass 4.
Pass 4 (codex changes-requested): re-scope Q1+Q2 RESOLVED; Q3+Q4 NOT: - Q3: contract row for macCheckedBeforeRootCompare 'punts ordering to manual grep', contradicting REQ-10's 'automated mock-injection' claim — an internal contradiction. - Q4: residual '6 follow the same skeleton' / '8 more tests' comments inside the verbatim block, inconsistent with '4 verbatim + 8 contracts'. Revision 4 resolves both honestly: - Renamed testReadTrustStore_macCheckedBeforeRootCompare -> testReadTrustStore_macAndRootBothChecked. The automated test now honestly asserts BOTH checks execute (State G wrong-mac-valid-root -> forged AND State H valid-mac-wrong-root -> forged). - ORDERING (G-before-H) is mechanically unverifiable in plain ESM without a mock library, and REQ-11 forbids new runtime deps (no mock framework). Ordering is therefore UNGUARDED-IN-CI via a named manual grep (verifyBgPayloadMac call line < projectRootSha256 compare line in bg-trust.ts). Made CONSISTENT across REQ-10 row, Safety matrix, catalog Group 1 + Group 6, contract table, Done Criteria. - Fixed residual verbatim-block comments: '6 follow the same skeleton' -> '3 verbatim + 7 contracted per the 8-test contract table'; the '8 more tests' comment now references the contract table by name. Review Consensus + Status updated with Pass 4 entry; awaiting Pass 5.
Pass 5 (codex): all 4 audit questions RESOLVED. - Q1: macCheckedBeforeRootCompare rename complete (only in historical row). - Q2: REQ-10 consistent across all 5 sites. - Q3: UNGUARDED-IN-CI tag + named manual grep template-compliant. - Q4: verbatim-block comments fixed, point to 8-test contract table. ## Verdict — approve → CONSENSUS. Status: plan ACCEPTED for slicing P5F-1 by a high-capability executor. Review Consensus table Pass 5 row filled; Status line updated. cmux workspace:32 closed.
lantiscooperdev
approved these changes
Jul 4, 2026
lantiscooperdev
pushed a commit
that referenced
this pull request
Jul 4, 2026
… (#144) Canonical workplan episode revised: 20260704-122521-...-d94b -> 20260704-141949-...-a0b1 WORKPLAN.md: chain-head pointer + Current-state line + Open-follow-ups list updated. P5F-1 marked as NEXT. P4R-PROJ + persistent-default- backend now annotated as 'unblocked via P5F (shipping)'. agents/P3_IMPLEMENTATION_SLICES.md: added P5F section with slice ladder (P5F-1 / P5F-2 / P5F-3), 5-round codex review trail, key load-bearing facts (projectTrusted boolean bg-state.ts:55; global MAC at ~/.pi/agent/bg/.session.mac; assertNoSymlink/readUtf8FileNoSymlink PRIVATE so bg-trust.ts inlines local equivalents), cmux process lesson. Planning-only merge; no implementation. P5F-1 ready to slice. Co-authored-by: Charlton D. Ho <dev@znp.pw>
This was referenced Jul 5, 2026
lantiscooperdev
pushed a commit
that referenced
this pull request
Jul 5, 2026
…APPROVED] (#145) * P5F-1: disk-backed per-project trust reader (pure extraction) Implements P5F-1 per the accepted P5F plan (PR #143, 0ca3ee6). Zero production callers — pure extraction. The trust module is imported only by tests until P5F-3 wires /agents bg + preflight. Files: - agents/lib/bg-trust.ts (new): resolveProjectRoot, projectRootSha256, sha256Hex, readOrCreateProjectTrustKey, readProjectTrustKey, readProjectTrustStore. Local no-follow helpers inlined to honor INV-2 (private bg-state.ts assertNoSymlink/readUtf8FileNoSymlink not imported). Sign/verify/keyGenId reuse bg-state.ts primitives. - agents/test/test-bg-trust.mjs (new): 12 tests covering all 9 reader states (A-H + I) plus REQ-7 key isolation, REQ-10 both-checks, and the projectTrustStore_symlink red-then-green guard. - agents/test/run-bg-trust-tests.sh (new): slice runner. Verified: - bash agents/test/run-bg-trust-tests.sh -> 12/12 passing - BREAK=1 forces foreign-root fixture to non-discriminating -> exit 1 - REQ-11 grep: only node:* + ./bg-state imports (no third-party) - INV-2 grep: empty (no resolveTrustedHome/readOrCreateSessionMacKey/ getBgStateDir/getBgSessionMacPath references) - REQ-10 ordering: verifyBgPayloadMac (L179) precedes projectRootSha256 compare (L182) - MAC before root, source order. - Existing bg-state + P5E1 suites still green (pure extraction). Note on the plan's verbatim bg-trust.ts source: the docstring comment on readOrCreateProjectTrustKey originally referenced readOrCreateSessionMacKey verbatim, which the INV-2 grep would pick up as a false positive. Reworded to "the session-MAC primitive at bg-state.ts:167-189" - semantic content preserved (line citation intact), forbidden substring removed. All other source matches the Appendix B step 1.1 verbatim block. Co-Authored-By: Claude <noreply@anthropic.com> * P5F-1 fix: fail-closed on malformed project trust key (codex P1 blocker) Addresses P1_BLOCKER from codex review of e51139d: readProjectTrustStore rethrew malformed-key failures instead of failing closed to the untrusted union, violating REQ-1/REQ-3. Only REQ-2 symlink carve-out may throw. Fix (consensus-reached with codex reviewer, gpt-5.5 high, surface:7): agents/lib/bg-trust.ts (readProjectTrustStore catch block, ~L175-178): - KEEP: ENOENT maps to {ok:false, reason:"forged"} (State E, key absent) - NEW: re-throw errors whose .message starts with "refusing symlinked " (the REQ-2 symlink security boundary, thrown by assertNoSymlinkLocal at L77) - NEW: all other non-ENOENT failures (malformed key content from parseTrustMac, EACCES/EPERM, anything else) map to {ok:false, reason:"forged"} (fail closed) - No custom error class (keeps pure-extraction slice API surface minimal) agents/test/test-bg-trust.mjs (+1 test, suite now 13): - testReadTrustStore_rejectsMalformedKey: writes "zzzz" to .pi/trust/.trust.mac (non-hex, triggers parseTrustMac throw at L139), asserts readProjectTrustStore returns {ok:false, reason:"forged"} via explicit try/catch that fails-on-throw (assert.fail if it throws). NO BREAK=1 shim per Q3 consensus — the try/catch directly covers the regression without test-only production code paths. Verified (run by orchestrator): - bash agents/test/run-bg-trust-tests.sh -> 13/13 passing - BREAK=1 node agents/test/test-bg-trust.mjs -> exit 1 (foreign-root discriminating test still works; new test has no BREAK shim, unaffected) - grep INV-2 (resolveTrustedHome|readOrCreateSessionMacKey|getBgStateDir| getBgSessionMacPath) agents/lib/bg-trust.ts -> empty - grep REQ-11 ^import -> only node:* + ./bg-state.ts - grep "refusing symlinked" -> exactly 2 hits (L77 throw + L177 carve-out) - git status -> only the 2 expected files modified Scope: catch-block discrimination + one test ONLY. No refactor, no API change. No production caller (still pure extraction until P5F-3). Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Charlton D. Ho <dev@znp.pw> Co-authored-by: Claude <noreply@anthropic.com>
lantiscooperdev
pushed a commit
that referenced
this pull request
Jul 5, 2026
…, 08a9be6) (#146) P5F-1 disk-backed per-project trust reader merged via PR #145 (squash `08a9be6`). Updates the two workplan pointer files to the new canonical episode head `20260705-130253-post-merge-sync-p5f-1-trust-reader-compl-a5b3` (supersedes the P5F-plan-accepted head `20260704-141949-...a0b1`). Changes: - WORKPLAN.md — current-state block: P5F-1 COMPLETE (#145, 08a9be6); P5F-2 writer+resolver step-tables (author + plan-review) is next. Chain head, revises, tags, summary all updated to the new episode. Follow-up bullet flipped from "P5F-1 (NEXT)" to "P5F-1 COMPLETE" + new "P5F-2 (NEXT)". - agents/P3_IMPLEMENTATION_SLICES.md — P5F section: P5F-1 marked SHIPPED (with the R1→R2 consensus fix trail + 13/13 tests + INV/REQ evidence), P5F-2 marked NEXT (step-tables deferred by design — now to be authored + plan-reviewed since the reader held up under the 13 P5F-1 tests), P5F-3 deferral note dropped from post-P5F-1-review to post-P5F-2-review. Also documents the closure of stale PR #140 (workplan-sync after P5b-2) which was CONFLICTING/DIRTY and fully superseded by #138/#142/#143/#144. The episodic memory episode is canonical; these markdown files are convenience references synced per project-runbook:workplan-update.md. Co-authored-by: Charlton D. Ho <dev@znp.pw> Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
Planning-only document (no implementation) for the disk-backed per-project trust reader.
Unblocks two deferred roadmap items that share one blocker:
selectBgTerminalBackend()preference probe picks non-deterministically when multiple backends installed)resolveTrustedHome())Design composes with the existing P4R MAC lifecycle (
bg-state.tsprimitives reused — no second HMAC scheme) and the P5-NL-bgprojectTrustedmanifest snapshot rather than introducing a parallel signing scheme.3 slices: P5F-1 (pure reader extraction, zero callers), P5F-2 (writer + resolver), P5F-3 (read-side wiring). 11 REQs, 16 unit tests + 2 static + 1 UNGUARDED-IN-CI smoke.
Status: planning only, pending plan review + adversarial review. Codex plan review dispatched via cmux.