diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index b1b8608531d..1aef92c4eae 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -4,6 +4,7 @@ # Usage: fm-spawn.sh --mode --yolo [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] # fm-spawn.sh --scout [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] # fm-spawn.sh [] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] --secondmate +# A missing or empty task-id is refused before launch; a missing or empty project-dir is also refused for ship or scout spawns, including batch pairs, with an actionable argument error. # --mode and --yolo are this task's delivery contract, REQUIRED for every ship # spawn and refused on --scout and --secondmate spawns. Firstmate resolves both # per task at intake (AGENTS.md section 7); data/projects.md holds the captain's @@ -687,6 +688,48 @@ done echo "error: --traceparent requires a non-empty value" >&2 exit 1 } +validate_positional_shape() { + local first idpart pair pair_id pair_proj batch=0 + [ "${#POS[@]}" -gt 0 ] && [ -n "${POS[0]:-}" ] || { + echo "error: spawn requires a task id positional argument ()" >&2 + return 1 + } + if [ "$RELAUNCH" -eq 1 ] || [ "$KIND" = secondmate ]; then + return 0 + fi + first=${POS[0]} + idpart=${first%%=*} + if [ "$first" != "$idpart" ]; then + case "$idpart" in + */*) ;; + *) batch=1 ;; + esac + fi + if [ "$batch" -eq 1 ]; then + for pair in "${POS[@]}"; do + case "$pair" in + *=*) + pair_id=${pair%%=*} + pair_proj=${pair#*=} + [ -n "$pair_id" ] || { + echo "error: spawn requires a task id positional argument ()" >&2 + return 1 + } + [ -n "$pair_proj" ] || { + echo "error: ${KIND} spawn requires a project directory positional argument ()" >&2 + return 1 + } + ;; + esac + done + else + [ "${#POS[@]}" -gt 1 ] && [ -n "${POS[1]:-}" ] || { + echo "error: ${KIND} spawn requires a project directory positional argument ()" >&2 + return 1 + } + fi +} +validate_positional_shape || exit 1 # A parent-delivered carrier replaces this home's own resolution, so it is # refused unless it is a secondmate spawn carrying a strictly valid W3C value. # Nothing else may reach the pane's TRACEPARENT export. @@ -1335,7 +1378,10 @@ if [ "${#POS[@]}" -gt 0 ] && [ "${POS[0]}" != "$idpart" ] && case "$idpart" in * [ "$YOLO_SET" -eq 0 ] || shared_args+=(--yolo "$YOLO") for pair in "${POS[@]}"; do case "$pair" in - *=*) : ;; + *=*) + pair_id=${pair%%=*} + pair_proj=${pair#*=} + ;; *) echo "error: batch dispatch expects every argument as id=repo; got '$pair'" >&2 rc=2 @@ -1346,14 +1392,15 @@ if [ "${#POS[@]}" -gt 0 ] && [ "${POS[0]}" != "$idpart" ] && case "$idpart" in * echo "error: batch dispatch does not support --secondmate; spawn each secondmate explicitly" >&2 rc=2 continue - elif [ "$KIND" = scout ]; then - if FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "${pair%%=*}" "${pair#*=}" "${shared_args[@]+"${shared_args[@]}"}" --scout; then :; else - echo "batch: FAILED to spawn ${pair%%=*} (${pair#*=})" >&2 + fi + if [ "$KIND" = scout ]; then + if FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "$pair_id" "$pair_proj" "${shared_args[@]+"${shared_args[@]}"}" --scout; then :; else + echo "batch: FAILED to spawn $pair_id ($pair_proj)" >&2 rc=1 fi else - if FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "${pair%%=*}" "${pair#*=}" "${shared_args[@]+"${shared_args[@]}"}"; then :; else - echo "batch: FAILED to spawn ${pair%%=*} (${pair#*=})" >&2 + if FM_SPAWN_NO_GUARD=1 "$FM_ROOT/bin/fm-spawn.sh" "$pair_id" "$pair_proj" "${shared_args[@]+"${shared_args[@]}"}"; then :; else + echo "batch: FAILED to spawn $pair_id ($pair_proj)" >&2 rc=1 fi fi diff --git a/tests/fm-spawn-batch.test.sh b/tests/fm-spawn-batch.test.sh index ab447397c0e..4f442e24004 100755 --- a/tests/fm-spawn-batch.test.sh +++ b/tests/fm-spawn-batch.test.sh @@ -113,6 +113,25 @@ ROWS # A ship batch carries one shared delivery contract. Missing flags must stop the # whole batch before any pair is dispatched, so a batch can never launch workers # whose delivery posture was never decided. +test_batch_empty_fields_refuse_with_actionable_errors() { + local out status + out=$(run_ship_spawn batch-empty-project-z13=) + status=$? + [ "$status" -ne 0 ] || fail "a batch pair without a project should exit non-zero" + printf '%s\n' "$out" | grep -F 'error: ship spawn requires a project directory positional argument ()' >/dev/null \ + || fail "an empty batch project did not name the required project directory argument" + assert_not_contains "$out" "cd: " "an empty batch project must not expose a raw cd error" + assert_not_contains "$out" "unbound variable" "an empty batch project must not expose a shell error" + + out=$(run_ship_spawn '=projects/none') + status=$? + [ "$status" -ne 0 ] || fail "a batch pair without a task id should exit non-zero" + printf '%s\n' "$out" | grep -F 'error: spawn requires a task id positional argument ()' >/dev/null \ + || fail "an empty batch task did not name the required task id argument" + assert_not_contains "$out" "unbound variable" "an empty batch task must not expose a shell error" + pass "batch dispatch refuses empty task and project fields before re-exec" +} + test_batch_requires_the_shared_delivery_contract() { local out status out=$(run_spawn nope-batch-nomode-z9=projects/none-a nope-batch-nomode-z10=projects/none-b) @@ -145,6 +164,7 @@ test_scout_batch_refuses_delivery_flags() { test_batch_dispatches_every_pair test_batch_mode_boundaries +test_batch_empty_fields_refuse_with_actionable_errors test_batch_requires_the_shared_delivery_contract test_scout_batch_refuses_delivery_flags test_projects_path_scoping diff --git a/tests/fm-spawn-dispatch-profile.test.sh b/tests/fm-spawn-dispatch-profile.test.sh index e192f61fb2c..27e937e1290 100755 --- a/tests/fm-spawn-dispatch-profile.test.sh +++ b/tests/fm-spawn-dispatch-profile.test.sh @@ -1266,6 +1266,29 @@ test_launch_environment_inaccessible_config_refuses test_launch_environment_inherited_by_secondmate test_launch_environment_inheritance_preserves_on_source_errors +test_missing_project_ship_spawn_refuses_with_actionable_error() { + local case_dir home fakebin launchlog id out status + id=missing-project-pos-z1 + case_dir="$TMP_ROOT/missing-project" + home="$case_dir/home" + launchlog="$case_dir/launch.log" + mkdir -p "$case_dir" + fakebin=$(make_spawn_fakebin "$case_dir/fake") + fm_test_spawn_home "$home" pi + enable_dispatch_profile "$home" + + out=$(run_spawn "$home" "$case_dir" "$fakebin" "$launchlog" \ + "$id" --mode no-mistakes --yolo off) + status=$? + expect_code 1 "$status" "a ship spawn without a project should refuse" + assert_contains "$out" "error: ship spawn requires a project directory positional argument ()" \ + "missing-project refusal should name the required project directory argument" + assert_not_contains "$out" "unbound variable" \ + "missing-project refusal should not expose a shell unbound-variable error" + [ ! -s "$launchlog" ] || fail "missing-project refusal must not launch a worker" + pass "fm-spawn: missing ship project reports an actionable argument error" +} + test_worker_launch_delivers_role_scope() { local rec id out launch kind prompt envelope encoded brief_kind brief content first_line role_line task_line inbox for brief_kind in heading legacy scaffold; do @@ -1437,6 +1460,7 @@ test_non_claude_harness_ignores_claude_permission_mode() { pass "config/claude-permission-mode changes claude launches only" } +test_missing_project_ship_spawn_refuses_with_actionable_error test_worker_launch_delivers_role_scope test_no_profile_keeps_claude_profile_defaults test_non_cursor_launch_clears_inherited_cursor_markers