fix(issue-36): merge review - a prompt file the runner cannot write is a failed submit; S9 asserts the libc wording on Linux only; an empty prompt file is refused
- runner._run_cmd: the write of the prompt file was outside the cleanup and error handling that the exec had (merge review: claude H1, codex R1). A failed write now removes the partial file and returns the same 126 failed submit an unstartable wrapper does, so the loop ends with rc 1 and no traceback. Test S4c. - test S9 required the literal "File too large" in stderr; bash 3.2 on macOS prints nothing for EFBIG, so the whole suite failed on this host (agy, grok, codex R2). The contract asserted everywhere is rc != 0, no leftover file and the portable diagnostic line; the libc wording is checked on Linux only. - the wrapper accepted an empty --prompt-file and registered an empty prompt (claude H3); it is refused now. Test S1e. 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
59be1c8920
commit
708262a598
3 files changed
+63
-7
No files matched your search
@@ -469,12 +469,26 @@ class LoopRunner:
|
|||||||
# MAX_ARG_STRLEN (131,072 bytes on Linux), and a review prompt
|
# MAX_ARG_STRLEN (131,072 bytes on Linux), and a review prompt
|
||||||
# carries the whole diff. The wrapper blocks until the job ends,
|
# carries the whole diff. The wrapper blocks until the job ends,
|
||||||
# so nothing reads the file after it returns.
|
# so nothing reads the file after it returns.
|
||||||
self.mam_dir.mkdir(parents=True, exist_ok=True)
|
prompt_file = ""
|
||||||
with tempfile.NamedTemporaryFile("w", encoding="utf-8", dir=self.mam_dir,
|
try:
|
||||||
prefix=f"prompt-{self.loop_id}-", suffix=".md",
|
self.mam_dir.mkdir(parents=True, exist_ok=True)
|
||||||
delete=False) as fh:
|
with tempfile.NamedTemporaryFile("w", encoding="utf-8", dir=self.mam_dir,
|
||||||
fh.write(p_text)
|
prefix=f"prompt-{self.loop_id}-", suffix=".md",
|
||||||
prompt_file = fh.name
|
delete=False) as fh:
|
||||||
|
prompt_file = fh.name
|
||||||
|
fh.write(p_text)
|
||||||
|
except OSError as e:
|
||||||
|
# The same class the shell paths handle (merge review H1): a
|
||||||
|
# partial file is removed and the submit fails like an exec
|
||||||
|
# error would, instead of the exception reaching the loop.
|
||||||
|
if prompt_file:
|
||||||
|
try:
|
||||||
|
os.unlink(prompt_file)
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
msg = (f"cannot write the prompt file under {self.mam_dir}: {e} "
|
||||||
|
f"(prompt {len(p_text.encode('utf-8'))} bytes)")
|
||||||
|
return subprocess.CompletedProcess(["bash", str(wrapper_script), "submit"], 126, "", msg)
|
||||||
cmd = [
|
cmd = [
|
||||||
"bash", str(wrapper_script), "submit",
|
"bash", str(wrapper_script), "submit",
|
||||||
"--agent-session", self.session_label(agent_session),
|
"--agent-session", self.session_label(agent_session),
|
||||||
|
|||||||
@@ -190,6 +190,7 @@ cmd_submit() {
|
|||||||
[[ -z "$PROMPT" ]] || { echo "submit takes --prompt or --prompt-file, not both" >&2; exit 1; }
|
[[ -z "$PROMPT" ]] || { echo "submit takes --prompt or --prompt-file, not both" >&2; exit 1; }
|
||||||
[[ "$PROMPT_FILE" == /* ]] || PROMPT_FILE="$(pwd)/$PROMPT_FILE"
|
[[ "$PROMPT_FILE" == /* ]] || PROMPT_FILE="$(pwd)/$PROMPT_FILE"
|
||||||
[[ -f "$PROMPT_FILE" && -r "$PROMPT_FILE" ]] || { echo "submit: cannot read --prompt-file $PROMPT_FILE" >&2; exit 1; }
|
[[ -f "$PROMPT_FILE" && -r "$PROMPT_FILE" ]] || { echo "submit: cannot read --prompt-file $PROMPT_FILE" >&2; exit 1; }
|
||||||
|
[[ -s "$PROMPT_FILE" ]] || { echo "submit: --prompt-file $PROMPT_FILE is empty" >&2; exit 1; }
|
||||||
PROMPT="$(<"$PROMPT_FILE")"
|
PROMPT="$(<"$PROMPT_FILE")"
|
||||||
fi
|
fi
|
||||||
[[ -n "$PROMPT" || -n "$PROMPT_FILE" ]] || { echo "submit requires --prompt or --prompt-file" >&2; exit 1; }
|
[[ -n "$PROMPT" || -n "$PROMPT_FILE" ]] || { echo "submit requires --prompt or --prompt-file" >&2; exit 1; }
|
||||||
|
|||||||
@@ -276,6 +276,44 @@ def test_s4_an_exec_error_is_a_failed_submit(runner, tmp_path, monkeypatch, caps
|
|||||||
assert _prompt_leftovers(tmp_path) == []
|
assert _prompt_leftovers(tmp_path) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_s4c_a_prompt_file_that_cannot_be_written_is_a_failed_submit(runner, tmp_path, monkeypatch, capsys):
|
||||||
|
"""Merge review H1: the write, not only the exec, can fail (ENOSPC, EFBIG).
|
||||||
|
The runner must report it like an exec error and leave no partial file."""
|
||||||
|
lr, r = runner
|
||||||
|
real = lr.tempfile.NamedTemporaryFile
|
||||||
|
|
||||||
|
class _Broken:
|
||||||
|
def __init__(self, *a, **k):
|
||||||
|
self._fh = real(*a, **k)
|
||||||
|
self.name = self._fh.name
|
||||||
|
def __enter__(self):
|
||||||
|
self._fh.__enter__()
|
||||||
|
return self
|
||||||
|
def __exit__(self, *exc):
|
||||||
|
return self._fh.__exit__(*exc)
|
||||||
|
def write(self, _):
|
||||||
|
raise OSError(errno.ENOSPC, "No space left on device")
|
||||||
|
|
||||||
|
monkeypatch.setattr(lr.tempfile, "NamedTemporaryFile", _Broken)
|
||||||
|
ran = []
|
||||||
|
monkeypatch.setattr(lr.subprocess, "run", lambda *a, **k: ran.append(a) or None)
|
||||||
|
assert r.delegate_job("c", "claude", "Worker", "p" * 1000) is None
|
||||||
|
err = capsys.readouterr().err
|
||||||
|
assert "submit failed (exit 126)" in err and "cannot write the prompt file" in err, err
|
||||||
|
assert "Traceback" not in err
|
||||||
|
assert ran == [], "the wrapper must not be started without a prompt file"
|
||||||
|
assert _prompt_leftovers(tmp_path) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_s1e_an_empty_prompt_file_is_refused(tmp_path):
|
||||||
|
"""Merge review H3: --prompt-file with nothing in it registered an empty job."""
|
||||||
|
reg = tmp_path / "jobs"
|
||||||
|
empty = tmp_path / "empty.md"
|
||||||
|
empty.write_text("")
|
||||||
|
res = _dry_submit(tmp_path, reg, "--prompt-file", str(empty))
|
||||||
|
assert res.returncode != 0 and "is empty" in res.stderr, res.stdout + res.stderr
|
||||||
|
|
||||||
|
|
||||||
def test_s4b_an_exec_error_on_the_busy_retry_is_reported_not_raised(runner, tmp_path, monkeypatch, capsys):
|
def test_s4b_an_exec_error_on_the_busy_retry_is_reported_not_raised(runner, tmp_path, monkeypatch, capsys):
|
||||||
lr, r = runner
|
lr, r = runner
|
||||||
calls = []
|
calls = []
|
||||||
@@ -599,7 +637,10 @@ def test_s9_shell_loop_removes_a_partial_prompt_file_and_says_why(tmp_path):
|
|||||||
'output=$(delegate_job_safe submit --prompt "$(cat input.md)")\n')
|
'output=$(delegate_job_safe submit --prompt "$(cat input.md)")\n')
|
||||||
result = subprocess.run(["bash", str(harness)], cwd=tmp_path, capture_output=True, text=True, timeout=60)
|
result = subprocess.run(["bash", str(harness)], cwd=tmp_path, capture_output=True, text=True, timeout=60)
|
||||||
assert result.returncode != 0
|
assert result.returncode != 0
|
||||||
assert "File too large" in result.stderr
|
if sys.platform == "linux":
|
||||||
|
# bash's own wording for EFBIG; macOS bash 3.2 prints nothing here
|
||||||
|
# (merge review N1), so only the portable line below is the contract
|
||||||
|
assert "File too large" in result.stderr
|
||||||
leftovers = list((tmp_path / ".mam").glob("prompt-shloop-*"))
|
leftovers = list((tmp_path / ".mam").glob("prompt-shloop-*"))
|
||||||
assert not leftovers, [(p.name, p.stat().st_size) for p in leftovers]
|
assert not leftovers, [(p.name, p.stat().st_size) for p in leftovers]
|
||||||
assert "delegate_job_safe: cannot write the prompt file" in result.stderr
|
assert "delegate_job_safe: cannot write the prompt file" in result.stderr
|
||||||
Reference in new issue
Block a user