Skip to content

A register that could not be read no longer reads as a lane with no session ids, so the dead-lane --force check and duplicate-holder fail closed (#67) - #221

Merged
brettheap merged 3 commits into
mainfrom
fix/session-ids-unreadable-register-67
Oct 18, 2026
Merged

brettheap merged 3 commits into
mainfrom
fix/session-ids-unreadable-register-67

Conversation

@brettheap

@brettheap brettheap commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

What was wrong

session_ids_of_lane and session_ids_local_of_lane answer the same empty set for two different facts: "this lane has no row" and "the register could not be read".

  • register_text (lanes-edit.sh:3202 at f87fd5a) runs its git show (or cat) inside $( ) and always ends in printf, so it answers 0 whatever the read did.
  • row_of_lane is register_text | awk, and awk answers 0 over an empty render.
  • A failed read was also cached as an empty register, in the variable and in the per-process file. After the first failed read, every later read in the process got an empty register and no failure to report.

The two callers that read an empty id set as a verdict were therefore fail-open:

Evidence. The new test block runs against f87fd5a's lanes-edit.sh in an isolated sandbox. In that sandbox the published register is unreadable, and this checkout's copy has no row for the holder because a peer published it after the clone. Results:

  • duplicate-holder repo67-1 exits 8.
  • claim --force pronounces the holder dead even though a live session on this workstation carries its id. It commits the takeover, and the rescan after the rebase withdraws it only because the pull happened to bring the row into this checkout. Both lines are pushed into an append-only log:
lanes-edit: committed: LOG(repo67-2@Eagle): TAKEOVER opensoft/repo67#5
lanes-edit: lane repo67-1 is no longer confirmed dead — its own log ends RETIRED, but a live session on this workstation now backs it up
lanes-edit: TAKEOVER ABANDONED — lane repo67-1 is alive again. Rule 1: stop and report; do not author a successor.
lanes-edit: committed: LOG(repo67-2@Eagle): CLAIM-LOST opensoft/repo67#5
lanes-edit: pushed origin/main (attempt 1)

The TAKEOVER line itself reads lane repo67-1 is dead (log ends RETIRED, no live session on this workstation, issue #30). If this checkout's copy were unreadable rather than behind, the rescan would not rescue it.

What changed

  • register_text_checked is now the read itself: 0 with the register, or 1 where the published git show or the working tree's cat failed. It caches only a read that succeeded.
  • register_text is now register_text_checked || :. Its output is byte for byte what it was, success or failure, so its pipeline callers are unchanged.
  • session_ids_checked (new) is the published-union-local id set with a contract:
    • 0 with the ids;
    • 8 when both copies were read and give the lane no transcript uuid;
    • 1 when either copy could not be read.
  • holder_is_dead asks the checked read. On 1 it sets a new HOLDER_REGISTER_UNREADABLE beside the existing HOLDER_TERMINAL_UNKNOWN, so every existing fail-closed reader, including claim_rescan_hook, refuses it unchanged. On 8 it goes on to live_holder exactly as before.
  • claim --force refuses HOLDER_REGISTER_UNREADABLE in its own words, ahead of the verb test and next to the Amendment 18(b) refusal. Otherwise an OPENED hold would get the verb refusal, which blames a live session nobody looked for. Exit 2.
  • duplicate_holder_pids asks the checked read and returns 1, with SESSION_FILES_ERR naming the register. The duplicate-holder arm's refusal now reads "could not read the register, this workstation's process table or its session records …". lane-end --retire already refuses on any code other than 0 or 8.
  • docs/README-lanes.md: one sentence in the dead-lane --force paragraph and one in the duplicate-holder paragraph state the new refusals.
  • The other ~10 callers of the session-id pair are unchanged.

managed_projection_read. This change does not make row_of_lane return nonzero on a failed read, so that function's expectation is still not met by row_of_lane itself. Its only caller, lane_is_managed_owned, already refuses an empty register_text before calling it, and managed_projection_check has no caller. Changing row_of_lane's status would reach every other caller, so it is left alone.

A side effect, and why I kept it. A failed read is no longer cached, so a later read in the same process asks git or cat again instead of being served an empty register. For a mode-000 working-tree register with nothing published, cat's error now repeats on each read instead of printing once. The exempt subcommands are unaffected: measured with a missing register, before and after, guard printed the same 9 stderr lines with no cat: line, and session-start printed nothing.

Functions and regions edited

  • lanes-edit.sh: register_text → register_text_checked + a one-line register_text (f87fd5a 3200-3222; head 3201-3245)
  • lanes-edit.sh: holder_is_dead — preamble comment and the HOLDER_* globals (f87fd5a 3751-3757), the reset (3765) and the id read (3813-3814); head 3776-3801 and 3849-3857
  • lanes-edit.sh: session_ids_checked — new, directly after session_ids_local_of_lane (head 5035-5059)
  • lanes-edit.sh: duplicate_holder_pids — only its header comment's last line (f87fd5a 6734) and the id read and empty-set check (6741-6743); head 6802 and 6809-6820. The --resume parsing and the child scan are untouched.
  • lanes-edit.sh: dispatcher, claim) — one new if … die in the --force dead-lane else-branch, after the Amendment 18(b) refusal (head 14799-14807)
  • lanes-edit.sh: dispatcher, duplicate-holder) — its exit-code comment and its *) die message (head 15928-15929, 15941)
  • docs/README-lanes.md — the dead-lane --force paragraph (f87fd5a 948) and the duplicate-holder paragraph (1743)
  • tests/test_lane_helpers.sh — one new block after the issue lanes-edit.sh: a dead lane's OPENED objects and its long-stale CLAIMED have no takeover path — claim --force takes a stale CLAIMED only, log TAKEOVER by hand is refused #30 section, before LANDING, LANDED, who --landing (head 1669-1759)

Tests

One block in tests/test_lane_helpers.sh, 15 assertions, in its own workspace: its own bare origin, a peer clone and this clone, the same shape as the Amendment 19 sandbox.

The fixture.

  • A peer publishes repo67-1 after this clone was made. repo67-1 has repoK-3's shape: its log ends RETIRED with an OPENED PR left behind, and a live session record carries its id.
  • A git shim fails the one show origin/main:lanes/LANES.md, the archive case's idiom.
  • Both halves of the union therefore come back empty. In the main sandbox the working tree carries every row, so the local half rescues the published one and the bug cannot be reproduced there.
  • The brief suggested a dangling-symlink register. It cannot reach this code: the dispatcher's [ -f "$LANES_FILE" ] guard refuses first. The git shim fails the read this bug is about.

The cases.

  • duplicate-holder, readable register and no --fork-session process: 8.
  • duplicate-holder, unreadable register with a stray in the table: 1, never 8, naming the register.
  • claim --force, readable register: refused 2 as a live holder.
  • claim --force, unreadable register: refused 2, naming the register, with no TAKEOVER line and no push.

The duplicate-holder assertions return before pgrep runs, or rest on pgrep's own "no match", so they don't depend on the --resume parsing or child scan that the parallel #76/#86 work changes.

Fail before, pass after. Proved in isolation, not by running the suite. A scratch runner, run under env -i, builds a temp $HOME, the suite's own pgrep/ps fakes, a live sleep, and write_record. It then sources this block verbatim, extracted with awk between its header and the next section (block sha256 3095ee7f… in both runs):

f87fd5a's lanes-edit.sh:  7 passed, 8 failed   (duplicate-holder: expected [1], got [8];
                                                 claim --force: expected [2], got [9], TAKEOVER written and pushed)
this branch:             15 passed, 0 failed

bash -n passes on both changed bash files. I also checked the three bash-3.2 text rules from tests/test_repo_hygiene.py (an unbraced $name before a non-ASCII character, case inside $( ), a quoted replacement half) against both files by regex: no hits. The full suite was not run locally, per the lane's testing rule, so CI is the record.

Closes #67

🤖 Generated with Claude Code

Summary by Sourcery

Fail closed when lane registers cannot be read so dead-lane takeovers and duplicate-holder checks require confirmed session data.

Bug Fixes:

  • Prevent unreadable lane registers from being treated as empty session-id sets, avoiding false dead-lane confirmations and duplicate-holder results.
  • Fail closed in claim --force and duplicate-holder when the register cannot be read, with explicit refusal and error reporting.

Enhancements:

  • Preserve successful register-read caching while retrying failed reads instead of caching empty results.

Documentation:

  • Document that unreadable registers cannot confirm dead lanes or the absence of duplicate holders.

Tests:

  • Add isolated regression coverage for unreadable-register behavior in claim --force and duplicate-holder.

…ession ids, so the dead-lane --force check and duplicate-holder fail closed (#67)

register_text threw away the status of its `git show` (or its `cat`), and
row_of_lane is `register_text | awk`, so session_ids_of_lane and
session_ids_local_of_lane answered the same empty set for "this lane has
no row" and "the register could not be read". holder_is_dead handed that
empty set to live_holder, whose 8 then confirmed dead a lane nobody had
checked, and duplicate_holder_pids returned 8 ("no duplicate holder is
live") without asking the process table anything. A failed read was also
cached as an empty register, so every later read in the process saw a
register with no failure left to report.

register_text_checked is now the read itself (0, or 1 where the read
failed), and it caches only a read that succeeded. register_text is that
read with the status dropped, unchanged for its pipeline callers.
session_ids_checked is the published-union-local id set with a contract:
0 with the ids, 8 when both copies were read and give the lane no
transcript uuid, 1 when either copy could not be read. Only the two
fail-open callers use it. holder_is_dead stops on 1 and sets
HOLDER_REGISTER_UNREADABLE beside HOLDER_TERMINAL_UNKNOWN, and claim
--force refuses it in its own words ahead of the verb test.
duplicate_holder_pids returns 1 and names the register, and the
duplicate-holder arm's refusal now names the register among what could
not be read. The other callers of the session-id pair are unchanged.

The new case in tests/test_lane_helpers.sh runs in its own workspace.
A peer publishes the holder's row after this clone was made, and a git
shim fails the one `show` of the published register, so both halves of
the union come back empty. Against f87fd5a it fails 8 of its 15
assertions: duplicate-holder answers 8, and claim --force grants a
dead-lane TAKEOVER of a lane a live session still backs and pushes it
before the rebase's rescan withdraws it.

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

sourcery-ai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR separates “no session IDs” from “register could not be read” by introducing checked register and session-ID reads, then makes dead-lane --force and duplicate-holder fail closed with targeted errors instead of treating unreadable data as proof that no live holder exists.

Sequence diagram for fail-closed dead-lane force check

sequenceDiagram
    participant Claim
    participant Holder as holder_is_dead
    participant IDs as session_ids_checked
    participant Register as register_text_checked
    participant Live as live_holder

    Claim->>Holder: holder_is_dead(lane)
    Holder->>IDs: session_ids_checked(lane)
    IDs->>Register: register_text_checked()
    Register-->>IDs: register + status
    alt register unreadable
        IDs-->>Holder: status 1
        Holder-->>Claim: HOLDER_REGISTER_UNREADABLE
        Claim-->>Claim: --force refused
    else register readable
        IDs-->>Holder: ids + status 0 or 8
        Holder->>Live: live_holder(ids)
        Live-->>Holder: liveness result
        Holder-->>Claim: dead or not dead
    end
Loading

Sequence diagram for duplicate-holder checked read

sequenceDiagram
    participant Command as duplicate-holder
    participant Check as duplicate_holder_pids
    participant IDs as session_ids_checked
    participant Register as register_text_checked
    participant Processes as ProcessTable

    Command->>Check: duplicate_holder_pids(lane)
    Check->>IDs: session_ids_checked(lane)
    IDs->>Register: register_text_checked()
    Register-->>IDs: register + status
    alt register unreadable
        IDs-->>Check: status 1
        Check-->>Command: refusal naming register
    else ids read successfully
        IDs-->>Check: ids + status 0 or 8
        Check->>Processes: scan holder processes(ids)
        Processes-->>Check: matching processes or none
        Check-->>Command: duplicate result
    end
Loading

File-Level Changes

Change Details Files
Preserve register read failures instead of collapsing them into an empty register, while retaining existing un-checked rendering behavior.
  • Added a status-returning register reader that caches only successful reads.
  • Kept the legacy rendering wrapper’s output and pipeline compatibility.
  • Added checked session-id resolution that distinguishes no IDs (8) from unreadable registers (1).
lanes-edit.sh
Make dead-lane takeover and duplicate-holder detection fail closed when register data is unavailable.
  • Propagated unreadable-register state through holder liveness checks.
  • Added an explicit --force refusal with exit 2 before dead-holder verb validation.
  • Made duplicate-holder detection return 1 and report the register read failure.
lanes-edit.sh
Document and test the distinction between an empty register result and an unreadable register.
  • Documented the new refusal behavior for --force and duplicate-holder.
  • Added an isolated peer/clone fixture covering readable and unreadable register cases, including no takeover or push.
docs/README-lanes.md
tests/test_lane_helpers.sh

Assessment against linked issues

Issue Objective Addressed Explanation
#67 Provide a session-ID read that distinguishes a successfully read register with no IDs from a register read failure. ✅
#67 Make the dead-lane claim --force check and duplicate-holder fail closed when the register cannot be read, rather than treating that failure as an empty ID set. ✅

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

…CI tests this PR on today's main

Main moved 15 commits past this branch's base f87fd5a, through the suite's machinery and pin (.github/workflows/tests.yml, contracts/openreposhape-pin.yaml, tests/conftest.py, tests/test_repo_hygiene.py, upstream/openRepoShape), after this PR's last green run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…CI tests this PR on today's main

Main moved 4 commits past this branch's base 2cb14a9, through the suite's machinery and pin (), after this PR's last green run.

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

Copy link
Copy Markdown

brettheap added a commit that referenced this pull request Oct 18, 2026
…ane holds the object (#74) (#204)

Landed on Brett Heap's word "land #204 #206 #208 #218 #220 #221 when green" (2026-10-18), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39137725493) was green on head af5b7ce with main 586dea2 in its tree.

Independent review (lane 5, Opus, of 4996156; the merges of main add nothing to the PR's diff): LAND.

Closes #74

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 18, 2026
…so a migrated PAUSED row stops showing NO LOG (#89) (#206)

Landed on Brett Heap's word "land #204 #206 #208 #218 #220 #221 when green" (2026-10-18), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39137733527) was green on head cd3ced0 with main 586dea2 in its tree. Main 0a49697 merged with head cd3ced0 is tree f88c2f1, which queue run 39137981522 (workflow_dispatch of tests on 3f5f4b9) tested whole and green.

Independent review (lane 5, Opus, of 00b75e6; the merges of main add nothing to the PR's diff): LAND. Brett Heap confirmed the unpickable (row) lanes as built (2026-10-18).

Closes #89

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 18, 2026
…or binding log instead of guessing a branch (#60) (#208)

Landed on Brett Heap's word "land #204 #206 #208 #218 #220 #221 when green" (2026-10-18), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39137741203) was green on head 3edaf0a with main 586dea2 in its tree. Main be82c20 merged with head 3edaf0a is tree cfd2ee6, which queue run 39137988005 (workflow_dispatch of tests on 9db4863) tested whole and green.

Independent review (lane 5, Opus, of c252a3c; the merges of main add nothing to the PR's diff): LAND.

Closes #60

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 18, 2026
…s why refuse it before writing, and the deferred lane-start command keeps --agent (#85, #72 items 5, 6) (#218)

Landed on Brett Heap's word "land #204 #206 #208 #218 #220 #221 when green" (2026-10-18), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39137747959) was green on head bfc5ddd with main 586dea2 in its tree. Main fca60f5 merged with head bfc5ddd is tree b188309, which queue run 39137992777 (workflow_dispatch of tests on 586df3b) tested whole and green.

Independent review (lane 5, Opus, of 99f3853; the merges of main add nothing to the PR's diff): LAND. Brett Heap confirmed refusing CR/LF at the door (2026-10-18).

Part of #85. Part of #72 (items 5-6).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Oct 18, 2026
…ane-handoff renames the window only once the handoff can no longer be refused (#80 slices B and C) (#220)

Landed on Brett Heap's word "land #204 #206 #208 #218 #220 #221 when green" (2026-10-18), by lane openRepoTools-5. Required Linux CI (guard-launch-mode, tests, tests-no-submodule; run 39137756635) was green on head 0b531e5 with main 586dea2 in its tree. Main 01eff2c merged with head 0b531e5 is tree a388312, which queue run 39137998354 (workflow_dispatch of tests on f335793) tested whole and green.

Independent review (lane 5, Opus, of 9f6fd2d; the merges of main add nothing to the PR's diff): LAND. Brett Heap confirmed the rename trade as built (2026-10-18).

Part of #80 (slices B and C).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@brettheap
brettheap marked this pull request as ready for review October 18, 2026 16:01
@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 1 day and 23 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit babd9fe into main Oct 18, 2026
5 checks passed
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…an-hardening-138

The one conflict is register_text, now #221's register_text_checked. It
keeps #221's shape: a text and a status, a failed read printed and never
cached, and register_text as the same read with the status dropped. The
published-or-not question inside it is #230's register_published, the
tree's answer (0 show, 8 this checkout's copy, 1 a refusal), not
`cat-file -e`.

The three #230 readers that relied on register_text's status now read
register_text_checked, as #230's body planned: rows_named_ci (add-row's
refusal), lanes_rows' one check (the withheld DORMANT class) and
lane_position_rows (next-free).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…86, so CI tests this PR on today's main

One conflict, in docs/README-lanes.md at the end of the duplicate-holder paragraph: #221 (now on main) added the clause "and a register that cannot be read is never 'no duplicate': the row's ids are the whole search, so it exits 1 and names the read (#67)" in front of the lane-end --retire sentence, and this PR rewrote that sentence for #86 (TERM every child, not only the first; report each pid; say RETIRED only once all are gone). Both stay: #221's clause first, then this PR's sentences, which read on from it unchanged. Nothing else conflicted; lanes-edit.sh and tests/test_lane_helpers.sh merged cleanly.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
brettheap pushed a commit that referenced this pull request Oct 18, 2026
brettheap pushed a commit that referenced this pull request Oct 18, 2026
…second checkout by LANES_REPO and LANES_FILE, as the suite's other 24 cases do under Amendment 23's explicit seam (#198)

Main's fe58808 merge brought a new helper, r67(), that pointed the
commands at a second clone through LANES_WORKSPACE_ROOT alone. Since this
slice, the suite exports LANES_REPO and LANES_FILE at $WIP for every case
(ort186 Q9's seam). An explicit LANES_REPO is honoured in place and W no
longer chooses the checkout a command runs in, so r67's reads and its
refused `claim --force` would have run in $WIP, not the #67 clone. r67 now
names the clone the same way the other 24 cases were converted
(LANES_REPO=<clone> LANES_FILE=<clone>/lanes/LANES.md). Its assertions are
unchanged.

Lane: openreposhape-2 (openRepoShape-2)
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