Skip to content

The handoff skill's two PAUSED writes carry the why it computed, sanitized as lane-handoff's sanitize does, and a why carrying a line break is refused before step 1 (#80 item 7, #85) - #264

Merged
brettheap merged 6 commits into
mainfrom
fix/handoff-skill-record-writes-80-85
Oct 19, 2026
Merged

brettheap merged 6 commits into
mainfrom
fix/handoff-skill-record-writes-80-85

Conversation

@brettheap

@brettheap brettheap commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What was wrong

#80 item 7: the two record writes carried the template, not the value. Step 4 of skills/handoff/SKILL.md computes why_text (the operator's why, then the kind's word). Then it writes neither record with it:

LANES_LANE="$lane" LANES_SESSION="$uuid" "$L" log PAUSED "lane:$lane" \
  → "$payload" \
  "on <operator>'s word: <sanitized verbatim>"
...
"$L" append-line "PAUSED — lane $lane, session $uuid@$ws, $(date -u +%Y-%m-%dT%H:%M:%SZ), on <operator>'s word \"<sanitized verbatim>\"; <what's open, or NOTHING CLAIMED>; handoff refreshed"

I ran step 4's own block, as main b707bd7 prints it, against a stub lanes-edit.sh with why="a usage reset". These are the two writes it made:

('log', "on <operator>'s word: <sanitized verbatim>")
('append-line', 'PAUSED — lane repoHS-1, session 0a1b2c3d-…@Eagle, 2026-10-19T01:02:03Z, on <operator>\'s word "<sanitized verbatim>"; <what\'s open, or NOTHING CLAIMED>; handoff refreshed')

lane-handoff is the same act from a shell (Amendment 17(a)). It hands both writes its computed $why_text: lane-handoff L2835 (the log PAUSED call) and L2850 (the append-line) on main 55aae16.

#85, the skill's half: a line break in the why. #218 did three things:

  • append-line refuses a CR or LF before it writes.
  • lane-handoff refuses a why carrying one right after reading its arguments, before anything is renamed or written.
  • lane-handoff's sanitize spells every break ; , as the second line of defence.

The skill did none of these:

  • Nothing checked $why.
  • Step 4's why_text="$why" sanitized nothing. It did not even replace —, which the step's own prose says it does.

So a why with a break reached step 2's top block, where a --- line ends the prompt lane-start hands the next session. Then both step 4 writers refused it, but only after step 2 had refreshed and committed the handoff. The act ended with a top block and no record behind it.

What changed

All in skills/handoff/SKILL.md:

  • A check before step 1. A new paragraph and sh block between the preamble's fence and ## 1. refuse a $why carrying a newline or a carriage return. The refusal exits 2, and nothing has been renamed or written at that point. It uses lane-handoff's own door words ("the why is ONE line", "with '; ' where the break was").
  • Step 4 sanitizes the why as lane-handoff's sanitize does. CRLF, a lone CR and a lone LF each become one ; , and so does every —. An empty why is written handoff, as lane-handoff writes it. The kind's word follows, as before. This is the second line of defence: where a break arrives anyway, the record is still written, on one line. Step 4's opening prose now says this.
  • Both record writes carry $why_text. Write (a)'s free text is "$why_text". Write (b) is lane-handoff's own register line, word for word: PAUSED — lane $lane, session $uuid@$ws, <UTC>, $why_text; handoff refreshed.

Both writes drop two pieces of template wording, to match lane-handoff's own lines:

  • the "on 's word" wording, from both (a)'s free text and (b)'s line;
  • the <what's open, or NOTHING CLAIMED> segment, from (b).

lane-handoff's lines never carried either, and nothing in the skill computes a value for the second. What is open is for the handoff's state line to say. The independent review checked that no tool reads either piece: lanes-index, lanes-edit.sh's RULE6_AWK, lane-end's NOTHING CLAIMED scrub (which reads the row's state cell only), lane-start, and append-line's lane_from_text. A grep of the tools on this branch agrees. This is a choice about what the record carries; the alternative is to compute a claims summary in both writers.

Main b707bd7, then 55aae16 (#246), is merged in (merges, never a rebase). #260's sread 15(d) arm, its canon-lane read after step 1's rungs and its whole-string window checks are unchanged, and they still sit in front of step 4's writes. #246 does not touch the skill. Its lane-handoff changes moved the lines the tests cut out (sanitize, the kind-word lines and the register-line template), and each is still found exactly once.

Functions and regions edited:

  • skills/handoff/SKILL.md: new paragraph and sh block between the preamble's L=/sread fence and "## 1." (≈L120-139 at the head)
  • skills/handoff/SKILL.md: step 4's opening prose paragraph (≈L492-499)
  • skills/handoff/SKILL.md: step 4 block, the why_text lines after payload="$payload; kind $kind" (≈L688-703)
  • skills/handoff/SKILL.md: step 4 block, write (a)'s free-text argument and write (b)'s append-line text (≈L737, ≈L743)
  • tests/test_handoff_skill_80_85.py (new file)

Not touched: sread, step 1's block, the step-4 window block (#260's), and lane-handoff.

Tests

tests/test_handoff_skill_80_85.py, new, 41 cases, in the style of tests/test_handoff_skill_72_80.py. It imports tests/test_skill_shell_blocks.py's fence reader and runs the skill's own blocks under bash and zsh -f. The environment is an explicit dict with a stub lanes-edit.sh that records each call's arguments NUL-separated, a tmux that answers nothing and a date that answers one instant.

Fail-before / pass-after. Every LANES_*, TMUX* and WORKBENCHES_* variable was unset. Each run was a plain flock "${TMPDIR:-/tmp}/openrepotools-pytest.lock" python3 -m pytest -q -p no:cacheprovider …, started detached with setsid -f.

  • Before: main b707bd7 exported by git archive, with this test copied in. Result: 39 failed in 8.22s, every case. 18 assertions show on <operator>'s word: <sanitized verbatim> where the computed why belongs, and 17 find no check block (found 0).
  • After: this branch. The six skill and hygiene files gave 385 passed, 1 failed at the merge commit: test_handoff_skill_80_85.py, test_handoff_skill_72_80.py, test_skill_shell_blocks.py, test_handoff_skill_commit_block.py, test_handoff_skill_98_99_100.py and test_repo_hygiene.py. The one failure was this file's door-position case, which matched lane-handoff inside a comment The handoff skill's step-1 fence refuses Amendment 15(d)'s exit 2 instead of reading it as no answer, and its window ref is checked as a whole string (#72 item 8, #80 item 4) #260 added to the preamble's fence. Commit c02e267 makes that case read only the lines that run, and the file then gives 39 passed in 20.06s. Commit 9b80eed moves the test onto test_handoff_skill_72_80.py's Sandbox instead of a copy of it, because SonarCloud's gate counted the copied sandbox as 5.9% duplicated new code (3% allowed). No case changed. With the file at head 9b80eed:
    • this branch: 98 passed in 42.01s for this file, test_handoff_skill_72_80.py and test_skill_shell_blocks.py;
    • main b707bd7 with the file copied in: 39 failed in 10.80s, for the same reasons as above.
  • Also run: py_compile on the test, and bash -n on every skill shell block through test_skill_shell_blocks.parse_problems (no problems).

I did not run the full suite locally (the AGENTS.md ruling of 2026-09-16; the box is shared). CI's tests job is the suite of record.

Fix round, after the independent review (LAND at 9b80eed)

Fix-round runs, each a plain locked pytest started with setsid -f, with every seam unset:

  • Before, at the pre-fix head f040eaa (9b80eed with main 55aae16 merged in), exported with this round's test copied in: 2 failed, 39 passed in 19.66s. The two failures are the new case: bash exits 127 with line 9: why: unbound variable, and zsh says why: parameter not set.
  • Before, at b707bd7: 41 failed in 11.66s.
  • After, at head a7c87c5: 388 passed in 81.90s for test_handoff_skill_80_85.py, test_handoff_skill_72_80.py, test_skill_shell_blocks.py, test_handoff_skill_commit_block.py, test_handoff_skill_98_99_100.py and test_repo_hygiene.py.

Squash message

Title:

The handoff skill's two PAUSED writes carry the why it computed, sanitized as lane-handoff's sanitize does, and a why carrying a line break is refused before step 1 (#80 item 7, #85)

Body:

#80 item 7: step 4 of skills/handoff/SKILL.md computed why_text and wrote
neither record with it. The object log's PAUSED free text was the literal
"on <operator>'s word: <sanitized verbatim>", and the register line carried
that and "<what's open, or NOTHING CLAIMED>". Both writes now carry
$why_text, and the register line is lane-handoff's own line word for word.
So both drop the "on <operator>'s word" wording, and the register line drops
the "<what's open, or NOTHING CLAIMED>" segment, which lane-handoff's line
never carried. No tool reads either: lanes-index, RULE6_AWK, lane-end's
NOTHING CLAIMED scrub (the row state cell only), lane-start and
append-line's lane_from_text.

#85, the skill's half: the skill refuses a why carrying CR or LF before
step 1, with lane-handoff's door words, and reads it as ${why-} so an unset
why passes under set -u. Step 4 sanitizes the why as lane-handoff's
sanitize does: CRLF, CR and LF each become "; ", and so does " — ". An
empty or unset why is written "handoff".

tests/test_handoff_skill_80_85.py runs the check and step 4 under bash and
zsh -f against a stub helper, in test_handoff_skill_72_80.py's sandbox. It
compares both writes with lane-handoff's own sanitize, kind-word lines and
register-line template, cut out of lane-handoff.

Closes #80
Closes #85

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

#80: every item, and what did it

By lane 5's triage, checked against gh issue view 80 --comments (eleven findings: seven in the body, two in each later comment, no others):

item finding done by
1 lane --dir swallows a following -* #220 (be0743b)
2 lane-handoff renames the window before its fallible record writes #220 (be0743b)
3 lane-handoff's window-id checks are prefix globs #220 (be0743b)
4 the same prefix glob in the skill's step-4 block #260 (add5c15)
5 no runtime suite exercises lane-handoff fcccbe0 (#47): the Amendment 17 section of tests/test_lane_helpers.sh runs $HANDOFF_CMD; recorded in #220
6 lane-handoff --lane= accepts an empty value #220 (be0743b)
7 the skill's record writes carry placeholders this PR
8 lane-handoff --writers-file= accepts an empty value #220 (be0743b)
9 the handoff file write is not atomic #121 (00971b9): publish-handoff stages beside the target and installs with mv -f; recorded in #220
10 have_tmux does not check the process is inside tmux #220 (be0743b)
11 a failed tmux rename-window is swallowed #220 (be0743b)

#85: every part, and what did it

part done by
append_text_line (and the append-line verb) refuses CR/LF before it writes #218 (01eff2c)
lane-handoff refuses a why carrying CR/LF at the door; its sanitize flattens CR/LF #218 (01eff2c)
the issue's test: lane-handoff with a newline in the why leaves the register clean; append-line refuses with nothing written #218 (01eff2c)
the /handoff skill's copy: refused before step 1, sanitized in step 4 as lane-handoff's sanitize does this PR

#218 listed the skill's half as the one part remaining for #85, and #85's comments hold only claims.

Closes #80
Closes #85

🤖 Generated with Claude Code

Brett Heap and others added 3 commits October 19, 2026 13:07
… carry the computed why, and a why with a line break is refused before step 1 (#80 item 7, #85)

Checkpoint made on the lane 5 coordinator's word ahead of the 13:06Z Eagle
reboot. This is not the finished slice.

Done:
- skills/handoff/SKILL.md: a new paragraph and sh block before step 1
  refuse a $why carrying CR or LF (exit 2), with lane-handoff's door words.
  Step 4's why_text is now sanitized as lane-handoff's sanitize does (CRLF,
  CR and LF each one "; ", and every " — "), with an empty why written
  "handoff". Writes (a) and (b) carry $why_text in place of the template's
  placeholders, and (b) is lane-handoff's own register line. Step 4's prose
  says so.
- tests/test_handoff_skill_80_85.py (new): runs the door block and step 4's
  block under bash and zsh -f against a stub helper, and compares them with
  lane-handoff's own sanitize and register-line template.
- Checked: every skill shell block parses (bash -n via
  test_skill_shell_blocks.parse_problems: none), zsh -n on the door block,
  and py_compile on the test.

Not done:
- The RED run (the new test against 5e88a2c's skill) and the GREEN run never
  executed: both were queued on the shared pytest lock and were killed for
  the reboot.
- No PR has been opened, and CI has not run.
- Merge main once #260 lands (merge, never rebase).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tized as lane-handoff's sanitize does, and a why carrying a line break is refused before step 1 (#80 item 7, #85)

This commit and the WIP checkpoint before it (520b98f) are one slice. Main
b707bd7 is merged in, so #260's sread 15(d) arm, its canon-lane read after
step 1's rungs and its whole-string window checks sit in front of step 4's
writes, unchanged.

#80 item 7: step 4 of skills/handoff/SKILL.md computed why_text and then
wrote neither record with it. The object log's PAUSED free text was the
literal "on <operator>'s word: <sanitized verbatim>", and the register's
file-level line carried that and "<what's open, or NOTHING CLAIMED>", so
running the block as printed recorded the placeholders. Both writes now
carry $why_text, and the register line is lane-handoff's own line word for
word, which is what "the same act writes the same record" (Amendment 17(a))
asks. That drops the "<what's open, or NOTHING CLAIMED>" segment, which
lane-handoff's line never carried.

#85, the skill's half: #218 made append-line and lane-handoff refuse a why
carrying CR or LF before anything is written, and made lane-handoff's
sanitize spell every break "; ". The skill did neither. Its why_text was
"$why" as typed, so it did not even replace " — ", though its own prose says
it does. A why with a break would reach step 2's top block, and then both
step 4 writers would refuse it after step 2 had already committed the
handoff. The skill now refuses such a why before step 1, with the words
lane-handoff uses at its own door. Step 4 then sanitizes the why as
lane-handoff's sanitize does (CRLF, CR and LF each one "; ", and every
" — "), with an empty why written "handoff". That is the second line of
defence.

tests/test_handoff_skill_80_85.py runs the door block and step 4's block
under bash and zsh -f against a stub helper. It compares the skill's two
writes with what lane-handoff's own sanitize, kind-word lines and
register-line template produce, all cut out of lane-handoff. This commit
makes the door-position case read only the lines that run. After the merge,
#260's comments in the preamble's fence cite lane-handoff by name, and a
comment that names an act is not that act.

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

sourcery-ai Bot commented Oct 19, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR fixes the handoff skill’s PAUSED records to carry the actual sanitized why, adds lane-handoff-compatible handling for empty and separator-containing values, and rejects multiline whys before any side effects; a new isolated 39-case test suite validates behavior and parity across supported shells.

Sequence diagram for safe handoff why handling

sequenceDiagram
    participant Operator
    participant HandoffSkill
    participant LaneHandoff as lane-handoff
    participant HandoffState as Handoff state
    participant Records as PAUSED records

    Operator->>HandoffSkill: /handoff [why]
    HandoffSkill->>HandoffSkill: Check $why for CR or LF
    alt multiline why
        HandoffSkill-->>Operator: Refuse with exit 2
        Note over HandoffSkill: No rename or write occurs
    else one-line why
        HandoffSkill->>HandoffState: Refresh and commit handoff
        HandoffSkill->>HandoffSkill: Sanitize why and append kind
        HandoffSkill->>Records: log PAUSED with $why_text
        HandoffSkill->>Records: append-line with $why_text
    end
Loading

Flow diagram for handoff why sanitization

flowchart TD
    A[Read operator why] --> B{Contains CR or LF?}
    B -->|Yes| C[Refuse before step 1]
    B -->|No| D[Replace every  —  with ; ]
    D --> E{Why is empty?}
    E -->|Yes| F[Use handoff]
    E -->|No| G[Keep sanitized why]
    F --> H[Append kind word]
    G --> H
    H --> I[Write PAUSED object log]
    I --> J[Write PAUSED register line]
Loading

File-Level Changes

Change Details Files
Adds an early validation gate that rejects multiline or carriage-return-containing whys before any handoff side effects occur.
  • Checks $why for LF or CR and exits 2 with guidance before step 1.
  • Places the validation before renames, publishes, commits, and record writes.
  • Adds coverage for LF, CR, CRLF, trailing breaks, accepted one-line values, and ordering.
skills/handoff/SKILL.md
tests/test_handoff_skill_80_85.py
Aligns step 4's why sanitization and empty-value behavior with lane-handoff.
  • Flattens CRLF, CR, LF, and — to ; .
  • Uses handoff for an empty why and appends the computed kind word.
  • Tests parity across representative why and handoff-kind combinations under Bash and zsh.
skills/handoff/SKILL.md
tests/test_handoff_skill_80_85.py
Replaces record templates with the computed sanitized why in both PAUSED writes.
  • Passes $why_text to the object-log PAUSED write.
  • Uses the matching lane-handoff register-line format for append-line.
  • Removes unsupported open-work and placeholder fields from the register line.
  • Verifies no write contains template placeholders and compares outputs with lane-handoff.
skills/handoff/SKILL.md
tests/test_handoff_skill_80_85.py
Introduces isolated shell-block regression tests for issues #80 item 7 and #85.
  • Extracts the skill's fenced shell blocks and executes them under Bash and optional zsh.
  • Uses a controlled environment with stubbed helper, tmux, and date commands.
  • Adds 39 regression cases covering writes, sanitization, refusal behavior, prose, and block ordering.
tests/test_handoff_skill_80_85.py

Assessment against linked issues

Issue Objective Addressed Explanation
#80 Make lane flag parsing reject malformed values: prevent lane --dir from swallowing a following spaced -* option and reject an empty lane-handoff --lane= value. ✅
#80 Make the lane-handoff executable safe on failure and validate window identifiers as full matches; add runtime regression coverage for its stateful behavior. ✅
#80 Bring the handoff skill’s copy-pasteable procedure into line with the executable: use full-match window validation, write the computed reason rather than placeholders, and refuse or sanitize line breaks in the reason. ✅
#85 Make append-line reject CR/LF in its text before appending, so a failed validation cannot leave LANES.md modified or dirty. ✅
#85 Make lane-handoff --why handle line breaks without corrupting the register or leaving the checkout dirty. ✅

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 19, 2026 14:10
…instead of a copy of it

SonarCloud's quality gate failed on #264 with 5.9% duplication on new code
(3% allowed). Its two duplicated blocks were the sandbox's PATH and
environment setup and its run method, both copied from
tests/test_handoff_skill_72_80.py's Sandbox. The test now subclasses that
Sandbox and imports its SHELLS, the way that file imports
test_skill_shell_blocks.py's fence reader. It adds only the recording stub,
tmux and date. The reader of the recorded calls is renamed recorded(),
because the parent keeps a `calls` attribute that would shadow a method of
that name. No case changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nder set -u, and the #80/#85 test names b707bd7 as its red base

A fix round on #264, from its independent review. main 55aae16 (#246) is
merged in before this commit.

The check before step 1 was `case "$why" in`. In a shell running under
`set -u`, with no why given, that stopped the shell there: bash said
"why: unbound variable" and zsh said "parameter not set". It now reads
"${why-}". An unset why is no why at all: it passes, and step 4 writes it
"handoff", as lane-handoff does. The paragraph above the block says so.

tests/test_handoff_skill_80_85.py gains one case, under bash and zsh -f. It
runs the check and then step 4 in one `set -u` shell with why unset, and
expects no error and "handoff kind unknown" in both writes. The block's
marker is now its refusal text, so the case finds the old block and fails
on the real error. The docstrings now name b707bd7, where the red run was
measured, instead of 5e88a2c.

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

Copy link
Copy Markdown

@sourcery-ai

sourcery-ai Bot commented Oct 19, 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 20 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@brettheap
brettheap merged commit 670a852 into main Oct 19, 2026
7 of 8 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment