fix(issue-23): an unacknowledged onboarding marks the job and keeps the session

Two defects in the started-based verdict, both found by the full suite.

The first was destructive. No `started` within T exited 1, which fired the
rollback trap and killed the session. That is worse than the bug being fixed:
create_session already has a rule for "the agent is not responding but the
process is alive" one step earlier - a TUI that never signalled ready is
preserved for diagnosis - and an onboarding that is merely slow is the same
situation. The job is marked `error` and warned about loudly; the session, its
row and the pane stay. MULTI_AGENT_RULES §6 has the GM gate on this job
anyway, so an unacknowledged onboarding still blocks real work.

It also broke 21 tests across 8 modules (create/sanity/tier2/tier3/tier4/
issue15/issue19/issue21) and added roughly an hour to the suite: no sandbox
has an agent to publish `started`, so every create_session test sat out the
full 180 s and then failed on the rollback. MAM_ONBOARD_ACK_S shortens that
wait, and the sandbox fixture sets 3 s. It is deliberately not the wrapper's
MAM_INJECT_ACK_GRACE_S - that is a different wait whose own tests assert on
the value they pass, and reusing it made issue18's acceptance2 read "within
3s" instead of "within 4s".

The second: the delivery failure published `error` and then the rollback
published `cancelled` on the same job, which the registry refuses
("conflicting cancelled after terminal error"). Terminal events are published
exactly once, so the trap now skips a job this run already closed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
GodopuandClaude Opus 5 committed 2026-09-22 06:24:06 +00:00
1 parent 2cd20423ac
commit 33b91fe0c8
3 files changed
+43 -12

No files matched your search

+6
View File
@@ -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)
+11 -7
View File
@@ -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])