From 794d86a92d8c032838802fd428ac0911e5e7ee22 Mon Sep 17 00:00:00 2001 From: dch0202 Date: Wed, 5 Aug 2026 10:07:17 +0900 Subject: [PATCH] fix(orchestrate): never put a second agent on one worktree (re-entry) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SKILL.md's re-entry path restarts a worker the coordinator believes is dead by calling orca-worker-start.sh again with --worktree/--agent. For a dev-loop worker GROUNDWORK_ESCALATION_DIR is always set, so that lands in worker mode, which ran `orca terminal create` unconditionally. If the liveness read was wrong — and this run proved a worker can be ALIVE yet wedged — that puts a second agent on the same checkout with the Dispatch bound to only one of them. The tmux path has no such hazard: launch-session.sh detects an existing session and reuses it, loudly. Worker mode now probes first, automatically, with no flag to pass: it joins `orchestration dispatch-show --task` against `terminal list --worktree` and binds the recorded assignee when that terminal is still live on this worktree, reporting the reuse on stderr. Fails toward the safe side. Creating the second agent is the damaging direction, so a probe that cannot answer is UNKNOWN, never "no agent": an unreadable dispatch or terminal list exits 6 and creates nothing. Only `ok:true` with `dispatch:null` counts as a definite first start. Both probe calls take ` 147: reuse, first start, orphaned, disconnected, empty list, assignee on another worktree, both probes returning ok:false, both returning malformed JSON, a blank payload, and proof the probe never runs in --terminal or composed mode. Co-Authored-By: Claude Opus 5 (1M context) --- .../orchestrate/scripts/orca-worker-start.sh | 81 +++++++++- tests/orca-worker-start.bats | 146 ++++++++++++++++++ 2 files changed, 226 insertions(+), 1 deletion(-) diff --git a/skills/orchestrate/scripts/orca-worker-start.sh b/skills/orchestrate/scripts/orca-worker-start.sh index 16c182e..e348766 100755 --- a/skills/orchestrate/scripts/orca-worker-start.sh +++ b/skills/orchestrate/scripts/orca-worker-start.sh @@ -12,6 +12,20 @@ # (same command shape as orca-spawn.sh), waits for `tui-idle`, and only then # binds the Dispatch with `worker-start --terminal`. # +# On re-entry this mode is idempotent, automatically — no flag to pass. +# Before creating anything it asks Orca whether this task's Dispatch already +# names a terminal that is still live on the worktree (`orchestration +# dispatch-show` joined against `terminal list --worktree`): +# live -> binds THAT terminal and creates nothing, saying so on +# stderr. --perm / --name are inert on this path: the agent +# is already running with the mode it was created with. +# not live -> unchanged: terminal create + tui-idle wait + bind. +# no record -> unchanged (this is the task's first start). +# can't tell-> exit 6, creates nothing, not retried automatically. +# To force a fresh agent, stop the old one first (`orca orchestration +# worker-stop`) and re-run. This is a check-then-create, so it protects a +# sequential re-entry — not two coordinators racing on one worktree. +# # REUSE MODE (--terminal ) # Bind this task's next phase (implement / rework) to the agent that already # holds the task's context. No new terminal, no env to re-inject. @@ -34,11 +48,15 @@ # ORCA_WORKER_START_DRYRUN print the orca commands instead of running them # ORCA_WORKER_START_JSON canned `worker-start --json` receipt (tests) # ORCA_WORKER_START_CREATE_JSON canned `terminal create --json` receipt (tests) +# ORCA_WORKER_START_DISPATCH_JSON canned `orchestration dispatch-show --json` (tests) +# ORCA_WORKER_START_TERMLIST_JSON canned `terminal list --json` (tests) # # On success prints: dispatch=, handle=, state=, setup= # Exit: 0 ok | 1 usage | 2 unsupported agent/placement for the escalation contract # | 3 receipt without a dispatch | 4 start failed (inspect stage/effects/ # residualResources; do NOT auto-retry) | 5 could not create the terminal +# | 6 could not determine whether a live agent terminal already exists — +# refused to create a second one (verify, then re-run or use --terminal) set -u ORCA="${ORCA_BIN:-orca}" @@ -129,8 +147,69 @@ dry="${ORCA_WORKER_START_DRYRUN:-}" rt="${LO_READY_TIMEOUT:-60}"; [ "$rt" -ge 1 ] 2>/dev/null || rt=60 -# --- worker mode: create the env-carrying agent terminal ourselves ------------ +reused="" + +# --- re-entry idempotency: never put a SECOND agent on one checkout ----------- +# SKILL.md's re-entry path restarts a worker the coordinator believes is dead by +# calling this script again with --worktree/--agent. If that liveness read was +# wrong, an unconditional `terminal create` lands a second agent on the same +# working tree, with the Dispatch bound to only one of them. So ask Orca first: +# does this task's Dispatch already name a terminal that is still live on this +# worktree? A probe that cannot answer is UNKNOWN, never "no agent" — creating +# the second agent is the damaging direction, so we refuse (exit 6). +# Both calls take `/dev/null) || dsp="" + fi + # ok:true with dispatch:null is a definite "no worker yet" (verified live); + # anything else means we could not read it — that is unknown, not none. + dsp_ok=$(printf '%s' "$dsp" | "$JQ" -r '.ok // false' 2>/dev/null || echo false) + if [ "$dsp_ok" != "true" ]; then + echo "orca-worker-start: cannot read the dispatch for '$task' — refusing to create a second agent on '$wt'; verify with \`orca orchestration dispatch-show --task $task\`, then re-run or bind the live terminal with --terminal" >&2 + exit 6 + fi + prev=$(printf '%s' "$dsp" | "$JQ" -r '.result.dispatch.assignee_handle // empty' 2>/dev/null) + + if [ -n "$prev" ]; then + # A recorded assignee is only a reuse target while it is STILL LIVE on the + # worktree we were asked to start on. + if [ -n "${ORCA_WORKER_START_TERMLIST_JSON:-}" ]; then + tl="$ORCA_WORKER_START_TERMLIST_JSON" + else + tl=$("$ORCA" terminal list --worktree "$wt" --json /dev/null) || tl="" + fi + tl_ok=$(printf '%s' "$tl" | "$JQ" -r '.ok // false' 2>/dev/null || echo false) + if [ "$tl_ok" != "true" ]; then + echo "orca-worker-start: cannot list terminals for '$wt' — refusing to create a second agent while '$prev' may still be live; verify with \`orca terminal list --worktree $wt\`" >&2 + exit 6 + fi + live=$(printf '%s' "$tl" | "$JQ" -r --arg h "$prev" \ + '[.result.terminals[]? | select(.handle==$h and .orphaned!=true and .connected==true)] | length' 2>/dev/null) + case "$live" in ''|*[!0-9]*) live=0 ;; esac + if [ "$live" -ge 1 ]; then + echo "orca-worker-start: worktree '$wt' already has a live agent terminal $prev — reusing it instead of creating a second agent (stop it first with \`orca orchestration worker-stop\` if you want a fresh one)" >&2 + term="$prev" + reused=1 + fi + fi + fi +fi + +# --- worker mode: create the env-carrying agent terminal ourselves ------------ +# (skipped when the probe above rebound us to the agent that is already there — +# the tui-idle wait below exists to pass a NEW CLI's trust screen, and a mid-task +# agent may never report idle) +if [ "$worker_mode" = 1 ] && [ -z "$reused" ]; then # Single-quote the values with embedded quotes escaped (`'\''`) so a path with # any metacharacter — including a quote — cannot break out of the command. esc_sq() { printf '%s' "$1" | sed "s/'/'\\\\''/g"; } diff --git a/tests/orca-worker-start.bats b/tests/orca-worker-start.bats index 46ddecd..0afacdd 100644 --- a/tests/orca-worker-start.bats +++ b/tests/orca-worker-start.bats @@ -4,8 +4,19 @@ # `worker-start --agent` cannot express); reuse mode must not re-create anything. setup() { + # These tests select worker mode by exporting GROUNDWORK_ESCALATION_DIR per + # case, and `run env FOO=bar ...` INHERITS the caller's environment. Run the + # suite inside a dev-loop worker (which always exports it) and every + # "no escalation env" case would silently take the worker-mode path instead. + # Declare the precondition here rather than depending on the ambient shell. + unset GROUNDWORK_ESCALATION_DIR GROUNDWORK_TASK_ID OWS="${BATS_TEST_DIRNAME}/../skills/orchestrate/scripts/orca-worker-start.sh" OK_RECEIPT='{"ok":true,"result":{"runId":"run_1","taskId":"task_1","dispatchId":"ctx_abc","state":"ready","stage":"input_accepted","setup":{"state":"running"},"effects":[{"kind":"terminal","role":"agent","action":"created","id":"term_agent"},{"kind":"dispatch_input","role":"agent","id":"term_agent","state":"accepted"}]}}' + # re-entry probe fixtures — payload shapes verified against the live Orca CLI: + # an unknown task answers ok:true/dispatch:null, so a first start is not a probe failure. + DSP_PREV='{"ok":true,"result":{"dispatch":{"assignee_handle":"term_prev"}}}' + DSP_NONE='{"ok":true,"result":{"dispatch":null}}' + TL_LIVE='{"ok":true,"result":{"terminals":[{"handle":"term_prev","orphaned":false,"connected":true}]}}' } # --- worker mode (escalation contract active) --------------------------------- @@ -84,6 +95,119 @@ setup() { [[ "$output" != *"dispatch="* ]] } +# --- re-entry idempotency (worker mode) --------------------------------------- +# SKILL.md restarts a "dead" worker with --worktree/--agent. If that liveness +# read was wrong, an unconditional `terminal create` puts a SECOND agent on the +# same checkout. The probe must bind the live one instead. + +@test "re-entry: a live agent terminal on the worktree is reused, not duplicated" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON="$TL_LIVE" \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"[--worktree] [id:r::/wt] [--terminal] [term_prev]"* ]] + [[ "$output" == *"already has a live agent terminal term_prev"* ]] + [[ "$output" != *"[terminal] [create]"* ]] +} + +@test "re-entry: no dispatch on record (first start) still creates the terminal" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_NONE" \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"[terminal] [create]"* ]] + [[ "$output" != *"reusing it"* ]] +} + +@test "re-entry: a recorded but ORPHANED terminal is dead — create (boundary)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON='{"ok":true,"result":{"terminals":[{"handle":"term_prev","orphaned":true,"connected":true}]}}' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"[terminal] [create]"* ]] + [[ "$output" != *"reusing it"* ]] +} + +@test "re-entry: a recorded but DISCONNECTED terminal is dead — create (boundary)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON='{"ok":true,"result":{"terminals":[{"handle":"term_prev","orphaned":false,"connected":false}]}}' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"[terminal] [create]"* ]] + [[ "$output" != *"reusing it"* ]] +} + +@test "re-entry: an empty terminal list is not a reuse target (boundary)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON='{"ok":true,"result":{"terminals":[]}}' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"[terminal] [create]"* ]] +} + +@test "re-entry: an assignee living on another worktree is not reused (boundary)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON='{"ok":true,"result":{"terminals":[{"handle":"term_other","orphaned":false,"connected":true}]}}' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"[terminal] [create]"* ]] + [[ "$output" != *"term_prev"* ]] +} + +# --- re-entry: a probe that cannot answer fails closed (exit 6) --------------- +# "Unknown" must never be treated as "no live agent": that is the direction that +# silently puts two agents on one checkout. + +@test "re-entry: dispatch-show returning ok:false refuses to create (exit 6)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON='{"ok":false,"error":{"code":"relay_unavailable"}}' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 6 ] + [[ "$output" == *"cannot read the dispatch for 'task_1'"* ]] + [[ "$output" != *"[terminal] [create]"* ]] + [[ "$output" != *"dispatch="* ]] +} + +@test "re-entry: a malformed dispatch receipt refuses to create (exit 6)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON='not json at all' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 6 ] + [[ "$output" != *"[terminal] [create]"* ]] +} + +@test "re-entry: terminal list ok:false refuses while the assignee may be live" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON='{"ok":false,"error":{"code":"selector_not_found"}}' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 6 ] + [[ "$output" == *"term_prev"* ]] + [[ "$output" != *"[terminal] [create]"* ]] +} + +@test "re-entry: a malformed terminal list refuses to create (exit 6)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON='garbage' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 6 ] + [[ "$output" != *"[terminal] [create]"* ]] +} + +@test "re-entry: a blank probe payload is unknown, not 'no worker' (boundary)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON=' ' \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 6 ] + [[ "$output" != *"[terminal] [create]"* ]] +} + # --- composed agent-first mode (no escalation contract requested) ------------- @test "no escalation env: uses Orca's agent-first worker-start with --setup run" { @@ -148,6 +272,28 @@ setup() { # --- reuse mode --------------------------------------------------------------- +@test "reuse mode: the re-entry probe never runs or overrides --terminal" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON="$TL_LIVE" \ + bash "$OWS" --task task_2 --terminal term_given + [ "$status" -eq 0 ] + [[ "$output" == *"[--terminal] [term_given]"* ]] + [[ "$output" != *"term_prev"* ]] + [[ "$output" != *"reusing it"* ]] +} + +@test "composed agent-first mode is not probed either (no escalation env)" { + run env ORCA_WORKER_START_DRYRUN=1 \ + ORCA_WORKER_START_DISPATCH_JSON="$DSP_PREV" \ + ORCA_WORKER_START_TERMLIST_JSON="$TL_LIVE" \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent codex + [ "$status" -eq 0 ] + [[ "$output" == *"[--agent] [codex]"* ]] + [[ "$output" != *"[--terminal]"* ]] + [[ "$output" != *"term_prev"* ]] +} + @test "reuse mode: passes --terminal only and creates nothing" { run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ bash "$OWS" --task task_2 --terminal term_agent