From d97e36300977695fa89cebaa228b13427662c0a1 Mon Sep 17 00:00:00 2001 From: DavidKoleczek <45405824+DavidKoleczek@users.noreply.github.com> Date: Wed, 26 Aug 2026 11:47:17 -0400 Subject: [PATCH] feat(prompt): add --prompt-file and take the prompt off argv when large MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prompt travelled as the final positional argv element, which imposed two transport limits on caller content. Size: Linux caps a single argv element at MAX_ARG_STRLEN (131072 bytes) and Windows caps the whole command line at 32767 chars. Past either, execve fails with E2BIG before the engine boots, surfacing as an opaque spawn failure with no actionable remedy. The engine had no other input path. Leading '-': a positional beginning with '-' is parsed as an option, so the turn died with exit 2 and "No such option" before the engine booted. Callers that compose a prompt from templated context blocks hit this routinely. The wrapper contract already applied the right mitigation to a much smaller field, spilling MCP server config to a file "so a large server map cannot overflow the OS argv limit". The prompt, the field most likely to be large, was left on argv. This extends that same treatment to it. Engine gains --prompt-file , read as UTF-8 and delivered verbatim. The positional PROMPT remains valid and unchanged; the two are mutually exclusive, with argv_prompt_conflict and argv_prompt_file_unreadable as section 4.1 envelopes at exit 2, matching the existing argv-validation convention. Both wrappers now emit an unconditional -- before a positional prompt, and spill a prompt of 16384 UTF-8 bytes or more to a 0600 file. The threshold is measured on encoded byte length rather than character count, because the OS limits are byte limits, and sits under the smaller Windows ceiling rather than the Linux one. Also fixes the Python wrapper's MCP config spill, which wrote in text mode with no explicit encoding. That inherits the locale codepage, cp1252 on Windows, while the reader opens the file as utf-8-sig and degrades a decode failure into a warning reporting no configured servers. The TypeScript wrapper was already correct. Scope of impact: coordinated cross-component. Engine, both wrappers, and the two specs that define the contract. docs/spec/cli.md and docs/spec/wrapper-contract.md updated in the same change. Adds tests/e2e/suites/prompt_input/, which fails on the previous code with "No such option '--prompt-file'" and passes here, plus TypeScript coverage for the new spill module. ๐Ÿค– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- CHANGELOG.md | 31 +++ docs/spec/cli.md | 21 +- docs/spec/wrapper-contract.md | 28 ++- src/amplifier_agent_cli/modes/single_turn.py | 47 +++- tests/e2e/suites/prompt_input/__init__.py | 1 + .../suites/prompt_input/test_prompt_input.py | 232 ++++++++++++++++++ .../src/amplifier_agent_py/argv_builder.py | 23 +- .../src/amplifier_agent_py/mcp_spill.py | 11 +- .../src/amplifier_agent_py/prompt_spill.py | 135 ++++++++++ .../src/amplifier_agent_py/session.py | 21 ++ wrappers/typescript/dist/argv-builder.d.ts | 15 +- wrappers/typescript/dist/argv-builder.js | 15 +- wrappers/typescript/dist/prompt-spill.d.ts | 55 +++++ wrappers/typescript/dist/prompt-spill.js | 99 ++++++++ wrappers/typescript/dist/session.d.ts | 1 + wrappers/typescript/dist/session.js | 21 ++ wrappers/typescript/src/argv-builder.ts | 31 ++- wrappers/typescript/src/prompt-spill.ts | 120 +++++++++ wrappers/typescript/src/session.ts | 28 +++ wrappers/typescript/test/argv-builder.test.ts | 1 + wrappers/typescript/test/prompt-spill.test.ts | 172 +++++++++++++ 21 files changed, 1095 insertions(+), 13 deletions(-) create mode 100644 tests/e2e/suites/prompt_input/__init__.py create mode 100644 tests/e2e/suites/prompt_input/test_prompt_input.py create mode 100644 wrappers/python-py/src/amplifier_agent_py/prompt_spill.py create mode 100644 wrappers/typescript/dist/prompt-spill.d.ts create mode 100644 wrappers/typescript/dist/prompt-spill.js create mode 100644 wrappers/typescript/src/prompt-spill.ts create mode 100644 wrappers/typescript/test/prompt-spill.test.ts 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(); + }); +});