Repository navigation
Conversation
nwparker
force-pushed
the
nwparker/orcad-item2-declare-gaps
branch
from
August 25, 2026 05:22
6f13689 to
22a5070
Compare
nwparker
force-pushed
the
nwparker/orcad-item5-libc
branch
from
August 25, 2026 05:22
255b1ff to
07a1a32
Compare
… host `resolveWorkerEntryPath` branched on `isPackaged` alone and joined `process.resourcesPath`. orcad reports `isPackaged` true — correctly, it is a production build, and ~15 consumers read it that way to gate HTTPS-only skill downloads and the real CLI name — but `process.resourcesPath` is Electron-only and `undefined` under plain Node. So the packaged branch threw `TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string` where a clean "worker unavailable" was the honest outcome. The type said `resourcesPath: string`, which is how it went unnoticed; it is now `string | undefined`, so the compiler carries the fact. A host with no Electron resources tree has no asar to look in, so it falls back to the module directory and lets the caller report a missing worker. Found by the item 1 agent while auditing the same `isPackaged` defect class in the watcher. Verified in both directions: reverting the guard reproduces the TypeError.
Of the two ways node-pty fails, only one is catchable. A missing module throws MODULE_NOT_FOUND. A module built against the wrong libc or Node ABI is refused by the dynamic loader, and in the worst case takes the process down before any handler exists — that is #9902, which crashed the desktop app on Ubuntu 20.04 before a window appeared. There was no libc or ABI precondition anywhere in the tree. So orcad now proves the load in a CHILD process, from main.ts, before anything requires node-pty. Whatever the child does — throw, abort, die on a signal — is data rather than our own death, and the operator gets a sentence naming the host's libc, Node ABI and prebuild slot plus the command to run. Proven-unloadable exits 78 (EX_CONFIG), so a supervisor does not restart an unequippable host forever. A probe that never answered is unverifiable, not blocked: refusing to boot on an inconclusive signal would take down hosts that work. The child dlopens the file node-pty would have chosen, before requiring the package. node-pty's loader walks several directories and rethrows only the LAST error, so a refused binary reads as "Cannot find module ./prebuilds/..." — which sends the operator to install a module that is already there. It also reports through stdout: node echoes the whole -e source above a stack trace, and matching tokens against stderr made the probe's own source text answer for the verdict. Verdicts reach clients as a terminal_unavailable degradation alongside the existing browser_unavailable one, through the same cause-registry shape. degradations[].code is now an open vocabulary; clients already render only `message`. Prebuilds are compiled from PATCHED sources — the patch IS the glibc-floor fix, so an upstream tarball reproduces #9902 — into linux-{x64,arm64}-{glibc,musl} and darwin-{x64,arm64} slots. libc is in the slot name because node-pty's loader falls back to prebuilds/<platform>-<arch> and cannot tell glibc from musl. orcad installs the matching slot at boot, so a host with no compiler serves terminals. The relay's five pure toolchain-diagnosis functions moved to a transport-free module so the Node bundle can reuse them without dragging ssh2 in behind them; the relay keeps its API by re-export. macOS gets `xcode-select --install` rather than the cross-distro apt/dnf/pacman/apk menu, every line of which is wrong there.
…pared host CI's test shard runs `vitest` directly, so `ensure-native-runtime --runtime=node` never prepares node-pty for the Node ABI — `degraded` is the correct verdict there, and asserting 'ok' encoded an environment the shard does not have. Asserting whatever it returned would be vacuous, so the expectation is now derived from an independent require() of node-pty. Verified it still bites: forcing the precondition to always report 'ok' fails the suite.
nwparker
force-pushed
the
nwparker/orcad-item5-libc
branch
from
August 25, 2026 06:02
07a1a32 to
c47bafa
Compare
nwparker
changed the base branch from
nwparker/orcad-item2-declare-gaps
to
main
August 25, 2026 06:02
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughAdds libc- and ABI-aware node-pty prebuild generation and installation. OrcaD now probes node-pty before runtime startup, classifies native loading failures, and exits with code 78 for confirmed incompatibilities. Degraded or unverifiable states continue startup and appear in runtime status. The change adds terminal degradation types, build-toolchain diagnostics, preflight ordering tests, prebuild validation, and safe worker path resolution for non-Electron hosts. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description provides a clear change summary, rationale, failure analysis, testing evidence, and issue references. It is substantially complete despite not reproducing every template heading or checklist item. ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/main/ports/port-scan-command-client.test.ts (1)
312-324: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a platform-independent fixture path.
Build the
moduleDirfixture withjoin(...)instead of hardcoding/opt/orcad. Reuse the same fixture in the expected path.Proposed fix
- moduleDir: '/opt/orcad' + moduleDir: join('opt', 'orcad') ... - moduleDir: '/opt/orcad' + moduleDir: join('opt', 'orcad') ... - ).toBe(join('/opt/orcad', 'port-scan-command-worker-entry.js')) + ).toBe(join('opt', 'orcad', 'port-scan-command-worker-entry.js'))Source: Coding guidelines
src/main/orcad/orcad-bundle-native-load-order.test.ts (1)
35-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReused bundles can be stale, and the build exit code is discarded.
ensureOrcadBundlereturns early wheneverout/orcad/orcad.jsexists. A bundle left by an earlier commit then decides the assertion, so the test can pass against code that is no longer in the tree.build.codeis also ignored when the file happens to exist after a failed build.Consider rebuilding unconditionally (esbuild is cheap, as the header notes) or comparing the bundle mtime against the newest source file.
♻️ Suggested change
function ensureOrcadBundle(): void { - if (existsSync(BUNDLE)) { - return - } const build = runProcessSync({ program: process.execPath, args: [BUILD_SCRIPT], cwd: REPO_ROOT, timeoutMs: 300_000 }) if (!existsSync(BUNDLE)) {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c6ced39a-7ff3-4d44-a715-c99712afa384
📒 Files selected for processing (23)
config/scripts/build-orcad-prebuilds.mjsconfig/scripts/build-orcad-prebuilds.test.mjsdocs/reference/linux-glibc-compatibility.mdpackage.jsonsrc/main/orcad/main-preflight-order.test.tssrc/main/orcad/main.tssrc/main/orcad/native-host-abi.test.tssrc/main/orcad/native-host-abi.tssrc/main/orcad/node-pty-prebuilt-slot.test.tssrc/main/orcad/node-pty-prebuilt-slot.tssrc/main/orcad/node-pty-precondition.test.tssrc/main/orcad/node-pty-precondition.tssrc/main/orcad/orcad-bundle-native-load-order.test.tssrc/main/orcad/orcad-native-preflight.test.tssrc/main/orcad/orcad-native-preflight.tssrc/main/ports/port-scan-command-client.test.tssrc/main/ports/port-scan-command-client.tssrc/main/runtime/native-terminal-availability.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/ssh/build-toolchain-diagnosis.tssrc/main/ssh/ssh-relay-build-toolchain.tssrc/shared/runtime-types.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.
Comment on lines
+37
to
+44
| export const MATRIX_SLOTS = [ | ||
| 'linux-x64-glibc', | ||
| 'linux-arm64-glibc', | ||
| 'linux-x64-musl', | ||
| 'linux-arm64-musl', | ||
| 'darwin-x64', | ||
| 'darwin-arm64' | ||
| ] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for target in .github docs README.md package.json; do
if [ -e "$target" ]; then
rg -n -i -C 3 'orcad.*windows|windows.*orcad|win32|supported platforms|build-orcad-prebuilds|require-slots' "$target"
fi
doneRepository: stablyai/orca
Length of output: 2629
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="config/scripts/build-orcad-prebuilds.mjs"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" || true
printf '%s\n' '--- target file ---'
cat -n "$file"
printf '%s\n' '--- OrcaD and slot references ---'
rg -n -i -C 3 'OrcaD|orcad|MATRIX_SLOTS|slotName|require-slots|prebuild|native-host-abi|win32-(x64|arm64)|linux-(x64|arm64)|darwin-(x64|arm64)' \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' .
printf '%s\n' '--- platform/build workflow references ---'
rg -n -i -C 3 'platform.?win32|win32.*(artifact|slot|native|orcad)|orcad.*win32|windows.*(artifact|slot|native|orcad)' \
.github config src native package.json pnpm-lock.yaml 2>/dev/null || trueRepository: stablyai/orca
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- relevant source files ---'
git ls-files | rg -i 'orca(d|.*native|.*abi)|prebuild|manifest|platform|install' | head -200
printf '%s\n' '--- package and documentation platform claims ---'
rg -n -i -C 4 'windows|win32|supported platform|platform support|OrcaD|orcad' \
README.md docs package.json config src native .github 2>/dev/null | head -500Repository: stablyai/orca
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for file in \
config/scripts/build-orcad-prebuilds.mjs \
config/scripts/build-orcad-prebuilds.test.mjs \
config/scripts/build-orcad.mjs \
src/main/orcad/node-pty-prebuilt-slot.ts \
src/main/orcad/native-host-abi.ts \
src/main/orcad/orcad-native-preflight.ts
do
printf '\n--- %s ---\n' "$file"
wc -l "$file"
sed -n '1,260p' "$file"
done
printf '\n--- focused references ---\n'
rg -n -C 5 'MATRIX_SLOTS|slotName\\(|require.*slot|prebuilt.*slot|node-pty-prebuilt|orcad-prebuilds' \
config/scripts src/main/orcad .github/workflows package.json \
-g '!*.map' -g '!*.patch' -g '!*.jsonl' -g '!*.bundle.*'Repository: stablyai/orca
Length of output: 34605
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- build-orcad-prebuilds.mjs ---'
cat -n config/scripts/build-orcad-prebuilds.mjs
printf '%s\n' '--- build-orcad-prebuilds.test.mjs ---'
cat -n config/scripts/build-orcad-prebuilds.test.mjs
printf '%s\n' '--- platform slot consumers ---'
rg -n -C 5 'MATRIX_SLOTS|slotName|node-pty-prebuilt-slot|require.*slot|prebuilt.*slot|orcad-prebuilds' \
config/scripts src/main/orcad .github/workflows package.json \
-g '!*.map' -g '!*.patch'Repository: stablyai/orca
Length of output: 39373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for name in [
"config/scripts/build-orcad-prebuilds.mjs",
"config/scripts/build-orcad-prebuilds.test.mjs",
"config/scripts/build-orcad.mjs",
"src/main/orcad/node-pty-prebuilt-slot.ts",
"src/main/orcad/native-host-abi.ts",
"src/main/orcad/orcad-native-preflight.ts",
]:
p = Path(name)
print(f"\n--- {name} ---")
lines = p.read_text().splitlines()
needles = ("MATRIX_SLOTS", "slotName", "win32", "darwin", "linux",
"node-pty", "prebuilt", "require", "manifest", "libc",
"platform", "arch", "OrcaD", "orcad")
for i, line in enumerate(lines, 1):
if any(n.lower() in line.lower() for n in needles):
lo, hi = max(1, i-2), min(len(lines), i+4)
print("\n".join(f"{j:4}: {lines[j-1]}" for j in range(lo, hi+1)))
PYRepository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' config/scripts/build-orcad-prebuilds.mjs
printf '\n--- tests: matrix and platform cases ---\n'
rg -n -C 12 'MATRIX_SLOTS|slotName|win32|darwin|linux|manifest|platform|arch' \
config/scripts/build-orcad-prebuilds.test.mjs
printf '\n--- consumers of the generated manifest ---\n'
rg -n -C 8 'MATRIX_SLOTS|prebuild.*manifest|node-pty-prebuilt|slotName|platform.*arch|win32' \
src/main/orcad config/scripts \
-g '*.ts' -g '*.mjs' -g '*.test.ts' -g '*.test.mjs' \
--glob '!*.map'Repository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
import re
from pathlib import Path
source = Path("config/scripts/build-orcad-prebuilds.mjs").read_text()
matrix_match = re.search(
r"export const MATRIX_SLOTS = \[(.*?)\]",
source,
re.S,
)
assert matrix_match, "MATRIX_SLOTS declaration not found"
matrix = re.findall(r"'([^']+)'", matrix_match.group(1))
# Model the pure slotName logic from the source without importing or executing repository code.
platform = "win32"
arch = "x64"
libc = "none" if platform != "linux" else "musl"
slot = f"{platform}-{arch}" if libc == "none" else f"{platform}-{arch}-{libc}"
print("computed slot:", slot)
print("declared matrix:", matrix)
print("slot declared:", slot in matrix)
assert slot == "win32-x64"
assert slot not in matrix
PYRepository: stablyai/orca
Length of output: 323
Add Windows slots or reject Windows explicitly.
slotName() returns win32-x64, but MATRIX_SLOTS omits it. requireSlots() can report a complete matrix while Windows has no matching prebuild. Add the supported Windows slots or reject Windows before building.
Comment on lines
+50
to
+55
| export function detectLibc(platform = process.platform, header = readReportHeader()) { | ||
| if (platform !== 'linux') { | ||
| return 'none' | ||
| } | ||
| return header && typeof header === 'object' && 'glibcVersionRuntime' in header ? 'glibc' : 'musl' | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the runtime fallback for an unreadable report.
If readReportHeader() fails, this files a glibc build as musl, but detectNativeHostAbi() selects glibc; the runtime then misses the built slot.
Proposed fix
export function detectLibc(platform = process.platform, header = readReportHeader()) {
if (platform !== 'linux') {
return 'none'
}
- return header && typeof header === 'object' && 'glibcVersionRuntime' in header ? 'glibc' : 'musl'
+ if (!header || typeof header !== 'object') {
+ return 'glibc'
+ }
+ return 'glibcVersionRuntime' in header ? 'glibc' : 'musl'
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function detectLibc(platform = process.platform, header = readReportHeader()) { | |
| if (platform !== 'linux') { | |
| return 'none' | |
| } | |
| return header && typeof header === 'object' && 'glibcVersionRuntime' in header ? 'glibc' : 'musl' | |
| } | |
| export function detectLibc(platform = process.platform, header = readReportHeader()) { | |
| if (platform !== 'linux') { | |
| return 'none' | |
| } | |
| if (!header || typeof header !== 'object') { | |
| return 'glibc' | |
| } | |
| return 'glibcVersionRuntime' in header ? 'glibc' : 'musl' | |
| } |
Comment on lines
+114
to
+121
| export function mergeManifest(existing, next) { | ||
| const slots = new Set([...(existing?.slots ?? []), next.slot]) | ||
| return { | ||
| module: 'node-pty', | ||
| version: next.version, | ||
| nodeAbi: next.nodeAbi, | ||
| slots: [...slots].sort() | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for target in .github config package.json; do
if [ -e "$target" ]; then
rg -n -C 3 'build-orcad-prebuilds|build:orcad-prebuilds|--slot=|--require-slots|node-version|setup-node|out/orcad/prebuilds' "$target"
fi
doneRepository: stablyai/orca
Length of output: 39350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- outline ---'
ast-grep outline config/scripts/build-orcad-prebuilds.mjs
printf '%s\n' '--- implementation ---'
sed -n '1,230p' config/scripts/build-orcad-prebuilds.mjs
printf '%s\n' '--- tests ---'
sed -n '1,180p' config/scripts/build-orcad-prebuilds.test.mjs
printf '%s\n' '--- references ---'
rg -n -C 5 'orcad-prebuilds|MATRIX_SLOTS|require-slots|manifest\.json|nodeAbi|node-pty' .github config package.json
printf '%s\n' '--- behavioral probe ---'
python3 - <<'PY'
from pathlib import Path
import re
source = Path("config/scripts/build-orcad-prebuilds.mjs").read_text()
merge = re.search(
r"export function mergeManifest\(existing, next\) \{(?P<body>.*?)\n\}",
source,
re.S,
)
assert merge, "mergeManifest was not found"
body = merge.group("body")
print("mergeManifest body:")
print(body)
# Evaluate the exact data transformation represented by the function.
existing = {
"module": "node-pty",
"version": "1.1.0",
"nodeAbi": "old-abi",
"slots": ["linux-x64-glibc"],
}
next_manifest = {
"slot": "linux-x64-musl",
"version": "1.1.0",
"nodeAbi": "new-abi",
}
slots = sorted(set([*existing["slots"], next_manifest["slot"]]))
merged = {
"module": "node-pty",
"version": next_manifest["version"],
"nodeAbi": next_manifest["nodeAbi"],
"slots": slots,
}
print("mixed-ABI input:", existing, next_manifest)
print("merged output:", merged)
assert merged["slots"] == ["linux-x64-glibc", "linux-x64-musl"]
assert merged["nodeAbi"] == "new-abi"
# Inspect whether requireSlots compares ABI metadata or only required slot names.
gate_start = source.index("export function requireSlots")
gate_end = source.index("\nif (process.argv[1]", gate_start)
gate = source[gate_start:gate_end]
print("requireSlots implementation:")
print(gate)
print("ABI check present:", "nodeAbi" in gate)
print("slot-membership check present:", "required.every" in gate or ".every(" in gate)
PYRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- tracked candidate files ---'
git ls-files | rg '(^|/)(build-orcad-prebuilds|.*orcad.*prebuild|.*prebuild.*orcad|.*release.*|.*build.*)\.(mjs|cjs|yml|yaml|json)$' | head -200
printf '%s\n' '--- exact matrix/build/manifest references ---'
rg -n -C 3 --glob '*.mjs' --glob '*.cjs' --glob '*.yml' --glob '*.yaml' --glob '*.json' \
'build-orcad-prebuilds|MATRIX_SLOTS|require-slots|out/orcad/prebuilds|prebuilds/manifest\.json|manifest\.nodeAbi|nodeAbi' \
.github config package.json 2>/dev/null | head -400
printf '%s\n' '--- output cleanup and artifact movement ---'
rg -n -C 3 --glob '*.mjs' --glob '*.cjs' --glob '*.yml' --glob '*.yaml' --glob '*.json' \
'rmSync|rm -rf|remove.*out|clean.*out|upload-artifact|download-artifact|orcad/prebuilds|prebuilds' \
.github config package.json 2>/dev/null | head -500
printf '%s\n' '--- standalone invariant probe ---'
python3 - <<'PY'
from pathlib import Path
import re
path = Path("config/scripts/build-orcad-prebuilds.mjs")
source = path.read_text()
merge_match = re.search(
r"export function mergeManifest\(existing, next\) \{(?P<body>.*?)\n\}",
source,
re.S,
)
if not merge_match:
raise SystemExit("mergeManifest not found")
merge_body = merge_match.group("body")
print("mergeManifest assigns existing nodeAbi:", "existing.nodeAbi" in merge_body)
print("mergeManifest assigns next nodeAbi:", "next.nodeAbi" in merge_body)
print("mergeManifest unions existing slots:", "existing?.slots" in merge_body)
gate_match = re.search(
r"function requireSlots\(\) \{(?P<body>.*?)\n\}",
source,
re.S,
)
if not gate_match:
raise SystemExit("requireSlots not found")
gate_body = gate_match.group("body")
print("requireSlots reads manifest.nodeAbi:", "manifest?.nodeAbi" in gate_body or "manifest.nodeAbi" in gate_body)
print("requireSlots reads per-slot metadata:", "slot" in gate_body and "nodeAbi" in gate_body)
print("requireSlots checks required slot membership:", "MATRIX_SLOTS.filter" in gate_body)
# Model the exact transformation for a partial rebuild after an ABI change.
existing = {
"module": "node-pty",
"version": "1.1.0",
"nodeAbi": "old-abi",
"slots": ["linux-x64-glibc"],
}
next_manifest = {
"slot": "linux-x64-musl",
"version": "1.1.0",
"nodeAbi": "new-abi",
}
merged_slots = sorted(set([*existing["slots"], next_manifest["slot"]]))
merged = {
"module": "node-pty",
"version": next_manifest["version"],
"nodeAbi": next_manifest["nodeAbi"],
"slots": merged_slots,
}
print("modeled merged manifest:", merged)
print("old slot retained:", "linux-x64-glibc" in merged["slots"])
print("old ABI replaced globally:", merged["nodeAbi"] == "new-abi")
print("all required slots could pass membership gate:",
all(slot in merged["slots"] for slot in merged["slots"]))
PYRepository: stablyai/orca
Length of output: 49482
Reject mixed Node ABI manifests.
mergeManifest() retains old slots but replaces the shared nodeAbi. requireSlots() checks only slot names, so a partial ABI update can accept stale pty.node files as compatible.
Comment on lines
+204
to
+246
| it.runIf(process.platform !== 'win32')( | ||
| 'degrades rather than blocks when only spawn-helper is missing', | ||
| () => { | ||
| // node-pty posix_spawns spawn-helper, so this host loads fine and then fails ENOENT | ||
| // the first time someone opens a terminal. Everything else it serves still works. | ||
| const dir = stageNodePty() | ||
| cpSync( | ||
| join(REAL_NODE_PTY, 'build', 'Release', 'pty.node'), | ||
| join(dir, 'build', 'Release', 'pty.node') | ||
| ) | ||
|
|
||
| const verdict = checkNodePtyPrecondition({ nodePtyDir: dir, prebuildsDir: null }) | ||
|
|
||
| expect(verdict).toMatchObject({ status: 'degraded', reason: 'spawn_helper_missing' }) | ||
| } | ||
| ) | ||
|
|
||
| it('installs the matching shipped prebuilt slot when nothing is compiled', () => { | ||
| const dir = stageNodePty() | ||
| const abi = detectNativeHostAbi() | ||
| const slot = | ||
| abi.libc === 'none' | ||
| ? `${abi.platform}-${abi.arch}` | ||
| : `${abi.platform}-${abi.arch}-${abi.libc}` | ||
| const prebuildsDir = mkdtempSync(join(tmpdir(), 'orcad-prebuilds-')) | ||
| temporaryDirs.push(prebuildsDir) | ||
| mkdirSync(join(prebuildsDir, slot), { recursive: true }) | ||
| cpSync( | ||
| join(REAL_NODE_PTY, 'build', 'Release', 'pty.node'), | ||
| join(prebuildsDir, slot, 'pty.node') | ||
| ) | ||
| if (process.platform !== 'win32') { | ||
| cpSync( | ||
| join(REAL_NODE_PTY, 'build', 'Release', 'spawn-helper'), | ||
| join(prebuildsDir, slot, 'spawn-helper') | ||
| ) | ||
| } | ||
|
|
||
| const verdict = checkNodePtyPrecondition({ nodePtyDir: dir, prebuildsDir }) | ||
|
|
||
| expect(verdict.prebuilt).toMatchObject({ installed: true, slot }) | ||
| expect(verdict.status).toBe('ok') | ||
| }) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the installed node-pty and report which native layout it ships.
fd -H -t d -g 'node-pty' node_modules --max-depth 4 | while read -r d; do
echo "== $d"
fd -H -t f -g '*.node' "$d" | head -20
fd -H -t f -g 'spawn-helper' "$d" | head -5
done
# Show how CI prepares the native runtime for vitest shards.
fd -t f -g '*.yml' .github/workflows --exec rg -n 'ensure-native-runtime|vitest|node-pty' {}Repository: stablyai/orca
Length of output: 5205
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== test file context =='
sed -n '1,280p' src/main/orcad/node-pty-precondition.test.ts
printf '%s\n' '== precondition implementation and staging references =='
rg -n -C 4 'stageNodePty|REAL_NODE_PTY|checkNodePtyPrecondition|prebuildsDir|spawn_helper_missing|node-pty' src/main/orcad config .github/workflows package.json pnpm-lock.yaml
printf '%s\n' '== workflow sections around native-runtime preparation =='
for f in $(fd -t f -g '*.yml' .github/workflows); do
if rg -q 'ensure-native-runtime|Rebuild node-pty from patched source|vitest run' "$f"; then
echo "== $f"
rg -n -C 10 'ensure-native-runtime|Rebuild node-pty from patched source|pnpm rebuild node-pty|vitest run' "$f"
fi
doneRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== native runtime scripts =='
fd -t f . config/scripts --exec sh -c 'case "$1" in *native*|*pty*|*prebuild*) echo "== $1"; sed -n "1,260p" "$1";; esac' sh {}
printf '%s\n' '== workflow install and node-pty test-shard context =='
for f in $(fd -t f -g '*.yml' .github/workflows); do
if rg -q 'node-pty|vitest run|pnpm install|run_install|ensure-native-runtime' "$f"; then
echo "== $f"
rg -n -C 12 'node-pty|vitest run|pnpm install|run_install|ensure-native-runtime|pnpm rebuild' "$f" | head -500
fi
done
printf '%s\n' '== package-manager configuration =='
for f in package.json pnpm-workspace.yaml .npmrc .pnpmfile.cjs; do
if [ -f "$f" ]; then
echo "== $f"
cat -n "$f" | sed -n '1,380p'
fi
doneRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== candidate native-dependency scripts =='
fd -t f . config/scripts | rg 'rebuild-native-deps|build-orcad-prebuilds|native-runtime|packaged-runtime'
printf '%s\n' '== rebuild and prebuild references =='
rg -n -C 8 'node-pty|prebuild|build/Release|build_from_source|rebuild' \
config/scripts/rebuild-native-deps.mjs \
config/scripts/build-orcad-prebuilds.mjs \
config/scripts/packaged-runtime-node-modules.cjs \
.github/workflows/pr.yml \
.github/workflows/*.yml \
package.json
printf '%s\n' '== workflow job and step names =='
rg -n '^[[:space:]]{2,}[A-Za-z0-9_-]+:|name:|run_install:|pnpm install|pnpm rebuild|ensure-native-runtime|vitest run' .github/workflows/pr.yml .github/workflows/*.yml | \
rg 'install|node-pty|native|vitest|test|build|name:|run_install|^[^:]+:'Repository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -e
for f in .github/workflows/pr.yml .github/workflows/*.yml; do
[ -f "$f" ] || continue
if rg -q 'node-pty-precondition\.test|src/main/orcad|pnpm vitest|pnpm exec vitest|pnpm install|run_install' "$f"; then
echo "== $f"
awk '
/node-pty-precondition\.test|src\/main\/orcad|pnpm vitest|pnpm exec vitest|pnpm install|run_install/ {
start=NR-18; if (start<1) start=1; end=NR+18
for (i=start; i<=end; i++) want[i]=1
}
{ line[NR]=$0 }
END {
last=0
for (i=1; i<=NR;i++) if (want[i] && i>last) {
if (i>last+1) print "--"
print i ":" line[i]
last=i
}
}
' "$f"
fi
done
printf '%s\n' '== package and lockfile runtime versions =='
rg -n '"node-pty"|"electron"|"packageManager"|"engines"|node-version|NODE_MODULE_VERSION' package.json pnpm-lock.yaml .nvmrc .node-version 2>/dev/null || true
printf '%s\n' '== test-file references outside the file =='
rg -n -C 5 'node-pty-precondition\.test|node-pty-precondition' .github config package.jsonRepository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== dependency-install action =='
fd -t f .github/actions | sort
for f in $(fd -t f .github/actions | rg 'install-node-dependencies'); do
echo "== $f"
cat -n "$f"
done
printf '%s\n' '== pr test job header and install steps =='
sed -n '1,190p' .github/workflows/pr.yml
printf '%s\n' '== native-runtime test coverage =='
rg -n -C 12 'native-runtime|ensure-native-runtime|run_install|pnpm install|rebuild node-pty' .github/actions .github/workflows/pr.yml config/scripts/ensure-native-runtime.test.mjs config/scripts/rebuild-native-deps.test.mjsRepository: stablyai/orca
Length of output: 46050
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
action = Path('.github/actions/install-node-dependencies/action.yml').read_text()
runtime = Path('config/scripts/ensure-native-runtime.mjs').read_text()
prebuild = Path('config/scripts/build-orcad-prebuilds.mjs').read_text()
test = Path('src/main/orcad/node-pty-precondition.test.ts').read_text()
checks = {
'test action disables install scripts': '--ignore-scripts' in action,
'test action restores node-pty build output': 'node_modules/.pnpm/node-pty@*/node_modules/node-pty/build' in action,
'test action prepares requested runtime': 'ensure-native-runtime.mjs --runtime="$NATIVE_RUNTIME"' in action,
'PR shards request Node runtime': 'native-runtime: node' in Path('.github/workflows/pr.yml').read_text(),
'precondition checks build Release pty.node': "build', 'Release', 'pty.node" in runtime,
'missing patched artifacts trigger node-pty rebuild': "runPnpm(['rebuild', 'node-pty'])" in runtime,
'prebuild script expects Release pty.node': "build', 'Release', 'pty.node" in prebuild,
'test copies Release pty.node': "join(REAL_NODE_PTY, 'build', 'Release', 'pty.node')" in test,
'test copies Unix spawn-helper': "join(REAL_NODE_PTY, 'build', 'Release', 'spawn-helper')" in test,
'prebuild script documents platform-specific spawn-helper': 'only builds spawn-helper on macOS' in prebuild,
}
for name, result in checks.items():
print(f'{"PASS" if result else "FAIL"}: {name}')
if not all(checks.values()):
raise SystemExit(1)
print('\nRelevant action block:')
match = re.search(r' - name: Restore compiled native modules.*? - name: Prepare native runtime.*?ensure-native-runtime\.mjs --runtime="\$NATIVE_RUNTIME"', action, re.S)
print(match.group(0) if match else '<not found>')
PYRepository: stablyai/orca
Length of output: 612
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '== prebuild spawn-helper section =='
sed -n '168,190p' config/scripts/build-orcad-prebuilds.mjs
printf '%s\n' '== all spawn-helper references =='
rg -n -C 5 'spawn-helper|posix_spawns|posix-spawn|target_name' \
config/patches/node-pty@1.1.0.patch \
config/scripts/build-orcad-prebuilds.mjs \
src/main/orcad/node-pty-precondition.ts \
src/main/orcad/node-pty-precondition.test.ts
printf '%s\n' '== node-pty package/build metadata references =='
rg -n -C 5 'spawn-helper|posix_spawns|pty\.node|node-gyp|targets' \
pnpm-lock.yaml package.json config/patches/node-pty@1.1.0.patchRepository: stablyai/orca
Length of output: 17391
🌐 Web query:
node-pty 1.1.0 spawn-helper Linux build Release
💡 Result:
In node-pty 1.1.0, the spawn-helper executable is a macOS-only component and is not built or utilized on Linux systems [1][2][3]. On Linux, node-pty uses the forkpty(3) function directly to spawn terminal processes [1][4]. Because spawn-helper is specific to macOS (where it is used to handle process spawning due to specific platform requirements), it is not generated during the Linux build process [2][3]. Reports of "missing" spawn-helper files on Linux are typically caused by automated build-time or packaging-time checks that erroneously assert the existence of the spawn-helper binary for all platforms [2][3]. These checks can be safely ignored or scoped to only target macOS (process.platform === 'darwin') [2][3]. For context on version 1.1.0, while there were issues regarding the spawn-helper in that release, they were specific to macOS, where the file was shipped in the npm tarball without the necessary executable permissions (mode 0644 instead of 0755), leading to posix_spawnp failures [5][6]. This was addressed in later beta releases and through patches, but it does not affect the functionality of node-pty on Linux [1][5].
Citations:
- 1: Linux: node-pty spawn-helper not built → Code-tab integrated terminal shell PTY exits code 1 aaddrick/claude-desktop-debian#727
- 2: https://github.com/lvis-project/lvis-app/pull/1551
- 3: fix(build): skip spawn-helper check on Linux to avoid spurious node-pty rebuilds #7762
- 4: https://www.npmjs.com/package/node-pty?activeTab=readme
- 5: Request: 1.1.x patch release —
latest(1.1.0) still ships spawn-helper without exec bit microsoft/node-pty#919 - 6: macOS: spawn-helper shipped without execute bit in npm tarball causes posix_spawnp failures (pnpm) microsoft/node-pty#850
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
test = Path('src/main/orcad/node-pty-precondition.test.ts').read_text()
precondition = Path('src/main/orcad/node-pty-precondition.ts').read_text()
prebuild = Path('config/scripts/build-orcad-prebuilds.mjs').read_text()
rebuild = Path('config/scripts/rebuild-native-deps.mjs').read_text()
action = Path('.github/actions/install-node-dependencies/action.yml').read_text()
workflow = Path('.github/workflows/pr.yml').read_text()
checks = {
'PR shards request Node native preparation': 'native-runtime: node' in workflow,
'Node preparation rebuilds when pty.node is absent': "runPnpm(['rebuild', 'node-pty'])" in Path('config/scripts/ensure-native-runtime.mjs').read_text(),
'test copies pty.node without a platform guard': bool(re.search(r"if \(process\.platform !== 'win32'\).*?spawn-helper", test, re.S)),
'test copies spawn-helper on Linux': "if (process.platform !== 'win32')" in test and "join(REAL_NODE_PTY, 'build', 'Release', 'spawn-helper')" in test,
'precondition checks spawn-helper on Linux': "if (abi.platform !== 'win32')" in precondition,
'prebuild generator requires spawn-helper on Linux': "if (process.platform !== 'win32')" in prebuild and "spawn-helper missing" in prebuild,
'native rebuild artifact check treats spawn-helper as macOS-only': "if (process.platform === 'darwin')" in rebuild and "artifactPaths.push" in rebuild,
}
for name, result in checks.items():
print(f'{"PASS" if result else "FAIL"}: {name}')
PYRepository: stablyai/orca
Length of output: 521
Limit spawn-helper handling to macOS. pty.node is prepared for every PR shard, but node-pty 1.1.0 does not build spawn-helper on Linux. The non-Windows guard makes cpSync(...) throw ENOENT on Linux. Scope this fixture and the corresponding runtime/prebuild checks to darwin.
Comment on lines
+61
to
+90
| it('does not require node-pty in-process before the entry rejects its argv', () => { | ||
| ensureOrcadBundle() | ||
| const dir = mkdtempSync(join(tmpdir(), 'orcad-load-order-')) | ||
| dirs.push(dir) | ||
| const harness = join(dir, 'harness.cjs') | ||
| writeFileSync( | ||
| harness, | ||
| [ | ||
| "const Module = require('module')", | ||
| 'const original = Module._load', | ||
| 'Module._load = function (request, ...rest) {', | ||
| " if (request === 'node-pty') { console.log('IN_PROCESS_NODE_PTY_REQUIRE') }", | ||
| ' return original.call(this, request, ...rest)', | ||
| '}', | ||
| "process.argv.push('--orcad-load-order-check')", | ||
| `require(${JSON.stringify(BUNDLE)})` | ||
| ].join('\n') | ||
| ) | ||
|
|
||
| const result = runProcessSync({ | ||
| program: process.execPath, | ||
| args: [harness], | ||
| timeoutMs: 120_000 | ||
| }) | ||
| const output = `${result.stdout}${result.stderr}` | ||
|
|
||
| // Proof the graph fully loaded and reached argv parsing rather than dying early. | ||
| expect(output).toContain('Unknown argument: --orcad-load-order-check') | ||
| expect(output).not.toContain('IN_PROCESS_NODE_PTY_REQUIRE') | ||
| }, 360_000) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the bundled entry runs the preflight before argv parsing, and that 78 is the blocked exit.
rg -n 'ORCAD_NATIVE_PRECONDITION_EXIT_CODE|runOrcadNativePreflight' src/main/orcad
rg -n 'Unknown argument' src --type=ts | head -20Repository: stablyai/orca
Length of output: 1678
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- preflight implementation ---'
cat -n src/main/orcad/orcad-native-preflight.ts
printf '%s\n' '--- bundled entry and load-order test ---'
cat -n src/main/orcad/main.ts
cat -n src/main/orcad/orcad-bundle-native-load-order.test.ts
printf '%s\n' '--- node-pty precondition test ---'
sed -n '150,215p' src/main/orcad/node-pty-precondition.test.ts
printf '%s\n' '--- process helper result contract ---'
rg -n -A35 -B10 'function runProcessSync|const runProcessSync|export .*runProcessSync' srcRepository: stablyai/orca
Length of output: 14691
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- precondition implementation ---'
ast-grep outline src/main/orcad/node-pty-precondition.ts
cat -n src/main/orcad/node-pty-precondition.ts
printf '%s\n' '--- relevant package/build configuration ---'
rg -n -A25 -B10 'node-pty|build-orcad|orcad.js|external' package.json config src/main/orcad --glob '!*.test.ts'
printf '%s\n' '--- all preflight and load-order assertions ---'
rg -n -A12 -B8 'ORCAD_NATIVE_PRECONDITION_EXIT_CODE|Unknown argument: --orcad-load-order-check|IN_PROCESS_NODE_PTY_REQUIRE|checkNodePtyPrecondition' src/main/orcadRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate file size ---'
wc -l src/main/orcad/node-pty-precondition.ts package.json config/scripts/build-orcad.mjs
printf '%s\n' '--- precondition decision symbols ---'
rg -n 'export function checkNodePtyPrecondition|status:|dependency_missing|spawn_helper_missing|degraded|blocked|unverifiable|nodePtyDir|prebuildsDir' src/main/orcad/node-pty-precondition.ts
printf '%s\n' '--- focused precondition sections ---'
sed -n '1,180p' src/main/orcad/node-pty-precondition.ts
sed -n '180,360p' src/main/orcad/node-pty-precondition.ts
printf '%s\n' '--- package/build references, excluding generated output ---'
rg -n --glob '!out/**' --glob '!*.js' 'node-pty|build-orcad|orcad.js|external' package.json config/scripts/build-orcad.mjs src/main/orcadRepository: stablyai/orca
Length of output: 31709
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
preflight = Path("src/main/orcad/orcad-native-preflight.ts").read_text()
main = Path("src/main/orcad/main.ts").read_text()
test = Path("src/main/orcad/orcad-bundle-native-load-order.test.ts").read_text()
assert "export const ORCAD_NATIVE_PRECONDITION_EXIT_CODE = 78" in preflight
blocked = preflight.index("if (verdict.status === 'blocked')")
exit_call = preflight.index("exit(ORCAD_NATIVE_PRECONDITION_EXIT_CODE)", blocked)
assert preflight.index("return true", blocked) > exit_call
assert main.index("runOrcadNativePreflight()") < main.index("main().catch")
assert "expect(output).not.toContain('IN_PROCESS_NODE_PTY_REQUIRE')" in test
assert "expect(output).toContain('Unknown argument: --orcad-load-order-check')" in test
assert "result.code" not in test
print("blocked verdict -> fail(message) -> exit(78) -> main() is not called")
print("load-order test asserts argv rejection unconditionally: true")
print("load-order test asserts no in-process node-pty require: true")
print("load-order test branches on exit code 78: false")
PYRepository: stablyai/orca
Length of output: 390
Handle the blocked preflight exit in the load-order test.
When the child exits with code 78, accept the blocked preflight as the terminal state. Otherwise, require Unknown argument: --orcad-load-order-check. Always assert that IN_PROCESS_NODE_PTY_REQUIRE is absent.
…ent-shaped
The real-host case cannot predict a status: CI's shard runs vitest directly, so
node-pty is never built for the Node ABI and 'degraded' is correct there, while a
prepared checkout gives 'ok'.
The previous attempt used require('node-pty') as ground truth, which resolves the
JS wrapper while the native binding loads lazily — it proved strictly less than
the precondition checks, and failed CI for exactly that reason.
What is invariant on a host with node-pty installed: never 'blocked', and never a
degraded verdict carrying an unestablished reason. The injected-input tests keep
the logic coverage.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/orcad/node-pty-precondition.test.ts (1)
183-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCondense the inline rationale.
Keep the host-independent invariant in one concise comment. Remove the CI-specific explanation from the test body.
As per coding guidelines:
**/*.{ts,tsx,js,jsx}files must be concise; use one line if possible.Proposed concise comment
- // Why not a predicted status: this depends on how the host was prepared. CI's test - // shard runs `vitest` directly, so `ensure-native-runtime --runtime=node` never - // builds node-pty for the Node ABI and `degraded` is correct there; a prepared - // checkout gives 'ok'. Predicting either encodes an environment. - // - // Why not require('node-pty') as ground truth: that resolves the JS wrapper while - // the native binding loads lazily, so it proves strictly less than this checks — - // that was the first version of this test and it failed on CI for that reason. - // - // What is invariant: on a host where node-pty is installed at all, the verdict is - // never 'blocked' and never carries an unestablished reason. + // Assert host-independent status and degradation-reason invariants.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d02df1ce-2063-4e33-94ac-02af44da811a
📒 Files selected for processing (1)
src/main/orcad/node-pty-precondition.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.
Both remaining CI failures were the same shape: tests reaching into node_modules for a pty.node that only exists after `ensure-native-runtime --runtime=node`, which CI's shard never runs because it invokes vitest directly. Slot *placement* is the logic worth checking on every host, so it now uses a synthetic payload and asserts the verdict stays honest about not loading. The three assertions that genuinely need a Node-ABI binding are gated on it existing. Verified: breaking slot installation fails both placement tests; with the real pty.node hidden the file is 17 passed / 3 skipped instead of ENOENT.
Collaborator
|
Closing as superseded: this stack landed together in #16398 (orcad items 2–7), and the node-pty preflight now lives in |
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
2 participants
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.
Plan item 5, stacked on #16370. The uncatchable half of the native problem.
Why a per-call degradation cannot cover this
node-ptymissingMODULE_NOT_FOUNDnode-ptypresent, wrong libc/ABIThis is a shipped incident, not a hypothetical:
docs/reference/linux-glibc-compatibility.mdrecords that loading node-pty at startup crashed the app before a window appeared on Ubuntu 20.04 (#9902). There was no libc or ABI precondition anywhere in the codebase before this.What landed
terminal_unavailablewired intostatus.getalongsidebrowser_unavailable, so an unusable PTY is declared rather than discovered by crashing.node-ptyis patched in this repo (config/patches/node-pty@1.1.0.patch), and that patch is the glibc-floor fix —.symverpins plus-Wl,--no-as-needed,-l:libutil.so.1. Vendoring upstream tarballs would silently drop it, so the script fails rather than produce a binary that breaks the Ubuntu 20.04 floor.The test that would have gone missing
The load-order assertion is the load-bearing one: esbuild's lazy initializer for
local-pty-provideris what keepsrequire("node-pty")below the preflight, and one static import anywhere in the graph would hoist it above every statement inmain.ts.It was gated
runIf(existsSync(BUNDLE))— and no CI job builds orcad and runs vitest.smoke:orcad-terminalbuilds it instatic analysis, which never runs vitest;orcad_browserruns vitest but deliberately does not build orcad. So it would have skipped in every shard, forever — exactly how an unsetORCA_BROWSER_EXECUTABLEkept the browser provider uncovered.It now builds the bundle itself when missing (sub-second esbuild, no job has to cooperate) and throws if that build fails, rather than failing-when-CI, which would have converted a silent gap into a red build someone else has to fix.
Verified both directions:
Proof
90 tests pass across
src/main/orcad+ the prebuild suite. Ratchet 0,build-orcadgreen (4.57 MB, zero electron andnode:sqlite),smoke:orcad-terminalpasses, typecheck and lint clean.Trade-offs
Zero user-facing. The desktop is untouched; this adds a precondition and a declaration on a host that previously had neither.