Repository navigation
Say which folder the agent works in, and warn when it's home - #804
Merged
Merged
Conversation
github-actions
Bot
added
commands
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Root detection is incorrect across platforms, and unsanitized paths can inject terminal controls.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Adds work-folder safety reporting to connector setup and diagnostics.
Changes:
- Reports the connector’s working folder.
- Warns for home and filesystem-root folders.
- Adds home, dedicated-folder, and symlink tests.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
internal/commands/connect.go |
Adds the check to connector setup. |
internal/commands/connect_doctor.go |
Adds the check to connector diagnostics. |
internal/commands/connect_workfolder.go |
Implements folder detection and warnings. |
internal/commands/connect_workfolder_test.go |
Tests home, dedicated, and symlinked folders. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Workers run in the folder the connector is started from and may change files there without asking. Nothing said so, and starting the connector from a terminal that happened to be in the home directory put the whole of it in reach. `connect setup` and `connect doctor` now report a "Work folder" check: the folder they run in, which is where the connector will work if it's started from there. The home directory, or the filesystem root, is a warning with a hint to start from a folder made for the agent or the project it should work on. Symlinks to home are seen through.
The work-folder check printed the raw path, and check messages are printed as they are, so a folder name carrying a newline or a terminal escape could forge lines or clear the screen. The path is now shown through SanitizeSingleLine; comparisons still use the raw path. A filesystem root was only recognised as "/", so on Windows a volume or UNC root passed, and "/" was called "your home folder". A root is now any path that is its own parent, and its warning names it as the root of the filesystem.
`cd /proc/self/root` keeps that path as the working directory, and it isn't its own parent until resolved, so the root went unwarned. The root check now resolves symlinks first. The tests set USERPROFILE as well as HOME, since os.UserHomeDir reads it on Windows, and the folder-name test skips on Windows, which forbids newlines and escapes in names.
The connector can't start in a folder it can't read: the dispatcher returns os.Getwd's error. The check only warned, and readiness accepts warnings, so setup could write ready:true and doctor pass for a shell whose folder had been removed. It is now a failure, with a hint to change to a folder that exists.
Cleaning before resolving symlinks turned a working directory like /proc/self/root/.. into /proc/self, which isn't where it leads, so the root went unwarned. The root check now resolves the path as given and cleans only when it can't. The tests now check the message names the folder, not just a phrase around it.
The home check compared resolved paths as text, so on a filesystem that ignores letter case, as a Mac's does by default, /Users/Alice and /users/alice were different folders and the home warning was missed. samePath now asks whether the two are the same file on disk.
If os.UserHomeDir failed, HOME unset on Unix for instance, the check had no home to compare with and passed every folder but the root, the home folder included. It now falls back to the account's own record, and if that says nothing either, it warns that it couldn't check rather than passing.
getcwd can hand back a path that no longer resolves, for instance when a parent folder lost its search permission after the shell went in. The check passed, but workers open their folder by that path and would fail to start. It now checks the path can be reached, and fails if not.
os.Stat on a folder succeeds without search permission on the folder itself, so a folder whose own permissions stopped anyone entering it still passed, while every worker, which enters it as cmd.Dir, would fail to start. The check now stats "folder/.", which only resolves through a folder that can be entered.
A $HOME that can't be looked at (a symlink through a folder the process can't search, say) was compared as text and never matched, so the home folder passed. A home that can't be looked at now counts as unknown, and the check warns that it couldn't tell. Appending "/." to a working directory already at the length limit made the probe fail with ENAMETOOLONG though the folder was fine; it now falls back to the path as given.
The "/." probe fell back to a plain stat at the length limit, and a plain stat doesn't need search permission on the folder itself, so a folder at the limit that couldn't be entered passed. The check now stats the path, which needs its parents searchable, and then ".", which needs the folder itself searchable, and never lengthens the path.
A working directory can reach / through more symlinks than EvalSymlinks can expand within the path-length limit, leaving only the lexical check, which then misses the root. The root check now asks first whether the folder is / itself, the same way home is recognised.
A HOME naming a file passed the reachability test, so the account's own record was never consulted and the real home folder passed. A home must now be a folder that can be looked at.
robzolkos
force-pushed
the
warn-home-workdir
branch
from
September 30, 2026 19:41
20119b9 to
321f72c
Compare
Copilot
AI
dismissed their stale review, a newer Copilot review was requested
September 30, 2026 19:41
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.
Your agent works in the folder the connector is started from (
os.Getwd(),internal/connector/dispatcher.go), and Claude Code runs there inacceptEditsmode, so it changes files without asking. Nothing said which folder that was. A connector started from a terminal that happened to be in~could change anything in the home directory.basecamp connect setupandbasecamp connect doctornow report a Work folder check:/, or a volume or UNC root on Windows, found as a path that is its own parent), with the hint "Start the connector from a folder made for the agent, or from the project it should work on." Home is recognised by identity on disk (os.SameFile), so symlinks and letter case (a Mac's filesystem ignores it) don't hide it. Roots are resolved as given, so/proc/self/rootand similar still count.richtext.SanitizeSingleLine, since a folder name can carry newlines or terminal escapes. Comparisons use the raw path.It's a warning, not a failure: some people will want the agent in a particular folder, and that's their call. Guided setup (#794) lists warning checks by name, so it shows there once both land.
Card: https://app.basecamp.com/2914079/buckets/48699913/card_tables/cards/10356301162
Summary by cubic
connect setupandconnect doctornow report the folder the agent works in, warning when it's the home directory or filesystem root.$HOMEor the account record may say where it is.Written for commit 321f72c. Summary will update on new commits.