Commit Graph

16 Commits

  • [codex] Add optional IDs to response items (#28812)
    ## Why
    
    `ResponseItem` variants do not have a consistent internal ID shape: some
    variants carry required IDs, some carry optional IDs, and some cannot
    represent an ID at all. The existing fields also use inconsistent serde,
    TypeScript, and JSON-schema annotations. A single enum-level access path
    is needed before history recording can assign and retain IDs.
    
    This PR establishes that internal model only. It intentionally does not
    generate or serialize IDs; allocation and wire persistence are isolated
    in the stacked follow-up.
    
    ## What changed
    
    - Give every concrete `ResponseItem` variant an `Option<String>` ID
    field.
    - Apply the same internal-only annotations to every ID field:
    `#[serde(default, skip_serializing)]`, `#[ts(skip)]`, and
    `#[schemars(skip)]`.
    - Add `ResponseItem::id()` and `ResponseItem::set_id()` as the shared
    accessors.
    - Preserve IDs when history items are rewritten for truncation.
    - Adapt consumers that previously assumed reasoning and image-generation
    IDs were required.
    - Regenerate app-server schemas so the hidden fields are represented
    consistently.
    
    The serde catch-all `ResponseItem::Other` remains ID-less because it
    must remain a unit variant.
    
    ## Test plan
    
    - `cargo check --tests -p codex-core -p codex-api -p codex-rollout-trace
    -p codex-image-generation-extension`
    - `just test -p codex-protocol`
    - `just test -p codex-app-server-protocol`
    - `just test -p codex-api -p codex-rollout-trace -p
    codex-image-generation-extension`
    - `just test -p codex-core event_mapping`
  • feat(core): add metadata field to ResponseItem (#28355)
    ## Description
    
    This PR adds an optional `metadata` field to `ResponseItem` for
    Responses API calls. Only mechanical plumbing, no actual values
    populated and sent yet. Turns out just adding a new field to
    `ResponseItem` has quite a large blast radius already.
    
    This change is backwards compatible because `metadata` is optional and
    omitted when absent, so existing response items and rollout history
    without it still deserialize and requests that do not set it keep the
    same wire shape. For provider compatibility, we strip out `metadata`
    before non-OpenAI Responses requests so Azure and AWS Bedrock never see
    this field.
    
    My followup PR here will actually make use of it to start storing and
    passing along `turn_id`: https://github.com/openai/codex/pull/28360
    
    ## What changed
    
    - Added `ResponseItemMetadata` with optional `turn_id`, plus optional
    `metadata` on Responses API item variants and inter-agent communication.
    - Preserved item metadata through response-item rewrites such as
    truncation, missing tool-output synthesis, compaction history
    rebuilding, visible-history conversion, rollout/resume, and generated
    app-server schemas/types.
    - Strip item metadata from non-OpenAI Responses requests while
    preserving it for OpenAI-shaped requests.
    - Updated the mechanical fixture/test construction churn required by the
    new optional field.
  • skills: make backend plugin skills invocable without an executor (#27387)
    ## Why
    
    #27198 made the extension-owned `codex_apps` MCP connection the hosted
    plugin runtime, but its `mcp/skill` resources still bypassed the skills
    extension. App-server could list and read those resources through
    generic MCP APIs, but a thread with no selected environment did not
    expose them in the model's skills catalog or load their `SKILL.md`
    through `$skill`.
    
    Hosted skills should stay remote while using the same typed catalog,
    source authority, deduplication, bounded contextual catalog, and
    selected-skill prompt injection as host and executor skills. They should
    not be downloaded or exposed as ambient filesystem paths.
    
    ## What changed
    
    - Add a session-scoped `McpResourceClient` over the replaceable MCP
    connection manager so resource list/read calls follow startup and
    refresh replacements.
    - Add a `BackendSkillProvider` that pages `codex_apps` resources,
    accepts bounded and validated `mcp/skill` entries, and reads a selected
    skill's `SKILL.md` through the same MCP connection.
    - Register the remote provider in app-server and include it in the
    skills catalog even when a thread has no selected capability roots or
    executor.
    - Contribute hosted skill metadata through the bounded
    `AvailableSkillsInstructions` developer-context path, exclude remote
    entries from per-turn catalog injection, and classify `<skills>`
    messages as contextual developer content so rollback can trim and
    rebuild them correctly.
    
    ## Testing
    
    - Extend the app-server MCP resource integration test with
    `environments: []` to exercise two-page discovery, filter a
    non-`mcp/skill` resource, verify the escaped developer catalog entry and
    user-role `<skill>` fragment containing the fetched `SKILL.md`, and
    preserve generic MCP resource reads.
    - Add core event-mapping coverage that classifies `<skills>` developer
    messages as contextual history.
  • [codex] Add token budget context feature (#27438)
    ## Why
    
    The model should be able to see bounded context-window budget metadata
    when the `token_budget` feature is enabled. The full-window message is
    only injected with full context, while normal turns get a smaller
    follow-up only when reported usage first crosses a budget threshold.
    
    ## What changed
    
    - Added the `TokenBudget` feature flag.
    - Added `<token_budget>` developer fragments for full context-window
    metadata and current-window remaining tokens.
    - Inserted the threshold message during normal turn handling by
    comparing token usage before and after sampling, avoiding persistent
    threshold bookkeeping.
    - Added core integration coverage for full-context-only metadata and
    25/50/75 percent threshold messages.
    
    ## Verification
    
    - `just test -p codex-core token_budget`
    - `git diff --check`
  • [2 of 2] Finish moving goal runtime to extension (#26548)
    ## Stack
    
    1. [#26547](https://github.com/openai/codex/pull/26547) - [1 of 2] Align
    goal extension with core behavior
    2. [#26548](https://github.com/openai/codex/pull/26548) - [2 of 2] Move
    goal runtime to extension
    
    ## Why
    
    This PR completes the switch of the goal behavior to the
    extension-backed runtime and removes the old core goal implementation.
    
    ## What Changed
    
    - Installs the goal extension for app-server `ThreadManager` sessions.
    - Routes app-server thread goal `get`, `set`, and `clear` through
    `GoalService`.
    - Uses thread-idle lifecycle emission after goal resume and snapshot
    ordering so the extension can decide whether to continue the goal.
    - Forwards extension goal updates through a FIFO async app-server
    notification path so backpressure does not drop them or reorder updates.
    - Keeps review turns from enabling goal runtime behavior.
    - Plans extension tools before dynamic tools so built-in goal tool names
    keep their old precedence when goals are enabled.
    - Removes the old core goal runtime, core goal tool handlers, and core
    goal tool specs.
    - Updates tests that were coupled to the core-owned goal runtime while
    leaving the legacy `<goal_context>` compatibility path in core for old
    threads.
    - Removes the stale cargo-shear ignore now that `codex-goal-extension`
    is used by the workspace.
    - Keeps realtime event matching exhaustive after removing the old
    goal-specific realtime text path.
    
    
    ## Validation
    
    - Ran manual `/goal` runs in TUI. Validated time accounting matched
    wall-clock time and goal lifecycle state transitions.
  • Expose local image paths to models (#25944)
    ## Why
    
    Local image attachments include image bytes, but the adjacent
    model-visible label omits the source path. Exposing the path lets
    model-selected workflows refer back to the intended local image
    explicitly.
    
    ## What changed
    
    - Include an escaped `path` attribute in model-visible local image
    opening tags.
    - Reuse the path-aware marker generator in rollout coverage.
    - Update protocol, replay, and rollout coverage for the new request
    shape.
    
    ## Validation
    
    - `just fmt`
    - `just test -p codex-protocol`
    - `just test -p codex-core skips_local_image_label_text`
    - `just test -p codex-core
    copy_paste_local_image_persists_rollout_request_shape`
    - `git diff --check`
  • Use internal model context fragments for goal steering (#24918)
    ## Why
    
    Goal steering is one form of runtime-owned model context, but the old
    `<goal_context>` wrapper made the contextual-fragment hiding path
    goal-specific. Using a source-labeled internal context fragment gives
    core and extensions a shared shape for hidden model steering while
    keeping those prompts out of visible turn history.
    
    The change also keeps legacy `<goal_context>` messages recognized as
    hidden contextual input so existing stored history does not start
    rendering old goal-steering prompts as user-visible turn items.
    
    ## What Changed
    
    - Replaces `GoalContext` with `InternalModelContextFragment` plus a
    validated `InternalContextSource`.
    - Renders goal steering as `<codex_internal_context
    source="goal">...</codex_internal_context>`.
    - Updates core goal steering and `ext/goal` steering to inject the new
    internal-context fragment.
    - Updates contextual-fragment, event-mapping, goal, and session tests
    for the new wrapper.
    
    ## Test Coverage
    
    - Adds coverage for detecting the new internal model context fragment.
    - Preserves coverage for hiding legacy `<goal_context>` fragments.
    - Verifies invalid internal context sources are rejected and arbitrary
    context tags are not hidden.
    - Updates goal steering/session assertions to expect the new
    `source="goal"` wrapper.
  • [codex] Steer budget-limited goal extension turns (#23718)
    ## What
    - Add a small extension capability for injecting model-visible response
    items into the active turn
    - Have the goal extension inject hidden goal-context steering when
    tool-finish accounting reaches `BudgetLimited`
    - Cover the extension backend path with an assertion on the injected
    steering item
    
    ## Why
    PR #23696 persists and emits the budget-limited goal update from
    tool-finish accounting, but it leaves the model unaware of that
    transition. The existing core runtime steers the model to wrap up in
    this case; the extension path should do the same through an explicit
    host capability.
    
    ## Testing
    - `just fmt`
    - `cargo test -p codex-goal-extension`
    - `cargo test -p codex-extension-api`
  • Preserve image detail in app-server inputs (#20693)
    ## Summary
    
    - Add optional image detail to user image inputs across core, app-server
    v2, thread history/event mapping, and the generated app-server
    schemas/types.
    - Preserve requested detail when serializing Responses image inputs:
    omitted detail stays on the existing `high` default, while explicit
    `original` keeps local images on the original-resolution path.
    - Support `high`/`original` consistently for tool image outputs,
    including MCP `codex/imageDetail`, code-mode image helpers, and
    `view_image`.
  • Improve goal continuation based on feedback (#22045)
    ## Summary
    
    This PR updates the goal continuation prompt to address feedback from
    early adopters. There are two primary changes:
    
    1. Goal continuation and budget-limit steering prompts now use hidden
    user-context messages instead of hidden developer messages.
    2. The goal continuation prompt is refined to improve the model's
    ability to fully complete the active goal rather than stop at a smaller
    or merely passing subset.
    
    The user-message transition is important for two reasons. First, it
    eliminates an issue where older steering messages could be responded to
    again after a new turn. Second, it works better with compaction because
    user messages are treated differently from developer messages during
    compaction.
    
    The prompt refinements make persistence explicit, ground work in current
    evidence, encourage `update_plan` for multi-step progress visibility,
    and require stronger completion audits before calling `update_goal`. It
    also removes the elapsed-time reporting in the prompt; I saw evidence
    that this was causing the model to shortcut work as it became nervous
    about time.
    
    These changes were tested with evals. Chriss4123 has also been running
    independent evals in
    [#19910](https://github.com/openai/codex/issues/19910), and many of the
    improvements in this PR were suggested by him.
    
    ## Verification
    
    - Tested with evals.
    - Added and updated focused `codex-core` coverage for hidden goal user
    context, continuation and budget-limit request shape, prompt rendering,
    and objective delimiter escaping.
  • Update image outputs to default to high detail (#18386)
    Do not assume the default `detail`.
  • chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
    ## Why
    
    `argument-comment-lint` was green in CI even though the repo still had
    many uncommented literal arguments. The main gap was target coverage:
    the repo wrapper did not force Cargo to inspect test-only call sites, so
    examples like the `latest_session_lookup_params(true, ...)` tests in
    `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
    
    This change cleans up the existing backlog, makes the default repo lint
    path cover all Cargo targets, and starts rolling that stricter CI
    enforcement out on the platform where it is currently validated.
    
    ## What changed
    
    - mechanically fixed existing `argument-comment-lint` violations across
    the `codex-rs` workspace, including tests, examples, and benches
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
    `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
    `--all-targets` unless the caller explicitly narrows the target set
    - fixed both wrappers so forwarded cargo arguments after `--` are
    preserved with a single separator
    - documented the new default behavior in
    `tools/argument-comment-lint/README.md`
    - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
    invocation and therefore enforces `--all-targets`, while Linux and
    Windows temporarily pass `-- --lib --bins`
    
    That temporary CI split keeps the stricter all-targets check where it is
    already cleaned up, while leaving room to finish the remaining Linux-
    and Windows-specific target-gated cleanup before enabling
    `--all-targets` on those runners. The Linux and Windows failures on the
    intermediate revision were caused by the wrapper forwarding bug, not by
    additional lint findings in those lanes.
    
    ## Validation
    
    - `bash -n tools/argument-comment-lint/run.sh`
    - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
    - shell-level wrapper forwarding check for `-- --lib --bins`
    - shell-level wrapper forwarding check for `-- --tests`
    - `just argument-comment-lint`
    - `cargo test` in `tools/argument-comment-lint`
    - `cargo test -p codex-terminal-detection`
    
    ## Follow-up
    
    - Clean up remaining Linux-only target-gated callsites, then switch the
    Linux lint lane back to the plain wrapper invocation.
    - Clean up remaining Windows-only target-gated callsites, then switch
    the Windows lint lane back to the plain wrapper invocation.
  • feat: structured multi-agent output (#15515)
    Send input now sends messages as assistant message and with this format:
    
    ```
    author: /root/worker_a
    recipient: /root/worker_a/tester
    other_recipients: []
    Content: bla bla bla. Actual content. Only text for now
    ```
  • [hooks] use a user message > developer message for prompt continuation (#14867)
    ## Summary
    
    Persist Stop-hook continuation prompts as `user` messages instead of
    hidden `developer` messages + some requested integration tests
    
    This is a followup to @pakrym 's comment in
    https://github.com/openai/codex/pull/14532 to make sure stop-block
    continuation prompts match training for turn loops
    
    - Stop continuation now writes `<hook_prompt hook_run_id="...">stop
    hook's user prompt<hook_prompt>`
    - Introduces quick-xml dependency, though we already indirectly depended
    on it anyway via syntect
    - This PR only has about 500 lines of actual logic changes, the rest is
    tests/schema
    
    ## Testing
    
    Example run (with a sessionstart hook and 3 stop hooks) - this shows
    context added by session start, then two stop hooks sending their own
    additional prompts in a new turn. The model responds with a single
    message addressing both. Then when that turn ends, the hooks detect that
    they just ran using `stop_hook_active` and decide not to infinite loop
    
    test files for this (unzip, move codex -> .codex):
    [codex.zip](https://github.com/user-attachments/files/26075806/codex.zip)
    
    ```
    › cats
    
    
    • Running SessionStart hook: lighting the observatory
    
    SessionStart hook (completed)
      warning: Hi, I'm a session start hook for wizard-tower (startup).
      hook context: A wimboltine stonpet is an exotic cuisine from hyperspace
    
    • Cats are tiny zen wizards, my friend: equal parts nap, mystery, and chaos. If you want, we can talk
      cat facts, cat breeds, cat names, or build something cat-themed in this repo.
    
    • Running Stop hook: checking the tower wards
    
    • Running Stop hook: sacking the guards
    
    • Running Stop hook: hiring the guards
    
    Stop hook (completed)
      warning: Wizard Tower Stop hook reviewed the completed reply (177 chars).
    
    Stop hook (blocked)
      warning: Wizard Tower Stop hook continuing conversation
      feedback: cook the stonpet
    
    Stop hook (blocked)
      warning: Wizard Tower Stop hook continuing conversation
      feedback: eat the cooked stonpet
    
    • Stonpet’s cooked, aloha style: flash-seared over a blue quasiflame, glazed with nebula salt, and
      rested until the hyperspace juices settle.
    
      Now we eat with gratitude, my friend. One mindful bite in, and the flavor is pure cosmic surf:
      smoky, bright, and totally out of this dimension.
    
    • Running Stop hook: checking the tower wards
    
    • Running Stop hook: sacking the guards
    
    • Running Stop hook: hiring the guards
    
    Stop hook (completed)
      warning: Wizard Tower Stop hook reviewed the completed reply (285 chars).
    
    Stop hook (completed)
      warning: Wizard Tower Stop hook saw a second pass and stayed calm to avoid a loop.
    
    Stop hook (completed)
      warning: Wizard Tower Stop hook saw a second pass and stayed calm to avoid a loop.
    ```
  • fix: move inline codex-rs/core unit tests into sibling files (#14444)
    ## Why
    PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This
    applies the same extraction pattern across the rest of `codex-rs/core`
    so the production modules stay focused on runtime code instead of large
    inline test blocks.
    
    Keeping the tests in sibling files also makes follow-up edits easier to
    review because product changes no longer have to share a file with
    hundreds or thousands of lines of test scaffolding.
    
    ## What changed
    - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**`
    with a path-based module declaration
    - moved each extracted unit test module into a sibling `*_tests.rs`
    file, using `mod_tests.rs` for `mod.rs` modules
    - preserved the existing `cfg(...)` guards and module-local structure so
    the refactor remains structural rather than behavioral
    
    ## Testing
    - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`)
    - `just fix -p codex-core`
    - `cargo fmt --check`
    - `cargo shear`