fix(core): the --ledger restore hint is a command that runs - #232
fix(core): the --ledger restore hint is a command that runs#232Max17190 wants to merge 1 commit into
Conversation
docs/extending.md documents rollback as "an ordinary cp from the objects directory", and --ledger's footer printed `restore with cp <objects>/<sha> <path>`. But the history lines abbreviate each sha to 12 chars for reading, while an object's filename is the full 64, so copying the value the history showed failed. Worse, an approval's bound code (a hook's script) was counted as "+N bound file" but its object hash and target path were never printed, so recovering a deleted approved hook, which needs both the manifest and its script, was impossible from what --ledger showed. Replace the placeholder template with a runnable, shell-quoted `cp` line per intact object, each naming the full-sha object path and the project path it belongs at: change records name their own file, and an approval names its manifest plus, for a hook, the bound code paths in `code` paired with the hashes in `also`. Reuses doctor::shell_quote (now pub) so both sides are POSIX quoted. Verified end to end: deleting an approved hook's script and running the printed command restores it byte-identically. Test plan: cargo test -p open-max-tui (approvals_only_ledger_reports_no_missing_ objects extended: asserts a full-sha, shell-quoted cp and no <sha> placeholder); cargo test --workspace; cargo clippy --workspace at zero warnings.
| restore.push((sha.clone(), r.path.clone())); | ||
| } | ||
| } | ||
| for (sha, path) in r.also.iter().zip(r.code.iter()) { |
There was a problem hiding this comment.
External-tool restore paths are dropped
Approved external tools store a project-local bound script's object hash in also, but do not populate the hook-specific code path list. Pairing these collections with zip therefore produces no restore target for the intact script object. openmax --ledger prints a command for the manifest but cannot tell an operator how to restore the approved script that the manifest depends on. Preserve or derive the corresponding external-tool script path when assembling restore targets instead of relying on code.
Artifacts
Focused external-tool ledger restore fixture source
- The executable fixture creates and approves an external tool bound to a project-local script, verifies its stored object, and asserts the ledger restore command, showing the exact exercised path.
Approval record and intact bound-script object
- The approval command created an external-tool record with the script hash in `also`, no `code` field, and an intact object at that script hash, establishing the pre-ledger state.
Ledger output missing bound-script restore command
- The executed `openmax --ledger` output prints the manifest restore command but not the project-local script command, and the fixture assertion fails, confirming the omission.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/tui/src/main.rs
Line: 546
Comment:
**External-tool restore paths are dropped**
Approved external tools store a project-local bound script's object hash in `also`, but do not populate the hook-specific `code` path list. Pairing these collections with `zip` therefore produces no restore target for the intact script object. `openmax --ledger` prints a command for the manifest but cannot tell an operator how to restore the approved script that the manifest depends on. Preserve or derive the corresponding external-tool script path when assembling restore targets instead of relying on `code`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Why
docs/extending.mddocuments rollback as "an ordinary cp from the objects directory", and--ledger's footer printedrestore with cp <objects>/<sha> <path>. But the history lines abbreviate each sha to 12 chars for reading, while an object's filename is the full 64, so copying the value the history showed failed with "No such file or directory". Worse, an approval's bound code (a hook's script) was counted as "+N bound file" but its object hash and target path were never printed, so recovering a deleted approved hook, which needs both the manifest and its script, was impossible from what--ledgershowed.Summary
cpline per intact object, each naming the full-sha object path and the project path it belongs at.codepaired with the hashes inalso(the second file the deleted-hook recovery needs).doctor::shell_quote(nowpub) so both sides are POSIX quoted; unique objects only.Test Plan
cargo test -p open-max-tui:approvals_only_ledger_reports_no_missing_objectsextended to assert a full-sha, shell-quotedcpand no<sha>placeholder.cargo test --workspace,cargo clippy --workspace --all-targetsat zero warnings.cpverbatim restores the script byte-identically.Greptile Summary
This change improves
openmax --ledgerby printing runnable, shell-quoted restore commands for intact stored objects. A reproduced recovery gap remains for approved external tools: when a tool is bound to a project-local script, the ledger output restores the manifest but omits the script command, so the recovered tool cannot be made runnable from the displayed instructions alone.Confidence Score: 4/5
Do not merge until ledger recovery output includes bound project-local scripts for approved external tools.
A focused end-to-end fixture created and approved an external tool, verified its persisted object, and directly observed that the ledger output omitted its required script restore command.
Files Needing Attention: crates/tui/src/main.rs needs to retain or derive restore paths for external-tool script hashes instead of pairing all additional hashes with the hook-only code path list.
What T-Rex did
Comments Outside Diff (1)
General comment
openmax --ledgeremits nocp <object> <script-path>restore command.alsofor all additional approved hashes, whilecodeis populated only for hook shapes.main.rspairs these collections withr.also.iter().zip(r.code.iter()), so an external-tool record withalso = [script_sha]andcode = []produces no script restore target.alsowith hook-onlycodeusingzip.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(core): the --ledger restore hint is ..." | Re-trigger Greptile