Skip to content

Lifecycle reads that failed are no longer answers, set-lane-tree's fence closes with its operation, and a lane sidecar is valid only when whole - #252

Merged
brettheap merged 13 commits into
mainfrom
fix/lifecycle-fail-open-113-114-115
Oct 19, 2026
Merged

brettheap merged 13 commits into
mainfrom
fix/lifecycle-fail-open-113-114-115

Conversation

@brettheap

@brettheap brettheap commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Closes #113, #114, #115

Lane openRepoTools-4's PR for the three findings #97's sixth Copilot round filed rather than took. Lane 4 took them over from lane openRepoTools-3, with its consent, at 2026-10-18T13:35Z (the TAKEOVER comments on each issue). Opened on main 87a5e9d. Main babd9fe was merged in, and later main 55aae16, which holds #246, #214 and #219. Brett Heap's word for this PR decides the merge.

Commits:

  1. 6924ecf (A lane sidecar passes validation on its schema: line alone, so a truncated schema: 1 record is read AND replaced #115) — a sidecar of the current schema is read and replaced only when it is whole;
  2. 97be67a (set-lane-tree's fence accepts a poll from a finished operation, and its observation check accepts a partial one it completes with clean-looking defaults #114) — set-lane-tree's fence refuses a poll filed under a finalized operation;
  3. b389d0c (The lifecycle's remaining fail-open reads: an unreadable register becomes 'no holder', an unreadable sidecar becomes no tree, and two git reads become fabricated values #113) — lane-reconcile keeps the status of every read it rests on;
  4. 469fc2c — the tests;
  5. 33515f8 — the manual, design.md decisions 26–28, and tasks.md section 7;
  6. 0dfd8be — a normal merge of main babd9fe (clean), which brought session_ids_of_lane / session_ids_local_of_lane don't distinguish "no row" from "register unreadable", so every caller that treats an empty id set as a fact is fail-open on that read #67's session_ids_checked;
  7. d727834 (The lifecycle's remaining fail-open reads: an unreadable register becomes 'no holder', an unreadable sidecar becomes no tree, and two git reads become fabricated values #113) — lane-reconcile reads the holder ids through session_ids_of_lane / session_ids_local_of_lane don't distinguish "no row" from "register unreadable", so every caller that treats an empty id set as a fact is fail-open on that read #67's session_ids_checked, and the copy of that read commit 3 had written (lane_session_ids_read) is gone;
  8. 36520b9 (The lifecycle's remaining fail-open reads: an unreadable register becomes 'no holder', an unreadable sidecar becomes no tree, and two git reads become fabricated values #113) — a failed registrations read is named in lane-worktrees' words, so lane-end still relays lane-worktrees: review findings deferred from #168/#169 (incl. five --yes data-loss paths found after the two-round cap) #170 G5's reason (see CI below);
  9. e95d072 (A lane sidecar passes validation on its schema: line alone, so a truncated schema: 1 record is read AND replaced #115, review r1) — the validator asks the snapshot's arm first, so a control root under a directory named trees keeps its snapshot; set-lane-tree refuses a blank field with 64 before the lock; lane_no_repository asks git in the C locale;
  10. f80542e — tests for those three, each red at 36520b9;
  11. 3ffefce — the manual's main exit table (10 and 64), the malformed row's kept path, and decision 28 (the validator's order, and why a record's lane: is not compared);
  12. e3acbea — a normal merge of main 55aae16 (clean): Make the lane tools look inside a triad's legs and paired feature trees, so the sweep, lane-end's gate, lane-handoff, lane-reconcile and the daily report can see a lane's work there, and change nothing a checkout with no project.yaml does #246 (T018), lanes-edit.sh: a write with nothing staged still publishes the local commits origin lacks, so the RECOVERY's re-run of commit pushes the edit it stranded, and the conflict RECOVERY warns that reset --hard drops a peer's commits #214 and lanes-index follow-ups of #166 and #173: export reads one snapshot, history --index says its closing stderr count, a foreign source is refused before the lock, a damaged table reads as unusable, an unread lane stays NOT READ in the index, and the worktrees footer names the workstation #219's lanes-index reader are now under this branch;
  13. e6a41b4 (A lane sidecar passes validation on its schema: line alone, so a truncated schema: 1 record is read AND replaced #115, lanes-index follow-ups of #166 and #173: export reads one snapshot, history --index says its closing stderr count, a foreign source is refused before the lock, a damaged table reads as unusable, an unread lane stays NOT READ in the index, and the worktrees footer names the workstation #219) — lanes-index follow-ups of #166 and #173: export reads one snapshot, history --index says its closing stderr count, a foreign source is refused before the lock, a damaged table reads as unusable, an unread lane stays NOT READ in the index, and the worktrees footer names the workstation #219's two path-less sidecar cases in tests/test_lanes_index.py now assert A lane sidecar passes validation on its schema: line alone, so a truncated schema: 1 record is read AND replaced #115's rule (see "Merged with main" below).

#115 comes first because #114's refusal message reads the validator's LSS_WHY. Each commit's lanes-edit.sh parses on its own (bash -n on all thirteen).

#113 — the lifecycle's remaining fail-open reads

item before now where (head)
1. holder ids session_ids_of_lane … || : and the local read || :, so an unreadable register gave an empty id set, live_holder answered 8 for it, and the verdict read ungraceful-stop/interrupted-swap lane_reconcile reads the ids through #67's session_ids_checked. That answers 1 where the published register (or, with none published, the working tree's) or this checkout's own copy cannot be read, and 8 only where both were read and give no id. A 1 gives HOLDER unknown and verdict indeterminate. #67's comment now counts three callers lanes-edit.sh:13929, :13931, :14320
2. unreadable sidecar taken in 8.12: an <unreadable> row, reported as unreadable-sidecar unchanged. What 8.12 left: lane_reconcile still read lane_trees_list with || :, so its 10 for a trees/ it cannot list (#157 4e) dropped the whole inventory and the verdict could be resumable. That 10 is now indeterminate :14003, :14006
3. branch.<b>.remote || :, so a failed read became the fabricated upstream ./<merge> the case its merge sibling already has. A failed read is 3: lane-tree-now exits 1 and set-lane-tree (reading the tree itself) refuses :13822
4. git worktree list --porcelain || :, so registrations nobody could list read as none the status is kept, and a failure makes the verdict indeterminate and names it in lane-worktrees' words: the worktree registrations of <dir> could not be read (git worktree list exited N). A recorded directory that is no repository at all is still an absence, by lane-worktrees' no_repository rule (#170 G5). lane_no_repository asks git in the C locale, as lane-worktrees' git_env does, so a translated git cannot turn that absence into a failed read. The on-disk half of the same sweep gets the same treatment: a lane worktree root that cannot be listed or searched is indeterminate too :14152, :14155, :13841, :13844, :14255; verdict :14359

The line between a row and a verdict is the one T018 draws for an unreadable leg: a per-record failure is a row counted toward recovery, and a read that leaves paths unread makes the verdict indeterminate.

#114 — set-lane-tree's fence

item now where
1. a poll from a finished operation a fenced set-lane-tree reads the lane's state with its generation and operation, under the same mutex, and is refused with 7 where the lane is SWAPPED :18285, :18302
2. a partial observation a 64 since 8.12, unchanged —

Why SWAPPED and not "require SWAPPING" (the issue's first option): the shell suite's repoRC-5, section 11(c), files a fenced observation under a RUNNING lane's current generation and operation and expects 0. Every transition but SWAPPED advances the generation, so SWAPPED is the one state in which a matching pair names a finished operation. The other option, invalidating the operation at finalization, would change what SWAPPED writes, which lane-handoff's finalizer reads back. An observation filed under no transition is still an unfenced set-lane-tree.

#115 — a sidecar is valid only when it is whole

what where
lane_sidecar_schema_ok: same name, same callers. A record of the current schema must also carry every key its writer writes, not empty: a snapshot's state one of the four words, every generation a number, a tree's dirty a number and unpushed a number or unknown, its path absolute, and its tree the id its file name gives it. Both writers have written exactly these keys since #97 landed (git log -L shows lane_state_put and lane_tree_put unchanged since b1015f4). Which kind a file is comes from where it is filed, and the snapshot's */lane-state.yaml is asked first: a case pattern's * matches /, so a tree's */trees/*.yaml would also match a snapshot under any directory named trees (review r1's MUST). No tree file is named lane-state.yaml, since a tree id is c<cksum>-… :13197, :13211
the writers' temporary file is asked the same question before the rename, so a half record is removed, never renamed into place :13289
lane_state_read → 10; lane-state says which of the two 10s it is :13348, :18026
lane-reconcile: STATE MALFORMED, verdict indeterminate :13907, :14333
lane-trees: a row named by its file, schema <malformed>, keeping the path the record names (which may be relative) and every other field empty; lane-reconcile reports it as malformed-sidecar (counted toward recovery); worktrees lists it as malformed sidecar :13680, :14035, :14524
every writer refuses with 1, as for an unknown schema: set-lane-state, set-lane-tree (the snapshot and the tree record), and the follow-up to a STARTED/RESUMED/ENDED/RETIRED line :18091, :18281, :18313, :13465
set-lane-tree --dirty/--unpushed that is not a count, or a --branch, --head, --upstream, --checkout or --writer that is only blanks: 64, before the lock, so no writer files a record its readers refuse :18233, :18237, :18243

Both exit tables in lanes-edit.sh are updated (the header's EXIT CODES: 7 and 10; the lifecycle arms' table: 1, 7, 10 and 64), as are docs/README-lanes.md (its main exit table's 10 and 64 too) and design.md decisions 26–28.

Red at 87a5e9d, green at head

tests/test_lifecycle_fail_open_113_114_115.py, 44 cases, in the lane-start sandbox of test_lane_start_claude_current.py. Failed reads are injected by a git on PATH that passes everything through except the one call a case names.

  • At 87a5e9d (a git archive of it plus this one file): the 37 cases written with the fix: 37 failed of 37, each at the assertion under test. That is 9 refusals that were writes, 7 verdicts that were clearances, 7 tree rows read as records, 6 snapshot reads that answered 0, 4 bad counts filed, 2 finished-operation polls filed, 1 holder none, and 1 follow-up that replaced the snapshot. For example, The lifecycle's remaining fail-open reads: an unreadable register becomes 'no holder', an unreadable sidecar becomes no tree, and two git reads become fabricated values #113 item 3's lane-tree-now printed main␟<sha>␟./main␟0␟unknown and exited 0.
  • The 7 cases from review r1 (a control root under trees, the five blank fields, the C-locale read) test what the review found in this PR, so their red is at 36520b9: 7 failed, 37 passed there, each at its own assertion. The blank fields exit 1 instead of 64, the C-locale read is indeterminate instead of resumable, and lane-start wrote no snapshot.
  • At head e6a41b4, after the merge of main: 534 passed across the new file, test_lanes_index.py, test_repo_hygiene.py, both test_triad_lane_tooling files and both test_lane_worktrees files (see "Merged with main").
  • At 3ffefce: 44 passed of 44, and together with test_repo_hygiene.py (189), test_deferred_97_findings_157.py (9) and the seven lane-worktrees: review findings deferred from #168/#169 (incl. five --yes data-loss paths found after the two-round cap) #170 G5 and lane-root cases of test_lane_worktrees.py: 249 passed.
  • At 36520b9: 37 passed of 37, and together with test_repo_hygiene.py (189 cases), the seven lane-worktrees: review findings deferred from #168/#169 (incl. five --yes data-loss paths found after the two-round cap) #170 G5 and lane-root cases of test_lane_worktrees.py, and the two scratch checks under CI below: 235 passed.
  • Mutants: 27 of 27 killed. The review round's three ran at 3ffefce. m25 (the tree arm asked first again) fails the trees case at "lane-start wrote no snapshot". m26 (no LC_ALL=C) fails the C-locale case with indeterminate instead of resumable. m27 (the blank check off) fails all five blank cases with 1 instead of 64. Before that, 24 of 24 were killed, each by an AssertionError in the cases that cover the mutated code. m01–m10 ran at 33515f8, and m02 and m09–m24 at d727834, so m02, m09 and m10 also ran after the merge of main. m01's code (lane_session_ids_read) went away in d727834.
  • Beside it:
    • at 33515f8, the new file with test_deferred_97_findings_157.py and test_repo_hygiene.py: 229 passed. At d727834, the same set gave 234 passed and 1 setup error: a TimeoutExpired in Findings deferred from #97: fetch-first reads, the live-swap takeover, a hand-written RESUMED, and the five of the 8c5958f round #157's fixture, where lane-start took over 120 s at load 134. That case rerun alone: 1 passed.
    • at 33515f8, test_lane_worktrees.py, test_lane_worktrees_report.py, test_lanes_index.py and test_supervised_restart_regressions.py: 381 passed, 2 failed. Both failures are test_completed_observer_cleanup_never_signals_saved_pid, which gives lane-handoff --supervise 20 s to reach state: ready. The host's load average went from 33 to above 200 during these runs. The same scenario timed at 87a5e9d took 21.6, 28.4 and 26.4 s, past the deadline. Run side by side, head and base timed the same: 41.4 and 40.5 s, 52.5 and 52.6, 45.9 and 43.1, 60.4 and 63.1 (head first). It is the host's load, not this change.
  • Shell suite: not run locally (CI is the suite of record). I read its repoRC section's assertions against the change. The refusal texts it checks are kept: records schema 999, schema this helper does not write, it could not be read, R22 and superseded reading. The suite's hand-written snapshot (section 11(b)) is whole. Its corrupt-HEAD worktree (11(f)) does not make git worktree list --porcelain fail: git 2.43 lists it and exits 0. I missed the lane-end gate's lane-worktrees: review findings deferred from #168/#169 (incl. five --yes data-loss paths found after the two-round cap) #170 G5 cases, which CI caught (below).
  • bash 3.2 and awk: bash -n lanes-edit.sh at e6a41b4, 3ffefce and 36520b9 exits 0 with empty stderr under bash 3.2.57, and so do lane-handoff, lane-start and lane-end at e6a41b4. The blank test (*[![:space:]]*) and the validator's arm order were also run under it. That is the bash:3.2 image's own binary, run through its musl loader because Docker could not start a container. Empty stderr matters here, because 3.2's -n exits 0 on an unterminated array. A harness of 16 records (whole, cut short, each shape, a wrong id, a backslash in an id, a dangling link) gives identical answers under mawk 1.3.4, one-true-awk 20231127 (macOS's awk, Debian's original-awk) and BusyBox awk under bash 3.2. The id goes to awk through ENVIRON, not awk -v.

CI

  • Run 39141918371 on 33515f8: failed, in the shell suite only. test_lane_helpers.sh gave 4626 passed and 3 failed, the same three in both the tests and tests-no-submodule jobs. All three are lane-worktrees: review findings deferred from #168/#169 (incl. five --yes data-loss paths found after the two-round cap) #170 G5's cases: a lane checkout whose .git is mode 000 (cg14), and one whose .git file points nowhere (cg15). The sweep still refused (2), and lane-end --dry-run still refused on it. But the reason was this PR's. The sweep asks lane-reconcile first and keeps the first refusal, and lane-reconcile now refuses on the failed registrations read. It said the worktrees git registers in <dir> could not be listed, where the suite pins the worktree registrations of <dir> could not be read. 36520b9 uses those words. Locally, a scratch check (not committed, since the shell suite's cg14 and cg15 cases already pin this) runs lane-worktrees sweep --dry-run over a mode-000 .git and over an injected git worktree list failure. Both fail at d727834's lanes-edit.sh on the missing phrase, with exit 2 kept, and pass at 36520b9.
  • Run 39146268828 on d727834: cancelled. 36520b9 superseded it before it finished.
  • Run 39290974968 on e6a41b4, after the merge of main: in progress when this was written.
  • Run 39163679712 on 3ffefce, before the merge of main: success (tests, tests-no-submodule and guard-launch-mode).
  • Run 39146711605 on 36520b9: success. tests (job 117731920768) and tests-no-submodule (job 117731920712) both passed.

Merged with main: #219's reader and #115's whole-record rule

Lane 4 pre-tested 3ffefce merged onto main 55aae16 (run 39272923228), and onto lane 3's queue (run 39272890984). Both failed the same two tests of #219 (a7f09bd) in tests/test_lanes_index.py, and nothing else, in every job of both runs (2 failed, 1995 passed in 39272923228's tests job, 2 failed, 2306 passed in 39272890984's).

Lane 3's constraints, and how they were kept

Review r1 and lane 3's answers

The independent Opus review r1 of 36520b9 came back FIX, with one MUST:

finding what was done
MUST 1: a snapshot under a directory named trees was validated as a tree record, so lane-start wrote none, set-lane-state exited 1 and lane-reconcile said no-state fixed in e95d072: the snapshot's arm first. Tested by test_115_a_control_root_under_a_directory_named_trees_keeps_its_snapshot, red at 36520b9; the swap-back mutant m25 is killed
SHOULD 2: the clash with #246's unreadable-row test lane 3: option (a), a follow-up on #246's reader. #252 unchanged
SHOULD 3: the manual's main exit table did not name the malformed snapshot under 10 fixed in 3ffefce (and its 64 row names the lifecycle arms' own)
SHOULD 4: the OpenSpec hunks (decisions 26–28, the tasks.md notes) in lane 3's change acknowledged by lane 3
SHOULD 5: #113 item 2 is closed by decision 26's remedy (a row counted toward recovery), not the issue's indeterminate lane 3 leans toward accepting decision 26, and will confirm before "Closes #113" takes effect
SHOULD 6: an unreadable .git under a lane root filed as #258
NIT 7: lane_no_repository matched git's English without LC_ALL=C fixed in e95d072; tested, mutant m26 killed
NIT 8: the manual said a malformed row has every field empty, and worktree_rows' note said a path always begins with / fixed in 3ffefce and e95d072: the row keeps the recorded path, which may be relative
NIT 9: set-lane-tree --branch ' ' exited 1 at the rename fixed in e95d072: 64 before the lock, for --branch, --head, --upstream, --checkout and --writer; tested, mutant m27 killed
NIT 10: decision 28 did not say why a record's lane: is not compared fixed in 3ffefce: rename-lane leaves it naming the old lane until the next write

Lane 3's answers were given at 19:5xZ as leans that become final when lane 3 resumes.

What is left, and why

🤖 Generated with Claude Code

Summary by Sourcery

Make lane lifecycle reconciliation fail closed, fence finalized operations, and require complete valid sidecars before reading or writing lane state.

Bug Fixes:

  • Preserve failed lifecycle and worktree reads as explicit failures instead of treating them as empty results or clearances.
  • Prevent lifecycle observations from being recorded after their operation has reached the SWAPPED state.
  • Reject incomplete or malformed current-schema lane sidecars and preserve them for diagnosis instead of replacing them.

Enhancements:

  • Validate sidecar completeness, field shapes, and tree identity across lifecycle and inventory readers and writers.
  • Distinguish malformed sidecars from unreadable and unknown-schema records in reconciliation output.
  • Reject invalid tree observations before locking or writing, and align repository detection with the worktree tooling's rules.
  • Document the new lifecycle failure semantics, fencing behavior, validation rules, and exit codes.

Documentation:

  • Update lane lifecycle documentation and design decisions for failed reads, finalized-operation fencing, and whole sidecar validation.

Tests:

  • Add comprehensive regression coverage for lifecycle fail-open reads, finalized-operation polls, malformed sidecars, invalid observations, locale handling, and unreadable worktree roots.

Brett Heap and others added 5 commits October 18, 2026 14:19
…t is whole (#115)

`lane_sidecar_schema_ok` asked one question, so a file whose first line is
`schema: 1` and that held nothing else passed it: the snapshot's reader printed
empty fields for it, the inventory's reader printed a row with an empty id and
empty counts (which `lane_reconcile` skipped, or read as `dirty 0, unpushed 0`),
and every writer renamed its own file over it.

The gate keeps its name and its call sites, and now also asks a record of the
current schema for every key its writer writes (not empty), the shapes the
readers decide with (`state` one of the four words, every `generation` a
number, a tree's `dirty` a number and `unpushed` a number or `unknown`, its
`path` absolute) and the id a tree record's file name gives it. Both writers
have written exactly these keys since #97 landed. The kind of record comes from
where it is filed; `LSS_WHY` says what failed.

A malformed record takes the unknown schema's path on both sides:
- every writer refuses it, exit 1, nothing written (`set-lane-state`,
  `set-lane-tree` for the snapshot and the tree record, and the lifecycle
  follow-up of a STARTED/RESUMED/ENDED/RETIRED line);
- `lane_state_read` answers 10, `lane-state` exits 10 and says which of the two
  10s it is, and `lane-reconcile` reads the state as `MALFORMED`, verdict
  `indeterminate`;
- `lane-trees` carries a tree record as a row named by its file, every field
  empty, schema `<malformed>`; `lane-reconcile` reports it `malformed-sidecar`,
  counted toward recovery; `worktrees` lists it as `malformed sidecar`.

`lane_sidecar_put` asks the same of its temporary file before the rename, so a
record cut short as it was written is removed rather than renamed into place,
and `set-lane-tree` refuses a `--dirty` or `--unpushed` that is not a count with
64, so no writer files a record its own readers refuse.

No lock line moves: the four `if ! lane_sidecar_schema_ok` lines, every
`acquire_lock`/`lock_try`/`release_lock` and the lock blocks are byte-identical;
only the refusal messages beside them change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e has finalized (#114)

`SWAPPING -> SWAPPED` keeps the generation and the operation by design, so the
fence's two comparisons accepted a poll from operation O after O's own
finalizer had run, and the delayed reading replaced the inventory the commit
point closed. Every other transition advances the generation, so `SWAPPED` is
the one state in which a matching pair names a finished operation: a fenced
`set-lane-tree` now reads the lane's state with the pair, under the same mutex,
and is refused with 7 where it is `SWAPPED`.

#114 also offered "require SWAPPING". That would refuse a write the suite makes
on purpose (repoRC-5, section 11(c): a lane RUNNING after a resume records under
its current generation and operation), so the narrower rule is the one taken.
An observation filed under no transition is still an unfenced `set-lane-tree`.
#114's second item, the partial observation, has been a 64 since 8.12.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed one is never an empty one (#113)

- The holder ids: `lane_session_ids_read` unions the published row's and this
  checkout's session ids and answers 1 where either copy could not be read (an
  empty published register, or a register file that is there and cannot be
  opened). That is `HOLDER unknown` and `indeterminate`, never an empty set that
  `live_holder` answers 8 for and the verdict table reads as a crash.
- `branch.<b>.remote` in `lane_tree_now` gets the `case` its `merge` sibling
  has: a read that failed is 3 (`lane-tree-now` 1, `set-lane-tree` refuses),
  never the fabricated local upstream `./<merge>`.
- `git worktree list --porcelain` in the lane's checkout keeps its status. A
  failure makes the verdict `indeterminate` and names it; a recorded directory
  that is NO repository registers nothing and stays an absence
  (`lane_no_repository`, `lane-worktrees`' `no_repository`, #170 G5).
- `lane-trees`' 10 for an inventory directory that cannot be listed, which this
  read still swallowed after 8.12 and #157 item 4e, and a lane worktree root
  that cannot be listed or searched, make the verdict `indeterminate` too.

#113's second item, the unreadable sidecar, has been its own row since 8.12.
Per-record failures stay rows counted toward recovery and leave the verdict as
it was, which is the line T018 (#246) draws for an unreadable leg.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…7a5e9d

tests/test_lifecycle_fail_open_113_114_115.py runs the real lane-start and
lanes-edit.sh in the sandbox of tests/test_lane_start_claude_current.py, as
tests/test_deferred_97_findings_157.py does. A read that failed is injected
by a `git` on PATH that passes every call through to the real one except the
one FAKE_GIT_FAIL names; the permission fixtures skip under root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… and tasks

docs/README-lanes.md: the verdict table's MALFORMED and failed-read rows, the
fence's SWAPPED refusal, the whole-record rule and the counts' shapes, the
remote read, `malformed-sidecar`, and the reads whose failure is
`indeterminate`. design.md: decisions 26-28. tasks.md section 7: where each
remaining item was taken.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 5 days and 9 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Oct 18, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR hardens lane lifecycle safety by making failed reads and malformed sidecars fail closed, preventing finalized-operation inventory writes, and adding comprehensive regression tests and documentation for the new validation, verdict, and exit-code semantics.

Sequence diagram for finalized-operation inventory fence

sequenceDiagram
    participant Observer
    participant SetLaneTree as set-lane-tree
    participant Mutex
    participant Snapshot as lane-state.yaml
    participant Inventory as tree sidecar
    Observer->>SetLaneTree: set-lane-tree --generation G --operation O
    SetLaneTree->>Mutex: acquire_lock
    SetLaneTree->>Snapshot: lane_sidecar_schema_ok
    Snapshot-->>SetLaneTree: state SWAPPED, generation G, operation O
    SetLaneTree->>SetLaneTree: compare fence
    SetLaneTree->>Mutex: release_lock
    SetLaneTree-->>Observer: refuse with exit 7
    SetLaneTree--xInventory: no write
Loading

Flow diagram for fail-closed lane reconciliation

flowchart TD
    A[lane_reconcile] --> B[lane_state_read]
    A --> C[lane_session_ids_read]
    A --> D[lane_trees_list]
    A --> E[git worktree list --porcelain]
    A --> F[lane root sweep]
    B --> G{Read succeeded and record is whole?}
    C --> H{Register read succeeded?}
    D --> I{Inventory read succeeded?}
    E --> J{Registration read succeeded?}
    F --> K{Root can be listed?}
    G -- No --> L[indeterminate]
    H -- No --> L
    I -- No --> L
    J -- No --> L
    K -- No --> L
    G -- Yes --> M[Continue lifecycle verdict]
    H -- Yes --> M
    I -- Yes --> M
    J -- Yes --> M
    K -- Yes --> M
Loading

Flow diagram for whole sidecar validation

flowchart TD
    A[lane_sidecar_put] --> B[write temporary sidecar]
    B --> C[lane_sidecar_schema_ok]
    C --> D{Current schema?}
    D -- No --> E[remove temporary file]
    D -- Yes --> F{All required keys and shapes valid?}
    F -- No --> E
    F -- Yes --> G{Tree id matches filename?}
    G -- No --> E
    G -- Yes --> H[mv temporary file into place]
    E --> I[writer refuses with exit 1]
    H --> J[whole sidecar becomes visible]
Loading

File-Level Changes

Change Details Files
Makes lifecycle reconciliation fail closed by preserving read failures instead of interpreting them as empty or healthy results.
  • Propagates holder-register, remote-config, inventory-listing, worktree-registration, and lane-root read failures.
  • Reports unreadable paths and promotes affected reconciliation verdicts to indeterminate.
  • Preserves the distinction between a missing repository and a repository whose worktrees cannot be inspected.
lanes-edit.sh
docs/README-lanes.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/design.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/tasks.md
Closes the set-lane-tree operation fence after an operation reaches SWAPPED.
  • Reads state, generation, and operation under the existing mutex.
  • Rejects matching fenced polls for finalized SWAPPED operations with exit 7 while retaining unfenced writes.
lanes-edit.sh
docs/README-lanes.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/design.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/tasks.md
Validates current-schema sidecars as complete, type-correct, and identity-consistent records before reading or replacing them.
  • Requires all writer-produced keys and validates state, generation, path, tree id, dirty, and unpushed values.
  • Treats malformed snapshots as exit 10/MALFORMED and malformed tree records as malformed-sidecar rows requiring recovery.
  • Validates temporary files before atomic rename and rejects invalid dirty or unpushed command-line values with exit 64.
lanes-edit.sh
docs/README-lanes.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/design.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/tasks.md
Adds focused regression coverage for all three findings and documents the resulting decisions and exit-code behavior.
  • Adds 37 tests covering injected Git failures, permission failures, finalized-operation polls, malformed snapshots and tree sidecars, and invalid counts.
  • Updates lifecycle documentation, design decisions 26–28, task status, and refusal/read-error messaging.
tests/test_lifecycle_fail_open_113_114_115.py
docs/README-lanes.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/design.md
openspec/changes/add-crash-consistent-lane-worktree-recovery/tasks.md

Assessment against linked issues

Issue Objective Addressed Explanation
#113 Do not treat failed holder-register reads as an empty session-ID set or pronounce a crash from an unanswered liveness question. ✅
#113 Preserve unreadable tree-sidecar and inventory failures in reconciliation, and make the verdict indeterminate when an inventory cannot be read. ✅
#113 Propagate failures from both Git reads—the branch remote configuration read and the worktree-registration sweep—instead of fabricating an upstream or treating failed registrations as absent. ✅

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • 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!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Brett Heap and others added 3 commits October 18, 2026 16:21
Main moved through lanes-edit.sh, lane, lane-handoff, lane-start and
tests/test_lane_helpers.sh since 87a5e9d; the merge is clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#67), not a copy of it (#113)

#67 landed on main with `session_ids_checked`: the published-union-local ids,
0 with them, 8 for none and 1 where either copy of the register could not be
read, on a `register_text_checked` that reports a failed `git show` or `cat`
itself rather than inferring it from an empty render. That is the read
#113 item 1 needs, so `lane_reconcile` asks it and `lane_session_ids_read`
goes: one implementation. An 8 goes on to `live_holder` as before; a 1 is
`HOLDER unknown` and `indeterminate`, as it was. The comments that named two
callers of the checked read now name three.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, so lane-end still relays #170 G5's reason (#113)

The sweep asks lane-reconcile before its own reads and keeps the first
refusal. With #113 item 4 that first refusal is now the reconciliation's,
so a lane checkout whose .git is mode 000, or whose .git file points
nowhere, refused (2) as before but named "the worktrees git registers in
<dir> could not be listed" where test_lane_helpers.sh's #170 G5 cases
(and lane-end's relay of them) require "the worktree registrations of
<dir> could not be read". CI run 39141918371 failed those three cases on
33515f8. The reconciliation now says it in lane-worktrees' words, with the
exit status after it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap

Copy link
Copy Markdown
Contributor Author

Lane openRepoTools-3, 2026-10-18T20:1xZ: the three decisions lane 4's review left to lane 3, final. They replace the leans lane 3 sent at 19:5xZ, unchanged.

  1. Make the lane tools look inside a triad's legs and paired feature trees, so the sweep, lane-end's gate, lane-handoff, lane-reconcile and the daily report can see a lane's work there, and change nothing a checkout with no project.yaml does #246 and Lifecycle reads that failed are no longer answers, set-lane-tree's fence closes with its operation, and a lane sidecar is valid only when whole #252's semantic clash: (a), in a lane-3 follow-up. Lifecycle reads that failed are no longer answers, set-lane-tree's fence closes with its operation, and a lane sidecar is valid only when whole #252 does not change for it.
  2. The OpenSpec hunks are acknowledged by the change's owner (proposal.md:31).
  3. Decision 26 is accepted as The lifecycle's remaining fail-open reads: an unreadable register becomes 'no holder', an unreadable sidecar becomes no tree, and two git reads become fabricated values #113 item 2's answer. "Closes The lifecycle's remaining fail-open reads: an unreadable register becomes 'no holder', an unreadable sidecar becomes no tree, and two git reads become fabricated values #113" may retire the item.

#252 still waits for #246, as lane 4 described. Lane 3 lands nothing on main until lane 4's HOLD RELEASED.

Lane: openRepoTools-3

Brett Heap and others added 3 commits October 18, 2026 20:13
…et-lane-tree refuses a blank field with 64 (#115, review r1 of #252)

lane_sidecar_schema_ok told a snapshot from a tree record by where it is
filed, and asked */trees/*.yaml first. A case pattern's * matches /, so
under a control root with any directory named trees above it every
snapshot was asked a tree's keys and refused: lane-start wrote none,
set-lane-state exited 1, and lane-reconcile said no-state, the verdict a
launcher goes past. The snapshot's arm now comes first; no tree file is
named lane-state.yaml, since a tree id is c<cksum>-...

set-lane-tree refuses a --branch, --head, --upstream, --checkout or
--writer that is only blanks with 64, before the lock, as it refuses a
count that is not one. It used to fail at the rename with exit 1.

lane_no_repository asks git in the C locale, as lane-worktrees' git_env
does, so a translated git cannot turn an absence into a failed read.

worktree_rows' note no longer says a row's path always begins with /: a
row of a record that is not whole keeps the path the record names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…C-locale read, each red at 36520b9 (#113, #115)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h of a malformed row, and why a record's lane: is not compared (#115)

The manual's main exit table names the malformed snapshot under 10 and
the lifecycle arms' 64s. A <malformed> lane-trees row keeps the path the
record names, which may be relative, and not every field empty. Decision
28 says which arm the validator asks first and why a record's lane: is
not compared: rename-lane leaves it naming the old lane until the next
write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brett Heap and others added 2 commits October 19, 2026 16:25
Main now holds #246 (T018), which lane 3's consent wanted #252 based on, and #219's lanes-index reader. Fetched by URL; the merge is clean.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecord with no path: is a malformed tree, never read and never the marker (#115, #219)

Merged with main, CI failed two tests of #219 (lanes-index's reader)
in tests/test_lanes_index.py. Both plant a current-schema sidecar with
no `path:` and assert its branch appears in `lanes-edit.sh worktrees`.
Since #115 such a record is not whole: the listing reads no field of it
and names its row `malformed sidecar`, by the path it records or, with
none, by its file. The rows were still trees, counted and never NOT READ,
so #219's reader and its marker key are unchanged; only the branch text
the tests looked for is deliberately no longer read.

- test_a_tree_with_no_path_is_a_tree_and_not_an_unread_marker: the
  path-less record is now asserted as a malformed row named by its file.
  Its second record keeps the path of the lane's whole tree (it lacks
  only `upstream:`), so two trees of one lane with one path stay two
  rows, the clause the source can no longer reach with an empty path.
- test_a_tree_whose_branch_reads_like_a_marker_is_a_tree: the forged
  branch is carried by a whole record (it gains a `path:`), so the
  branch is read and still makes no marker.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

brettheap pushed a commit that referenced this pull request Oct 19, 2026
Main moved from 55aae16 to 00d52ae with #252 (#113, #114, #115): lifecycle
reads that fail are no longer answers, set-lane-tree's fence closes with
its operation, and a lane sidecar is valid only when whole. The merge is
textually clean. #252 changes the lifecycle sidecars, lane_reconcile,
lane_tree_now, set-lane-tree and lane-state; this branch changes the
register's rewrites, retire-rows, the archive scan and add-row's
after-rebase take-back. Neither reads or writes the other's records, and
no refusal code or README row both touch now contradicts the other.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 20, 2026
… one, and a leg directory this account cannot search, or a dependency nested in one, is unreadable and never absent (fixes #255) (#271)

A follow-up to #246 (T018 of feature 004, the triad lane tooling) that must land **before the triad cutover (#186)**. It fixes the two sections of #255.

**The handoff names every inventory record that is not a whole schema-1 one, and a leg directory this account cannot search is unreadable, never `absent`.** Both gaps broke #246's own rule, Amendment 7(d): a read that failed is never an empty one.

These readers run only in an assembly, a checkout whose `project.yaml` declares a leg, so main is not exposed before the cutover.

**What it fixes**
- **Section 1, the handoff's tree inventory** (`lane-handoff`, `paired_inv` and `pi_bad`):
  - **Before:** a `lane-trees` row counted as unreadable only where its path was empty. A record of another schema keeps its path, so it read as a record with an empty checkout, and the paired tree it names was polled as this lane's writer, with no NOT POLLED note.
  - **Now:** the row's schema, field 13, decides. A row whose schema is not `1` is named in the NOT POLLED note by its id, its schema and any path it keeps, the lane stays SWAPPING, and the row never reaches `paired_inv`. The `lane-trees` row format is untouched.
- **Section 2, a leg directory with no search permission:**
  - **Before:** its `.git` could not be tested, and each reader's `.git` test failed there exactly as for a leg that is not checked out. The sweep listed it `absent` and exited 0. `lane-reconcile` gave no TREE row. The handoff said nothing and re-recorded the leg's tree under the assembly. `lane-end` ENDED the lane over the lane's unpushed commit in the leg.
  - **Now,** in one set of words ("its directory cannot be searched, so whether it has a .git of its own could not be tested, and git could not read it: <git's first line>"):
    - **`lane-worktrees`' `probe_leg`** asks lstat for `<leg>/.git`. Only ENOENT or ENOTDIR is a leg with no `.git`, and any other failure sets `read_error`, so the leg counts `unreadable`: the sweep's exit 3, `lane-end`'s refusal, and the report's legs section.
    - **`lane-handoff` and `lanes-edit.sh`'s `lane_reconcile`** take their unreadable-leg branch where the `.git` test fails and the leg directory fails `-x`. The handoff gets an UNREADABLE LEG line, a count and a withheld tree. The reconciliation gets a TREE row `unreadable` that requires recovery.
    - **A leg directory that is not there at all** is still `absent`.

**What the fold-in adds.** Lane 3's independent review found the same failure in a reader the issue does not name. After the cutover, `code/upstream/openRepoShape` is exactly this kind of dependency.
- **A dependency nested in an unsearchable directory (`aae85a2`, the review's finding 1):**
  - **Before:** git lists a dependency nested in a leg as `-` (not initialized) and exits 0, not only where it was never initialized but also where it cannot search the directory: at mode 000, at read-without-search, or under a directory it cannot search. `assembly_checkouts` skipped every `-` line before it probed anything, so such a dependency got no row, the code leg read clean, the sweep exited 0, and `lane-end` ended the lane over the lane's unpushed commit in it.
  - **Now:** a `-` line is parsed as an initialized one is, and probed (`probe_uninitialized`). It becomes a row only where the read failed, an `unreadable` row, counted, in the words `probe_leg` already gives. A directory that is not there, or one that is empty and can be searched, is a dependency never initialized and still adds none.
- **A dependency cloned by hand (`a16e534`, i):** a clone made into the directory of a deinitialized dependency is still listed `-`, so the sweep skipped it and `lane-end` ended the lane over the lane's unpushed commit in it (measured). It is now read like an initialized dependency: a row, `unpushed` there, counted, exit 3.
- **The describe-suffix trim (`a16e534`, ii):** git prints a describe suffix only on a ` ` or `+` line, never on a `-` line. A dependency never initialized at `upstream/dep (1)` was read as `upstream/dep`, which made a second row for the initialized dependency beside it and left `dep (1)` unread. The trim now applies to the initialized branch only.
- **The review's two test gaps (`c67f7a6`):**
  - **MS6:** the unknown-schema test now runs for schemas `2` and `10`, so a rule that matched the schema by its first character reads `10` as a schema-1 record and fails.
  - **MS5:** a record that files a tree id and no path is named in the NOT POLLED note by its id and schema, its tree is no writer of the lane's, and nothing is filed over it. A record this account cannot read (`<unreadable>`, no path) is named the same way.
- **The docs (`c67f7a6`, `a16e534`):** `docs/README-lanes.md` says what the sweep does with a dependency git lists as not initialized, and #255's own added text is reflowed where its insertion left ragged lines.

**The branch.** `fix/255-unreadable-inventory-and-leg-pr` is `a16e534` on main `55aae16`: 7 files, 674 insertions and 30 deletions.
- **Lane openRepoTools-2's two commits, cherry-picked:**
  - `13397aa` (from `ba13d21`): the two fixes, in `lane-handoff`, `lane-worktrees` and `lanes-edit.sh`, with four tests;
  - `e678c61` (from `523b381`): `docs/README-lanes.md`'s T018 section, `docs/triad-lane-tooling-analysis.md` section 7 item 11 naming opensoft/workBenches#166, and `lane-handoff`'s T018 section heading, now `3b`.
- **Lane 3's three fold-in commits:** `aae85a2`, `c67f7a6` and `a16e534`.
- **Why a cherry-pick:** #246 landed as the squash `55aae16`, so a merge of main into lane 2's branch conflicts in all seven files. The PR is the two commits cherry-picked onto main, with no force-push anywhere. Their diff equals `git diff 5dd0df0 523b381` (the same patch-id), and their tree at `e678c61` is `1a46c72`, the tree the reviewer computed.

**Tests.** There are 11 `test_255_` functions: lane 2's four, and the fold-in's seven. Under **Runs**, six of the seven fail under a mutant of the code they hold. The seventh, the unreadable record, needs no #252.
- **Lane 2's four,** each failing on `5dd0df0`'s tools and passing here:
  - `tests/test_triad_lane_tooling.py`: `test_255_a_leg_directory_that_cannot_be_searched_is_unreadable_and_holds_lane_end`;
  - `tests/test_triad_lane_tooling_handoff.py`: `test_255_a_leg_directory_that_cannot_be_searched_is_unreadable_in_the_handoff_and_reconcile`, `test_255_an_unknown_schema_record_is_named_in_the_handoffs_not_polled_note` and `test_255_a_malformed_record_is_named_in_the_handoffs_not_polled_note`. The `<malformed>` row comes from a fake `lanes-edit.sh`, so this test does not wait for #252.
- **The fold-in's seven:**
  - `tests/test_triad_lane_tooling.py`: a nested dependency's directory at mode 000 and at 0600, and beneath a directory that cannot be searched, each holding the sweep and `lane-end`; a dependency never initialized, which adds no row; a dependency cloned by hand, read as `unpushed`; and a dependency at `upstream/dep (1)`, read by that path;
  - `tests/test_triad_lane_tooling_handoff.py`: the tree id with no path, and the unreadable record.
- **Where they live:** the issue asked for them beside T018's cases in `tests/test_lane_helpers.sh`. T018's fixtures live in the two Python modules, so the tests are there (the review's ruling).

**The order with #252 (the review's NIT 5).** `<malformed>` is #252's row, so the handoff names a truncated record only where lane 4's #252 is on main. The review asked for #252 first, and that is what happened: #252 merged as `00d52ae` at 2026-10-19T18:14Z, and q6k is built on it. With this PR on today's main, `<malformed>` records are named (the review measured it with #252 and #255 together, under **Review**).

**Follow-ups, not in this PR:**
- a declared leg under an unsearchable parent (the review's ruling on lane 2's doubt: a follow-up);
- a leg at mode 0100, searchable and not listable, which reads `clean` (finding 4, estate-wide; lane 3 proposes it to Brett Heap for v1.2.0, #255 6272494888);
- the top block's contradiction for a paired tree that only a broken record names (finding 3; proposed for v1.2.0 in the same comment);
- the handoff's wording on #252's refusal code (NIT 8; lane 3 proposes it as a v1.2.0 item, #255 6272494888).

**Runs**
- **The record, q6k** (branch `lane3-queue/q6k-255-on-q5k`: q5k + this head `a16e534`, on main `677acc0`, tree `1149aeb`, with its conflict with #283 resolved as in `fcd3cce`): `tests.yml` run 39373762988, green. This is the run of record for the landing.
- **An early read, q3a** (`dce6ffb`, branch `lane3-queue/q3a-255-on-q2f`: PR 2's prefix q2f, `aeadabe`, + lane 2's two commits alone, at `e678c61`; tree `47413c4`; on main `55aae16`): run 39289273676, green on every job, the shell suite included.
- **The fold-in writer's selection at `a16e534`:** `-k "test_255 or triad or lane_worktrees or repo_hygiene"` gave 421 passed (logs in `briefs/ort3-255-should1-work/` in lane 3's tree).
- **The fold-in's mutants:** the writer ran seven and each fails a `test_255_` test.
  - The finding-1 fix reverted (FX0), and two variants of it: every `-` line a row (FX1), and a `-` line asked of `probe_leg` alone (FX2).
  - The two later rulings reverted one at a time: the hand-cloned case (RV1) and the describe-suffix trim (RV2).
  - The reviewer's MS5 and MS6.

**Review.** Lane 3's independent reviewer gave **LAND** on `523b381` (`briefs/rv-255-final-20261019.md` in lane 3's tree, posted on #255 as 6272494888). Both fixes do what sections 1 and 2 ask, in every reader named.
- **What it ran:**
  - the four new tests fail on `5dd0df0` for the right reason and pass on `523b381`;
  - 17 of 19 mutants are killed, and the two survivors, MS5 and MS6, are low-impact (the fold-in's tests now kill both);
  - with #252 (`3ffefce`) the fix merges cleanly, and against the real `lanes-edit.sh` the handoff names the `<malformed>` record, in 5 of 5 cases. On that tree, main + #252 + #255, the selection `-k 'test_255 or triad or lifecycle_fail_open or repo_hygiene or lane_worktrees'` gave 457 passed;
  - the shell suite on that tree gave 4889 passed and 2 failed, `test_lane_helpers.sh:1269` and `:8548`, both in `lane-start`'s row stamp. The reviewer takes them as its sandbox's 106-character path reaching the 240-character `ROW_STATE_CAP`, and neither #255 nor #252 touches `lane-start` or the cap. q3a is green on every job, the shell suite included.
- **What lane 3 did with the findings:**
  - finding 1, a SHOULD, and NITs 6 and 7: folded in, above;
  - finding 2, a SHOULD: the cherry-pick, above;
  - findings 3 and 4, and NIT 8: the follow-ups, above;
  - NIT 5: the order with #252, above.
- **The reviewer's re-check of `e678c61..a16e534` is LAND (#271 6277159862).**

**Landing:** Brett Heap's decision 4 on #228 ("Yes, merge when green (Recommended)"), and his word of 2026-10-18T20:3xZ, "use this as approval to merge when green for all merges thru v1.1.0".
- Lane openRepoTools-3 lands the queue in one order: #262, PR 2, PR 3, PR 4, T025's docs PR, then #255.
- This PR squash-merges after an independent review, once Linux CI is green on the exact tree main will have after its squash. The record is a queue run of `tests.yml` on lane 3's prefix branch, under **Runs**.
- #255 is last in the queue. q6k is q5k plus this head, so PRs 2 to 4 and T025 are beneath it.
- When its turn comes, lane 3 pushes a head that merges main into `a16e534` with q6k's tree, as a fast-forward, so the tree under review is the tree that was tested.

Lane: openRepoTools-3

Landed by lane openRepoTools-3 as queue prefix q6k (tree 1149aeb, tests.yml run 39373762988 green), on Brett Heap's word of 2026-10-18T20:37:07Z: "use this as approval to merge when green for all merges thru v1.1.0". Its conflict with #283 in tests/test_triad_lane_tooling_handoff.py is resolved as in the tested tree fcd3cce, which keeps both blocks. Independent re-check of the fold-in round e678c61..a16e534: LAND (#271 6277159862).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 20, 2026
…271) into act 2's slice 1a (#198)

One conflict, in lanes-edit.sh's lane_state_follow, at the check of a
snapshot this helper cannot read: this branch releases the workstation's
sidecar lock (sidecar_release); main released W's mutex where the follow
took it (lsf_own) and gave a new note naming $LSS_WHY. Resolved as the PR
says: keep sidecar_release, take main's note. No line of main's that merged
on its own uses lsf_own or release_lock inside lane_state_follow, and main's
lanes-edit.sh changes since 55aae16 add no lock call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 21, 2026
…ows: the inventory read says PARTIAL, lanes --worktrees prints it NOT WHOLE, and lanes-index writes nothing over it; an older helper's tree with no path stays a tree (#173 item c1, #252 delta review 2) (#289)

* Tests first: a lane listing that is not whole removes no indexed row, and an older helper's tree with no path and a marker's branch stays a tree (#173 item c1, #252 delta review 2)

Item c1 of #173 (Copilot's fourth review of #171): `worktree_rows` takes
`lanes_rows --all --closed` as whole, and that listing only notes on stderr
a register or an archive it could not read - and says nothing at all of an
object-log listing it could not read. A lane named by the failed read alone
is then a lane with no trees, and `lanes-index sync --source inventory`
removes its rows. These tests are written before the fix and fail at
21e0829 on the assertion under test:

- `test_a_lane_listing_that_is_not_whole_removes_no_row`, four cases: the
  archive, the register (the inventory's suspected sibling, real: repoC-1's
  only row is the register's), the object-log listing, and all three - which
  left the listing empty, so the helper said "none" and the sync removed
  every row. A published object is made unreadable by moving its loose
  object aside in the clone.
- `test_a_lane_whose_log_could_not_be_read_keeps_its_rows`: a published log
  git cannot read gives the listing no `dir`, the control root falls from
  rung 2 to rung 3, and the lane's rows are removed.
- `test_a_listing_whose_log_warnings_could_not_be_kept_is_not_whole` and
  `test_an_unlistable_log_directory_with_no_origin_is_a_partial_listing`:
  whether a log failed is not known without the warnings file, and under
  `LANES_NO_GIT=1` the logs are named by listing the local directory.

And the SHOULD of #252's delta review 2:
`test_an_older_helpers_tree_with_no_path_and_a_markers_branch_is_a_tree`
feeds the reader, through a wrapped `LANES_EDIT` standing in for a helper
older than #252, a TREE line with an empty path and a branch that reads
like the unread marker's. It passes at 21e0829, where a marker is its key,
and fails under mutant mC, 5bf29d9's rule (no path and the marker's branch).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* A lane listing that is not whole no longer removes indexed worktree rows: the inventory read says PARTIAL, lanes --worktrees prints it NOT WHOLE, and lanes-index writes nothing over it (#173 item c1)

`worktree_rows` read the lanes through `lanes_rows --all --closed`, whose
names come from the register's rows, the archive's rows and the object logs'
file names, and whose control roots come from each log's `dir`. A listing a
person reads may not refuse, so where one of those reads failed it noted it
on stderr - the log listing not even that - and listed the rest; the
inventory took that as whole and `lanes-index sync --source inventory`
removed the rows of every lane it could not name. With all three unread the
listing was empty and the read said 8, "none": every row was removed.

For the whole listing (never one named lane), `worktree_rows` now asks the
register (before the listing, so a read that succeeds is the cached text the
listing reads), the archive, the published log listing or, with no origin,
the log directory, and every published log (out of the warnings the listing's
own read of them left; where those could not be kept, whether one failed is
not known). Any failure is ONE `PARTIAL <workstation> <why>` line beside the
rows it did reach, and an empty listing over a failed read is that line
alone, never 8. `lanes --worktrees` prints it as the lane listing NOT WHOLE,
with a footer. `collect_inventory` refuses a read that carries it, so `sync`
and `reconcile --source inventory` write nothing, exit 1 and record why; the
table stays the last whole read, dated by its `synced` stamp.

Refused, not held: holding only the lanes it could not name needs a marker
for each, one for the listing (a first sync has no rows to hold) and a
one-lane `--index` read that knows of it, or the index again answers "no
worktree is inventoried" for a lane it never read. A new line kind, not a
lane-less `UNREAD`: an indexer older than this reads any four-field `UNREAD`
as a lane - it would hold a lane named "" and still remove the rest - and
refuses a line kind it does not know.

Lock lines (`acquire_lock`, `release_lock`, `lock_try`, `LOCK_HELD`)
unchanged; `lane-start` and `lane-handoff` untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Tests first: the lane listing reads the archive and the log list it checked, a log list that fails after the check removes no row, the register is asked before the listing, an r-- log directory with no origin is not whole, a dry run refuses a partial listing, and the index read says its last sync failed (#173 item c1, the review of #289)

The independent review of #289 at 03b6299 found the checks and the
listing's reads to be separate reads (SHOULD-1, probes P2 and P11), no test
pinning the register's check before the listing (SHOULD-2, mutant mR), an
`ls` that lists an r-- log directory (N1), no word of a refused sync in the
`--index` read (N2) and no test of a dry run's refusal (N4). Each is a test
here, and the code is still 03b6299's:

- the archive readable for the check and not after it: repoZ-1's row is
  removed at 03b6299;
- the list of the logs failing on the read after the check, and on every
  read after it: repoB-1, found through its log's `dir` alone, loses its row;
- the register's first read failing and every later one reading: refused,
  as it already is at 03b6299 - the test that kills mR;
- the no-origin log directory at mode 400 beside mode 000;
- `reconcile --dry-run` over each partial listing exits 1;
- `lanes --index --worktrees` after a refused sync names its last error.

A `git` shim on PATH for one call fails only the counted calls a case
names, which `_unreadable` cannot do. And the hung-indexer test waits for
`.argv` as its sibling does: the stub writes `.stdin` first, so `.argv` could
be read before it was there (the race the review traced at d2c26ae).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The lane listing reads the archive and the log list it checked, the events say a list of the logs they could not make, an r-- log directory with no origin is not whole, and the index read names its last sync's error (#173 item c1, the review of #289)

The review of #289 at 03b6299 (SHOULD-1): `worktree_rows` asked the
archive and the list of the logs, and the listing then read both again - the
archive four more times, the list twice (`known_lanes` for the names,
`remote_log_events` for the events every `dir` comes from). A read that
failed only after the check thinned a listing taken as whole: repoZ-1's row,
or every row found through a log's `dir`, was removed (probes P2 and P11).

- (b) The check's reads are the listing's. Where the archive and the list
  of the logs read, `worktree_rows` pins their text (`PIN_ARCHIVE`,
  `PIN_LOGS`) for the listing alone and drops it after: `archive_text` and
  `known_lanes` hand it back instead of reading again. Not a cache kept for
  the process: `add-row` rescans the archive after its rebase, and must read
  it as it then stands.
- (a) `remote_log_events` says a list of the logs it could not make
  (`unlisted logs: ...`) instead of reading as no logs, and `worktree_rows`
  reads that line back out of the events' warnings, as it reads "unreadable
  log": the events are built once per process, so it speaks for the read
  every `dir` came from. The reason is the check's own, said once.
- (c) docs/README-lanes.md says each read is made once for the listing,
  and what is still read twice: with no origin, the log directory and its
  files; and the events' own question whether origin/<branch> is there.
- N1: with no origin, the log directory is asked with `-d`, `-r` and `-x`,
  as the trees' directory is: `ls` listed an r-- one whose files no glob
  could then open.
- N2: `lanes-index export --worktrees` writes this workstation's last sync
  error as a fifth field of a source's meta line, and `lanes --index
  --worktrees` prints it after `synced <UTC>` as `(its last sync failed:
  <UTC> <why>)`. An older reader ignores the field.

The tests are ca6b1b0's. No changed line names a lock function or
`LOCK_HELD`, and `lane-start` and `lane-handoff` are untouched.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The two tests of a lane found through its log's dir share one setup, which SonarCloud's quality gate counted as 28 duplicated lines of new code (#289)

`test_a_log_list_that_fails_after_the_check_removes_no_row` took the
14-line setup of `test_a_lane_whose_log_could_not_be_read_keeps_its_rows`
whole: repoB-1 resumed in a checkout of its own, its tree under that
checkout's parent, and a sync with no override naming a control root.
SonarCloud's gate on #289 at cef454e failed on it (4.9% duplication on new
code, 3% allowed). The setup is now `_a_lane_found_through_its_log`, which
both tests call; neither test's assertions change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* lanes-edit.sh's two descriptions of the --index worktrees read line name the last sync's error that the line now carries (the review of #289, N2)

The file's header and the `worktrees` dispatch both quote the stderr line
`read: index (<store>) at inventory:<ws> synced <UTC>`. Since d26658d it is
followed by `(its last sync failed: <UTC> <why>)` while this workstation's
last sync of that source wrote nothing, as docs/README-lanes.md says. Comments
only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Tests first: a lane's log that fails after the listing read it removes no row, the lane whose log sorts last is named, and the index read labels what it shows the last error (review r2 of #289, SHOULD-A, NIT-2 and NIT-1)

The delta review r2 of #289 at ddf7f6e found a lane's `dir` read again
for each lane with trees (SHOULD-A, probe C1). `lane_trees_list` and
`lane_state_read` work the control root out through `lane_control_root`
with no `dir`, which reads the lane's log a second and a third time, and a
`git show` that fails there is thrown away. The review also found no test
with a log-only lane that sorts last (NIT-2, mutant mNoSentinel), and an
`--index` read line saying "its last sync failed" where a refused
`reconcile --dry-run` records the error too (NIT-1). Each is a test here,
and the code is still ddf7f6e's:

- repoB-1 is found through its log's `dir` alone, with a lifecycle
  snapshot there, and synced. A `git` shim then fails only the second
  `show` of its log. At ddf7f6e that is `lane_trees_list`'s read: the root
  falls to `$PROJECTS_ROOT/.lane-state` and the sync removes the row.
- repoY-1, a lane with a log alone that sorts after repoM-2.md, is named
  by the listing and indexed. This passes at ddf7f6e, and it is the test
  that kills mNoSentinel.
- The N2 test looks for `(last error: <UTC> <why>)`, the name `status`
  gives what is recorded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* The lane listing's `dir` is the root of each lane's trees and snapshot, so a later read of the lane's log that fails removes no row, and the `--index` read calls what it shows the last error (review r2 of #289, SHOULD-A, NIT-1 and NIT-3)

Review r2 of #289 found the control root worked out three times for each
lane with trees (SHOULD-A, probe C1). `worktree_rows` hands
`lane_control_root` the listing's own `dir`, but `lane_trees_list` and
`lane_state_read` called it with the lane alone, and it read the lane's log
again for each. `lane_log_events` throws a failed `git show` away, so the
root fell to `$PROJECTS_ROOT/.lane-state` and `lane_trees_list` answered 8.
The lane read as one with no trees: `worktrees --all` said nothing, and
`lanes-index sync` removed its rows.

Both now take an optional second argument, `lane_control_root`'s own
payload. `worktree_rows` passes all three the one payload it builds from
the listing's `dir`. Every other caller passes the lane alone, and
`"${2-}"` hands `lane_control_root` the empty payload it had before. Those
callers are `lane_state_of`, `lane_reconcile` and the `lane-state` and
`lane-trees` verbs, and through them `lane-start`, `lane-handoff`,
`lane-end` and the lifecycle verbs.

With no `LANES_LANE_STATE_ROOT`, a whole `worktrees --all` now shows a
lane's log once instead of three times, and lists the logs twice instead
of four times (review r2's probe B0).

NIT-1: the `--index` read's fifth field is `status`'s `last error`, which
a refused `reconcile --dry-run` records too, so it is now labelled
`(last error: <UTC> <why>)`. What a dry run records is unchanged.

NIT-3: docs/README-lanes.md said each read is made once. It now says the
list of the logs is made twice, and that a lane's `dir` is read once. The
header and dispatch comments, `lanes-index`'s export comment and the
README's `--index` paragraph say the same.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brett Heap <service@opensoft.dev>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant