Skip to content

fix(server): restore Windows file-manager actions - #12393

Open
MatthewFeroz wants to merge 2 commits into
pingdotgg:mainfrom
MatthewFeroz:t3code/fix-windows-markdown-context-menu
Open

MatthewFeroz wants to merge 2 commits into
pingdotgg:mainfrom
MatthewFeroz:t3code/fix-windows-markdown-context-menu

Conversation

@MatthewFeroz

@MatthewFeroz MatthewFeroz commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

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: false and retain the existing unref behavior. 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.

Before: Reveal in File Explorer in the failing context menu

After: Explorer opens the intended diagnose directory and selects SKILL.md (Windows hides the extension in this view).

After: the correct directory with SKILL.md selected

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.
  • Targeted lint and format checks pass; vp run --filter t3 typecheck passes.
  • The change applies to native Windows server launch behavior. macOS/Linux/WSL options, provider adapters, wire contracts, and client action visibility are unchanged. It adds no persistent state or reverse operation. Existing user instructions remain accurate, so no documentation change is needed.
  • Reproduction state and evidence are outside tracked source; the live T3 database was not modified.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for the user-visible behavior
  • I included a video for the interaction failure

Implemented with GPT-6 Astra via the Codex harness in T3 Code.

October 4 branch update

Merged upstream main at eac52f0087d9ba5dee5542f24788d1482affae43 into this branch without rewriting its history. Updated head: f05ea994ef8d6343907064c900ed11aee843e277.

  • Focused regression run: Tests 41 passed | 2 skipped (43).
  • Scoped typechecks passed for t3. Targeted lint passed; formatting and whitespace checks passed.
  • The merged source tree matches the locally validated tree. Existing screenshots and recordings document the original affected flow; this branch refresh was checked by command line. Platform-specific skips are preserved, and no new native UI verification is claimed.

Branch update validated with GPT-6.1 Sol through the Codex harness in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 18, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 375b839

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.

@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cf9a0f72-2fbc-4fca-89ca-7b4b4a272a5a
📥 Commits

Reviewing files that changed from the base of the PR and between 375b839 and f05ea99.

📒 Files selected for processing (2)
  • apps/server/src/process/externalLauncher.test.ts
  • apps/server/src/process/externalLauncher.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Windows 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 /select argument.

Changes

Windows file-manager launch handling

Layer / File(s) Summary
Launch path and detachment behavior
apps/server/src/process/externalLauncher.ts
EditorLaunch accepts an optional detached flag. Windows file-manager paths use backslashes. Explorer reveal launches set detached: false; other launches remain detached by default.
Windows launch validation
apps/server/src/process/externalLauncher.test.ts
Tests verify the normalized path for a plain launch and run the reveal helper with a recorder to verify its /select argument.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to f05ea

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 Review

Security architecture risk: 🔵 Low · up to f05ea

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed execution affects the Windows host running the server and targets reachable by its launched file manager. Existing acceptance of raw Windows paths already allowed caller-selected local and network-share targets; normalization alone does not establish expanded maximum authority. Actual remote-share credential behavior was not verified.

Trust Boundaries and Controls

  • observed — WebSocket upgrade authentication supplies session scopes to RPC authorization. shellOpenInEditor requires AuthOrchestrationOperateScope, and the authorization layer rejects calls without that scope before executing the handler.
  • observed — The reveal helper preserves apostrophe escaping in PowerShell string literals and UTF-16LE/base64 command transport. The executable-resolution path uses shell:false for PowerShell's .exe launch. These controls are unchanged by the detachment override.

Resilience and Maintainability Implications

  • observed — The launcher retains ignored stdio and scoped spawn followed by unref, without retaining a completion or recovery registry. Source establishes that sequence, but not the concrete spawner's behavior during interruption, concurrent launches, or shutdown of a non-detached child. No resulting security regression was demonstrated.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR addresses #11780. On Windows, reveal launches set detached: false and retain the existing unref behavior. The regression test runs the PowerShell helper and verifies Explorer receives the `…
Out of Scope Changes check ✅ Passed The changes stay within #11780. The launcher changes and regression tests support Windows file reveal and file-manager path handling. No unrelated changes are shown.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Title check ✅ Passed The title clearly and concisely describes the main change: restoring Windows file-manager actions.
Description check ✅ Passed The description explains the problem, change, issue link, scope, verification, and user-visible behavior. It omits the template’s exact headings and does not provide explicit maintainer approval or ex…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Reveal in File Explorer does not reveal chat-linked files on Windows (desktop 0.0.40)

1 participant