Skip to content

fix(core): the --ledger restore hint is a command that runs - #232

Open
Max17190 wants to merge 1 commit into
mainfrom
ledger-restore-hint-is-runnable
Open

fix(core): the --ledger restore hint is a command that runs#232
Max17190 wants to merge 1 commit into
mainfrom
ledger-restore-hint-is-runnable

Conversation

@Max17190

@Max17190 Max17190 commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Why

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 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 --ledger showed.

Summary

  • 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; an approval names its manifest plus, for a hook, the bound code paths in code paired with the hashes in also (the second file the deleted-hook recovery needs).
  • Reuse doctor::shell_quote (now pub) so both sides are POSIX quoted; unique objects only.
  • The store-gone and damaged-object warnings are unchanged.

Test Plan

  • cargo test -p open-max-tui: approvals_only_ledger_reports_no_missing_objects extended to assert a full-sha, shell-quoted cp and no <sha> placeholder.
  • cargo test --workspace, cargo clippy --workspace --all-targets at zero warnings.
  • Manual end to end: approving a hook, deleting its script, and running the printed cp verbatim restores the script byte-identically.

Greptile Summary

This change improves openmax --ledger by 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.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex generated a proof for a posted P1 finding.
  • T-Rex generated a second finding-comment-proof for a P1 finding.
  • T-Rex documented the contract-validation work, noting that the approval record stores the script SHA in also but has no code path array, and that the ledger output prints only the manifest restore command.
  • T-Rex organized the proof artifacts by mapping the four artifact entries to the general-contract-validation-proof to support review.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 External-tool bound script restore command is omitted from --ledger

    • Bug
      • An approved external tool with a project-local script stores the script hash and intact object, but openmax --ledger emits no cp <object> <script-path> restore command.
    • Cause
      • Approval records use also for all additional approved hashes, while code is populated only for hook shapes. main.rs pairs these collections with r.also.iter().zip(r.code.iter()), so an external-tool record with also = [script_sha] and code = [] produces no script restore target.
    • Fix
      • Record corresponding paths for approved external-tool project-local code, or derive their paths from the approved manifest when rendering restore targets; do not pair also with hook-only code using zip.

    T-Rex Ran code and verified through T-Rex

Prompt To Fix All With AI
### Issue 1
crates/tui/src/main.rs:546
**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.

Reviews (1): Last reviewed commit: "fix(core): the --ledger restore hint is ..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

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.
Comment thread crates/tui/src/main.rs
restore.push((sha.clone(), r.path.clone()));
}
}
for (sha, path) in r.also.iter().zip(r.code.iter()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

View artifacts

T-Rex 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant