[brettheap]

Closes #173

This PR does #173's last open item, c1 (comment 6013909513, from Copilot's fourth review of #171), and adds the test that #252's delta review 2 asked for under #173's unread marker. The rest of #173 is already on main, from #219 (a7f09bd): the issue's own finding, by option 1 (the per-lane unread marker, so a first sync's unread lane and a held lane's kept rows both say NOT READ in the index), and items c2, c3 and c4. That leaves c1, so this PR closes the issue. It is lane openRepoTools-4's, under Brett Heap's 10-11 assignment of #90–#194, and it merges only on his word for this PR.

Fix round 1 answers the independent review at 03b6299 (LAND, no MUST). Its SHOULD-1 and SHOULD-2 and its N1 to N4 are done here, and the hung-indexer race it traced is fixed; its SHOULD-3 and SHOULD-4 are #290 and #291. See "Fix round 1" below.

Fix round 2 answers the delta review r2 of fix round 1, at ddf7f6e (LAND, no MUST). Its SHOULD-A, a lane's dir read again for each lane with trees, and its NIT-1 to NIT-3 are done here. Its SHOULD-B, a bash 3.2 exit in lanes --worktrees that main has too, is #292. See "Fix round 2" below.

The defect

worktree_rows lists the lanes through lanes_rows --all --closed and takes that listing as whole.

• The listing's lanes are the names three reads give: the register's rows, the archive's rows and the object logs' file names.
• Each lane's trees are found under the control root that its log's dir places.

lanes_rows is a listing a person reads before a launch, so it may not refuse. When one of those reads fails, it says so on stderr and lists what the other reads gave. For the object-log listing it does not even say so, because known_lanes answers 0 either way.

The inventory took that partial listing as whole. lanes-index sync --source inventory then removed the rows of every lane the listing could not name.

Each row of the table below was reproduced at 21e0829 against the test fixture's seed. In each case, a lane with a tree is named by one read alone, and that read's published object is made unreadable:

read that failed	lane that read alone names	what the helper said	what the sync did at 21e0829
the archive (c1)	repoZ-1, whose only row is the archive's	a note on stderr; exit 0 without the lane	removed its row
the register (the inventory's suspected sibling, which is real)	repoC-1, a row with no log	a note on stderr	removed its row
the object-log listing (found here)	repoB-1, a log with no row	nothing	removed its row
all three	every lane	exit 8, "none"	0 upserted, 8 removed: the table emptied
one published log (found here)	repoB-1, whose dir only its log records	only the log's own warning	the control root fell from rung 2 to rung 3, and the row was removed

The design

• The read says what it could not read.
  • For the whole listing only, worktree_rows now asks each read the listing rests on:
    • the register, the archive and the list of the object logs, before the listing is made. Where one of them reads, the listing reads that very text: the register out of its cache, the archive and the log list pinned for that one listing. So a read of them that fails after the check cannot thin the listing (fix round 1). The events list the logs once more, and that list is asked as the last item below says (worded in fix round 2);
    • with no origin, the log directory, which must be a directory this account can list and search;
    • every published log, and the events' own list of the logs, out of which every lane's dir comes. Both are taken from the warnings their read left, and the events are read once per process, so this is the read the listing used. Where those warnings could not be kept, whether a log failed is not known, and the read says so. Each lane's dir is read there once: its control root, its trees and its lifecycle snapshot are all found under that dir, and its log is not read again for them (fix round 2).
  • If any of them failed, one more line is printed beside the rows it did reach: PARTIAL, the workstation, and why.
  • An empty listing over a failed read is that line alone, and never exit 8.
  • A read of one NAMED lane goes by its name, which no listing gives, and never prints PARTIAL.
• lanes --worktrees prints it. The line shows (every lane) in the LANE column and lane listing NOT WHOLE: followed by the reason. A footer says that a lane missing from such a listing is NOT a lane with no worktrees.
• The indexer writes nothing over it.
  • collect_inventory refuses a read that carries a PARTIAL line.
  • sync and reconcile --source inventory, --dry-run included, exit 1 with unread: the lane listing ... was not whole - (why) - ... - nothing written, and status keeps that as the last error.
  • The table stays the last whole read, dated by its synced stamp. The first sync over a whole listing writes what moved in the meantime.
• The --index read says so too (fix round 1, worded in fix round 2). Where this workstation recorded an error for a source, lanes --index --worktrees prints (last error: <UTC> <why>) after that source's synced <UTC>. It is the last error status shows: a sync or a reconcile of the source that refused or failed records it, a --dry-run included, and a sync or a reconcile that is not a dry run clears it when it succeeds. lanes-index export --worktrees carries the error as a fifth field of the source's meta line, and an older reader ignores that field. Another workstation's error is in its own status file, so a shared store's other sources never show one.

Where this departs from the design it started from, and why

It refuses rather than holds. The design I was given, and the announcement on #173, held every stored lane missing from the read and wrote the rest. #219's marker exists so that a held lane does not read as current, so holding by listing would have needed four more pieces:

• a marker for every held lane;
• a marker for the listing itself. A first sync over a partial listing has no rows to hold, so without it the index would again answer "no worktree is inventoried" for a lane the listing could not name, which is lanes-index inventory follow-ups from Copilot rounds 3 and 4 on #171: unread-lane answers, partial listings, foreign-source marks, SQLite read errors, footer remedy #173's own defect;
• a status line for that listing marker;
• a one-lane --index fallback that knows about it, across every workstation in a shared store.

Refusing follows rules the code already has:

• collect_inventory already refuses a listing it could not read at all (a helper exit other than 0 or 8);
• next-free refuses a position over this same partial read.

A read that is not whole is the same failure, in part. The register, the archive and the logs belong to the workstation's one register checkout, so a failed read of them is never just one lane's problem.

It is a new line kind, not a lane-less UNREAD. An indexer older than this PR reads any four-field UNREAD as a lane. It would hold a lane named "", give it a marker, and still remove every other lane missing from the read. The same older indexer refuses a line kind it does not know ("a line this indexer does not read ... is the helper on PATH another version?"), so it writes nothing over a newer helper's partial read.

Fix round 1

The independent review at 03b6299 found no MUST. This round is two commits on 03b6299: ca6b1b0, the tests alone on 03b6299's code, and d26658d, the fix. Then cef454e merges main a42946e. After CI's first run, two more:

• ecfc523 moves a 14-line test setup that the two log tests shared into one helper, _a_lane_found_through_its_log. SonarCloud's gate had counted it as duplicated new code: 4.9% at cef454e, where 3% is allowed, and 0 at ddf7f6e. Neither test's assertions change.
• ddf7f6e updates two comments in lanes-edit.sh that quote the --index read line, for N2.

finding	what this round does
SHOULD-1: the archive and log-list checks were reads separate from the listing's own (probes P2 and P11)	(b) the archive and the log list are pinned for the listing (PIN_ARCHIVE, PIN_LOGS), so archive_text and known_lanes hand back the text the check read instead of reading again. (a) remote_log_events notes a list of the logs it could not make (unlisted logs: ...), and worktree_rows reads that note back, so a failure of the read every dir comes from is never silent. That did not make each dir the one used: a lane with trees still had its log read again, for its trees and its snapshot, until fix round 2 (review r2's SHOULD-A). (c) docs/README-lanes.md says what is still read twice.
SHOULD-2: no test pinned the register's check before the listing (mutant mR)	a git shim fails only the first show of the register, and mR is now killed
N1: with no origin, ls listed an r-- log directory (probe P12)	the directory is asked with -d, -r and -x, as the trees' directory is
N2: the --index read said nothing of a refused sync (probe P9)	its line names this workstation's last sync error
N3: "No exit code changes" was wrong	corrected under "Docs"
N4: no test of reconcile --dry-run over a partial listing	the four test_a_lane_listing_that_is_not_whole_removes_no_row cases run it: exit 1, the reason, and no removals
the hung-indexer race at tests/test_lanes_index.py:631	it waits for .argv (:633), as its sibling at :2155 does, both lines at ddf7f6e

Why a pin and not a cache for the process. The review suggested caching the archive and the log list per process, as the register is cached. A cache for the process must be dropped wherever the ref moves, and the register's is dropped in three places: a fetch, an --index read and a write's re-read. The archive has one more reader that needs it fresh. add-row rescans the archive after its rebase (retired_identity_hits) to catch a position retired in the meantime, and a stale cache there would offer that position again. The pin is set by worktree_rows from its own check and dropped as soon as the listing is made, so no other read sees it.

What is still read twice (now also in the docs):

• The list of the logs: by the check, whose text the listing's names come from, and by the events, which say so where their list fails (added in fix round 2, review r2's NIT-3).
• With no origin, the log directory is asked, then listed again for the names and again for the events. A log file in it that will not open leaves no warning naming it (A named worktree read answers "none" for a lane whose log or register could not be read (from #289's review) #290, item 4).
• The events ask for themselves whether origin/<branch> is there. A rev-parse that failed in that read alone would read this checkout's logs in place of origin's.

Fix round 2

The delta review r2 of fix round 1, at ddf7f6e, found no MUST. This round is two commits on ddf7f6e: d223430, the tests alone on ddf7f6e's code, and f2723b1, the fix. Main is still a42946e, which cef454e already merged.

finding	what this round does
SHOULD-A: a lane's dir was read again for each lane with trees, and a failure of that read removed the lane's rows (probe C1; also at 21e0829)	lane_trees_list and lane_state_read take an optional second argument, lane_control_root's own payload, and worktree_rows hands all three reads the one payload it builds from the listing's dir
NIT-1: (its last sync failed: ...) also showed the error of a refused reconcile --dry-run	the label is (last error: ...), the name status gives it, in the code, the README and this body. What a dry run records is unchanged
NIT-2: no test had a log-only lane that sorts last, so mutant mNoSentinel survived	a test whose repoY-1, a lane with a log alone that sorts after repoM-2.md, must be named and indexed
NIT-3: "made ONCE" (README) and "never made" (this body) overstated	both now say that the list of the logs is made twice
SHOULD-B: lanes --worktrees exits with no output under bash 3.2, on main too	not here: #292

Every other caller keeps its behaviour. lane_state_of, lane_reconcile, and the lane-state and lane-trees verbs pass the lane alone. lane-start, lane-handoff, lane-end and the lifecycle verbs reach these functions only through those verbs. For each of them "${2-}" is the empty payload lane_control_root was handed before, so the root is found exactly as it was. No other file calls either function.

Read counts. A probe estate with no LANES_LANE_STATE_ROOT, whose log-only repoB-1 is found through its dir and has a snapshot there, counted the git calls of one whole worktrees --all:

read	at ddf7f6e	at f2723b1
ls-tree of lanes/log/	4	2
show of repoB-1's log	3	1
show of any log (the estate has two)	4	2
show of the register	1	1
show of the archive	1	1

With LANES_LANE_STATE_ROOT set, the counts and the rows are the same at both heads. With only the second show of repoB-1's log failing:

• at ddf7f6e, worktrees --all exits 8 with no PARTIAL;
• at f2723b1, it exits 0 with repoB-1's tree, RUNNING at generation 1.

Through lanes-index sync, ddf7f6e shows that log three times and writes 0 upserted, 1 removed. The three are the events' read, lane_trees_list's read, which fails, and the indexer's own lane-state read of the lane's generation. f2723b1 shows it once and writes nothing. Bash 3.2.57 gives the same counts and rows.

Tests

The tests went in first, in their own commit d2c26ae, whose code is 21e0829's.

test	at d2c26ae (21e0829's code)	at 03b6299
test_a_lane_listing_that_is_not_whole_removes_no_row[archive]	FAILS on the kept-row assertion: 1 upserted, 1 removed, repoZ-1's row gone	passes
test_a_lane_listing_that_is_not_whole_removes_no_row[register]	FAILS: 4 upserted, 1 removed, repoC-1's row gone	passes
test_a_lane_listing_that_is_not_whole_removes_no_row[log-listing]	FAILS: 1 upserted, 1 removed, repoB-1's row gone	passes
test_a_lane_listing_that_is_not_whole_removes_no_row[all-three]	FAILS: 0 upserted, 8 removed, the rows are {}	passes
test_a_lane_whose_log_could_not_be_read_keeps_its_rows	FAILS: 0 upserted, 1 removed	passes
test_a_listing_whose_log_warnings_could_not_be_kept_is_not_whole	FAILS: no PARTIAL line ([] == [['PARTIAL', ...]])	passes
test_an_unlistable_log_directory_with_no_origin_is_a_partial_listing	FAILS: 0 upserted, 1 removed	passes
test_an_older_helpers_tree_with_no_path_and_a_markers_branch_is_a_tree	passes: at 21e0829 a marker is its key	passes; FAILS under mC

Counts:

• Red, tests/run.sh -k test_lanes_index at d2c26ae: 8 failed, 82 passed. Seven of the failures are the c1 cases above, each on its assertion and none on a crash. The eighth, test_a_hung_indexer_delays_no_writer_beyond_the_fork, is an existing race in that test's stub, fixed in fix round 1 (see "Fix round 1"). It passed in the green run, and also under mC and mC-whole, where the whole module ran.
• Green, tests/run.sh -k "test_lanes_index or test_repo_hygiene" at 03b6299: 325 passed.
• CI, workflow_dispatch run 39342747260 on 03b6299: tests, tests-no-submodule and guard-launch-mode all passed.

How a case makes a read fail. It moves the object's loose file aside in the clone, so a git show or git ls-tree that needs it fails, as on a damaged clone or a partial clone that cannot fetch it. Only the read under test fails.

What each c1 case checks. Besides keeping every row, each case checks that:

• the whole store is byte-for-byte unchanged, and a change to a listed lane made in the meantime is not written either;
• the refusal names the exact reason, and status and reconcile say the same;
• lanes --worktrees prints the NOT WHOLE line and its footer;
• a named lane's read is not partial;
• the first sync over a whole listing writes the change, after which the --index read equals the source read again.

Fix round 1's tests

The tests went in first, in ca6b1b0, whose code is 03b6299's.

test	at ca6b1b0 (03b6299's code)	at d26658d
test_the_listing_names_lanes_from_the_archive_it_checked (P2: the shim fails every archive show after the first)	FAILS on the kept-row assertion: 0 upserted, 1 removed, repoZ-1's row gone	passes: the listing read the archive once
test_a_log_list_that_fails_after_the_check_removes_no_row[the-next-read] (the shim fails the second ls-tree of lanes/log/)	FAILS: 0 upserted, 1 removed. That second list was known_lanes', so repoB-1 went unnamed	passes: the second list is now the events', which says so, and the sync refuses
test_a_log_list_that_fails_after_the_check_removes_no_row[every-later-read] (the shim fails every ls-tree of lanes/log/ after the first: at 03b6299, P11's read and the one before it)	FAILS: 0 upserted, 1 removed	passes, the same way
test_the_register_is_asked_before_the_listing (P3: the shim fails only the first register show)	passes: the check is that first read	passes; FAILS under mR
test_an_unlistable_log_directory_with_no_origin_is_a_partial_listing[mode-400] (P12)	FAILS: 0 upserted, 1 removed	passes
test_the_index_read_says_its_last_sync_failed (N2)	FAILS: the line is only synced <UTC>	passes
the four test_a_lane_listing_that_is_not_whole_removes_no_row cases, now with N4's dry run, and [mode-000]	pass	pass

In the two log-list cases no override names a control root, so repoB-1's trees are found through its log's dir alone, as in P11.

Counts:

• Red, tests/run.sh -k test_lanes_index at ca6b1b0: 5 failed, 91 passed. The five are the FAILS above, each on its kept-row or read-line assertion and none on a crash.
• Green, tests/run.sh -k "test_lanes_index or test_repo_hygiene or test_register_scan_hardening_138" at d26658d: 358 passed. Again at ddf7f6e, after the refactor, -k "test_lanes_index or test_repo_hygiene": 333 passed.
• CI: the pull_request run 39467365169 on ddf7f6e passed tests, tests-no-submodule, package and guard-launch-mode, the two test jobs on the merge of ddf7f6e into main a42946e, and SonarCloud's quality gate passed.

How a round-1 case makes a read fail. For that one call, a git shim on PATH counts the calls whose arguments carry the read's text. It fails, with exit 128, only the counted calls the case names, and every other call is the real git. _unreadable cannot do this, because it fails every read of the object. A probe at each head counted the calls a sync makes: the archive was shown 5 times at 03b6299 and once at d26658d, and the log list was listed 3 times and then twice. Under bash 3.2.57 (the musl build) the fix answered both probes as it does under 5.2.

Fix round 2's tests

The tests went in first, in d223430, whose code is ddf7f6e's.

test	at d223430 (ddf7f6e's code)	at f2723b1
test_a_lanes_log_that_fails_after_the_listing_removes_no_row (C1: repoB-1 found through its log's dir alone, with a snapshot there; the shim fails only the second show of its log)	FAILS on the kept-row assertion: 0 upserted, 1 removed, repoB-1's row gone	passes: the listing's show is the only one, and the sync writes nothing
test_the_listing_names_the_lane_whose_log_sorts_last (NIT-2: repoY-1 has a log alone, and it sorts last)	passes	passes; FAILS under mNoSentinel
test_the_index_read_says_its_last_sync_failed (N2, now looking for (last error: ...))	FAILS: the line says (its last sync failed: ...)	passes

Besides the kept row, the C1 test checks that the store is byte-for-byte unchanged and that no lane is held. A snapshot read under the wrong root reads as generation 0, and a sync holds the lane's rows for that rather than write them. With no snapshot, both roots give the same answer, so the test gives repoB-1 one.

Counts:

• Red, tests/run.sh -k "test_lanes_index and (fails_after_the_listing or sorts_last or last_sync_failed)" at d223430: 2 failed, 1 passed. Each fails on the assertion above, and neither on a crash.
• Green, tests/run.sh -k "test_lanes_index or test_repo_hygiene" at f2723b1: 335 passed, which is round 1's 333 and the two new tests.
• Bash 3.2.57 (the musl build first on PATH), -k "test_lanes_index and (fails_after_the_listing or sorts_last or last_sync_failed or log_list_that_fails or log_could_not or not_whole)" at f2723b1: 7 passed, 4 failed. The four are the not_whole c1 cases: their lanes --worktrees exits 1 with no output at Under macOS's bash 3.2, a failing command <utility> in a list or an if condition exits a set -e script: lanes --worktrees dies silently (lanes:317), and wip's .gitignore step can stop (openRepoTools:4014) #292's lanes:321, as the review found at ddf7f6e.
• CI: the pull_request run 39507250560 on f2723b1 passed tests, tests-no-submodule, package and guard-launch-mode, the two test jobs on c594a3a, a merge of f2723b1 into main a42946e whose tree 6e6f2ae is the merge ref's, and SonarCloud's quality gate passed.

The mC test

test_an_older_helpers_tree_with_no_path_and_a_markers_branch_is_a_tree uses a wrapped LANES_EDIT as a stand-in for a helper older than #252. The wrapper adds one TREE line with an empty path and a branch that reads like the marker's. The test checks that the line:

• is exported back as its TREE line, so the --index read is the source read;
• is counted by status as a worktree;
• is still one of its lane's kept rows when the lane is held.

It passes at 21e0829, where a marker is its key.

The mutant comes in two forms:

• mC as stated: 5bf29d9's rule alone. It also fails the existing test_a_first_sync_that_cannot_read_a_lane_says_not_read_in_the_index, but only because status has fetched just the lane column since 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, so no rule based on the branch can see a marker there.
• mC-whole: 5bf29d9's rule together with the status fetch it had then. This new test is the only test in the module that kills it.

Mutants

Each mutant was applied alone to 03b6299. mC and mC-whole were run over the whole module (tests/run.sh -k test_lanes_index). The others were run over the new tests (-k "test_lanes_index and (not_whole or log_could_not or warnings_could_not or unlistable_log or older_helpers)"). Every mutant was killed.

mutant	what it changes	killed by
mC	is_unread_marker takes 5bf29d9's rule (no path, and a branch beginning unreadable inventory () in place of the key	the mC test, where the --index read is no longer the source read; and the existing test_a_first_sync_that_cannot_read_a_lane_says_not_read_in_the_index, because status has fetched only lane since #219. Whole module: 2 failed, 88 passed
mC-whole	mC, plus cmd_status fetching path and branch as it did at 5bf29d9	the mC test alone, on that same assertion. Whole module: 1 failed, 89 passed
m1	worktree_rows does not ask the archive	[archive] (repoZ-1's row removed); [all-three] (the reason lacks the archive)
m2	it does not ask the register	[register] (repoC-1's row removed); [all-three]
m3a	it does not ask the published object-log listing	[log-listing] (repoB-1's row removed); [all-three]
m3b	with no origin, it does not ask the log directory	test_an_unlistable_log_directory_with_no_origin_is_a_partial_listing (row removed)
m4	a published log git could not read does not make the read partial	test_a_lane_whose_log_could_not_be_read_keeps_its_rows (row removed)
m5	warnings that could not be kept are read as no warnings	test_a_listing_whose_log_warnings_could_not_be_kept_is_not_whole (no PARTIAL line)
m6	a NAMED lane's read asks the listing's reads too	the four test_a_lane_listing_that_is_not_whole_removes_no_row cases (NOT WHOLE in a named read)
m7	collect_inventory reads past a PARTIAL line	the four cases, the log test and the no-origin test: 6, each a removed row
m8	the line is a lane-less UNREAD, still refused by the indexer	the four cases (lanes --worktrees prints NOT READ, not NOT WHOLE) and the warnings test (the line's kind): 5
m9	an empty listing over a failed read is exit 8, "none", again	[all-three] (every row removed)
m10	lanes prints no line for PARTIAL, as an older renderer would	the four cases
m10b	lanes prints no footer for it	the four cases
m11	the log warnings are asked even where the listing found no lane	[all-three] (its reason gains "not known")

Fix round 1's mutants were each applied alone to cef454e (the fix, merged with main) and run over -k "test_lanes_index and (names_lanes_from_the_archive or log_list_that_fails or register_is_asked or last_sync_failed or unlistable_log or not_whole or log_could_not or warnings_could_not)", 13 tests. Every one was killed, each by a removed row or the read's line:

mutant	what it changes	killed by
r1-a-note	remote_log_events does not note a list of the logs it could not make	both log-list cases (repoB-1's row removed)
r1-a-scan	the note is made, and worktree_rows does not read it back	both log-list cases
r1-b-archive-pin	archive_text ignores the pin, so the listing reads the archive again	the archive test (repoZ-1's row removed)
r1-b-logs-pin	known_lanes ignores the pin, so the names come from a second ls-tree	[the-next-read] (repoB-1's row removed)
mR (the review's)	the register is asked AFTER the listing, with the reasons kept in order	the register test (repoC-1's row removed)
r1-N1-ls	with no origin, the log directory is asked with ls again	[mode-400] (repoB-1's row removed)
r1-N2-render	the --index read's line drops the fifth field	the N2 test
r1-N2-export	the export does not write the last error	the N2 test

After ecfc523's refactor, r1-a-note, r1-a-scan and r1-b-logs-pin were run again at ddf7f6e, together with m4 rewritten for this round's code, over the two refactored tests (-k "test_lanes_index and (log_list_that_fails or log_could_not)"). Each was killed by the same tests as before, and m4 by test_a_lane_whose_log_could_not_be_read_keeps_its_rows.

Fix round 2's mutants were each applied alone to f2723b1. The two marked broad ran over the sixteen tests of rounds 1 and 2 (-k "test_lanes_index and (names_lanes_from_the_archive or log_list_that_fails or register_is_asked or last_sync_failed or unlistable_log or not_whole or log_could_not or warnings_could_not or hung_indexer or fails_after_the_listing or sorts_last)"), and the others over the one test the table names:

mutant	what it changes	result
m2-pass (broad)	both reads work the root out again, as at ddf7f6e	KILLED, 1 failed and 15 passed. Only the C1 test fails, at :2942, with 0 upserted, 1 removed and repoB-1's row gone
m2-pass-trees	only lane_trees_list's call drops the payload	KILLED by the C1 test at :2942: 0 upserted, 1 removed
m2-pass-state	only lane_state_read's call drops it	KILLED by the C1 test at :2944: the sync holds repoB-1, whose generation reads 0 against the 1 its rows were written at
m2-callee-trees	lane_trees_list ignores the payload it is handed	KILLED by the C1 test at :2942, as m2-pass-trees
m2-callee-state	lane_state_read ignores it	KILLED by the C1 test at :2944, as m2-pass-state
mNoSentinel (the review's, broad)	the pinned list of the logs loses its last newline	KILLED, 1 failed and 15 passed. Only the NIT-2 test fails, at :2968, with repoY-1 not indexed (5 upserted, 0 removed)
m2-N1-label	the label is its last sync failed again	KILLED by the N2 test at :3012

Lane 3's terms

• Lock lines. No changed line names acquire_lock, release_lock, lock_try or LOCK_HELD. The md5 of the 73 lines naming them is 97878eb141e051c60a82e339cfe537c8 at 21e0829, 03b6299, d26658d, main a42946e, the merge cef454e, ddf7f6e, d223430 and f2723b1; git diff a42946e f2723b1 has 0 such lines.
• lane-start and lane-handoff are not touched: both are main's at f2723b1.
• Syntax. bash -n passes under GNU bash 3.2.57 (the musl build, run through its own loader) and under 5.2.21, on lanes-edit.sh and lanes at d26658d, cef454e and ddf7f6e, and on lanes-edit.sh at f2723b1, the one shell file fix round 2 changes.
• Merge. cef454e merges main a42946e. Main moved from this branch's base 21e0829 by nine commits: lane-handoff's lifecycle read, Releases: annotated tags, release/vX.Y branches, and openrepotools on TestPyPI and PyPI from v1.1.0 #228's release series, The handoff names every inventory record that is not a whole schema-1 one, and a leg directory this account cannot search, or a dependency nested in one, is unreadable and never absent (fixes #255) #271, PR 3's re-check survivors are killed: tests/test_release_workflow.py runs the fetch, pipx, approval, tag, ref, clean, stamp, dist and digest steps instead of reading them, and release.yml changes only in comments (#228, PR 3b) #284 and The shell suite's wrapper prints the suite's timing table and teardown line on a pass, where CI's log can read them (#77, step 1b) #265. Of this PR's files, only lanes-edit.sh (lane_reconcile's leg sweep) and docs/README-lanes.md (the triad section) changed there, away from its hunks, and the merge is clean. Main has not moved since. refs/pull/289/merge is now ca2410f, with first parent a42946e and second parent f2723b1, and its tree 6e6f2ae is f2723b1's.

Docs

• docs/README-lanes.md, in the worktrees table section:
  • a paragraph, "A lane listing that is not whole writes nothing";
  • a paragraph on which reads the listing makes once and which twice: fix round 1's, which fix round 2 corrects (the list of the logs is made twice) and extends (a lane's dir is read once);
  • the read's NOT WHOLE line, and the --index read's (last error: ...).
• Exit codes (corrected in fix round 1; the README said it right). worktrees --all still exits 0 with its rows, and the PARTIAL line is now among them. Where the failed reads left the listing EMPTY, it used to exit 8 ("none"), and it now exits 0 with the PARTIAL line alone. The indexer's refusal uses its existing exit 1.
• The same is said in worktree_rows's comment, lanes --help and lanes-index's docstrings.

Not covered

Filed from the review: #290 (SHOULD-3 and the four items below that are marked with it) and #291 (SHOULD-4). All of them exist at 21e0829. From review r2: #292 (its SHOULD-B, on main too), under which the four not_whole cases fail on bash 3.2, at lanes:321 (:317 on main).

• A named read whose lane's log cannot be read answers "none" (A named worktree read answers "none" for a lane whose log or register could not be read (from #289's review) #290, the review's SHOULD-3). Fix round 1 said the indexer never makes such a read. It did: the --all read worked each lane's control root out again from the lane's log, for its trees and its snapshot, and threw a failed git show away (review r2's SHOULD-A). Fix round 2 hands those two reads the listing's dir, so the --all read no longer reads a lane's log a second time. A named read still answers "none" where its one read of the lane's log fails.
• A control root that cannot be searched reads as no trees (A lane control root that cannot be searched reads as "no trees", so an inventory sync removes its rows (from #289's review) #291, the review's SHOULD-4). An inventory sync removes its rows.
• One unreadable log file with no origin (A named worktree read answers "none" for a lane whose log or register could not be read (from #289's review) #290). With no origin, or under LANES_NO_GIT=1, a log FILE that cannot be read, as opposed to its directory, leaves no "unreadable log" warning, because log_events runs awk over the files.
• A named lane's read with the register unreadable (A named worktree read answers "none" for a lane whose log or register could not be read (from #289's review) #290). canon_lane cannot find the row's spelling. A lane asked for in another case (worktrees repoa-1 for repoA-1) answers "no worktree is inventoried" with exit 8.
• The lanes listing itself (A named worktree read answers "none" for a lane whose log or register could not be read (from #289's review) #290). It still prints nothing when the list of the logs fails, and silently leaves out the lanes only a log names. Since fix round 1 the events note the failure, but the listing replays their warnings to /dev/null, and a probe at d26658d showed an empty stderr.
• writer_live with the register unreadable (A named worktree read answers "none" for a lane whose log or register could not be read (from #289's review) #290). In the source read it can be 0 rather than unknown, for a lane whose ids only the published row holds. The index never stores that value now, because the sync refuses.
• Refusing is conservative. While the register checkout cannot be read whole, the inventory index does not move at all, and status and now the --index read say why. An unreadable log refuses the sync even where its lane's trees were found, because its binding (and so writer_live) and its dir are unknown.

🤖 Generated with Claude Code

Summary by Sourcery

Protect inventory state from incomplete lane listings by reporting partial reads and refusing to write until a whole listing is available.

New Features:

• Report partial lane listings explicitly and propagate their reasons through worktree, status, and indexed views.

Bug Fixes:

• Prevent inventory sync and reconciliation from deleting or changing rows when the lane listing is incomplete, including failures involving the register, archive, log listing, log directory, or published logs.
• Preserve consistent listing data across related reads so transient later read failures cannot silently omit lanes or trees.
• Correctly classify legacy tree rows with empty paths as trees rather than unread markers.

Enhancements:

• Display the last failed inventory synchronization alongside indexed source timestamps until a later successful write clears it.

Documentation:

• Document partial-listing behavior, refusal semantics, indexed error reporting, and remaining read limitations.

Tests:

• Add coverage for incomplete listings, unreadable logs and directories, pinned reads, failed sync reporting, legacy tree compatibility, and the associated race condition.

[sourcery-ai]

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 1 day and 9 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[sourcery-ai]

Reviewer's Guide

The PR makes lane-listing completeness explicit, prevents partial inventory reads from deleting or modifying indexed rows, renders the condition to users, and adds broad regression coverage including legacy-helper compatibility.

Sequence diagram for refusing a partial lane inventory sync

sequenceDiagram
    participant Caller
    participant WorktreeRows as worktree_rows
    participant Register
    participant Archive
    participant Logs as ObjectLogs
    participant Inventory as lanes-index

    Caller->>WorktreeRows: worktree_rows()
    WorktreeRows->>Register: register_text_checked()
    WorktreeRows->>Archive: archive_text()
    WorktreeRows->>Logs: git ls-tree or ls log directory
    WorktreeRows->>WorktreeRows: lanes_rows --all --closed
    WorktreeRows-->>Inventory: rows plus PARTIAL workstation reason
    Inventory->>Inventory: collect_inventory()
    Inventory-->>Caller: exit 1, nothing written

Loading

Flow diagram for whole and partial lane listings

flowchart TD
    A[Read lane listing] --> B{All required reads succeeded?}
    B -->|Yes| C[Emit lane rows]
    C --> D[Sync inventory changes]
    B -->|No| E[Emit rows reached plus PARTIAL]
    E --> F[Keep last whole index unchanged]
    F --> G[Report NOT WHOLE to users]

Loading

File-Level Changes

Change	Details	Files
Detect and propagate incomplete lane listings instead of treating partial results as complete inventory state.	Re-check register, archive, log listing, and published-log readability for whole-list reads. Emit a single PARTIAL record with specific failure or unknown-warning reasons, including empty-list cases. Keep named-lane reads independent of whole-list partial status.	lanes-edit.sh lanes
Prevent inventory mutations when the source listing is not whole and expose the condition consistently to users.	Reject PARTIAL input during inventory collection and make sync/reconcile exit without writes while preserving the last whole snapshot. Surface the condition in status, worktree output, help text, and documentation with NOT WHOLE messaging and footer guidance. Preserve compatibility by using a distinct PARTIAL record kind rather than a lane-less UNREAD marker.	lanes-index lanes docs/README-lanes.md
Add regression and compatibility coverage for partial reads and legacy helper output.	Test archive, register, log-listing, all-read failures, unreadable logs, lost warning capture, and unlistable no-origin log directories. Verify rows, SQLite state, status, reconcile, named-lane reads, recovery, and NOT WHOLE rendering remain correct. Verify an older helper's empty-path TREE record is retained as a tree rather than misclassified as an unread marker.	tests/test_lanes_index.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#173	Prevent a first sync from publishing usable index provenance that makes a lane with an unreadable trees directory appear to have no inventoried worktrees.	❌	The PR marks a listing PARTIAL when the reads used to discover lane names fail, and refuses to sync that listing. It does not address an individual lane whose trees directory is unreadable: that lane can still be reported as UNREAD while the overall listing is whole, and the PR does not withhold provenance in that case. Its new tests cover listing-source failures and an older helper's tree row, not the issue's first-sync unread-lane scenario.
#173	Make an index read distinguish previously stored worktree rows for a lane held because its inventory could not be read from rows freshly observed at the last sync.	❌	The PR prevents index writes when the lane listing itself is not whole, but it adds no per-lane held/unread marker or equivalent provenance for a lane whose trees inventory is unreadable. Consequently, an index read still cannot tell that lane's retained rows are held.

---

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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sonarqubecloud]

Quality Gate passed

Issues
10 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud