Repository navigation
fix(server): restore Windows file-manager actions - #12393
MatthewFeroz wants to merge 2 commits into
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, well-contained Windows file-manager bug fix with explicit platform scoping and regression coverage. Existing non-Windows and non-file-manager launch behavior remains unchanged, and no product defaults or static-analysis policies are modified. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughWindows file-manager launches now normalize paths. The launcher supports per-launch process detachment, and Windows reveal launches run without detachment. Tests verify plain-launch arguments and the reveal helper’s ChangesWindows file-manager launch handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~12 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change restores Reveal in File Explorer for Windows chat file links by normalizing paths and keeping the reveal helper attached. No concrete merge-blocking risk was identified; the author did not test native Windows menu selection, which is normal uncertainty for a change like this. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change preserves authentication, launch permissions, and command construction. No security regression was demonstrated, but cancellation and shutdown behavior of the attached Windows helper remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
What Changed
On Windows, selecting Reveal in File Explorer from a chat file link closes the menu but does not reveal the file. The PowerShell helper can exit with code 0 without executing its command when launched detached with ignored stdio.
Start that Windows helper with
detached: falseand retain the existingunrefbehavior. Normalize Windows paths for the ordinary File Explorer fallback as well, so forward-slash paths are not misinterpreted by Explorer.Why
Fixes #11780. The helper introduced in #7140 retained detached spawning after #9551 fixed reveal path separators. #12281 refactors frontend actions but does not fix this server launch behavior.
The new regression test runs the real Windows PowerShell process with the launcher's options, substituting an argument recorder for Explorer. It failed before the fix because no output was produced despite exit code 0, and passes afterward. It waits for process completion without sleeps or polling.
UI Changes
Before: selecting Reveal closes the menu without revealing the file. Short reproduction video.
After: Explorer opens the intended
diagnosedirectory and selectsSKILL.md(Windows hides the extension in this view).The after image comes from an isolated Windows Electron Dev instance using the real Markdown component, server request, and patched launcher. The native menu was observed, but menu selection was supplied by a renderer test harness because native pointer automation could not obtain window geometry. A fully automated native click pass and Obsidian's handling of the ordinary Open action have not been verified.
Verification
vp test run apps/server/src/process/externalLauncher.test.ts: 11 passed, 15 platform-specific skips on Windows. Includes real-process coverage with spaces and an apostrophe, plus ordinary Windows path normalization.vp run --filter t3 typecheckpasses.Checklist
Implemented with GPT-6 Astra via the Codex harness in T3 Code.
October 4 branch update
Merged upstream
mainateac52f0087d9ba5dee5542f24788d1482affae43into this branch without rewriting its history. Updated head:f05ea994ef8d6343907064c900ed11aee843e277.t3. Targeted lint passed; formatting and whitespace checks passed.Branch update validated with GPT-6.1 Sol through the Codex harness in T3 Code.