diff --git a/CHANGELOG.md b/CHANGELOG.md index b4fc29d5..1c9e061a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,37 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`amplifier-agent run --prompt-file `,** a second transport for the prompt. The + positional `PROMPT` argument remains valid and unchanged; the two are mutually exclusive. + File contents are decoded as UTF-8 and delivered verbatim, with no stripping and no newline + translation. Supplying both raises `argv_prompt_conflict`; an unreadable or non-UTF-8 file + raises `argv_prompt_file_unreadable`. Both are §4.1 envelopes with exit 2, matching the + existing argv-validation convention. + + argv is a bounded channel: Linux caps a single argv element at 131072 bytes and Windows caps + the whole command line at 32767 chars. Past either, `execve` fails with `E2BIG` before the + engine boots, and a prompt is unbounded caller data. The wrapper contract already spilled the + much smaller MCP config to a file for exactly this reason; the prompt did not. + +### Fixed + +- **Wrappers now emit `--` before the positional prompt.** A prompt beginning with `-` was + parsed as an option, so the turn died with exit 2 and `No such option` before the engine + booted. The separator is emitted unconditionally rather than only for `-`-leading prompts. + Affects both the Python and TypeScript wrappers. + +- **Wrappers spill a prompt of 16384 UTF-8 bytes or more to a `0600` file** and pass + `--prompt-file`, so a large prompt can no longer overflow the OS argv limit. The threshold is + measured on encoded byte length, not character count, because the OS limits are byte limits. + +- **The Python wrapper's MCP config spill now writes with an explicit UTF-8 encoding.** It used + text mode with no encoding, which inherits the locale codepage — cp1252 on Windows — while the + reader opens the file as `utf-8-sig`. A non-ASCII server config would either fail to write or + fail to decode, and the reader turns a decode failure into a warning and reports no configured + servers. The TypeScript wrapper was already correct. + ## [0.16.0] — 2026-08-25 ### Changed diff --git a/docs/spec/cli.md b/docs/spec/cli.md index 8e88eb88..3b2cde5c 100644 --- a/docs/spec/cli.md +++ b/docs/spec/cli.md @@ -65,13 +65,26 @@ streams. Do not add a check for it. ### Prompt discipline +The prompt has two mutually exclusive transports: the positional `PROMPT` argument and +`--prompt-file `. Both are valid at every size. + ``` -PROMPT omitted, stdin IS a TTY -> stderr: "Missing argument 'PROMPT'." exit 2 -PROMPT omitted, stdin NOT a TTY -> stderr line, exit 2: - [error] prompt_required: pass prompt as argument: `amplifier-agent run "..."`. +PROMPT given, --prompt-file absent -> the positional is the prompt +--prompt-file given, PROMPT absent -> file contents are the prompt +BOTH given -> envelope, argv_prompt_conflict exit 2 +--prompt-file unreadable / not UTF-8 -> envelope, argv_prompt_file_unreadable exit 2 + +PROMPT omitted, stdin IS a TTY -> stderr: "Missing argument 'PROMPT'." exit 2 +PROMPT omitted, stdin NOT a TTY -> stderr line, exit 2: + [error] prompt_required: pass prompt as argument: `amplifier-agent run "..."`, + or from a file: `amplifier-agent run --prompt-file `. ``` -The non-TTY branch writes a bare stderr line, not an envelope. +The non-TTY branch writes a bare stderr line. The two `--prompt-file` rejections write a +§4.1 envelope, matching the existing argv-validation convention. + +File contents are decoded as `utf-8` (not `utf-8-sig`) and delivered verbatim: no +stripping, no newline translation. ### Verbosity and approval resolution diff --git a/docs/spec/wrapper-contract.md b/docs/spec/wrapper-contract.md index 305bf1d1..19c54745 100644 --- a/docs/spec/wrapper-contract.md +++ b/docs/spec/wrapper-contract.md @@ -80,9 +80,13 @@ run [--display text|ndjson] (only when explicitly set) [--workspace ] (only when set and non-empty) -y | -n | (nothing) (approval policy) - (final positional) +--prompt-file (when the prompt was spilled) +-- (otherwise; the -- separator is ALWAYS emitted) ``` +Exactly one of the last two lines is emitted, never both. The `--` separator is emitted +unconditionally, not only when the prompt begins with `-`. + argv assembly must be a pure transformation of already-resolved inputs. Spill file writing, environment resolution, and capability composition happen before it, not inside it. @@ -212,6 +216,28 @@ An empty or absent map produces no file and no path. When a path was produced, t up through its own config discovery. Spill file cleanup is an idempotent unlink that tolerates a missing file, performed on every iterator exit path and again on cancellation. +### Prompt spill + +A prompt at or above the threshold is spilled to a file and passed as `--prompt-file `, so a +large prompt cannot overflow the OS argv limit. + +``` +threshold 16384 bytes, measured on the UTF-8 encoded length, not the character count +base dir $XDG_RUNTIME_DIR/amplifier-agent (typically tmpfs on Linux) + /amplifier-agent (fallback) +path //prompt.txt directory mode 0700, file mode 0600 +content the prompt text verbatim, UTF-8, no newline translation +``` + +Below the threshold the prompt stays positional, behind the `--` separator. Both transports are +valid at every size; a wrapper MAY spill unconditionally. + +The file is written and closed before the subprocess is spawned. Cleanup is the same idempotent +unlink as the MCP spill, on the same exit paths. + +Mode bits are POSIX-only. On Windows the file's confidentiality rests on the per-user ACL of the +system temp directory instead. + ## Session handle lifecycle Creating a handle does no subprocess work beyond the version probe: it validates lifecycle, rejects diff --git a/src/amplifier_agent_cli/modes/single_turn.py b/src/amplifier_agent_cli/modes/single_turn.py index 3d18a600..2de7a101 100644 --- a/src/amplifier_agent_cli/modes/single_turn.py +++ b/src/amplifier_agent_cli/modes/single_turn.py @@ -568,6 +568,13 @@ async def _execute_turn(spec: _TurnSpec) -> dict[str, Any]: @click.command() @click.argument("prompt", required=False, default=None) +@click.option( + "--prompt-file", + "prompt_file", + default=None, + type=click.Path(), + help="Read the prompt from a UTF-8 file instead of the positional argument.", +) @click.option("--session-id", default=None, help="Session ID to resume or tag.") @click.option("--resume", is_flag=True, default=False, help="Resume an existing session.") @click.option("--fresh", is_flag=True, default=False, help="Force a fresh session (discard saved state).") @@ -640,6 +647,7 @@ async def _execute_turn(spec: _TurnSpec) -> dict[str, Any]: ) def run( prompt: str | None, + prompt_file: str | None, session_id: str | None, resume: bool, fresh: bool, @@ -700,11 +708,48 @@ def run( raise click.UsageError("--resume and --fresh are mutually exclusive") # (3) Prompt discipline. + # + # The prompt has two mutually exclusive transports: the positional argument + # and --prompt-file. The file path exists because argv is a bounded channel: + # Linux caps one argv element at MAX_ARG_STRLEN (131072 bytes) and Windows + # caps the whole command line at 32767 chars, so a large turn context cannot + # ride on argv at all. A file also sidesteps click's option parsing, which + # would otherwise read a '-'-leading prompt as a flag. + if prompt is not None and prompt_file is not None: + # Two sources of truth for one field is ambiguous. Silently preferring + # one would hide the calling host's bug, so reject instead. + _emit_argv_envelope( + "argv_prompt_conflict", + "Both a positional PROMPT and --prompt-file were supplied; the prompt has exactly one source.", + exit_code=2, + remediation="Pass the prompt EITHER as the positional argument OR via --prompt-file, not both.", + ) + return # unreachable; _emit_argv_envelope calls sys.exit + + if prompt_file is not None: + # encoding="utf-8" is REQUIRED, not stylistic: text mode without it + # inherits locale.getencoding(), which is cp1252 on Windows, and this + # file is machine-written UTF-8. Plain "utf-8" (not "utf-8-sig") so a + # legitimate leading U+FEFF in caller text survives instead of being + # silently swallowed. Content is delivered verbatim -- no strip, no + # transform; the prompt is caller data. + try: + prompt = Path(prompt_file).expanduser().read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as exc: + _emit_argv_envelope( + "argv_prompt_file_unreadable", + f"Could not read --prompt-file {prompt_file!r}: {exc}", + exit_code=2, + remediation="Ensure the path exists, is readable by this process, and contains UTF-8 encoded text.", + ) + return # unreachable; _emit_argv_envelope calls sys.exit + if prompt is None: if is_stdin_tty(): raise click.UsageError("Missing argument 'PROMPT'.") click.echo( - '[error] prompt_required: pass prompt as argument: `amplifier-agent run "..."`.', + '[error] prompt_required: pass prompt as argument: `amplifier-agent run "..."`, ' + "or from a file: `amplifier-agent run --prompt-file `.", err=True, ) sys.exit(2) diff --git a/tests/e2e/suites/prompt_input/__init__.py b/tests/e2e/suites/prompt_input/__init__.py new file mode 100644 index 00000000..df720634 --- /dev/null +++ b/tests/e2e/suites/prompt_input/__init__.py @@ -0,0 +1 @@ +"""Prompt-input suite: how a prompt reaches the engine, regardless of its size or shape.""" diff --git a/tests/e2e/suites/prompt_input/test_prompt_input.py b/tests/e2e/suites/prompt_input/test_prompt_input.py new file mode 100644 index 00000000..b75d7551 --- /dev/null +++ b/tests/e2e/suites/prompt_input/test_prompt_input.py @@ -0,0 +1,232 @@ +"""E2E: a prompt reaches the engine regardless of its SIZE or its leading characters. + +Contract under test +------------------- +A prompt is caller data. Its size and its first character are properties of the user's +content, not of our transport, so neither may decide whether a turn can run at all. + + prompt of any size -> the turn runs + prompt beginning with '-' -> the turn runs, the text is not parsed as options + +Today the prompt travels as the final positional argv element +(``docs/spec/wrapper-contract.md``, "argv assembly"), which imposes two limits that +belong to the transport rather than to the content: + +1. SIZE. Linux caps a single argv element at ``MAX_ARG_STRLEN`` (32 pages = 131072 + bytes); Windows caps the whole command line at 32767 chars. Past that, ``execve`` + fails with ``E2BIG`` before the engine boots. There is no other input path: the run + command takes only a positional prompt and the engine has no stdin ingestion + (``docs/spec/cli.md``). + +2. LEADING '-'. A positional that begins with '-' is parsed by click as an option, so + the turn dies with exit 2 before the engine boots -- unless a ``--`` separator + precedes it. + +The same spec already applies the right mitigation to a SMALLER field +(``docs/spec/wrapper-contract.md``, "MCP config spill"): + + "MCP server configuration is always spilled to a file, never passed on argv, + so a large server map cannot overflow the OS argv limit." + +The prompt is the field most likely to be large, and it was left on argv. + +Cases +----- +``prompt-file-oversized`` RED -- >MAX_ARG_STRLEN prompt via --prompt-file runs +``argv-separator-leading-dashes`` CONTROL -- '--' makes a '-'-leading positional run +``prompt-file-rejects-both-inputs`` RED -- --prompt-file AND a positional is a caller error + +Why ``prompt-file-oversized`` also begins with '---' +---------------------------------------------------- +It folds the leading-dash half of the contract into the same model call. A file-borne +prompt never touches argv, so its first character must be irrelevant; asserting that +here costs nothing extra and pins both properties of the file path at once. + +Why ``argv-separator-leading-dashes`` is a CONTROL and not RED +-------------------------------------------------------------- +The engine already honours ``--`` today. That is precisely why it is worth pinning: the +wrapper-side fix for a '-'-leading prompt is to emit ``--`` before the positional, so +this case is the engine-side guarantee that fix depends on. If a future click upgrade or +a ``context_settings`` change breaks it, the wrappers break silently and this case names +the cause. + +Why this is a bespoke suite (not ``framework.harness``) +------------------------------------------------------- +``run_cli_case`` hardcodes ``exit_code == 0`` and its ``check`` callable never sees +stderr or the exit code, so it cannot express ``prompt-file-rejects-both-inputs``. This +follows the precedent set by ``suites/modes/test_unknown_mode.py`` and +``suites/skills/test_sigil_dispatch.py`` and builds its commands locally, leaving +``framework/`` untouched (docs/E2E_TESTING.md: "stable; rarely touched"). + +Why the payload is generated INSIDE the DTU +-------------------------------------------- +``dtu.exec_json`` is ``shlex.join``ed into one string and run as ``bash -lc ``, +so that string is itself a single argv element on the HOST and is bound by the host's +own ``MAX_ARG_STRLEN``. Passing a 200000-byte literal would fail on the host before it +ever reached the container, testing the harness instead of the engine. Generating the +payload in-container keeps the outer command ~100 bytes and puts the size pressure +exactly where the contract lives. Generation and the run share ONE exec because the DTU +filesystem is not guaranteed to persist between exec calls. +""" + +from __future__ import annotations + +import json + +import pytest +from framework import dtu + +pytestmark = pytest.mark.dtu + +# Host-config seeded into every DTU by provisioning (anthropic provider, approval "yes"). +_CONFIG = "/root/e2e/host-config.json" + +_PROMPT_FILE = "/tmp/e2e-oversized-prompt.txt" + +# Comfortably past Linux MAX_ARG_STRLEN (131072). Chosen so the case still fails on a +# kernel with a larger page size rather than passing vacuously. +_OVERSIZE_BYTES = 200_000 + +# A short, closed-class reply keeps the assertion unambiguous and the completion cheap. +_SENTINEL = "banana" + +# Leading '---' so the file path's indifference to the first character is pinned too. +_LEAD = "--- turn context ---" + +_INSTRUCTION = f"Ignore the padding above. Reply with exactly one word: {_SENTINEL}" + + +def _generate_prompt_file() -> str: + """A shell fragment that writes an oversized, '---'-leading prompt to _PROMPT_FILE.""" + return ( + "python3 - <<'PYEOF' > " + _PROMPT_FILE + "\n" + "import sys\n" + f"lead = {_LEAD!r}\n" + f"instruction = {_INSTRUCTION!r}\n" + f"pad_to = {_OVERSIZE_BYTES}\n" + "line = 'padding that carries no instruction.\\n'\n" + "body = line * (max(0, pad_to - len(lead) - len(instruction)) // len(line) + 1)\n" + "sys.stdout.write(lead + '\\n' + body + '\\n' + instruction + '\\n')\n" + "PYEOF" + ) + + +def _envelope(result: dict, context: str) -> dict: + """Parse the section 4.1 JSON envelope from a run's stdout.""" + stdout = result.get("stdout", "") + for line in reversed(stdout.splitlines()): + line = line.strip() + if line.startswith("{"): + try: + return json.loads(line) + except json.JSONDecodeError: + continue + raise AssertionError( + f"[{context}] no JSON envelope on stdout\n" + f"exit_code: {result.get('exit_code')}\n" + f"stdout:\n{stdout}\nstderr:\n{result.get('stderr', '')}" + ) + + +def _fail_detail(context: str, result: dict) -> str: + return ( + f"[{context}] exit_code={result.get('exit_code')}\n" + f"stdout:\n{result.get('stdout', '')}\n" + f"stderr:\n{result.get('stderr', '')}" + ) + + +def test_prompt_file_oversized(dtu_id: str) -> None: + """A prompt past the OS argv limit runs, and its leading '-' is not parsed as an option. + + RED until the engine grows a non-argv prompt input path. The failure to expect on old + code is exit 2 with "No such option '--prompt-file'". + """ + command = ( + _generate_prompt_file() + + "\n" + + " ".join( + [ + "amplifier-agent run -y", + f"--config {_CONFIG}", + "--output json", + f"--prompt-file {_PROMPT_FILE}", + ] + ) + ) + result = dtu.exec_json(dtu_id, ["bash", "-lc", command]) + + context = "prompt-file-oversized" + assert result.get("exit_code") == 0, _fail_detail(context, result) + + envelope = _envelope(result, context) + assert not envelope.get("error"), f"[{context}] unexpected error: {envelope.get('error')!r}" + assert _SENTINEL in envelope.get("reply", "").lower(), ( + f"[{context}] expected {_SENTINEL!r} in reply, got {envelope.get('reply', '')!r}" + ) + + +def test_argv_separator_preserves_leading_dashes(dtu_id: str) -> None: + """`--` makes a '-'-leading positional prompt reach the engine intact. + + CONTROL. Green today. This is the engine-side guarantee the wrapper fix relies on + when it emits `--` before the positional prompt. + """ + prompt = f"{_LEAD}\n{_INSTRUCTION}" + result = dtu.exec_json( + dtu_id, + [ + "amplifier-agent", + "run", + "-y", + "--config", + _CONFIG, + "--output", + "json", + "--", + prompt, + ], + ) + + context = "argv-separator-leading-dashes" + assert result.get("exit_code") == 0, _fail_detail(context, result) + + envelope = _envelope(result, context) + assert not envelope.get("error"), f"[{context}] unexpected error: {envelope.get('error')!r}" + assert _SENTINEL in envelope.get("reply", "").lower(), ( + f"[{context}] expected {_SENTINEL!r} in reply, got {envelope.get('reply', '')!r}" + ) + + +def test_prompt_file_rejects_both_inputs(dtu_id: str) -> None: + """Supplying BOTH --prompt-file and a positional prompt is a caller error. + + Two sources of truth for one field is ambiguous, and silently preferring one would + make a host's bug invisible. Mirrors the existing argv-validation convention: the + section 4.1 envelope on stdout, `error.code` set, exit 2. Costs no model call. + + RED until --prompt-file exists. + """ + command = ( + _generate_prompt_file() + + "\n" + + " ".join( + [ + "amplifier-agent run -y", + f"--config {_CONFIG}", + "--output json", + f"--prompt-file {_PROMPT_FILE}", + "-- 'a positional prompt as well'", + ] + ) + ) + result = dtu.exec_json(dtu_id, ["bash", "-lc", command]) + + context = "prompt-file-rejects-both-inputs" + assert result.get("exit_code") == 2, _fail_detail(context, result) + + envelope = _envelope(result, context) + error = envelope.get("error") or {} + assert error.get("code") == "argv_prompt_conflict", ( + f"[{context}] expected error.code == 'argv_prompt_conflict', got {error!r}" + ) diff --git a/wrappers/python-py/src/amplifier_agent_py/argv_builder.py b/wrappers/python-py/src/amplifier_agent_py/argv_builder.py index fab44eb3..30ce8631 100644 --- a/wrappers/python-py/src/amplifier_agent_py/argv_builder.py +++ b/wrappers/python-py/src/amplifier_agent_py/argv_builder.py @@ -23,7 +23,12 @@ class AssembleArgvInput: wrappers/typescript/src/argv-builder.ts exactly. - ``session_id`` — caller-supplied; never generated here. - - ``prompt`` — emitted as the final positional argument. + - ``prompt`` — emitted as the final positional argument, behind a + literal ``--`` separator, when ``prompt_file`` is None. + - ``prompt_file`` — path the prompt was spilled to upstream; when set, + emits ``--prompt-file `` and NO positional + prompt. The spill itself happens in ``session.py`` + (this module does no I/O). - ``protocol_version`` — emitted via ``--protocol-version ``. - ``resume`` — when True, emit ``--resume``; else ``--fresh``. - ``cwd`` — emits ``--cwd `` when set. @@ -37,6 +42,7 @@ class AssembleArgvInput: session_id: str prompt: str + prompt_file: str | None = None protocol_version: str resume: bool = False cwd: str | None = None @@ -103,7 +109,20 @@ def assemble_argv(input_: AssembleArgvInput) -> list[str]: argv.append("-n") # mode == "prompt": deliberately emit no flag. - # Prompt is the final positional argument. + # Prompt transport. When the prompt was spilled to a file upstream it rides + # on --prompt-file and NO positional is emitted; argv has hard size ceilings + # (131072 bytes per element on Linux, 32767 chars for the whole command line + # on Windows) that a large turn context blows straight past. + if input_.prompt_file is not None: + argv.extend(["--prompt-file", input_.prompt_file]) + return argv + + # Otherwise the prompt is the final positional argument, always preceded by + # a literal "--". Unconditional, not conditional on a leading '-': a guard + # is a second thing to get wrong, and the engine already accepts "--" for + # every prompt. Without it, click parses a '-'-leading prompt as an option + # and the turn dies exit 2 with "No such option". + argv.append("--") argv.append(input_.prompt) return argv diff --git a/wrappers/python-py/src/amplifier_agent_py/mcp_spill.py b/wrappers/python-py/src/amplifier_agent_py/mcp_spill.py index 340d3c73..5629cea8 100644 --- a/wrappers/python-py/src/amplifier_agent_py/mcp_spill.py +++ b/wrappers/python-py/src/amplifier_agent_py/mcp_spill.py @@ -92,10 +92,19 @@ def resolve_mcp_config_path( # Write with restrictive perms. We write to the final path with 0600 # using os.open() so file contents are never world-readable. + # + # encoding="utf-8" is REQUIRED, not stylistic. Text mode without it inherits + # locale.getencoding(), which is cp1252 (or another ANSI codepage) on Windows + # unless PEP 540 UTF-8 mode is active. The reader in amplifier-module-tool-mcp + # opens this file as "utf-8-sig", so a non-ASCII server config -- a path, an arg, + # an env value -- would either raise UnicodeEncodeError here or decode wrong + # there, and that reader swallows the failure into a logger.warning and returns + # "no servers configured". The TypeScript wrapper's writeFile already defaults + # to utf-8; this keeps the two in parity. flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC fd = os.open(str(file_path), flags, 0o600) try: - with os.fdopen(fd, "w") as f: + with os.fdopen(fd, "w", encoding="utf-8") as f: f.write(payload) except Exception: # If write fails, ensure the partial file does not linger. diff --git a/wrappers/python-py/src/amplifier_agent_py/prompt_spill.py b/wrappers/python-py/src/amplifier_agent_py/prompt_spill.py new file mode 100644 index 00000000..4d70a63b --- /dev/null +++ b/wrappers/python-py/src/amplifier_agent_py/prompt_spill.py @@ -0,0 +1,135 @@ +"""Prompt spill-to-file for oversized prompts. + +Companion to :mod:`mcp_spill` and structured identically. Large prompts cannot +ride on argv: the prompt travels as the final positional element, and a single +argv element is capped at ``MAX_ARG_STRLEN`` (131072 bytes) on Linux while the +whole command line is capped at 32767 chars on Windows. Past those ceilings the +spawn fails with ``E2BIG`` before the engine even boots. So the wrapper spills +the prompt to a 0600 tmpfile under +``${XDG_RUNTIME_DIR || tempfile.gettempdir()}/amplifier-agent//prompt.txt`` +and passes ``--prompt-file `` instead of the positional argument. + +This mirrors the treatment the MCP server map already gets — see +docs/spec/wrapper-contract.md: "MCP server configuration is always spilled to a +file, never passed on argv, so a large server map cannot overflow the OS argv +limit." + +``cleanup_spill_file()`` is the matching teardown — idempotent unlink that +swallows ``FileNotFoundError`` so callers can call it unconditionally on every +exit path. +""" + +from __future__ import annotations + +import os +import tempfile +from dataclasses import dataclass +from pathlib import Path + +# Spill any prompt whose UTF-8 encoded length reaches this many bytes. +# +# The threshold must sit safely under the SMALLEST platform ceiling. That is +# NOT Linux's 131072-byte per-element cap — it is Windows' 32767-CHARACTER cap +# on the ENTIRE command line, which the prompt shares with the binary path and +# every other flag (--session-id, --config, --cwd, --workspace, ...). 16384 +# leaves roughly half of that budget as headroom for the rest of the argv, and +# still keeps ordinary prompts on the fast in-memory path. +# +# The comparison is made against the UTF-8 ENCODED byte length +# (``len(prompt.encode("utf-8"))``), never ``len(prompt)``: a multibyte prompt +# occupies more bytes on the wire than it has characters, and the OS limit is +# denominated in bytes. +PROMPT_SPILL_THRESHOLD_BYTES = 16384 + + +@dataclass(frozen=True, kw_only=True) +class PromptSpillResult: + """Result of deciding whether to spill the prompt to a tmpfile. + + When the prompt fits on argv: ``prompt_file`` is ``None`` and the caller + passes the prompt positionally as before. + Otherwise: ``prompt_file`` points at the 0600 spill file containing the + prompt text verbatim. + """ + + prompt_file: str | None + + +def _spill_base_dir() -> Path: + """Compute the base directory for spill files. + + Prefers ``$XDG_RUNTIME_DIR/amplifier-agent`` (typically a tmpfs on Linux) + and falls back to ``tempfile.gettempdir()/amplifier-agent`` otherwise. + """ + xdg = os.environ.get("XDG_RUNTIME_DIR") + if xdg: + return Path(xdg) / "amplifier-agent" + return Path(tempfile.gettempdir()) / "amplifier-agent" + + +def resolve_prompt_file_path(prompt: str, session_id: str) -> PromptSpillResult: + """Spill *prompt* to a 0600 tmpfile when it is too large for argv. + + Args: + prompt: The caller's prompt text. + session_id: Session identifier; used as the per-session subdirectory + under the spill base so concurrent sessions never clash. + + Returns: + ``PromptSpillResult`` with the on-disk prompt path, or ``None`` when the + prompt is small enough to travel as a positional argv element. + """ + if len(prompt.encode("utf-8")) < PROMPT_SPILL_THRESHOLD_BYTES: + return PromptSpillResult(prompt_file=None) + + base = _spill_base_dir() + session_dir = base / session_id + session_dir.mkdir(parents=True, exist_ok=True) + # Tighten the per-session directory to 0700. + try: + session_dir.chmod(0o700) + except PermissionError: + # Best effort; if we cannot tighten perms, still proceed with the file write. + pass + + file_path = session_dir / "prompt.txt" + + # Write with restrictive perms. We write to the final path with 0600 using + # os.open() so the prompt -- which is caller data and may carry secrets -- + # is never world-readable. + # + # encoding="utf-8" is REQUIRED, not stylistic. Text mode without it inherits + # locale.getencoding(), which is cp1252 (or another ANSI codepage) on Windows + # unless PEP 540 UTF-8 mode is active, and the engine reads this file as + # strict "utf-8". newline="" is equally load-bearing: the default newline + # translation rewrites every "\n" to "\r\n" on Windows, which would silently + # corrupt the caller's prompt in transit. + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC + fd = os.open(str(file_path), flags, 0o600) + try: + with os.fdopen(fd, "w", encoding="utf-8", newline="") as f: + f.write(prompt) + except Exception: + # If write fails, ensure the partial file does not linger. + try: + file_path.unlink() + except FileNotFoundError: + pass + raise + + return PromptSpillResult(prompt_file=str(file_path)) + + +def cleanup_prompt_spill_file(prompt_file: str | None) -> None: + """Idempotently remove a prompt spill file. + + Safe to call with ``None`` (no-op) and safe to call when the file is + already gone (``FileNotFoundError`` swallowed). Other I/O errors + propagate. + """ + if not prompt_file: + return + try: + os.unlink(prompt_file) + except FileNotFoundError: + return diff --git a/wrappers/python-py/src/amplifier_agent_py/session.py b/wrappers/python-py/src/amplifier_agent_py/session.py index d04ee36b..97bfeff2 100644 --- a/wrappers/python-py/src/amplifier_agent_py/session.py +++ b/wrappers/python-py/src/amplifier_agent_py/session.py @@ -40,6 +40,7 @@ from .argv_builder import ApprovalMode, AssembleArgvInput, DisplayMode, assemble_argv from .errors import AaaError from .mcp_spill import cleanup_spill_file, resolve_mcp_config_path +from .prompt_spill import cleanup_prompt_spill_file, resolve_prompt_file_path from .run_output_parser import STDERR_TAIL_BYTES, SubprocessOutcome, parse_run_output from .types import ( ActivityEvent, @@ -117,6 +118,7 @@ def __init__(self, params: SessionHandleParams) -> None: self._submitted = False self._subprocess: asyncio.subprocess.Process | None = None self._mcp_spill_path: str | None = None + self._prompt_spill_path: str | None = None self._timeout_cancel_task: asyncio.Task[None] | None = None self._engine_info = EngineInfo( binary_path=params.binary_path, @@ -152,11 +154,22 @@ async def _make_iterable(self, prompt: str) -> AsyncIterator[DisplayEvent]: spill = resolve_mcp_config_path(self._params.mcp_servers, self._params.session_id) self._mcp_spill_path = spill.config_path + # (ii-b) Same treatment for an oversized prompt: spill it to a 0600 + # tmpfile so it never has to fit through argv, which is capped at + # 131072 bytes per element on Linux and 32767 chars for the whole + # command line on Windows. prompt_file is None when the prompt is + # small enough to travel positionally. The file is fully written + # and closed here, BEFORE the spawn below, because on Windows a + # child cannot open a file the parent still holds. + prompt_spill = resolve_prompt_file_path(prompt, self._params.session_id) + self._prompt_spill_path = prompt_spill.prompt_file + # (iii) Build argv (pure function — no I/O). argv = assemble_argv( AssembleArgvInput( session_id=self._params.session_id, prompt=prompt, + prompt_file=prompt_spill.prompt_file, protocol_version=self._params.protocol_version, resume=self._params.resume, cwd=self._params.cwd, @@ -212,6 +225,8 @@ def finalize(ev: DisplayEvent) -> None: ) cleanup_spill_file(self._mcp_spill_path) self._mcp_spill_path = None + cleanup_prompt_spill_file(self._prompt_spill_path) + self._prompt_spill_path = None return self._subprocess = proc @@ -341,6 +356,8 @@ async def watch_timeout() -> None: await t cleanup_spill_file(self._mcp_spill_path) self._mcp_spill_path = None + cleanup_prompt_spill_file(self._prompt_spill_path) + self._prompt_spill_path = None async def cancel(self) -> None: """Cancel the running subprocess via SIGTERM-then-SIGKILL. @@ -370,6 +387,10 @@ async def cancel(self) -> None: path = self._mcp_spill_path self._mcp_spill_path = None cleanup_spill_file(path) + if self._prompt_spill_path is not None: + prompt_path = self._prompt_spill_path + self._prompt_spill_path = None + cleanup_prompt_spill_file(prompt_path) async def dispose(self) -> None: """Graceful shutdown — alias for ``cancel()`` (D3).""" diff --git a/wrappers/typescript/dist/argv-builder.d.ts b/wrappers/typescript/dist/argv-builder.d.ts index cdabd62a..2a8c7b4f 100644 --- a/wrappers/typescript/dist/argv-builder.d.ts +++ b/wrappers/typescript/dist/argv-builder.d.ts @@ -12,8 +12,21 @@ export interface AssembleArgvInput { /** Session identifier (provided by caller, never generated here). */ sessionId: string; - /** Final user prompt — emitted last as a positional argument. */ + /** + * Final user prompt — emitted last as a positional argument, behind a + * literal `--` separator. Ignored when `promptFile` is set. + */ prompt: string; + /** + * Path to a spill file holding the prompt, produced upstream by + * `resolvePromptFilePath` in `prompt-spill.ts`. When set, the prompt rides + * on `--prompt-file ` and NO positional prompt is emitted; the two + * transports are mutually exclusive and the engine rejects both at once + * with `argv_prompt_conflict`. + * + * Defaults to undefined, so callers that never spill are unaffected. + */ + promptFile?: string; /** Protocol version the wrapper speaks (e.g. "0.3.0"). */ protocolVersion: string; /** When true, emit `--resume` instead of `--fresh`. */ diff --git a/wrappers/typescript/dist/argv-builder.js b/wrappers/typescript/dist/argv-builder.js index bd44cfdf..0204185f 100644 --- a/wrappers/typescript/dist/argv-builder.js +++ b/wrappers/typescript/dist/argv-builder.js @@ -77,7 +77,20 @@ export function assembleArgv(input) { else { // mode === "prompt": deliberately emit no flag. } - // Prompt is the final positional argument. + // Prompt transport. When the prompt was spilled to a file upstream it rides + // on --prompt-file and NO positional is emitted; argv has hard size ceilings + // (131072 bytes per element on Linux, 32767 chars for the whole command line + // on Windows) that a large turn context blows straight past. + if (input.promptFile !== undefined) { + argv.push("--prompt-file", input.promptFile); + return argv; + } + // Otherwise the prompt is the final positional argument, always preceded by + // a literal "--". Unconditional, not conditional on a leading '-': a guard + // is a second thing to get wrong, and the engine already accepts "--" for + // every prompt. Without it, click parses a '-'-leading prompt as an option + // and the turn dies exit 2 with "No such option". + argv.push("--"); argv.push(input.prompt); return argv; } diff --git a/wrappers/typescript/dist/prompt-spill.d.ts b/wrappers/typescript/dist/prompt-spill.d.ts new file mode 100644 index 00000000..3004904f --- /dev/null +++ b/wrappers/typescript/dist/prompt-spill.d.ts @@ -0,0 +1,55 @@ +/** + * Spill any prompt whose UTF-8 encoded length reaches this many bytes. + * + * The threshold must sit safely under the SMALLEST platform ceiling. That is + * NOT Linux's 131072-byte per-element cap — it is Windows' 32767-CHARACTER cap + * on the ENTIRE command line, which the prompt shares with the binary path and + * every other flag (`--session-id`, `--config`, `--cwd`, `--workspace`, ...). + * 16384 leaves roughly half of that budget as headroom for the rest of the + * argv, and still keeps ordinary prompts on the fast in-memory path. + * + * The comparison is made against the UTF-8 ENCODED byte length + * (`Buffer.byteLength(prompt, "utf8")`), never `prompt.length`: a multibyte + * prompt occupies more bytes on the wire than it has characters, and the OS + * limit is denominated in bytes. + */ +export declare const PROMPT_SPILL_THRESHOLD_BYTES = 16384; +/** + * Result of deciding whether to spill the prompt to a tmpfile. + * + * - When the prompt fits on argv: `promptFile` is `null` and the caller passes + * the prompt positionally as before. + * - Otherwise: `promptFile` points at the 0600 spill file containing the + * prompt text verbatim, and the caller emits `--prompt-file ` with no + * positional prompt. + */ +export interface PromptSpillResult { + promptFile: string | null; +} +/** + * Resolve the prompt file path to pass as `--prompt-file`. + * + * Spills to a 0600 tmpfile under a 0700 per-session dir when the prompt's + * UTF-8 byte length reaches `PROMPT_SPILL_THRESHOLD_BYTES`; otherwise does no + * I/O at all and reports `promptFile: null`. + * + * The write completes — and the file handle is closed — before this promise + * resolves, which matters on Windows, where a child process cannot open a file + * the parent still holds. The content is the prompt text verbatim: UTF-8, with + * no newline translation, because the engine reads the file as strict `utf-8` + * and any `\n` → `\r\n` rewrite would corrupt the caller's prompt in transit. + * + * @param prompt The caller's prompt text. + * @param sessionId Session identifier; used as the per-session subdirectory + * under the spill base so concurrent sessions never clash. + * + * @returns A `PromptSpillResult` with the on-disk prompt path (or null when + * the prompt is small enough to travel as a positional argv element). + */ +export declare function resolvePromptFilePath(prompt: string, sessionId: string): Promise; +/** + * Idempotently remove a prompt spill file. Safe to call with `null` (no-op) + * and safe to call when the file is already gone (ENOENT swallowed). Other + * I/O errors propagate. + */ +export declare function cleanupPromptSpillFile(promptFile: string | null | undefined): Promise; diff --git a/wrappers/typescript/dist/prompt-spill.js b/wrappers/typescript/dist/prompt-spill.js new file mode 100644 index 00000000..7fa2bfd4 --- /dev/null +++ b/wrappers/typescript/dist/prompt-spill.js @@ -0,0 +1,99 @@ +/** + * prompt-spill.ts — oversized-prompt spill-to-file resolution. + * + * Companion to `mcp-spill.ts` and structured identically. A large prompt + * cannot ride on argv: it travels as the final positional element, and a + * single argv element is capped at `MAX_ARG_STRLEN` (131072 bytes) on Linux + * while the whole command line is capped at 32767 chars on Windows. Past + * those ceilings the spawn fails with `E2BIG` before the engine even boots. + * So the wrapper spills the prompt to a 0600 tmpfile under + * `${XDG_RUNTIME_DIR || os.tmpdir()}/amplifier-agent//prompt.txt` + * and passes `--prompt-file ` instead of the positional argument. + * + * This mirrors the treatment the MCP server map already gets — see + * docs/spec/wrapper-contract.md, "Prompt spill". + * + * `cleanupPromptSpillFile` is the matching teardown — idempotent unlink that + * swallows ENOENT so callers can call it unconditionally on every exit path. + */ +import { mkdir, writeFile, unlink } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +/** + * Spill any prompt whose UTF-8 encoded length reaches this many bytes. + * + * The threshold must sit safely under the SMALLEST platform ceiling. That is + * NOT Linux's 131072-byte per-element cap — it is Windows' 32767-CHARACTER cap + * on the ENTIRE command line, which the prompt shares with the binary path and + * every other flag (`--session-id`, `--config`, `--cwd`, `--workspace`, ...). + * 16384 leaves roughly half of that budget as headroom for the rest of the + * argv, and still keeps ordinary prompts on the fast in-memory path. + * + * The comparison is made against the UTF-8 ENCODED byte length + * (`Buffer.byteLength(prompt, "utf8")`), never `prompt.length`: a multibyte + * prompt occupies more bytes on the wire than it has characters, and the OS + * limit is denominated in bytes. + */ +export const PROMPT_SPILL_THRESHOLD_BYTES = 16384; +/** + * Compute the base directory for spill files. Prefers + * `$XDG_RUNTIME_DIR/amplifier-agent` (typically a tmpfs on Linux) and falls + * back to `os.tmpdir()/amplifier-agent` otherwise. + */ +function spillBaseDir() { + const xdg = process.env["XDG_RUNTIME_DIR"]; + if (xdg && xdg.length > 0) { + return join(xdg, "amplifier-agent"); + } + return join(tmpdir(), "amplifier-agent"); +} +/** + * Resolve the prompt file path to pass as `--prompt-file`. + * + * Spills to a 0600 tmpfile under a 0700 per-session dir when the prompt's + * UTF-8 byte length reaches `PROMPT_SPILL_THRESHOLD_BYTES`; otherwise does no + * I/O at all and reports `promptFile: null`. + * + * The write completes — and the file handle is closed — before this promise + * resolves, which matters on Windows, where a child process cannot open a file + * the parent still holds. The content is the prompt text verbatim: UTF-8, with + * no newline translation, because the engine reads the file as strict `utf-8` + * and any `\n` → `\r\n` rewrite would corrupt the caller's prompt in transit. + * + * @param prompt The caller's prompt text. + * @param sessionId Session identifier; used as the per-session subdirectory + * under the spill base so concurrent sessions never clash. + * + * @returns A `PromptSpillResult` with the on-disk prompt path (or null when + * the prompt is small enough to travel as a positional argv element). + */ +export async function resolvePromptFilePath(prompt, sessionId) { + if (Buffer.byteLength(prompt, "utf8") < PROMPT_SPILL_THRESHOLD_BYTES) { + return { promptFile: null }; + } + const dir = join(spillBaseDir(), sessionId); + await mkdir(dir, { recursive: true, mode: 0o700 }); + const filePath = join(dir, "prompt.txt"); + // The prompt is caller data and may carry secrets, so the file is created + // 0600. `writeFile` opens, writes and closes before the promise resolves. + await writeFile(filePath, prompt, { encoding: "utf8", mode: 0o600 }); + return { promptFile: filePath }; +} +/** + * Idempotently remove a prompt spill file. Safe to call with `null` (no-op) + * and safe to call when the file is already gone (ENOENT swallowed). Other + * I/O errors propagate. + */ +export async function cleanupPromptSpillFile(promptFile) { + if (!promptFile) + return; + try { + await unlink(promptFile); + } + catch (err) { + const code = err.code; + if (code === "ENOENT") + return; + throw err; + } +} diff --git a/wrappers/typescript/dist/session.d.ts b/wrappers/typescript/dist/session.d.ts index 3f1c4fb7..69224ff2 100644 --- a/wrappers/typescript/dist/session.d.ts +++ b/wrappers/typescript/dist/session.d.ts @@ -230,6 +230,7 @@ export declare class SessionHandle { private submitted; private subprocess; private mcpSpillPath; + private promptSpillPath; private readonly engineInfo; constructor(params: SessionHandleParams); /** Return resolved engine metadata (D5). */ diff --git a/wrappers/typescript/dist/session.js b/wrappers/typescript/dist/session.js index 956ba58e..6a9f545e 100644 --- a/wrappers/typescript/dist/session.js +++ b/wrappers/typescript/dist/session.js @@ -27,6 +27,7 @@ import { spawn as childSpawn } from "node:child_process"; import { assembleArgv } from "./argv-builder.js"; import { resolveMcpConfigPath, cleanupSpillFile } from "./mcp-spill.js"; +import { resolvePromptFilePath, cleanupPromptSpillFile, } from "./prompt-spill.js"; import { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; import { parseNdjsonStream } from "./transport.js"; /** Typed error for AaA wrapper lifecycle and protocol violations. */ @@ -99,6 +100,7 @@ export class SessionHandle { submitted = false; subprocess = null; mcpSpillPath = null; + promptSpillPath = null; engineInfo; constructor(params) { this.params = params; @@ -157,6 +159,15 @@ export class SessionHandle { // rejects assignability without the cast. const spill = await resolveMcpConfigPath((this.params.mcpServers ?? null), this.params.sessionId); this.mcpSpillPath = spill.configPath; + // (ii-b) Same treatment for an oversized prompt: spill it to a 0600 + // tmpfile and pass --prompt-file, because argv is capped at 131072 bytes + // per element on Linux and 32767 chars for the whole command line on + // Windows. `promptFile` is null when the prompt is small enough to ride + // on argv positionally. This await also guarantees the file is fully + // written AND closed before the child is spawned — on Windows a child + // cannot open a file the parent still holds. + const promptSpill = await resolvePromptFilePath(prompt, this.params.sessionId); + this.promptSpillPath = promptSpill.promptFile; // (iii) build argv (pure function — no I/O). The MCP config path is // forwarded to the engine via AMPLIFIER_MCP_CONFIG (subprocess env) // rather than via an argv flag. `configPath` (Issue #1) and @@ -165,6 +176,9 @@ export class SessionHandle { const argv = assembleArgv({ sessionId: this.params.sessionId, prompt, + ...(promptSpill.promptFile !== null + ? { promptFile: promptSpill.promptFile } + : {}), protocolVersion: this.params.protocolVersion, resume: this.params.resume, cwd: this.params.cwd, @@ -344,6 +358,8 @@ export class SessionHandle { clearTimeout(timeoutHandle); await cleanupSpillFile(this.mcpSpillPath); this.mcpSpillPath = null; + await cleanupPromptSpillFile(this.promptSpillPath); + this.promptSpillPath = null; } } /** @@ -384,6 +400,11 @@ export class SessionHandle { this.mcpSpillPath = null; await cleanupSpillFile(path); } + if (this.promptSpillPath !== null) { + const promptPath = this.promptSpillPath; + this.promptSpillPath = null; + await cleanupPromptSpillFile(promptPath); + } } /** Graceful shutdown — alias for `cancel()` (D3). */ async dispose() { diff --git a/wrappers/typescript/src/argv-builder.ts b/wrappers/typescript/src/argv-builder.ts index 07b67958..396c9e24 100644 --- a/wrappers/typescript/src/argv-builder.ts +++ b/wrappers/typescript/src/argv-builder.ts @@ -13,8 +13,21 @@ export interface AssembleArgvInput { /** Session identifier (provided by caller, never generated here). */ sessionId: string; - /** Final user prompt — emitted last as a positional argument. */ + /** + * Final user prompt — emitted last as a positional argument, behind a + * literal `--` separator. Ignored when `promptFile` is set. + */ prompt: string; + /** + * Path to a spill file holding the prompt, produced upstream by + * `resolvePromptFilePath` in `prompt-spill.ts`. When set, the prompt rides + * on `--prompt-file ` and NO positional prompt is emitted; the two + * transports are mutually exclusive and the engine rejects both at once + * with `argv_prompt_conflict`. + * + * Defaults to undefined, so callers that never spill are unaffected. + */ + promptFile?: string; /** Protocol version the wrapper speaks (e.g. "0.3.0"). */ protocolVersion: string; /** When true, emit `--resume` instead of `--fresh`. */ @@ -155,7 +168,21 @@ export function assembleArgv(input: AssembleArgvInput): string[] { // mode === "prompt": deliberately emit no flag. } - // Prompt is the final positional argument. + // Prompt transport. When the prompt was spilled to a file upstream it rides + // on --prompt-file and NO positional is emitted; argv has hard size ceilings + // (131072 bytes per element on Linux, 32767 chars for the whole command line + // on Windows) that a large turn context blows straight past. + if (input.promptFile !== undefined) { + argv.push("--prompt-file", input.promptFile); + return argv; + } + + // Otherwise the prompt is the final positional argument, always preceded by + // a literal "--". Unconditional, not conditional on a leading '-': a guard + // is a second thing to get wrong, and the engine already accepts "--" for + // every prompt. Without it, click parses a '-'-leading prompt as an option + // and the turn dies exit 2 with "No such option". + argv.push("--"); argv.push(input.prompt); return argv; diff --git a/wrappers/typescript/src/prompt-spill.ts b/wrappers/typescript/src/prompt-spill.ts new file mode 100644 index 00000000..22a4ee2e --- /dev/null +++ b/wrappers/typescript/src/prompt-spill.ts @@ -0,0 +1,120 @@ +/** + * prompt-spill.ts — oversized-prompt spill-to-file resolution. + * + * Companion to `mcp-spill.ts` and structured identically. A large prompt + * cannot ride on argv: it travels as the final positional element, and a + * single argv element is capped at `MAX_ARG_STRLEN` (131072 bytes) on Linux + * while the whole command line is capped at 32767 chars on Windows. Past + * those ceilings the spawn fails with `E2BIG` before the engine even boots. + * So the wrapper spills the prompt to a 0600 tmpfile under + * `${XDG_RUNTIME_DIR || os.tmpdir()}/amplifier-agent//prompt.txt` + * and passes `--prompt-file ` instead of the positional argument. + * + * This mirrors the treatment the MCP server map already gets — see + * docs/spec/wrapper-contract.md, "Prompt spill". + * + * `cleanupPromptSpillFile` is the matching teardown — idempotent unlink that + * swallows ENOENT so callers can call it unconditionally on every exit path. + */ +import { mkdir, writeFile, unlink } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/** + * Spill any prompt whose UTF-8 encoded length reaches this many bytes. + * + * The threshold must sit safely under the SMALLEST platform ceiling. That is + * NOT Linux's 131072-byte per-element cap — it is Windows' 32767-CHARACTER cap + * on the ENTIRE command line, which the prompt shares with the binary path and + * every other flag (`--session-id`, `--config`, `--cwd`, `--workspace`, ...). + * 16384 leaves roughly half of that budget as headroom for the rest of the + * argv, and still keeps ordinary prompts on the fast in-memory path. + * + * The comparison is made against the UTF-8 ENCODED byte length + * (`Buffer.byteLength(prompt, "utf8")`), never `prompt.length`: a multibyte + * prompt occupies more bytes on the wire than it has characters, and the OS + * limit is denominated in bytes. + */ +export const PROMPT_SPILL_THRESHOLD_BYTES = 16384; + +/** + * Result of deciding whether to spill the prompt to a tmpfile. + * + * - When the prompt fits on argv: `promptFile` is `null` and the caller passes + * the prompt positionally as before. + * - Otherwise: `promptFile` points at the 0600 spill file containing the + * prompt text verbatim, and the caller emits `--prompt-file ` with no + * positional prompt. + */ +export interface PromptSpillResult { + promptFile: string | null; +} + +/** + * Compute the base directory for spill files. Prefers + * `$XDG_RUNTIME_DIR/amplifier-agent` (typically a tmpfs on Linux) and falls + * back to `os.tmpdir()/amplifier-agent` otherwise. + */ +function spillBaseDir(): string { + const xdg = process.env["XDG_RUNTIME_DIR"]; + if (xdg && xdg.length > 0) { + return join(xdg, "amplifier-agent"); + } + return join(tmpdir(), "amplifier-agent"); +} + +/** + * Resolve the prompt file path to pass as `--prompt-file`. + * + * Spills to a 0600 tmpfile under a 0700 per-session dir when the prompt's + * UTF-8 byte length reaches `PROMPT_SPILL_THRESHOLD_BYTES`; otherwise does no + * I/O at all and reports `promptFile: null`. + * + * The write completes — and the file handle is closed — before this promise + * resolves, which matters on Windows, where a child process cannot open a file + * the parent still holds. The content is the prompt text verbatim: UTF-8, with + * no newline translation, because the engine reads the file as strict `utf-8` + * and any `\n` → `\r\n` rewrite would corrupt the caller's prompt in transit. + * + * @param prompt The caller's prompt text. + * @param sessionId Session identifier; used as the per-session subdirectory + * under the spill base so concurrent sessions never clash. + * + * @returns A `PromptSpillResult` with the on-disk prompt path (or null when + * the prompt is small enough to travel as a positional argv element). + */ +export async function resolvePromptFilePath( + prompt: string, + sessionId: string, +): Promise { + if (Buffer.byteLength(prompt, "utf8") < PROMPT_SPILL_THRESHOLD_BYTES) { + return { promptFile: null }; + } + + const dir = join(spillBaseDir(), sessionId); + await mkdir(dir, { recursive: true, mode: 0o700 }); + const filePath = join(dir, "prompt.txt"); + // The prompt is caller data and may carry secrets, so the file is created + // 0600. `writeFile` opens, writes and closes before the promise resolves. + await writeFile(filePath, prompt, { encoding: "utf8", mode: 0o600 }); + + return { promptFile: filePath }; +} + +/** + * Idempotently remove a prompt spill file. Safe to call with `null` (no-op) + * and safe to call when the file is already gone (ENOENT swallowed). Other + * I/O errors propagate. + */ +export async function cleanupPromptSpillFile( + promptFile: string | null | undefined, +): Promise { + if (!promptFile) return; + try { + await unlink(promptFile); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code; + if (code === "ENOENT") return; + throw err; + } +} diff --git a/wrappers/typescript/src/session.ts b/wrappers/typescript/src/session.ts index 0fcbc591..16dc3648 100644 --- a/wrappers/typescript/src/session.ts +++ b/wrappers/typescript/src/session.ts @@ -30,6 +30,10 @@ import type { ChildProcess, SpawnOptions } from "node:child_process"; import { assembleArgv } from "./argv-builder.js"; import { resolveMcpConfigPath, cleanupSpillFile } from "./mcp-spill.js"; +import { + resolvePromptFilePath, + cleanupPromptSpillFile, +} from "./prompt-spill.js"; import { parseRunOutput, STDERR_TAIL_BYTES } from "./run-output-parser.js"; import { parseNdjsonStream } from "./transport.js"; import type { McpServerConfig } from "./types.js"; @@ -283,6 +287,7 @@ export class SessionHandle { private submitted = false; private subprocess: ChildProcess | null = null; private mcpSpillPath: string | null = null; + private promptSpillPath: string | null = null; private readonly engineInfo: EngineInfo; constructor(private readonly params: SessionHandleParams) { @@ -354,6 +359,19 @@ export class SessionHandle { ); this.mcpSpillPath = spill.configPath; + // (ii-b) Same treatment for an oversized prompt: spill it to a 0600 + // tmpfile and pass --prompt-file, because argv is capped at 131072 bytes + // per element on Linux and 32767 chars for the whole command line on + // Windows. `promptFile` is null when the prompt is small enough to ride + // on argv positionally. This await also guarantees the file is fully + // written AND closed before the child is spawned — on Windows a child + // cannot open a file the parent still holds. + const promptSpill = await resolvePromptFilePath( + prompt, + this.params.sessionId, + ); + this.promptSpillPath = promptSpill.promptFile; + // (iii) build argv (pure function — no I/O). The MCP config path is // forwarded to the engine via AMPLIFIER_MCP_CONFIG (subprocess env) // rather than via an argv flag. `configPath` (Issue #1) and @@ -362,6 +380,9 @@ export class SessionHandle { const argv = assembleArgv({ sessionId: this.params.sessionId, prompt, + ...(promptSpill.promptFile !== null + ? { promptFile: promptSpill.promptFile } + : {}), protocolVersion: this.params.protocolVersion, resume: this.params.resume, cwd: this.params.cwd, @@ -547,6 +568,8 @@ export class SessionHandle { if (timeoutHandle !== null) clearTimeout(timeoutHandle); await cleanupSpillFile(this.mcpSpillPath); this.mcpSpillPath = null; + await cleanupPromptSpillFile(this.promptSpillPath); + this.promptSpillPath = null; } } @@ -588,6 +611,11 @@ export class SessionHandle { this.mcpSpillPath = null; await cleanupSpillFile(path); } + if (this.promptSpillPath !== null) { + const promptPath = this.promptSpillPath; + this.promptSpillPath = null; + await cleanupPromptSpillFile(promptPath); + } } /** Graceful shutdown — alias for `cancel()` (D3). */ diff --git a/wrappers/typescript/test/argv-builder.test.ts b/wrappers/typescript/test/argv-builder.test.ts index 349bcc6e..5dcfbab6 100644 --- a/wrappers/typescript/test/argv-builder.test.ts +++ b/wrappers/typescript/test/argv-builder.test.ts @@ -37,6 +37,7 @@ describe("assembleArgv", () => { "--protocol-version", "0.2.0", "-y", + "--", "hello", ]); }); diff --git a/wrappers/typescript/test/prompt-spill.test.ts b/wrappers/typescript/test/prompt-spill.test.ts new file mode 100644 index 00000000..e0407723 --- /dev/null +++ b/wrappers/typescript/test/prompt-spill.test.ts @@ -0,0 +1,172 @@ +/** + * Tests for prompt-spill.ts: resolvePromptFilePath() and cleanupPromptSpillFile() + * + * TDD cases: + * (i) a prompt under the threshold returns { promptFile: null } and writes + * nothing + * (ii) a prompt of exactly PROMPT_SPILL_THRESHOLD_BYTES bytes spills (the + * comparison is `< threshold`, so the boundary value itself spills) + * (iii) a prompt one byte below the threshold does not spill + * (iv) the threshold is measured in UTF-8 BYTES, not characters: a prompt + * with fewer characters than the threshold but more bytes must spill + * (v) the spilled file round-trips the prompt verbatim (no newline + * translation, no stripping, non-ASCII preserved) + * (vi) file mode is 0600 + * (vii) promptFile is //prompt.txt + * (viii) cleanupPromptSpillFile removes the file + * (ix) cleanupPromptSpillFile is idempotent (ENOENT is fine) + * (x) cleanupPromptSpillFile is a no-op for null/undefined + */ +import { describe, it, expect, afterEach } from "vitest"; +import { stat, readFile, access, rm } from "node:fs/promises"; + +import { + resolvePromptFilePath, + cleanupPromptSpillFile, + PROMPT_SPILL_THRESHOLD_BYTES, +} from "../src/prompt-spill.js"; +import type { PromptSpillResult } from "../src/prompt-spill.js"; + +const SID = "test-session-prompt"; + +// Track every spill file created across tests so afterEach cleans them up +// even when a test fails mid-assertion. +const created: string[] = []; + +afterEach(async () => { + while (created.length > 0) { + const p = created.pop(); + if (!p) continue; + try { + await rm(p, { force: true }); + } catch { + /* swallow */ + } + } +}); + +describe("resolvePromptFilePath", () => { + it("(i) returns {promptFile: null} for a prompt under the threshold", async () => { + const prompt = "a short prompt that rides on argv"; + const result: PromptSpillResult = await resolvePromptFilePath(prompt, SID); + expect(result).toEqual({ promptFile: null }); + }); + + it("(ii) spills a prompt of exactly PROMPT_SPILL_THRESHOLD_BYTES bytes", async () => { + const prompt = "a".repeat(PROMPT_SPILL_THRESHOLD_BYTES); + expect(Buffer.byteLength(prompt, "utf8")).toBe(PROMPT_SPILL_THRESHOLD_BYTES); + + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + created.push(result.promptFile!); + + const contents = await readFile(result.promptFile!, "utf8"); + expect(contents).toBe(prompt); + }); + + it("(iii) does not spill a prompt one byte below the threshold", async () => { + const prompt = "a".repeat(PROMPT_SPILL_THRESHOLD_BYTES - 1); + expect(Buffer.byteLength(prompt, "utf8")).toBe( + PROMPT_SPILL_THRESHOLD_BYTES - 1, + ); + + const result = await resolvePromptFilePath(prompt, SID); + expect(result).toEqual({ promptFile: null }); + }); + + it("(iv) measures UTF-8 bytes, not characters — spills when characters < threshold but bytes >= threshold", async () => { + // 6000 CJK characters at 3 bytes each = 18000 bytes. The character count + // (6000) is far below the threshold; the byte count is above it. A check + // written against `prompt.length` would wrongly skip the spill here. + const prompt = "日".repeat(6000); + expect(prompt.length).toBeLessThan(PROMPT_SPILL_THRESHOLD_BYTES); + expect(Buffer.byteLength(prompt, "utf8")).toBeGreaterThanOrEqual( + PROMPT_SPILL_THRESHOLD_BYTES, + ); + + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + created.push(result.promptFile!); + + const contents = await readFile(result.promptFile!, "utf8"); + expect(contents).toBe(prompt); + }); + + it("(v) round-trips the prompt verbatim — leading '---', embedded and trailing newlines, non-ASCII", async () => { + const prompt = + "---\ntitle: spill round trip\n---\n" + + "naïve café — 日本語のテキスト\nsecond line\n".repeat(600); + expect(prompt.startsWith("---")).toBe(true); + expect(prompt.endsWith("\n")).toBe(true); + expect(Buffer.byteLength(prompt, "utf8")).toBeGreaterThanOrEqual( + PROMPT_SPILL_THRESHOLD_BYTES, + ); + + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + created.push(result.promptFile!); + + // Byte-for-byte identical: no newline translation, no stripping. + const contents = await readFile(result.promptFile!, "utf8"); + expect(contents).toBe(prompt); + expect(Buffer.byteLength(contents, "utf8")).toBe( + Buffer.byteLength(prompt, "utf8"), + ); + }); + + it("(vi) spills with 0600 mode", async () => { + const prompt = "s".repeat(PROMPT_SPILL_THRESHOLD_BYTES); + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + created.push(result.promptFile!); + + // File mode should be 0600 (owner read/write only) + const st = await stat(result.promptFile!); + const mode = st.mode & 0o777; + expect(mode).toBe(0o600); + }); + + it("(vii) promptFile is prompt.txt under the per-session spill dir", async () => { + const prompt = "p".repeat(PROMPT_SPILL_THRESHOLD_BYTES); + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + created.push(result.promptFile!); + + expect(result.promptFile!).toMatch( + /amplifier-agent[/\\]test-session-prompt[/\\]prompt\.txt$/, + ); + }); + + it("(viii) cleanupPromptSpillFile removes the spilled file", async () => { + const prompt = "c".repeat(PROMPT_SPILL_THRESHOLD_BYTES); + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + const path = result.promptFile!; + + // File exists before cleanup + await expect(access(path)).resolves.toBeUndefined(); + + await expect(cleanupPromptSpillFile(path)).resolves.toBeUndefined(); + + // File is gone after cleanup + await expect(access(path)).rejects.toThrow(); + }); + + it("(ix) cleanupPromptSpillFile is idempotent — second call on missing file does not throw", async () => { + const prompt = "i".repeat(PROMPT_SPILL_THRESHOLD_BYTES); + const result = await resolvePromptFilePath(prompt, SID); + expect(result.promptFile).not.toBeNull(); + const path = result.promptFile!; + + // First cleanup removes it + await expect(cleanupPromptSpillFile(path)).resolves.toBeUndefined(); + + // Second cleanup on missing path must not throw (ENOENT swallowed) + await expect(cleanupPromptSpillFile(path)).resolves.toBeUndefined(); + }); + + it("(x) cleanupPromptSpillFile is a no-op for null and undefined input", async () => { + await expect(cleanupPromptSpillFile(null)).resolves.toBeUndefined(); + await expect(cleanupPromptSpillFile(undefined)).resolves.toBeUndefined(); + }); +});