diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index c7c33cd..9c9d125 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -18,7 +18,7 @@ "name": "work-system", "source": "./plugins/work-system", "description": "Generic task and worktree workflow system for Claude Code. Manage tasks as markdown files, run them in isolated git worktrees with a choice of worker agent (Claude, codex, or grok), and track progress across the define/kickoff/continue/status/close lifecycle.", - "version": "1.9.0" + "version": "1.9.1" }, { "name": "pr-flow", diff --git a/.claude/knowledge/features/herdr-kickoff-automation.md b/.claude/knowledge/features/herdr-kickoff-automation.md index 88aec61..10f144b 100644 --- a/.claude/knowledge/features/herdr-kickoff-automation.md +++ b/.claude/knowledge/features/herdr-kickoff-automation.md @@ -1,10 +1,10 @@ --- title: "herdr /kickoff + /continue-reopen Automation" createdAt: 2026-06-24 -updatedAt: 2026-07-18 +updatedAt: 2026-07-19 createdFrom: "PR #17" -updatedFrom: "session: 2026-07-18 (kickoff agent selection)" -pluginVersion: 1.9.0 +updatedFrom: "session: 2026-07-19 (surface herdr launch errors)" +pluginVersion: 1.9.1 prime: false reindexedAt: 2026-07-12 --- @@ -53,6 +53,64 @@ truth; this entry captures the durable design and one non-obvious gotcha. unrelated, workspace), and both `herdr` and `python3` are on `PATH`. `--no-focus` keeps the kickoff session in front; any failure (empty `$pane`) degrades to the unchanged manual block — never block kickoff on herdr. +- **Failure diagnostics surface herdr's own error (1.9.1).** Every herdr call on a + failure path (`agent start`, `pane move --new-tab`, resume's `tab create`, resume's + `pane run "claude -c"`) now captures stderr instead of `2>/dev/null` and runs it + through a shared `herdr_diag` helper: parse herdr's `{"error":{"code","message"}}` + JSON defensively (any exception falls back to the raw stderr text, never a + traceback), print it, then the existing generic message stays as the last-resort + line. This was diagnosability-only (no fallback/self-heal logic — actively rejected, + see below) after a real incident where the launch failed with only "herdr agent + start did not return a pane id" while herdr's stderr — discarded by the old + `2>/dev/null` — had named the exact cause. When the parsed `code` is + `agent_placement_not_found` (or the message otherwise names the workspace target), + `herdr_diag` appends one hint line pointing at a stale `$HERDR_WORKSPACE_ID` — the + env var is frozen at Claude-spawn time (see the `resume`/`reused` discussion below) + and never refreshed, so a herdr server restart / `update --handoff` that reassigns + workspace ids strands it. Stdout contract (`pane=`/`tab=`/`moved=`/… key=value + lines, exit codes) is untouched — only stderr got richer. + A swarm review of this change (external-only: codex + grok) caught three + refinements before merge, all applied: (1) the stale-workspace hint is scoped by + a `ws_relevant` flag on `herdr_diag` — only `agent start`/`tab create` actually + send `--workspace`, so `pane move`/`pane run` never get an + `agent_placement_not_found` misattributed to the workspace id; (2) the message + fallback (no parseable `code`) requires the workspace id to appear as a + bounded token, not a loose substring (`ws=w1` no longer false-positives on a + message naming `w12`); (3) `herdr_diag` strips control/escape bytes from + herdr's stderr before it reaches the terminal — herdr's stderr is untrusted + server output, so an embedded ANSI/OSC sequence must never be interpreted by + the user's terminal. **The diagnostic is only as useful as the skill layer + that relays it** — kickoff/continue SKILL.md's failure branches now explicitly + instruct relaying the helper's stderr to the user (they didn't before, which + would have silently dropped this entire improvement at exactly the layer the + original incident was observed at: the model narrating only the generic + guard message, never the captured herdr error). + **A second swarm-review round on that first-round fix found it was still + wrong in four ways** — sanitization/anchoring code is exactly the kind of + code whose own fix deserves an adversarial pass, not just the original + feature: (1) control bytes were stripped only from the RAW stderr blob, but a + JSON ``-style escape is still plain printable text at that point — only + after `python3`'s `json.load` decodes it does it become a real ESC byte, so + `code`/`message` need their own strip pass post-decode; (2) the "workspace id + as a bounded token" fix from round one checked token-presence and keyword- + presence as independent ANDs, so `"workspace w1 is healthy; agent placement + is unavailable"` still false-triggered — a fixed character window wasn't + enough either (a short sentence puts the keyword in range regardless), so the + check now splits the message on `;`/`.`/`,` and requires both in the SAME + clause; (3) the ERE-escaping of `$ws` before embedding it in a `grep -E` + pattern only escaped `. [ \ * ^ $`, leaving `+ ? ( ) { } |` live — a + workspace id containing one of those could match unrelated text as a regex, + not a literal; (2) and (3) together made the bash sed/grep chain fragile + enough that it was replaced with a single `python3 re`-based check + (`re.escape` + per-clause matching, case-insensitive) instead of iterating + the bash version further; (4) `$HERDR_WORKSPACE_ID` itself (interpolated into + the hint line) was never sanitized — only herdr's own stderr was. All four + fixed, plus: the orphaned-tab `herdr tab close` cleanup call's own stderr was + being discarded (`2>&1 >/dev/null || true`) even though the CHANGELOG claimed + every failing call surfaces stderr — now captured too. `test_herdr_launch.py` + (new, mirrors `test_agent_registry.py`'s stub-the-CLI-on-PATH pattern) locks + in all of this — every case above, plus the stdout contract, as regression + coverage the first round shipped without. ## `resume` mode: reopen a task tab a `/exit` closed diff --git a/CHANGELOG.md b/CHANGELOG.md index e7f2646..808ef4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,9 @@ entries are grouped per plugin, newest first. ## work-system +### 1.9.1 — 2026-07-20 +- `herdr-launch.sh` no longer swallows herdr's own error on a launch/resume failure (`2>/dev/null`). The `agent start`, `pane move`, `tab create`, and `pane run` calls (plus the `tab close` orphan-cleanup that can run after a malformed `tab create`) now capture stderr, strip control/escape bytes both before AND after JSON-decoding a message (a ``-style escape only becomes a real control byte once decoded), parse herdr's `{"error":{"code","message"}}` JSON defensively (raw text as fallback, never a traceback), and print it before the existing generic diagnostic — `pane run`'s failure line now follows the same before-the-generic-message order as the other call sites. When the error names an invalid workspace placement (`agent_placement_not_found`, or the message names the sanitized workspace id as a bounded token within the same clause as "not found"/"placement", case-insensitively), a one-line actionable hint is appended pointing at a stale `HERDR_WORKSPACE_ID` — scoped to the two calls that actually send `--workspace` (`agent start`, `tab create`), since `pane move`/`pane run` can't have a workspace-id problem. `kickoff`/`continue` SKILL.md now explicitly instruct relaying the helper's stderr diagnostic to the user on every failure path, so the richer diagnostic actually reaches the user instead of staying in the tool output — worded per call site, since the stale-workspace hint can only ever appear on the `agent start`/`tab create` paths. Diagnostics only — stdout key=value lines, exit codes, and `moved`/`reused`/`resumed`/`focused` semantics are unchanged. Covered by new `test_herdr_launch.py`. + ### 1.9.0 — 2026-07-17 - `/kickoff` no longer hardcodes Claude as the worktree worker. With no flag it launches the repo's **default** agent — a single committed per-project setting (`.claude/work-system-agent`); if none is set, `/kickoff` shows a picker and offers to save your choice as that default. No global default and no shipped fallback. Override per run with a flag: `--fable`/`--opus`, `--codex`/`--sol`, `--grok`, `--agent `, or `--pick` (force the picker). New `scripts/agent-registry.sh` is the single source of truth (registry-driven aliases, the project `default get`/`set`, and an availability probe — CLI install + auth, plus model-level for grok via `grok models` so a model the CLI no longer offers reads as unavailable instead of failing at launch); `herdr-launch.sh` execs the resolved worker argv instead of a hardcoded `claude`. Non-Claude workers degrade honestly: they get a bootstrap prompt (read `TASK.md`, drive to a PR) instead of `/continue`, `/close` teardown stays CLI-agnostic, and `/continue`'s reopen documents the per-CLI resume command. Covered by `test_agent_registry.py`. diff --git a/plugins/work-system/.claude-plugin/plugin.json b/plugins/work-system/.claude-plugin/plugin.json index c12d67b..2b5a720 100644 --- a/plugins/work-system/.claude-plugin/plugin.json +++ b/plugins/work-system/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "work-system", "description": "Generic task and worktree workflow system for Claude Code. Manage tasks as markdown files, run them in isolated git worktrees with a choice of worker agent (Claude, codex, or grok), and track progress across the define/kickoff/continue/status/close lifecycle.", - "version": "1.9.0", + "version": "1.9.1", "author": { "name": "gering" }, diff --git a/plugins/work-system/scripts/herdr-launch.sh b/plugins/work-system/scripts/herdr-launch.sh index df3cbc1..38bb085 100755 --- a/plugins/work-system/scripts/herdr-launch.sh +++ b/plugins/work-system/scripts/herdr-launch.sh @@ -103,6 +103,13 @@ command -v python3 >/dev/null 2>&1 || { echo "python3 not on PATH" >&2; exit 1; } [ -d "$worktree" ] || { echo "worktree dir not found: $worktree" >&2; exit 1; } +# The four herdr-call stderr-capture tempfiles below are each cleaned up with an +# explicit `rm -f` on their own success/failure branches, but a SIGINT/SIGTERM +# between their `mktemp` and that `rm -f` would otherwise leak the file — matching +# the existing convention in herdr-tab-glyph.sh. `${var:-}` tolerates a variable +# that isn't assigned yet (or on a code path that never used it) under `set -u`. +trap 'rm -f "${start_err:-}" "${move_err:-}" "${create_err:-}" "${run_err:-}" "${close_err:-}"' EXIT + # Stamp the task's CURRENT state glyph onto the sidebar label (○ ● ◇ ◆ ✓ — the # same mapping the [ws …] statusline renders; ws-statusline.sh is the single # source, applied via herdr-tab-glyph.sh). Best-effort: any failure keeps the @@ -147,6 +154,90 @@ try: print(pane + "|" + tab) except Exception: print("|")' +# error diagnostics: herdr emits {"error":{"code":…,"message":…}} on stderr for +# every failed call above. Extract it defensively — any exception (not JSON, no +# "error" key, …) yields nothing, never a traceback on the user's terminal. +extract_herdr_error='import sys, json +try: + err = json.load(sys.stdin)["error"] + print((err.get("code") or "") + "|" + (err.get("message") or "")) +except Exception: + pass' + +# stale_ws_check — read a message on stdin, print 1 if $ws names +# the failing placement target, else 0. Bounded-token match (not a bare +# substring — ws=w1 must not match a message naming w12) with the "not +# found"/"placement" keyword required in the SAME clause as the token (split on +# ;/./,), not merely present anywhere else in the message — a fixed character +# window is not enough: "workspace w1 is healthy; agent placement is +# unavailable" puts the keyword within any reasonable window of a short +# sentence's token even though the two halves are unrelated clauses. +# Case-insensitive ("Not Found" from herdr must still match). python3's +# re.escape covers every ERE metacharacter — a hand-rolled sed/grep escape +# class is exactly the kind of thing that quietly misses one (`+ ? ( ) { } |` +# are the ones a naive `. [ \ * ^ $` class leaves live). +stale_ws_check='import sys, re +ws = sys.argv[1] if len(sys.argv) > 1 else "" +msg = sys.stdin.read() +if not ws: + print(0); sys.exit(0) +tok_re = re.compile(r"(? — turn a captured herdr +# stderr blob into one-or-two diagnostic lines: herdr's error.code/message when +# the blob parses as the JSON error schema, else the raw text relayed verbatim +# (trimmed). Prints nothing for an empty blob. Control/escape bytes are stripped +# TWICE — once from the raw blob, once from the code/message pulled out of it — +# because a JSON ``-style escape is still plain printable text before +# python3's json.load decodes it into a real ESC byte; stripping only the raw +# blob lets a compromised herdr's escaped control sequence survive decoding and +# reach the terminal, defeating the whole point of the filter. +# gates the stale-$HERDR_WORKSPACE_ID hint: pass 1 only for calls +# that actually send `--workspace $ws` (agent start, tab create) — for calls that +# don't (pane move, pane run) an agent_placement_not_found can't be a workspace-id +# problem, so the hint would misattribute the cause. $ws itself is sanitized +# before it's ever interpolated into the printed hint (it comes from +# $HERDR_WORKSPACE_ID, an environment value this script doesn't control). +# Never more than one hint line, so the diagnostic doesn't nag. +herdr_diag() { + local raw ws ws_relevant clean parsed code msg line stale ws_hit ws_clean + raw="$1" + ws="$2" + ws_relevant="${3:-1}" + [ -n "$raw" ] || return 0 + clean="$(printf '%s' "$raw" | tr -dc '[:print:]\t\n')" + parsed="$(printf '%s' "$clean" | python3 -c "$extract_herdr_error" 2>/dev/null || true)" + if [ -n "$parsed" ]; then + code="$(printf '%s' "${parsed%%|*}" | tr -dc '[:print:]\t\n')" + msg="$(printf '%s' "${parsed#*|}" | tr -dc '[:print:]\t\n')" + line="herdr error" + [ -n "$code" ] && line="$line code=$code" + [ -n "$msg" ] && line="$line: $msg" + else + code="" + msg="$clean" + line="herdr error: $(printf '%s' "$clean" | tr '\n' ' ' | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//')" + fi + stale=0 + if [ "$ws_relevant" = 1 ]; then + case "$code" in + agent_placement_not_found) stale=1 ;; + esac + if [ "$stale" != 1 ] && [ -n "$ws" ]; then + ws_hit="$(printf '%s' "$msg" | python3 -c "$stale_ws_check" "$ws" 2>/dev/null || echo 0)" + [ "$ws_hit" = 1 ] && stale=1 + fi + fi + if [ "$stale" = 1 ]; then + ws_clean="$(printf '%s' "$ws" | tr -dc '[:print:]\t\n')" + line="$line +HERDR_WORKSPACE_ID=$ws_clean is not a valid workspace on this herdr server (likely a stale id after a herdr restart/handoff) — relaunch from a live session or start the worker manually." + fi + printf '%s\n' "$line" +} case "$mode" in launch) @@ -193,21 +284,39 @@ EOF_RESOLVE [ ${#worker_argv[@]} -gt 0 ] || { echo "agent-registry resolved no argv for $selector" >&2; exit 1; } fi - # Spawn the worker as argv and read back the pane id. + # Spawn the worker as argv and read back the pane id. stderr is captured (not + # discarded) so a failure can surface herdr's actual error.code/message instead + # of only the generic "no pane id" guard below. + start_err="$(mktemp)" start_json="$(herdr agent start "$label" --workspace "$workspace" \ - --cwd "$worktree" --no-focus -- "${worker_argv[@]}" 2>/dev/null || true)" + --cwd "$worktree" --no-focus -- "${worker_argv[@]}" 2>"$start_err" || true)" pane="$(printf '%s' "$start_json" | python3 -c "$extract_agent_pane" 2>/dev/null || true)" # Empty pane id → the agent did not start (broken socket / bad response). - [ -n "$pane" ] || { echo "herdr agent start did not return a pane id" >&2; exit 1; } + # Print herdr's own error first when captured — the generic message stays as + # the last-resort fallback for a truly pane-less/malformed response. + if [ -z "$pane" ]; then + diag="$(herdr_diag "$(cat "$start_err")" "$workspace" 1)" + rm -f "$start_err" + [ -n "$diag" ] && printf '%s\n' "$diag" >&2 + echo "herdr agent start did not return a pane id" >&2 + exit 1 + fi + rm -f "$start_err" # caller's tab). If the move fails, the worker is still running — report it in # place rather than claiming a tab that does not exist. `agent=` tells the # caller which CLI×model was launched; the tab uses the glyph-stamped label. - if move_json="$(herdr pane move "$pane" --new-tab --label "$tab_label" --no-focus 2>/dev/null)"; then + move_err="$(mktemp)" + if move_json="$(herdr pane move "$pane" --new-tab --label "$tab_label" --no-focus 2>"$move_err")"; then tab="$(printf '%s' "$move_json" | python3 -c "$extract_moved_tab" 2>/dev/null || true)" + rm -f "$move_err" printf 'pane=%s\ntab=%s\nmoved=yes\nagent=%s\n' "$pane" "$tab" "$agent_name" else + diag="$(herdr_diag "$(cat "$move_err")" "$workspace" 0)" + rm -f "$move_err" + [ -n "$diag" ] && printf '%s\n' "$diag" >&2 + echo "herdr pane move --new-tab failed for pane $pane (worker still running; no dedicated tab)" >&2 printf 'pane=%s\ntab=\nmoved=no\nagent=%s\n' "$pane" "$agent_name" fi ;; @@ -262,8 +371,9 @@ EOF_RESOLVE # (shell) pane id and tab id in one python3 pass. Split on the FIRST `|`, so an # empty pane id (with a present tab id) stays empty and trips the guard below, # rather than the tab id being mis-read as the pane id. + create_err="$(mktemp)" create_json="$(herdr tab create --workspace "$workspace" \ - --cwd "$worktree" --label "$tab_label" 2>/dev/null || true)" + --cwd "$worktree" --label "$tab_label" 2>"$create_err" || true)" pane_tab="$(printf '%s' "$create_json" | python3 -c "$extract_root_pane_tab" 2>/dev/null || true)" pane="${pane_tab%%|*}" tab="${pane_tab#*|}" @@ -272,12 +382,25 @@ EOF_RESOLVE # socket / bad JSON / pane-less result). Cannot run claude -c without a pane. If a # tab id WAS parsed (a pane-less/partial result from schema drift), the tab is real # and would be orphaned — close it before bailing so a drifted response can't leak a - # blank tab on every resume; then the caller shows the manual block. + # blank tab on every resume; then the caller shows the manual block. herdr's own + # error is printed first when captured — the generic message stays as the + # last-resort fallback. if [ -z "$pane" ]; then - [ -n "$tab" ] && herdr tab close "$tab" >/dev/null 2>&1 || true + if [ -n "$tab" ]; then + close_err="$(mktemp)" + if ! herdr tab close "$tab" >/dev/null 2>"$close_err"; then + close_diag="$(herdr_diag "$(cat "$close_err")" "$workspace" 0)" + [ -n "$close_diag" ] && printf 'herdr tab close cleanup for orphaned tab %s also failed: %s\n' "$tab" "$close_diag" >&2 + fi + rm -f "$close_err" + fi + diag="$(herdr_diag "$(cat "$create_err")" "$workspace" 1)" + rm -f "$create_err" + [ -n "$diag" ] && printf '%s\n' "$diag" >&2 echo "herdr tab create did not return a pane id" >&2 exit 1 fi + rm -f "$create_err" # Run `claude -c` INSIDE the shell pane — the /exit hardening (a later /exit # returns to the shell, keeping the tab alive). Prefix an explicit `cd ` @@ -290,10 +413,14 @@ EOF_RESOLVE # the user must run it by hand. (Catches a failed send, not `claude -c` erroring # later on a cwd with no prior session — the caller's wording stays tentative.) resumed=yes - if ! herdr pane run "$pane" "cd $(printf '%q' "$worktree") && claude -c" >/dev/null 2>&1; then + run_err="$(mktemp)" + if ! herdr pane run "$pane" "cd $(printf '%q' "$worktree") && claude -c" >/dev/null 2>"$run_err"; then resumed=no + diag="$(herdr_diag "$(cat "$run_err")" "$workspace" 0)" + [ -n "$diag" ] && printf '%s\n' "$diag" >&2 echo "herdr pane run could not start 'claude -c' in $pane (tab is open; run it by hand)" >&2 fi + rm -f "$run_err" # Focus the reopened tab — unlike kickoff's background launch, the user is # switching to it now. Report whether the focus took, so the caller doesn't diff --git a/plugins/work-system/scripts/test_herdr_launch.py b/plugins/work-system/scripts/test_herdr_launch.py new file mode 100644 index 0000000..0ca71d9 --- /dev/null +++ b/plugins/work-system/scripts/test_herdr_launch.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +"""Tests for herdr-launch.sh's error-diagnostics path (herdr_diag) — run +standalone (`python3 test_herdr_launch.py`) or via check-structure.py's +"plugin tests" check. + +A fake `herdr` binary on PATH returns canned JSON/text on stderr for each +subcommand, so these tests exercise the REAL herdr-launch.sh end to end (not +herdr_diag in isolation) against the exact integration surface skills call. +Covers: the JSON error-schema extraction, the ws_relevant-gated + clause- +bounded stale-workspace hint (the real w9 incident, the "unrelated clause" +false-positive a loose AND-of-substrings would trip, case-insensitivity, and +an ERE-metachar workspace id), the double control-byte strip (pre- AND +post-JSON-decode, plus $ws itself), the tab-close cleanup diagnostic, and that +the stdout key=value contract is untouched by any of this on the success path. +""" +import json as jsonlib +import os +import subprocess +import sys +import tempfile +from pathlib import Path + +HERE = Path(__file__).parent +SCRIPT = HERE / "herdr-launch.sh" + +FAILS = [] + + +def check(name, cond): + if not cond: + FAILS.append(name) + + +def shquote(s): + return "'" + s.replace("'", "'\\''") + "'" + + +def herdr_stub(cases): + """Build a fake `herdr` script from {" ": (stdout, stderr, exit)}.""" + lines = ["#!/usr/bin/env bash", 'case "$1 $2" in'] + for key, (out, err, rc) in cases.items(): + lines.append(f' "{key}")') + if out: + lines.append(f" printf '%s' {shquote(out)}") + if err: + lines.append(f" printf '%s' {shquote(err)} >&2") + lines.append(f" exit {rc}") + lines.append(" ;;") + lines.append(' *) echo "unhandled herdr stub call: $*" >&2; exit 9 ;;') + lines.append("esac") + return "\n".join(lines) + + +class Env: + """A throwaway PATH with a fake `herdr` stub, plus a worktree dir for + herdr-launch.sh to target.""" + + def __init__(self, cases): + self.tmp = tempfile.TemporaryDirectory() + root = Path(self.tmp.name) + self.worktree = root / "wt" + self.worktree.mkdir() + bindir = root / "bin" + bindir.mkdir() + herdr = bindir / "herdr" + herdr.write_text(herdr_stub(cases)) + herdr.chmod(0o755) + self.env = dict(os.environ) + self.env["PATH"] = f"{bindir}:{self.env['PATH']}" + + def run(self, *args): + return subprocess.run( + ["bash", str(SCRIPT), *args], + env=self.env, capture_output=True, text=True, timeout=20, + ) + + def close(self): + self.tmp.cleanup() + + +# --- the real incident: agent_placement_not_found names the workspace ------ # +e = Env({"agent start": ("", jsonlib.dumps( + {"error": {"code": "agent_placement_not_found", + "message": "agent placement target w9 not found"}}), 1)}) +r = e.run("launch", "t", str(e.worktree), "w9") +check("incident: exit 1", r.returncode == 1) +check("incident: code/message shown", + "agent_placement_not_found" in r.stderr and "target w9 not found" in r.stderr) +check("incident: stale hint shown", + "HERDR_WORKSPACE_ID=w9 is not a valid workspace" in r.stderr) +check("incident: generic last-resort message still present", + "did not return a pane id" in r.stderr) +check("incident: no stdout on failure", r.stdout == "") +e.close() + +# --- token present + keyword present, but in UNRELATED clauses -> no hint -- # +e = Env({"agent start": ("", jsonlib.dumps( + {"error": {"code": "some_code", + "message": "workspace w1 is healthy; agent placement is unavailable"}}), 1)}) +r = e.run("launch", "t", str(e.worktree), "w1") +check("unrelated clauses: no stale hint", "is not a valid workspace" not in r.stderr) +check("unrelated clauses: code/message still shown", "some_code" in r.stderr) +e.close() + +# --- case-insensitive match ------------------------------------------------ # +e = Env({"agent start": ("", jsonlib.dumps( + {"error": {"code": "weird", "message": "Workspace w1 Not Found on server"}}), 1)}) +r = e.run("launch", "t", str(e.worktree), "w1") +check("case-insensitive: stale hint shown", "HERDR_WORKSPACE_ID=w1 is not a valid" in r.stderr) +e.close() + +# --- ERE-metachar workspace id must not false-match ------------------------ # +e = Env({"agent start": ("", jsonlib.dumps( + {"error": {"code": "other", "message": "aaab placement not found somewhere"}}), 1)}) +r = e.run("launch", "t", str(e.worktree), "a+b") +check("ere-metachar: no false stale hint", "is not a valid workspace" not in r.stderr) +e.close() + +# --- ws_relevant=0 (pane move) never gets the hint, even with the trigger code # +e = Env({ + "agent start": (jsonlib.dumps({"result": {"agent": {"pane_id": "w1:p5"}}}), "", 0), + "pane move": ("", jsonlib.dumps( + {"error": {"code": "agent_placement_not_found", "message": "pane target gone"}}), 1), +}) +r = e.run("launch", "t", str(e.worktree), "w1") +check("pane move: exit 0 (moved=no is not a hard failure)", r.returncode == 0) +check("pane move: code/message shown", "agent_placement_not_found" in r.stderr) +check("pane move: no stale hint (ws not relevant here)", "is not a valid workspace" not in r.stderr) +check("pane move: moved=no on stdout", "moved=no" in r.stdout) +check("pane move: pane= still reported", "pane=w1:p5" in r.stdout) +e.close() + +# --- control bytes stripped AFTER json-decoding too (not just the raw blob) # +esc_payload = jsonlib.dumps({"error": {"code": "x", "message": "hi \x1b[31mRED\x1b[0m end"}}) +e = Env({"agent start": ("", esc_payload, 1)}) +r = e.run("launch", "t", str(e.worktree), "w1") +check("json-escaped ESC stripped post-decode", "\x1b" not in r.stderr) +check("surrounding text preserved", "RED" in r.stderr and "end" in r.stderr) +e.close() + +# --- $HERDR_WORKSPACE_ID itself is sanitized before interpolation ---------- # +evil_ws = "w1\x1bevil" +e = Env({"agent start": ("", jsonlib.dumps( + {"error": {"code": "agent_placement_not_found", "message": "target gone"}}), 1)}) +r = e.run("launch", "t", str(e.worktree), evil_ws) +check("evil $ws: ESC stripped from the printed hint", "\x1b" not in r.stderr) +check("evil $ws: remaining text still present", "evil" in r.stderr) +e.close() + +# --- tab-close cleanup failure surfaces its own diagnostic (resume path) --- # +e = Env({ + "pane list": (jsonlib.dumps( + {"result": {"panes": [{"tab_id": "w1:t1", "cwd": "/nowhere"}]}}), "", 0), + "tab create": (jsonlib.dumps({"result": {"tab_id": "w1:t9"}}), "", 0), + "tab close": ("", jsonlib.dumps( + {"error": {"code": "tab_not_found", "message": "tab w1:t9 already gone"}}), 1), +}) +r = e.run("resume", "t", str(e.worktree), "w1") +check("tab-close cleanup: exit 1", r.returncode == 1) +check("tab-close cleanup: close diag surfaced", + "tab_not_found" in r.stderr and "also failed" in r.stderr) +check("tab-close cleanup: create diag also surfaced", + "herdr tab create did not return a pane id" in r.stderr) +e.close() + +# --- success path: stdout contract untouched by any of the above ----------- # +e = Env({ + "agent start": (jsonlib.dumps({"result": {"agent": {"pane_id": "w1:p5"}}}), "", 0), + "pane move": (jsonlib.dumps( + {"result": {"move_result": {"created_tab": {"tab_id": "w1:t9"}}}}), "", 0), +}) +r = e.run("launch", "t", str(e.worktree), "w1") +check("success: exit 0", r.returncode == 0) +check("success: stdout contract unchanged", + r.stdout == "pane=w1:p5\ntab=w1:t9\nmoved=yes\nagent=claude\n") +check("success: no stderr diagnostics", r.stderr == "") +e.close() + + +if FAILS: + print("FAIL:") + for f in FAILS: + print(" -", f) + sys.exit(1) +print("herdr-launch.sh (herdr_diag): all tests passed") diff --git a/plugins/work-system/skills/continue/SKILL.md b/plugins/work-system/skills/continue/SKILL.md index 8308861..611b1e1 100644 --- a/plugins/work-system/skills/continue/SKILL.md +++ b/plugins/work-system/skills/continue/SKILL.md @@ -115,9 +115,15 @@ the prefix-stripped task name) — comparing the raw argument instead misroutes. append: "couldn't confirm the tab focus — switch to it manually (pane ``)." - **exit 0, `reused=no resumed=no`** → the tab opened but `claude -c` could not be sent; tell the user the tab is up at the worktree and to run `claude -c` in it by - hand. + hand. The helper's stderr for this call carries herdr's own error (code/message) + — **relay that stderr text**, not just "run it by hand." (This call never sends + `--workspace`, so unlike the failure below there is no stale-workspace hint to + expect here.) - **non-zero exit** → the helper could not automate (herdr/python3 missing, broken - socket, or no pane id). Show the manual block (b). + socket, or no pane id). **Relay the helper's stderr to the user first** — on a + broken-socket/no-pane-id failure it carries herdr's own error (code/message, and + a stale-workspace hint when applicable), the actual reason this happened, not + just the generic guard text. Then show the manual block (b). Success report for the `reused=no resumed=yes` case (fill `Tab` from the `tab=` line; drop the line if `tab=` is empty): diff --git a/plugins/work-system/skills/kickoff/SKILL.md b/plugins/work-system/skills/kickoff/SKILL.md index 53e15ba..0f25eef 100644 --- a/plugins/work-system/skills/kickoff/SKILL.md +++ b/plugins/work-system/skills/kickoff/SKILL.md @@ -193,7 +193,10 @@ is a per-repo committed file (`.claude/work-system-agent`), set via (`tab=`) with worker `agent=`. Report success (template below). - **exit 0 with `moved=no`** → the worker started but the tab move failed, so it is running as a split in *this* session's tab — tell the user it's here, not in - a new tab. + a new tab. The helper's stderr for this call carries herdr's own error + (code/message) ahead of its own generic line — **relay that stderr text to the + user**, not just "the move failed." (This call never sends `--workspace`, so + unlike the failure below there is no stale-workspace hint to expect here.) - **After a successful launch (either `moved=` value), if `OFFER_DEFAULT=yes`** (the picker path, user chose to save): `bash "$REG" default set ""` (the `agent=` value) to write the repo's committed default. Mention it, and @@ -204,7 +207,10 @@ is a per-repo committed file (`.claude/work-system-agent`), set via `note=`). Report it verbatim (e.g. "codex not ready — run: codex login") and re-offer the picker or another selector. Nothing was spawned. - **other non-zero exit** → the helper could not automate (herdr/python3 missing, - broken socket, or no pane id). Show the manual block (b). + broken socket, or no pane id). **Relay the helper's stderr to the user first** + — on a broken-socket/no-pane-id failure it carries herdr's own error + (code/message, and a stale-workspace hint when applicable), the actual reason + this happened, not just the generic guard text. Then show the manual block (b). Success report (fill `Tab` from the helper's `tab=` line, `Agent` from `agent=`): ```