From 4e76e1e097c5a54faedb4a8f44bac2e0b15bda7b Mon Sep 17 00:00:00 2001 From: dch0202 Date: Fri, 14 Aug 2026 15:56:44 +0900 Subject: [PATCH 1/4] fix(orchestrate): {ORCH_DIR} absolute-path token for worker-facing orchestration artifacts (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Worker-facing templates referenced briefs/plans/reviews via bare `.orchestration/...` relative paths, which resolve to nothing once a worker's cwd is its own worktree. Introduce {ORCH_DIR} (substituted like {STATUS_DIR}) and convert every such reference in session-prompt.md (§1-§4, §O1-§O4) and brief.md to it, including the ad-hoc `$(dirname {STATUS_DIR})/reviews/...` form in §3/§O3. Document the two-convention rule (orchestration artifacts absolute, repo files relative) in SKILL.md next to the brief/plan composition text. Bump the pinned Orca-block cksum in tests/send-prompt.bats to match, per that test's own same-commit-update convention. --- skills/orchestrate/SKILL.md | 10 ++ skills/orchestrate/templates/brief.md | 9 +- .../orchestrate/templates/session-prompt.md | 22 +-- tests/send-prompt.bats | 6 +- tests/session-prompt-paths.bats | 127 ++++++++++++++++++ 5 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 tests/session-prompt-paths.bats diff --git a/skills/orchestrate/SKILL.md b/skills/orchestrate/SKILL.md index e184455..289032d 100644 --- a/skills/orchestrate/SKILL.md +++ b/skills/orchestrate/SKILL.md @@ -391,6 +391,16 @@ reasoning-effort flags) that `worker-start` cannot express. a UI-facing task fill `` with the `design` role's pulled spec (Phase 2) — then + The brief and plan are what the worker reads, not what it writes, so every + reference to them inside a composed prompt uses the `{ORCH_DIR}` token + (absolute path to this run's `.orchestration` dir, substituted like + `{STATUS_DIR}`) — the worker's cwd is its own worktree, which does not + contain `.orchestration/`. Repo files (source, tests, tracked docs) stay + relative to that cwd instead: an absolute repo path would make the worker + edit the main worktree rather than its own. This coordinator's own + `briefs/.md` / `plans/.md` references above stay relative — the + coordinator's cwd is the main repo root. + **2a. Plan it yourself, here, before launching.** Invoke the bundled `wiki-plan` skill for this task and write the result to `plans/.md`. Planning runs in THIS coordinator session on purpose: a worker can be pinned to a cheaper tier diff --git a/skills/orchestrate/templates/brief.md b/skills/orchestrate/templates/brief.md index 24c798c..e3c2be9 100644 --- a/skills/orchestrate/templates/brief.md +++ b/skills/orchestrate/templates/brief.md @@ -1,7 +1,7 @@ # Brief template — XML task brief The orchestrator fills this in per task and writes it to -`.orchestration/briefs/{TASK}.md`. Structure follows the delegation 4-part +`{ORCH_DIR}/briefs/{TASK}.md`. Structure follows the delegation 4-part contract (objective / output / tools / boundaries) plus done + effort, in XML tags so the session can re-recognize each section. Heavy context goes near the top (long-context guidance); the session prompt's one-line trigger cites the @@ -61,8 +61,11 @@ specific tags below as authority. - .orchestration/plans/{TASK}.md — written by the coordinator; adopt, verify against this brief, do not re-author + worker's tier). Orchestration artifacts (this brief, the plan, reviews) live + outside every worktree, so they are always addressed via {ORCH_DIR} (absolute); + repo files the worker edits stay relative to its own worktree cwd — an absolute + repo path would target the main worktree instead of the worker's own. --> + {ORCH_DIR}/plans/{TASK}.md — written by the coordinator; adopt, verify against this brief, do not re-author diff --git a/skills/orchestrate/templates/session-prompt.md b/skills/orchestrate/templates/session-prompt.md index 9a44dfa..eb934d9 100644 --- a/skills/orchestrate/templates/session-prompt.md +++ b/skills/orchestrate/templates/session-prompt.md @@ -6,7 +6,11 @@ SINGLE line (no newlines). `orca orchestration task-create`, one Task per task-phase, so it MAY span multiple lines. Never tell an Orca worker to wait — it reports and ends its turn. -Tokens: `{TASK}` task id · `{STATUS_DIR}` abs path · `{SKILL}` orchestrate skill dir abs path · +Tokens: `{TASK}` task id · `{STATUS_DIR}` abs path · `{ORCH_DIR}` abs path of this run's +`.orchestration` dir — every orchestration artifact (briefs/plans/reviews) is +addressed through this token; repo files (source, tests, tracked docs) stay +relative to the worker's own worktree cwd, since an absolute repo path would +target the main worktree instead · `{SKILL}` orchestrate skill dir abs path · `{INTEG}` integration branch · `{BRANCH}` this session's branch · `{N}` rework round · `{ORCA_TASK_ID}` Orca Task id from `task-create` (§O only) · `{ORCA_DISPATCH_ID}` Orca Dispatch id from `orca-worker-start.sh` (§O only) — the @@ -68,15 +72,15 @@ block — to every §1–§4 prompt, flattened into the single sent line. ## (1) Plan — injected at session launch -You are the session for {TASK}. Treat .orchestration/briefs/{TASK}.md `` as authority — especially ``, ``, and ``. Use the loop-implement skill but STOP after planning. The coordinator has ALREADY run `wiki-plan` and written the plan to .orchestration/plans/{TASK}.md, so take loop-implement step 2's "a plan already exists" path: ADOPT that plan, do not re-plan it. Check it against the brief — every decision actually made (nothing left "as appropriate"), each with its decision->page map entry, and no contradiction with ``, ``, or ``. If it fails any of those, do NOT quietly rewrite it: report the specific gap as a failure and stop, so the coordinator re-plans on the planning model. Otherwise run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} plan_ready worktree=$PWD` and wait for an approval message. Do NOT write implementation code yet. +You are the session for {TASK}. Treat {ORCH_DIR}/briefs/{TASK}.md `` as authority — especially ``, ``, and ``. Use the loop-implement skill but STOP after planning. The coordinator has ALREADY run `wiki-plan` and written the plan to {ORCH_DIR}/plans/{TASK}.md, so take loop-implement step 2's "a plan already exists" path: ADOPT that plan, do not re-plan it. Check it against the brief — every decision actually made (nothing left "as appropriate"), each with its decision->page map entry, and no contradiction with ``, ``, or ``. If it fails any of those, do NOT quietly rewrite it: report the specific gap as a failure and stop, so the coordinator re-plans on the planning model. Otherwise run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} plan_ready worktree=$PWD` and wait for an approval message. Do NOT write implementation code yet. ## (2) Implement — injected after plan approval -Approved. Implement .orchestration/plans/{TASK}.md via the loop-implement skill: respect `` (max 3 retries), write tests first, run them, self-review, and at step 6.5 you MUST call the test-quality-auditor subagent. Never touch anything in ``. Confirm EVERY `` item, then run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} impl_done worktree=$PWD` and wait. Do not commit, push, or PR. +Approved. Implement {ORCH_DIR}/plans/{TASK}.md via the loop-implement skill: respect `` (max 3 retries), write tests first, run them, self-review, and at step 6.5 you MUST call the test-quality-auditor subagent. Never touch anything in ``. Confirm EVERY `` item, then run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} impl_done worktree=$PWD` and wait. Do not commit, push, or PR. ## (3) Rework — injected when review requests changes -Address the issues in .orchestration/reviews/{TASK}-r{N}.md via the loop-implement skill (re-run step 6.5 audit; never weaken or skip tests). Per finding: fix it, or answer its Question with the concrete reason and leave it — either way, append `- **Answer (r{N})** — fixed` or `- **Answer (r{N})** — stands: ` under that finding in the absolute file $(dirname {STATUS_DIR})/reviews/{TASK}-r{N}.md; silence on any finding is not a valid resolution. This obligation binds blocking findings only — answering Non-blocking findings is encouraged, not required. Then run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} impl_done worktree=$PWD` and wait. +Address the issues in {ORCH_DIR}/reviews/{TASK}-r{N}.md via the loop-implement skill (re-run step 6.5 audit; never weaken or skip tests). Per finding: fix it, or answer its Question with the concrete reason and leave it — either way, append `- **Answer (r{N})** — fixed` or `- **Answer (r{N})** — stands: ` under that finding in the absolute file {ORCH_DIR}/reviews/{TASK}-r{N}.md; silence on any finding is not a valid resolution. This obligation binds blocking findings only — answering Non-blocking findings is encouraged, not required. Then run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} impl_done worktree=$PWD` and wait. ## (4) Merge-prep — injected after final approval @@ -102,11 +106,11 @@ prompt here says "wait" — the worker reports and ends its turn. ## (O1) Plan — `--spec` of the plan Task -You are the worker session for {TASK}. Treat .orchestration/briefs/{TASK}.md +You are the worker session for {TASK}. Treat {ORCH_DIR}/briefs/{TASK}.md `` as authority — especially ``, ``, and ``. Use the loop-implement skill but STOP after planning. The coordinator has ALREADY run `wiki-plan` and written the plan to -.orchestration/plans/{TASK}.md, so take loop-implement step 2's "a plan already exists" +{ORCH_DIR}/plans/{TASK}.md, so take loop-implement step 2's "a plan already exists" path: ADOPT that plan, do not re-plan it. Read it against the brief and check it is executable — every design decision actually made (nothing left "as appropriate"), each one carrying its decision->page map entry, and no contradiction with @@ -122,7 +126,7 @@ Then END YOUR TURN. Do NOT write implementation code yet. ## (O2) Implement — `--spec` of the implement Task -Approved. Implement .orchestration/plans/{TASK}.md via the loop-implement skill: respect +Approved. Implement {ORCH_DIR}/plans/{TASK}.md via the loop-implement skill: respect `` (max 3 retries), write tests first, run them, self-review, and at step 6.5 you MUST call the test-quality-auditor subagent. Never touch anything in ``. Confirm EVERY `` item, then run @@ -134,11 +138,11 @@ Then END YOUR TURN. Do not commit, push, or PR — the orchestrator merges. ## (O3) Rework — `--spec` of the rework Task -Address the issues in .orchestration/reviews/{TASK}-r{N}.md via the loop-implement skill +Address the issues in {ORCH_DIR}/reviews/{TASK}-r{N}.md via the loop-implement skill (re-run step 6.5 audit; never weaken or skip tests). Per finding: fix it, or answer its Question with the concrete reason and leave it — either way, append `- **Answer (r{N})** — fixed` or `- **Answer (r{N})** — stands: ` under that finding in the absolute -file $(dirname {STATUS_DIR})/reviews/{TASK}-r{N}.md; silence on any finding is not a +file {ORCH_DIR}/reviews/{TASK}-r{N}.md; silence on any finding is not a valid resolution. This obligation binds blocking findings only — answering Non-blocking findings is encouraged, not required. Then run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} impl_done worktree=$PWD` diff --git a/tests/send-prompt.bats b/tests/send-prompt.bats index e61c278..4abcb0d 100644 --- a/tests/send-prompt.bats +++ b/tests/send-prompt.bats @@ -429,9 +429,13 @@ tpl_sections_single_line() { # Bumped from 4060540920/6077 when §O3 gained the fix-or-answer obligation: # per finding, fix it or answer its Question and leave it, recorded via an # Answer (r{N}) line in the review file; silence is not a valid resolution. + # Bumped from 3269123258/6531 when every `.orchestration/briefs|plans|reviews/` + # reference became `{ORCH_DIR}/...` (issue #87): a worker's cwd is its own + # worktree, which does not contain `.orchestration/`, so the bare relative + # form resolved to nothing. run sh -c "sed -n '/^\*\*Orca substrate\.\*\*/,/^## Subagent usage protocol/p' '$TPL' | cksum" [ "$status" -eq 0 ] - [ "$output" = "3269123258 6531" ] + [ "$output" = "2521059482 6502" ] } @test "template: the Orca ask rule forbids deciding a timed-out question" { diff --git a/tests/session-prompt-paths.bats b/tests/session-prompt-paths.bats new file mode 100644 index 0000000..8f11b23 --- /dev/null +++ b/tests/session-prompt-paths.bats @@ -0,0 +1,127 @@ +#!/usr/bin/env bats +# Tests for the {ORCH_DIR} absolute-path token (issue #87). Orchestration +# artifacts (briefs/plans/reviews) live outside every worker's worktree, so a +# bare `.orchestration/...` reference in a worker-facing template section +# resolves to nothing when a worker's cwd is its own worktree. These tests +# assert every such reference is expressed via {ORCH_DIR} instead, and that +# no bare form (including the ad-hoc `$(dirname {STATUS_DIR})/...` form) is +# left behind in the worker-facing sections. + +setup() { + SP_TEMPLATE="${BATS_TEST_DIRNAME}/../skills/orchestrate/templates/session-prompt.md" + BRIEF_TEMPLATE="${BATS_TEST_DIRNAME}/../skills/orchestrate/templates/brief.md" +} + +extract_section() { + local heading="$1" file="$2" + awk -v h="$heading" ' + index($0, h) == 1 { flag=1; next } + /^## / { flag=0 } + flag { print } + ' "$file" +} + +# Everything from the first numbered/lettered section onward — the text that +# actually gets delivered to a worker (§1-§4, §O1-§O4, and the protocol +# blocks appended to them). Excludes the leading coordinator-facing +# documentation (substrate notes, token legend, delivery mechanics). +worker_facing_region() { + local file="$1" + awk '/^## \(1\)/ { flag=1 } flag { print }' "$file" +} + +# Collapses embedded newlines (and the whitespace runs they leave behind) to +# a single space so a substring assertion survives prose hard-wrapped across +# physical lines (wiki/testing/quality/checks-that-cannot-pass.md: a +# newline-split phrase reads as absent under a raw substring match). +normalize_ws() { + printf '%s' "$1" | tr '\n' ' ' | tr -s ' ' +} + +# --- normal: §1/§O1 (plan) reference both briefs and plans via {ORCH_DIR} --- + +@test "§1: briefs and plans are referenced via {ORCH_DIR}" { + flat="$(normalize_ws "$(extract_section '## (1)' "$SP_TEMPLATE")")" + [[ "$flat" == *'{ORCH_DIR}/briefs/{TASK}.md'* ]] + [[ "$flat" == *'{ORCH_DIR}/plans/{TASK}.md'* ]] +} + +@test "§O1: briefs and plans are referenced via {ORCH_DIR}" { + flat="$(normalize_ws "$(extract_section '## (O1)' "$SP_TEMPLATE")")" + [[ "$flat" == *'{ORCH_DIR}/briefs/{TASK}.md'* ]] + [[ "$flat" == *'{ORCH_DIR}/plans/{TASK}.md'* ]] +} + +# --- normal: §2/§O2 (implement) reference plans via {ORCH_DIR} ------------- + +@test "§2: plan is referenced via {ORCH_DIR}" { + flat="$(normalize_ws "$(extract_section '## (2)' "$SP_TEMPLATE")")" + [[ "$flat" == *'{ORCH_DIR}/plans/{TASK}.md'* ]] +} + +@test "§O2: plan is referenced via {ORCH_DIR}" { + flat="$(normalize_ws "$(extract_section '## (O2)' "$SP_TEMPLATE")")" + [[ "$flat" == *'{ORCH_DIR}/plans/{TASK}.md'* ]] +} + +# --- normal: §3/§O3 (rework) reference reviews via {ORCH_DIR}, no ad-hoc --- +# --- $(dirname {STATUS_DIR}) form left behind (D3) ------------------------- + +@test "§3: reviews are referenced via {ORCH_DIR}, ad-hoc dirname form is gone" { + flat="$(normalize_ws "$(extract_section '## (3)' "$SP_TEMPLATE")")" + [[ "$flat" == *'{ORCH_DIR}/reviews/{TASK}-r{N}.md'* ]] + [[ "$flat" != *'$(dirname'* ]] +} + +@test "§O3: reviews are referenced via {ORCH_DIR}, ad-hoc dirname form is gone" { + flat="$(normalize_ws "$(extract_section '## (O3)' "$SP_TEMPLATE")")" + [[ "$flat" == *'{ORCH_DIR}/reviews/{TASK}-r{N}.md'* ]] + [[ "$flat" != *'$(dirname'* ]] +} + +# --- normal: the coordinator-facing token legend documents {ORCH_DIR} ------ + +@test "token legend documents {ORCH_DIR}" { + legend="$(sed -n '1,15p' "$SP_TEMPLATE")" + [[ "$legend" == *'{ORCH_DIR}'* ]] +} + +# --- boundary: brief.md's line and header note use {ORCH_DIR} ------- + +@test "brief.md: line uses {ORCH_DIR}" { + line="$(grep -F '' "$BRIEF_TEMPLATE")" + [[ "$line" == *'{ORCH_DIR}/plans/{TASK}.md'* ]] + [[ "$line" != *'.orchestration/'* ]] +} + +@test "brief.md: header note uses {ORCH_DIR}" { + header="$(sed -n '1,10p' "$BRIEF_TEMPLATE")" + [[ "$header" == *'{ORCH_DIR}/briefs/{TASK}.md'* ]] +} + +# --- error/negative: no bare .orchestration/ path in worker-facing text ---- + +@test "negative gate: no bare .orchestration/ path in session-prompt.md worker-facing sections" { + region="$(worker_facing_region "$SP_TEMPLATE")" + count="$(printf '%s\n' "$region" | grep -cF '.orchestration/' || true)" + [ "$count" -eq 0 ] +} + +@test "negative gate: no bare .orchestration/ path anywhere in brief.md" { + count="$(grep -cF '.orchestration/' "$BRIEF_TEMPLATE" || true)" + [ "$count" -eq 0 ] +} + +# --- negative control: the gate above fires on a fixture that still has ---- +# --- a bare path, proving it can fail (checks-that-cannot-pass) ------------ + +@test "negative control: the bare-path gate fires on a stripped fixture" { + stripped="${BATS_TEST_TMPDIR}/stripped-section.md" + cat > "$stripped" <<'EOF' +## (3) Rework — injected when review requests changes + +Address the issues in .orchestration/reviews/{TASK}-r{N}.md via the loop-implement skill. +EOF + count="$(grep -cF '.orchestration/' "$stripped" || true)" + [ "$count" -gt 0 ] +} From b89d84a372bbe473082eaa6459e6f0b0a72621c2 Mon Sep 17 00:00:00 2001 From: dch0202 Date: Fri, 14 Aug 2026 16:04:59 +0900 Subject: [PATCH 2/4] fix(orchestrate): watch-status.sh reached-target stall-skip + stall-reason surfacing (#88, #89) A worker already at/above the watched target phase was still stall-checked and could false-report exit 7 even though its silence was exactly what the session prompt ordered. The stall check now gates on rank < target_rank; the dead-worker/liveness check is unchanged and still applies. When a genuine stall IS reported, classify_stall() reads the pane scrollback (capture-pane -S -1000) and enriches the exit-7 message with a usage-limit reset time or chooser-pending state, falling back to the bare message otherwise. LO_LIMIT_EXTRA additively extends the usage-limit pattern. Annotations use octal printf escapes (\302\267, \342\200\224) rather than \xNN hex escapes, since dash (ubuntu CI's /bin/sh) does not interpret \x. --- skills/orchestrate/scripts/watch-status.sh | 66 ++++- tests/watch-status.bats | 284 +++++++++++++++++++++ 2 files changed, 347 insertions(+), 3 deletions(-) diff --git a/skills/orchestrate/scripts/watch-status.sh b/skills/orchestrate/scripts/watch-status.sh index 9897a0f..221d706 100755 --- a/skills/orchestrate/scripts/watch-status.sh +++ b/skills/orchestrate/scripts/watch-status.sh @@ -108,6 +108,47 @@ command -v "$TMUX_BIN" >/dev/null 2>&1 || TMUX_BIN="" # script DISABLES the check (empty), mirroring the TMUX_BIN treatment above. STALL_SCRIPT="${WATCH_STALL_SCRIPT:-$(dirname "$0")/tmux-worker-stalled.sh}" [ -f "$STALL_SCRIPT" ] || STALL_SCRIPT="" + +# classify_stall — read the pane SCROLLBACK (not just the visible +# region) for a stalled session and print an annotation for the exit-7 +# message, or nothing if no known pattern matches. Never fails the watch: +# every step degrades to "no annotation" on any capture/parse failure. +# Precedence: usage-limit wins over chooser (the chooser IS the limit +# chooser in the case this was built from). +classify_stall() { + [ -n "$TMUX_BIN" ] || return 0 + cs_pane=$("$TMUX_BIN" capture-pane -t "=$1:" -p -S -1000 2>/dev/null) || cs_pane="" + [ -n "$cs_pane" ] || return 0 + + # ASCII-only ERE on purpose: the real message's "·" and curly quote are + # multibyte, and BSD/GNU grep + locale differences make matching them + # fragile. LO_LIMIT_EXTRA is an ADDITIVE fixed-string match, checked only + # when the default pattern misses — unset/empty means default-only, never + # an off switch. + cs_line=$(printf '%s\n' "$cs_pane" | grep -E "hit your (session|weekly) limit" | tail -n 1) + if [ -z "$cs_line" ] && [ -n "${LO_LIMIT_EXTRA:-}" ]; then + cs_line=$(printf '%s\n' "$cs_pane" | grep -F "$LO_LIMIT_EXTRA" | tail -n 1) + fi + if [ -n "$cs_line" ]; then + cs_tail=$(printf '%s\n' "$cs_line" | sed -n 's/.*resets \(.*\)/\1/p') + if [ -n "$cs_tail" ]; then + printf 'usage limit \302\267 resets %s' "$cs_tail" + else + printf 'usage limit' + fi + return 0 + fi + + # Chooser: last 30 lines only — it is live UI at the bottom, so a confirm + # hint deep in scrollback history is stale, not a pending chooser. + cs_last30=$(printf '%s\n' "$cs_pane" | tail -n 30) + if printf '%s\n' "$cs_last30" | grep -qF "Enter to confirm"; then + printf 'chooser pending \342\200\224 answer with send-prompt.sh keys' + return 0 + fi + return 0 +} + while [ "$elapsed" -lt "$budget" ]; do # A worker's guardrails `ask`, recorded as an escalation, wakes the coordinator # immediately rather than waiting out the timeout. The coordinator MUST resolve @@ -153,12 +194,16 @@ while [ "$elapsed" -lt "$budget" ]; do case ",$only," in *",$base,"*) : ;; *) continue ;; esac fi ph=$("$JQ" -r '.phase // "pending"' "$f" 2>/dev/null || echo "pending") + r=$(rank "$ph") tk=$("$JQ" -r '.task // "?"' "$f" 2>/dev/null || echo "?") summary="$summary $tk:$ph" [ "$ph" = "failed" ] && { failed=$((failed+1)); continue; } # dead-worker (zombie) detection: a non-terminal task whose tmux session is # gone is treated as a failure, so the run aborts fast instead of waiting the # whole timeout. Terminal phases are skipped (the session may legitimately end). + # This gate is UNCHANGED by the stall-skip gate below (#88): a vanished + # session at a reached-but-non-terminal phase (e.g. impl_done) is still + # reportable — reached-target only silences the STALL check, not liveness. case "$ph" in done|merged|approved) : ;; *) @@ -170,20 +215,35 @@ while [ "$elapsed" -lt "$budget" ]; do # Per-session stall check: only rc 1 marks a stall. rc 0 (progressing), # rc 2 (unknown), or a broken script are all NOT stalled — the explicit # rc capture means no failure here can abort the watch loop. - if [ -n "$STALL_SCRIPT" ] && [ -n "$TMUX_BIN" ] && [ -n "$sess" ]; then + # Extra gate vs. the dead-worker check above: a task at/above the + # target is never stall-checked (#88) — its silence is exactly what + # the session prompt ordered ("signal and wait"), not a wedged worker. + if [ "$r" -lt "$target_rank" ] && [ -n "$STALL_SCRIPT" ] && [ -n "$TMUX_BIN" ] && [ -n "$sess" ]; then src=0; sh "$STALL_SCRIPT" "$sess" >/dev/null 2>&1 || src=$? if [ "$src" -eq 1 ]; then stalled="$stalled $tk:$sess"; fi fi ;; esac - r=$(rank "$ph") [ "$r" -ge "$target_rank" ] && done_count=$((done_count+1)) done echo "[watch ->$target] $done_count/$expected |$summary" [ "$failed" -gt 0 ] && { echo "[watch] failed session detected — abort"; exit 3; } [ "$done_count" -ge "$expected" ] && { echo "[watch] all reached $target"; exit 0; } # Stall is the weakest signal: failed(3) and all-reached(0) above win over it. - if [ -n "$stalled" ]; then echo "[watch] worker stalled —$stalled"; exit 7; fi + if [ -n "$stalled" ]; then + stmsg="" + for se in $stalled; do + setk=${se%%:*}; sesess=${se#*:} + reason=$(classify_stall "$sesess") + if [ -n "$reason" ]; then + stmsg="$stmsg $setk:$sesess ($reason)" + else + stmsg="$stmsg $setk:$sesess" + fi + done + echo "[watch] worker stalled —$stmsg" + exit 7 + fi sleep "$interval"; elapsed=$((elapsed+interval)) done echo "[watch] TIMEOUT (${budget}s, source=${budget_src}):$summary"; exit 2 diff --git a/tests/watch-status.bats b/tests/watch-status.bats index 4cd54bc..5dd67ed 100644 --- a/tests/watch-status.bats +++ b/tests/watch-status.bats @@ -285,3 +285,287 @@ setup() { [ "$status" -eq 3 ] [[ "$output" == *"dead worker"* ]] } + +# Substring assertions below use grep, not `[[ "$output" == *"..."* ]]`: BATS +# executes test bodies via eval in its own parent bash process (not a fresh +# `bash -c`), and on this machine's bash 3.2.57 that context's `[[ ]]` glob +# matching has been observed to report a false MATCH for a substring that is +# genuinely absent from $output (verified 2026-08-14: `[[ "$output" == +# *"(usage limit)"* ]]` passed against output containing no such text, while +# `grep -qF` on the same value correctly failed). This is the same +# bash-version/platform pitfall PR #94 hit — the tools_guidance note "prefer +# grep for pattern matching" exists for exactly this reason. +assert_output_has() { printf '%s\n' "$output" | grep -qF -- "$1"; } +refute_output_has() { ! printf '%s\n' "$output" | grep -qF -- "$1"; } + +# ---- reached-target skip (issue #88) ----------------------------------------- +# A task whose phase already satisfies the watch target must never be +# stall-checked — its silence is exactly what the session prompt ordered +# ("signal and wait"), not a wedged worker. This is a pure boolean gate +# (r < target_rank) with no error path by construction, so per +# testing-quality-minimum-case-set's edge-case allowance the trio below is +# normal (above target) + two boundaries (exactly at target; the differential +# dead-worker gate) + one contrast case proving the gate does not over-fire. + + +# The two tests below need a SECOND task that never reaches target, so the +# "all reached" exit-0 branch (which runs before the stall check on every +# poll) can't short-circuit the result and mask whether the stall-skip gate +# actually fired. The stub branches per-session ($1) so only t1 reports +# stalled — t2 is deliberately never-reaching and never-stalled filler. + +@test "reached-target: a task ABOVE the target with a stalled pane exits 0, not 7 (normal)" { + printf '{"task":"t1","phase":"impl_done","session":"lo-x"}' > "$ORCH/status/t1.json" + printf '{"task":"t2","phase":"pending","session":"lo-y"}' > "$ORCH/status/t2.json" + printf 'case "$1" in lo-x) exit 1 ;; *) exit 0 ;; esac\n' > "$BATS_TEST_TMPDIR/stall_stub.sh" + run env WATCH_TMUX=true WATCH_STALL_SCRIPT="$BATS_TEST_TMPDIR/stall_stub.sh" \ + sh "$WS" "$ORCH/status" implementing 2 2 1 + [ "$status" -eq 2 ] + refute_output_has "worker stalled" +} + +@test "reached-target: a task EXACTLY AT the target with a stalled pane exits 0, not 7 (boundary)" { + printf '{"task":"t1","phase":"impl_done","session":"lo-x"}' > "$ORCH/status/t1.json" + printf '{"task":"t2","phase":"implementing","session":"lo-y"}' > "$ORCH/status/t2.json" + printf 'case "$1" in lo-x) exit 1 ;; *) exit 0 ;; esac\n' > "$BATS_TEST_TMPDIR/stall_stub.sh" + run env WATCH_TMUX=true WATCH_STALL_SCRIPT="$BATS_TEST_TMPDIR/stall_stub.sh" \ + sh "$WS" "$ORCH/status" impl_done 2 2 1 + [ "$status" -eq 2 ] + refute_output_has "worker stalled" +} + +@test "reached-target: a task ONE RANK BELOW the target is still stall-checked (contrast)" { + printf '{"task":"t1","phase":"implementing","session":"lo-x"}' > "$ORCH/status/t1.json" + printf 'exit 1\n' > "$BATS_TEST_TMPDIR/stall_stub.sh" + run env WATCH_TMUX=true WATCH_STALL_SCRIPT="$BATS_TEST_TMPDIR/stall_stub.sh" \ + sh "$WS" "$ORCH/status" impl_done 1 2 1 + [ "$status" -eq 7 ] + assert_output_has "worker stalled" +} + +@test "reached-target: a task AT target whose session VANISHED still reports dead worker (differential gate, boundary)" { + printf '{"task":"t1","phase":"impl_done","session":"lo-x"}' > "$ORCH/status/t1.json" + run env WATCH_TMUX=false sh "$WS" "$ORCH/status" impl_done 1 2 1 + [ "$status" -eq 3 ] + assert_output_has "dead worker" +} + +# ---- stall reason surfacing (issue #89) -------------------------------------- +# When a genuine stall IS reported, enrich the bare message with the +# machine-readable reason read from the pane SCROLLBACK (capture-pane -S, +# not the visible region). fake_tmux's capture-pane only returns the full +# fixture (with the limit line near the top) when the invocation includes +# "-S -1000"; without it, only the last 24 lines come back — so a test that +# omits -S would fail here exactly the way it would against real tmux. + +fake_tmux() { + path="$1"; outfile="${2:-/dev/null}"; caprc="${3:-0}"; hsrc="${4:-0}" + cat > "$path" <