Fix packaged terminal helper resolution - #22
Merged
Conversation
sambitcreate
marked this pull request as ready for review
August 11, 2026 19:48
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
resolveNodePtyDiskPackageDirregex ASAR normalization — newterminal-spawn-helper.tsuses(/([/\\])app\.asar([/\\])/u, "$1app.asar.unpacked$2")to normalize package-directory paths before any filesystem operation, mirroring node-pty's ownhelperPath.replace('app.asar', 'app.asar.unpacked')but requiring path-separator delimiters on both sides, which prevents double-.unpackedrewriting that node-pty's naive.replace()would produce.resolveNodePtySpawnHelperPathsseam — areadDirectoryinjectable lets regression tests exercise the exact production resolver including normalization and discovery without requiring an Electron ASAR mount; normalization runs beforereaddirso neither discovery norchmodever targets a virtual ASAR path.defaultSpawnHelperPathsdelegation — the production resolver now delegates toresolveNodePtySpawnHelperPaths, replacing the old inlined logic that enumerated the virtual ASAR prebuilds directory and appended duplicate unpacked entries.- Terminal service test coverage expansion —
fakePtyandownerStatenow record writes, resizes, data/exit events, and IPC sends; 7 new tests cover output buffering, history append+flush, IPC notification channels, teardown scoping by workspace and web-contents, per-document session caps, and history-flush deduplication. - Package verification hardening —
verify-macos-package.mjsgainsverifyPackagedNodePtyResourcesthat asserts bothdarwin-arm64anddarwin-x64spawn-helperentries are ASAR-unpacked regular files with mode0755on the unpacked filesystem copy; wired intoverifyMacPackage. - E2E workspace terminal spec — new Playwright test seeds a folder workspace, opens the terminal drawer, types
echo $((314159+271828)); pwd, polls the persisted history file for585987and the workspace directory, and closes the drawer. pretestintegration —test:terminal:coverageis now part ofpretestwith coverage floors of 100/100/100 on the resolver and 95/80/90 on the terminal service.
DeepSeek Pro | 𝕏
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.

What changed
app.asarandnode_modules.asarlocations to their real unpacked directories before any filesystem discovery or permission repair.0755.Root cause
The packaged app correctly shipped node-pty under
app.asar.unpacked, and the release hook already madespawn-helperexecutable. Electron ASAR-aware filesystem calls also exposed a virtualapp.asarplaceholder as a regular non-executable file. Aiden enumerated that virtual location and attempted tochmodit, which failed withENOTDIRbefore node-pty could use the valid executable underapp.asar.unpacked.Why tests missed it
Four independent blind spots lined up:
spawnHelperPathsorprepareSpawnHelper, bypassing the production package-directory resolver.The new tests close all four gaps. The bug-critical resolver is CI-gated at 100/100/100 coverage. The broader terminal service is currently 95.46% lines, 81.20% branches, and 94.74% functions, with CI floors of 95/80/90 to prevent regression. Behavior that only exists in a packaged layout is additionally enforced by package verification rather than relying on code coverage alone.
User impact
Opening the integrated terminal in the installed macOS app no longer fails with the
terminal:create/spawn-helperENOTDIRerror.Validation
npm run test:terminal:coverage— resolver 100% line/branch/function coverage; terminal service floors passednpm test— 1,295/1,295 application tests, 33/33 native worktree-remover tests, and 41/41 Rust broker testsnpm run test:e2e— 7/7 Electron E2Es, including the real workspace terminal flownpm run type-checkandnpm run type-check:e2egit diff --checknpm run package— fresh signed development package built and after-sign verification passednpm run package:verify— hardened package and both unpacked executable node-pty helpers verified