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:
1 parent
f4ba205c9f
commit
05b0ea4762
6 files changed
+76
-5
No files matched your search
@@ -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
|
||||||
|
|||||||
@@ -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."""
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
Reference in new issue
Block a user