Commit Graph

8 Commits

  • feat(requirements): support allowed_approval_reviewers (#16701)
    ## Description
    
    Add requirements.toml support for `allowed_approvals_reviewers =
    ["user", "guardian_subagent"]`, so admins can now restrict the use of
    guardian mode.
    
    Note: If a user sets a reviewer that isn’t allowed by requirements.toml,
    config loading falls back to the first allowed reviewer and emits a
    startup warning.
    
    The table below describes the possible admin controls.
    | Admin intent | `requirements.toml` | User `config.toml` | End result |
    |---|---|---|---|
    | Leave Guardian optional | omit `allowed_approvals_reviewers` or set
    `["user", "guardian_subagent"]` | user chooses `approvals_reviewer =
    "user"` or `"guardian_subagent"` | Guardian off for `user`, on for
    `guardian_subagent` + `approval_policy = "on-request"` |
    | Force Guardian off | `allowed_approvals_reviewers = ["user"]` | any
    user value | Effective reviewer is `user`; Guardian off |
    | Force Guardian on | `allowed_approvals_reviewers =
    ["guardian_subagent"]` and usually `allowed_approval_policies =
    ["on-request"]` | any user reviewer value; user should also have
    `approval_policy = "on-request"` unless policy is forced | Effective
    reviewer is `guardian_subagent`; Guardian on when effective approval
    policy is `on-request` |
    | Allow both, but default to manual if user does nothing |
    `allowed_approvals_reviewers = ["user", "guardian_subagent"]` | omit
    `approvals_reviewer` | Effective reviewer is `user`; Guardian off |
    | Allow both, and user explicitly opts into Guardian |
    `allowed_approvals_reviewers = ["user", "guardian_subagent"]` |
    `approvals_reviewer = "guardian_subagent"` and `approval_policy =
    "on-request"` | Guardian on |
    | Invalid admin config | `allowed_approvals_reviewers = []` | anything |
    Config load error |
  • chore: clean up argument-comment lint and roll out all-target CI on macOS (#16054)
    ## Why
    
    `argument-comment-lint` was green in CI even though the repo still had
    many uncommented literal arguments. The main gap was target coverage:
    the repo wrapper did not force Cargo to inspect test-only call sites, so
    examples like the `latest_session_lookup_params(true, ...)` tests in
    `codex-rs/tui_app_server/src/lib.rs` never entered the blocking CI path.
    
    This change cleans up the existing backlog, makes the default repo lint
    path cover all Cargo targets, and starts rolling that stricter CI
    enforcement out on the platform where it is currently validated.
    
    ## What changed
    
    - mechanically fixed existing `argument-comment-lint` violations across
    the `codex-rs` workspace, including tests, examples, and benches
    - updated `tools/argument-comment-lint/run-prebuilt-linter.sh` and
    `tools/argument-comment-lint/run.sh` so non-`--fix` runs default to
    `--all-targets` unless the caller explicitly narrows the target set
    - fixed both wrappers so forwarded cargo arguments after `--` are
    preserved with a single separator
    - documented the new default behavior in
    `tools/argument-comment-lint/README.md`
    - updated `rust-ci` so the macOS lint lane keeps the plain wrapper
    invocation and therefore enforces `--all-targets`, while Linux and
    Windows temporarily pass `-- --lib --bins`
    
    That temporary CI split keeps the stricter all-targets check where it is
    already cleaned up, while leaving room to finish the remaining Linux-
    and Windows-specific target-gated cleanup before enabling
    `--all-targets` on those runners. The Linux and Windows failures on the
    intermediate revision were caused by the wrapper forwarding bug, not by
    additional lint findings in those lanes.
    
    ## Validation
    
    - `bash -n tools/argument-comment-lint/run.sh`
    - `bash -n tools/argument-comment-lint/run-prebuilt-linter.sh`
    - shell-level wrapper forwarding check for `-- --lib --bins`
    - shell-level wrapper forwarding check for `-- --tests`
    - `just argument-comment-lint`
    - `cargo test` in `tools/argument-comment-lint`
    - `cargo test -p codex-terminal-detection`
    
    ## Follow-up
    
    - Clean up remaining Linux-only target-gated callsites, then switch the
    Linux lint lane back to the plain wrapper invocation.
    - Clean up remaining Windows-only target-gated callsites, then switch
    the Windows lint lane back to the plain wrapper invocation.
  • permissions: remove macOS seatbelt extension profiles (#15918)
    ## Why
    
    `PermissionProfile` should only describe the per-command permissions we
    still want to grant dynamically. Keeping
    `MacOsSeatbeltProfileExtensions` in that surface forced extra macOS-only
    approval, protocol, schema, and TUI branches for a capability we no
    longer want to expose.
    
    ## What changed
    
    - Removed the macOS-specific permission-profile types from
    `codex-protocol`, the app-server v2 API, and the generated
    schema/TypeScript artifacts.
    - Deleted the core and sandboxing plumbing that threaded
    `MacOsSeatbeltProfileExtensions` through execution requests and seatbelt
    construction.
    - Simplified macOS seatbelt generation so it always includes the fixed
    read-only preferences allowlist instead of carrying a configurable
    profile extension.
    - Removed the macOS additional-permissions UI/docs/test coverage and
    deleted the obsolete macOS permission modules.
    - Tightened `request_permissions` intersection handling so explicitly
    empty requested read lists are preserved only when that field was
    actually granted, avoiding zero-grant responses being stored as active
    permissions.
  • chore(exec_policy) ExecPolicyRequirementScenario tests (#15415)
    ## Summary
    Consolidate exec_policy_tests on `ExecApprovalRequirementScenario` for
    consistency.
    
    ## Testing
    - [x] These are tests
  • fix(core) disable command_might_be_dangerous when unsandboxed (#15036)
    ## Summary
    If we are in a mode that is already explicitly un-sandboxed, then
    `ApprovalPolicy::Never` should not block dangerous commands.
    
    ## Testing
    - [x] Existing unit test covers old behavior
    - [x] Added a unit test for this new case
  • fix(subagents) share execpolicy by default (#13702)
    ## Summary
    If a subagent requests approval, and the user persists that approval to
    the execpolicy, it should (by default) propagate. We'll need to rethink
    this a bit in light of coming Permissions changes, though I think this
    is closer to the end state that we'd want, which is that execpolicy
    changes to one permissions profile should be synced across threads.
    
    ## Testing
    - [x] Added integration test
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • fix: move inline codex-rs/core unit tests into sibling files (#14444)
    ## Why
    PR #13783 moved the `codex.rs` unit tests into `codex_tests.rs`. This
    applies the same extraction pattern across the rest of `codex-rs/core`
    so the production modules stay focused on runtime code instead of large
    inline test blocks.
    
    Keeping the tests in sibling files also makes follow-up edits easier to
    review because product changes no longer have to share a file with
    hundreds or thousands of lines of test scaffolding.
    
    ## What changed
    - replaced each inline `mod tests { ... }` in `codex-rs/core/src/**`
    with a path-based module declaration
    - moved each extracted unit test module into a sibling `*_tests.rs`
    file, using `mod_tests.rs` for `mod.rs` modules
    - preserved the existing `cfg(...)` guards and module-local structure so
    the refactor remains structural rather than behavioral
    
    ## Testing
    - `cargo test -p codex-core --lib` (`1653 passed; 0 failed; 5 ignored`)
    - `just fix -p codex-core`
    - `cargo fmt --check`
    - `cargo shear`