Skip to content

fix: treat current search root as local - #232

Merged
ualtinok merged 1 commit into
cortexkit:mainfrom
llc1123:fix/aft-search-current-root
Aug 17, 2026
Merged

fix: treat current search root as local#232
ualtinok merged 1 commit into
cortexkit:mainfrom
llc1123:fix/aft-search-current-root

Conversation

@llc1123

@llc1123 llc1123 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • short-circuit canonical current-root aliases before external Git-root probing
  • return the configured root's original path form after the normalized comparison
  • cover non-Git absolute-root, dot-root, distinct non-Git, and restricted-root behavior

Closes #227

Verification

  • red phase on the parent commit: the restricted same-root case failed with not_a_git_root
  • focused restriction regression: 1 passed
  • full aft_search_contract_test module: 42 passed
  • cargo +1.93.0 check -p agent-file-tools --tests
  • cargo +1.93.0 fmt --all -- --check
  • live NDJSON tool_call probe with restrict_to_project_root: true: path: "." found the non-Git workspace symbol, while a distinct Git root returned path_outside_root

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Treats search requests that alias the configured project root (including "." and symlinks) as local, skipping external Git-root probing. Previously these aliases could be treated as external, returning not_a_git_root or path_outside_root in non-Git workspaces or with restrict_to_project_root enabled.

  • Normalize the requested path and the configured root once before comparison; when equal, return the configured root’s original path form.
  • Normalize upfront so Git probing uses a non-verbatim path on Windows; remove the now-redundant later normalization.
  • Tests: non-Git same-root and "." return byte-identical responses to default search; with restrict_to_project_root=true, same-root is allowed and external roots are rejected; external non-Git roots still return not_a_git_root.

Written for commit 843f79e. Summary will update on new commits.

Review in cubic

Greptile Summary

This PR treats canonical aliases of the configured search root as local, avoiding unnecessary Git-root probing while preserving the configured root’s original path form.

  • Normalizes the requested path before comparing it with the configured project root.
  • Returns the configured root directly when both paths identify the same location.
  • Adds coverage for non-Git roots, dot aliases, root restrictions, and distinct external paths.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
crates/aft/src/readonly_artifacts.rs Normalizes the nearest existing requested path and short-circuits when it identifies the configured project root.
crates/aft/tests/integration/aft_search_contract_test.rs Adds regression coverage for same-root aliases in non-Git and root-restricted search configurations.

Reviews (2): Last reviewed commit: "fix: treat current search root as local" | Re-trigger Greptile

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@llc1123
llc1123 force-pushed the fix/aft-search-current-root branch from 747be8c to 843f79e Compare August 17, 2026 10:02

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found and verified against the latest diff

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/aft/src/readonly_artifacts.rs">

<violation number="1" location="crates/aft/src/readonly_artifacts.rs:77">
P2: When the configured project_root is a subdirectory of a larger git repository, the new short-circuit returns project_root for a same-root request and skips the git probe, but any sub-path inside project_root still resolves through git_toplevel and returns the enclosing repo toplevel. The same function therefore returns different roots for the workspace root versus paths under it, which were consistent before this change. Consider limiting the local short-circuit to non-Git roots, or applying the same local handling to sub-paths, so the resolved root is consistent within the configured workspace.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

// symlinks remain local without requiring the workspace itself to be Git.
let existing = crate::inspect::job::canonicalize_normalized(&existing);
if existing == crate::inspect::job::canonicalize_normalized(project_root) {
return Ok(project_root.to_path_buf());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When the configured project_root is a subdirectory of a larger git repository, the new short-circuit returns project_root for a same-root request and skips the git probe, but any sub-path inside project_root still resolves through git_toplevel and returns the enclosing repo toplevel. The same function therefore returns different roots for the workspace root versus paths under it, which were consistent before this change. Consider limiting the local short-circuit to non-Git roots, or applying the same local handling to sub-paths, so the resolved root is consistent within the configured workspace.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/aft/src/readonly_artifacts.rs, line 77:

<comment>When the configured project_root is a subdirectory of a larger git repository, the new short-circuit returns project_root for a same-root request and skips the git probe, but any sub-path inside project_root still resolves through git_toplevel and returns the enclosing repo toplevel. The same function therefore returns different roots for the workspace root versus paths under it, which were consistent before this change. Consider limiting the local short-circuit to non-Git roots, or applying the same local handling to sub-paths, so the resolved root is consistent within the configured workspace.</comment>

<file context>
@@ -67,18 +67,21 @@ pub(crate) fn resolve_git_root_from_user_path(
+    // symlinks remain local without requiring the workspace itself to be Git.
+    let existing = crate::inspect::job::canonicalize_normalized(&existing);
+    if existing == crate::inspect::job::canonicalize_normalized(project_root) {
+        return Ok(project_root.to_path_buf());
+    }
+
</file context>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for checking this. The differing behavior is intentional under the contract accepted in #227. aft_search.path selects another project root; it is explicitly not a subtree filter. Only a canonical path exactly equal to the configured root is an alias for omitting path, so it must stay local before Git probing. Any distinct path keeps the pre-existing git rev-parse --show-toplevel routing.

Applying the local short-circuit to descendants would also swallow paths that select a nested child Git repository, violating the accepted requirement that child/external Git roots preserve their current routing. Limiting the short-circuit to non-Git configured roots would conversely break the same-root alias contract when the configured workspace is a subdirectory of a larger repository. The restriction regression in this PR intentionally locks the boundary: the configured root is allowed, while a distinct Git root remains refused. No code change is needed here.

@ualtinok
ualtinok merged commit edbd696 into cortexkit:main Aug 17, 2026
22 of 23 checks passed
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.

aft_search rejects redundant current-root paths for non-Git workspaces

2 participants