docs: align trust map with the records era - #93
Conversation
LIMITS and THREAT-MODEL still described the deleted module/manifest world and claimed no stored content-hashes; correct those claims in place now that pharn.records.json ships. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe change updates trust-model documentation for records-era hashing, offline removal, and update behavior. It also adds planning, review, verification, regression, shipping, and execution-state records for the increment. ChangesTrust map records-era increment
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.dev/features/trust-map-records-era/GRILL.md:
- Around line 20-27: Update the evidence fields at the referenced FINDING
entries in GRILL.md to distinguish quoted claims from validated facts: label
install-payload, unrecorded-file, and manifest-pruning statements as rejected
premises when sourced from PLAN.md, and include the corresponding live evidence
that disproves or qualifies them. Ensure each evidence field aligns with its
finding’s problem and Interrogation rather than presenting rejected claims as
supporting evidence.
In @.dev/features/trust-map-records-era/PLAN.md:
- Around line 56-62: Update the fenced code blocks in PLAN.md beginning at the
referenced sections to include appropriate language identifiers, resolving
MD040. In the regular expression near the block around Lines 242–256, replace
the intentional trailing literal space with `[ ]` to resolve MD038 while
preserving the validation command’s behavior.
- Around line 202-205: Update the “no § header changed” validation row to match
the resolved requirement in the later plan section: allow the LIMITS.md §1d
heading change while validating that only the “1d.” identifier remains
unchanged, reusing the existing check from the relevant Lines 253-256 section.
In @.dev/features/trust-map-records-era/REGRESSION.md:
- Around line 10-17: Update the fenced code block in REGRESSION.md containing
the path list to specify the text language identifier, resolving markdownlint
MD040 without changing the listed paths.
In @.dev/features/trust-map-records-era/REVIEW.md:
- Around line 96-105: Correct the cross-document consistency section around “New
§4c vs docs/reference/pharn-records.md” so it does not claim unrestricted
agreement: clarify that the documents agree on skip/restore behavior, while the
status-label wording is corroborated separately and uses only “unverifiable,”
not “unrecorded.”
In `@LIMITS.md`:
- Around line 72-77: Qualify the LIMITS.md statement about remove pruning
pharn.records.json by stating that pruning occurs only when the records store is
present, readable, and correctly stamped; otherwise it remains unchanged.
Reference the documented fail-closed rule in docs/reference/pharn-records.md
without altering the surrounding reconstruction claims.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 91856c82-9aca-4ec7-8253-a95f9c54521f
📒 Files selected for processing (15)
.dev/features/trust-map-records-era/GRILL.md.dev/features/trust-map-records-era/PLAN.md.dev/features/trust-map-records-era/REGRESSION.md.dev/features/trust-map-records-era/REVIEW.md.dev/features/trust-map-records-era/SHIP.md.dev/features/trust-map-records-era/VERIFY.md.dev/features/trust-map-records-era/regression-report.json.dev/features/trust-map-records-era/verify-report.json.pharn/pharn-dev-regress/base-results.json.pharn/pharn-dev-regress/head-results.json.pharn/writes-scope.jsonCHANGELOG.mdLIMITS.mdTHREAT-MODEL.mddocs/reference/pharn-records.md
| ```yaml | ||
| - type: FINDING | ||
| rule_id: "P2" | ||
| severity: blocking | ||
| file: ".dev/features/trust-map-records-era/PLAN.md:171" | ||
| problem: "The Trust audit asserts the two edited files are install payload whose bytes ship to users; live code shows the installed copies are read from the fetched pharn-oss clone, so editing pharn-cli's own copies changes zero user-facing bytes and the stated justification for the CHANGELOG line is false." | ||
| evidence: "both target files are in `protect-trusted-paths.cjs` `DEFAULT_PROTECTED` (`:58`) and are **install payload** (`constants.ts:32-33`) — copied verbatim into user projects in the **flat** layout ... Editing them changes bytes that ship to users, which is precisely why the `CHANGELOG.md` line is in scope (P4/P7)." | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Disambiguate the rejected evidence.
At Lines 26, 60, and 93, the evidence fields repeat claims that the related problem and Interrogation sections reject. These claims concern install payloads, unrecorded files, and manifest-based pruning. If these strings quote PLAN.md, label them as rejected premises and add the live evidence. Otherwise, the GRILL record presents false evidence as support.
Also applies to: 54-60, 87-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/trust-map-records-era/GRILL.md around lines 20 - 27, Update
the evidence fields at the referenced FINDING entries in GRILL.md to distinguish
quoted claims from validated facts: label install-payload, unrecorded-file, and
manifest-pruning statements as rejected premises when sourced from PLAN.md, and
include the corresponding live evidence that disproves or qualifies them. Ensure
each evidence field aligns with its finding’s problem and Interrogation rather
than presenting rejected claims as supporting evidence.
| ``` | ||
| current branch : feat/remove-prunes-records @ 4b8a0be | ||
| main : 21db522 | ||
| git branch --contains 4b8a0be --all | ||
| → feat/remove-prunes-records | ||
| → remotes/origin/feat/remove-prunes-records # NOT main | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Resolve the reported markdownlint errors.
The fenced blocks starting at Lines 56, 105, and 245 have no language identifier, which triggers MD040. Add a language identifier to each block. MD038 also reports Lines 242 and 250. Encode the intentional trailing space in the Line 250 regular expression as [ ]; do not remove it because that changes the validation command.
Also applies to: 105-111, 242-256
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/trust-map-records-era/PLAN.md around lines 56 - 62, Update the
fenced code blocks in PLAN.md beginning at the referenced sections to include
appropriate language identifiers, resolving MD040. In the regular expression
near the block around Lines 242–256, replace the intentional trailing literal
space with `[ ]` to resolve MD038 while preserving the validation command’s
behavior.
Source: Linters/SAST tools
| | "no § header changed" | **floor: grep** — `git diff main -- LIMITS.md THREAT-MODEL.md \| grep -E '^[-+]#{2,3} ' \| wc -l` → 0 (**conditional on Q1 = A**) | | ||
| | "no test churn" | **floor: `npm run check`** exit 0 with zero test files modified | | ||
| | "markdown well-formed" | **floor: `npm run lint:md`** exit 0 | | ||
| | "only whitelisted files written" | **floor: hook** — `set-writes-scope.cjs` + `enforce-writes-scope.cjs` + `protect-trusted-paths.cjs` | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Update the superseded section-header validation rule.
Line 202 still claims that no section header changes and expects zero heading changes. Lines 239-256 resolve the opposite: LIMITS.md §1d changes its header, while only the 1d. identifier must remain unchanged. The final LIMITS.md confirms that heading change. A reviewer following this row can reject the intended diff or rely on stale evidence.
Replace this row with the identifier-only check from Lines 253-256, or mark the current row as historical.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/trust-map-records-era/PLAN.md around lines 202 - 205, Update
the “no § header changed” validation row to match the resolved requirement in
the later plan section: allow the LIMITS.md §1d heading change while validating
that only the “1d.” identifier remains unchanged, reusing the existing check
from the relevant Lines 253-256 section.
| ``` | ||
| CHANGELOG.md | ||
| LIMITS.md | ||
| THREAT-MODEL.md | ||
| docs/reference/pharn-records.md | ||
| .dev/features/trust-map-records-era/{PLAN,GRILL,SHIP}.md | ||
| .dev/features/trust-map-records-era/regression-report.json | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the fenced block.
markdownlint reports MD040 at Line 10. Use text for this path list instead of an untyped fence.
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/trust-map-records-era/REGRESSION.md around lines 10 - 17,
Update the fenced code block in REGRESSION.md containing the path list to
specify the text language identifier, resolving markdownlint MD040 without
changing the listed paths.
Source: Linters/SAST tools
| ### Cross-document consistency (requested check) | ||
|
|
||
| - **New §1d vs `THREAT-MODEL.md §2 #6` — agree.** §2 #6 already read *"`status`/`update`/`diff` resolve | ||
| against `@main` HEAD"*, correctly omitting `remove`. The corrected §1d now says the same thing. The | ||
| pre-existing contradiction between the two documents is **resolved**, and resolved toward the line that | ||
| was already true. | ||
| - **New §4c vs `docs/reference/pharn-records.md` — agree.** `pharn-records.md:12-14` states the identical | ||
| split (*"skips every **present** file that differs (`unverifiable`) but still **restores** missing | ||
| ones; byte-identical files are no-ops"*). Note it uses **only** `unverifiable` for this condition — | ||
| independently corroborating the first finding above. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Correct the cross-document agreement statement.
Lines 98-101 say the documents agree. Lines 102-105 say pharn-records.md uses only unverifiable, which supports the preceding finding that unrecorded is not valid for the stated condition. Change “agree” or explain that the agreement covers only skip/restore behavior, not the status labels.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/trust-map-records-era/REVIEW.md around lines 96 - 105, Correct
the cross-document consistency section around “New §4c vs
docs/reference/pharn-records.md” so it does not claim unrestricted agreement:
clarify that the documents agree on skip/restore behavior, while the
status-label wording is corroborated separately and uses only “unverifiable,”
not “unrecorded.”
| - **Struck claim:** "`remove`/`update` always know exactly what the pinned version installed." | ||
| - **True statement:** they reconstruct from the **current** upstream; a path renamed since the pin is | ||
| **reported as an orphan**, not guessed at. | ||
| - **Backstop:** a missing `from` is reported, never silently deleted or created (P5/P6). | ||
| - **True statement:** `update`/`status` reconstruct from the **current** upstream, so a capability | ||
| renamed since the pin is **reported**, never guessed; `remove` reconstructs from the **recorded | ||
| config**, deleting the directory the recorded `layout` addresses for that capability | ||
| (`src/commands/remove.ts:72`) and pruning that capability's entries from `pharn.records.json` | ||
| (`:117`). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Qualify the record-pruning claim.
The text says remove prunes the capability entries after resolving from configuration. docs/reference/pharn-records.md Lines 88-90 says that an absent, unreadable, or differently stamped store remains unchanged. Add that condition here or link to the fail-closed rule. Otherwise LIMITS.md overstates what remove guarantees.
Proposed clarification
- (`src/commands/remove.ts:72`) and pruning that capability's entries from `pharn.records.json`
- (`:117`).
+ (`src/commands/remove.ts:72`) and, when the existing store is readable and stamped for the
+ current config, pruning that capability's entries from `pharn.records.json` (`:117`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Struck claim:** "`remove`/`update` always know exactly what the pinned version installed." | |
| - **True statement:** they reconstruct from the **current** upstream; a path renamed since the pin is | |
| **reported as an orphan**, not guessed at. | |
| - **Backstop:** a missing `from` is reported, never silently deleted or created (P5/P6). | |
| - **True statement:** `update`/`status` reconstruct from the **current** upstream, so a capability | |
| renamed since the pin is **reported**, never guessed; `remove` reconstructs from the **recorded | |
| config**, deleting the directory the recorded `layout` addresses for that capability | |
| (`src/commands/remove.ts:72`) and pruning that capability's entries from `pharn.records.json` | |
| (`:117`). | |
| - **Struck claim:** "`remove`/`update` always know exactly what the pinned version installed." | |
| - **True statement:** `update`/`status` reconstruct from the **current** upstream, so a capability | |
| renamed since the pin is **reported**, never guessed; `remove` reconstructs from the **recorded | |
| config**, deleting the directory the recorded `layout` addresses for that capability | |
| (`src/commands/remove.ts:72`) and, when the existing store is readable and stamped for the | |
| current config, pruning that capability's entries from `pharn.records.json` (`:117`). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@LIMITS.md` around lines 72 - 77, Qualify the LIMITS.md statement about remove
pruning pharn.records.json by stating that pruning occurs only when the records
store is present, readable, and correctly stamped; otherwise it remains
unchanged. Reference the documented fail-closed rule in
docs/reference/pharn-records.md without altering the surrounding reconstruction
claims.
1 participant
Summary
LIMITS.mdandTHREAT-MODEL.mdclaims that still described the deleted module/manifest subsystem and a world with no stored file hashes.pharn.records.jsonhashes are dest-side drift baselines (not upstream provenance), thatremoveis offline from config, and that an absent/skewed store makesupdateskip present files while still restoring missing ones.docs/reference/pharn-records.mdand note the correction inCHANGELOG.md.Test plan
LIMITS.md§1b / §1d andTHREAT-MODEL.md§4c — claims match currentupdate/remove/status+ records behaviorpharn-records.mdlinks resolveMade with Cursor
Summary by CodeRabbit
Documentation
Verification