Repository navigation
Handoff skill: resolve lanes-edit.sh on PATH first, correct the comments that still promised write (c) without a workstation, and refuse the object-log and register-line writes for a lane that has never had a session recorded - #207
Merged
Merged
Conversation
|
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 23 hours and 41 minutes by commenting |
Reviewer's GuideUpdates the handoff skill to use the same PATH-first helper resolution as restart, corrects misleading write-related comments, and fail-closed guards session-keyed writes when a lane has no recorded session UUID; a hermetic test suite verifies the prose, command resolution, and write behavior. Sequence diagram for session-guarded handoff writessequenceDiagram
participant Skill as HandoffSkill
participant Shell
participant lanes_edit_sh as lanes-edit.sh
participant Operator
Skill->>Shell: command -v lanes-edit.sh
Shell-->>Skill: PATH helper or fallback path
Skill->>Skill: Read session uuid from lane row
alt uuid is recorded
Skill->>lanes_edit_sh: log PAUSED
Skill->>lanes_edit_sh: append-line PAUSED
else uuid is missing
Skill->>Operator: Report NO SESSION RECORDED
Skill-->>Skill: Skip log PAUSED and append-line
Skill->>Operator: Print lane-start --no-launch <repo> <n>
end
Skill->>Skill: Refresh handoff and write row state
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…e with no session only when neither answers and the row was read, and make the message and the handoff say only what step 4 will do Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This was referenced Oct 13, 2026
…er expansion, which zsh runs as bash does, and say only what the shape checks cover; run step 2 under zsh in the tests Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This was referenced Oct 17, 2026
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This was referenced Oct 18, 2026
This was referenced Oct 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



What was wrong
Three findings from opensoft/workBenches#93's audit of
skills/handoff/SKILL.md, all pre-existing:R-A11-5has the skill supply the uuid and refuse before it writes where it cannot, naming the act that supplies one, with the gap named in the handoff. The skill supplied it (LANES_SESSION="$uuid") and had no second guard: for a lane with no session to name,log PAUSEDran into the writer's refusal mid-swap, and the register line would have readsession @<ws>.R-A11-27refuses; the code gates (c) behind$ws_missingwith (a) and (b). Step 1's comment carried the same promise in other words ("skips (a) and (b) and still does (c)"), which the issue did not name and which is fixed here too.L=~/projects/xFactory/lanes-edit.shwas the only hard-coded helper path in the vendored tree;skills/restart/SKILL.mdresolves it PATH-first.What changed (
skills/handoff/SKILL.md, line numbers in the new file)$Liscommand -v lanes-edit.sh, else the symlink, byte for byte the line/restarthas; the sentence introducing$Lsays so$CLAUDE_CODE_SESSION_ID, shape-checked; the comment says only that, as the launcher's window exports and$CLAUDE_PROFILE_NAMEare in step 4), the row's last uuid second, aslane-handofftakes them.uuid_missingis set only where neither answers and the row was read (a failedregister-rowread is no answer, Amendment 7(d)). TheNO SESSION RECORDEDmessage prints the act filled in from$lane(lane-start --no-launch <repo> <n>, Amendment 6(c)'s session-cell append) and says whether the row's cell will be set (it is not when$ws_missingis also set). The command is split from$laneby parameter expansion (${lane%-*},${lane##*-}) behind[[ "$lane" =~ ^.+-[0-9]+$ ]], as step 1 does, because zsh, the shell a session's Bash tool runs in, fills$matchand notBASH_REMATCH. Nothing is written here.$uuidcomes fromsession unknownon its first line, aslane-handoffwrites it)if [[ -z "$uuid_missing" ]]around (a) and (b); (b)'s comment carries the exception. The outerif [[ -z "$ws_missing" ]]line is untouched.Where neither id answers (a plain shell outside any session, on a row recording none; the rare case
R-A11-5describes), step 4 skips the object-logPAUSEDand the register's file-level line and still sets the row's state cell.lane-handoffskips (b) the same way (lane-handoff:2293-2297) but still attempts (a) and prints the writer's refusal (:2278); the skill is told first instead, which is the guardR-A11-5asks for.Question for Brett
For that rare case
R-A11-5's wording (Amendment 11,:972-976: "the swap writes what it legally can: the register's file-level:995-997) saysPAUSEDline and the row's state cell") reads as the file-level line (b) being written. This PR skips it, aslane-handoffdoes.append-lineitself checks no session field, so skipping is a choice and not something the writer forces;R-A11-15(R-A11-11's rule that (b) and (c) survive the uuid refusal is "never a licence to invent a value for a field", which a third reading takes as: write (b) with the session clause omitted. Which do you want, skip (b) as now, or write (b) with no session clause?Neighbours
#199 (#197) has landed (83d2690), with #201, #212, #215 and #216, and main is now f595e44. I merged this branch against the heads of
main(f595e44) and of #200 (f12abc3) withgit merge-tree: no conflicts with either, and every shell block in the merged result of #200 passesbash -n. The block that failsbash -nonmain(block 11, with a<profile>placeholder) is #200's to fix.main; my step 2 paragraph (332-339) sits before the$ws_missingparagraph above that block, and no hunk of mine is inside it. The line numbers in this description are those of this branch's file.pclaude <profile>that fails to parse, unlike every other placeholder in the file #78, skills/handoff/SKILL.md: step 5's comment still calls /rename "the only act there is", unreconciled with Amendment 12's guard (pre-existing since 8a36eb3) #79) edits the step 5 and step 6 text and addstests/test_skill_shell_blocks.py. Every shell block I added or changed passesbash -n.Tests
New
tests/test_handoff_skill_98_99_100.py(29 cases). It extracts the skill's fenced blocks and runs them under bash against a stublanes-edit.shthat records every call, with a stubtmux, a minimal environment and a temporary$HOME:L=line equals the restart skill's; with the helper onPATHit wins, without it the symlink path is the fallback; the introducing sentence says PATH first.ws_missing=1makes none of the three writes.CLAUDE_CODE_SESSION_IDset and a row recording none, nothing is flagged andlogandappend-lineare attempted with that id; the session's id beats the row's last one; a malformed id is ignored; with neither id,logandappend-lineare skipped,set-row-stateis still called withtranscript none, andsession @is never written; a failedregister-rowread (exit 3) prints no "never had a session" message; the message and step 4 agree on whether the row's cell is set with and without a workstation; the command is filled in from the lane, and left as<repo> <n>for a name that is not<repo>-<n>; the handoff paragraph sayssession unknown.zsh -f -c) in five cases (no session, no session with a lane name that is not<repo>-<n>, the session's own id, the row's last id, a failed read) and the two outputs must be identical; the first also nameslane-start --no-launch repoHS 17. It runs only where zsh is installed, so it does not run on CI. The Linux image has no zsh: thetests-no-submodulejob of run 39069499556, which lists its skips, printedSKIPPED [5] … zsh is not installedfor it, and thetestsjob runs on the same image. It does run on the workstations, where zsh is the login shell and the shell a session's Bash tool runs in, which is where the defect shows. Installing zsh in the Linuxtestsjob would change that workflow's rule that pytest is the only thing it installs (ci: run the test suite on Linux only #227), so that is left to Brett and is not done here. The restart command had been filled fromBASH_REMATCH, which zsh does not set, so under zsh it printed both fields empty and a bash-only test missed it.Checked by running each test function by hand: all 29 pass on this head. The zsh case
no-sessionis red atbb3a23f(zsh and bash disagree: zsh printed the command with both fields empty; the other zsh cases there are controls), five of the eight cases added in the second round and the strengthened handoff-paragraph case are red at00814c0, and the first round's 10 are red onmain.Under the shared lock (
flock -w 7200 ${TMPDIR:-/tmp}/openrepotools-pytest.lock python3 -m pytest … -q -p no:cacheprovider, one hold for both):tests/test_handoff_skill_98_99_100.pyon5675b70gave29 passed in 6.70s; the same file with-k zshagainst agit archiveofbb3a23fgave1 failed, 4 passed, 24 deselected in 5.08s, the failure beingtest_step_2_says_the_same_under_zsh_as_under_bash[no-session…repoHS-17].CI run 39069499556 on
5675b70is green:tests,tests-no-submoduleandguard-launch-mode(which holds the repository hygiene checks) allsuccess. Runs 38448681350 (00814c0) and 38459000363 (bb3a23f) were green.Closes #98
Closes #99
Closes #100
🤖 Generated with Claude Code
Summary by Sourcery
Harden the handoff skill against missing session and workstation context while aligning helper resolution, guidance, and tests with the supported behavior.
Bug Fixes:
Enhancements:
Tests: