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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VCgu9Fa9iT7TbJUSzdq4Ff
This commit is contained in:
GodopuandClaude Fable 5.1 committed 2026-09-24 06:47:57 +09:00
1 parent f4ba205c9f
commit 05b0ea4762
6 files changed
+76 -5

No files matched your search

+4 -1
View File
@@ -2952,7 +2952,10 @@ for idx, entry in enumerate(dialogs):
continue continue
last = state.get(idx) last = state.get(idx)
if last is not None and 0 <= now - last < cooldown: 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: if kind == "trust" and not auto_trust:
print("skip|%d|" % idx) print("skip|%d|" % idx)
sys.exit(0) sys.exit(0)
@@ -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 blocking-modal signatures and their accept keys live in each agent adapter
(`lib_py/agents/adapters/*.py`, published as `MAM_STARTUP_DIALOGS`), and they (`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` 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 blocking notices (for example codex's update prompt, where Skip changes
nothing). Trust matters most for **codex**: trusting a directory makes it load 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 that project's local config, hooks and exec policy, so `--no-auto-trust` is the
@@ -66,8 +66,10 @@ ONBOARD=1
# Issue #24: answer a start-up directory-trust dialog for the --workspace the # Issue #24: answer a start-up directory-trust dialog for the --workspace the
# operator named. --no-auto-trust leaves those unanswered (notices are always # operator named. --no-auto-trust leaves those unanswered (notices are always
# answered); codex in particular loads project-local config, hooks and exec # answered); codex in particular loads project-local config, hooks and exec
# policy once trusted. # policy once trusted. MAM_AUTO_TRUST=0 in the environment is the same switch
AUTO_TRUST=1 # (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 while [ $# -gt 0 ]; do
case "$1" in case "$1" in
@@ -30,7 +30,7 @@ DRY_RUN=0
# Issue #24: a resumed CLI shows the same directory-trust dialog as a fresh # 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 # one, so resume takes the same switch as create (MAM_AUTO_TRUST is what
# handle_startup_dialogs reads). # 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 while [ $# -gt 0 ]; do
case "$1" in case "$1" in
+26
View File
@@ -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) 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): 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 """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.""" the create_session call site, so the gate's absence is the regression."""
+39
View File
@@ -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"] 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( def test_r4_the_shipped_handler_really_runs_for_a_non_claude_resume(
mam_sandbox, mock_herdr, mock_agents): mam_sandbox, mock_herdr, mock_agents):
"""R4 (no override): with the real handle_startup_dialogs, a codex resume """R4 (no override): with the real handle_startup_dialogs, a codex resume