fix(open-knowledge): node-pty spawn-helper +x in dev/CI build#383
Merged
Conversation
* fix: make node-pty spawn-helper executable in dev/CI build+install
node-pty ships its prebuilt spawn-helper non-executable (mode 0644,
node-pty#850) and bun install preserves that mode. The packaged .app
re-enforces +x in scripts/afterPack.mjs, but the dev/CI build path
(bun install + bun run build:desktop, electron-vite, no afterPack) had no
equivalent step, so node-pty.spawn() threw "posix_spawnp failed" and the
in-app terminal dock showed "exited" in the lume macOS VM and the CI
desktop-smoke job.
Enforce the executable bit at the install producer. Refactor
ensure-node-pty-exec.mjs to share one chmod primitive between the packaged
layout (afterPack, unchanged signature and error contract) and the dev
layout (node_modules/node-pty/prebuilds), then call the dev wrapper first
and unconditionally at the top of the desktop postinstall, before the
ELECTRON_SKIP_REBUILD and CI early-exits, since CI desktop-smoke is one of
the affected environments and the executable bit is independent of the
native-rebuild those guards gate. A failure is softened to a warning so a
pathological node-pty layout cannot gate the monorepo install.
Adds a RED test: tests/unit/ensure-node-pty-exec.test.ts gains a dev-path
suite (node_modules layout 0644 to 0755, multi-arch, absent shipped-arch
throws) mirroring the existing afterPack coverage.
Verified in a worktree off origin/main: bug test 3-fail to 6-pass, full
desktop suite 2064 pass 0 fail (post-build), tsc exit 0, biome and oxlint
exit 0, plus a live postinstall smoke that flips a forced 0644 helper back
to 0755 ahead of the skip. Does not modify CI workflows or un-quarantine
the node-pty terminal-dock smoke suite.
* refactor(open-knowledge): tidy postinstall spawn-helper fixup per review
- Strip D33/D34 decision-marker citations from postinstall.mjs comments (OK
comment-discipline prohibits D-num markers in source; substance kept).
- Extract the postinstall soft-fail into a pure, tested
ensureNodePtySpawnHelperExecutableInNodeModulesSafe wrapper returning
{ ok, ... } instead of throwing, so a pathological node-pty layout can never
gate the monorepo bun install. Pin the non-throw contract with two unit tests.
No behavior change: spawn-helper is still chmod'd +x on install; failures still
soften to a warning.
GitOrigin-RevId: 238e487c4e8323f35f6038a9899372c5e29fa558
![inkeep-internal-ci[bot]](https://avatars.githubusercontent.com/u/123133689?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated approval from agents-private public-mirror-sync (run: https://github.com/inkeep/agents-private/actions/runs/28411495575). Source of truth is the monorepo; direct edits on inkeep/open-knowledge are overwritten on next sync.
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
1 participant
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.
No description provided.