Skip to content

Add path-scoped review filters to codex exec review#2

Open
0xble wants to merge 1 commit intomainfrom
review-pathscope-upstream
Open

Add path-scoped review filters to codex exec review#2
0xble wants to merge 1 commit intomainfrom
review-pathscope-upstream

Conversation

@0xble
Copy link
Owner

@0xble 0xble commented Mar 11, 2026

Summary

This adds path-scoped review filtering to codex exec review and the app-server review APIs.

Related upstream issue:

New behavior / usage examples:

  • codex exec review --paths src/lib.rs src/main.rs --uncommitted
  • codex exec review --paths src/lib.rs --paths src/main.rs --uncommitted
  • codex exec review --pathspec-from-file review-paths.txt --uncommitted
  • codex exec review --paths src/lib.rs src/main.rs -- "focus on reviewer ergonomics"

Motivation

Users often want review on a narrow part of the repo rather than the full diff. The goal here is to make that scope explicit and reliable without changing the overall review flow.

The important correctness requirements are:

  • paths must be interpreted relative to the repository root
  • path filtering must remain correct when review starts from a nested cwd
  • Git path filters must be rendered literally rather than accidentally invoking Git pathspec magic

Implementation

  • Adds pathspecs to the review request protocol/app-server transport.
  • Adds listable --paths and --pathspec-from-file to codex exec review.
  • Centralizes pathscope handling in core review prompt construction:
    • trims and validates path inputs
    • normalizes ./... paths
    • resolves nested-cwd inputs back to repo-root-relative paths
    • rejects paths that escape the repo root
    • renders safe Git filters such as :(top,literal)... and :(top,glob)**
    • generates scoped user-facing hints like commit 1234567: ... in src/lib.rs and src/main.rs
  • App-server and exec layers only transport the raw path list; core owns the actual scoping semantics.

Why this approach

This is mostly a prompt-layer scoping feature with thin transport plumbing.

That keeps the implementation simple:

  • CLI parses and forwards the paths
  • protocol/app-server carry them
  • core is the single place that decides how those paths should be interpreted and rendered for review

This avoids scattering path normalization and Git-pathspec behavior across multiple layers.

Testing

Added or updated coverage for:

  • repo-root scoping from nested cwd
  • repo-root ./ handling
  • blank path rejection
  • outside-repo path rejection
  • exec request building keeps pathspecs as transport input
  • pathspec file loading and empty-file rejection
  • CLI parsing for:
    • --paths a b
    • repeated --paths
    • positional prompt after --
  • app-server review start includes scoped paths in entered-review-mode state
  • app-server rejects empty pathspecs

QA run:

  • cargo test -p codex-core review_prompts::tests -- --nocapture
  • cargo test -p codex-exec review_accepts_ -- --nocapture
  • cargo test -p codex-exec builds_review_request_from_pathspec_file -- --nocapture
  • cargo test -p codex-exec rejects_empty_pathspec_file -- --nocapture
  • cargo test -p codex-app-server review_start_includes_paths_in_entered_review_mode -- --nocapture
  • cargo test -p codex-app-server review_start_rejects_empty_pathspec -- --nocapture

Notes for reviewers

This PR intentionally keeps pathscope semantics centralized in codex_core::review_prompts rather than spreading them across exec/app-server layers.

That makes the behavior easier to reason about and fixes a class of issues around:

  • wrong repo-root scoping
  • wrong relative path rendering from subdirectories
  • literal path filters being treated like normal Git pathspecs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant