Repository navigation
Three hardenings Copilot round 5 raised on #93, filed rather than taken: the register rewrite's unchecked status, the sweep's weakened published-path preflight, and retire-rows' wrong exit for an already-closed log #120
Description
Copilot's round 5 on #93 (review of 758a536, 2026-09-16T01:57Z), filed rather
than taken under Brett Heap's ruling of 2026-09-16: Copilot is capped at two
rounds per pull request, and #93 is at five. The three threads are replied to
with this number and resolved on the pull request; none of them is a
correctness hole in what #93 publishes, and each is stated here with the
measurement or the words that decide it, so the next round does not have to
re-derive them.
All three are in lanes-edit.sh on main after #93 lands.
1. The register rewrite's status is ignored, in both places that make one
delete_lines (lanes-edit.sh:1115) and its sibling replace_line
(lanes-edit.sh:1061) both end in
cat -- "$dl_out" > "$LANES_FILE" # redirect FOLLOWS the symlinkand this file sets set -u and not set -e, so a cat that fails falls
through to note and the function returns 0. The > truncates before cat
writes, so the failure mode is not "the write did not happen" but a truncated
register, and in archive-rows the caller then commits the archive beside it.
This is the one of the three with a real consequence, and the fix is the proof
the appends already make one screen up: append_text_line and
append_text_block both prove what landed with cmp -s, so the two rewrites
should do the same —
cat -- "$dl_out" > "$LANES_FILE" && cmp -s -- "$LANES_FILE" "$dl_out" ||
die "the register rewrite did not land ... nothing has been published" 5— which catches a partial write as well as a failed one. replace_line is
pre-existing and has the identical hole; the rule should be written once and
applied to both, because "the register is only ever rewritten in two places" is
the sentence that makes it checkable.
Copilot's pointer to "line 8771 of the same file" is stale — at 758a536 that
line is inside add-row's published-scan comment. The real sibling is
replace_line.
2. The sweep's published-path preflight asks a weaker question than ensure_log does
retire_rows' scan (lanes-edit.sh:8325):
rr_pub="$rr_pubp"
if [ "${rr_pub##*/}" != "$(log_path_for "$rr_l")" ] && [ "$rr_pub" != "$(log_path_for "$rr_l")" ] && [ "$rr_ln2" = 0 ]; thenensure_log's own guard (lanes-edit.sh:1847) is:
if [ "$el_pub" != "$(log_path_for "$lane")" ] && [ "${el_from##*/}" != "${el_pub##*/}" ]; thenTwo differences, and Copilot is right about the second:
[ "${rr_pub##*/}" != "$(log_path_for "$rr_l")" ]is dead. It compares a
BASENAME against a repo-relative PATH (repo19-4.mdagainst
lanes/log/repo19-4.md), which can never be equal whileLANES_LOG_PREFIX
is non-empty. It is a copy ofensure_log's clause with one##*/missing
from the other side.[ "$rr_ln2" = 0 ]is the weakening. Where the canonical log exists HERE
andorigin's tree still carries the old-cased path — the state a case-only
rename leaves when its commit has not been pushed, which is legitimate on a
checkout that is merely AHEAD — the preflight passes andensure_logthen
refuses inside the write loop.retire_rowsis one commit over several
lanes and says so in its own header ("ANY ONE OF THEM REFUSES THE WHOLE RUN,
before a byte is written: this is one commit, so it is one decision"), so a
refusal that arrives after lane 1's log has been appended leaves the checkout
dirty for a reason the scan was supposed to have caught.
The fix is to make the preflight be ensure_log's question rather than a
paraphrase of it: drop the dead clause, drop rr_ln2 = 0, and add
[ "${rr_from##*/}" != "${rr_pub##*/}" ] with rr_from the local file the
count above already found. Carrying ensure_log's own "a case-only rename whose
commit never landed" sentence into the message is the other half — the two
refusals should read the same because they are the same refusal.
3. rr_open — DECLINED as a rule, TAKEN as a sentence
Copilot (lanes-edit.sh:8277): "This only remembers whether any open verb was
seen, not whether it is the final lane-kind verb. A log with STARTED followed
by ENDED therefore leaves rr_open=STARTED and refuses a lane that is already
closed."
The rule is declined, with the amendment's own words. 19(c) refuses "a row
whose log already carries a lane-kind line other than ENDED/RETIRED" —
carries, not ends with. A STARTED … ENDED log carries a STARTED line.
This is the same ground Brett Heap declined round 3's sibling finding on
(#93 (comment)), and
taking it now would put the writer out of step with the clause it implements and
with the listing beside it. It would also reverse Copilot's OWN round-3 finding
on the same function, which asked that retire-rows stop "appending another
RETIRED line to a lane that already has history".
It is unreachable from the word a person types, besides: DORMANT is no object
log, so a row with any log is never in lane-end --retire-dormant's list. Only
a hand-typed retire-rows <lane> reaches it.
The sentence is worth taking. For a STARTED … ENDED log the refusal
currently reads "a lane whose log says it is running is a lane a person can
still pick up. End it as itself: lane-end <lane>" — and lane-end is not the
exit for a lane that has already ended. A row whose log's LAST lane-kind line is
ENDED/RETIRED should be told it is already CLOSED, already hidden by 19(b),
and that archive-rows <repo> is the act that takes a finished row out of the
register. That is one extra read of the same event stream and a second branch on
the message.
Filed by lane openRepoTools-3. The macOS failures in the same CI round are not
here: they were root-caused and fixed on #93 itself (one-true-awk refusing a
multi-line awk -v in delete_lines, and the liveness fixture's bound having
fallen below the wrapper's own TIMEOUT_SECONDS).
Activity
The ten macOS reds at 758a536 were two defects: a `-v` that spans lin…
brettheap commentedon Sep 16, 2026
Note for whoever takes finding 1:
replace_lineis also being touched on #121, which fixes an intermittent exit 5 from itsnumstatguard when two byte-identical row writes land inside one second (the guard reads "nothing changed" as "more than one line changed"). That is a different defect in the same function — this one is the UNCHECKED STATUS of thecat -- "$out" > "$LANES_FILE"underneath it, and the>truncating beforecatwrites is what makes a failure a truncated register rather than an unwritten one. Land #121 first, or take the two together; they are four lines apart.delete_lines, the sibling this finding was raised on, has no such race — it is called once, byarchive-rows.Amendment 19: a closed or dormant lane leaves the listing, one sweep …