Skip to content

fix(codex): pass prompts via stdin#137

Draft
attid wants to merge 1 commit into
PleasePrompto:mainfrom
attid:fix/codex-prompt-stdin
Draft

fix(codex): pass prompts via stdin#137
attid wants to merge 1 commit into
PleasePrompto:mainfrom
attid:fix/codex-prompt-stdin

Conversation

@attid

@attid attid commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Pass Codex prompts through stdin instead of argv on all platforms.

Why

Ductor composes prompts with hooks, background task context, and memory. Passing the full prompt as a command-line argument can exceed Linux argv limits before the Codex CLI starts, raising OSError: [Errno 7] Argument list too long.

Changes

  • Use - as the Codex prompt marker for new and resumed codex exec calls.
  • Add optional stdin payload support to the shared CLI subprocess executor.
  • Keep Dockerized Codex stdin open with docker exec -i so piped prompts arrive.
  • Cover non-streaming, resume, streaming, and CLI parameter command shape with tests.

Validation

  • uv run pytest tests/cli/test_codex_provider.py tests/integration/test_cli_parameter_flow.py tests/cli/test_executor_timeout.py tests/cli/test_claude_provider.py tests/cli/test_docker_wrap.py -q
  • uv run ruff check ductor_bot/cli/base.py ductor_bot/cli/executor.py ductor_bot/cli/codex_provider.py tests/cli/test_codex_provider.py tests/integration/test_cli_parameter_flow.py
  • uv run mypy ductor_bot

@PleasePrompto

Copy link
Copy Markdown
Owner

Thanks — this is the right direction: the argv-limit problem is real for ductor's composed prompts, and moving to stdin also keeps the prompt out of /proc/<pid>/cmdline and logs, so it's a net security win too. Before un-drafting:

  1. Rebase needed: Fix Windows compatibility and Telegram streaming edge cases #147 just merged and touches tests/cli/test_codex_provider.py (same _make_process_mock helpers) and claude_provider.py, so this branch will conflict.
  2. Dead parameter: after this change nothing sets windows_only=True on _feed_stdin_and_close (cli/base.py) — please remove it.
  3. Streaming deadlock risk: in the streaming path the whole prompt is written + drained before the stdout read loop starts. For prompts larger than the OS pipe buffer (~64 KiB — exactly the use case this PR targets), if codex starts emitting stdout before consuming stdin, drain() and the child can deadlock. The oneshot path is safe (communicate), the streaming path isn't — feeding stdin concurrently (asyncio.create_task) or at least documenting the assumption would close this.
  4. Consistency note: claude_provider.py still passes the prompt via argv on POSIX and has the same latent Argument list too long bug. Fine to defer, but please state in the PR whether claude moves to stdin_text here or in a follow-up.

Looking forward to merging this once rebased. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants