[brettheap]

Part of #80: slices B (flag values) and C (tmux and the window). Slice A, the two fixes to skills/handoff/SKILL.md (items 4 and 7), is deferred and remains open. Details under Remaining.

What was wrong

These are Copilot's findings on workBenches#91 against the vendored bytes. I checked each one at f87fd5a by running the new cases against that commit's code; the results are quoted below.

• Item 1, lane:215. In lane --dir --all, --dir takes --all as its value. At f87fd5a this exits 0 with a checkout called --all, so the "every repository" listing the person asked for never runs. --force --dry-run has the same flaw: --dry-run becomes the why of a real takeover, and that why is written into another place's append-only log.
• Items 6 and 8, lane-handoff:242 and :248. --lane= and --writers-file= accept an empty value. So do the other five = forms: --operation=, --at=, --agent=, --transcript= and --dir=. At f87fd5a all seven exit 0.
• A gap under items 6 and 8. The spaced form never refused either. Every spaced arm reads x="$(flag_value …)", and a die inside $( ) only exits that subshell. The command prints REFUSED and then carries on with the value empty. At f87fd5a, lane-handoff --lane --restart clear exits 0 after this:

  lane-handoff: REFUSED: --lane needs a value and '--restart' is a flag. Use --lane=--restart where that really is the value.
  lane-handoff: lane=repoHF-4 window=hfsess:0 @21 workstation=Eagle
  

  It handed off whatever lane the window resolved to, as a plain handoff, because --restart had been consumed. --writers-file "" also exits 0, and writes a handoff with no briefs.
• Item 10, lane-handoff:351. have_tmux checks only for the binary. Run from a shell outside tmux (env -u TMUX), f87fd5a renames the window and records window hfsess:0 @21, both read from whatever client the tmux server last saw.
• Item 3, lane-handoff:434. *:[0-9]* and @[0-9]* are prefix globs. At f87fd5a, the append-only log takes window hfsess:1junk @21junk when the values come from tmux, and window hfsess:7junk @77junk when they come from the launcher's environment.
• Items 2 and 11, lane-handoff:519. The rename ran before every remaining refusal: the --late preconditions, the writers' snapshot mktemp, the row read (sread register-row) and the --restart binding. Two of those refusals say "nothing was renamed". At f87fd5a a refused --late renames the window first (rename-window repoHF-7 is logged: before 1, after 2). A failed rename was also swallowed. With the fake tmux refusing the rename, f87fd5a exits 0 and writes the PAUSED record and the row anyway.

Already fixed on main (no change here):

• Item 5: the Amendment 17 section of tests/test_lane_helpers.sh runs $HANDOFF_CMD (38 invocations at f87fd5a).
• Item 9: lanes-edit.sh publish-handoff stages the new handoff with mktemp "$pbh_target.resume.XXXXXX" in the target's own directory and installs it with mv -f, after resolving the symlink (landed in Keep supervised manual ctx as legacy compatibility and refuse managed lanes #121). lane-handoff writes the handoff through it.

What changed

• lane: a new not_a_flag helper. The spaced --dir, --force and --wait arms now refuse a value that starts with -, exit 64, and name the = spelling (--dir=--all). Empty = values were already refused at each arm.
• lane-handoff parser:
  • One arm, ahead of the seven =* arms, refuses --operation= | --at= | --lane= | --agent= | --transcript= | --writers-file= | --dir= with exit 2. Because case takes the first match, the =* arms still accept a value that starts with -. For example, --lane=-x still reaches the lane-name check.
  • Every spaced arm now ends || exit, so flag_value's refusal ends the command and keeps its own exit status.
  • A spaced flag as the last argument no longer makes the argument loop spin forever. This is lane-handoff only, in the seven spaced arms (--operation, --at, --lane, --agent, --transcript, --writers-file, --dir). At f87fd5a, lane-handoff --lane printed REFUSED from inside $( ), then ran shift 2 with one argument left. That shift fails and consumes nothing, so the loop never ended and printed REFUSED on every pass. At this head the same command exits 2, with one REFUSED. lane never had the loop: its spaced --force, --wait and --dir arms die in the main shell and exit 64.
• have_tmux: now also requires $TMUX, the same test as lane's tmux_here. The header's environment line says so.
• Window refs: window_ref_ok (^.+:[0-9]+$) and window_id_ok (^@[0-9]+$) are written in POSIX case, the way lane-start writes its id check. They are applied to the live tmux values and to the WORKBENCHES_CLAUDE_WINDOW_REF/_ID fallbacks.
• The rename:
  • It moved to a new section 3a. That is after the handoff refresh and commit, which hold the last refusal (the --restart binding), and before section 4, where the PAUSED record is built and written.
  • A rename that tmux refuses now ends the command with exit 2 and tmux's own message. Nothing of the record is written, and no pane is touched. The refusal says what had already landed (the handoff's new top block, SWAPPING) and gives the re-run.
  • --late still renames, just before its own PAUSED, as it did before.
  • The header's exit-code-2 text describes this new case.

Functions and regions edited

At head 9f6fd2d, as old-line to new-line ranges against f87fd5a:

• lane: new not_a_flag() (new 228-245, after take_arg). The argument loop's --force, --wait and --dir spaced arms (old 236, 238, 240).
• lane-handoff: header exit-code and env comments (old 105, 111).
• lane-handoff: comment above flag_value() (after old 243).
• lane-handoff: argument loop. New empty-= arm after --restart-status, and || exit on the spaced arms at old 276-288. Old 289 (--dir=*) and the --)/*) why arms at old 290-292 are untouched; A one-line record can't take a newline: append-line and lane-handoff's why refuse it before writing, and the deferred lane-start command keeps --agent (#85, #72 items 5, 6) #218 (append-line proves its one line AFTER writing it, so a lane-handoff --why carrying a newline corrupts LANES.md and leaves the checkout dirty #85/Ten hardenings Copilot rounds 4–6 raised on #47, filed rather than taken: the test lock's ownership, three signal/status gates, a deferred command that drops --agent, and the handoff skill catching up with its command #72) edits those.
• lane-handoff: have_tmux() and its comment (old 408-411).
• lane-handoff: new window_ref_ok()/window_id_ok() and the four checks in the identity-triple block (old 1551-1564).
• lane-handoff: removed the rename block (old 1817-1825) and left a 4-line pointer.
• lane-handoff: new section 3a with the rename, inserted after old 2147. That is between the handoff commit block and section 4's header. Old 2148-2152 sit between it and sanitize (old 2153), which A one-line record can't take a newline: append-line and lane-handoff's why refuse it before writing, and the deferred lane-start command keeps --agent (#85, #72 items 5, 6) #218 edits.
• tests/test_lane_helpers.sh: the shared fake tmux. A header note (after old 299) and the FAKE_TMUX_RENAME_FAIL hook in rename-window (after old 355).
• tests/test_lane_helpers.sh: the lane section, after the --led-name fence cases (after old 5929).
• tests/test_lane_helpers.sh: the Amendment 17 section, new parts 3b and 3c after the --late cases (after old 9787). They add lanes repoHF-80…repoHF-83.

Conflict checks: I ran git merge-tree --write-tree of this head against every open PR head, #195-#218. All merge clean, including #218 (#85/#72, the why/sanitize edits). Lane 3's feat/triad-lane-tooling and feat/triad-lane-tooling-handoff also merge clean. Their nearest hunks are at old 1894, 1974-1996, 2057, 2217 and 2339. The removed block (old 1817-1825) and the new 3a (after old 2147) are 69 or more lines from any of them.

Tests

New cases in tests/test_lane_helpers.sh:

• lane section: lane --dir --all gives 64 with the --dir=--all hint. --force --dry-run and --wait --switch give 64 in the same words. None of the three launches anything.
• Amendment 17, part 3b:
  • All seven empty = forms give 2, each with its own flag named.
  • --lane --restart gives 2 with exactly one REFUSED and no lane resolved after it.
  • --writers-file "" gives 2 and never reaches the poll.
  • Across all of those, the logs, the register and its HEAD are byte-identical.
  • --lane=-repoHF-4 still reaches the lane-name check.
• Amendment 17, part 3c:
  • A refused --late renames nothing.
  • With FAKE_TMUX_RENAME_FAIL=1 the result is 2, with tmux's words and the tmux rename-window act, the rename attempted once, no PAUSED and no row flip. A plain re-run then gives 0, renames, and writes one PAUSED and flips the row.
  • Run with env -u TMUX, the handoff gives 0, renames nothing, and records no window.
  • Malformed tmux refs are dropped for well-formed launcher fallbacks (; window hfsess:7 @77;).
  • Malformed fallbacks give no window sub-field.

Fail-before and pass-after, locally and in isolation. Per the lane's rule, I did not run the full suite locally. I built a trimmed copy of the new tests/test_lane_helpers.sh, containing the global setup, the lane fence cases and the whole Amendment 17 section up to its launcher table, plus the footer. I ran it under env -i HOME PATH LANG USER in two scratch trees that carry the same test file: git archive f87fd5a and this head's files.

code	result
f87fd5a	161 passed, 41 failed. All 40 new-case failures are the defects above.
this head	201 passed, 1 failed

The single failure in both runs is an existing assertion that reads repoSW-1, a fixture seeded in a section the trim leaves out. It is not in a section this PR touches.

I tried to run the tests/test_supervised_restart_regressions.py cases that drive lane-handoff --restart through tests/run.sh … -k …. The wrapper waited out its 590-second bound on another lane's live pytest run, so those are left to CI, the suite of record.

bash -n passes on lane, lane-handoff and tests/test_lane_helpers.sh. Every construct used is bash 3.2-safe: case, ${x%=}, ${x##*:} and if ! v="$(…)".

Remaining

This is not a full fix for #80. Slice A is still open:

• Item 4: the same prefix-glob window check in skills/handoff/SKILL.md's fenced block, at about lines 454-456.
• Item 7: the literal <sanitized verbatim> / <what's open…> placeholders in that block's record writes, at about lines 547-552.

It is deferred because other writers are editing SKILL.md in that range.

🤖 Generated with Claude Code

Summary by Sourcery

Harden lane and lane-handoff argument and tmux handling so refused handoffs do not cause unintended changes or record inconsistent state.

Bug Fixes:

• Reject empty flag values and flag-shaped values where lane or lane-handoff expect an argument, preventing unintended operations and ensuring invalid invocations stop immediately.
• Prevent lane-handoff from acting on tmux state when run outside the current tmux client and ignore malformed window references.
• Defer window renaming until all handoff refusal points have passed, and treat a failed rename as an error without writing the PAUSED record or updating the lane row.

Enhancements:

• Strengthen validation of tmux window references and preserve support for explicitly supplied dash-prefixed values through equals-form flags.

Tests:

• Add coverage for invalid lane and lane-handoff arguments, refusal side effects, tmux context handling, malformed window references, and rename failures.

[sourcery-ai]

Reviewer's Guide

This PR hardens lane and lane-handoff argument parsing, validates tmux context and window metadata, and moves window renaming to the final non-refusable point so failed or refused handoffs do not produce misleading window state or PAUSED records. It includes focused regression tests for all affected behaviors; the documented SKILL.md fixes remain deferred.

File-Level Changes

Change	Details	Files
Reject ambiguous or empty flag values before they can be interpreted as operands or silently ignored.	Added dash-prefixed value detection for spaced lane options with exit-64 guidance to use =. Added a centralized empty-= validation arm for the seven lane-handoff value options. Propagated spaced-value refusal failures out of command substitution so parsing stops immediately and preserves side-effect-free behavior.	lane lane-handoff tests/test_lane_helpers.sh
Restrict tmux interaction and window metadata to the current shell and fully valid window references.	Required TMUX in have_tmux before querying or renaming a window. Added whole-string validation for tmux window references and IDs, including launcher-provided fallbacks. Covered malformed live and fallback values, and execution outside tmux, with regression tests.	lane-handoff tests/test_lane_helpers.sh
Defer window renaming until all handoff refusal points have passed and make rename failures transactional.	Moved the rename after handoff refresh/commit and the final restart binding, immediately before PAUSED record creation. Propagated tmux rename failures as exit 2 with tmux diagnostics and avoided writing the PAUSED record or flipping the row. Retained the --late rename behavior while ensuring refused operations never rename the window.	lane-handoff tests/test_lane_helpers.sh
Add focused regression coverage for flag parsing, tmux context, window validation, and rename ordering.	Added fake-tmux support for deterministic rename failure simulation. Verified empty and flag-shaped values produce no writes or lane resolution. Verified refused and failed renames, outside-tmux handoffs, malformed references, and successful retry behavior.	tests/test_lane_helpers.sh

Possibly linked issues

• Crash-consistent lane worktree recovery: canonical inventory and RUNNING → SWAPPING → SWAPPED lifecycle #91: The PR directly fixes several findings in this review, while explicitly leaving the two handoff/SKILL.md findings for a later change.

---

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
0 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

[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.