Skip to content

fix(codex): pass prompts via stdin#137

Merged
PleasePrompto merged 1 commit into
PleasePrompto:mainfrom
attid:fix/codex-prompt-stdin
Jul 12, 2026
Merged

fix(codex): pass prompts via stdin#137
PleasePrompto merged 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. 👍

@attid
attid force-pushed the fix/codex-prompt-stdin branch from cc89f40 to b228658 Compare July 10, 2026 15:13
@PleasePrompto
PleasePrompto marked this pull request as ready for review July 12, 2026 06:08
@PleasePrompto
PleasePrompto merged commit 3eebb2f into PleasePrompto:main Jul 12, 2026
PleasePrompto added a commit that referenced this pull request Jul 12, 2026
- drop the now-unused windows_only parameter from _feed_stdin_and_close
- feed streaming stdin concurrently with the stdout read loop: a prompt
  larger than the OS pipe buffer could deadlock against a child that
  emits stdout before draining stdin (oneshot path already used
  communicate() and was safe)
- update #168-era resume-command tests to the stdin prompt marker

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PleasePrompto added a commit that referenced this pull request Jul 12, 2026
Post-merge fixes threading the Slack transport through main's newer
subsystems (#164 per-session effort, #157 append_system_prompt_files,
#166 topic-scoped kills, #137/#138 task changes):

- Delete fork-only .github/workflows/upstream-release-watch.yml
- Remove dead _is_message_addressed() in messenger/slack/bot.py
- Fix trailing comma in config.example.json Slack section (invalid JSON)
- Drop fork-only "ductor-slack" brand strings from en/wizard.toml
- Harden _normalized_transport_list() in __main__.py: honor a non-empty
  transports list whenever the primary is a member (primary-first, others
  preserved) instead of collapsing to [primary] on any ordering mismatch,
  so telegram+matrix multi-transport setups are never silently dropped;
  add regression tests
- named.py create(): keep both reasoning_effort and key params (# noqa:
  PLR0913, matching the codebase convention e.g. tasks/registry.py)
- Update #164 create() test stubs for the new key kwarg
- mypy: guard the optional slack_bolt import behind TYPE_CHECKING/else to
  satisfy strict no-redef
- uv lock: add slack-bolt/slack-sdk (also picks up pre-existing lock drift)
- ruff format integration-touched files

Note: orch._sessions.list_active_for_chat() in slack/bot.py follows the
existing private-access convention (telegram/startup.py does the same);
no public accessor exists.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hyoseop1231 pushed a commit to hyoseop1231/ductor that referenced this pull request Jul 21, 2026
Wires opencode (opencode.ai) in as a full provider alongside
Claude/Codex/Gemini/Antigravity, following the same pattern as the
existing providers:

- opencode_provider.py: async wrapper around `opencode run --format
  json`. Composes the prompt (system_prompt + prompt + append_system_
  prompt, no --system-prompt flag) and pipes it via stdin (no message
  positional -- keeps it out of argv/cmdline, matching PleasePrompto#137's stdin
  convention for all providers). `--auto` for bypassPermissions (opencode
  has no sandbox levels, just one auto-approve switch); `--variant` for
  reasoning_effort; `--session <id>` to resume.
- opencode_events.py: JSONL parser for opencode's event shape (verified
  against a live opencode CLI, not guessed):
  `{"type":"text"|"reasoning"|"tool_use"|"step_start"|"step_finish"|
  "error", "sessionID":..., "part":{...}}`. tool_use parts already
  arrive `state.status == "completed"` with no separate in-progress
  signal in current opencode versions; OpencodeToolDedup guards against
  a future version emitting both an in-progress and completed line for
  the same callID.
- ModelRegistry.provider_for(): any model ID containing "/" routes to
  opencode -- opencode's own model IDs are "provider/model" strings
  (e.g. "minimax/MiniMax-M2.5"), a convention no other provider here
  uses, so this is unambiguous. This is what makes `/model <id>`
  actually route to the new provider instead of falling through to the
  Codex catch-all.
- auth.py: check_opencode_auth() -- binary presence + `opencode models`
  returning output as a proxy for "at least one provider configured"
  (opencode manages its own multi-provider credentials independently).
- factory.py, service.py (CLIParameters + cli_parameters_for_provider),
  core.py (both CLIServiceConfig construction sites), providers.py
  (active_provider_name, default_model_for_provider, resolve_session_
  directive, build_provider_info), config.py (CLIParametersConfig),
  init_wizard.py (onboarding CLI detection + i18n strings),
  cli_commands/agents.py (sub-agent provider picker).

Scope notes (matches existing precedent -- antigravity is excluded from
the same places): opencode is NOT added to param_resolver.py's
_TASK_PROVIDERS (cron/webhook task execution), skill_sync.py's
_SYNCABLE_PROVIDERS, rules_selector.py, or welcome.py's auth summary --
none of these are exhaustively maintained for every provider today.
There is also no dynamic opencode model-list cache/discovery (unlike
Gemini/Antigravity/Codex) in this pass -- users pick an exact
`provider/model` string themselves; `default_model_for_provider`
returns "" (opencode CLI picks its own default), matching Gemini's
existing "" pattern.

Tests: 54 new (opencode_events + opencode_provider command-building/
parse_output/final-result). Full tests/cli/ + tests/config/ suite
(714 tests) passes; tests/orchestrator/ (401 tests, pre-existing slow
suite) verified separately.
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