Skip to content

fix(windows): strip quotes from repaired PATH - #209

Merged
rynfar merged 2 commits into
pylonfrom
upstream/2026-08-31-windows-path-quotes
Aug 31, 2026
Merged

fix(windows): strip quotes from repaired PATH#209
rynfar merged 2 commits into
pylonfrom
upstream/2026-08-31-windows-path-quotes

Conversation

@rynfar

@rynfar rynfar commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

On Windows a PATH repaired from the registry can contain quoted segments. Pylon
kept the quotes when splitting and rejoining, so the resulting entries did not
resolve and provider CLIs looked missing on machines whose PATH happened to be
quoted.

Quotes are now stripped from each segment as PATH is normalized, in both the
shared helper and the desktop shell environment.

Adopted from upstream pingdotgg/t3code#8746 (12fe2d6d0), cherry-picked clean.
packages/shared/src/shell.ts was identical to the upstream base; Pylon's
divergence in DesktopShellEnvironment.ts is in unrelated hunks.

Verified: vp test run --dir packages/shared packages/shared/src/shell.test.ts
(30 passed) and vp test run --dir apps/desktop apps/desktop/src/shell/DesktopShellEnvironment.test.ts
(15 passed).

Not verified on a Windows host — this machine is macOS. The change is exercised by
the unit tests above, which cover the quoted-segment cases.

Reviewed and integrated with Claude Opus 5 in Claude Code.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

(cherry picked from commit 12fe2d6d03062c5d7fc3beff168f8c4af1235b50)
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S labels Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.5 KiB −10 B (−0.1%) 15.1 KiB
Codex Thread snapshot wire 6.9 KiB 6.9 KiB +7 B (+0.1%) 7.3 KiB
Codex Live turn WebSocket wire 6.6 KiB 6.6 KiB −17 B (−0.3%) 7.8 KiB
Codex Live turn WebSocket decoded 57.2 KiB 57.2 KiB 0 B (0.0%) 66.4 KiB
Codex Live turn messages 10 10 0 (0.0%) 21
Claude Total thread wire 13.6 KiB 13.5 KiB −45 B (−0.3%) 15.1 KiB
Claude Thread snapshot wire 6.9 KiB 6.9 KiB −3 B (−0.0%) 7.3 KiB
Claude Live turn WebSocket wire 6.6 KiB 6.6 KiB −42 B (−0.6%) 7.8 KiB
Claude Live turn WebSocket decoded 58.1 KiB 58.0 KiB −44 B (−0.1%) 66.4 KiB
Claude Live turn messages 10 9 −1 (−10.0%) 21

Baseline: 9ade80d · PR result: effbf20 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 109.5 KiB
  • Claude decoded thread snapshot: 110.2 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Review follow-ups on the #8746 port. Stripping quotes from each segment after
splitting on the raw delimiter destroyed entries that legitimately contain one:
PATH=C:\bin;"C:\my;dir";C:\other previously survived byte-for-byte and now
became four entries, dropping the real directory and adding a relative "dir"
resolved against the child process's cwd. The split honours quoting, and an
entry containing the delimiter is re-quoted on the way out so consumers do not
split it apart again. Unbalanced quotes fall back to a plain split, since those
are stray characters rather than quoting.

Stripping also left degenerate fragments: C:" became C:, a drive-relative entry
that made spawned providers search the child's own directory. Those are dropped.

Both copies of the helper stay in sync.
@rynfar

rynfar commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator Author

Adversarial review confirmed the core change is correct and that dedupe behaviour is unchanged (pathComparisonKey already stripped wrapping quotes), replaceAll is fine under target: ESNext, no caller depends on quotes surviving the merge, and both copies of the helper were kept in sync. Two edge-case findings, both fixed.

  1. Quoted PATH entries containing the delimiter were destroyed. PATH=C:\bin;"C:\my;dir";C:\other previously survived the split/rejoin byte-for-byte, because the fragments kept their quotes. Stripping quotes after splitting on the raw delimiter turned it into four entries — dropping the real directory and injecting a relative dir resolved against the child process's cwd. The split now honours quoting, and an entry containing the delimiter is re-quoted on the way out, without which the joined value would just be re-split by consumers. Unbalanced quotes fall back to a plain split, since those are stray characters rather than quoting — that keeps the existing C:" case working instead of swallowing every later entry.

  2. Degenerate fragments were emitted rather than dropped. C:" became C:, a drive-relative entry, so spawned providers searched the child's own directory — somewhere that previously never matched, since C:" was not a valid directory name. Bare drive letters are now dropped, and both the shared and desktop test expectations updated to match.

Re-verified: shell.test.ts 31 passed (including a new case for the quoted delimiter), DesktopShellEnvironment.test.ts 15 passed, @t3tools/shared and @t3tools/desktop typecheck clean.

Still not verified on a Windows host — this machine is macOS. The unit tests cover both cases.

@github-actions github-actions Bot added size:L and removed size:S labels Aug 31, 2026
@rynfar
rynfar merged commit 10fa1a1 into pylon Aug 31, 2026
19 checks passed
@rynfar
rynfar deleted the upstream/2026-08-31-windows-path-quotes branch August 31, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 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.

2 participants