You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
openRepoTools --install retires a word by REMOVING the copy it once placed
(retire_commands, landed in #45), and the only evidence it had that a file at ~/.local/bin/<name> is one it wrote was a CONTENT SUBSTRING: the sentence Installed on PATH by + this installer's own name, which its own commands
carry in their header. Copilot's round-3 review of #45 (openRepoTools:263)
named that — "a user-owned restart script copied from an old installation or
containing the same banner will satisfy grep and be deleted on the next
install" — and it was DECLINED for restart, because no installer that ever
placed restart wrote a receipt and the marker is therefore the only evidence
that exists for the copies on people's PATHs today.
But every retirement after this one can have better, which is #57. So --install now writes down what it placed, and a later retirement reads that
first.
What changes
The receipt.${OPENREPOTOOLS_DATA_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/openRepoTools}/installed.tsv,
mode 0600, one row per REGULAR FILE this run placed: name<TAB>destination<TAB>sha256<TAB>UTC. Written whole through mktemp in
the same directory + mv -f, which is the shape place_skill_and_hook
already writes settings.json with, minus its chmod: the 0600 is the one mktemp gives the temporary, and there is deliberately no mode stamp. --install's mode-stamp targets prove -w, not that a chmod will succeed (ownership) #48
rules that every chmod this command performs fails through die, and --install writes a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57
rules that a receipt it cannot write is a note and never a refused install;
a stamp that may neither die nor fall silent is one that must not exist, and
0600 wants none (one account's evidence, read only by that account's own
retirements). The code, test_install_writes_a_receipt_of_every_file_it_placed, --help and the README all say 0600.
It covers every placed regular file, not only the thirteen in the bin
directory: the SKILL.mds and the command files are regular files this run
placed too. The two hook entries get no row — an entry inside
somebody else's JSON file is not a file this command placed — and the receipt
carries no row for itself, so it is not an extra artifact: today that is
27 artifacts, 25 of them files, 25 rows (13 + 6 + 6 since Amendment 16 added lane-rename), and the tests derive the count rather than pin it.
A row's subject is its DESTINATION, not its name. The lane's decision was
"rows for names placed this run replace that name's earlier row"; the name is
not unique once the skills and the command files are covered (handoff is a
skill at two paths AND a command file at two more), and retire_commands
looks a path up rather than a name. So the key is the path, the name column
is the word --install PRINTS beside that placement (park, handoff, /ctx), and a run replaces the row of every destination it placed and keeps
every other row — which also means moving $OPENREPOTOOLS_BIN_DIR does not
throw away the evidence for the copies still in the old directory.
retire_commands consults it FIRST. A row whose sha256 still matches the
file on disk is this installer's copy beyond argument → removed, and its row
dropped. A row whose sha256 has MOVED is a person's edit of an installed
command → named, left, the rm printed for them, row kept — and that is true even where the header marker is still in the file, because the receipt is
the stronger evidence and that is the whole point of --install writes a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57. No row at all is
today's header-marker fallback, byte for byte, so nothing that works now
stops working. A symlink is still refused outright, before the receipt is
asked at all.
A receipt that cannot be written is a NOTE, never a refused install. One
line on stdout: what it could not write, where, why, that the install itself
is complete, and that a retirement falls back to the header. The exit belongs
to the reason rather than to the line — a path it may not write names $OPENREPOTOOLS_DATA_DIR, a machine with neither digest tool is told that
instead. Fail closed, out loud, carry on.
sha256sum, else shasum -a 256 — one helper, bash-3.2 clean, because a
stock macOS ships no sha256sum. The file is fed on stdin rather than
named as an operand: GNU sha256sum escapes a filename containing a
backslash and prefixes the line with \, which would put a \ on the front
of the digest for a bin directory somebody spelled with one.
A destination carrying a TAB or a NEWLINE writes NO receipt at all and
says so, rather than a row that is silently two columns or two rows. A TSV
has no escape, $OPENREPOTOOLS_BIN_DIR is a path a person chooses, and a
receipt missing exactly the file a retirement will ask about is worse than
none.
The receipt path is never written through a symlink.mv -f replaces the
link itself, so an install over one would swap a person's link for a regular
file; path_kind — the same helper both planners use — is asked first, and
anything that is not a regular file there is named and left.
The receipt is only ever asked about a path this command computed. It is
never read as a list of paths to remove, so a receipt somebody else authored
can change one answer and can never name a file for removal.
usage() gains the paragraph (where it lives, what it is for) and both $OPENREPOTOOLS_DATA_DIR and $XDG_DATA_HOME in the variable block, which test_help_names_every_variable_it_reads now pins; the README gains the same
in its install section and a row in its variable table (the README is 484 lines and the cap in tests/test_repo_hygiene.py moved 472 -> 484 on 2026-10-02 with a dated entry
for --install writes a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57, after main itself had reached 472).
The decision on test_installing_twice_changes_nothing
"Changes nothing" is about the bin directory and the artifacts — the lines
each placement prints — and that test asserts exactly those, so it is untouched
and still honest. The receipt's UTC is the stamp of the run that RECORDED the
row and does change on a re-install; that is the honest reading of a column
written by a run that verified a file it did not rewrite, and it is held by test_installing_twice_leaves_no_duplicate_rows, which pins that the rows
themselves (name, destination, digest) do not move and that no destination
gains a second row.
How it was tested
tests/run.sh tests/test_openrepotools_command.py tests/test_install_skill_and_hook.py tests/test_repo_hygiene.py
on Linux, and eight new cases in tests/test_openrepotools_command.py (seven
more from the 2026-10-02 fix round are listed at the end):
test_install_writes_a_receipt_of_every_file_it_placed — one row per placed
file (ARTIFACTS - HOOK_ENTRIES, derived from the three lists), each digest
recomputed with hashlib against the bytes at that destination, each stamp
parsed, mode 0600, and the line that says so.
test_a_retirement_reads_the_receipt_before_the_header — a file with no
marker at all is removed on the strength of a matching row, and its row is
dropped.
test_a_file_the_receipt_no_longer_recognises_is_named_and_left — a file with the marker whose row's digest has moved is named, left byte for
byte, printed with its rm, and its row is kept.
test_a_receipt_it_cannot_write_is_a_note_and_never_a_refusal — a regular
file where the data directory goes (a mkdir -p no user can win, root
included): exit 0, thirteen of thirteen placed, and the note naming the path,
the reason and the fallback.
test_a_receipt_that_is_a_symlink_is_not_written_through — the link and
what it points at are both untouched.
test_a_digest_it_cannot_take_is_a_note_too — sha256sum and shasum
shims that exit 1, first on $PATH: the install still succeeds and no
receipt is written.
The fourth arm's sibling — a machine with NEITHER sha256sum NOR shasum on
its PATH at all — is a one-line guard checked by hand against a $PATH built
without them (it names the missing tools rather than blaming the first file it
could not digest); building that $PATH inside a test would not survive the
macOS job, so the shim test above covers the same receipt_note path.
command_env now clears $OPENREPOTOOLS_DATA_DIRand $XDG_DATA_HOME, so
a developer who exports the latter — normal on a Linux desktop — cannot have
this suite writing a receipt into their real data directory however carefully $HOME is redirected.
Merge note
The receipt is its own section of helper functions with ONE call line in install_commands (between place_skill_and_hook and retire_commands) and a
four-arm if in retire_commands, so the hunks inside those two functions are
as small as they can be. Measured against the other branches touching this
file: #81 changes the INSTALLABLES= line and the counts around it — this
section is inserted after the blank line below that assignment, so the two
changed ranges are separated and do not overlap — and the sibling branch's chmod lines in the install loop are untouched here.
Copilot's six threads of 2026-10-02 were all taken: a row that cannot be verified (no digest tool) is named and left, never removed through the header fallback; destinations are recorded and queried canonical and absolute (cd && pwd -P), so a relative OPENREPOTOOLS_BIN_DIR cannot point a later run at another directory's file; the tab/newline TSV guard has its regression tests; --help, the README and this body say 0600; the artifact counts are derived and current.
Seven further cases: test_a_row_it_cannot_verify_is_named_and_left_never_removed, the two relative-destination cases, the two tab/newline guard cases, test_the_help_and_the_readme_say_the_mode_the_receipt_is_born_at, and the old-shape-row case.
Add receipt-based ownership tracking to --install so retirements can safely distinguish unchanged installed files from user-owned edits.
New Features:
Record every regular file placed by --install in a secure, atomically written receipt containing its destination, digest, and timestamp.
Use receipt evidence during retirement to remove only unchanged installed files while preserving modified or unverifiable files and retaining the existing header fallback when no receipt row exists.
Bug Fixes:
Prevent receipt data loss and unsafe removals when receipt files, paths, digests, or destination types cannot be read, verified, or represented safely.
Preserve receipt entries for files in directories no longer targeted and avoid duplicate or relative-path entries across repeated or relocated installations.
Enhancements:
Add clear install and retirement notes for receipt failures, missing digest tools, unreadable records, and unsupported paths.
Canonicalize receipt and destination paths and protect receipt symlinks from being overwritten.
Documentation:
Document the receipt location, format, permissions, and retirement behavior in the README and command help.
Tests:
Add comprehensive coverage for receipt creation, retirement precedence, digest failures, unreadable and unwritable receipts, symlink and TSV safeguards, repeated installs, relocated directories, and relative paths.
… reads that before it reads a header substring
#57. `retire_commands` retires a word by REMOVING the copy
this installer once placed, and the only evidence it had that a file at
`~/.local/bin/<name>` is one it wrote was a CONTENT SUBSTRING — the
`Installed on PATH by ` header its own commands carry. Copilot's round-3 review
of #45 (`openRepoTools:263`) named that: a person's own script copied from an
old installation carries the same banner and would be deleted. It was DECLINED
for `restart`, and the reason does not generalise — no installer that ever
placed `restart` wrote a receipt, so for the copies on people's PATHs today the
marker is the only evidence there is — but every retirement AFTER this one can
have better, which costs nothing except that a run write down what it did.
So `--install` writes one row per regular file it placed —
`name<TAB>destination<TAB>sha256<TAB>UTC` — to
`${OPENREPOTOOLS_DATA_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/openRepoTools}/installed.tsv`
at 0644, through `mktemp` in the same directory + `chmod` + `mv -f`, which is
the shape `place_skill_and_hook` already writes `settings.json` with. It covers
all twenty-four files rather than only the twelve in the bin directory: the six
`SKILL.md`s and the six command files are regular files this run placed too.
The two hook entries get no row, because an entry inside somebody else's JSON
file is not a file this command placed, and the receipt carries no row for
itself — it is not a twenty-seventh artifact.
A ROW'S SUBJECT IS ITS DESTINATION AND NOT ITS NAME. The name is not unique
once the skills and the command files are covered (`handoff` is a skill at two
paths and a command file at two more) and `retire_commands` looks a PATH up, so
the key is the path and the `name` column is the word `--install` prints beside
that placement. A run replaces the row of every destination it placed and keeps
every other row, which is also why moving `$OPENREPOTOOLS_BIN_DIR` does not
throw away the evidence for the copies still in the old one.
`retire_commands` asks the receipt first. A row whose digest still matches is
this installer's copy beyond argument and is removed, and its row goes with it;
a row whose digest has MOVED is a person's edit of an installed command, named
and left with the `rm` printed for them — and that is true even where the
header marker is still in the file, because the receipt is the stronger
evidence and that is the whole of what #57 asks for. No row at all is today's
header-marker fallback byte for byte, so nothing that works now stops working,
and a symlink is still refused outright before the receipt is asked at all.
A RECEIPT THAT CANNOT BE WRITTEN IS A NOTE AND NEVER A REFUSED INSTALL: one
line saying what it could not write, where, why, that the install itself is
complete, and that a retirement falls back to the header. The digest is
`sha256sum` else `shasum -a 256`, because a stock macOS ships no `sha256sum`,
and the file is fed on stdin rather than named as an operand because GNU
`sha256sum` escapes a filename carrying a backslash and puts a `\` on the front
of the line. A destination carrying a TAB or a NEWLINE writes no receipt at all
and says so, rather than a row that is silently two columns or two rows; and
the receipt path is never written through a symlink, because `mv -f` replaces
the link itself.
THE UTC IS THE STAMP OF THE RUN THAT RECORDED THE ROW, which does change on a
re-install. `test_installing_twice_changes_nothing` is about the bin directory
and the artifacts — the lines each placement prints — and is untouched and
still honest; `test_installing_twice_leaves_no_duplicate_rows` is what pins
that the rows themselves do not move and that no destination gains a second
row.
Seven cases in `tests/test_openrepotools_command.py` cover the receipt's shape
and digests, a retirement by receipt of a file carrying no marker, a file
carrying the marker whose digest has moved being named and left, an unwritable
receipt directory being a note beside a successful install, a symlinked receipt
being left alone, the absence of duplicate rows, and the rows of a bin
directory this run no longer writes being kept. `command_env` clears
`$XDG_DATA_HOME` as well as `$OPENREPOTOOLS_DATA_DIR`, so a developer who
exports the former cannot have the suite writing into their real data
directory. `usage()` and the README say where the receipt lives and what it is
for, and name the new variable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The installer now atomically records SHA-256 ownership receipts for all regular files it places, while retirement trusts matching receipt evidence before falling back to the legacy header marker; failures remain non-fatal, behavior is documented, and comprehensive tests cover precedence, portability, safety, and retention.
Sequence diagram for receipt-backed installation and retirement
sequenceDiagram
participant User
participant Installer as openRepoTools
participant Receipt as installed.tsv
participant Filesystem
User->>Installer: --install
Installer->>Filesystem: Place regular files
Installer->>Installer: sha256sum or shasum -a 256
Installer->>Receipt: Write complete receipt via temp file, chmod, mv -f
Installer-->>User: Installation complete
User->>Installer: --install word
Installer->>Filesystem: Check computed destination
Installer->>Receipt: Look up destination row first
alt Receipt digest matches
Installer->>Filesystem: Remove installed copy
Installer->>Receipt: Drop row
else Receipt digest differs
Installer-->>User: Name edited file and print rm
else No receipt row
Installer->>Filesystem: Check Installed on PATH by header
Installer-->>User: Apply legacy retirement behavior
end
Loading
Flow diagram for safe receipt writing
flowchart TD
A[Install places regular files] --> B{Receipt path is regular or absent?}
B -- No, symlink or other object --> C[Print non-fatal receipt note]
B -- Yes --> D{Destination paths contain TAB or NEWLINE?}
D -- Yes --> C
D -- No --> E{sha256sum or shasum available and succeeds?}
E -- No --> C
E -- Yes --> F[Write rows to temporary file]
F --> G[chmod 0644]
G --> H[mv -f into installed.tsv]
C --> I[Keep installation successful]
H --> I
Loading
File-Level Changes
Change
Details
Files
Add atomic, destination-keyed installation receipts covering every regular file placed by an install.
Resolve the receipt under the configurable data directory.
Record destination, printed name, SHA-256 digest, and UTC timestamp for each placed regular file.
Replace receipt contents atomically with mode 0644, preserving rows for destinations not placed by the current run.
Skip hook entries and the receipt itself.
openRepoTools
Use receipt ownership evidence before the existing header-marker fallback during retirement.
Remove a file only when its receipt digest still matches.
Leave edited files in place, retain their rows, and print a removal command.
Drop rows for successfully retired files and restrict receipt lookups to computed destinations.
Refuse to write through a symlink and retain the existing symlink protection.
openRepoTools
Make receipt failures non-blocking and portable across digest-tool environments.
Fall back from sha256sum to shasum -a 256 using stdin.
Emit an explanatory note and continue installation when receipt storage or digesting fails.
Decline to write receipts for destinations containing tabs or newlines.
openRepoTools
Document the receipt configuration and retirement behavior.
Describe receipt format, scope, atomic replacement, and fallback behavior in the install documentation.
Document OPENREPOTOOLS_DATA_DIR and expose both data-directory variables in help output.
README.md openRepoTools
Expand test coverage for receipt creation, precedence, failure modes, and row retention.
Validate row count, destinations, names, digests, timestamps, permissions, and output.
Cover matching and changed digests, unwritable or symlinked receipts, unavailable digest tools, duplicate prevention, and moved bin directories.
Clear receipt-related environment variables from test subprocesses and pin help-variable coverage.
Have --install write durable ownership receipts recording each regular file it places, including its destination, SHA-256 digest, and UTC timestamp, with a documented data-directory choice and safe atomic replacement.
Make retirement consult receipt evidence before the legacy header marker: remove files whose recorded digest still matches, leave and report edited files whose digest differs, and retain the header-marker fallback for files without receipt rows.
Ensure receipt failures do not refuse installation, avoid writing through symlinks or producing malformed records, and add coverage and documentation for the receipt behavior and failure cases.
Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment @sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment @sourcery-ai review to trigger a new review!
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Receipt error handling, concurrent updates, and deletion/update consistency have unresolved issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds installation receipts so retirements can distinguish unchanged installer files from edited or user-owned files.
Changes:
Records destination, digest, and timestamp for installed files.
Uses receipts during retirement with header fallback.
Adds tests and documents receipt storage and configuration.
File summaries
File
Summary
tests/test_openrepotools_command.py
Tests receipt creation, retention, retirement, and failure handling.
README.md
Documents receipt storage and related variables.
openRepoTools
Implements receipt creation, merging, verification, and retirement integration.
Review details
Suppressed comments (7)
README.md:289
The receipt is TSV, but this documentation shows spaces between fields. Destinations may contain spaces, so this can lead readers to produce or interpret invalid receipt rows; document the tab separators explicitly.
`<name> <destination> <sha256> <UTC>` per regular file, at
openRepoTools:574
When a receipt row exists but sha256_of cannot read the file or run the digest tool, this branch returns unknown, and retire_commands then falls through to the header-marker fallback. That can delete an edited retired script carrying the marker after the digest tools disappear, even though the receipt explicitly owns the path; the fallback should be limited to a missing receipt/row, while an existing but unverifiable row is left in place and reported as unverifiable.
If an existing receipt is a regular file but cannot be read, the input redirection on this while fails and the function still reaches its unconditional return 0. Because both callers invoke it in an || context, set -e will not stop that path, so receipt_record can overwrite the unreadable receipt with only this run's rows and silently discard the old-directory evidence it promises to keep. Propagate the loop/redirection failure so the old receipt is left untouched and the install emits the existing note.
done <"$1"
openRepoTools:535
The receipt merge is a read/modify/write with no lock: two concurrent installs (for example, targeting different OPENREPOTOOLS_BIN_DIR values) can both read the same old receipt, stage their own rows, and let the last mv overwrite the other's rows. The atomic rename prevents a torn file but not this lost update, so the promised evidence for one directory can disappear. Serialize the full receipt merge (and the retirement-side update) on a lock in the data directory.
# THIS RUN'S ROWS FIRST, THEN EVERY EARLIER ROW IT DID NOT REPLACE.
receipt_rows_except "$file" "$RECEIPT_DESTS" "$RECEIPT_ROWS" ||
{ receipt_note "$file" "the rows already there could not be read"; return 0; }
if receipt_replace "$RECEIPT_ROWS"; then
openRepoTools:140
This is a tab-separated file, but the help renders the fields as space-separated. Since destinations are allowed to contain spaces, this notation is ambiguous and does not describe a row that can actually be parsed; show the literal tab separators.
`<name> <destination> <sha256> <UTC>` each — at
openRepoTools:418
The new fail-closed branch for a destination containing a TAB or newline is not covered by the added tests. Since this is the safeguard that prevents malformed TSV rows, add a regression test using a bin/data path with each character and assert that installation succeeds without creating or replacing the receipt.
case "$dest" in
*"$RECEIPT_TAB"*|*"$RECEIPT_NEWLINE"*)
RECEIPT_WHY="$dest has a tab or a newline in it and this file is tab-separated"
return 1
;;
esac
digest="$(sha256_of "$dest")" || {
RECEIPT_WHY="$dest could not be digested"
return 1
openRepoTools:454
Use command grep for this lookup. The exit status controls whether an older receipt row is retained, and this repository documents that its test/runtime harness can provide a shell function named grep with nonstandard status; a bare call can therefore keep rows that should be replaced (or vice versa).
if grep -qFx -e "$dest" -- "$2"; then continue; fi
…etirement's row-drop is silent
Copilot round 1 on #103, `openRepoTools:660`. `receipt_forget` runs AFTER the
`rm` it accompanies and returned 0 on three write failures of its own — staging
the kept rows, staging the destination, and reading the rows already there — so
a full disk or a data directory that went away in that instant left the receipt
naming a path this installer's copy had just left, with nothing on screen to say
so. Every one of those four steps now reaches one note, and the note says what
the stale row is and what clears it.
AND THE WINDOW CLOSES BY ITSELF, which is the half that does not depend on a
write succeeding: the merge no longer keeps a row whose file is not at that path
any more, so the next `--install` drops a stale row whatever happened during the
retirement — a `receipt_forget` that could not write, a kill between the `rm` and
the rewrite, or a receipt somebody hand-edited. It also stops the file growing
for ever with the bin directories and profile roots people delete. `-L` beside
`-e`, for the reason `retire_commands` gives one function down: `-e` is false for
a dangling symlink, and a link is something at that path whatever it points at.
`test_a_row_whose_file_is_gone_is_not_kept` holds the new half, and the rows of
the files that ARE there are asserted untouched beside it.
WHAT IS NOT CHANGED, and the reason. The finding's stated consequence — that a
later install could "classify a newly created file at that path as `placed` and
delete it when its bytes happen to match" — is a file whose sha256 EQUALS the
digest of the bytes this installer placed, which makes it that file restored
from a backup or an older install rather than an unrelated file of somebody's.
Removing it is the retirement doing its work, and catching an unrelated file
would take a sha256 collision. The receipt is also never read as a list of paths
to remove: `retire_commands` walks `RETIRED` against its own `$dir` and asks the
receipt one question about a target it already has in hand.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved critical and moderate findings remain in receipt cleanup, diagnostics, and concurrent receipt updates.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (6)
openRepoTools:691
If receipt_verdict returns unknown because the digest tool or receipt is unavailable, this branch still removes a marker-bearing file via the legacy fallback but never calls receipt_forget. A receipt row for that path therefore survives after the file is gone and can be consulted for a later file at the same destination; drop the row after this removal as well.
elif [ ! -L "$target" ] && [ -f "$target" ] && grep -qF -e 'Installed on PATH by `openRepoTools --install`' -- "$target" 2>/dev/null; then
rm -f -- "$target"
say "$name: RETIRED, removed from $target (lane-collision-protocol Amendment 18 Addendum 2; \`lane <name>\` is that act now)"
openRepoTools:179
The help text gives the receipt default as $XDG_DATA_HOME/openRepoTools, but receipt_file() falls back to $HOME/.local/share when XDG_DATA_HOME is unset. This makes the documented default point at /openRepoTools or an unset variable instead of the actual per-user location; show the ${XDG_DATA_HOME:-$HOME/.local/share} fallback here.
$OPENREPOTOOLS_DATA_DIR where the receipt goes (default
$XDG_DATA_HOME/openRepoTools)
$XDG_DATA_HOME the data root it falls back to (default ~/.local/share)
openRepoTools:413
The TAB/newline guard is the only protection against producing an invalid TSV, but none of the added tests exercises this branch. Since the PR explicitly promises a successful install with no receipt for such destinations, add a case with a OPENREPOTOOLS_BIN_DIR containing a TAB or newline and assert the note, installed artifacts, and absent receipt.
*"$RECEIPT_TAB"*|*"$RECEIPT_NEWLINE"*)
RECEIPT_WHY="$dest has a tab or a newline in it and this file is tab-separated"
return 1
openRepoTools:412
dest is user-controlled and this branch explicitly handles destinations containing a newline, but it copies that value verbatim into RECEIPT_WHY. receipt_note is documented as a one-line diagnostic, so a bin/profile path containing LF produces a multi-line note and an ambiguous message. Avoid embedding the raw destination here (or escape control characters before reporting it).
RECEIPT_WHY="$dest has a tab or a newline in it and this file is tab-separated"
openRepoTools:470
If an existing receipt is a regular file but cannot be read, the input redirection on this while fails. Because this function is called inside ||/&& lists, set -e is suppressed and the unconditional return 0 below treats that failure as success; receipt_record then replaces the receipt and silently discards the older rows instead of emitting its read-failure note. Propagate the loop status to the caller.
done <"$1"
openRepoTools:549
The temp-file rename makes each replacement atomic, but the read/merge/write sequence is not serialized. Two concurrent installs (for example, one using each of two bin directories) can both read the same old receipt, stage their own rows, and let the last mv discard the other run's destinations, defeating the documented preservation of old-directory evidence. Serialize this merge (and the retirement rewrite) with a receipt lock.
# THIS RUN'S ROWS FIRST, THEN EVERY EARLIER ROW IT DID NOT REPLACE.
receipt_rows_except "$file" "$RECEIPT_DESTS" "$RECEIPT_ROWS" ||
{ receipt_note "$file" "the rows already there could not be read"; return 0; }
if receipt_replace "$RECEIPT_ROWS"; then
… is the same test the row was written under
Copilot round 2 on #103, `openRepoTools:468`. The rule round 1 added asked
`[ -e "$dest" ] || [ -L "$dest" ]` — "is there anything at all at that path" —
so a destination that had become a DIRECTORY, a socket, a FIFO or a SYMLINK kept
its row. A row is evidence about a regular file this installer placed, and where
one of those sits at that path the file the row is about is not there.
So the keep test is now `receipt_add`'s own write test, spelled the same way:
`[ -f "$dest" ] && [ ! -L "$dest" ]`. Two halves of one rule in one sentence
each rather than two different ones — which is the same argument
`unplaceable_kind` and `path_kind` are single implementations for, one function
group along.
`test_a_row_whose_path_is_no_longer_a_regular_file_is_not_kept` holds it for
both shapes a person actually makes, a directory and a symlink, and asserts the
twenty-four rows of the files that ARE there are untouched beside it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved critical receipt-update race and moderate correctness and safety issues remain.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
openRepoTools:393
receipt_note interpolates the receipt path and reason verbatim. If a user-selected OPENREPOTOOLS_BIN_DIR contains a newline, the dest included in RECEIPT_WHY makes this supposedly single-line failure note span multiple physical lines and become ambiguous; escape these values or omit the raw path before printing.
say "receipt: NOT written to $1 — $2. Everything above is installed; a later retirement falls back to the \`Installed on PATH by\` header, which is the evidence it has today."
openRepoTools:413
The TSV-safety branch is not exercised by the added tests: none runs --install with an OPENREPOTOOLS_BIN_DIR containing a tab or newline and asserts that no receipt is created. Add a parameterized case for both characters, since a regression here would produce rows that retirement cannot parse safely.
*"$RECEIPT_TAB"*|*"$RECEIPT_NEWLINE"*)
RECEIPT_WHY="$dest has a tab or a newline in it and this file is tab-separated"
return 1
openRepoTools:425
receipt_add unconditionally appends to RECEIPT_DESTS and the rows file. The two placement loops can produce the same destination when the configurable roots overlap (for example CLAUDE_PROFILES_HOME=$HOME and CLAUDE_USER_DIR=$HOME/shared), so one physical SKILL.md or command file is recorded twice. That breaks the destination-key invariant and the no-duplicate-row guarantee; skip destinations already staged before hashing/appending.
printf '%s\t%s\t%s\t%s\n' "$name" "$dest" "$digest" "$RECEIPT_STAMP" \
>>"$RECEIPT_ROWS" || {
RECEIPT_WHY="the rows could not be staged in $WORKDIR"
return 1
}
printf '%s\n' "$dest" >>"$RECEIPT_DESTS" || {
openRepoTools:339
OPENREPOTOOLS_DATA_DIR is used verbatim here, so a relative value makes the receipt location depend on the caller's current directory. An install from A with OPENREPOTOOLS_DATA_DIR=state writes A/state/installed.tsv, while a later install from B reads B/state/installed.tsv and silently loses the ownership records (and falls back to the header during retirement). Normalize this configured path to an absolute path, or reject relative values, before recording and reading the receipt.
The receipt records $OPENREPOTOOLS_BIN_DIR/$name without normalizing it. Relative bin directories are accepted by the existing planner, so the same bin/park row refers to different physical files after running --install from another working directory; the merge then replaces the old row and loses evidence for the first directory. Normalize the bin destination (and use the same canonical form in retirement), or reject relative bin-directory values.
local dir="${OPENREPOTOOLS_BIN_DIR:-$HOME/.local/bin}"
local file="" kind="" name="" target="" why=""
file="$(receipt_file)"
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
One critical, two moderate, and two nit findings remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
README.md:297
This describes the receipt fields with spaces, but the implementation is a TSV and explicitly rejects tabs/newlines in destinations. That notation is ambiguous for anyone consuming installed.tsv; use the repository's established <TAB> notation so the on-disk format is documented accurately.
`<name> <destination> <sha256> <UTC>` per regular file, at
openRepoTools:412
When OPENREPOTOOLS_BIN_DIR contains a newline, this embeds that raw newline in RECEIPT_WHY, and receipt_note prints it verbatim. The documented one-line receipt failure note is therefore split into multiple lines for one of the explicitly rejected TSV destinations; use an escaped/display-safe path or a fixed reason instead.
RECEIPT_WHY="$dest has a tab or a newline in it and this file is tab-separated"
openRepoTools:338
The new ${XDG_DATA_HOME:-...} branch is not exercised by the receipt tests: command_env removes XDG_DATA_HOME for every run and receipt_path() always asserts the $HOME/.local/share default. Add a case with OPENREPOTOOLS_DATA_DIR unset and XDG_DATA_HOME set, so a regression in the documented XDG location or its precedence cannot pass unnoticed.
The usage text has the same format ambiguity: it shows spaces between fields even though installed.tsv is tab-separated and destinations containing tabs/newlines are rejected. Document the literal tab separators here, matching the established <TAB> notation used throughout this repository.
…s rule that every mode stamp fails through die and #57's rule that a receipt failure is a note both hold
Merging main c1531eb (PR #102) into this branch brought `test_every_mode_stamp_in_this_command_fails_through_die`, which reads the script's text and holds every `chmod` to `|| die`. The receipt's `chmod 644 "$tmp" … || { rm -f; return 1; }` was the one stamp that could not obey it: #57 rules that a receipt this command cannot write is a note and never a refused install. A stamp that may neither die nor fall silent is a stamp that must not exist. The temporary `mktemp` makes is 0600 whatever the umask, which is `settings.json`'s mode too and enough for one account's evidence read by that account's own retirements, so the `chmod` is removed and the test asserts 0600 with both rules named.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Unresolved receipt permission and failure-handling issues could cause unsafe deletion or lost receipt data.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
openRepoTools:485
The implementation deliberately leaves the receipt at mktemp's 0600 mode, but the PR contract and the updated README document this file as 0644, while the new test also enforces 0600. These surfaces currently describe and verify different behavior; please make the mode choice consistent, including the non-fatal failure handling required if a mode change is made.
# AND NO `chmod`: the receipt's mode is the 0600 `mktemp` gives the temporary,
# whatever the umask, which is `settings.json`'s mode too. Two rules meet here
# and this is how both are kept. #48 (PR #102) rules that EVERY `chmod` this
# command performs fails through `die` — a mode-stamp failure is a refusal —
openRepoTools:598
When a matching receipt row exists but sha256_of cannot run (for example, both digest tools are missing or a tool temporarily fails), this returns unknown, and retire_commands then falls through to the header-marker removal arm. That can delete an edited file carrying the marker even though the receipt is present and the digest was never shown to match; the header fallback should apply only when no matching row exists, while an unverifiable row should be left/named or otherwise fail closed.
This second unguarded workdir call has the same failure mode after the retired file has already been removed: a temporary-directory failure exits the install without reporting that the receipt row could not be dropped. Receipt bookkeeping must remain a non-fatal diagnostic even on the retirement path.
workdir
openRepoTools:531
Receipt failures are supposed to be non-fatal, but workdir is unguarded here. If mktemp -d fails (for example because the temporary filesystem is unavailable), set -e aborts the install after the artifacts are already placed and no receipt_note is emitted. Route this failure through the same note-and-continue path.
workdir
openRepoTools:476
The caller relies on this helper returning nonzero when the existing receipt cannot be read, but the unconditional return 0 masks a failed input redirection. If installed.tsv is unreadable, the loop is skipped and the staged receipt can overwrite it with only the current rows (and receipt_forget can discard surviving rows) instead of taking the failure-note path. Propagate the loop/redirection failure before replacing the receipt.
…es to ten, its stale "0644" corrected to the 0600 the receipt is born at, and the README cap raised 472 -> 484 with a dated entry for #57 saying what the twelve lines buy
The paragraph carried the twenty-six/twenty-four artifact arithmetic and the
list of ways a receipt cannot be written; it now keeps only the facts a person
meets: the row format and path, the mode, what a later `--install` does with a
matching, a moved and an absent row, rows keyed by destination, the two things
that get no row, and that an unwritable receipt is one line and never a
refused install.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Landing note from lane openRepoTools-1 (2026-10-02). CI on 26f1d95: tests (Linux, full suite incl. the shell suite), tests-no-submodule, tests-windows, guard-launch-mode, SonarCloud and Sourcery pass; tests-macos fails inside tests/test_lane_helpers.sh with 9 cases. That job is red on main itself: main's last macOS run (69bf48d, 2026-09-30) failed the same suite with 19 cases, and 7 of this PR's 9 are in that set. The other two (an answer wider than the shell's integers refuses with 2, …saying it is not in the range the question offered) are lane picker cases; main's #135/#136 landed after that run with no macOS run of their own. This branch changes only the installer, README.md, tests/test_openrepotools_command.py and the README cap in tests/test_repo_hygiene.py — none of the lane helpers or their suite — so it cannot have introduced them. The macOS state belongs to the suite's owner (see #77). A fix round for Copilot's six threads of 2026-10-02 follows before the squash.
… on the strength of the header: `receipt_verdict` answers `unverified` when a row names the path and no digest could be taken, and the retirement keeps the header fallback for "no matching row" only (Copilot on #103 at `openRepoTools:711`, #57)
The first shape folded "no row" and "a row I cannot check" into one word, `unknown`, so on a host whose `sha256sum`/`shasum` was missing or failing the marker removed an edited, receipt-tracked `restart` — the ownership mistake the receipt exists to prevent. The new arm prints one line: the destination, that its row could not be checked because no digest tool answered, and the `rm -f --` for the person to run. Tested both with a `$PATH` that has neither tool at all (a symlink farm of the real one) and with shims that fail, and the receipt is asserted untouched.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…so a relative `$OPENREPOTOOLS_BIN_DIR` means the same file to the run that wrote a row and to a later run from any other directory (Copilot on #103 at `openRepoTools:524`, #57)
`receipt_dest` resolves a path's directory with `(cd -- "$dir" && pwd -P)` — no `realpath` on macOS or bash 3.2 — with `CDPATH` emptied and a trailing `X` so a directory name ending in a newline is not shortened past the TSV guard; `receipt_add` (record), `receipt_verdict` and `receipt_forget` all go through it, and the line a person reads still prints the path as they spelled it. `receipt_rows_except` no longer keeps a row whose destination is relative, because its `-f` answered for whichever directory the next run stood in. Two tests drive it from two working directories: an install with `bin` leaves only absolute rows and a run from elsewhere retires the right `restart`, and an old-shape `bin/restart` row can never remove another directory's file.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…NREPOTOOLS_BIN_DIR` with a TAB or a NEWLINE in its name installs everything, leaves the receipt byte for byte as it was, and prints the reason (Copilot on #103 at `openRepoTools:425`, #57)
The planner accepts both characters, so both are driven; the receipt is seeded with a row for a file that does not exist, which any rewrite would drop, and the data directory is asserted to hold nothing but `installed.tsv` so no temporary was left behind. Checked to have teeth by disabling the guard in a scratch edit, which turns both cases red on "the receipt was replaced".
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-16 totals: a row goes to every placed regular file (the installables plus the skill and command files, with no row for the two hook entries or for the receipt), and the number is derived as `ARTIFACTS - HOOK_ENTRIES` and named as 27 artifacts and 25 rows only where it must appear (Copilot on #103 at `openRepoTools:325` and `tests/test_openrepotools_command.py:822`, #57)
`INSTALLABLES` has thirteen files since `lane-rename` (Amendment 16, #81), so "twenty-six artifacts, twenty-four of them files, twenty-four rows" and "twenty-four rows, not twenty-six" were each two short; the three other places that carried the old totals ("beside the twelve", "twelve of the twenty-four", "twelve more rows every run", "twenty-four plus one more bin directory") now say what they mean without a number that the next list change would strand again.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…test reads each of them, and the pull request description is corrected to match (Copilot on #103 at `openRepoTools:503`, #57)
The description said 0644 while the code, the test and the README said 0600 — on purpose: #48 rules that every `chmod` this command performs fails through `die` and #57 rules that a receipt it cannot write is a note, so a stamp that may neither die nor stay silent must not exist, and `mktemp`'s 0600 stands. The usage paragraph named no mode at all, so it now states it and the clause a row it cannot check is named and left; the README paragraph says the same in the ten lines it already had, so its 484-line cap does not move; and `test_the_help_and_the_readme_say_the_mode_the_receipt_is_born_at` holds both so a fifth place cannot drift from the four.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the configured data directory with the same `pwd -P` the row destinations go through, every read, write and printed line uses that path, and a RELATIVE data directory is said out loud with where it resolved to (Copilot round 4 on #103 at `openRepoTools:365`, #57)
`receipt_file` handed the configured string to every caller, so a relative `$OPENREPOTOOLS_DATA_DIR` was `data/installed.tsv` in every line and every `[ -f ]`. It now goes through `receipt_dest`: canonical wherever the directory exists to resolve, the configured spelling where it does not (a getter that created directories would make a retirement's read a write), and `receipt_replace` makes the directory first and asks again, so a first install writes to and names the resolved path. What resolving cannot do is make `data` mean one directory from two working directories — a relative value names a different one from each — so `receipt_record` adds one line beside the one naming the receipt: the value is a RELATIVE path, where this run resolved it to, and to name an absolute one. The test installs from `here` with `data`, then runs from `there` with `../here/data` and checks the same receipt is read, the row is found, the file is removed and no second receipt appears anywhere.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…r a license for the header: `receipt_rows_except` fails instead of reporting a clean scan, so the install leaves the old receipt byte for byte and prints its "could not be read" note, and `receipt_verdict` answers `unreadable`, so the retirement names and leaves the file with the `rm` printed (Copilot round 4 on #103 at `openRepoTools:575` and `openRepoTools:716`, #57)
`receipt_rows_except` ended in an unconditional `return 0`, so a failed `done <"$1"` read as no rows and `receipt_record` replaced the receipt with this run's rows alone, losing every row about a copy it did not place; `receipt_verdict` read the same failure as `unknown`, which the header fallback answered by removing an edited file. Both now ask the question the loop is about to get — can the file be opened — before the loop; `unknown` is left for a scan that ran and found no row, and the new `unreadable` arm names the receipt and the destination. Two tests make the receipt mode 000 (skipped as root, the suite's own idiom): the install is whole, the note is printed, the seeded row survives byte for byte with no temporary left behind; and an edited, header-carrying `restart` is left intact with the line naming the unreadable receipt.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Fail closed for non-regular receipt paths during retirement
openRepoTools:747
A receipt symlink (or directory) is treated as unknown here, so retire_commands falls through to the legacy header check. If installed.tsv is a user-owned symlink/directory and an edited retired file still carries the marker, this can delete that file even though the receipt path is unusable; the fail-closed unreadable arm only covers regular files that cannot be opened. Distinguish a truly absent receipt from any existing non-regular receipt and return unreadable so the target is named and left.
…ve-the-listing
#83 (Amendment 18, one binding per lane) and this branch both added
columns to the end of `lanes-edit.sh lanes`'s row, and both changed what
column 10 says for a LIVE lane.
THE COLUMNS. #83 landed first, so its locality keeps column 13, and
Amendment 19's four move to 14-17 (class, started, head, flags) in every
reader: `lane_groups` (the class test was still reading $13, which is now
#83's `here`/`elsewhere`), `lane`'s split_row, `lanes`'s loop, the sweep's
row reader in `lane-end`, the manual's table, the field count (17) and
the eighteen `a19_field` cases. Column 13 is now printed `none` where no
binding stands rather than left empty: those three readers split the row
with a tab-IFS `read`, and a tab is IFS whitespace, so an empty 13 would
have handed the class to the variable named for the locality and shifted
every field after it. `lane_groups` files `none` exactly as it filed
empty (only `elsewhere` changes the group); #83's one assertion of the
empty value now asserts `none`.
COLUMN 10 FOR A LIVE ROW is #83's: the attach filled in (`tmux
switch-client`/`tmux attach -t <session>:<@id>`), rendered by `lanes` as
`attach:`. It supersedes this branch's `lane <name>` for the same row,
which was only ever there because the column said `none`; the two A19
assertions of that word now assert only that a LIVE row is never offered
a launch.
THE LIVENESS FIXTURE is #83's derived form (TIMEOUT_SECONDS + 600, read
out of the wrapper), which at this branch's 5400 is 6000. The wrapper's
pair test now accepts that form, proving the sed read finds the number
the wrapper runs with.
The rest is unions: both facts reads, both sections of the suite (A19
then A18, each restoring its own fixtures), both writers in the
workstation guard, both subcommand lists, and the AGENTS.md cap at the
merged count, 316.
Lane: openRepoTools-3
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Bring in #119, #103, #83, #139 and #93 so #61 lands last in the queue
on the main it will actually land on. One textual conflict:
- lanes-edit.sh, the `*)` arm's unknown-subcommand list: main added
`binding`, `request-handoff`, `register-row-local`, `retired-identity`
(#83) and `retire-rows`, `archive-rows` (#93); this branch added
`duplicate-holder`. Every name is kept, in main's order with
`duplicate-holder` after `forks` as before, and the list still equals
the dispatcher's arms exactly (test_repo_hygiene's derived check).
Checked against #61's code, nothing else needed a change: the suite's
fake-command block (tmux, claude, pgrep, ps, gh) is intact and no new
main code calls pgrep or ps; #93's columns 14-17 and #83's column 13
are in the `lanes` listing rows, which #61 never reads; LOG_AWK's
fields, live_holder's, transcript_holders' and lane_forks' output
formats are unchanged; #83's HANDOFF-REQUESTED is a lane-kind verb that
changes no state and holder_is_dead's five-verb whitelist skips it as
every last-line reader must; and lane-end's --retire-dormant sweep is
refused beside --retire and exits before the fork/duplicate retire path.
Lane: openRepoTools-3
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bring #97 onto a current base before the seam rework Brett Heap ruled on
2026-10-04 ("managed ledger owns enrolled lanes; #97 owns legacy — rework
both"). Main moved 14 commits past 957a26f, to daed209: #81 lane-rename,
#83 Amendment 18, #93 Amendment 19, #61 claim --force takeover, #146,
#119, #101-#103, #129, #131, #133, #135, #136, #139 and #134.
Four files conflicted, and in every one both intents survive:
- lanes-edit.sh, the function sections: both sides appended after
migrate_state_cells. Main's Amendment 19(c)/(d) sweep and archive stay
directly under it, because its header says "the shape
`migrate-state-cells` has one screen up"; #97's lifecycle and
inventory section follows them.
- lanes-edit.sh, the unknown-subcommand refusal: the union of both lists,
56 names, the same set as the dispatcher's 56 arms.
- lanes-edit.sh, the exit-code table, and the manual's copy of it: #97's
7 row ("another act got there first", in three verbs) is kept, and exit
9 now has two meanings, one per verb. #61 spent 9 on `claim --force`'s
abandoned takeover and #97 spent it on `lane-state`'s unreadable
snapshot, and each PR took it as unused. Neither verb can return the
other's 9, so the table names both and nothing is renumbered in a
merge. The manual's 9 row said "(`claim` only)", which the merge would
have made false; its 7 row now names #97's two fenced writers too, as
the code table already did. The next commit moves #97's 9 to 10.
- lane-handoff, the late-record restart line: #129's
`$lane_profile_word` (lclaude) with #97's lifecycle line under it.
- docs/README-lanes.md: Amendment 19's section, then #97's #91 section,
so the amendment sections stay in number order.
- tests/test_lane_helpers.sh: main's Amendment 19 and Amendment 18
sections run first, in main's order (A18's restores its fixtures), and
#97's self-contained #91 section follows them, before the
workstation-seam section, as it did on #97's branch.
lane-start auto-merged around #134's rewrite. #97's section 4a still
prints the reconciliation after Amendment 18's binding gate and before
section 5 and the STARTED/RESUMED write in 5b. AGENTS.md, README.md,
tests/test_repo_hygiene.py and skills/ are byte-identical to main, so
the 316- and 486-line caps already match the merged counts.
Locally, tests/run.sh -k 'lane_helpers_suite or repo_hygiene or
lane_start_claude_current or guard_launch_mode or install_skill_and_hook':
274 passed, 600 deselected, in 1560 s.
Lane: openRepoTools-3
Co-Authored-By: Claude Opus 5.5 <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
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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
What this is
openRepoTools --installretires a word by REMOVING the copy it once placed(
retire_commands, landed in #45), and the only evidence it had that a file at~/.local/bin/<name>is one it wrote was a CONTENT SUBSTRING: the sentenceInstalled on PATH by+ this installer's own name, which its own commandscarry in their header. Copilot's round-3 review of #45 (
openRepoTools:263)named that — "a user-owned
restartscript copied from an old installation orcontaining the same banner will satisfy
grepand be deleted on the nextinstall" — and it was DECLINED for
restart, because no installer that everplaced
restartwrote a receipt and the marker is therefore the only evidencethat exists for the copies on people's PATHs today.
But every retirement after this one can have better, which is #57. So
--installnow writes down what it placed, and a later retirement reads thatfirst.
What changes
${OPENREPOTOOLS_DATA_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/openRepoTools}/installed.tsv,mode 0600, one row per REGULAR FILE this run placed:
name<TAB>destination<TAB>sha256<TAB>UTC. Written whole throughmktempinthe same directory +
mv -f, which is the shapeplace_skill_and_hookalready writes
settings.jsonwith, minus itschmod: the 0600 is the onemktempgives the temporary, and there is deliberately no mode stamp.--install's mode-stamp targets prove-w, not that a chmod will succeed (ownership) #48rules that every
chmodthis command performs fails throughdie, and--installwrites a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57rules that a receipt it cannot write is a note and never a refused install;
a stamp that may neither die nor fall silent is one that must not exist, and
0600 wants none (one account's evidence, read only by that account's own
retirements). The code,
test_install_writes_a_receipt_of_every_file_it_placed,--helpand the README all say 0600.directory: the
SKILL.mds and the command files are regular files this runplaced too. The two hook entries get no row — an entry inside
somebody else's JSON file is not a file this command placed — and the receipt
carries no row for itself, so it is not an extra artifact: today that is
27 artifacts, 25 of them files, 25 rows (13 + 6 + 6 since Amendment 16 added
lane-rename), and the tests derive the count rather than pin it."rows for names placed this run replace that name's earlier row"; the name is
not unique once the skills and the command files are covered (
handoffis askill at two paths AND a command file at two more), and
retire_commandslooks a path up rather than a name. So the key is the path, the
namecolumnis the word
--installPRINTS beside that placement (park,handoff,/ctx), and a run replaces the row of every destination it placed and keepsevery other row — which also means moving
$OPENREPOTOOLS_BIN_DIRdoes notthrow away the evidence for the copies still in the old directory.
retire_commandsconsults it FIRST. A row whose sha256 still matches thefile on disk is this installer's copy beyond argument → removed, and its row
dropped. A row whose sha256 has MOVED is a person's edit of an installed
command → named, left, the
rmprinted for them, row kept — and that is trueeven where the header marker is still in the file, because the receipt is
the stronger evidence and that is the whole point of
--installwrites a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57. No row at all istoday's header-marker fallback, byte for byte, so nothing that works now
stops working. A symlink is still refused outright, before the receipt is
asked at all.
line on stdout: what it could not write, where, why, that the install itself
is complete, and that a retirement falls back to the header. The exit belongs
to the reason rather than to the line — a path it may not write names
$OPENREPOTOOLS_DATA_DIR, a machine with neither digest tool is told thatinstead. Fail closed, out loud, carry on.
sha256sum, elseshasum -a 256— one helper, bash-3.2 clean, because astock macOS ships no
sha256sum. The file is fed on stdin rather thannamed as an operand: GNU
sha256sumescapes a filename containing abackslash and prefixes the line with
\, which would put a\on the frontof the digest for a bin directory somebody spelled with one.
says so, rather than a row that is silently two columns or two rows. A TSV
has no escape,
$OPENREPOTOOLS_BIN_DIRis a path a person chooses, and areceipt missing exactly the file a retirement will ask about is worse than
none.
mv -freplaces thelink itself, so an install over one would swap a person's link for a regular
file;
path_kind— the same helper both planners use — is asked first, andanything that is not a regular file there is named and left.
never read as a list of paths to remove, so a receipt somebody else authored
can change one answer and can never name a file for removal.
usage()gains the paragraph (where it lives, what it is for) and both$OPENREPOTOOLS_DATA_DIRand$XDG_DATA_HOMEin the variable block, whichtest_help_names_every_variable_it_readsnow pins; the README gains the samein its install section and a row in its variable table (the README is 484 lines and the cap in
tests/test_repo_hygiene.pymoved 472 -> 484 on 2026-10-02 with a dated entryfor
--installwrites a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57, after main itself had reached 472).The decision on
test_installing_twice_changes_nothing"Changes nothing" is about the bin directory and the artifacts — the lines
each placement prints — and that test asserts exactly those, so it is untouched
and still honest. The receipt's UTC is the stamp of the run that RECORDED the
row and does change on a re-install; that is the honest reading of a column
written by a run that verified a file it did not rewrite, and it is held by
test_installing_twice_leaves_no_duplicate_rows, which pins that the rowsthemselves (name, destination, digest) do not move and that no destination
gains a second row.
How it was tested
tests/run.sh tests/test_openrepotools_command.py tests/test_install_skill_and_hook.py tests/test_repo_hygiene.pyon Linux, and eight new cases in
tests/test_openrepotools_command.py(sevenmore from the 2026-10-02 fix round are listed at the end):
test_install_writes_a_receipt_of_every_file_it_placed— one row per placedfile (
ARTIFACTS - HOOK_ENTRIES, derived from the three lists), each digestrecomputed with
hashlibagainst the bytes at that destination, each stampparsed, mode 0600, and the line that says so.
test_a_retirement_reads_the_receipt_before_the_header— a file with nomarker at all is removed on the strength of a matching row, and its row is
dropped.
test_a_file_the_receipt_no_longer_recognises_is_named_and_left— a filewith the marker whose row's digest has moved is named, left byte for
byte, printed with its
rm, and its row is kept.test_a_receipt_it_cannot_write_is_a_note_and_never_a_refusal— a regularfile where the data directory goes (a
mkdir -pno user can win, rootincluded): exit 0, thirteen of thirteen placed, and the note naming the path,
the reason and the fallback.
test_a_receipt_that_is_a_symlink_is_not_written_through— the link andwhat it points at are both untouched.
test_a_digest_it_cannot_take_is_a_note_too—sha256sumandshasumshims that exit 1, first on
$PATH: the install still succeeds and noreceipt is written.
test_installing_twice_leaves_no_duplicate_rows.test_the_receipt_keeps_the_rows_of_a_directory_it_no_longer_writes.The fourth arm's sibling — a machine with NEITHER
sha256sumNORshasumonits
PATHat all — is a one-line guard checked by hand against a$PATHbuiltwithout them (it names the missing tools rather than blaming the first file it
could not digest); building that
$PATHinside a test would not survive themacOS job, so the shim test above covers the same
receipt_notepath.command_envnow clears$OPENREPOTOOLS_DATA_DIRand$XDG_DATA_HOME, soa developer who exports the latter — normal on a Linux desktop — cannot have
this suite writing a receipt into their real data directory however carefully
$HOMEis redirected.Merge note
The receipt is its own section of helper functions with ONE call line in
install_commands(betweenplace_skill_and_hookandretire_commands) and afour-arm
ifinretire_commands, so the hunks inside those two functions areas small as they can be. Measured against the other branches touching this
file: #81 changes the
INSTALLABLES=line and the counts around it — thissection is inserted after the blank line below that assignment, so the two
changed ranges are separated and do not overlap — and the sibling branch's
chmodlines in the install loop are untouched here.Closes #57
Lane: openRepoTools-1
After the 2026-10-02 resume (lane openRepoTools-1)
origin/mainmerged in twice (c1531eb, then cbb5981). The first merge met Everychmod--installperforms fails throughdie, and a second writer of either hook refuses whether or not our own entry is beside it #102's new rule that everychmodthe installer performs fails throughdie: the receipt's ownchmod 644was the one stamp that could not obey it, because--installwrites a receipt of what it placed, so a later retirement has ownership evidence stronger than a header substring #57 rules that a receipt failure is a note and never a refused install, so thatchmodis gone and the receipt is born atmktemp's 0600, which issettings.json's mode too.cd && pwd -P), so a relativeOPENREPOTOOLS_BIN_DIRcannot point a later run at another directory's file; the tab/newline TSV guard has its regression tests;--help, the README and this body say 0600; the artifact counts are derived and current.test_a_row_it_cannot_verify_is_named_and_left_never_removed, the two relative-destination cases, the two tab/newline guard cases,test_the_help_and_the_readme_say_the_mode_the_receipt_is_born_at, and the old-shape-row case.tests-macosis red insidetests/test_lane_helpers.sh, as it is onmainitself (see the landing note comment and The shell suite takes 45 minutes on macOS — the cap it just crossed is not the fix, its duration is #77); this branch touches no lane helper.🤖 Generated with Claude Code
Summary by Sourcery
Add receipt-based ownership tracking to
--installso retirements can safely distinguish unchanged installed files from user-owned edits.New Features:
--installin a secure, atomically written receipt containing its destination, digest, and timestamp.Bug Fixes:
Enhancements:
Documentation:
Tests: