feat(claude-ops): lanes skill — scripted background loop-lane launcher (#508)

[kyle-sexton]

Summary

Adds a lanes skill to claude-ops: a scripted launcher that starts, restarts, stops, and reports loop lanes as named background Claude Code sessions seeded from canonical prompt files. It replaces the manual morning refresh — for each lane: cancel the loop, clear, re-paste its canonical prompt — with one command across N lanes.

• start (default) / restart first git pull --ff-only and claude plugin marketplace update, then launch each configured lane with claude --bg -n <lane> mirroring that lane's model/effort.
• status prints a per-lane running/stopped table with the live sessionId.
• stop ends a lane via claude stop <sessionId>.
• Acts on a session only when its name is a configured lane, so a hand-started session (an interactive window, an unrelated PR Babysit) is never touched.

Implementation

• New skill plugins/claude-ops/skills/lanes/ — mirrors the sibling skill shape (SKILL.md + scripts/*.sh + scripts/*.test.sh + context/ + evals/). disable-model-invocation: true (mutating, operator-initiated).
• scripts/lane-launcher.sh — action router (start/restart/status/stop), set -uo pipefail, jq-driven config parse, --dry-run/--no-pull/--no-update/--config/--repo/--agents-json flags, exit codes 0/3/4.
• Lane config (context/config.md carries the full contract): JSON resolved first-hit-wins --config → $CLAUDE_OPS_LANES_CONFIG → <repo>/.work/lanes.json. Each lane: name, prompt path, optional model/effort. Lane names are not hardcoded — the set is config-driven, matching how morning-brief treats loop lanes ([a-z0-9_-]+, dynamic).
• Prompt storage seam (feat(claude-ops or session-flow): loop-prompt authoring skill — generate and maintain standing loop-lane prompts #480). Prompts are read from a session-local .work dir (prompt_dir, default .work) — provisional. The launcher resolves the prompt dir in exactly one place (resolve_prompt_dir); a TODO documents that feat(claude-ops or session-flow): loop-prompt authoring skill — generate and maintain standing loop-lane prompts #480 (loop-prompt authoring skill) will own the durable home, at which point only that seam is repointed. This PR deliberately does not build durable storage.
• Version bump claude-ops 0.14.0 → 0.15.0 (minor — new feature), re-read from live origin/main (still 0.14.0 at PR time). CHANGELOG top entry; plugin.json/README skill count Six → Seven with the new skill's line.

Verification

The issue's phrasing was verified, not trusted. #508 wrote claude --bg --name "<lane>" and claude agents as if final. I confirmed the real CLI surface on this machine (claude 2.1.215) by --help and a real throwaway session:

claude --help (relevant flags, quoted): --bg, --background Start the session as a background agent and return immediately (manage with 'claude agents'); -n, --name <name> Set a display name for this session; --model <model>; --effort <level> Effort level ... (low, medium, high, xhigh, max). claude agents --json Print active sessions ... as a JSON array (for scripting; does not require a TTY).

Real launch → name → list → stop cycle (this is what corrected the issue — there is no claude agents stop verb; stop is claude stop <id>):

$ claude --bg -n "verify-508-throwaway" "Reply with the single word OK and then stop. Do nothing else."
backgrounded · 1ef5a7f5 · verify-508-throwaway
  claude stop 1ef5a7f5      stop this session

$ claude agents --json   # (filtered)
verify-508-throwaway | 1ef5a7f5-a733-4ca7-864a-5dcc123dd20b | background | idle

$ claude stop 1ef5a7f5
stopped 1ef5a7f5

$ claude agents --json   # (filtered)
CONFIRMED GONE — no throwaway session in list

claude stop --help: Usage: claude stop <id> Stop a background session. Its conversation is kept; resume it later with 'claude attach <id>'.

The launcher itself (real dry-run + live status against claude agents --json):

$ lane-launcher.sh start --repo <demo> --agents-json <empty> --dry-run
== lanes: start ==
DRY-RUN: git -C <demo> pull --ff-only
DRY-RUN: claude plugin marketplace update
  start work --model opus --effort high
DRY-RUN: claude --bg -n work --model opus --effort high <prompt: .../work.md (27B)>
  start babysit --model sonnet --effort medium
DRY-RUN: claude --bg -n babysit --model sonnet --effort medium <prompt: .../babysit.md (41B)>

$ lane-launcher.sh status --repo <demo>   # live `claude agents --json`
  LANE         MODEL    EFFORT   STATE    SESSION
  work         opus     high     stopped  –
  babysit      sonnet   medium   stopped  –

Gates (all green):

• lane-launcher.test.sh — 40 cases PASS, including a PATH-stubbed (claude + git) non-dry-run start case proving the real dispatch path (git pull, marketplace update, and claude --bg seeded with the prompt-file body as a single trailing arg).
• skill-quality static contract gate: CHECK-SKILL lanes: PASS — 0 errors, 0 warning(s) (frontmatter, <500-line cap, markdownlint, evals present, script test).
• shellcheck clean; shfmt clean (2-space per repo .editorconfig); markdownlint-cli2 clean; claude plugin validate plugins/claude-ops → Validation passed.
• Version-collision check against live origin/main: no open PR touches plugins/claude-ops/.claude-plugin/plugin.json; origin/main still 0.14.0 → 0.15.0 free.

Closes #508

Related

• ops: scripted lane launcher — start/restart loop lanes headlessly from canonical prompt files #508 — this issue.
• feat(claude-ops or session-flow): loop-prompt authoring skill — generate and maintain standing loop-lane prompts #480 (loop-prompt authoring skill) — forward dependency, deliberately not built here. It is slated to own durable prompt storage; this skill reads prompts from today's session-local .work and leaves the single resolve_prompt_dir seam for when feat(claude-ops or session-flow): loop-prompt authoring skill — generate and maintain standing loop-lane prompts #480 lands.
• loops/skills: orchestrator context economy — subagent return-payload contracts + GitHub-externalized state #496 (context economy / restart discipline) — the rationale for restarting lanes.
• ops: loop-lane telemetry — running per-lane status (babysit / triage / work) #502 (telemetry) — the per-lane loop telemetry the running lanes feed; morning-brief reads it, this skill starts/stops the lanes.

🤖 Generated with a Claude Code implementation subagent (issue #508)

[claude]

Claude finished @kyle-sexton's task in 3m 7s —— View job

---

Code Review

• Gather context and understand the request
• Review lane-launcher.sh — correctness, security, error handling
• Review lane-launcher.test.sh — coverage, correctness
• Review SKILL.md, context/config.md, evals/evals.json
• Review plugin manifest and README changes
• Compile and post findings

---

Overall this is a well-constructed skill. The CLI surface was verified empirically, the argument sanitisation is solid (array-based cmd construction, jq --arg throughout), the safety invariant ("only touches configured lane names") is both documented and tested, and the test suite has meaningful coverage including a real-dispatch path. A few correctness issues are worth addressing before merge.

Medium — Flag squashing in option parsing

File: lane-launcher.sh L100–116

--config)
  CONFIG="${2:-}"
  shift
  ;;

${2:-} unconditionally consumes the next token, so --config --dry-run silently assigns --dry-run as the config path — the flag is swallowed and DRY_RUN is never set. Same problem on --repo and --agents-json. A minimal fix:

--config)
  [[ -n "${2:-}" && "${2}" != -* ]] || { err "--config requires a non-option argument"; exit 3; }
  CONFIG="$2"; shift
  ;;

Fix this →

Medium — startedAt assumed present in claude agents --json

File: lane-launcher.sh L229

'[ .[] | select(.name == $n) ] | sort_by(.startedAt) | last | .sessionId // empty'

The verified CLI surface listed in the PR body enumerates pid, cwd, kind, sessionId, name, status — startedAt is absent. If it doesn't exist in real output, sort_by(.startedAt) sorts all nulls equally (no-op), and last returns whichever element happens to be last in the array, not the most-recently-started one. The test fixture adds the field explicitly, so tests pass regardless. If duplicate same-name sessions never occur in practice this is low-impact, but the comment "most recently started wins" is the documented contract. Options: (a) confirm startedAt is real and add it to the verified-surface comment, or (b) drop the sort (first-match semantics are fine for the non-duplicate case).

Fix this →

Medium — Restart can leave a lane stopped with exit 0 on prompt failure

File: lane-launcher.sh L370–374

_restart_one() {
  local name="$1" model="$2" effort="$3" prompt_path="$4"
  stop_lane_if_running "$name" || true      # lane stopped ✓
  info "  start $name…"
  launch_lane "$name" "$model" "$effort" "$prompt_path"  # returns 1 on missing prompt
}

launch_lane returns 1 when the prompt file is missing/empty or the effort is invalid, printing a per-lane error to stderr. _restart_one ignores that return code, so the lane is left stopped but not restarted and the script exits 0. A user doing restart who moved a prompt file would only catch the error if they read stderr carefully. _start_one has the same issue (no exit code propagation from launch_lane). Consider tracking a per-invocation failure flag and returning it, or at minimum propagating the return code so main can exit non-zero.

Fix this →

Low — ${TARGET_LANES:+…} on an array tests the first element, not the array length

File: lane-launcher.sh L400, L407

info "== lanes: restart${TARGET_LANES:+ (${TARGET_LANES[*]})} =="

${arr:+word} on an array expands ${arr[0]} — it evaluates to word only when element 0 is non-empty, not when the array has any elements. This happens to work here because lane names are always non-empty, but ${#TARGET_LANES[@]} is the idiomatic guard. A cosmetic issue, not a runtime bug.

Low — Test gap: --agents-json FILE not found

File: lane-launcher.test.sh

Lines 213–216 of lane-launcher.sh (the [[ -f "$AGENTS_JSON_FILE" ]] guard) are not exercised. The malformed-config cases are tested, so the pattern is in the suite; just a gap. Low priority — the code path is simple.

Low — Test gap: -- end-of-options separator

File: lane-launcher.sh L122–128

The -- passthrough that appends remaining tokens to TARGET_LANES is not covered. Also a low-priority gap given the feature is narrow.

---

What's solid

• Safety invariant (never stops a session whose name isn't a configured lane) is correctly implemented and tested — the test at line 154 with sid-other in the fixture is exactly the right gate.
• No eval, no shell word-splitting on user data, jq --arg/--argjson throughout. The prompt-body-as-arg path (cmd+=("$prompt")) is correct.
• require_claude skips in dry-run — the test suite runs cleanly without the CLI installed.
• resolve_prompt_dir as a named seam for the feat(claude-ops or session-flow): loop-prompt authoring skill — generate and maintain standing loop-lane prompts #480 dependency is well-documented at every layer (script header, SKILL.md, config.md). The TODO is specific and actionable.
• 40-case test suite including real-dispatch (non-dry-run) path with PATH-stubbed claude and git — proves the actual shelling-out, not just the echo.
• disable-model-invocation: true is the right call for a mutating, operator-initiated skill.
• CHANGELOG, README, plugin.json version/description changes are consistent and accurate.

[claude]

Claude finished @kyle-sexton's task in 2m 59s —— View job

---

Code Review

• Gather context and understand the request
• Read changed files in their current state (latest commit e0e1e83)
• Check if previous review issues were addressed
• Review lane-launcher.sh — correctness, security, error handling
• Review lane-launcher.test.sh — coverage, correctness
• Compile and post findings

---

What the new commits fixed

be9dc26 adds the real-dispatch (non-dry-run) test that walks the actual claude --bg seeding path via PATH-stubbed claude and git — a meaningful coverage addition.

e0e1e83 correctly fixed two CI issues:

• Script: require_claude now returns early when ACTION == status && AGENTS_JSON_FILE is set — status --agents-json reads a fixture and makes no live claude call, so requiring the CLI was wrong. The guard is tightly scoped to the one combination that's truly offline.
• Tests: export PATH="$STUB_BIN:$PATH" is now suite-wide (line 90), so every test case resolves claude/git to the logging stubs regardless of what's installed on the runner. Previously the stop/real-dispatch cases were the only ones using the stubs.

Both changes are correct and the reasoning in the commit message is accurate.

---

Still open from the previous review

Medium — Flag squashing in option parsing

lane-launcher.sh L100–116

--config)
  CONFIG="${2:-}"
  shift
  ;;

${2:-} unconditionally consumes the next token, so --config --dry-run silently sets CONFIG=--dry-run and never sets DRY_RUN. Same for --repo and --agents-json. Not addressed.

Fix this →

Medium — startedAt assumed present in claude agents --json

lane-launcher.sh L233

'[ .[] | select(.name == $n) ] | sort_by(.startedAt) | last | .sessionId // empty'

The verified CLI surface (line 14–17, SKILL.md line 80) lists pid, cwd, kind, sessionId, name, status — no startedAt. The test fixture explicitly adds the field (test line 64), so tests pass regardless. If startedAt is absent in real output, sort_by is a no-op and last returns an arbitrary element — the "most recently started wins" comment becomes misleading documentation. Not addressed.

Options remain: (a) confirm startedAt is real and update the verified-surface comment, or (b) drop the sort since duplicate-name sessions don't occur in practice.

Fix this →

Medium — Partial launch failure exits 0

lane-launcher.sh L363–379

_start_one and _restart_one both end with launch_lane, which returns 1 on missing/empty prompt or invalid effort. for_each_lane doesn't track failures (no set -e), and action_start/action_restart don't either. So if lane 2 of 3 fails to launch, lanes 1 and 3 proceed and the script exits 0. Not addressed.

A user who moved a prompt file mid-restart would see errors on stderr but get exit 0 and no indication of how many lanes are actually running. Tracking a per-invocation failure flag in for_each_lane and propagating it through main would fix this.

Fix this →

---

New finding

Low — stop_lane_if_running ignores the claude stop exit code; a failed stop proceeds to relaunch

lane-launcher.sh L297–306

stop_lane_if_running() {
  local name="$1" sid
  sid="$(running_session_id "$name")"
  if [[ -n "$sid" ]]; then
    info "  stop $name ($sid)"
    run claude stop "$sid"   # exit code is NOT checked
    return 0                 # always 0 whether stop succeeded or not
  fi
  return 1
}

When called from _restart_one, a claude stop failure is swallowed (return 0), _restart_one proceeds to launch_lane, and the user ends up with two sessions sharing a name — the one that wasn't actually stopped and the newly launched one. Low-priority because claude stop rarely fails in practice, but it's a real failure mode.

Fix this →

---

Carry-forward low-priority gaps (unchanged)

• ${TARGET_LANES:+…} on an array (L404): tests element 0, not array length. Works in practice but ${#TARGET_LANES[@]} is the correct idiom.
• Test gap: --agents-json FILE not found path (script L217–220) still uncovered.
• Test gap: -- end-of-options passthrough (script L122–128) still uncovered.

---

What's solid

The e0e1e83 hermetic fix is clean and well-scoped. The offline-status gate (ACTION == status && AGENTS_JSON_FILE) is correctly narrow — it doesn't over-exempt stop --agents-json which still shells out to claude stop. The suite-wide PATH stub is the right approach: it removes the ambient-claude dependency without making individual cases responsible for it.

The core safety invariant (only configured lane names are ever stopped), array-based command construction, and jq --arg throughout all remain solid. The resolve_prompt_dir seam is well-documented at every layer.

The three medium issues above are the outstanding blockers; the low items can reasonably ship as-is.

[claude]

Claude finished @kyle-sexton's task in 2m 2s —— View job

---

Code Review

• Gather context and understand the request
• Read current state of lane-launcher.sh and lane-launcher.test.sh
• Check if previously flagged medium issues were addressed
• Review new changes from 01102de (sync root plugin index + mark scripts executable)
• Compile and post findings

---

What 01102de changed

This commit is a pure CI hygiene fix — no functional code changes:

1. Root README.md — the validate-plugins.sh gate requires the root README entry to mirror each plugin.json description verbatim; the claude-ops line was still saying "Six skills" while plugin.json had already been bumped to "Seven". Updated to match.
2. Script permissions — lane-launcher.sh and lane-launcher.test.sh were committed at mode 100644 (non-executable) despite carrying shebangs. Mode bumped to 100755 to match every sibling skill script. No content changes.

Both fixes are correct and narrowly scoped. No concern here.

---

Status of prior review findings

The three medium issues flagged in previous rounds remain unaddressed in the current HEAD (01102de).

Medium — Flag squashing in option parsing

lane-launcher.sh L100–116

--config)
  CONFIG="${2:-}"
  shift
  ;;

${2:-} unconditionally consumes the next positional regardless of whether it is another flag. --config --dry-run silently assigns CONFIG=--dry-run and never sets DRY_RUN. Same bug on --repo and --agents-json. Not addressed.

Fix this →

Medium — startedAt assumed present in claude agents --json

lane-launcher.sh L233

'[ .[] | select(.name == $n) ] | sort_by(.startedAt) | last | .sessionId // empty'

The verified CLI surface comment at lines 14–17 enumerates pid, cwd, kind, sessionId, name, status — startedAt is not listed. The test fixture (line 63) explicitly adds it to mask the gap. If startedAt is absent in real output, sort_by is a no-op and last picks an arbitrary element. Not addressed.

Options: (a) run claude agents --json on this machine, confirm startedAt is present, and add it to the verified-surface comment, or (b) drop the sort_by since duplicate-name sessions don't occur in practice and "first match" semantics are fine.

Fix this →

Medium — Partial launch failure exits 0

lane-launcher.sh L358, L363–379

for_each_lane (L327) calls the callback in a loop with no failure tracking. _start_one and _restart_one both tail-call launch_lane, which returns 1 on a missing/empty prompt or an invalid effort value — but neither propagates that to the caller. A restart where one prompt file has been moved stops the lane, fails to relaunch it, prints errors to stderr, and exits 0. Not addressed.

Fix this →

---

Carry-forward low-priority issues (all unchanged)

• stop_lane_if_running ignores claude stop exit code (L302): run claude stop "$sid" return code is discarded, then return 0 unconditionally — a failed stop proceeds to relaunch and can leave two sessions sharing a name.
• ${TARGET_LANES:+…} on an array (L404): expands element 0, not array length. Works in practice; ${#TARGET_LANES[@]} is the correct idiom.
• Test gap: --agents-json FILE not found path (script L218–221) uncovered.
• Test gap: -- end-of-options passthrough (script L122–128) uncovered.

---

Assessment

The 01102de commit is correct and clean — the CI failures (README description mismatch + non-executable scripts) are real validation gate failures that needed fixing. No new concerns introduced.

The three medium issues above are the remaining blockers before merge. The low-priority items can reasonably ship as-is given the narrow operator-only audience.

[claude]

Claude finished @kyle-sexton's task in 1m 40s —— View job

---

Code Review

• Gather context and understand the request
• Read current state of lane-launcher.sh at HEAD (f57d560)
• Check if previously flagged medium issues were addressed
• Compile and post findings

---

What f57d560 changed

This commit is a comment-hygiene CI gate fix only — no functional changes:

• Line 24-25: "Issue #480 (loop-prompt authoring skill)" → "A forthcoming loop-prompt authoring skill" (code-comment tracker-ref gate compliance)
• Line 205: "# The one prompt-storage seam. #480 will repoint this at a durable home." → "# The one prompt-storage seam — repoint here when a durable prompt home exists."

Both rewrites are accurate and correct. The #480 citation is preserved in SKILL.md and context/config.md where markdown is outside the gate's scan scope.

---

Status of prior review findings

The three medium issues flagged across three prior rounds remain unaddressed in the current HEAD.

Medium — Flag squashing in option parsing

lane-launcher.sh L100–116

--config)
  CONFIG="${2:-}"
  shift
  ;;

${2:-} unconditionally consumes the next positional regardless of whether it's another flag. --config --dry-run silently assigns CONFIG=--dry-run and never sets DRY_RUN. Same bug at L105 (--repo) and L113 (--agents-json). Not addressed.

Fix this →

Medium — startedAt assumed present in claude agents --json

lane-launcher.sh L233

'[ .[] | select(.name == $n) ] | sort_by(.startedAt) | last | .sessionId // empty'

The verified CLI surface at L14–17 enumerates pid, cwd, kind, sessionId, name, status — startedAt is absent. If absent in real output, sort_by is a no-op and last returns an arbitrary element. The test fixture adds the field explicitly (masking the gap). Not addressed.

Options: (a) run claude agents --json on this machine, confirm startedAt is present, add it to the verified-surface comment, or (b) drop the sort_by — first-match semantics are fine since duplicate-name sessions don't occur in practice.

Fix this →

Medium — Partial launch failure exits 0

lane-launcher.sh L358, L363–379

for_each_lane (L327) calls the callback with no failure tracking; _start_one and _restart_one both tail-call launch_lane (which returns 1 on missing/empty prompt or invalid effort) without propagating that return code. A restart where one prompt file has been moved stops the lane, fails to relaunch it, prints errors to stderr, and exits 0. Not addressed.

Fix this →

---

Carry-forward low-priority issues (all unchanged)

• stop_lane_if_running ignores claude stop exit code (L302): run claude stop "$sid" return code is discarded, then return 0 unconditionally — a failed stop proceeds to relaunch and can leave two sessions sharing a name.
• ${TARGET_LANES:+…} on an array (L404, L411): expands element 0, not array length. Works in practice; ${#TARGET_LANES[@]} is the correct idiom.
• Test gap: --agents-json FILE not found path (script L218–221) uncovered.
• Test gap: -- end-of-options passthrough (script L122–128) uncovered.

---

Assessment

f57d560 is a correct and narrowly scoped CI gate fix (comment hygiene). No new concerns introduced.

The three medium issues above remain the outstanding blockers before merge. The low-priority items can ship as-is given the narrow operator-only audience.

[claude]

Claude finished @kyle-sexton's task in 2m 26s —— View job

---

Code Review

• Gather context and understand the request
• Read lane-launcher.sh at HEAD (f889c94) — verify the 3 medium fixes
• Read lane-launcher.test.sh — check for new test coverage
• Assess low-priority carry-forward items
• Post final review findings

---

f889c94 resolves every outstanding finding from the prior three review rounds. Here's the per-issue audit.

---

Medium 1 — Flag squashing ✅ Fixed

lane-launcher.sh L83–92, L112–132

A check_optarg() helper is extracted (L88–92) and called before every shift on --config, --repo, and --agents-json. The guard is well-commented (kept out of a subshell so the caller's exit actually fires), which is the subtle correctness point that a naïve inline expansion would miss. The =VALUE forms (--config=FILE) are also handled alongside the space-separated forms.

Three new test cases at L208–217 verify each option rejects a following flag with exit 3.

---

Medium 2 — startedAt field in claude agents --json ✅ Verified and documented

lane-launcher.sh L14–18

The verified CLI surface comment now includes startedAt in the field list, confirming the field is real and the sort_by(.startedAt) at L247 is correct. The fixture (test L64) was already including it; it now matches the documented surface rather than masking a gap.

---

Medium 3 — Partial launch failure exits 0 ✅ Fixed

lane-launcher.sh L360, L375–377, L434–439, L443–448

for_each_lane now tracks a failures=0 flag, sets it on any callback non-zero, and returns it. action_start and action_restart propagate that into their own rc variable and return it; main exits with the result. The sweep-continues-on-per-lane-failure behaviour (all remaining lanes still get their turn) is explicitly documented with a comment at L373–374.

Two tests at L221–227 verify the non-zero exit on bad-prompt config and the zero exit on a clean config.

---

Low — stop_lane_if_running ignores claude stop exit code ✅ Fixed (and more)

lane-launcher.sh L311–319, L392–405, L407–418

stop_lane_if_running no longer unconditionally return 0s — it now falls through to propagate run claude stop "$sid"'s exit code. The three return-code semantics (0 = stopped OK, 2 = was not running, other = stop failed) are documented on the function header.

_restart_one reads the stop result and refuses to relaunch when the stop genuinely failed (rather than the benign not-running case), with a clear error message about the duplicate-session risk. _stop_one has a case dispatcher that surfaces the failure correctly.

Four tests at L232–243 exercise the STUB_CLAUDE_STOP_RC=1 path for both restart and stop.

---

Low — ${TARGET_LANES:+…} on an array ✅ Fixed

lane-launcher.sh L431

The inline expansion is replaced by a lane_scope() helper that uses ((${#TARGET_LANES[@]})) — the correct array-length idiom. Both action_restart and action_stop call it; action_start omits it (consistent with the usage spec, which does not accept lane targets for start).

---

Low test gaps ✅ Both closed

lane-launcher.test.sh L247–255

• --agents-json FILE not found: exit 4 + message checked (L247–249).
• -- end-of-options passthrough: lane after -- accepted and targeted correctly (L251–255).

---

Test count

40 → 55 cases. All additions exercise the specific paths that were previously gaps or regressions.

---

Assessment

All findings from three prior review rounds are resolved — correctly and with test coverage. The fixes are well-reasoned (the _restart_one duplicate-session guard is especially solid), and no new issues are introduced. This PR is ready to merge.

[kyle-sexton]

Review findings — classification & resolution

All three MEDIUM findings and every low-priority carry-forward are fixed in the latest commit (lane-launcher.sh / lane-launcher.test.sh). No finding is deferred.

#	Finding	Severity	Classification	Action
M1	Flag squashing: --config/--repo/--agents-json consumed the next token unconditionally, so --config --dry-run silently swallowed --dry-run	Medium	Valid bug	Fixed — added check_optarg guard rejecting a missing or flag-shaped value ("$2" != -*); the three space-separated handlers call it before assigning. Kept out of a subshell so the exit 3 fires. Tests 41-44.
M2	sort_by(.startedAt) sorted on a field not listed in the verified-surface comment — risked a silent no-op returning an arbitrary session	Medium	Valid concern; field confirmed present	Fixed (option a) — ran real claude agents --json: startedAt is present on every session (keys: pid, cwd, kind, startedAt, sessionId, name, status). Added startedAt to the verified-surface comment (L14-18). The most-recent-wins sort is correct, not a no-op.
M3	Partial launch failure exited 0: for_each_lane didn't track callback failures, so a lane that failed to (re)launch left $?=0	Medium	Valid bug	Fixed — for_each_lane accumulates per-lane callback failures and returns non-zero (the sweep still completes every lane); action_start/action_restart propagate it; main (and thus the script) exits non-zero. Tests 45-47.
L1	stop_lane_if_running discarded claude stop's exit code (always returned 0), so a failed stop then relaunched — risking two sessions sharing a name	Low	Valid	Fixed — new return contract (0 stopped · 2 not running · other = stop's exit); _restart_one refuses to relaunch on a real stop failure and returns non-zero; _stop_one reports it. Tests 47-51.
L2	${TARGET_LANES:+…} tested element 0, not array length	Low	Valid	Fixed — replaced with a lane_scope helper using ((${#TARGET_LANES[@]})).
L3	Test gaps: --agents-json FILE-not-found path and -- end-of-options passthrough uncovered	Low	Valid	Fixed — added both (tests 52, 54-55). The not-found test surfaced a latent bug: the old existence guard sat inside sessions_json, which runs on the left of a | (a subshell), so its exit 4 never propagated. Moved the check into main (main shell); it now correctly exits 4.

Verification: lane-launcher.test.sh — 55 cases PASS (was 40); shellcheck + shfmt clean; CHECK-SKILL lanes: PASS — 0 errors, 0 warning(s); all required CI gates green.

[claude]

Claude finished @kyle-sexton's task in 2m 42s —— View job

---

Code Review

• Gather context and understand the request
• Read lane-launcher.sh at HEAD (f889c94) — verify final state
• Read lane-launcher.test.sh — verify test coverage
• Read SKILL.md, context/config.md, evals/evals.json
• Review plugin manifest and README changes
• Check CLAUDE.md alignment (repo-agnostic, configurable, plugin-form-safe, versioned, security)
• Compile and post final findings

---

Four prior review rounds identified and addressed every medium and low-priority finding. This pass audits the final state of HEAD (f889c94) for correctness, security, and CLAUDE.md design-rule compliance.

CLAUDE.md Design Rules

Rule	Assessment
Repo-agnostic	✅ No hardcoded paths. --repo / git rev-parse resolves the repo root; config is consumer-owned.
Configurable without editing	✅ Lane set entirely config-driven ($CLAUDE_OPS_LANES_CONFIG, --config, or <repo>/.work/lanes.json). No fork needed.
Plugin-form-safe	✅ SKILL.md references ${CLAUDE_PLUGIN_ROOT}/skills/lanes/scripts/lane-launcher.sh. No ../ reach-outs; state is in the consumer's .work (provisional, with documented seam for #480).
No PII / secrets	✅ Nothing hardcoded; no credentials or tokens.
Versioned	✅ 0.14.0 → 0.15.0 (minor, new feature). Consistent across plugin.json, CHANGELOG, README, and root README.
Security-reviewed	✅ No eval; all config values go through jq --arg/--argjson; prompt body passed as a single array element (cmd+=("$prompt")); effort allowlisted against VALID_EFFORTS; REPO canonicalized via cd && pwd; config validated as JSON before parsing. Egress limited to git pull + claude plugin marketplace update, both documented.

Correctness — Final State

All prior-round fixes verified:

• Flag squashing (M1): check_optarg (L88–92) correctly guards --config, --repo, --agents-json space-separated forms. The =VALUE forms (--config=FILE, L117/123/132) also handled. The || exit 3 fires in the main shell, not a subshell — the comment at L86–87 correctly explains this. ✅
• startedAt field (M2): Verified CLI surface comment at L14–18 now includes startedAt. The sort_by(.startedAt) | last at L247 is correctly documented and tested. ✅
• Partial launch failure (M3): for_each_lane tracks failures=0 (L360), sets it on any callback non-zero (L375), returns it (L377). action_start/action_restart propagate it; main exits with the result. ✅
• claude stop exit code (L1): stop_lane_if_running (L311–319) propagates the stop's exit code. _restart_one (L395–402) refuses to relaunch on a genuine stop failure (distinguishes s==2 = not-running from a real failure). ✅
• Array length idiom (L2): lane_scope at L431 uses ((${#TARGET_LANES[@]})). ✅
• --agents-json not-found path: Validated in main (L469–472), not inside sessions_json which runs in a subshell — the latent exit 4 propagation bug (surfaced by the test) is correctly fixed. ✅

Test Coverage (55 cases)

The suite is hermetic: export PATH="$STUB_BIN:$PATH" at L93 covers all cases without ambient claude or git. Coverage includes:

• Config resolution and validation (missing, malformed JSON, empty-lanes) ✅
• Status output from fixture, non-lane sessions excluded ✅
• Start (dry-run): skip running, launch missing, model/effort mirroring, refresh step ✅
• Start (real dispatch via PATH-stub): git pull, marketplace update, prompt body as trailing arg ✅
• Restart (dry-run): stop-then-start, scope enforcement ✅
• Stop (real dispatch via PATH-stub): dispatches claude stop <sessionId> for configured lanes only ✅
• Unknown lane rejection (exit 3) ✅
• Missing/empty prompt and invalid effort (skipped, non-zero exit, sweep continues) ✅
• Flag squashing guard (3 options × exit 3 assertions) ✅
• Failed claude stop → no relaunch, non-zero exit (STUB_CLAUDE_STOP_RC=1) ✅
• --agents-json file not found (exit 4) ✅
• -- end-of-options passthrough ✅

Minor Documentation Gap (non-blocking)

SKILL.md L80 — The Verified CLI Surface section lists claude agents --json fields as pid, cwd, kind, sessionId, name, status, omitting startedAt. The M2 fix correctly updated the script header (L14–18) but didn't propagate to this description. The sort_by(.startedAt) is confirmed correct; this is a prose-only inconsistency and does not affect behavior.

Fix this →

What's Solid

• The core safety invariant — stop/restart only act on sessions whose name is a configured lane — is correctly implemented in for_each_lane (target-name validation against config, L349–358) and in running_session_id (name-filtered select(.name == $n), L246–247), and tested directly (the sid-other fixture entry at test L65–66 is the right gate).
• sessions_json correctly defers the --agents-json existence check to main (L469–472), fixing the latent subshell bug where exit 4 would only kill the subshell. The comment at L231–233 explains the invariant.
• _restart_one's duplicate-session guard (L399–402) is the right defensive shape: a failed stop is not a benign "not running" (return code 2), so refusing to relaunch avoids leaving two sessions sharing a name.
• evals.json has 5 targeted evals covering the four actions plus the prompt-storage seam, with accurate expected outputs and expectations.
• CHANGELOG, plugin.json, plugins/claude-ops/README.md, and root README.md are all consistent at 0.15.0 / "Seven skills".

Assessment

All medium and low-priority findings from four prior review rounds are resolved. The one item above is a prose-only doc gap in SKILL.md and does not block merge. This PR is ready.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f889c94d22

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 52s —— View job

---

Code Review

• Gather context and understand the request
• Read lane-launcher.sh at HEAD (3240071) — verify Codex P1 fixes and final state
• Read lane-launcher.test.sh — verify test coverage for new fixes
• Check SKILL.md startedAt doc gap from prior round
• Compile and post findings

---

What 3240071 changed

fix(claude-ops): abort lane-launcher on a failed live session list — directly addresses both Codex P1 findings.

---

Codex P1.1 — Propagate failures from claude agents --json ✅ Fixed

lane-launcher.sh L251–266, L500–507

load_sessions now propagates a real CLI failure instead of coercing it to []. On failure, if --dry-run is active, it assigns SESSIONS_JSON='[]' and returns 0 (the documented offline-dry-run behaviour — mutates nothing so a stale view is harmless); otherwise returns 1. main catches that return code and exits 4 with a clear message explaining why (not acting on fabricated state). The comment block at L230–248 is thorough and correctly documents the invariant.

The jq -e 'type == "array"' guard at L264 catches a non-array response (e.g. an error string printed to stdout), which is a sensible defensive layer on top of the exit-code check.

Three test cases at L271–286 cover the abort-on-failure path, the successful live-list path, and the dry-run-tolerates-failure path.

---

Codex P1.2 — Exclude terminal sessions from the running-lane lookup ✅ Addressed (doc + CLI guarantee)

lane-launcher.sh L244–248

# `claude agents --json` (no --all) lists ACTIVE sessions only — interactive and
# running background; completed/terminal background sessions are excluded by the
# CLI (they surface only under --all, carrying a `state` like "done" rather than
# an active `status`). A name match here is therefore already a live-lane match.
# Do NOT add --all without also filtering terminal sessions out of this lookup.

The code already relied on this CLI guarantee; the comment now makes the invariant explicit and warns against the footgun (--all without filtering). No code change needed because the behavior was correct; the gap was documentation. The warning is exactly what future maintainers need.

---

Carry-forward doc gap (still open, non-blocking)

SKILL.md L80

`claude agents --json` (list sessions: pid, cwd, kind, sessionId, name, status)

startedAt is still absent from this prose description. The script header (lane-launcher.sh L14–18) was updated in f889c94 to include it, but SKILL.md L80 was not propagated. Confirmed non-blocking — the sort is correct, the script header is authoritative, and SKILL.md is user-facing prose, not a behavioral contract. But it will mislead a consumer reading the skill description.

Fix this →

---

Test count

55 → 58 cases (5 new assertions across 3 logical test blocks, with the Codex P1 section adding the abort, success, and dry-run-tolerance paths).

---

Assessment

3240071 is correct and cleanly targeted. Both Codex P1 findings are resolved — the abort-on-failure is now code, and the terminal-session concern is explained away by the confirmed CLI guarantee (documented so future maintainers don't inadvertently break it). The SKILL.md startedAt prose gap is the single remaining item and does not block merge.

All findings from six prior review rounds are addressed. This PR is ready to merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3240071f75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 29s —— View job

---

Code Review

• Gather context and understand the request
• Read current state of lane-launcher.sh at HEAD (6449e32)
• Read lane-launcher.test.sh — verify test coverage
• Check SKILL.md — confirm startedAt doc gap is fixed
• Assess Codex P1 (claude agents --json cwd) and P2 (duplicate lane names) findings
• Compile and post final findings

---

What 6449e32 changed

docs(claude-ops): add startedAt to the lanes Verified CLI Surface — fixes the single open prose gap from the previous review round.

SKILL.md L80–81:

`claude agents --json` (list active sessions: pid, cwd, kind, startedAt,
sessionId, name, status),

startedAt is now present in the field list, matching the script header at lane-launcher.sh L14–18. The doc gap flagged across the last two review rounds is resolved. ✅

---

Codex P1 — claude agents --json cwd dependency

Claim: claude agents --json must be run from $REPO or it returns a cwd-scoped list, causing start to misread lane state and launch duplicates.

Assessment: false alarm for the --json scripting path.

The Codex finding cites plugins/playbooks/skills/boris/vendor/SKILL.md:1260:

"Run claude agents from a root code directory; it tracks every session under that root."

That tip describes the interactive TUI (claude agents with no flags), which presents a tree view rooted in the invoking directory. The --json flag is a separate scripting mode — claude agents --help (verified on 2.1.215, documented at lane-launcher.sh L12–19) returns all active sessions globally as an array, not a cwd-filtered subset. This is confirmed by the field list: sessions carry cwd as a data field, not as a filter — the caller reads the cwd if it needs it, but the list itself is not scoped to the caller's directory.

The existing comment at L244–248 (# Do NOT add --all without also filtering terminal sessions out of this lookup) already addresses the closely related interactive-vs-scripting distinction. The cwd concern is worth a brief companion note for future maintainers — but no code change is needed because the behavior is correct.

Optional doc-only note (non-blocking): A one-line comment on load_sessions explaining that --json returns a global active-session list (not cwd-scoped) would preempt this category of concern in future reviews.

Add doc note →

---

Codex P2 — Duplicate lane names not validated

lane-launcher.sh L213–218 (resolve_config)

A config with two entries sharing a name (e.g. two "work" lanes) passes all current validation gates. The consequence at runtime: SESSIONS_JSON is captured once before the iteration loop (by design — a single consistent snapshot). On start against an empty sessions snapshot:

1. First "work" entry — running_session_id("work") returns empty (nothing in snapshot) → launches session A
2. Second "work" entry — running_session_id("work") still returns empty (snapshot is pre-launch) → launches session B

Two sessions share the name work. A subsequent stop runs sort_by(.startedAt) | last and only stops the newer session B; session A is left running. SKILL.md's Gotchas section and context/config.md both document "two lanes must not share a name" / "keep distinct" — the contract is stated. Enforcement is absent.

Severity: low. The operator audience is expected to read the schema contract, and the failure mode (two running sessions, one surviving a stop) is visible rather than silent. The for_each_lane target-validation loop (L376–385) that already rejects unknown lane names would be the natural place to add a uniqueness check.

Minimal fix:

resolve_config() {
  ...
  local n
  n="$(jq -r '(.lanes // []) | length' "$CONFIG")"
  [[ "$n" -gt 0 ]] || { err "lane config has no lanes: $CONFIG"; exit 3; }
  # Duplicate names cause double-launch on start and missed-stop on stop.
  local dupes
  dupes="$(jq -r '[.lanes[].name] | group_by(.) | map(select(length > 1) | .[0]) | .[]' "$CONFIG")"
  [[ -z "$dupes" ]] || { err "lane config has duplicate lane name(s): $dupes"; exit 3; }
}

Fix this →

---

Status of all prior findings

Finding	Status
M1 — Flag squashing (check_optarg)	✅ Fixed (f889c94)
M2 — startedAt field verification	✅ Fixed + doc gap closed (6449e32)
M3 — Partial launch exits 0	✅ Fixed (f889c94)
L1 — claude stop exit code discarded	✅ Fixed (f889c94)
L2 — Array length idiom (lane_scope)	✅ Fixed (f889c94)
L3 — --agents-json not-found gap	✅ Fixed (f889c94)
L3 — -- passthrough gap	✅ Fixed (f889c94)
Codex P1.1 — Live session list failure propagated	✅ Fixed (3240071)
Codex P1.2 — Terminal sessions in lookup	✅ Documented as non-issue (CLI guarantee)
SKILL.md startedAt prose gap	✅ Fixed (6449e32)
Codex P1 (new) — claude agents --json cwd	⬜ Non-issue for --json scripting mode; doc note optional
Codex P2 (new) — Duplicate lane names	⬜ Low — documented contract, not enforced

---

Assessment

6449e32 is a correct, narrowly targeted documentation fix that resolves the last open item from prior review rounds.

The two new Codex findings on the current HEAD: the cwd concern (P1) does not apply to the --json scripting path — no code change required. The duplicate-name concern (P2) is real but low-priority given the documented schema contract and the operator audience.

This PR is ready to merge. The two optional items above (cwd doc note, duplicate-name validation) are improvements worth considering but not blockers.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6449e32506

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 3s —— View job

---

Code Review

• Gather context and understand the request
• Read lane-launcher.sh at HEAD (fd62819) — verify latest fixes
• Read lane-launcher.test.sh — verify test coverage
• Assess outstanding Codex findings (P1: kind filter, P2: duplicate names)
• Compile and post findings

---

What fd62819 changed

fix(claude-ops): reject duplicate lane names in lanes config — directly addresses Codex P2.

lane-launcher.sh L218–227

A jq-driven uniqueness check is added to resolve_config, after the existing length guard:

dupes="$(jq -r '[.lanes[].name | select(. != null)] | group_by(.) | map(select(length > 1) | .[0]) | join(", ")' "$CONFIG")"
[[ -z "$dupes" ]] || {
  err "lane config has duplicate lane names: $dupes (names must be unique): $CONFIG"
  exit 3
}

The jq expression is correct: group_by(.) | map(select(length > 1) | .[0]) collects the first instance of each duplicated name. The comment block (L218–221) explains why this matters — the single-snapshot model means a duplicated name causes double-launch on start and single-stop on stop. The fix is fail-closed (exit 3), consistent with the rest of resolve_config.

Test coverage (lane-launcher.test.sh L123–132): a two-entry config with "work" twice is run through start --dry-run; asserts exit 3 and the duplicate-name message. Correct — the validation runs in resolve_config before load_sessions, so the --agents-json fixture is irrelevant but harmless. ✅

---

Remaining open finding — Codex P1: restrict lane lookups to background sessions

lane-launcher.sh L280–285

running_session_id() {
  local name="$1"
  jq -r --arg n "$name" \
    '[ .[] | select(.name == $n) ] | sort_by(.startedAt) | last | .sessionId // empty' \
    <<<"$SESSIONS_JSON"
}

The select filters by .name only — no .kind filter. If an interactive session is active under a configured lane name (e.g. a hand-opened claude -n work shell), this function returns its sessionId:

• start: skips launching the background lane (thinks it's already running), so the actual loop lane is never started.
• stop/restart: passes that interactive session's ID to claude stop, which the PR description and SKILL.md §Purpose both state will never happen: "a hand-started session (e.g. an interactive work window, or an unrelated PR Babysit) is never stopped by this skill."

The SKILL.md Gotchas section acknowledges this as a footgun ("keep lane names distinct from ad-hoc session names"), but that's a workaround for a code gap — the fix is a one-character jq filter addition, and it closes the gap between the stated guarantee and the actual behavior:

'[ .[] | select(.name == $n and .kind == "background") ] | sort_by(.startedAt) | last | .sessionId // empty'

The existing test fixture already carries an interactive-kind session in AGENTS_RUNNING (sid-other / "PR Babysit"), but the current test only verifies exclusion by name (assert_not_contains "status ignores non-lane sessions" "$out" "sid-other"). A companion test with a configured lane name on an interactive-kind session would cover the new filter. Minimal addition:

# kind-filter: interactive session sharing a lane name must not block start or be stopped
AGENTS_INTERACTIVE_WORK="$TMP/agents-interactive-work.json"
cat >"$AGENTS_INTERACTIVE_WORK" <<'JSON'
[{ "pid": 333, "cwd": "/repo", "kind": "interactive", "startedAt": 200,
   "sessionId": "sid-work-interactive", "name": "work", "status": "busy" }]
JSON
out="$(run_launcher start --dry-run --repo "$REPO" --config "$CONFIG" --agents-json "$AGENTS_INTERACTIVE_WORK" 2>&1)"
assert_contains "interactive same-name session does not block start" "$out" "claude --bg -n work"
assert_not_contains "stop never targets an interactive session" ... "stop sid-work-interactive"

The kind == "background" field is confirmed in the verified CLI surface comment (L14–18: pid, cwd, kind, startedAt, sessionId, name, status) and present in the existing test fixture, so no new CLI surface dependency is introduced.

Fix this →

---

Status of all prior findings

Finding	Status
M1 — Flag squashing (check_optarg)	✅ Fixed (f889c94)
M2 — startedAt field verification + SKILL.md doc	✅ Fixed (f889c94 + 6449e32)
M3 — Partial launch exits 0	✅ Fixed (f889c94)
L1 — claude stop exit code discarded	✅ Fixed (f889c94)
L2 — Array length idiom (lane_scope)	✅ Fixed (f889c94)
L3 — --agents-json not-found + -- passthrough gaps	✅ Fixed (f889c94)
Codex P1.1 — Live session list failure propagated	✅ Fixed (3240071)
Codex P1.2 — Terminal sessions (cwd / --json)	✅ Non-issue for --json scripting mode; documented
Codex P2 — Duplicate lane names	✅ Fixed (fd62819)
Codex P1 (latest) — interactive-session kind filter	⬜ Open — see above

---

What's solid

The fd62819 commit is correct and narrowly targeted. The jq uniqueness expression is right, the comment explains the invariant clearly, and the test exercises the rejection path with an assertable error message.

The one remaining item is the kind filter. Given that the SKILL.md Purpose section makes the "never stops a hand-started interactive session" guarantee explicitly and it's testable with the field already in the verified surface, this is worth closing before merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fd628190aa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 29s —— View job

---

Code Review

• Gather context and understand the request
• Read lane-launcher.sh at HEAD (68d53b3) — verify background-session kind filter fix
• Read lane-launcher.test.sh — verify new test coverage
• Assess remaining open Codex findings (refresh-failure abort, target-before-refresh)
• Compile and post final findings

---

What 68d53b3 changed

fix(claude-ops): match lanes only to background sessions — directly addresses the Codex P1 "Restrict lane matches to background sessions" finding.

Background-kind filter ✅ Fixed

lane-launcher.sh L287–291

running_session_id() {
  local name="$1"
  jq -r --arg n "$name" \
    '[ .[] | select(.name == $n and .kind == "background") ] | sort_by(.startedAt) | last | .sessionId // empty' \
    <<<"$SESSIONS_JSON"
}

The filter now requires .kind == "background" alongside the name match. The comment block at L280–286 explains the invariant clearly: all real lanes are launched with --bg, so a background-kind filter can only exclude non-lane sessions (like an interactive work window), never a live lane. This makes the filter safe against false negatives.

Test coverage — 6 new cases at L288–301 cover the interactive-namesake scenario with a dedicated fixture (AGENTS_INTERACTIVE_WORK, L74–80 — an interactive-kind session named "work"):

Case	What it verifies
"interactive same-name is not a running lane"	status reports stopped for the lane
"interactive same-name sessionId not shown"	The interactive sid-int-work never appears in status output
"start launches lane despite interactive namesake"	start still emits claude --bg -n work
"start does not skip work for interactive namesake"	start does not print skip work
"stop treats interactive namesake as not running"	stop prints work — not running
"stop never targets the interactive namesake"	claude log contains no sid-int-work

All 6 test the correct behavior end-to-end.

SKILL.md — The "Name is the identity" Gotcha at L93–96 is updated to reflect the kind filter: "Lanes are matched by session name and kind: background". The note about hand-started interactive windows and background sessions with conflicting names is now accurate. ✅

Test count advances from 62 → 68. All cases pass hermetically via the PATH-stubbed claude/git.

---

Remaining open Codex findings from the prior round

These two findings landed on commit fd628190 and are not addressed in 68d53b3.

Low — Refresh failure does not abort lane launch

lane-launcher.sh L477–484

action_start() {
  local rc=0
  info "== lanes: start =="
  refresh_repo_and_plugins || rc=1   # sets rc=1, but does NOT abort
  info "lanes:"
  for_each_lane _start_one || rc=1   # runs regardless
  return "$rc"
}

When git pull --ff-only or claude plugin marketplace update fails, rc is set to 1 but the sweep continues. Lanes are launched from stale state; the script exits 1 at the end. For a tool whose documented contract is "refresh then launch," silently proceeding on a dirty or diverged repo may be surprising — particularly if the user runs restart expecting a clean re-seed. The --no-pull/--no-update escape hatches exist for intentional bypass. The fix is a guard: refresh_repo_and_plugins || return 1 (or exit 4). Low severity given the operator audience and the fact that the failure is at least surfaced via exit code.

Fix this →

Low — Target validation runs after refresh mutations

lane-launcher.sh L486–493

action_restart() {
  local rc=0
  info "== lanes: restart$(lane_scope) =="
  refresh_repo_and_plugins || rc=1   # git pull + marketplace update run FIRST
  info "lanes:"
  for_each_lane _restart_one || rc=1 # unknown-target check is here (L393-401)
  return "$rc"
}

restart does-not-exist runs git pull --ff-only and claude plugin marketplace update before for_each_lane discovers the target is unknown and exits 3. The refresh steps are idempotent and harmless, but the DX is inconsistent — stop does-not-exist also validates late, after nothing has happened. A pre-validation pass on TARGET_LANES before refresh_repo_and_plugins would make the error immediate and avoid mutations for a clearly invalid invocation.

Fix this →

---

Status of all findings

Finding	Status
M1 — Flag squashing (check_optarg)	✅ Fixed (f889c94)
M2 — startedAt field + SKILL.md doc	✅ Fixed (f889c94 + 6449e32)
M3 — Partial launch exits 0	✅ Fixed (f889c94)
L1 — claude stop exit code discarded	✅ Fixed (f889c94)
L2 — Array length idiom (lane_scope)	✅ Fixed (f889c94)
L3 — --agents-json not-found / -- passthrough gaps	✅ Fixed (f889c94)
Codex P1.1 — Live session list failure propagated	✅ Fixed (3240071)
Codex P1.2 — Terminal sessions in --json	✅ Non-issue (CLI guarantee documented)
Codex P2 — Duplicate lane names	✅ Fixed (fd62819)
Codex P1 — Interactive-session kind filter	✅ Fixed (68d53b3)
Codex P2 — Refresh failure continues to launch	⬜ Open — low severity
Codex P2 — Target validation after refresh	⬜ Open — low severity

---

What's solid

The kind-filter change is the correct and necessary fix for the stated invariant ("a hand-started interactive work window is never stopped by this skill"). The implementation is tight: one added clause in the jq filter, a comment that explains both what the guard does and why it cannot create false negatives, SKILL.md Gotchas updated to match, and 6 targeted test cases. No risk of over-excluding real lanes since all real lanes are by construction kind: background.

The two remaining open items are low severity and represent Codex's final P2 suggestions. This PR is ready to merge on the current evidence. The two open items are worth a follow-up but are not blockers for an operator-only skill with a documented escape hatch (--no-pull/--no-update).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68d53b38c1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 1m 42s —— View job

---

Code Review

• Gather context and understand the request
• Read lane-launcher.sh at HEAD (18c3465) — verify Codex P1 preflight fix
• Read lane-launcher.test.sh — verify test coverage
• Assess remaining open items (refresh failure abort, target-before-refresh)
• Compile and post findings

---

What 18c3465 changed

fix(claude-ops): preflight lane restart inputs before stopping — directly addresses the Codex P1 finding about validating inputs before stopping a running lane.

Codex P1 — Preflight before stop ✅ Fixed

lane-launcher.sh L323–337, L444–460

A validate_launch_inputs function is extracted (L323–337) that checks prompt exists, is non-empty, and effort is in the valid set — returning 1 with a clear per-lane error on the first failure. launch_lane now delegates to it (L341) rather than doing the checks inline.

The key change is in _restart_one (L444–460):

_restart_one() {
  local name="$1" model="$2" effort="$3" prompt_path="$4"
  # Preflight the launch inputs BEFORE stopping...
  validate_launch_inputs "$name" "$effort" "$prompt_path" || return 1
  stop_lane_if_running "$name"
  local s=$?
  if [[ $s -ne 0 && $s -ne 2 ]]; then
    err "  $name — stop failed; not relaunching (would duplicate the session name)"
    return 1
  fi
  info "  start $name…"
  launch_lane "$name" "$model" "$effort" "$prompt_path"
}

validate_launch_inputs fires before stop_lane_if_running, so a recoverable prompt or effort error never takes down a healthy running lane. The comment is accurate, and the s -ne 0 && s -ne 2 guard from the previous round correctly distinguishes a "was not running" (benign) from a real stop failure.

Minor observation (not a bug): launch_lane itself also calls validate_launch_inputs at L341, so validation runs twice in the happy restart path — once as preflight, once inside launch_lane. This is harmless since nothing changes between the two calls, but it is redundant. Could be simplified by calling launch_lane directly after the preflight passes, or by having _restart_one not call validate_launch_inputs separately (trusting launch_lane's internal call). For the restart case the preflight semantics are the point — the double-call is intentional and correct.

Test coverage (lane-launcher.test.sh L274–290):

# restart-badprompt.json: work → missing.md (file does not exist)
# AGENTS_RUNNING: work is running as sid-work-1
out="$(run_launcher restart work --repo … --config "$TMP/restart-badprompt.json" --agents-json "$AGENTS_RUNNING" 2>&1)"
assert_eq "restart with a bad prompt exits non-zero" 1 "$rc"
assert_contains "restart bad-prompt error surfaced" "$out" "prompt file not found"
assert_not_contains "restart does not stop the healthy running lane" "$log" "stop sid-work-1"

This is exactly the right gate: AGENTS_RUNNING has work running as sid-work-1, but stop sid-work-1 never appears in the stub log — the preflight fires before any claude stop call. The three assertions together prove the full contract: exits non-zero, error is surfaced, healthy session untouched.

---

Status of all prior findings

Finding	Status
M1 — Flag squashing (check_optarg)	✅ Fixed (f889c94)
M2 — startedAt field + SKILL.md doc	✅ Fixed (f889c94 + 6449e32)
M3 — Partial launch exits 0	✅ Fixed (f889c94)
L1 — claude stop exit code discarded	✅ Fixed (f889c94)
L2 — Array length idiom (lane_scope)	✅ Fixed (f889c94)
L3 — --agents-json not-found + -- passthrough gaps	✅ Fixed (f889c94)
Codex P1.1 — Live session list failure propagated	✅ Fixed (3240071)
Codex P1.2 — Terminal sessions in --json	✅ Non-issue (CLI guarantee documented)
Codex P2 — Duplicate lane names	✅ Fixed (fd62819)
Codex P1 — Interactive-session kind filter	✅ Fixed (68d53b3)
Codex P1 — Preflight before stop in restart	✅ Fixed (18c3465)
Codex P2-a — Refresh failure continues to launch	⬜ Open — low severity
Codex P2-b — Target validation after refresh	⬜ Open — low severity

---

Carry-forward low-priority items (unchanged)

Both were assessed as non-blocking in the prior review round.

Codex P2-a — Refresh failure does not abort lane launch (lane-launcher.sh L488–495):

refresh_repo_and_plugins || rc=1 sets rc but the lane sweep always proceeds. A diverged or dirty checkout will still launch lanes from stale state. The --no-pull/--no-update escape hatches allow intentional bypass. Low severity because the failure is surfaced via exit code and the operator audience is expected to check it.

Codex P2-b — Target validation runs after refresh (lane-launcher.sh L497–504):

restart does-not-exist runs git pull --ff-only and claude plugin marketplace update before for_each_lane discovers the target is unknown and exits 3. Mutations happen before the error. Low severity — the mutations are idempotent.

---

Test count

68 → 71 cases (3 new assertions in the Codex P1 preflight section, verifying exit status, error message, and stop suppression).

---

Assessment

18c3465 is correct and well-tested. The Codex P1 preflight finding is cleanly resolved: validate_launch_inputs is extracted into a reusable function, called before stop_lane_if_running in _restart_one, and the test proves the stop is suppressed when inputs fail. No new issues are introduced.

All medium findings from the review series are resolved. The two remaining low-priority items are unchanged and non-blocking for an operator-only skill with documented escape hatches. This PR is ready to merge.