Skip to content

Conversation

dylan-hurd-oai
Copy link
Collaborator

Summary

When resolving our current directory as a project, we want to be a little bit more clever:

  1. If we're in a sub-directory of a git repo, resolve our project against the root of the git repo
  2. If we're in a git worktree, resolve the project against the root of the git repo

Testing

  • Added unit tests
  • Confirmed locally with a git worktree (the one i was using for this feature)

@dylan-hurd-oai dylan-hurd-oai added the codex-rust-review Perform a detailed review of Rust changes. label Aug 22, 2025
@github-actions github-actions bot added codex-rust-review-in-progress and removed codex-rust-review Perform a detailed review of Rust changes. labels Aug 22, 2025
Copy link

Summary
Adds worktree-aware trust resolution: core now treats a linked git worktree as trusted if the main repo is trusted, and TUI trusts the root project when setting trust. Includes unit tests for the resolver.

Review
Nice, cohesive change that aligns core logic and TUI UX around worktrees. A couple small idiomatic tweaks could tighten things up.

  • codex-rs/core/src/config.rs: In the trust check, avoid allocations by using map_or(false, |p| p.trust_level.as_deref() == Some("trusted")) instead of clone().unwrap_or("".to_string()) == "trusted".
  • codex-rs/core/src/git_info.rs: The .git parser looks good; consider handling multi-line .git content by scanning for the first line starting with gitdir: to be extra robust.
  • codex-rs/tui/src/onboarding/trust_directory.rs: Great UX improvement using the resolved root; the error message uses inline variables in format!, consistent with project style.

If not already in the PR body, please add a short motivation: “Trust git worktrees based on the main repo’s trust, so users don’t have to trust each worktree individually.”

Follow-up (outside this PR scope)

  • Consider an enum for trust level instead of string comparisons for better type safety.

View workflow run

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex Review: Here are some suggestions.

Reply with @codex fix comments to fix any unresolved comments.

About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".

/// - If `cwd` is inside a linked worktree (where the repo root contains a
/// `.git` file that points at `<main>/.git/worktrees/<name>`), returns the
/// path to the main repository working directory (parent of `<main>/.git`).
pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option<PathBuf> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just shell out to git for this.

git rev-parse --git-common-dir

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, nice

@dylan-hurd-oai
Copy link
Collaborator Author

@nornagon-openai updated 👍

Copy link
Collaborator

@nornagon-openai nornagon-openai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with nits

@dylan-hurd-oai dylan-hurd-oai merged commit 6f0b499 into main Aug 22, 2025
15 checks passed
@dylan-hurd-oai dylan-hurd-oai deleted the dh--detect-worktrees branch August 22, 2025 20:54
@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants