Commit Graph

12 Commits

  • path-uri: clarify host-native path conversion (#29501)
    ## Why
    
    Downstream refactors are producing confusing code with this
    functionality having a very generic name. Encoding the specific
    conversion approach in the method name makes it clearer.
    
    ## What
    
    Rename `PathUri::from_path` to `PathUri::from_host_native_path` and
    update its Rust call sites.
  • apply-patch: carry paths as PathUri (#28854)
    ## Why
    
    Allows the model to edit files that are hosted on a different OS than
    where app-server is running.
    
    ## What
    
    * Use `PathUri` for apply_patch-internal data structures
    * Limit `PathUri` -> `AbsolutePathBuf` conversion to cases where the
    inferred path convention matches the host OS, allows requiring valid
    paths to pass to perms check
    * Adds `PathConvention::path_segments()` for iterating over path
    segments regardless of OS
    * Handle cross-platform relative paths in path filename parsing for
    sniffing a shell
    * Ensure we can apply patches in the wine e2e test
  • unified-exec: preserve PathUri through exec-server (#28681)
    ## Why
    
    It should be possible for app-server to handle "foreign" OS paths in
    unified_exec working directories, allowing e.g. a Linux app-server to
    run processes on e.g. a Windows exec-server.
    
    ## What
    
    Convert the core unified_exec cwd values to use `PathUri`.
    
    Adds fallible path conversion in several places to try to minimize the
    scope of this change. The only time this change suppresses errors from
    converting `PathUri` to an `AbsolutePathBuf` is when the turn is
    configured with no sandboxing at all to allow us to make progress
    testing without sandboxing.
    
    Future changes to apply_patch and sandboxing will clean up these error
    paths.
    
    A tool's cwd is resolved from joining a model-provided workdir to the
    environment's cwd. When using `AbsolutePathBuf::join()`, an
    absolute-path workdir would overwrite the environment's cwd and we would
    resolve permissions/sandboxing against the model-provided path. This
    change extends `PathUri::join()` to also treat an absolute rhs as an
    override of the base/lhs.
    
    This also removes some coverage from the remove_env_windows tests until
    a follow-up converts foreign paths in command exec events correctly.
    
    ## Breaking Changes
    
    When using `AbsolutePathBuf::join()` for workdir resolution, we ended up
    resolving tilde-prefixed paths against the app-server's `$HOME`, e.g.
    `~/foo/bar` becomes `/home/anp/foo/bar`. It's difficult to do this with
    `PathUri` joining, so after offline discussion this PR no longer
    implements it.
    
    A quick check of some power users' rollouts suggests that models don't
    actually generate home-prefixed absolute working directories for their
    spawns, so this shouldn't have any real blast radius.
  • [codex] make PathUri::from_abs_path infallible (#27976)
    ## Why
    
    `PathUri::from_abs_path` can fail for absolute paths that do not have a
    normal `file:` URI representation, forcing filesystem call sites to
    handle a conversion error even though the original path can be preserved
    losslessly.
    
    ## What
    
    Make `from_abs_path` infallible and migrate its callers. Unrepresentable
    paths use `file:///%00/bad/path/<base64>`, encoding Unix bytes or
    Windows UTF-16LE; `to_abs_path` validates and decodes that fallback. The
    leading encoded null reserves a namespace that cannot collide with a
    real Unix or Windows path, and fallback URIs remain opaque to lexical
    path operations.
    
    ## Validation
    
    Added path-URI coverage for Unix null and non-UTF-8 paths, Windows
    device/verbatim and non-Unicode paths, serialization, malformed
    fallbacks, opaque lexical operations, invalid native payloads, and
    literal `/bad/path` collision resistance.
  • [codex] migrate ExecutorFileSystem paths to PathUri (#27424)
    ## Why
    
    We're moving exec-server to use PathUri for its internal path
    representations.
    
    ## What
    
    Move `ExecutorFileSystem` APIs to use `PathUri` instead of
    `AbsolutePathBuf`. Future changes will convert higher-level parts of
    exec-server.
  • 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>
  • Make turn diff tracking operation backed (#21180)
    ## Summary
    - replace filesystem-based turn diff tracking with an operation-backed
    accumulator
    - preserve enough verified apply_patch state to render move-overwrite
    cases correctly
    - keep the turn/diff/updated contract intact while removing remote-only
    turn-diff test skips
    
    This takes the assumption that no 3P services rely on the output format
    of `apply_patch`
    
    ## Why
    For the CCA file system isolation push
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • Run exec-server fs operations through sandbox helper (#17294)
    ## Summary
    - run exec-server filesystem RPCs requiring sandboxing through a
    `codex-fs` arg0 helper over stdin/stdout
    - keep direct local filesystem execution for `DangerFullAccess` and
    external sandbox policies
    - remove the standalone exec-server binary path in favor of top-level
    arg0 dispatch/runtime paths
    - add sandbox escape regression coverage for local and remote filesystem
    paths
    
    ## Validation
    - `just fmt`
    - `git diff --check`
    - remote devbox: `cd codex-rs && bazel test --bes_backend=
    --bes_results_url= //codex-rs/exec-server:all` (6/6 passed)
    
    ---------
    
    Co-authored-by: Codex <noreply@openai.com>
  • [codex] Migrate apply_patch to executor filesystem (#17027)
    - Migrate apply-patch verification and application internals to use the
    async `ExecutorFileSystem` abstraction from `exec-server`.
    - Convert apply-patch `cwd` handling to `AbsolutePathBuf` through the
    verifier/parser/handler boundary.
    
    Doesn't change how the tool itself works.
  • 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.
  • chore(apply-patch) move invocation tests (#8111)
    ## Summary:
    This PR is a pure copy and paste of tests from lib.rs into
    invocation.rs, to colocate logic and tests.
    
    ## Testing
    - [x] Purely a test refactor
  • chore(apply-patch) move invocation parsing (#8110)
    lib.rs has grown quite large, and mixes two responsibilities:
    1. executing patch operations
    2. parsing apply_patch invocations via a shell command
    
    This PR splits out (2) into its own file, so we can work with it more
    easily. We are explicitly NOT moving tests in this PR, to ensure
    behavior stays the same and we can avoid losing coverage via merge
    conflicts. Tests are moved in a subsequent PR.
    
    ---
    [//]: # (BEGIN SAPLING FOOTER)
    Stack created with [Sapling](https://sapling-scm.com). Best reviewed
    with [ReviewStack](https://reviewstack.dev/openai/codex/pull/8110).
    * #8111
    * __->__ #8110