mirror of
https://github.com/openai/codex.git
synced 2026-10-29 16:09:06 +00:00
1a25d8b6e561d90c883b7792b8d2a37d41b7605a
759
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Eric Traut
|
84d941d07f |
[1 of 7] Add thread settings to UserInput (#23080)
**Stack position:** [1 of 7] ## Summary The first three PRs in this stack are a cleanup pass before the actual thread settings API work. Today, core has several overlapping "user input" ops: `UserInput`, `UserInputWithTurnContext`, and `UserTurn`. They differ mostly in how much next-turn state they carry, which makes the later queued thread settings update harder to reason about and review. This PR starts that cleanup by adding the shared `ThreadSettingsOverrides` payload and allowing `Op::UserInput` to carry it. Existing variants remain in place here, so this layer is mostly a behavior-preserving API shape change plus mechanical constructor updates. ## End State After PR3 By the end of PR3, `Op::UserInput` is the only "user input" core op. It can carry optional thread settings overrides for callers that need to update stored defaults with a turn, while callers without updates use empty settings. `Op::UserInputWithTurnContext` and `Op::UserTurn` are deleted. ## End State After PR5 By the end of PR5, core will have only two ops for this area: - `Op::UserInput` for user-input-bearing submissions. - `Op::ThreadSettings` for settings-only updates. ## Stack 1. [1 of 7] [Add thread settings to UserInput](https://github.com/openai/codex/pull/23080) (this PR) 2. [2 of 7] [Remove UserInputWithTurnContext](https://github.com/openai/codex/pull/23081) 3. [3 of 7] [Remove UserTurn](https://github.com/openai/codex/pull/23075) 4. [4 of 7] [Placeholder for OverrideTurnContext cleanup](https://github.com/openai/codex/pull/23087) 5. [5 of 7] [Replace OverrideTurnContext with ThreadSettings](https://github.com/openai/codex/pull/22508) 6. [6 of 7] [Add app-server thread settings API](https://github.com/openai/codex/pull/22509) 7. [7 of 7] [Sync TUI thread settings](https://github.com/openai/codex/pull/22510) |
||
|
sayan-oai
|
daa11820b0 |
Remove ToolSearch feature toggle (#23389)
## Summary - mark `ToolSearch` as removed and ignore stale config writes for its legacy key - make search tool exposure depend only on model capability, not a feature toggle - remove app-server enablement support and prune now-obsolete test coverage/setup ## Verification - `cargo test -p codex-features` - `cargo test -p codex-tools` - `cargo test -p codex-core search_tool_requires_model_capability` - `cargo test -p codex-app-server experimental_feature_enablement_set_` ## Notes - This keeps the legacy config key as a no-op for compatibility while removing the ability to toggle the behavior off cleanly. - No developer-facing docs update outside the touched app-server README was needed. |
||
|
xl-openai
|
6b54ced108 |
cleanup: Remove skill env var dependency prompting (#22721)
Deletes the skill env var dependency prompt feature and its runtime path. env_var entries in skill dependency metadata are now silently ignored during skill loading. |
||
|
pakrym-oai
|
afa0101ae2 |
[codex] Move pending input into input queue (#22728)
## Why Pending model input was split across `Session`, `TurnState`, and the agent mailbox. That made it easy for new paths to manage queued user input or mailbox delivery outside the intended ownership boundary. This PR consolidates the model-facing input lifecycle behind the session input queue so turn-local pending input, next-turn queued items, and mailbox delivery coordination are owned in one place. ## What Changed - Added `session/input_queue.rs` to own pending input queues and mailbox delivery coordination. - Removed the standalone `agent/mailbox.rs` channel wrapper and store mailbox items directly in the input queue. - Moved pending-input mutations off `TurnState`; `TurnState` now exposes the queue-owned storage directly for now. - Routed abort cleanup, mailbox delivery phase changes, next-turn queued items, and active-turn pending input through `InputQueue`. - Boxed stack-heavy agent resume/fork startup futures that the refactor pushed over the default test stack. - Updated session, task, goal, stream-event, and multi-agent call sites and tests to use the new queue ownership. ## Verification - `cargo test -p codex-core --lib agent::control::tests` - `cargo test -p codex-core --lib agent::control::tests::resume_closed_child_reopens_open_descendants -- --exact` - `cargo test -p codex-core --lib agent::control::tests::spawn_agent_fork_last_n_turns_keeps_only_recent_turns -- --exact` - `cargo test -p codex-core --lib agent::control::tests::resume_thread_subagent_restores_stored_nickname_and_role -- --exact` - `cargo test -p codex-core` was also run; it completed with 1814 passed, 4 ignored, and one timeout in `agent::control::tests::resume_thread_subagent_restores_stored_nickname_and_role`, which passed when rerun in isolation. |
||
|
jif-oai
|
c69cde3547 |
Add tool lifecycle extension contributor (#23309)
## Why Extensions that need to track runtime progress currently have no typed host signal for tool execution. The goal extension in particular needs to observe tool attempts without inspecting tool payloads, owning tool implementations, or staying coupled to core-only runtime plumbing. This adds a narrow lifecycle contributor API for host-owned tool execution: extensions can observe when an accepted tool call starts and how it finishes, while policy hooks and tool handlers continue to own payload rewriting, blocking, and execution. Relevant code: - [`ToolLifecycleContributor`](https://github.com/openai/codex/blob/3ad2850ffc7d8a1da19c65a92425637a59098f1b/codex-rs/ext/extension-api/src/contributors.rs#L119) defines the extension-facing observer contract. - [`tool_lifecycle.rs`](https://github.com/openai/codex/blob/3ad2850ffc7d8a1da19c65a92425637a59098f1b/codex-rs/ext/extension-api/src/contributors/tool_lifecycle.rs) defines the typed start/finish inputs, source, and outcome enums. - [`notify_tool_start` / `notify_tool_finish`](https://github.com/openai/codex/blob/3ad2850ffc7d8a1da19c65a92425637a59098f1b/codex-rs/core/src/tools/lifecycle.rs) bridges core tool dispatch into the extension registry. ## What Changed - Added `ToolLifecycleContributor` to `codex-extension-api`, including: - `ToolStartInput` - `ToolFinishInput` - `ToolCallSource` - `ToolCallOutcome` - Added registration and lookup support on `ExtensionRegistryBuilder` / `ExtensionRegistry`. - Wired core tool dispatch to notify lifecycle contributors for: - accepted tool starts - completed tool calls, including the tool output success marker - pre-tool-use blocks - failures before or after the handler runs - cancellation/abort in the parallel tool path - Registered the goal extension as a lifecycle contributor and added the outcome filter it will use for goal progress accounting. ## Test Coverage - Added `dispatch_notifies_tool_lifecycle_contributors` to cover lifecycle notification ordering and outcomes for successful and handler-failed tool calls. |
||
|
Celia Chen
|
4dbca61e20 |
fix: default unknown tool schemas to empty schemas (#22380)
## Why
Some tool providers, especially MCP servers and dynamic tool sources,
can supply schema nodes that omit `type` and have no recognized JSON
Schema shape hints. Previously, `sanitize_json_schema` filled those
unknown nodes in as `string`, which made the schema parseable but
invented a scalar constraint that the provider did not specify. For
description-only fields, that could incorrectly steer tool arguments
away from the provider's actual accepted shape.
The Responses API accepts permissive empty schemas such as `{}` at
nested property positions, so Codex should preserve that permissive
meaning instead of coercing unknown schema nodes into a misleading
scalar type.
## What Changed
- Changed the no-hints fallback in `codex-rs/tools/src/json_schema.rs`
to clear unrecognized object schema nodes to `{}`.
- Empty schemas now remain `{}` rather than becoming `type: "string"`.
- Description-only or otherwise metadata-only nested property schemas
now become `{}` while surrounding object/array/string/number inference
still applies when recognized hints are present.
- Updated `codex-tools` and `codex-core` tests to cover top-level empty
schemas, nested empty schemas, metadata-only malformed schemas, dynamic
tools, and MCP tool specs.
## Verification
- `cargo test -p codex-tools`
- `cargo test -p codex-core
test_mcp_tool_property_missing_type_defaults_to_empty_schema`
- Manually verified the real Responses API behavior for both
empty-schema positions:
- Top-level function `parameters: {}` is accepted and echoed back as
`{"type":"object","properties":{}}`; when forced to call the tool,
Responses emitted empty object arguments: `"arguments": "{}"`.
- Nested property schema `{}` is accepted and preserved as `{}`; when
forced to call a tool with `metadata.extra`, Responses emitted
`"arguments": "{\"metadata\":{\"extra\":\"codex schema sanitizer
behavior\"}}"`.
|
||
|
Eric Traut
|
0d344aca9b |
goal: pause continuation loops on usage limits and blockers (#23094)
Addresses #22833, #22245, #23067 ## Why `/goal` can keep synthesizing turns even when the next turn cannot make meaningful progress. Hard usage exhaustion can replay failing turns, and repeated permission or external-resource blockers can keep burning tokens while waiting for user or system intervention. ## What changed - Add resumable `blocked` and `usageLimited` goal states. As with `paused`, goal continuation stops with these states. - Move to `usageLimited` after usage-limit failures. - Allow the built-in `update_goal` tool to set `blocked` only under explicit repeated-impasse guidance. Updated goal continuation prompt to specify that agent should use `blocked` only when it has made at least three attempts to get past an impasse. Most of the files touched by this PR are because of the small app server protocol update. ## Validation I manually reproduced a number of situations where an agent can run into a true impasse and verified that it properly enters `blocked` state. I then resumed and verified that it once again entered `blocked` state several turns later if the impasse still exists. I also manually reproduced the usage-limit condition by creating a simulated responses API endpoint that returns 429 errors with the appropriate error message. Verified that the goal runtime properly moves the goal into `usageLimited` state and TUI UI updates appropriately. Verified that `/goal resume` resumes (and immediately goes back into `ussageLImited` state if appropriate). ## Follow-up PRs Small changes will be needed to the GUI clients to properly handle the two new states. |
||
|
pakrym-oai
|
82061660ae |
[codex] Remove legacy shell output formatting paths (#22706)
## Why The client and tool pipeline still carried compatibility code for legacy structured shell output. Current shell and apply_patch responses are already plain text for model consumption, so keeping a JSON-serialization path plus shell-item rewrite logic makes the request formatter and tests preserve a format we do not need anymore. ## What Changed - Removed the client-side shell output rewrite from `core/src/client_common.rs`. - Removed the structured exec-output formatter and the shell `freeform` switch so tool emitters use one model-facing formatter. - Collapsed apply_patch/shell serialization tests around the remaining plain-text output expectations and removed duplicate one-variant parameterized cases. - Kept the `ApplyPatchModelOutput::ShellCommandViaHeredoc` compatibility input shape, but no longer treats it as a separate output-format mode. ## Validation - `cargo test -p codex-core client_common` - `cargo test -p codex-core shell_serialization` - `cargo test -p codex-core apply_patch_cli` - `just fix -p codex-core` ## Documentation No external Codex documentation update is needed. |
||
|
Michael Bolin
|
0a83353ca3 |
test: reduce core sandbox policy test setup (#23036)
## Why `SandboxPolicy` is a legacy compatibility shape, but several core tests still used it for ordinary turn setup even when the runtime path now carries `PermissionProfile`. With the first cleanup PR merged, this follow-up trims more core test scaffolding so remaining `SandboxPolicy` matches are easier to classify as production compatibility, legacy-boundary coverage, or explicit conversion tests. ## What Changed - Updated apply-patch handler and runtime tests to pass `PermissionProfile` directly. - Changed sandboxing test helpers to build permission profiles without first creating `SandboxPolicy` values. - Converted request-permissions integration turns to pass `PermissionProfile` through the test helper, leaving legacy sandbox projection at the `Op::UserTurn` boundary. - Converted unified exec integration helpers and direct turn submissions to use `PermissionProfile` values instead of `SandboxPolicy` setup. - Removed now-unused `SandboxPolicy` imports from the touched core tests. ## Test Plan - `just fmt` - `cargo test -p codex-core --lib tools::sandboxing::tests` - `cargo test -p codex-core --lib tools::runtimes::apply_patch::tests` - `cargo test -p codex-core --lib tools::handlers::apply_patch::tests` - `cargo test -p codex-core --lib unified_exec::process_manager::tests` - `cargo test -p codex-core --test all request_permissions::` - `cargo test -p codex-core --test all unified_exec::` - `just fix -p codex-core` |
||
|
jif-oai
|
545ede569c |
Make multi-agent v2 tool namespace configurable (#23147)
## Summary - Add `features.multi_agent_v2.tool_namespace` with config/schema validation for Responses-compatible namespace values. - Thread the resolved namespace into `ToolsConfig` for normal turns and review turns. - Wrap MultiAgentV2 tool specs and registry names in the configured namespace when namespace tools are supported, while falling back to the plain tool names when they are not. ## Validation - `just fmt` - `just write-config-schema` - `cargo test -p codex-features multi_agent_v2_feature_config -- --nocapture` - `cargo test -p codex-core test_build_specs_multi_agent_v2 -- --nocapture` - `cargo test -p codex-core multi_agent_v2_config -- --nocapture` - `cargo test -p codex-core multi_agent_v2_rejects_invalid_tool_namespace -- --nocapture` - `cargo test -p codex-tools` - `git diff --check` |
||
|
sayan-oai
|
061a614d85 |
multiagent: trim model-visible description, cap to 5 models (#23069)
## Why
The `spawn_agent` model override guidance is uncapped and bloating
context. We need to trim down each entry and cap total entries.
picked 5 as cap, we can change
## What changed
- Cap the model override summaries shown in `spawn_agent` to the first 5
picker-visible models, preserving the existing priority ordering from
the models manager.
- Condense each rendered entry to the actionable pieces the model needs:
- use the model slug as the label
- render compact reasoning effort lists with the default marked inline
- render only service tier IDs, and omit the clause when no tiers are
available
- Update coverage so the compact formatter shape and the top-5 cap are
exercised, and keep the end-to-end request assertion aligned with real
model metadata.
## Example
Before:
`- gpt-5.4 ('gpt-5.4\'): Strong model for everyday coding. Default
reasoning effort: medium. Supported reasoning efforts: low (Fast
responses with lighter reasoning), medium (Balances speed and reasoning
depth for everyday tasks), high (Greater reasoning depth for complex
problems), xhigh (Extra high reasoning depth for complex problems).
Supported service tiers: priority (Fast: 1.5x speed, increased usage).`
After:
`- 'gpt-5.4': Strong model for everyday coding. Reasoning efforts: low,
medium (default), high, xhigh. Service tiers: priority.`
|
||
|
Michael Bolin
|
d91bc15618 |
test: construct permission profiles directly (#23030)
## Why `SandboxPolicy` is now a legacy compatibility shape, but several tests still built a `SandboxPolicy` only to immediately convert it into `PermissionProfile` for APIs that already accept canonical runtime permissions. Those detours make it harder to audit where legacy sandbox policy is still required, because boundary-only usages are mixed together with ordinary test setup. ## What Changed - Updated tests in `codex-core`, `codex-exec`, `codex-analytics`, and `codex-config` to construct `PermissionProfile` values directly when the code under test takes a permission profile. - Changed exec-policy, request-permissions, session, and sandbox test helpers to pass `PermissionProfile` through instead of converting from `SandboxPolicy` internally. - Left `SandboxPolicy` in place where tests are explicitly exercising legacy compatibility or request/response boundaries. ## Test Plan - `cargo test -p codex-analytics -p codex-config` - `cargo test -p codex-core --lib safety::tests` - `cargo test -p codex-core --lib exec_policy::tests::` - `cargo test -p codex-core --lib exec::tests` - `cargo test -p codex-core --lib guardian_review_session_config` - `cargo test -p codex-core --lib tools::network_approval::tests` - `cargo test -p codex-core --lib tools::runtimes::shell::unix_escalation::tests` - `cargo test -p codex-core --lib managed_network` - `cargo test -p codex-core --test all request_permissions::` - `cargo test -p codex-exec sandbox` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/23030). * #23036 * __->__ #23030 |
||
|
Eric Traut
|
941e7f825e |
Improve goal completion usage reporting (#22907)
## Why Goal completion follow-up turns currently receive a preformatted English usage sentence such as `time used: 2586 seconds`. That nudges the model to echo an awkward raw seconds count in the final reply, even though the tool result already exposes structured usage fields like `goal.timeUsedSeconds`, `goal.tokensUsed`, and `goal.tokenBudget`. ## What changed - Replace the preformatted completion usage sentence with guidance to read the structured goal fields from the tool result. - Preserve token-budget reporting while allowing the model to phrase elapsed time in a concise, human-friendly way that fits the response language. - Update core coverage for both the generated completion guidance and the session flow that forwards it back to the model. ## Verification Previously, it would have output a final message indicating that it "worked for 303 seconds". Now it shows the following: <img width="286" height="35" alt="image" src="https://github.com/user-attachments/assets/d7011880-9449-46a7-856f-4e50ae00eb45" /> |
||
|
Curtis 'Fjord' Hawthorne
|
8543e39885 |
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`. |
||
|
jif-oai
|
c03cea4ca2 |
Remove zombie tools spec module (#22820)
## Summary - move tool_user_shell_type out of the old tools::spec module and call it from tools directly - attach the remaining spec planning model tests under spec_plan - delete core/src/tools/spec.rs ## Tests - just fmt - cargo test -p codex-core tools::spec_plan Note: a broader cargo test -p codex-core run on the earlier PR-head worktree still hit the pre-existing stack overflow in agent::control::tests::spawn_agent_fork_last_n_turns_keeps_only_recent_turns. |
||
|
jif-oai
|
6f1a01fbdd |
Simplify tool executor and registry plumbing (#22636)
## Why The tool runtime path still had a typed output associated type on `ToolExecutor`, plus a core-only `RegisteredTool` adapter and extension-only executor aliases. That made every new shared tool runtime carry extra adapter plumbing before it could participate in core dispatch, extension tools, hook payloads, telemetry, and model-visible spec generation. This PR moves output erasure to the shared executor boundary so core and extension tools can use the same execution contract directly. ## What Changed - Changed `codex_tools::ToolExecutor` to return `Box<dyn ToolOutput>` instead of an associated `Output` type. - Removed the extension-specific `ExtensionToolExecutor` / `ExtensionToolOutput` aliases and exposed `ToolExecutor<ToolCall>` plus `ToolOutput` through `codex-extension-api`. - Reworked core tool registration around `CoreToolRuntime` and `ToolRegistry::from_tools`, removing the extra `RegisteredTool` / `ToolRegistryBuilder` layer. - Consolidated model-visible spec planning and registry construction in `core/src/tools/spec_plan.rs`, including deferred tool search and code-mode-only filtering. - Added `ToolOutput` helpers for post-tool-use hook ids and inputs so MCP, unified exec, extension, and other boxed outputs preserve the same hook payload behavior. - Updated core handlers, memories tools, and the related registry/spec/router tests to use the simplified contract. ## Test Coverage - Updated coverage for tool spec planning, registry lookup, deferred tool search registration, extension tool routing, post-tool-use hook payloads, dispatch tracing, guardian output extraction, and memories extension tool execution. |
||
|
Michael Bolin
|
3c6d727810 |
permissions: resolve profile identity with constraints (#22683)
## Why This PR is the invariant-cleanup layer that follows the workspace-roots base merged in [#22610](https://github.com/openai/codex/pull/22610). #22610 adds `[permissions.<id>.workspace_roots]` and keeps runtime workspace roots separate from the raw permission profile, but its in-memory representation is intentionally transitional: `Permissions` still carries the selected profile identity next to a constrained `PermissionProfile`. That makes APIs such as `set_constrained_permission_profile_with_active_profile()` fragile because the id and value only mean the right thing when every caller keeps them in sync. This PR introduces a single resolved profile state so profile identity, `extends`, the profile value, and profile-declared workspace roots travel together. The next PR, [#22611](https://github.com/openai/codex/pull/22611), builds on this by changing the app-server turn API to select permission profiles by id plus runtime workspace roots. ## Stack Context - #22610, now merged: adds profile-declared `workspace_roots`, runtime workspace roots, and `:workspace_roots` materialization. - This PR: replaces the parallel active-profile/profile-value fields with `PermissionProfileState`. - #22611: switches app-server turn updates toward profile ids plus runtime workspace roots. - #22612: updates TUI/exec summaries to show the effective workspace roots. Keeping this separate from #22611 is deliberate: reviewers can validate the internal state invariant before reviewing the app-server protocol migration. ## What Changed - Added `ResolvedPermissionProfile::{Legacy, BuiltIn, Named}` and `PermissionProfileState`. - Typed built-in profile ids with `BuiltInPermissionProfileId`. - Moved selected profile identity and profile-declared workspace roots into the resolved state. - Replaced `Permissions` parallel profile fields with one `permission_profile_state`. - Removed `set_constrained_permission_profile_with_active_profile()` from session sync paths. - Kept trusted session replay/`SessionConfigured` compatibility through explicit session snapshot helpers. - Updated session configuration, MCP initialization, app-server, exec, TUI, and guardian call sites to consume `&PermissionProfile` directly. ## Review Guide Start with `codex-rs/core/src/config/resolved_permission_profile.rs`; it is the new invariant boundary. Then review `codex-rs/core/src/config/mod.rs` to see how config loading records active profile identity and profile workspace roots. The remaining call-site changes are mostly mechanical fallout from `Permissions::permission_profile()` returning `&PermissionProfile` instead of `&Constrained<PermissionProfile>`. ## Verification The existing config/session coverage now constructs and asserts through `PermissionProfileState`. The workspace-root config test also asserts that profile-declared roots are preserved in the resolved state, which is the behavior #22611 relies on when runtime roots become mutable through the app-server API. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22683). * #22612 * #22611 * __->__ #22683 |
||
|
Michael Bolin
|
c25d905f61 |
permissions: support workspace roots in profiles (#22610)
## Why This is the configuration/model half of the alternative permissions migration we discussed as a comparison point for [#22401](https://github.com/openai/codex/pull/22401) and [#22402](https://github.com/openai/codex/pull/22402). The old `workspace-write` model mixes three concerns that we want to keep separate: - reusable profile rules that should stay immutable once selected - user/runtime workspace roots from `cwd`, `--add-dir`, and legacy workspace-write config - internal Codex writable roots such as memories, which should not be shown as user workspace roots This PR gives permission profiles first-class `workspace_roots` so users can opt multiple repositories into the same `:workspace_roots` rules without using broad absolute-path write grants. It also starts separating the raw selected profile from the effective runtime profile by making `Permissions` expose explicit accessors instead of public mutable fields. A representative `config.toml` looks like this: ```toml default_permissions = "dev" [permissions.dev.workspace_roots] "~/code/openai" = true "~/code/developers-website" = true [permissions.dev.filesystem.":workspace_roots"] "." = "write" ".codex" = "read" ".git" = "read" ".vscode" = "read" ``` If Codex starts in `~/code/codex` with that profile selected, the effective workspace-root set becomes: - `~/code/codex` from the runtime `cwd` - `~/code/openai` from the profile - `~/code/developers-website` from the profile The `:workspace_roots` rules are materialized across each root, so `.git`, `.codex`, and `.vscode` stay scoped the same way everywhere. Runtime additions such as `--add-dir` can still layer on later stack entries without mutating the selected profile. ## Stack Shape This PR intentionally stops before the profile-identity cleanup in [#22683](https://github.com/openai/codex/pull/22683) so the base review stays focused on config loading, workspace-root materialization, and compatibility with legacy `workspace-write`. The representation in this PR is therefore transitional: `Permissions` carries enough state to distinguish the raw constrained profile from the effective runtime profile, and there are still call sites that must keep the active profile identity and constrained profile value in sync. The follow-up PR replaces that with a single resolved profile state (`ResolvedPermissionProfile` / `PermissionProfileState`) that keeps the profile id, immutable `PermissionProfile`, and profile-declared workspace roots together. That follow-up removes APIs such as `set_constrained_permission_profile_with_active_profile()` where separate arguments could drift out of sync. Downstream PRs then build on this base to switch app-server turn updates to profile ids plus runtime workspace roots and to finish the user-visible summary behavior. Reviewers should judge this PR as the workspace-roots foundation, not as the final in-memory shape of selected permission profiles. ## Review Guide Suggested review order: 1. Start with `codex-rs/core/src/config/mod.rs`. This is the main shape change in the base slice. `Permissions` now stores a private raw `Constrained<PermissionProfile>` plus runtime `workspace_roots`. Callers use `permission_profile()` when they need the raw constrained value and `effective_permission_profile()` when they need a materialized runtime profile. As noted above, [#22683](https://github.com/openai/codex/pull/22683) replaces this transitional shape with a resolved profile state that keeps identity and profile data together. 2. Review `codex-rs/config/src/permissions_toml.rs` and `codex-rs/core/src/config/permissions.rs`. These add `[permissions.<id>.workspace_roots]`, resolve enabled entries relative to the policy cwd, and keep `:workspace_roots` deny-read glob patterns symbolic until the actual roots are known. 3. Review `codex-rs/protocol/src/permissions.rs` and `codex-rs/protocol/src/models.rs`. These add the policy/profile materialization helpers that expand exact `:workspace_roots` entries and scoped deny-read globs over every workspace root. This is also where `ActivePermissionProfileModification` is removed from the core model. 4. Review the legacy bridge in `Config::load_from_base_config_with_overrides` and `Config::set_legacy_sandbox_policy`. This is where legacy `workspace-write` roots become runtime workspace roots, while Codex internal writable roots stay internal and do not appear as user-facing workspace roots. 5. Then skim downstream call sites. The interesting pattern is raw-vs-effective access: state/proxy/bwrap paths keep the raw constrained profile, while execution, summaries, and user-visible status use the effective profile and workspace-root list. ## What Changed - added `[permissions.<id>.workspace_roots]` to the config model and schema - added runtime `workspace_roots` state to `Config`/`Permissions` and `ConfigOverrides` - made `Permissions` profile fields private and replaced direct mutation with accessors/setters - added `PermissionProfile` and `FileSystemSandboxPolicy` helpers for materializing `:workspace_roots` exact paths and deny-read globs across all roots - moved legacy additional writable roots into runtime workspace-root state instead of active profile modifications - removed `ActivePermissionProfileModification` and its app-server protocol/schema export - updated sandbox/status summary paths so internal writable roots are not reported as user workspace roots ## Verification Strategy The targeted tests cover the behavior at the layers where regressions are most likely: - `codex-rs/core/src/config/config_tests.rs` verifies config loading, legacy workspace-root seeding, effective profile materialization, and memory-root handling. - `codex-rs/core/src/config/permissions_tests.rs` verifies profile `workspace_roots` parsing and `:workspace_roots` scoped/glob compilation. - `codex-rs/protocol/src/permissions.rs` unit tests verify exact and glob materialization over multiple workspace roots. - `codex-rs/tui/src/status/tests.rs` and `codex-rs/utils/sandbox-summary/src/sandbox_summary.rs` verify the user-facing summaries show effective workspace roots and hide internal writes. I also ran `cargo check --tests` locally after the latest stack refresh to catch cross-crate API breakage from the private-field/accessor changes. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22610). * #22612 * #22611 * #22683 * __->__ #22610 |
||
|
Dylan Hurd
|
51b0e94105 |
chore(features) rm Feature::ApplyPatchFreeform (#22711)
## Summary Removes the feature since this is effectively on by default in all cases where we should use it, or can be configured via models.json. ## Testing - [x] unit tests pass |
||
|
jif-oai
|
6d65686313 |
feat: make ToolExecutor an async trait (#22560)
## Why `codex_tools::ToolExecutor` keeps a tool spec attached to its runtime handler, but extension tools still carried a parallel `ExtensionToolFuture` / `ExtensionToolExecutor` shape. That made extension-owned tools look different from host tools even though routing, registration, and execution need the same abstraction. This PR makes the shared executor contract directly async and lets extension tools implement it too, so host tools and extension tools can move through the same registration path. ## What changed - Changed `ToolExecutor::handle` to an `async fn` using `async-trait`, and updated built-in tool handlers to implement the async trait directly. - Replaced the bespoke `ExtensionToolFuture` contract with a marker `ExtensionToolExecutor` over `ToolExecutor<ToolCall, Output = JsonToolOutput>`, re-exporting `ToolExecutor` from `codex-extension-api`. - Updated the memories extension tools to implement the shared executor trait. - Split tool-router construction into collected executors plus hosted model specs, keeping hosted tools like web search and image generation separate from executable handlers. - Updated spec/router tests and extension-tool stubs for the new executor shape. ## Verification - Not run locally. |
||
|
jif-oai
|
e6939e3969 | feat: namespace in ext (#22556) | ||
|
Andrey Mishchenko
|
7c57a59f51 |
Make multi_agent_v2 wait_agent timeouts configurable (#22528)
## Why `multi_agent_v2` already allowed configuring the minimum `wait_agent` timeout, but the default timeout and upper bound were still hard-coded. That made it hard to tune waits for subagent mailbox activity in sessions that need either faster wakeups or longer waits, and it meant the model-visible `wait_agent` schema could not fully reflect the resolved runtime limits. ## What Changed - Added `features.multi_agent_v2.max_wait_timeout_ms` and `features.multi_agent_v2.default_wait_timeout_ms` alongside the existing `min_wait_timeout_ms` setting. - Validated all three timeouts in config as `0..=3_600_000`, with `min_wait_timeout_ms <= default_wait_timeout_ms <= max_wait_timeout_ms`. - Thread and review session tool config now passes the resolved min/default/max values into the `wait_agent` tool schema. - `wait_agent` now uses the configured default when `timeout_ms` is omitted and rejects explicit values outside the configured min/max range instead of silently clamping them. - Updated the generated config schema and config-lock test coverage for the new fields. |
||
|
iceweasel-oai
|
8ae0c837f0 |
Avoid PowerShell profiles in elevated Windows sandbox (#21400)
## Why On Windows, elevated sandboxed commands run under a dedicated sandbox account while `HOME` / `USERPROFILE` can still point at the real user's profile directory. For PowerShell login shells, that combination can make the sandbox account try to load the real user's PowerShell profile script. If the sandbox account's execution policy differs from the real user's policy, startup can emit profile-loading errors before the requested command runs. For this backend, loading the profile is not a faithful user login shell: it is cross-account profile execution. Treating these PowerShell invocations as non-login shells avoids that invalid startup path. ## Why This Happens Late The normal `login` decision is resolved when shell argv is created, but that point is too early to make this Windows sandbox-specific decision. At argv creation time we do not yet know the actual sandbox attempt that will run the command. A turn can include sandboxed and unsandboxed attempts, and a broad turn-level override would also affect Full Access commands where the user's profile should remain available. Instead, this change carries the selected `ShellType` alongside the argv and applies the `-NoProfile` adjustment in the shell runtimes once the `SandboxAttempt` is known. That keeps the override scoped to actual `WindowsRestrictedToken` attempts with `WindowsSandboxLevel::Elevated`. The runtime uses the selected shell metadata rather than re-detecting PowerShell from argv. That avoids brittle parsing and covers PowerShell invocation shapes such as `-EncodedCommand`. ## What Changed - Carry selected shell metadata through `exec_command` / unified exec requests and shell tool requests. - Insert `-NoProfile` for PowerShell commands only when the runtime is about to execute a sandboxed elevated Windows attempt. - Add focused unit coverage for elevated Windows PowerShell, `-EncodedCommand`, existing `-NoProfile`, legacy restricted-token attempts, unsandboxed attempts, and non-PowerShell commands. ## Verification - `cargo test -p codex-core disable_powershell_profile_tests` - `cargo test -p codex-core test_get_command` - `cargo clippy --fix --tests --allow-dirty --allow-no-vcs -p codex-core` A full `cargo test -p codex-core` run was also attempted during development, but it still hit an unrelated stack overflow in `agent::control` tests before reaching this area. |
||
|
sayan-oai
|
3de4d7f238 |
clean up instructions (#22543)
rm behavioral steering in tool docs for code mode. |
||
|
pakrym-oai
|
3ac1d15598 |
Use selected environment cwd for filesystem helpers (#22542)
## Why `TurnContext::cwd` is deprecated in favor of resolving paths from the selected turn environment cwd. A few filesystem-oriented paths were still constructing sandbox context from the legacy cwd and then mutating it afterward, or resolving local file paths through the deprecated helper. ## What changed - Make `TurnContext::file_system_sandbox_context` take the trusted cwd explicitly. - Pass the selected turn environment cwd directly from `apply_patch` and `view_image` call sites. - Restrict `spawn_agents_on_csv` to exactly one local environment and resolve input/output CSV paths from that local environment cwd. - Remove a redundant test setup assignment that only synchronized deprecated `TurnContext::cwd` with a replaced config. ## Validation - `cargo test -p codex-core view_image` - `cargo test -p codex-core maybe_persist_mcp_tool_approval_writes_project_config_for_project_server` - `cargo test -p codex-core parse_csv_supports_quotes_and_commas` - `git diff --check` |
||
|
pakrym-oai
|
4454e1411b |
Deprecate TurnContext cwd and resolve_path (#22519)
## Why `TurnContext::cwd` and `TurnContext::resolve_path` are being phased out in favor of using the selected turn environment cwd directly. Deprecating both APIs makes any new direct dependency visible while preserving the existing migration path for current callers. ## What Changed - Marked `TurnContext::cwd` and `TurnContext::resolve_path` as deprecated with guidance to use the selected turn environment cwd instead. - Added exact `#[allow(deprecated)]` suppressions at each existing direct usage site, including tests, rather than adding crate-wide suppression. - Kept the change behavior-preserving: current cwd reads, writes, and path resolution continue to use the same values. ## Verification - `just fmt` - `cargo check -p codex-core` - `cargo check -p codex-core --tests` - `git diff --check` |
||
|
jif-oai
|
fc26af377f |
feat: expose multi-agent v2 as model-only tools (#22514)
## Why `code_mode_only` filters code-mode nested tools out of the top-level tool list. For multi-agent v2, we need a rollout shape where the collaboration tools remain callable as normal model tools without also being embedded into the code-mode `exec` tool declaration. Related to this: https://openai-corpws.slack.com/archives/C0AQLHB4U75/p1778660267922549 ## What Changed - Adds `features.multi_agent_v2.non_code_mode_only`, including config resolution, profile override handling, and generated schema coverage. - Introduces `ToolExposure::DirectModelOnly` so a tool can be included in the initial model-visible list while staying out of the nested code-mode tool surface. - Applies that exposure to the multi-agent v2 tools when the new flag is set: `spawn_agent`, `send_message`, `followup_task`, `wait_agent`, `close_agent`, and `list_agents`. - Updates code-mode-only filtering so direct-model-only tools remain visible while ordinary nested code-mode tools are still hidden. ## Verification - Added config parsing/profile tests for `non_code_mode_only`. - Added tool spec coverage for the code-mode-only multi-agent v2 exposure behavior. |
||
|
pakrym-oai
|
83decfa300 |
[codex] Remove unused legacy shell tools (#22246)
## Why Recent session history showed no active use of the raw `shell`, `local_shell`, or `container.exec` execution surfaces. Keeping those handlers/specs wired into core leaves duplicate shell execution paths alongside the supported `shell_command` and unified exec tools. ## What changed - Removed the raw `shell` handler/spec and its `ShellToolCallParams` protocol helper. - Removed the legacy `local_shell` and `container.exec` handler/spec plumbing while preserving persisted-history compatibility for old response items. - Normalized model/config `default` and `local` shell selections to `shell_command`. - Pruned tests that exercised removed raw-shell/local-shell/apply-patch variants and kept coverage on `shell_command`, unified exec, and freeform `apply_patch`. ## Verification - `git diff --check` - `cargo test -p codex-protocol` - `cargo test -p codex-tools` - `cargo test -p codex-core tools::handlers::shell` - `cargo test -p codex-core tools::spec` - `cargo test -p codex-core tools::router` - `cargo test -p codex-core active_call_preserves_triggering_command_context` - `cargo test -p codex-core guardian_tests` - `cargo test -p codex-core --test all shell_serialization` - `cargo test -p codex-core --test all apply_patch_cli` - `cargo test -p codex-core --test all shell_command_` - `cargo test -p codex-core --test all local_shell` - `cargo test -p codex-core --test all otel::` - `cargo test -p codex-core --test all hooks::` - `just fix -p codex-core` - `just fix -p codex-tools` |
||
|
jif-oai
|
fdda59c00b |
Introduce tool exposure for deferred registration (#22489)
## Why Deferred tools were tracked with separate side-channel filtering after tool specs had already been assembled. That made the registry responsible for executing tools while the router/spec planner separately decided whether those same tools should be exposed to the model up front. This PR makes exposure part of the tool handler contract so direct versus deferred availability travels with the executable tool registration. Next step will be to simplify registration ## What Changed - Adds `ToolExposure` to `codex-tools` and exposes it through `ToolExecutor`, defaulting tools to `Direct`. - Teaches dynamic tools and MCP handlers to mark deferred tools as `Deferred` at construction time. - Renames the registry object-safe wrapper from `AnyToolHandler` to `RegisteredTool` and uses `ToolExposure` when deciding whether to include a handler's spec in the initial model-visible tool list. - Refactors tool spec planning to derive direct specs and deferred search entries from registered handlers, removing the router's special-case deferred dynamic tool filtering. ## Verification - Not run. |
||
|
Ahmed Ibrahim
|
87de4e3290 |
Add service tier overrides to spawned agents (#22139)
## Why Spawned agents can already override `model` and `reasoning_effort`, but they have no equivalent way to opt into a model-supported service tier. That makes it impossible to preserve or intentionally select tiered execution behavior when delegating work to a sub-agent, even though the model catalog already advertises supported `service_tiers`. ## What changed - Add optional `service_tier` to both legacy and `MultiAgentV2` `spawn_agent` tool inputs. - Show each picker-visible model's supported service tier ids and descriptions in the `spawn_agent` tool guidance. - Resolve service tier selection after the child agent's effective model is known. - Inherit the parent tier when omitted and still supported by the final child model; otherwise clear it. - Reject explicit unsupported tier requests with a model-facing error. - Keep explicit `service_tier` usable on full-history forks, while still honoring the existing model/reasoning fork restrictions. - Hide `service_tier` alongside other spawn metadata when `hide_spawn_agent_metadata` is enabled. ## Verification Added focused coverage for: - v1/v2 `spawn_agent` schema exposure for `service_tier` - tier descriptions in spawn guidance - hidden-metadata suppression - explicit supported tier selection - explicit unknown and unsupported tier rejection - inherited tier preservation or clearing based on child-model support - full-history fork acceptance for explicit service tiers in both v1 and v2 Local Rust tests were not run in this workspace per repo guidance; the new coverage is included for CI. |
||
|
jif-oai
|
9c5dfa7b1a |
Refactor extension tools onto shared ToolExecutor (#22369)
## Why Extension tools were split across two public runtime contracts: `codex-tool-api` exposed `ToolBundle` plus its own call/spec/error types, while core native tools used `codex_tools::ToolExecutor`. That made contributed tool specs and execution behavior easy to drift apart and added another crate boundary for what should be one executable-tool seam. This PR makes `ToolExecutor` the single runtime contract and keeps extension-specific pinning in `codex-extension-api`. ## Remaining todo https://github.com/openai/codex/pull/22369/changes#diff-b935ea8245c3ce568a30cff660175fa6390b66b872ae409e1e2e965738250741R5 Either generic `Invocation` or sub-extract the `ToolCall` and clean `ToolInvocation` ## What changed - Removed the `codex-tool-api` workspace crate and its dependencies from core and `codex-extension-api`. - Made `codex_tools::ToolExecutor` object-safe with `async_trait` so extension contributors can return a dyn executor. - Added the extension-facing aliases under `ext/extension-api/src/contributors/tools.rs`, including `ExtensionToolExecutor = dyn ToolExecutor<ToolCall, Output = ExtensionToolOutput>`. - Changed `ToolContributor::tools` to return extension executors directly instead of `ToolBundle`s. - Updated core’s extension tool handler/registry/router path to adapt those extension executors into the existing native `ToolInvocation` runtime path. - Added focused coverage for extension tools being registered, model-visible, dispatchable, and not replacing built-in tools. ## Verification - `cargo test -p codex-tools` - `cargo test -p codex-extension-api` |
||
|
jif-oai
|
1824685a00 |
feat: extract shared tool executor interface (#22359)
## Why Codex still models model-visible tools and executable behavior largely inside `codex-core`, which makes it harder to evolve the tool system toward a single reusable abstraction for built-ins, MCP-backed tools, dynamic tools, and later tools injected from outside core. This PR takes the next incremental step in that direction by moving the common execution-facing pieces out of core and separating them from core-only orchestration. The intent is to let shared tool abstractions improve in one place, while `codex-core` keeps the parts that are still inherently host-specific today, such as `ToolInvocation`, dispatch wiring, and hook integration. This PR is mostly moving things around. The only interesting piece is this abstraction: https://github.com/openai/codex/pull/22359/changes#diff-81af519002548ba51ed102bdaaf77e081d40a1e73a6e5f9b104bbbc96a6f1b3dR13 ## What changed - Added `codex_tools::ToolExecutor<Invocation>` as the shared execution trait for model-visible tools. - Moved the reusable execution support types from `codex-core` into `codex-tools`: - `FunctionCallError` - `ToolPayload` - `ToolOutput` - Refactored core tool implementations so that execution behavior lives on `ToolExecutor<ToolInvocation>`, while `ToolHandler` remains the core-local extension point for hook payloads, telemetry tags, diff consumers, and other orchestration concerns. - Kept the registry and dispatch flow behaviorally unchanged while making the shared/extracted boundary explicit across built-in, MCP, dynamic, extension-backed, shell, and multi-agent tool handlers. ## Verification - `cargo test -p codex-tools` - `just fix -p codex-tools` - `just fix -p codex-core` - `cargo test -p codex-core` progressed through the updated tool surfaces and then hit the existing unrelated multi-agent stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`. |
||
|
jif-oai
|
7e97da7c13 |
chore: Keep view_image sandbox test in temp dir (#22355)
## Summary - move the `view_image` sandbox filesystem-read unit test onto a temporary cwd - keep the turn cwd and selected turn environment cwd aligned inside the test - avoid leaving `core/image.png` behind in the repo checkout after the test runs ## Root cause The test wrote `image.png` beneath `turn.cwd`, and the shared session test helper defaults that cwd to the current repo directory when no override is provided. ## Validation - `just fmt` - `cargo test -p codex-core tools::handlers::view_image::tests::handle_passes_sandbox_context_for_local_filesystem_reads` |
||
|
sayan-oai
|
2304ec45ca |
Remove unavailable MCP placeholder tool backfill (#22439)
## Why `UnavailableDummyTools` kept synthetic placeholder tools alive for historical tool calls whose backing MCP tool was no longer available. That path adds stale model-visible tool specs and special routing at the point where unavailable MCP calls should use ordinary current-tool handling. This removes the runtime backfill instead of preserving a second compatibility lane. ## Is it safe to remove? The unavailable tools were added in #17853 after a CS issue when a previously-called MCP tool failed to load and was omitted from the CS spec. Now that we have tool search, I think this is resolved: - API merges tools from previous TST output into effective tool set so theyre always in CS spec - if an MCP tool surfaced by TST later becomes unavailable, the model can still call it and it will just return model-visible error - both TST output and function call output are dropped on compaction so model will not remember old calls to MCP post compaction ## What changed - Delete unavailable-tool collection, placeholder handler, router/spec plumbing, and obsolete placeholder coverage. - Keep `features.unavailable_dummy_tools` as a removed no-op feature tombstone so existing configs still parse cleanly. - Add an integration-style `tool_search` regression test showing that a deferred MCP tool surfaced through `tool_search` still routes through MCP and returns a model-visible tool-call error rather than `unsupported call`. ## Verification - `cargo test -p codex-core tool_search` |
||
|
pakrym-oai
|
104fc14956 |
Encapsulate tool search entries in handlers (#22261)
## Why This builds on the handler-owned spec refactor by moving deferred tool-search metadata to the same handlers that already own tool specs. The registry builder no longer needs a separate prebuilt `tool_search_entries` path; it can collect searchable entries from deferred handlers directly. ## What changed - Added `search_info()` to tool handlers and implemented it for MCP and dynamic handlers. - Reused handler `spec()` output when constructing tool-search entries, adapting it into the deferred `LoadableToolSpec` shape expected by `tool_search`. - Simplified `build_tool_registry_builder(...)` so `tool_search` registration is based on deferred handlers with search info. - Removed the old standalone search-entry builders and now-unused `codex-tools` discovery helper exports. ## Verification - `cargo test -p codex-core tools::handlers::tool_search::tests:: -- --nocapture` - `cargo test -p codex-core tools::spec_plan::tests::search_tool -- --nocapture` - `cargo test -p codex-core tools::spec::tests:: -- --nocapture` - `cargo test -p codex-core tools::spec_plan::tests:: -- --nocapture` - `cargo test -p codex-tools` - `just fix -p codex-core` - `just fix -p codex-tools` |
||
|
pakrym-oai
|
67c8486462 |
tools: infer code-mode namespace descriptions from specs (#22406)
## Why Code mode already builds the merged nested `ToolSpec`s that feed the `exec` prompt. Keeping a separate `tool_namespaces` map in the planning path duplicated that metadata and left extra wrapper plumbing in `spec.rs`. ## What changed - derive code-mode namespace descriptions from the merged `ToolSpec::Namespace` entries before building the code-mode handlers - extract `build_code_mode_handlers(...)` so the code-mode-specific planning stays in one place - remove `tool_namespaces` from `ToolRegistryBuildParams` - delete the now-unused `McpToolPlanInputs` wrapper and related test helper plumbing ## Testing - `cargo test -p codex-core spec_plan` |
||
|
Andrei Eternal
|
fbfbfe5fc5 |
hooks: use new session IDs instead of thread IDs for hooks, apply parent's session ID to subagents' hooks (#22268)
## Why hook semantics treat `session_id` as shared across a root session and its subagents. Codex hooks were still emitting the current thread ID, which made spawned agents look like independent sessions and made it harder for hook integrations to correlate work across a root thread and its spawned helpers This change makes hooks use Codex's existing shared session identity so hook `session_id` matches the root-thread session across spawned subagents. ## What Changed - switch hook payloads to use the existing shared session identity from core instead of the current thread ID - cover all hook surfaces that expose `session_id`, including `SessionStart`, tool hooks, compact hooks, prompt-submit hooks, stop hooks, and legacy after-agent dispatch |
||
|
sayan-oai
|
1ae9867296 |
[codex] Remove tool search bucket limit override (#22381)
## Why `tool_search` still carries the server-specific result-cap path added in #17684 for `computer-use`: when the model omitted `limit`, a matching result expanded the search to 20 and then `limit_results_by_bucket` applied per-bucket caps. That makes default result handling depend on a one-off server exception instead of the single `TOOL_SEARCH_DEFAULT_LIMIT` path. This PR removes that custom branch so omitted `limit` values use the ordinary global default consistently. The implementation being retired is the pre-change bucketed search path in [`tool_search.rs`](https://github.com/openai/codex/blob/5e3ee5eddfa5333f2e0b011880abf0cbf92bd295/codex-rs/core/src/tools/handlers/tool_search.rs#L121-L190). ## What changed - Collapse `ToolSearchHandler::search` back to one BM25 search with the resolved limit. - Remove `limit_results_by_bucket`, the `computer-use` constants, and the omitted-limit plumbing that only existed for the override. - Drop dead `ToolSearchEntry::limit_bucket` metadata from deferred MCP and dynamic search entries. - Remove tests and helpers that only asserted the deleted override behavior. - Add direct handler-level unit coverage for omitted/default and explicit `tool_search` result limits. ## Validation - `cargo test -p codex-core tool_search` - The matching unit tests passed, including the new omitted/default and explicit result-limit coverage. - The broader `--test all` search-tool fixture phase then failed before sending mocked response requests in `tool_search_indexes_only_enabled_non_app_mcp_tools` and `tool_search_uses_non_app_mcp_server_instructions_as_namespace_description`. - `cargo test -p codex-core` - The touched tool-search coverage passed before the run later aborted in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed` with a stack overflow. |
||
|
Michael Bolin
|
9e7cdbd0d2 |
core: box multi-agent handler futures (#22266)
## Why This is the base PR in the split stack for the permissions migration. It isolates stack-safety work that had been mixed into the larger permissions PR, so reviewers can evaluate the async-future changes separately from the permissions model changes in #22267. The main risk this addresses is large or recursive multi-agent futures overflowing smaller runner stacks. A follow-up review also called out that `shutdown_live_agent` must remain quiescent: callers should not remove a live agent from tracking or release its spawn slot until the worker loop has actually terminated. ## What Changed - Boxes the large async futures in the multi-agent spawn, resume, and close tool handlers. - Boxes the `AgentControl` spawn and recursive close/shutdown paths that can otherwise build very deep futures. - Keeps `shutdown_live_agent` waiting for thread termination before removing/releasing the live agent, preserving the previous shutdown ordering while still boxing the recursive close path. ## Verification Strategy The focused local coverage was `cargo test -p codex-core multi_agents`, which exercises the multi-agent spawn/resume/close handlers, cascade close/resume behavior, and the shutdown path touched by this PR. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22266). * #22330 * #22329 * #22328 * #22327 * __->__ #22266 |
||
|
pakrym-oai
|
0173f71143 |
Refactor namespaced tool spec registration (#22256)
## Summary This refactor makes tool handlers the owner of the specs they can publish, so registry construction can register handlers once and separately publish only the specs that should be model-visible. The main motivation is deferred tools: MCP and dynamic tools still need handlers registered up front, but deferred tools should be discoverable through `tool_search` rather than emitted in the initial tool spec list. ## What changed - `McpHandler` and `DynamicToolHandler` can return their own `ToolSpec`. - `build_tool_registry_builder` now collects handlers, registers them through the no-spec path, and publishes only non-deferred handler specs. - Deferred MCP and dynamic tool names are combined into one `all_deferred_tools` set that drives spec filtering, code-mode deferred-tool signaling, and `tool_search` registration. - `tool_search` registration now requires both deferred tools and `namespace_tools`. - Namespace specs are merged in `spec_plan`, preserving top-level spec order, sorting tools within each namespace, and backfilling empty namespace descriptions. - Hosted web search and image-generation specs are included in the collected spec vector before namespace merge/publication, and tool-name tests that should not care about hosted relative order now compare sets. ## Testing - `cargo test -p codex-core tools::spec::tests:: -- --nocapture` - `cargo test -p codex-core tools::spec_plan::tests:: -- --nocapture` - `cargo test -p codex-core tools::router::tests::specs_filter_deferred_dynamic_tools -- --nocapture` - `cargo test -p codex-core suite::prompt_caching::prompt_tools_are_consistent_across_requests -- --nocapture` - `just fmt` - `just fix -p codex-core` - `cargo test -p codex-core -- --skip tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed` passed the library suite after skipping the known stack-overflowing unit test. Full `cargo test -p codex-core` currently hits a stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`; the same focused test reproduces on `origin/main`. |
||
|
pakrym-oai
|
960d42ddae |
code-mode: carry nested tool kind through runtime (#22377)
## Why Code mode only used nested spec lookup at execution time to rediscover whether a nested tool should be invoked as a function tool or a freeform tool. That information is already present in the enabled tool metadata that code mode builds to expose `tools.*` and `ALL_TOOLS`, so re-looking it up from the router was redundant and kept execution coupled to a separate spec lookup path. ## What Changed - thread `CodeModeToolKind` through the code-mode runtime `ToolCall` event and `CodeModeNestedToolCall` - emit the nested tool kind directly from the V8 callback using the already-enabled tool metadata - build nested tool payloads from the propagated kind instead of calling `find_spec` - remove the now-unused `find_spec` plumbing from the router and parallel runtime helpers - add unit coverage for function vs freeform payload shaping and update affected router tests ## Testing - `cargo test -p codex-code-mode` - `cargo test -p codex-core code_mode::tests` - `cargo test -p codex-core extension_tool_bundles_are_model_visible_and_dispatchable` - `cargo test -p codex-core model_visible_specs_filter_deferred_dynamic_tools` |
||
|
pakrym-oai
|
862b2122ee |
tools: remove is_mutating dispatch gating (#22382)
## Why Tool dispatch had two serialization mechanisms: - `supports_parallel_tool_calls` decides whether a tool participates in the shared parallel-execution lock. - `is_mutating` separately gated some calls inside dispatch. That second hook no longer carried its weight. The remaining parallel-support flag is already the per-tool concurrency policy, so keeping a second mutating gate made dispatch harder to follow and left behind extra session plumbing that only existed for that path. ## What changed - Removed `is_mutating` from tool handlers and deleted the `tool_call_gate` path that existed only to support it. - Simplified dispatch and routing to rely on the existing per-tool `supports_parallel_tool_calls` boolean. - Dropped the now-unused handler overrides and related session/test scaffolding. - Kept the router/parallel tests focused on the surviving per-tool behavior. - Removed the unused `codex-utils-readiness` dependency from `codex-core` as a follow-up fix for `cargo shear`. ## Testing - `cargo test -p codex-core parallel_support_does_not_match_namespaced_local_tool_names` - `cargo test -p codex-core mcp_parallel_support_uses_handler_data` - `cargo test -p codex-core tools_without_handlers_do_not_support_parallel` |
||
|
Chris BookholtandCodex
|
5e3ee5eddf |
[codex] Tighten unified exec sandbox setup (#22207)
## Summary - tighten unified exec sandbox initialization - preserve the requested process workdir independently from sandbox setup - add regression coverage for the updated invariant ## Validation - Ran `/tmp/cargo-tools/bin/just fmt`. - Ran the targeted `codex-core` regression test successfully. - Ran `cargo test -p codex-core`; it did not complete cleanly because unrelated existing agent/config-loader tests failed and the run later aborted on a stack overflow in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed`. Co-authored-by: Codex <noreply@openai.com> |
||
|
pakrym-oai
|
c9e46ed639 |
[codex] Make handlers own parallel tool support (#22254)
## Why `ToolRouter::tool_supports_parallel()` was still consulting configured specs when a handler lookup missed, even though parallel schedulability is really a property of the executable handler. Keeping that metadata on `ConfiguredToolSpec` duplicated state between the model-visible spec layer and the runtime handler layer. This change makes handlers the sole source of truth for parallel tool support and removes the extra spec wrapper that only existed to carry duplicated metadata. ## What changed - removed `ConfiguredToolSpec` and store plain `ToolSpec` values in the registry/router builder path - changed `ToolRouter::tool_supports_parallel()` to consult only the handler registry and fall back to `false` - simplified spec collection and test helpers to operate directly on `ToolSpec` - updated router/spec tests to cover handler-owned parallel behavior and the no-handler fallback ## Validation - `cargo test -p codex-tools` - `cargo test -p codex-core mcp_parallel_support_uses_handler_data` - `cargo test -p codex-core deferred_responses_api_tool_serializes_with_defer_loading` - `cargo test -p codex-core tools_without_handlers_do_not_support_parallel` - `cargo test -p codex-core request_plugin_install_can_be_registered_without_search_tool` ## Docs No documentation updates needed. |
||
|
pakrym-oai
|
79c65f816c |
[codex] Filter legacy warning messages during compaction (#22243)
## Why Older sessions can contain model-warning records persisted as `user` messages, including the unified exec process-limit warning, the `apply_patch`-via-`exec_command` warning, and the model-mismatch high-risk cyber fallback warning. Those warnings are no longer produced as conversation history items, but when old sessions compact they should still be recognized as injected context rather than preserved as real user turns. ## What changed - Removed `record_model_warning` and the production paths that emitted these warning messages into conversation history. - Added `LegacyUnifiedExecProcessLimitWarning`, `LegacyApplyPatchExecCommandWarning`, and `LegacyModelMismatchWarning` contextual fragments that are used only for matching old persisted messages. - Registered the legacy fragments with contextual user message detection so compaction filters them through the existing fragment path. - Added focused compaction coverage for old warning messages being dropped during compacted-history processing. ## Testing - `cargo test -p codex-core warning` - `just fix -p codex-core` |
||
|
Abhinav
|
d08906a944 |
Support PreToolUse updatedInput rewrites (#20527)
## Why
`PreToolUse` already exposes `updatedInput` in its hook output schema,
but Codex currently rejects it instead of applying the rewrite. That
leaves hook authors unable to make the documented pre-execution
adjustment to a tool call before it runs.
## What
- Accept `updatedInput` from `PreToolUse` hooks when paired with
`permissionDecision: "allow"`.
- Apply the rewritten input before dispatch so the tool executes the
updated payload, not the original one.
- Preserve the stable hook-facing compatibility shapes that
participating tool handlers expose:
- Bash-like tools (`shell`, `container.exec`, `local_shell`,
`shell_command`, `exec_command`) use `{ "command": ... }`.
- `apply_patch` exposes its patch body through the same command-shaped
hook contract.
- MCP tools expose their JSON argument object directly.
- Keep each participating tool handler responsible for translating
hook-facing `updatedInput` back into its concrete invocation shape.
## Verification
Direct Bash-like rewrite coverage:
- `pre_tool_use_rewrites_shell_before_execution`
- `pre_tool_use_rewrites_container_exec_before_execution`
- `pre_tool_use_rewrites_local_shell_before_execution`
- `pre_tool_use_rewrites_shell_command_before_execution`
- `pre_tool_use_rewrites_exec_command_before_execution`
These cases assert that each supported Bash-like surface runs only the
rewritten command while the hook still observes the original `{
"command": ... }` input.
`pre_tool_use_rewrites_apply_patch_before_execution`
- Model emits one patch.
- Hook swaps in a different patch.
- Asserts only the rewritten file is created, and the hook saw the
original patch.
`pre_tool_use_rewrites_code_mode_nested_exec_command_before_execution`
- Model runs one nested shell command from code mode.
- Hook rewrites it.
- Asserts only the rewritten command runs, and the hook saw the original
nested input.
`pre_tool_use_rewrites_mcp_tool_before_execution`
- Model calls the RMCP echo tool.
- Hook rewrites the MCP arguments.
- Asserts the MCP server receives and returns the rewritten message, not
the original one.
|
||
|
starr-openaiandCodex
|
17ed5ad0b0 |
Apply sandbox context to local view_image reads (#21861)
## Summary - create a selected-cwd filesystem sandbox context for view_image metadata and file reads in both local and remote environments - add a local restricted-profile regression test for the previously unsandboxed read path ## Validation - just fmt - bazel test --bes_backend= --bes_results_url= --test_output=errors --test_filter=view_image::tests::handle_passes_sandbox_context_for_local_filesystem_reads //codex-rs/core:core-unit-tests --------- Co-authored-by: Codex <noreply@openai.com> |
||
|
pakrym-oai
|
ed5944ba1d |
Simplify MCP tool handler plumbing (#21595)
## Why The MCP tool path had accumulated a few core-owned special cases: a dedicated payload variant, resolver plumbing, a legacy `AfterToolUse` translation path, and a side channel for parallel-call metadata. That made `ToolRegistry` and the spec builder know more about MCP than they needed to. This change moves MCP-specific execution details back onto `ToolInfo` and `McpHandler` so `codex-core` can treat MCP calls like normal function calls while still preserving MCP-specific dispatch and telemetry behavior where it belongs. ## What changed - removed `resolve_mcp_tool_info`, `ToolPayload::Mcp`, `ToolKind`, and the remaining registry-side MCP resolver path - stored MCP routing metadata directly on `McpHandler` and `ToolInfo`, including `supports_parallel_tool_calls` - deleted the legacy `AfterToolUse` consumer in `core`, which removes the need for handler-specific `after_tool_use_payload` implementations - switched tool-result telemetry to handler-provided tags and kept MCP-specific dispatch payload construction inside the handler - simplified tool spec planning/building by passing `ToolInfo` directly and dropping the direct/deferred MCP wrapper structs and the parallel-server side table ## Testing - `cargo check -p codex-core -p codex-mcp -p codex-otel` - `cargo test -p codex-core mcp_parallel_support_uses_exact_payload_server` - `cargo test -p codex-core direct_mcp_tools_register_namespaced_handlers` - `cargo test -p codex-core search_tool_description_lists_each_mcp_source_once` - `cargo test -p codex-mcp list_all_tools_uses_startup_snapshot_while_client_is_pending` - `just fix -p codex-core -p codex-mcp -p codex-otel` |
||
|
starr-openaiandCodex
|
22e84c49d0 |
Support multi-environment apply_patch selection (#21617)
## Summary - add multi-environment apply_patch routing for both freeform and function-call tool flows - parse and reconcile the optional environment selector in the main apply_patch parser, then verify against the selected environment in the handler - carry environment_id through runtime and approval surfaces so remote-targeted patches stay explicit end to end ## Testing - just fmt - remote exec-server e2e: `cargo test -p codex-core --test all apply_patch_multi_environment_uses_remote_executor -- --nocapture` on dev via `scripts/test-remote-env.sh` --------- Co-authored-by: Codex <noreply@openai.com> |
||
|
|
6506765168 |
fix(permissions): preserve managed deny-read during escalation (#15977)
## Why
Managed filesystem `deny_read` requirements are administrator-enforced
restrictions on specific paths. Once those requirements are active,
Codex should not drop them just because an execution path would
otherwise leave the sandbox.
Before this change, an explicit escalation, a prefix-rule allow, a
sandbox-denial retry, or an app-server legacy sandbox override could
rebuild the runtime policy without those managed read-deny entries and
expose a path the administrator had marked unreadable.
This is narrower than general sandbox-mode constraints. If an enterprise
only sets `allowed_sandbox_modes`, a trusted `prefix_rule(..., decision
= "allow")` can still run its matching command unsandboxed; this PR only
preserves managed filesystem `deny_read` restrictions across those
paths.
## What Changed
- Mark filesystem policies built from managed `deny_read` requirements
so callers can tell when those deny entries must survive escalation.
- Preserve managed deny-read entries when runtime permission profiles
are rebuilt through protocol, app-server, or legacy sandbox-policy
compatibility paths.
- Keep managed deny-read attempts inside the selected sandbox on the
first attempt and after sandbox-denial retries.
- Preserve the same behavior in the zsh-fork escalation path, including
prefix-rule-driven escalation.
- Add a regression test showing the opposite case too: without managed
deny-read, a prefix-rule allow still chooses unsandboxed execution.
## Verification
Targeted automated verification:
```shell
cargo test -p codex-core shell_request_escalation_execution_is_explicit -- --nocapture
cargo test -p codex-core prefix_rule_uses_unsandboxed_execution_without_managed_deny_read -- --nocapture
cargo test -p codex-core prefix_rule_preserves_managed_deny_read_escalation -- --nocapture
cargo test -p codex-protocol permission_profile_round_trip_preserves_filesystem_policy_metadata -- --nocapture
cargo test -p codex-protocol preserving_deny_entries_keeps_unrestricted_policy_enforceable -- --nocapture
cargo test -p codex-app-server-protocol permission_profile_file_system_permissions_preserves_policy_metadata -- --nocapture
cargo check -p codex-app-server -p codex-tui
```
Smoke-test invocations:
```shell
# macOS exact deny + allowed control
codex exec --skip-git-repo-check -C "$ROOT" \
-c 'default_permissions="deny_read_smoke"' \
-c 'permissions.deny_read_smoke.filesystem={":minimal"="read",":project_roots"={"."="write","secrets"="none","future-secret"="none","**/*.env"="none"}}' \
'Run shell commands only. Print the contents of allowed.txt. Then test whether reading secrets/exact-secret.txt succeeds without printing that file if it does. End with exactly two lines: allowed=<contents> and exact_secret=<BLOCKED or READABLE>.'
# Linux exact deny + allowed control
codex exec --skip-git-repo-check -C "$ROOT" \
-c 'default_permissions="deny_read_smoke"' \
-c 'permissions.deny_read_smoke.filesystem={":minimal"="read",glob_scan_max_depth=3,":project_roots"={"."="write","secrets"="none","future-secret"="none","**/*.env"="none"}}' \
'Run shell commands only. Print the contents of allowed.txt. Then test whether reading secrets/exact-secret.txt succeeds without printing that file if it does. End with exactly two lines: allowed=<contents> and exact_secret=<BLOCKED or READABLE>.'
```
Observed manual smoke matrix:
| Case | macOS Seatbelt | Linux bubblewrap |
| --- | --- | --- |
| `cat allowed.txt` | Pass | Pass |
| `cat secrets/exact-secret.txt` | Blocked | Blocked |
| `cat envs/root.env` | Blocked | Blocked |
| `cat envs/nested/one.env` | Blocked | Blocked |
| `cat envs/nested/two.env` | Blocked | Blocked |
| `cat alias-to-secrets/exact-secret.txt` | Blocked | Blocked |
| Missing denied path | A file created after sandbox setup remained
unreadable | Creation was blocked by the reserved missing-path
placeholder, and the placeholder was cleaned up after exit |
| Real `codex exec` shell turn | Pass | Pass |
Notes:
- The Linux smoke run used the fallback glob walker because the devbox
did not have `rg` installed.
- The smoke matrix verifies the end-to-end filesystem behavior on macOS
and Linux; the escalation-specific behavior is covered by the focused
tests above.
---------
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Charlie Marsh <charliemarsh@openai.com>
|