Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughRepository creation now checks a parent path as entered before using its trimmed form. This applies to local, remote, and runtime repository creation. Floating workspace directory resolution and sanitization also check the raw path first. The renderer passes the original parent path to repository creation. Tests cover these flows and trailing-space paths in worktree parsing and matching. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The reviewed path handling is mergeable after normal checks; no actionable issue remains. Security Architecture ReviewSecurity architecture risk: ⚪ Minimal · up to The change preserves real directory names without removing the existing path-validation or directory-trust controls. No material security risk introduced or worsened by this change was identified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
887aea73-99ac-42c6-acd2-0ac1b15f00c3
📒 Files selected for processing (6)
src/main/ipc/floating-workspace-directory.test.tssrc/main/ipc/floating-workspace-directory.tssrc/main/ipc/repos-create.test.tssrc/main/ipc/repos/repo-creation-handlers.tssrc/main/runtime/worktree-selector-trailing-space-path.test.tssrc/renderer/src/components/sidebar/useCreateRepo.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
A trailing space is a legal POSIX directory name (Google Drive exports "foo/" from the cloud as "foo " locally), but the create-project and floating-workspace paths trimmed it off before validating: the picker's verified path was resolved as its trimmed spelling, so Orca created or looked for the wrong directory and `spawn git` failed with ENOENT. Resolve those paths raw-first: if the requested spelling exists, use it as-is; the trim only applies when it does not, which keeps the forgiving behavior for a typed stray space. The renderer now sends the picker's path untrimmed for the same reason. Windows is naturally unaffected (NTFS forbids trailing spaces, so the raw probe and the trimmed spelling coincide there). The cross-machine selector_not_found half of the issue is not addressed here: the worktree selector serialization chain was audited end to end (porcelain parsing, id minting/splitting, selector matching, profile keys) and preserves trailing spaces at every point, so no defect to fix was found on that path. Fixes stablyai#25386 Generated-by: GLM-5.3-Flash (ZCode)
… raw parent
The bare catch in resolveCreateParentPath treated every access failure
as "the raw spelling does not exist", so an EACCES/EPERM/ELOOP silently
fell back to the trimmed path and the project could be created in a
directory the user did not pick. Non-absence errors now propagate and
the handler surfaces them as its existing { error } shape; the fallback
only applies to genuine absence. Message-based detection keeps the
existing code-less ENOENT test mocks working.
Generated-by: GLM-5.3-Flash (ZCode)
|
Rebased onto current main (40d35fb): main's #24863 Phase 3 added a host-resolution guard to |
aeffb92 to
1076497
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5b7e272b-e633-453f-b89d-d0dd77433c35
📒 Files selected for processing (3)
src/main/ipc/floating-workspace-directory.test.tssrc/main/ipc/floating-workspace-directory.tssrc/renderer/src/components/sidebar/useCreateRepo.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
…eation useCreateRepo now sends the parent as typed, but the runtime registration controller and createRemoteRepo still trimmed it, so a valid parent whose name ends with a space resolved to a different directory. Both creation paths now probe the raw spelling first and fall back to the trim only when the raw path is genuinely absent (ENOENT); other probe failures surface instead of silently creating the project under the trimmed spelling. The SSH probe reuses isENOENT, which already covers relay errors that cross as canonical ENOENT messages, and re-expands ~ for both spellings. Also adds the SAFETY: rationale the changed-casting gate requires to an existing as-never in floating-workspace-directory.test.ts flagged by the same scan. Generated-by: GLM-5.3-Flash (ZCode)
This branch has not been deployed
1 participant
ELI5
A folder whose name ends with a space (Google Drive exports cloud folders containing
/that way on macOS) could not be added: Orca silently trimmed the space off the path, looked for the wrong folder, and failed withspawn git ENOENT. Now Orca first tries the path exactly as given — if that directory exists, it is used as-is; the trim only kicks in when it does not, which keeps the forgiving behavior when a stray space was just a typing slip.What Changed
repos:create(repo-creation-handlers.ts) resolves the parent path raw-first: if the untrimmed spelling exists on disk it wins, otherwise the trimmed spelling is used (typed-typo tolerance, with a test locking it in).useCreateRepo.ts) sends the picker's path untrimmed so the main process can make that raw-first decision at all — previously it pre-trimmed and the real spelling never reached the handler.floating-workspace-directory.ts) had the same disease in three spots: the trusted-directory list was trimmed while reading (breaking trust matching for real spaced paths), the terminal cwd resolver stat'ed the trimmed spelling (silently falling back to the default workspace whenfoowas real), and the setting sanitizer rejected real directories. All three now probe raw-first.mkdirreceiving the trimmed spelling) plus a typed-typo case pinning the fallback, and a round-trip characterization test over the worktree selector chain.Why
Trim-on-sight conflates two different things: forgiving a stray space somebody typed, and rewriting a path the file system just handed us. A picker/drag path is already verified to exist, so rewriting it can only lose information. Raw-first is the smallest change that keeps both behaviors; the fallback direction preserves the old UX for genuine typos. Windows is naturally unaffected (NTFS forbids trailing spaces, so raw and trimmed spellings coincide there) — no platform branch needed.
Linked Issue
Fixes #25386
One honest scope note: the issue also reports
selector_not_foundwhen opening the same worktree from a second machine. We audited that whole chain — porcelain parsing, worktree id minting and splitting, selector matching, profile keys — and every point preserves trailing spaces; no defect to fix was found there, so this PR does not touch it. If the second machine registered the repo through a path that hit one of the trimmed spots fixed here, the registration fork would explain the symptom. We'd love to know how the repo was registered on machine two (picker vs. synced settings vs. typed) to chase the rest.Visual Proof
N/A — no UI change; the before/after is in the git subprocess result (
spawn git ENOENTvs. success), pinned by tests.Testing
Commands actually run locally (macOS): new cases were red before the fix (floating fell back to the default workspace;
mkdirreceived the space-less spelling) and green after — 6 files / 106 tests viapnpm test; adjacent worktree selector/porcelain suites (26 files / 324 tests) and rendererAddRepoCreateStep/app/repos-remotesuites all green;pnpm tcandpnpm run typecheck:nodeclean;oxlintandoxfmt --checkclean on all six touched files. Real Google Drive and a second machine were not available locally — the spaced-directory behavior is pinned at the handler level with real temp directories.AI Disclosure
Investigation, coding and verification by GLM-5.3-Flash (ZCode agent); orchestrated, reviewed and committed by a main agent with the same model, human overseeing.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Pure path-handling change: no wire/compat impact, no spawn-argument changes (the paths were always passed correctly; they were resolved to the wrong spelling before), SSH remote-registration chain audited and untouched (its parent input is typed, where trim is the intended cleanup). Windows behavior is a no-op by NTFS semantics.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)