diff --git a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh index e16a6af..1d9cbfc 100755 --- a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh +++ b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh @@ -331,8 +331,12 @@ cleanup_herdr_on_error() { # Release the onboarding job: `cancelled` is a terminal transition the # registry knows, and it says what happened - the job was never really # dispatched. Best effort; a failure here must not stop the rollback. - if [ -n "${DELEGATE_JOB_ID:-}" ]; then + # `completed`/`error`/`cancelled` are terminal and published exactly once, so + # a job this run already closed is left alone - a second one is refused by + # the registry anyway ("conflicting cancelled after terminal error"). + if [ -n "${DELEGATE_JOB_ID:-}" ] && [ "${JOB_TERMINALIZED:-0}" != "1" ]; then delegate_publish_event "$DELEGATE_JOB_ID" cancelled "rolled back by create_session" "${DELEGATE_JOB_TOKEN:-}" "${DELEGATE_JOB_ATTEMPT:-}" || true + JOB_TERMINALIZED=1 fi if [ "$alive" = "1" ]; then @@ -645,6 +649,7 @@ Task: $SUBMIT_JOB_PROMPT" # text never appeared - none of which a later `started` can repair. if [ "$rc" -ne 0 ] && [ "$rc" -ne 4 ]; then delegate_publish_event "$DELEGATE_JOB_ID" error "instruction injection failed (prompt-lock, rc=$rc)" "$DELEGATE_JOB_TOKEN" "$DELEGATE_JOB_ATTEMPT" + JOB_TERMINALIZED=1 exit 1 fi if [ "$rc" -eq 4 ]; then @@ -654,10 +659,26 @@ Task: $SUBMIT_JOB_PROMPT" # ISSUE-23: this script no longer publishes `started` on the agent's behalf. # It used to, which made the registry show a started job even when the agent # never saw the prompt, and left any started-based verdict meaningless. - if ! delegate_wait_started "$DELEGATE_JOB_ID" "${DELEGATE_JOB_ACK_S:-180}" "$DELEGATE_JOB_TOKEN"; then - delegate_publish_event "$DELEGATE_JOB_ID" error "onboarding delivery unconfirmed: no started within ${DELEGATE_JOB_ACK_S:-180}s" "$DELEGATE_JOB_TOKEN" "$DELEGATE_JOB_ATTEMPT" - echo "ERROR: '$SESSION_NAME' did not acknowledge the onboarding prompt within ${DELEGATE_JOB_ACK_S:-180}s" >&2 - exit 1 + # + # How long to wait for the handshake: MAM_ONBOARD_ACK_S, else the wrapper's + # MAM_INJECT_ACK_GRACE_S, else the job's own ack timeout. The create-specific + # variable comes first so this wait can be shortened - in a sandbox with no + # real agent, for instance - without also retuning the delegate-job + # wrapper's injection ack, which is a different wait with its own tests. + ONBOARD_ACK_S="${MAM_ONBOARD_ACK_S:-${MAM_INJECT_ACK_GRACE_S:-${DELEGATE_JOB_ACK_S:-180}}}" + if ! delegate_wait_started "$DELEGATE_JOB_ID" "$ONBOARD_ACK_S" "$DELEGATE_JOB_TOKEN"; then + # The job failed, the session did not: the pane is up and the row is valid, + # only the handshake never landed. Killing a live agent over a slow or + # silent onboarding would destroy more than it protects, and it would + # contradict this script's own rule for the same situation one step + # earlier - a TUI that never signalled ready is preserved for diagnosis, + # not rolled back. So: mark the job, warn loudly, keep the session. + # MULTI_AGENT_RULES ยง6 has the GM gate on this job's `completed` anyway, + # so an unacknowledged onboarding already blocks real work. + delegate_publish_event "$DELEGATE_JOB_ID" error "onboarding delivery unconfirmed: no started within ${ONBOARD_ACK_S}s" "$DELEGATE_JOB_TOKEN" "$DELEGATE_JOB_ATTEMPT" + JOB_TERMINALIZED=1 + echo "WARNING: '$SESSION_NAME' did not acknowledge the onboarding prompt within ${ONBOARD_ACK_S}s." >&2 + echo " The session is preserved and job $DELEGATE_JOB_ID is marked error; re-send the onboarding with multi-agent-mux-delegate-job submit once the agent is up." >&2 fi # Trigger immediate priority reconcile cycle asynchronously # Issue #21 phase 2 (A8): the control root's reconcile/watchdog, never the diff --git a/tests/conftest.py b/tests/conftest.py index 14ee7ac..951dfb4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -227,6 +227,12 @@ def mam_sandbox(tmp_path, monkeypatch, request): # (`reconcile.sh --subscribe --idle-timeout 0`) from a sandbox. Tests that # exercise the daemon itself override this with "0" or delenv. monkeypatch.setenv("MAM_NO_SUBSCRIBER_DAEMON", "1") + # Issue #23: onboarding delivery now waits for the agent's own `started`. + # No sandbox has a real agent to publish one, so without a short grace + # every create_session test would sit out the full 180 s ack timeout. + # Create-specific on purpose: MAM_INJECT_ACK_GRACE_S is the delegate-job + # wrapper's knob and its own tests assert on the value they pass. + monkeypatch.setenv("MAM_ONBOARD_ACK_S", "3") monkeypatch.delenv("HERDR_SESSION_NAME", raising=False) monkeypatch.delenv("HERDR_SERVER_NAME", raising=False) monkeypatch.delenv("HERDR_WORKSPACE_ID", raising=False) diff --git a/tests/test_issue23_24_startup_path.py b/tests/test_issue23_24_startup_path.py index ee8e02b..9321743 100644 --- a/tests/test_issue23_24_startup_path.py +++ b/tests/test_issue23_24_startup_path.py @@ -539,19 +539,23 @@ def test_s7c_delivery_succeeds_when_the_registry_left_pending(mam_sandbox): assert "DELIVERED" in res.stdout, res.stdout + res.stderr -def test_s7d_no_started_within_t_is_a_delivery_failure(mam_sandbox): - """S7d: fail closed, and say so on the job.""" +def test_s7d_no_started_marks_the_job_but_keeps_the_session(mam_sandbox): + """S7d: the job failed, the session did not. The handshake is marked + `error` and warned about loudly, but a live agent is not killed over a + slow onboarding - the same rule this script already applies one step + earlier to a TUI that never signalled ready.""" res, published = _run_delivery(mam_sandbox, 0) - assert res.returncode == 1, res.stdout + res.stderr - assert "DELIVERED" not in res.stdout + assert res.returncode == 0, res.stdout + res.stderr assert published == ["error"], published + assert "did not acknowledge the onboarding prompt" in res.stderr + assert "preserved" in res.stderr def test_s7e_a_started_from_another_dispatch_is_not_evidence(mam_sandbox): """S7e: a loop job keeps one events.log across attempts, so only a started carrying THIS dispatch token counts.""" - res, _ = _run_delivery(mam_sandbox, 0, started_token="tok-OTHER") - assert res.returncode == 1, res.stdout + res.stderr + res, published = _run_delivery(mam_sandbox, 0, started_token="tok-OTHER") + assert published == ["error"], published def test_s8_rc4_is_a_warning_and_the_started_wait_decides(mam_sandbox): @@ -564,8 +568,8 @@ def test_s8_rc4_is_a_warning_and_the_started_wait_decides(mam_sandbox): assert published == [], published res, published = _run_delivery(mam_sandbox, 4) - assert res.returncode == 1, res.stdout + res.stderr assert published == ["error"], published + assert "did not acknowledge the onboarding prompt" in res.stderr @pytest.mark.parametrize("rc", [1, 2, 3])