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
{{ message }}
Repository navigation
--install writes every file beside its target and renames it over the target, so a lane tool running mid-install finishes on its own text (#167) - #172
./openRepoTools --install no longer writes an installed file in place with cp. place_by_rename is used for the bin files, the skills and the command files. It works in four steps:
Write the bytes to a mktemp temporary in the target's own directory (.<name>.openrepotools.XXXXXX).
Set the mode on the temporary (755 for bin files, 644 for skills and command files).
Refuse if a directory now sits at the target, because mv would put the file inside it.
mv -f the temporary over the target.
The cmp -s skip stays. A file whose bytes are already right is not rewritten, and its mode is stamped in place.
Every failure cleans up. A failed mktemp, cp, chmod or mv removes the temporary and refuses with exit 2, naming the target. The old copy is left untouched.
A signal cleans up too.$PLACING holds the temporary's prefix, .<name>.openrepotools.<pid>., from before mktemp runs until after the mv. The EXIT trap that workdir sets removes "$PLACING"*. No instant exists in which the file is there and the trap does not know it (Copilot round 2).
The owner stays the owner (Copilot round 1). owner_of reads the target's uid:gid with ls -lnd, not stat. Where it differs, the temporary is chowned to it before the rename, so a root --install into another user's home leaves their copies theirs, as cp did. Where a differing owner cannot be kept, the placement refuses and the old copy stays, and so does an existing target whose owner cannot be read at all (Copilot at bef4b85). A group that alone cannot be kept is no reason to refuse: at 755 and 644 the group bits equal the other bits.
The planners refuse what they refused before: a symlink, a directory, or a file this user cannot write. Their reasons are restated for a rename, which would replace such a target unasked rather than fail.
This complements #106, the missing inter-process lock, and does not replace it. Two racing installs now each place whole files.
Sibling PRs:#168 and #169 (lane openRepoTools-3). At this head, bef4b85, git merge-tree is clean with #168 (87da332), with #169 (45fa45d), with #174 (c550e89, lane-worktrees only) and with main (00971b9). On the merged tree with #169, which contains #168, the targeted tests passed 32. lanes-edit.sh, lane-worktrees and any new installable are covered automatically: the bin loop calls place_by_rename for every INSTALLABLES entry. On that tree lane-worktrees was replaced by rename (new inode, 17 of 17 placed, no temporary left).
Before and after
Run against main fea9a29's openRepoTools, the new tests fail 8 of 11:
test_a_running_lane_tool_finishes_on_its_own_text_when_an_install_replaces_it: an installed lanes-edit.sh is running while --install updates it. The running process printed new: read from the old offset and exited 7, because it read the new text from its old byte offset.
test_every_placement_replaces_its_target_rather_than_rewriting_it: .../.local/bin/lanes-edit.sh was rewritten in place: same inode 85214120.
test_a_placement_that_fails_leaves_the_old_copy_and_no_temporary[cp|chmod|mv]: each exited 0 instead of refusing.
test_a_mode_stamp_the_bin_loop_cannot_make_reads_as_a_refusal: main leaves the new bytes at the target when the mode stamp fails.
On this branch all of them pass. The running process prints old: finished and exits 0, and the path holds the new bytes at 755.
The signal cleanup was verified on the real --install, held mid-placement by a slow fake cp. SIGTERM to the pid, SIGHUP to the pid and SIGINT to the process group (Ctrl-C) each killed the run, and no temporary was left. With the trap's PLACING clause removed, every one left .openRepoTools.openrepotools.* behind. That property is now test_a_run_killed_mid_placement_leaves_no_temporary, with SIGTERM and group SIGINT, each over a slow cp and a slow mktemp. The mktemp cases fail at 0043d37, where the trap learnt the name only from mktemp's answer.
Ownership is covered by a fake ls that reports another account as the owner, in the same way the suite's fake chmod stands in for ownership. A recording chown shows the temporary handed back to the old uid:gid exactly once. The real chown, run as non-root, shows the refusal with the old copy and its inode intact. Both fail at e05c2e9.
Platforms
Linux and macOS: the installer tests run end to end.
Windows Git Bash: the installer tests carry WINDOWS_SKIP. tests/test_install_by_rename.py therefore runs the installer's own place_by_rename text under each platform's bash. On Windows that is the Git Bash beside the git running the suite. The test covers a running script replaced mid-run, and a directory at the target.
Suite
Main merged at bef4b85 (merge commit, no rebase): main at 00971b9, #121, which changes lane-handoff, lane-start, lanes-edit.sh, tests.yml and the lane-helper tests. It shares no file with this PR, and --install places those very tools, so the combination was gated before this push:
the four touched modules under the lock at the fix head (test_install_by_rename, test_openrepotools_command, test_install_skill_and_hook, test_repo_hygiene)
321 passed
CI and tests-macos at the head
pending, filled in when done
The results below are from the previous head, f85e618:
Local, under the workstation lock, at f85e618 (tests/test_install_by_rename.py, test_openrepotools_command.py, test_install_skill_and_hook.py, test_repo_hygiene.py): 320 passed.
tests-windows: 155 passed, 783 skipped. Main has 153 passed; the two extra are the Git Bash tests in tests/test_install_by_rename.py.
parse-macos, guard-launch-mode and SonarCloud: pass.
tests-macos at f85e618, through workflow_dispatch run 37382514387: 938 passed under Apple's /bin/bash 3.2.57. The PR's own tests-macos waits for the ready label, as AGENTS.md rules.
Review
Copilot round 1 (e05c2e9): one thread, on ownership. Taken in 0043d37, answered, and resolved.
Copilot round 2 (0043d37): one finding with no inline thread, the mktemp window, plus a note on groups. Both taken in f85e618 and answered in a PR comment.
Copilot round 3 (f85e618, past the two-round cap): "Approval recommended", no findings.
Copilot at bef4b85 (requested explicitly; a merge-only push drew no automatic review): one thread. An existing target with an unreadable owner was renamed over. Taken test-first, answered and resolved.
Make --install replace files atomically and safely so concurrent or interrupted installations cannot alter running tools or leave partial files behind.
New Features:
Install files atomically by placing temporary copies beside their targets and renaming them into place, so running tools continue using the original inode.
Bug Fixes:
Prevent partial or unsafe installations by cleaning up failed placements and signal-interrupted temporaries while preserving existing targets.
Preserve target ownership where possible and refuse replacement when ownership cannot be read or retained.
Refuse directory targets during placement instead of allowing rename operations to place files inside them.
Enhancements:
Extend installer coverage across binaries, skills, commands, and hooks while retaining byte-comparison skips and mode updates.
Tests:
Add end-to-end and cross-platform tests covering atomic replacement, running scripts, failure preservation, ownership handling, directory targets, and signal cleanup.
… target, so a lane tool running mid-install finishes on its own text (#167)
`cp` onto an installed file kept its inode and rewrote its bytes, and bash
reads a running script from its file at an offset, so a lane tool that was
running when an install replaced it read the NEW text from the OLD offset
(measured 2026-09-30: `A.sh: line 4: ript,: command not found`).
`place_by_rename` writes each placement to a `mktemp` temporary in the
target's own directory, stamps its mode there, refuses a directory at the
target, then `mv -f`s it over the target. Every failure removes the
temporary and refuses naming the target, leaving the old copy untouched;
`$PLACING` and the EXIT trap remove it on a signal. The `cmp -s` skip stays
and stamps the mode in place. Used for the sixteen bin files, the skills
and the command files. The planners' refusals keep refusing a symlink, a
directory and a read-only file, with their reasons restated for a rename.
README.md is left alone here: its install paragraph is being rewritten by
the sibling PRs #168 and #169, so its one sentence about `cp` following a
symlink is corrected after they land rather than in a conflicting hunk.
Tests: a running lane tool finishes on its own text across an install; a
new inode at every changed path while an open handle keeps the old bytes; an
unchanged install renames nothing; a failing cp, chmod or mv leaves the old
copy and no temporary; a run killed by SIGTERM or SIGINT mid-placement
leaves no temporary; and the function itself on each platform's bash,
including Git Bash on Windows.
Lane: openxfactory-5 (openXfactory-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The installer now stages every changed file in a temporary beside its destination, applies its mode, and atomically renames it over the target, allowing running tools and open readers to finish on the old inode while preserving no-op behavior and robust cleanup. New focused and end-to-end tests cover POSIX/Git Bash behavior, directory and tool failures, signal cleanup, inode replacement, modes, and unchanged installs.
File-Level Changes
Change
Details
Files
Replace in-place installation writes with atomic, same-directory temporary-file placement and rename.
Stage bytes beside each target with mktemp, apply the intended mode, validate the target is not a directory, then mv -f into place.
Preserve cmp-based no-op behavior and in-place mode stamping for byte-identical files.
Track the active temporary in PLACING so the EXIT cleanup removes it on signals.
Keep planner refusals for symlinks, directories, and unwritable files, with rename-specific failure messages.
openRepoTools
Add cross-platform tests for the installer’s placement primitive and Git Bash rename semantics.
Extract the installer’s place_by_rename function into a bash harness rather than duplicating its implementation.
Verify a running script continues on its original inode after replacement and directories at targets are refused.
Run the tests under POSIX bash and Git Bash located beside the active git executable.
tests/test_install_by_rename.py
Expand end-to-end installer coverage for atomic replacement and failure cleanup.
Verify all installed file categories receive new inodes while held readers retain old bytes and modes remain correct.
Verify unchanged files are not renamed, and cp, chmod, and mv failures leave the old target intact with no temporary.
Verify SIGTERM and SIGINT during placement remove temporary files, and update mode-failure expectations for staging.
Change --install so every differing installed file is written to a temporary file in the target's directory, given the correct mode, and atomically replaced with mv -f rather than rewritten in place.
Ensure the replacement protects processes already running from an installed script, so they continue reading the old inode and complete on their original contents.
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!
… and gid before the rename, and a copy another account owns is refused rather than taken over (Copilot on #172)
A rename puts the temporary's inode at the path, and the temporary belongs
to whoever runs the install, so a root --install into another user's home
would have left that user root-owned copies their own next --install
refuses as unwritable. `cp` kept the owner by keeping the inode.
`place_by_rename` now asks `owner_of` (`ls -lnd`, no `stat`) for the
target's uid:gid and, where the owner differs from the temporary's, gives
the temporary that owner before the rename. Where that `chown` is refused,
the placement removes the temporary and refuses, leaving the old copy as it
was. The cross-platform harness carries `owner_of` beside the function.
Tests: a fake `ls` stands in for another account's ownership. A recording
fake `chown` shows the temporary handed back to uid:gid exactly once, and
the real `chown` (not root) shows the refusal with the old copy, its inode
and no temporary left.
Lane: openxfactory-5 (openXfactory-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Record temporary path before mktemp for reliable signal cleanup
openRepoTools:1041
A terminating signal can arrive after mktemp has created the file but before the next command assigns PLACING. In that window the EXIT trap still sees an empty value and leaves the temporary behind, so the advertised signal cleanup is not guaranteed; the new test only sends its signal after it has observed a temporary, which normally occurs after this assignment. Record a collision-safe candidate path before creating it (or otherwise make creation and trap ownership share a pre-recorded identifier) so cleanup also covers interruption during mktemp.
… that cannot be kept does not stop a placement (Copilot round 2 on #172)
A signal that arrived after `mktemp` had made the temporary and before its
name was assigned to `$PLACING` left the file behind, because the trap knew
only `mktemp`'s answer. `$PLACING` is now the temporary's PREFIX,
`.<name>.openrepotools.<pid>.`, set before `mktemp` runs, and the EXIT trap
removes `"$PLACING"*`. Reproduced with a slow `mktemp` that makes the file
and stalls before printing its name: SIGTERM to the run and SIGINT to its
group both left the temporary at the previous head and leave nothing now.
The owner rule now tries to keep the group as well. Where only the group
differs and cannot be kept, the copy is placed anyway: at 755 and 644 the
group bits equal the other bits, so no permission moves. A differing owner
that cannot be kept is still a refusal.
Lane: openxfactory-5 (openXfactory-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot round 2 (review 5421300782, on 0043d37) had one finding, listed under "Previously missed" with no inline thread, plus a note in its overview. Both are taken in f85e618.
The temporary's path is recorded before mktemp creates the file (openRepoTools:1041). Before, a signal that arrived after mktemp had made the temporary but before its name was assigned to $PLACING left the file behind. $PLACING is now the temporary's prefix, .<name>.openrepotools.<pid>., set before mktemp runs. The EXIT trap removes "$PLACING"*, and only when the prefix is set. The pid keeps the prefix to this run.
Reproduced with a slow mktemp that makes the file and stalls before printing its name. At 0043d37, SIGTERM to the run and SIGINT to its group both left .openRepoTools.openrepotools.* behind. At f85e618 neither does.
test_a_run_killed_mid_placement_leaves_no_temporary is now parametrized over a slow cp and a slow mktemp, for both signals.
"Replacement can silently change a target's group ownership" (overview). The group is kept where it can be: the temporary is chowned to the target's uid:gid whenever they differ. Where only the group differs and that chown is refused, the copy is placed anyway, because at 755 and 644 the group bits equal the other bits and no permission moves. The comment above place_by_rename says so. test_a_group_that_cannot_be_kept_does_not_stop_the_placement holds it.
Main moved one commit since f85e618: #121, supervised manual ctx kept as
legacy compatibility, which changes lane-handoff, lane-start, lanes-edit.sh,
tests.yml and the lane-helper tests. No file is shared with this branch; the
merge gates the combination, because --install places those very tools.
Lane: openxfactory-5 (openXfactory-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…renamed over (Copilot on #172 at bef4b85)
`owner_of` answers nothing where `ls` cannot say, and an empty answer is
also what an absent target gives, so `place_by_rename` read a target that
EXISTS with an unreadable owner as one with nothing to keep, and a root
--install could rename its own temporary over it after all. Such a target
is now refused before the rename: the temporary is removed, the old copy
keeps its bytes and its inode, and the refusal names the target.
Test first: a `ls` that fails for the one target exited 0 and replaced it at
bef4b85, and is refused (2) now.
Lane: openxfactory-5 (openXfactory-5)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LANDED — lane openXfactory-5, session bbe4d255-02db-4578-840b-09924442e544@Eagle, 2026-10-06T17:35:20Z, PR #172 into opensoft/openRepoTools main → merge 3660224; --install writes every file beside its target and renames it over the target, so a lane tool running mid-install finishes on its own text (#167)
Bring #169 current with main after #168 landed as 2080f3d, so the
lander can squash #169 on a green head. Main also carries #171
(f5444c5) and #172 (3660224). The merge base is 00971b9 because the
squash is not an ancestor of this branch, so git saw #168's files as
added on both sides. Four files conflicted:
- lane-worktrees (add/add) and README.md: main's copy is byte-identical
to #168's old head 87da332, and main changed neither file after it,
so this branch's copy (#168 plus #169) is the resolution.
- docs/README-lanes.md: the one block is #169's two report subsections
against nothing on main's side. They stay, and #171's worktrees-table
subsection, which merged cleanly, stays too.
- lanes-edit.sh: #171 and #169 each added a usage line, a dispatcher arm
before `*)`, and a refusal-list entry. Main's text comes first, then
#169's: the `worktrees` line and arm, then `pathspec-check`. The list
ends `|set-restart-intent|worktrees|pathspec-check`, so it still
matches the arms the repo-hygiene parser reads (64).
Changed-line comparison: this merge over main equals #169's own diff,
and over 45fa45d equals main's changes since 87da332 (#171 and #172).
The only difference in each is the refusal-list line that carries both
entries.
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.
Lane: openxfactory-5 (openXfactory-5)
Brett Heap's word, 2026-09-30, verbatim: "file and fix both". This is the fix half for the first of the two.
Closes #167
What
./openRepoTools --installno longer writes an installed file in place withcp.place_by_renameis used for the bin files, the skills and the command files. It works in four steps:mktemptemporary in the target's own directory (.<name>.openrepotools.XXXXXX).mvwould put the file inside it.mv -fthe temporary over the target.cmp -sskip stays. A file whose bytes are already right is not rewritten, and its mode is stamped in place.mktemp,cp,chmodormvremoves the temporary and refuses with exit 2, naming the target. The old copy is left untouched.$PLACINGholds the temporary's prefix,.<name>.openrepotools.<pid>., from beforemktempruns until after themv. TheEXITtrap thatworkdirsets removes"$PLACING"*. No instant exists in which the file is there and the trap does not know it (Copilot round 2).owner_ofreads the target'suid:gidwithls -lnd, notstat. Where it differs, the temporary ischowned to it before the rename, so a root--installinto another user's home leaves their copies theirs, ascpdid. Where a differing owner cannot be kept, the placement refuses and the old copy stays, and so does an existing target whose owner cannot be read at all (Copilot at bef4b85). A group that alone cannot be kept is no reason to refuse: at 755 and 644 the group bits equal the other bits.cpfollows a symlink, and an install through one leaves the command uninstalled…", is now a stale reason for a refusal that still holds. It gets a one-line follow-up once lane-worktrees sweep: retire a lane's abandoned worktrees, rescue first, never a live writer (#162, part 1 of 2) #168 and lane-worktrees: the daily estate report, and nothing new lands beside the code (#162, part 2) #169 land.This complements #106, the missing inter-process lock, and does not replace it. Two racing installs now each place whole files.
Sibling PRs: #168 and #169 (lane openRepoTools-3). At this head, bef4b85,
git merge-treeis clean with #168 (87da332), with #169 (45fa45d), with #174 (c550e89,lane-worktreesonly) and with main (00971b9). On the merged tree with #169, which contains #168, the targeted tests passed 32.lanes-edit.sh,lane-worktreesand any new installable are covered automatically: the bin loop callsplace_by_renamefor everyINSTALLABLESentry. On that treelane-worktreeswas replaced by rename (new inode,17 of 17 placed, no temporary left).Before and after
Run against main fea9a29's
openRepoTools, the new tests fail 8 of 11:test_a_running_lane_tool_finishes_on_its_own_text_when_an_install_replaces_it: an installedlanes-edit.shis running while--installupdates it. The running process printednew: read from the old offsetand exited 7, because it read the new text from its old byte offset.test_every_placement_replaces_its_target_rather_than_rewriting_it:.../.local/bin/lanes-edit.sh was rewritten in place: same inode 85214120.test_a_placement_that_fails_leaves_the_old_copy_and_no_temporary[cp|chmod|mv]: each exited 0 instead of refusing.test_a_mode_stamp_the_bin_loop_cannot_make_reads_as_a_refusal: main leaves the new bytes at the target when the mode stamp fails.On this branch all of them pass. The running process prints
old: finishedand exits 0, and the path holds the new bytes at 755.The signal cleanup was verified on the real
--install, held mid-placement by a slow fakecp. SIGTERM to the pid, SIGHUP to the pid and SIGINT to the process group (Ctrl-C) each killed the run, and no temporary was left. With the trap'sPLACINGclause removed, every one left.openRepoTools.openrepotools.*behind. That property is nowtest_a_run_killed_mid_placement_leaves_no_temporary, with SIGTERM and group SIGINT, each over a slowcpand a slowmktemp. Themktempcases fail at 0043d37, where the trap learnt the name only frommktemp's answer.Ownership is covered by a fake
lsthat reports another account as the owner, in the same way the suite's fakechmodstands in for ownership. A recordingchownshows the temporary handed back to the olduid:gidexactly once. The realchown, run as non-root, shows the refusal with the old copy and its inode intact. Both fail at e05c2e9.Platforms
WINDOWS_SKIP.tests/test_install_by_rename.pytherefore runs the installer's ownplace_by_renametext under each platform's bash. On Windows that is the Git Bash beside thegitrunning the suite. The test covers a running script replaced mid-run, and a directory at the target.Suite
Main merged at bef4b85 (merge commit, no rebase): main at 00971b9, #121, which changes
lane-handoff,lane-start,lanes-edit.sh,tests.ymland the lane-helper tests. It shares no file with this PR, and--installplaces those very tools, so the combination was gated before this push:python3 -m pytest tests -qunder the workstation locktests/test_install_by_rename.pytests/test_openrepotools_command.pytests/test_install_skill_and_hook.pytests/test_supervised_restart_regressions.py(#121)tests/test_lane_helpers_suite.py(runstests/test_lane_helpers.sh, #121's changes included)test_a_copy_whose_owner_cannot_be_read_is_refused_and_left_as_it_wastest_install_by_rename,test_openrepotools_command,test_install_skill_and_hook,test_repo_hygiene)tests-macosat the headThe results below are from the previous head, f85e618:
tests/test_install_by_rename.py,test_openrepotools_command.py,test_install_skill_and_hook.py,test_repo_hygiene.py): 320 passed.tests: 938 passed.tests-no-submodule: 730 passed, 208 skipped.tests-windows: 155 passed, 783 skipped. Main has 153 passed; the two extra are the Git Bash tests intests/test_install_by_rename.py.parse-macos,guard-launch-modeand SonarCloud: pass.tests-macosat f85e618, throughworkflow_dispatchrun 37382514387: 938 passed under Apple's/bin/bash3.2.57. The PR's owntests-macoswaits for thereadylabel, as AGENTS.md rules.Review
mktempwindow, plus a note on groups. Both taken in f85e618 and answered in a PR comment.🤖 Generated with Claude Code
Summary by Sourcery
Make
--installreplace files atomically and safely so concurrent or interrupted installations cannot alter running tools or leave partial files behind.New Features:
Bug Fixes:
Enhancements:
Tests: