From 05b0ea4762de3fad013e8986d45db21c4e6aa465 Mon Sep 17 00:00:00 2001 From: Godopu Date: Thu, 24 Sep 2026 06:47:57 +0900 Subject: [PATCH] fix(issue-23-24): merge review - the environment form of the auto-trust switch reaches the handler; a rate-limited dialog does not hide the next one Two probes from the merge-side codex review (its report was lost to an API outage; the probes and their results were recovered from its transcript): - create_session.sh and resume_session.sh set AUTO_TRUST=1 and then exported MAM_AUTO_TRUST from it, so the documented MAM_AUTO_TRUST=0 without a flag reached handle_startup_dialogs as 1: the operator had turned automatic trust off and it stayed on. Now AUTO_TRUST defaults to the environment value and an explicit --auto-trust/--no-auto-trust wins. - _mam_dialog_pick exited on the first dialog inside its resend cooldown instead of looking further down the screen, so a skipped (--no-auto-trust) or just-answered trust prompt hid the codex update notice below it, which the docs say is always answered. It continues to the next dialog now; S5 and S5b (one send per cooldown, one retry after it) still hold. Regressions: S6c, S5f (startup path), R3b and C3 (resume/create, env only, flag over env). Each fails without the fix. The create skill doc names the environment form and its precedence. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff --- .agents/skills/lib.sh | 5 ++- .../skills/multi-agent-mux-create/SKILL.md | 3 +- .../scripts/create_session.sh | 6 ++- .../scripts/resume_session.sh | 2 +- tests/test_issue23_24_startup_path.py | 26 +++++++++++++ tests/test_issue24_resume_dialogs.py | 39 +++++++++++++++++++ 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/.agents/skills/lib.sh b/.agents/skills/lib.sh index 94559a6..65c02dc 100644 --- a/.agents/skills/lib.sh +++ b/.agents/skills/lib.sh @@ -2952,7 +2952,10 @@ for idx, entry in enumerate(dialogs): continue last = state.get(idx) if last is not None and 0 <= now - last < cooldown: - sys.exit(1) + # rate-limited: look at the other dialogs on this screen instead of + # giving up (merge review, codex probe: a skipped trust prompt or a + # freshly answered one used to hide the notice below it) + continue if kind == "trust" and not auto_trust: print("skip|%d|" % idx) sys.exit(0) diff --git a/.agents/skills/multi-agent-mux-create/SKILL.md b/.agents/skills/multi-agent-mux-create/SKILL.md index 9fc5617..3c8a378 100644 --- a/.agents/skills/multi-agent-mux-create/SKILL.md +++ b/.agents/skills/multi-agent-mux-create/SKILL.md @@ -116,7 +116,8 @@ The bootstrapped server receives a **sanitized copy of the caller's environment* blocking-modal signatures and their accept keys live in each agent adapter (`lib_py/agents/adapters/*.py`, published as `MAM_STARTUP_DIALOGS`), and they are answered while waiting for the TUI rather than after it. `--no-auto-trust` - leaves directory-trust dialogs for a human to accept and still answers other + (or `MAM_AUTO_TRUST=0` in the environment; an explicit flag wins over the + environment) leaves directory-trust dialogs for a human to accept and still answers other blocking notices (for example codex's update prompt, where Skip changes nothing). Trust matters most for **codex**: trusting a directory makes it load that project's local config, hooks and exec policy, so `--no-auto-trust` is the 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 79e4454..1494ce1 100755 --- a/.agents/skills/multi-agent-mux-create/scripts/create_session.sh +++ b/.agents/skills/multi-agent-mux-create/scripts/create_session.sh @@ -66,8 +66,10 @@ ONBOARD=1 # Issue #24: answer a start-up directory-trust dialog for the --workspace the # operator named. --no-auto-trust leaves those unanswered (notices are always # answered); codex in particular loads project-local config, hooks and exec -# policy once trusted. -AUTO_TRUST=1 +# policy once trusted. MAM_AUTO_TRUST=0 in the environment is the same switch +# (the skill docs say so); an explicit --auto-trust/--no-auto-trust wins over it +# (merge review, codex probe: the export below used to overwrite the caller's 0). +AUTO_TRUST="${MAM_AUTO_TRUST:-1}" while [ $# -gt 0 ]; do case "$1" in diff --git a/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh b/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh index 0776864..78a58d0 100755 --- a/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh +++ b/.agents/skills/multi-agent-mux-resume/scripts/resume_session.sh @@ -30,7 +30,7 @@ DRY_RUN=0 # Issue #24: a resumed CLI shows the same directory-trust dialog as a fresh # one, so resume takes the same switch as create (MAM_AUTO_TRUST is what # handle_startup_dialogs reads). -AUTO_TRUST=1 +AUTO_TRUST="${MAM_AUTO_TRUST:-1}" # the environment form of the same switch; a flag wins while [ $# -gt 0 ]; do case "$1" in diff --git a/tests/test_issue23_24_startup_path.py b/tests/test_issue23_24_startup_path.py index e75ac47..ddcdac6 100644 --- a/tests/test_issue23_24_startup_path.py +++ b/tests/test_issue23_24_startup_path.py @@ -578,6 +578,32 @@ def test_s6_no_auto_trust_leaves_trust_dialogs_alone_but_answers_notices( assert sent == ["send-keys -t test-sess 2"], (res.stderr, sent) +def test_s6c_a_skipped_trust_dialog_does_not_hide_the_notice_below_it(mam_sandbox): + """S6c (merge review, codex probe): with --no-auto-trust and a codex screen + that shows the trust prompt and the update notice together, the trust + prompt is skipped and recorded, and the skip's cooldown must not stop the + picker from answering the notice on the next poll.""" + res, sent = _run_dialogs( + mam_sandbox, "codex", + "Do you trust the contents of this directory?\n > 1. Yes, continue\n" + "Update available\n 1. Update now\n 2. Skip\n 3. Skip until next version", + timeout=3, env={"MAM_AUTO_TRUST": "0", "MAM_DIALOG_RESEND_S": "30"}) + assert sent == ["send-keys -t test-sess 2"], (res.stderr, sent) + assert "no-auto-trust" in res.stderr, res.stderr + + +def test_s5f_a_dialog_inside_its_cooldown_does_not_block_the_next_one(mam_sandbox): + """S5f (merge review, codex probe): two dialogs on one screen, auto-trust on. + The trust prompt is answered first; while it is rate-limited the notice + must still be answered, once each.""" + res, sent = _run_dialogs( + mam_sandbox, "codex", + "Do you trust the contents of this directory?\n > 1. Yes, continue\n" + "Update available\n 1. Update now\n 2. Skip\n 3. Skip until next version", + timeout=3, env={"MAM_DIALOG_RESEND_S": "60"}) + assert sent == ["send-keys -t test-sess Enter", "send-keys -t test-sess 2"], (res.stderr, sent) + + def test_s6b_create_session_no_longer_gates_dialogs_on_claude(mam_sandbox): """S6b (source read): the #24 root cause was the `AGENT = claude` gate at the create_session call site, so the gate's absence is the regression.""" diff --git a/tests/test_issue24_resume_dialogs.py b/tests/test_issue24_resume_dialogs.py index 19e1dd3..f3a12ff 100644 --- a/tests/test_issue24_resume_dialogs.py +++ b/tests/test_issue24_resume_dialogs.py @@ -100,6 +100,45 @@ def test_r3_no_auto_trust_reaches_the_dialog_handler(mam_sandbox, mock_herdr, mo assert _calls(log) == ["sess=test-r3-codex timeout=20 agent=codex auto_trust=0"] +@pytest.mark.parametrize("env_value,flag,expected", [ + ("0", (), "0"), # the documented environment form alone + ("0", ("--auto-trust",), "1"), # an explicit flag wins over the environment + ("1", ("--no-auto-trust",), "0"), +]) +def test_r3b_the_environment_form_of_the_switch_reaches_the_handler_on_resume( + mam_sandbox, mock_herdr, mock_agents, monkeypatch, env_value, flag, expected): + """R3b (merge review, codex probe): MAM_AUTO_TRUST=0 without a flag used to + be overwritten by the script's own export of its default.""" + tmp_path = mam_sandbox + ws = _create_and_stop(tmp_path, "codex", "test-r3b-codex") + log = _record_dialog_calls(tmp_path) + monkeypatch.setenv("MAM_AUTO_TRUST", env_value) + res = _resume(tmp_path, "codex", "test-r3b-codex", ws, extra=flag) + assert res.returncode == 0, res.stdout + res.stderr + assert _calls(log) == ["sess=test-r3b-codex timeout=20 agent=codex auto_trust=%s" % expected] + + +@pytest.mark.parametrize("env_value,flag,expected", [ + ("0", (), "0"), + ("0", ("--auto-trust",), "1"), + ("1", ("--no-auto-trust",), "0"), +]) +def test_c3_the_environment_form_of_the_switch_reaches_the_handler_on_create( + mam_sandbox, mock_herdr, mock_agents, monkeypatch, env_value, flag, expected): + """C3 (merge review, codex probe): the same contract on create_session.""" + tmp_path = mam_sandbox + log = _record_dialog_calls(tmp_path) + ws = str(tmp_path / "ws_c3") + os.makedirs(ws, exist_ok=True) + create = _script(tmp_path, SCRIPTS[0]) + monkeypatch.setenv("MAM_AUTO_TRUST", env_value) + res = subprocess.run(["bash", create, "--workspace", ws, "--agent", "codex", + "--role", "creator", "--session", "test-c3-codex", "--no-onboard", *flag], + cwd=str(tmp_path), capture_output=True, text=True) + assert res.returncode == 0, res.stdout + res.stderr + assert _calls(log) == ["sess=test-c3-codex timeout=15 agent=codex auto_trust=%s" % expected] + + def test_r4_the_shipped_handler_really_runs_for_a_non_claude_resume( mam_sandbox, mock_herdr, mock_agents): """R4 (no override): with the real handle_startup_dialogs, a codex resume