fix(permission): worktree-relative paths fall back to directory in non-git projects#26694
Open
BennD wants to merge 1 commit into
Open
fix(permission): worktree-relative paths fall back to directory in non-git projects#26694BennD wants to merge 1 commit into
BennD wants to merge 1 commit into
Conversation
Contributor
|
The following comment was made by an LLM, it may be inaccurate: Potential Duplicate FoundPR #18761: "fix(opencode): Use standard resolve function to get proper filePaths for tools" Why they're related:This is the original PR that addresses the same issue. According to the current PR description:
The current PR essentially supersedes or duplicates the work of #18761 with refinements, so these two PRs should be reviewed together to determine which approach should be kept. |
…n-git projects Project.fromDirectory uses worktree: "/" as a sentinel for non-git projects. Tools compute path.relative(instance.worktree, file) for permission patterns, titles, and summary lines, which in a non-git project yields "home/u/proj/file.ts" instead of "file.ts" — silently breaks rules like "src/*" and produces ugly titles. Add workspaceRoot(ctx) next to containsPath in instance-context.ts (both invert the same "/" sentinel) and route the four file tools and agent.ts through it. Closes anomalyco#24694
81cc9ec to
faa501b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #24694
Type of change
What does this PR do?
Project.fromDirectorysetsworktree: "/"as a sentinel for non-git projects (packages/opencode/src/project/project.ts:210). Tools then dopath.relative(instance.worktree, file)to build worktree-relative permission patterns / titles / summary lines. In a non-git project that produces"home/u/proj/src/foo.ts"(the absolute path with the leading/stripped) instead of"src/foo.ts", so a config rule like"src/*"silently never matches and titles read as the absolute path.This PR adds
workspaceRoot(ctx)next tocontainsPathinsrc/project/instance-context.ts. It returnsctx.directorywhenctx.worktree === "/", otherwisectx.worktree— i.e. the same sentinelcontainsPathalready inverts. All call sites that compute worktree-relative strings switch topath.relative(workspaceRoot(instance), …):tool/read.ts(permission pattern + title)tool/edit.ts(two permission asks + title)tool/write.ts(permission ask + title)tool/apply_patch.ts(permission ask + per-filerelativePath+ summary lines + LSP diagnostic header)tool/lsp.ts(title display)reference/reference.tsreferencePath(same inline ternary; helper signature widened toPick<InstanceContext, "directory" | "worktree">so this caller's plain{directory, worktree, value}shape can use it)Relationship to #18761
#18761 proposes the same fix and the author has been keeping it actively merged with dev (latest ~5 days ago). It's currently conflicting after #26583 landed; #26583 covers the absolute→relative shift for
read.tsin git projects, but the non-git fallback for the four file tools and the title/summary/relativePath call sites still needs to land.This PR re-does the same idea on a clean baseline with a few differences:
src/project/instance-context.tsnext tocontainsPath(both invert the sameworktree === "/"sentinel) instead ofsrc/tool/relative.ts.workspaceRootto avoid shadowingpath.relativeat every call site.Instance.providewith newerprovideTmpdirInstance/it.live.How did you verify your code works?
bun typecheckfrompackages/opencode— clean.bun test test/tool/{read,edit,write,apply_patch,lsp}.test.ts— 117/117 pass (one new non-git test per tool).bun test test/permission test/tool— 369/369 pass.readasserts on the captured permission pattern (the file's existing idiom for permission tests),edit/write/lspassert onresult.title,apply_patchasserts onresult.outputandmetadata.files[0].relativePath(the same shape "applies add/update/delete in one patch" already uses). All sites compute their relative path through the new helper, so an assertion on any one of them exercises the fix.--singlebinary in a non-git tmpdir;readandeditrules undersrc/*now deny as configured (silently passed through before).Behavior change for users
For non-git projects:
"read": { "src/*": "deny" }) now actually match — they were silently passing through to the wildcard before.src/foo.ts) instead ofhome/u/proj/src/foo.ts.For git projects: no change —
worktreewas already a meaningful root.Checklist