Commit Graph

20 Commits

  • [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`
  • [codex] Make contextual user fragments dyn-renderable (#23397)
    ## Why
    `ContextualUserFragment` needs to be usable behind `dyn` for render-only
    paths, but associated constants made the trait non-object-safe.
    
    ## What changed
    - Replaced associated constants with trait methods so `dyn
    ContextualUserFragment` can render fragments.
    - Preserved the existing typed `T::matches_text(text)` registration
    pattern via `type_markers()`.
    - Kept default `render()` on the main trait so implementations only
    provide role, markers, and body.
    - Added unit coverage for rendering a `Box<dyn ContextualUserFragment>`.
    
    ## Verification
    - `cargo test -p codex-core contextual_user_fragment_is_dyn_compatible`
    - `just fix -p codex-core`
  • context: remove legacy permissions instructions helper (#22790)
    ## Why
    
    The permissions instruction builder should consume the new permissions
    model directly. Keeping a `SandboxPolicy` conversion helper in this path
    encourages new code to route through legacy sandbox policy values even
    when the caller already has a `PermissionProfile`.
    
    ## What Changed
    
    - Removed `PermissionsInstructions::from_policy`.
    - Removed the test that exercised that legacy helper.
    - Left the existing profile-based instruction coverage in place.
    
    ## How To Review
    
    Review `codex-rs/core/src/context/permissions_instructions.rs` first.
    This PR is intentionally narrow: the production behavior should be
    unchanged for profile callers, and the deleted surface was only a
    convenience adapter from `SandboxPolicy`.
    
    ## Verification
    
    - `cargo test -p codex-core builds_permissions_from_profile`
    
    
    
    
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/22790).
    * #22795
    * #22792
    * #22791
    * __->__ #22790
  • 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
  • [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`
  • 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.
  • [codex] compact network context rendering (#21875)
    ## Why
    
    The model-visible `<network>` context currently repeats indentation and
    a pair of XML tags for every allowed or denied domain. Large domain sets
    spend a surprising amount of prompt budget on that scaffolding instead
    of the actual policy values.
    
    ## What changed
    
    - Render allowed domains as one comma-separated `<allowed>` value
    instead of one element per domain.
    - Render denied domains the same way.
    - Keep the full allow/deny domain sets model-visible while updating the
    serialization and settings-update coverage for the denser shape.
    
    ## Example
    
    Before:
    ```xml
    <network enabled="true">
      <allowed>api.example.test</allowed>
      <allowed>cdn.example.test</allowed>
      <denied>blocked.example.test</denied>
    </network>
    ```
    
    After:
    ```xml
    <network enabled="true"><allowed>api.example.test,cdn.example.test</allowed><denied>blocked.example.test</denied></network>
    ```
    
    ## Validation
    
    - `cargo test -p codex-core environment_context`
    - `cargo test -p codex-core
    build_settings_update_items_emits_environment_item_for_network_changes`
    - Ran a local `codex` session with a real network context containing 121
    allowed domains and 42 denied domains, then inspected the raw prompt
    with `raw_token_viewer_cli.py`. With the same domain set, the rendered
    `<network>` section shrank from 7,175 characters across 161 lines to
    3,666 characters on one line, and the containing environment-context
    block fell from 6,428 tokens to 5,379 tokens.
  • Avoid hard-coded environment context shell (#21390)
    ## Summary
    - make resolved turn environment shell metadata optional instead of
    hard-coding bash
    - render environment context shells from explicit environment metadata
    when present, falling back to the existing session shell
    - update environment context tests for inherited PowerShell-style
    fallback and explicit per-environment shell override
    
    ## Testing
    - Not run (not requested; formatted with `just fmt`).
    
    Co-authored-by: Codex <noreply@openai.com>
  • Prepare selected environment plumbing (#20669)
    ## Why
    This is a prep PR in the multi-environment process-tool stack. It
    separates ownership/config cleanup from the behavior change that teaches
    process tools to route by selected environment, so the follow-up PR can
    focus on model-facing `environment_id` behavior.
    
    ## Stack
    1. https://github.com/openai/codex/pull/20646 - `EnvironmentContext`
    rendering for selected environments
    2. https://github.com/openai/codex/pull/20669 - selected-environment
    ownership and tool config prep (this PR)
    3. https://github.com/openai/codex/pull/20647 - process-tool
    `environment_id` routing
    
    ## What Changed
    - keep the resolved turn environment list wrapped in
    `ResolvedTurnEnvironments` through `TurnContext` instead of unwrapping
    it back to a raw `Vec`
    - add `TurnContext::resolve_path_against` so cwd-relative path
    resolution has one shared helper
    - replace the old tool config boolean with `ToolEnvironmentMode::{None,
    Single, Multiple}`
    
    ## Testing
    - Tests not run locally; this prep refactor is covered by GitHub CI for
    the stack.
    
    Co-authored-by: Codex <noreply@openai.com>
  • Surface multi-environment choices in environment context (#20646)
    ## Why
    The model needs a way to see which environments are available during a
    multi-environment turn without changing the legacy single-environment
    prompt surface or pulling replay/persistence changes into the same
    review.
    
    ## Stack
    1. https://github.com/openai/codex/pull/20646 - `EnvironmentContext`
    rendering for selected environments (this PR)
    2. https://github.com/openai/codex/pull/20669 - selected-environment
    ownership and tool config prep
    3. https://github.com/openai/codex/pull/20647 - process-tool
    `environment_id` routing
    
    ## What Changed
    - extend `environment_context` so multi-environment turns render an
    `<environments>` block with the selected environment ids and cwd values
    - keep zero- and single-environment turns on the existing cwd-only
    render path
    - keep replay and persistence paths on the legacy surface for now so
    this PR stays scoped to live prompt rendering
    - add focused coverage in
    `codex-rs/core/src/context/environment_context_tests.rs`
    
    ## Testing
    - CI
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • feat: split memories part 2 (#19860)
    Keep extracting memories out of core and moving the write trigger in the
    app-server
    This is temporary and it should move at the client level as a follow-up
    This makes core fully independant from `codex-memories-write`
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • permissions: make runtime config profile-backed (#19606)
    ## Why
    
    This supersedes #19391. During stack repair, GitHub marked #19391 as
    merged into a temporary stack branch rather than into `main`, so the
    runtime-config change needed a fresh PR.
    
    `PermissionProfile` is now the canonical permissions shape after #19231
    because it can distinguish `Managed`, `Disabled`, and `External`
    enforcement while also carrying filesystem rules that legacy
    `SandboxPolicy` cannot represent cleanly. Core config and session state
    still needed to accept profile-backed permissions without forcing every
    profile through the strict legacy bridge, which rejected valid runtime
    profiles such as direct write roots.
    
    The unrelated CI/test hardening that previously rode along with this PR
    has been split into #19683 so this PR stays focused on the permissions
    model migration.
    
    ## What Changed
    
    - Adds `Permissions.permission_profile` and
    `SessionConfiguration.permission_profile` as constrained runtime state,
    while keeping `sandbox_policy` as a legacy compatibility projection.
    - Introduces profile setters that keep `PermissionProfile`, split
    filesystem/network policies, and legacy `SandboxPolicy` projections
    synchronized.
    - Uses a compatibility projection for requirement checks and legacy
    consumers instead of rejecting profiles that cannot round-trip through
    `SandboxPolicy` exactly.
    - Updates config loading, config overrides, session updates, turn
    context plumbing, prompt permission text, sandbox tags, and exec request
    construction to carry profile-backed runtime permissions.
    - Preserves configured deny-read entries and `glob_scan_max_depth` when
    command/session profiles are narrowed.
    - Adds `PermissionProfile::read_only()` and
    `PermissionProfile::workspace_write()` presets that match legacy
    defaults.
    
    ## Verification
    
    - `cargo test -p codex-core direct_write_roots`
    - `cargo test -p codex-core runtime_roots_to_legacy_projection`
    - `cargo test -p codex-app-server
    requested_permissions_trust_project_uses_permission_profile_intent`
    
    
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19606).
    * #19395
    * #19394
    * #19393
    * #19392
    * __->__ #19606
  • permissions: remove legacy read-only access modes (#19449)
    ## Why
    
    `ReadOnlyAccess` was a transitional legacy shape on `SandboxPolicy`:
    `FullAccess` meant the historical read-only/workspace-write modes could
    read the full filesystem, while `Restricted` tried to carry partial
    readable roots. The partial-read model now belongs in
    `FileSystemSandboxPolicy` and `PermissionProfile`, so keeping it on
    `SandboxPolicy` makes every legacy projection reintroduce lossy
    read-root bookkeeping and creates unnecessary noise in the rest of the
    permissions migration.
    
    This PR makes the legacy policy model narrower and explicit:
    `SandboxPolicy::ReadOnly` and `SandboxPolicy::WorkspaceWrite` represent
    the old full-read sandbox modes only. Split readable roots, deny-read
    globs, and platform-default/minimal read behavior stay in the runtime
    permissions model.
    
    ## What changed
    
    - Removes `ReadOnlyAccess` from
    `codex_protocol::protocol::SandboxPolicy`, including the generated
    `access` and `readOnlyAccess` API fields.
    - Updates legacy policy/profile conversions so restricted filesystem
    reads are represented only by `FileSystemSandboxPolicy` /
    `PermissionProfile` entries.
    - Keeps app-server v2 compatible with legacy `fullAccess` read-access
    payloads by accepting and ignoring that no-op shape, while rejecting
    legacy `restricted` read-access payloads instead of silently widening
    them to full-read legacy policies.
    - Carries Windows sandbox platform-default read behavior with an
    explicit override flag instead of depending on
    `ReadOnlyAccess::Restricted`.
    - Refreshes generated app-server schema/types and updates tests/docs for
    the simplified legacy policy shape.
    
    ## Verification
    
    - `cargo check -p codex-app-server-protocol --tests`
    - `cargo check -p codex-windows-sandbox --tests`
    - `cargo test -p codex-app-server-protocol sandbox_policy_`
    
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/19449).
    * #19395
    * #19394
    * #19393
    * #19392
    * #19391
    * __->__ #19449
  • feat: Compress skill paths with root aliases (#19098)
    Add skill root tracking so model-visible skill lists can use short path
    aliases when absolute paths would exceed the metadata budget.
  • Make MultiAgentV2 interruption markers assistant-authored (#19124)
    ## Why
    
    `MultiAgentV2` follow-up messages are delivered to agents as
    assistant-authored `InterAgentCommunication` envelopes. When
    `followup_task` used `interrupt: true`, the interrupted-turn guidance
    was still persisted as a contextual user message, so model-visible
    history made a system-generated interruption boundary look
    user-authored.
    
    This keeps interruption guidance consistent with the rest of the v2
    inter-agent message stream while preserving the legacy marker shape for
    non-v2 sessions.
    
    ## What changed
    
    - Make `interrupted_turn_history_marker` feature-aware.
    - Record the interrupted-turn marker as an assistant `OutputText`
    message when `Feature::MultiAgentV2` is enabled.
    - Keep the existing user contextual fragment for non-v2 sessions.
    - Apply the same feature-aware marker to interrupted fork snapshots.
    - Add coverage for the live `followup_task` interrupt path and the
    helper-level v2 marker shape.
    
    ## Testing
    
    - `cargo test -p codex-core
    multi_agent_v2_followup_task_interrupts_busy_child_without_losing_message
    -- --nocapture`
    - `cargo test -p codex-core
    multi_agent_v2_interrupted_marker_uses_assistant_output_message --
    --nocapture`
    - `cargo test -p codex-core interrupted_fork_snapshot -- --nocapture`
  • feat: drop spawned-agent context instructions (#19127)
    ## Why
    
    MultiAgentV2 children should not receive an extra model-visible
    developer fragment just because they were spawned. The parent/configured
    developer instructions should carry through normally, but the dedicated
    `<spawned_agent_context>` block is no longer desired.
    
    ## What changed
    
    - Removed the `SpawnAgentInstructions` context fragment and its
    `<spawned_agent_context>` wrapper.
    - Stopped appending spawned-agent instructions in
    `codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs`.
    - Updated subagent notification coverage to assert inherited parent
    developer instructions without expecting the spawned-agent wrapper.
    
    ## Verification
    
    - `cargo test -p codex-core --test all
    spawned_multi_agent_v2_child_inherits_parent_developer_context --
    --nocapture`
    - `cargo test -p codex-core --test all
    skills_toggle_skips_instructions_for_parent_and_spawned_child --
    --nocapture`
    - `cargo test -p codex-core --test all subagent_notifications --
    --nocapture`
  • Rename approvals reviewer variant to auto-review (#19056)
    ## Why
    
    `approvals_reviewer` now uses `auto_review` as the canonical config/API
    value after #18504, but the Rust enum variant and nearby helper/test
    names still used `GuardianSubagent` / guardian approval wording. That
    made follow-up code and reviews confusing even though the external value
    had already moved to Auto-review.
    
    ## What changed
    
    - Renamed `ApprovalsReviewer::GuardianSubagent` to
    `ApprovalsReviewer::AutoReview`.
    - Updated protocol, app-server, config, core, TUI, exec, and analytics
    test callsites.
    - Renamed nearby helper/test names from guardian approval wording to
    Auto-review wording where they refer to the approvals reviewer mode.
    - Preserved wire compatibility:
      - `auto_review` remains the canonical serialized value.
      - `guardian_subagent` remains accepted as a legacy alias.
    
    This intentionally does not rename the `[features].guardian_approval`
    key, `Feature::GuardianApproval`, `core/src/guardian`, analytics event
    names, or app-server Guardian review event types.
    
    ## Verification
    
    - `cargo test -p codex-protocol
    approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent`
    - `cargo test -p codex-app-server-protocol
    approvals_reviewer_serializes_auto_review_and_accepts_legacy_guardian_subagent`
    - `cargo test -p codex-config approvals_reviewer`
    - `cargo test -p codex-tui update_feature_flags`
    - `cargo test -p codex-core permissions_instructions`
    - `cargo test -p codex-tui permissions_selection`
  • Split DeveloperInstructions into individual fragments. (#18813)
    Split DeveloperInstructions into individual fragments.
  • Organize context fragments (#18794)
    Organize context fragments under `core/context`. Implement same trait on
    all of them.