diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc731f98d9..9edf013629 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,7 @@ jobs: release_bump_only: ${{ steps.classify.outputs.release_bump_only }} run_heavy_ci: ${{ steps.classify.outputs.run_heavy_ci }} channels_changed: ${{ steps.classify.outputs.channels_changed }} + packaging_changed: ${{ steps.classify.outputs.packaging_changed }} steps: - name: Checkout uses: actions/checkout@v6 @@ -69,13 +70,32 @@ jobs: esac done + # The packed-artifact checks are expensive (Docker image build + a full + # registry install), and they can only regress when dependencies, the + # packed file list, the build, or the checks themselves change. + PACKAGING_CHANGED=false + for file in $CHANGED_FILES; do + case "$file" in + package.json|bun.lock|build.js|scripts/postinstall-patches.js|scripts/check-minimal-linux-npm-artifact.js|scripts/check-windows-node-pty-artifact.js|.github/workflows/ci.yml) + PACKAGING_CHANGED=true + ;; + esac + done + RELEASE_BUMP_ONLY=false if [ "$TITLE_MATCH" = true ] && [ "$FILES_MATCH" = true ]; then RELEASE_BUMP_ONLY=true fi + # A release bump touches package.json only; the artifact was already + # verified on the commit it bumps. + if [ "$RELEASE_BUMP_ONLY" = true ]; then + PACKAGING_CHANGED=false + fi + echo "release_bump_only=$RELEASE_BUMP_ONLY" >> "$GITHUB_OUTPUT" echo "channels_changed=$CHANNELS_CHANGED" >> "$GITHUB_OUTPUT" + echo "packaging_changed=$PACKAGING_CHANGED" >> "$GITHUB_OUTPUT" if [ "$RELEASE_BUMP_ONLY" = true ]; then echo "run_heavy_ci=false" >> "$GITHUB_OUTPUT" @@ -85,6 +105,7 @@ jobs: echo "title=$TITLE" echo "channels_changed=$CHANNELS_CHANGED" + echo "packaging_changed=$PACKAGING_CHANGED" printf 'changed_files=%s\n' "$CHANGED_FILES" check: @@ -198,6 +219,16 @@ jobs: - name: Build bundle run: bun run build + # Covers both Linux legs: the x64 and arm64 prebuilds are published + # independently, so a missing arm64 prebuild breaks npx the same way. + # Gated on packaging changes / pushes to main because the Docker build and + # registry install cost several minutes per leg. + - name: Minimal Linux npm artifact check + if: runner.os == 'Linux' && (github.event_name == 'push' || needs.classify.outputs.packaging_changed == 'true') + env: + LETTA_CODE_MINIMAL_LINUX_ARTIFACT_SKIP_BUILD: "1" + run: bun run check:minimal-linux-npm-artifact + - name: CLI help smoke test run: ./letta.js --help @@ -230,6 +261,13 @@ jobs: npm install -g (Get-Item letta-ai-letta-code-*.tgz).FullName letta --help + - name: Windows node-pty artifact smoke test + if: runner.os == 'Windows' + shell: pwsh + run: | + $env:LETTA_CODE_GLOBAL_NODE_MODULES = npm root -g + bun run check:windows-node-pty-artifact + - name: Test npm install flow (Unix) if: runner.os != 'Windows' shell: sh diff --git a/AGENTS.md b/AGENTS.md index 47caf72766..948e08a59c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,6 +213,23 @@ also rejects staged parent-relative imports (`../`); use the `@/` alias. 11. **biome** — format + lint across source files 12. **typescript** — full `tsc --noEmit` +### Packaged Artifact Checks (not part of `bun run check`) + +These validate the **published npm tarball** rather than the source tree, so +they need Docker or a global install and are wired into CI separately. `bun run +check` does *not* cover packaging regressions. + +| Command | What it proves | Requirements | +|---------|----------------|--------------| +| `check:minimal-linux-npm-artifact` | `npm install` of the packed tarball succeeds in Ubuntu 24.04 with **no** `make`/`python3`, `letta --help` produces output, and node-pty loads a prebuilt binary and allocates a real TTY | Docker; runs on both Linux CI legs | +| `check:windows-node-pty-artifact` | The globally-installed package ships a Windows node-pty prebuild and its ConPTY actually executes written input | Windows; `npm install -g ` must have run first | + +The Linux check derives its Docker platform from `process.arch`; override with +`--platform=linux/arm64` or `LETTA_CODE_MINIMAL_LINUX_ARTIFACT_PLATFORM`, and +pass `--skip-build` to reuse existing build output. In CI it is gated on the +`packaging_changed` output of the `classify` job — add a path there if you +introduce a file that can change the packed artifact. + ### Environment Variables | Variable | Effect | @@ -231,3 +248,5 @@ also rejects staged parent-relative imports (`../`); use the `@/` alias. - **`new URL("./path.ts", import.meta.url)` in tests** is not a static import and is not caught by the `@/` import codemod. Scan for `new URL(` manually when moving source files. - **grep exits 1 on no matches** — pre-commit hooks use `|| true` on grep pipes to prevent false failures on clean commits. - **macOS case-insensitive FS** — `existsSync("bash.ts")` returns `true` when `Bash.ts` exists. Rename scripts that use `existsSync` to check kebab-case targets will silently skip single-word PascalCase files. Use `git mv` for renames. +- **`node-pty` is pinned to an exact prerelease, on purpose.** `1.1.0` (still npm `latest`) ships no `prebuilds/linux-x64`, so `npm install` falls through to `node-gyp rebuild` and fails on minimal Linux images without `make` — this broke public `npx` installs (letta-ai/letta-acp#50). `1.2.0-beta.14` is the first release with Linux prebuilds. Do not "tidy" it back to a caret range or bump it casually; revisit only when upstream `1.2.0` ships stable, and re-run `check:minimal-linux-npm-artifact` when you do. Note `1.2.0` also drops winpty, so Windows now requires build ≥ 18309. +- **A node-pty prebuild can install cleanly and still be unusable.** node-pty's install script only checks that `prebuilds/-/` exists — it never validates the binary, and its presence suppresses the source-build fallback. The linux-x64 prebuild is glibc-linked (GLIBC_2.28), which fails two different ways: on older glibc `require` throws a dynamic-link error, and **on musl (Alpine) `require` succeeds and the first `spawn()` segfaults the process** (verified on `node:22-alpine`). Never `require("node-pty")` directly — go through `requireNodePty()` in `src/utils/node-pty-loader.ts`, which refuses a glibc prebuild on a musl runtime and turns both failures into a tagged error callers degrade on (`exec_command` falls back to a pipe). diff --git a/bun.lock b/bun.lock index 8e55e862e4..4601379436 100644 --- a/bun.lock +++ b/bun.lock @@ -14,7 +14,7 @@ "cross-spawn": "^7.0.6", "glob": "^13.0.0", "ink-link": "^5.0.0", - "node-pty": "^1.1.0", + "node-pty": "1.2.0-beta.14", "open": "^10.2.0", "react": "18.2.0", "sharp": "^0.34.5", @@ -822,7 +822,7 @@ "node-fetch": ["node-fetch@2.7.0", "", { "dependencies": { "whatwg-url": "^5.0.0" }, "peerDependencies": { "encoding": "^0.1.0" }, "optionalPeers": ["encoding"] }, "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A=="], - "node-pty": ["node-pty@1.1.0", "", { "dependencies": { "node-addon-api": "^7.1.0" } }, "sha512-20JqtutY6JPXTUnL0ij1uad7Qe1baT46lyolh2sSENDd4sTzKZ4nmAFkeAARDKwmlLjPx6XKRlwRUxwjOy+lUg=="], + "node-pty": ["node-pty@1.2.0-beta.14", "", { "dependencies": { "node-addon-api": "^7.1.0" } }, "sha512-XORU9BQgpxVgqr7WivjJ17mLenOHUgKKWzuZZNaw3NDYgHc/wPJQMSaoLDrpEgqV6aU1nNwil1o/OqYj6lWmUA=="], "node-source-walk": ["node-source-walk@7.0.2", "", { "dependencies": { "@babel/parser": "^7.29.0" } }, "sha512-71kFFjYaSshDTA8/a2HiTYPLdASWjLJxUyJxGE+ffxU+KhxSBtM9kiLUX+R2yooFdSFKMFpi4n3PFtDy6qXv8A=="], diff --git a/package.json b/package.json index 211ae972c3..2f2fe8d0c5 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,7 @@ "cron-parser": "^5.6.1", "glob": "^13.0.0", "ink-link": "^5.0.0", - "node-pty": "^1.1.0", + "node-pty": "1.2.0-beta.14", "open": "^10.2.0", "react": "18.2.0", "sharp": "^0.34.5", @@ -169,6 +169,8 @@ "check:test-coverage": "node scripts/check-test-coverage.cjs", "check:skill-frontmatter": "node scripts/check-skill-frontmatter.js", "check:bundled-skill-scripts": "node scripts/check-bundled-skill-scripts.js", + "check:minimal-linux-npm-artifact": "node scripts/check-minimal-linux-npm-artifact.js", + "check:windows-node-pty-artifact": "node scripts/check-windows-node-pty-artifact.js", "check": "bun run scripts/check.js", "dev": "node scripts/dev.cjs", "build": "node scripts/postinstall-patches.js && bun run build.js", diff --git a/scripts/check-minimal-linux-npm-artifact.js b/scripts/check-minimal-linux-npm-artifact.js new file mode 100644 index 0000000000..a486ae0227 --- /dev/null +++ b/scripts/check-minimal-linux-npm-artifact.js @@ -0,0 +1,252 @@ +#!/usr/bin/env node + +/** + * Installs the packed npm artifact into a minimal Ubuntu image that has no C++ + * toolchain (no make, no python3) and proves that: + * 1. `npm install` succeeds — i.e. node-pty resolved a prebuilt binary + * instead of falling back to `node-gyp rebuild`. + * 2. The installed CLI runs (`letta --help` produces real output). + * 3. node-pty loads and allocates an actual TTY. + * + * Regression guard for letta-ai/letta-acp#50, where `npx @letta-ai/letta-code` + * failed on minimal Ubuntu images with `Error: not found: make`. + * + * Usage: + * node scripts/check-minimal-linux-npm-artifact.js [--skip-build] [--platform=linux/arm64] + * + * Env: + * LETTA_CODE_MINIMAL_LINUX_ARTIFACT_SKIP_BUILD=1 reuse existing build output + * LETTA_CODE_MINIMAL_LINUX_ARTIFACT_PLATFORM docker platform override + */ + +import { execFileSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const tempDir = mkdtempSync(join(tmpdir(), "letta-code-minimal-linux-npm-")); +const imageTag = `letta-code-minimal-linux-npm:${process.pid}`; +const nodeVersion = "22.19.0"; + +const NODE_ARCH_BY_PLATFORM = { + "linux/amd64": "x64", + "linux/arm64": "arm64", +}; + +function resolvePlatform() { + const flag = process.argv.find((arg) => arg.startsWith("--platform=")); + const requested = + flag?.slice("--platform=".length) || + process.env.LETTA_CODE_MINIMAL_LINUX_ARTIFACT_PLATFORM || + (process.arch === "arm64" ? "linux/arm64" : "linux/amd64"); + if (!NODE_ARCH_BY_PLATFORM[requested]) { + throw new Error( + `Unsupported platform "${requested}". Expected one of: ${Object.keys(NODE_ARCH_BY_PLATFORM).join(", ")}`, + ); + } + return requested; +} + +const platform = resolvePlatform(); +const nodeArch = NODE_ARCH_BY_PLATFORM[platform]; +const skipBuild = + process.argv.includes("--skip-build") || + process.env.LETTA_CODE_MINIMAL_LINUX_ARTIFACT_SKIP_BUILD === "1"; + +function run(command, args, options = {}) { + console.log(`$ ${[command, ...args].join(" ")}`); + execFileSync(command, args, { + cwd: repoRoot, + stdio: "inherit", + timeout: 10 * 60 * 1000, + ...options, + }); +} + +function capture(command, args, options = {}) { + console.log(`$ ${[command, ...args].join(" ")}`); + return execFileSync(command, args, { + cwd: repoRoot, + encoding: "utf8", + stdio: ["ignore", "pipe", "inherit"], + timeout: 10 * 60 * 1000, + ...options, + }); +} + +function assertDockerAvailable() { + try { + execFileSync("docker", ["version"], { stdio: "ignore", timeout: 60_000 }); + } catch (error) { + throw new Error( + "Docker is required for check:minimal-linux-npm-artifact but `docker version` failed. " + + "Install Docker and make sure the daemon is running, then re-run. " + + `Underlying error: ${error instanceof Error ? error.message : String(error)}`, + ); + } +} + +// The image deliberately omits make/python3 so a node-gyp fallback fails loudly. +const dockerfile = ` +FROM ubuntu:24.04 + +ENV DEBIAN_FRONTEND=noninteractive + +RUN apt-get update \\ + && apt-get install -y --no-install-recommends ca-certificates curl xz-utils \\ + && rm -rf /var/lib/apt/lists/* + +RUN set -eux; \\ + curl -fsSL "https://nodejs.org/dist/v${nodeVersion}/node-v${nodeVersion}-linux-${nodeArch}.tar.xz" -o /tmp/node.tar.xz; \\ + tar -xJf /tmp/node.tar.xz -C /usr/local --strip-components=1; \\ + rm /tmp/node.tar.xz; \\ + node --version; \\ + npm --version; \\ + if command -v make >/dev/null 2>&1; then \\ + echo "make unexpectedly present in minimal image" >&2; \\ + exit 1; \\ + fi + +WORKDIR /workspace +`; + +const ptySmoke = ` +set -eux +if command -v make >/dev/null 2>&1; then + echo "make unexpectedly present before npm install" >&2 + exit 1 +fi +mkdir -p /workspace/app +cd /workspace/app +npm init -y >/dev/null +npm install --omit=dev --foreground-scripts /tmp/letta-code.tgz +./node_modules/.bin/letta --help >/tmp/letta-help.txt +grep -q letta /tmp/letta-help.txt || { + echo "letta --help produced no recognizable output" >&2 + cat /tmp/letta-help.txt >&2 + exit 1 +} +node <<'NODE' +const { existsSync } = require("node:fs"); +const { dirname, join } = require("node:path"); +const { createRequire } = require("node:module"); +const requireFromLetta = createRequire(require.resolve("@letta-ai/letta-code")); +const ptyRoot = dirname(requireFromLetta.resolve("node-pty/package.json")); + +// A prebuild directory for this platform is what keeps npm from shelling out to +// node-gyp; \`build/\` only exists when a source rebuild ran (it is not shipped +// in the tarball). Assert both so a future node-pty bump that drops prebuilds +// fails here rather than in a user's npx install. +const prebuildDir = join( + ptyRoot, + "prebuilds", + process.platform + "-" + process.arch, +); +if (!existsSync(prebuildDir)) { + console.error("node-pty is missing a prebuild at " + prebuildDir); + process.exit(1); +} +if (existsSync(join(ptyRoot, "build"))) { + console.error( + "node-pty was rebuilt from source (" + + join(ptyRoot, "build") + + " exists); the prebuild was not used", + ); + process.exit(1); +} +console.log("node-pty prebuild in use: " + prebuildDir); + +const pty = requireFromLetta("node-pty"); +const term = pty.spawn("/bin/bash", ["-lc", "test -t 0 && printf tty-ok"], { + name: "xterm-256color", + cols: 80, + rows: 24, + cwd: process.cwd(), + env: { ...process.env, TERM: "xterm-256color" }, +}); +let output = ""; +let settled = false; +const timeout = setTimeout(() => { + if (settled) return; + settled = true; + try { term.kill(); } catch {} + console.error("PTY smoke timed out; output=" + JSON.stringify(output)); + process.exit(1); +}, 20000); +term.onData((data) => { + output += data; +}); +term.onExit(({ exitCode }) => { + if (settled) return; + settled = true; + clearTimeout(timeout); + if (exitCode !== 0) { + console.error( + "PTY smoke exited with " + exitCode + "; output=" + JSON.stringify(output), + ); + process.exit(1); + } + if (!output.includes("tty-ok")) { + console.error( + "PTY smoke did not allocate a tty; output=" + JSON.stringify(output), + ); + process.exit(1); + } + console.log("PTY smoke passed"); +}); +NODE +`; + +try { + assertDockerAvailable(); + console.log(`Checking packed artifact on ${platform} (node ${nodeVersion})`); + if (skipBuild) { + console.log("$ bun run build (skipped; using existing build output)"); + } else { + run("bun", ["run", "build"]); + } + + const packOutput = capture("npm", [ + "pack", + "--pack-destination", + tempDir, + "--json", + ]); + const packed = JSON.parse(packOutput); + const filename = packed?.[0]?.filename; + if (!filename) { + throw new Error(`npm pack did not report a filename: ${packOutput}`); + } + const tarball = join(tempDir, filename); + + writeFileSync(join(tempDir, "Dockerfile"), dockerfile); + run("docker", [ + "build", + "--pull", + "--platform", + platform, + "-t", + imageTag, + tempDir, + ]); + run("docker", [ + "run", + "--rm", + "--platform", + platform, + "-v", + `${tarball}:/tmp/letta-code.tgz:ro`, + imageTag, + "sh", + "-c", + ptySmoke, + ]); + console.log(`Minimal Linux packed artifact check passed (${platform})`); +} finally { + try { + execFileSync("docker", ["rmi", "-f", imageTag], { stdio: "ignore" }); + } catch {} + rmSync(tempDir, { recursive: true, force: true }); +} diff --git a/scripts/check-windows-node-pty-artifact.js b/scripts/check-windows-node-pty-artifact.js new file mode 100644 index 0000000000..9eaf35a9c7 --- /dev/null +++ b/scripts/check-windows-node-pty-artifact.js @@ -0,0 +1,119 @@ +#!/usr/bin/env node + +/** + * Proves the globally-installed npm artifact has a working ConPTY on Windows: + * node-pty resolves from the installed package, ships a prebuild for this + * platform, and a spawned shell actually executes what we write to it. + * + * Requires `npm install -g ` to have run first (the CI npm install + * flow step does this). + */ + +import { execSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { createRequire } from "node:module"; +import { dirname, join } from "node:path"; + +// `set /a` is evaluated by cmd itself, so the expected value cannot appear in +// the ConPTY echo of the input line. Asserting on an echoed literal would pass +// even if the shell never executed the command. +const expression = "6*7"; +const expected = "42"; + +if (process.platform !== "win32") { + console.log( + "Windows node-pty artifact check skipped on non-Windows platform", + ); + process.exit(0); +} + +const globalRoot = ( + process.env.LETTA_CODE_GLOBAL_NODE_MODULES || + execSync("npm root -g", { encoding: "utf8" }) +).trim(); +const packageJson = join(globalRoot, "@letta-ai", "letta-code", "package.json"); + +if (!existsSync(packageJson)) { + throw new Error( + `Expected global @letta-ai/letta-code package at ${packageJson}. Run the npm install flow before this check.`, + ); +} + +const requireFromLetta = createRequire(packageJson); +const ptyRoot = dirname(requireFromLetta.resolve("node-pty/package.json")); +const prebuildDir = join( + ptyRoot, + "prebuilds", + `${process.platform}-${process.arch}`, +); + +// Windows runners have MSVC, so a node-pty version that dropped Windows +// prebuilds would still pass the PTY smoke here via `node-gyp rebuild` while +// breaking every user without a toolchain. Assert the prebuild explicitly. +if (!existsSync(prebuildDir)) { + throw new Error( + `node-pty is missing a prebuild at ${prebuildDir}. Users without a C++ toolchain would fall back to node-gyp.`, + ); +} +console.log(`node-pty prebuild present: ${prebuildDir}`); + +const pty = requireFromLetta("node-pty"); +const shell = process.env.ComSpec || "cmd.exe"; +const term = pty.spawn(shell, ["/d", "/q"], { + name: "xterm-256color", + cols: 80, + rows: 24, + cwd: process.cwd(), + env: { ...process.env, TERM: "xterm-256color" }, +}); + +let output = ""; +let settled = false; +let sent = false; + +function fail(message) { + if (settled) return; + settled = true; + clearTimeout(timeout); + try { + term.kill(); + } catch {} + console.error(`${message}; output=${JSON.stringify(output)}`); + process.exit(1); +} + +// Cold Windows runners can take several seconds to attach the console. +const timeout = setTimeout(() => { + fail("Windows node-pty smoke timed out"); +}, 20_000); + +term.onData((data) => { + output += data; + // ConPTY can drop input written before the console is attached, so wait for + // the first byte from the shell (its prompt) before writing. + if (!sent) { + sent = true; + term.write(`set /a ${expression}\r\n`); + term.write("exit\r\n"); + } +}); + +term.onExit(({ exitCode }) => { + if (settled) return; + if (exitCode !== 0) { + fail(`Windows node-pty smoke exited with ${exitCode}`); + return; + } + if (!output.includes(expected)) { + fail( + `Windows node-pty smoke did not see evaluated result ${expected} for ${expression}`, + ); + return; + } + settled = true; + clearTimeout(timeout); + // ConPTY can retain a native handle after the child exits. This is a one-shot + // check, so exit explicitly once the PTY proof is complete. + console.log("Windows node-pty PTY smoke passed"); + process.exit(0); +}); diff --git a/src/tools/impl/exec-command.ts b/src/tools/impl/exec-command.ts index f11b96ce7c..0c0ef99ae9 100644 --- a/src/tools/impl/exec-command.ts +++ b/src/tools/impl/exec-command.ts @@ -1,5 +1,9 @@ import { type ChildProcess, spawn } from "node:child_process"; import { getCurrentWorkingDirectory } from "@/runtime-context"; +import { + isNodePtyUnavailableError, + requireNodePty, +} from "@/utils/node-pty-loader"; import { noteExpectedWorktreeForLauncher } from "@/websocket/listener/worktree-ownership"; import { appendBackgroundProcessOutput, @@ -569,8 +573,24 @@ function spawnPtyProcess(params: { }; } - // eslint-disable-next-line @typescript-eslint/no-require-imports - const pty = require("node-pty") as NodePtyModule; + let pty: NodePtyModule; + try { + pty = requireNodePty() as NodePtyModule; + } catch (error) { + if (!isNodePtyUnavailableError(error)) throw error; + // No usable PTY on this install (musl/old-glibc prebuild). Run the command + // through a pipe instead of failing the tool call — output is still + // captured, but stdin is closed, so write_stdin will be rejected. The + // reason lands in the session output so the caller can see why `tty: true` + // was downgraded. + appendOutput( + `${(error as Error).message}\nFalling back to a non-TTY pipe for this command; stdin is closed.\n`, + "stderr", + ); + params.session.tty = false; + return spawnPipeProcess(params); + } + const ptyProcess = pty.spawn(executable, args, { name: "xterm-256color", cols: 80, diff --git a/src/utils/node-pty-loader.test.ts b/src/utils/node-pty-loader.test.ts new file mode 100644 index 0000000000..0cd2132917 --- /dev/null +++ b/src/utils/node-pty-loader.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, test } from "bun:test"; +import { + findUnusableNodePtyReason, + formatNodePtyUnavailableMessage, + isNodePtyUnavailableError, + loadNodePtyWith, +} from "@/utils/node-pty-loader"; + +describe("loadNodePtyWith", () => { + test("returns the loaded module untouched", () => { + const module = { spawn: () => {} }; + expect(loadNodePtyWith(() => module)).toBe(module); + }); + + test("tags load failures so callers can degrade to a pipe", () => { + const cause = new Error( + "Error relocating /app/node_modules/node-pty/prebuilds/linux-x64/pty.node: __libc_start_main: symbol not found", + ); + + let thrown: unknown; + try { + loadNodePtyWith(() => { + throw cause; + }); + } catch (error) { + thrown = error; + } + + expect(isNodePtyUnavailableError(thrown)).toBe(true); + expect((thrown as Error).cause).toBe(cause); + expect((thrown as Error).message).toContain("symbol not found"); + expect((thrown as Error).message).toContain( + "npm_config_build_from_source=true", + ); + }); + + test("handles non-Error throws", () => { + expect(() => + loadNodePtyWith(() => { + throw "boom"; + }), + ).toThrow(/boom/); + }); +}); + +describe("isNodePtyUnavailableError", () => { + test("does not match unrelated errors", () => { + expect(isNodePtyUnavailableError(new Error("spawn ENOENT"))).toBe(false); + expect(isNodePtyUnavailableError("nope")).toBe(false); + expect(isNodePtyUnavailableError(undefined)).toBe(false); + }); +}); + +describe("findUnusableNodePtyReason", () => { + const musl = { isMuslRuntime: () => true, hasSourceBuild: () => false }; + + test("refuses the glibc prebuild on a musl runtime", () => { + // Loading it succeeds and the first spawn() segfaults, which no try/catch + // can recover from — so the load has to be refused up front. + const reason = findUnusableNodePtyReason({ platform: "linux", ...musl }); + expect(reason).toContain("musl"); + expect(reason).toContain("npm_config_build_from_source=true"); + }); + + test("allows a locally compiled binding on musl", () => { + expect( + findUnusableNodePtyReason({ + platform: "linux", + isMuslRuntime: () => true, + hasSourceBuild: () => true, + }), + ).toBeNull(); + }); + + test("allows glibc linux", () => { + expect( + findUnusableNodePtyReason({ + platform: "linux", + isMuslRuntime: () => false, + hasSourceBuild: () => false, + }), + ).toBeNull(); + }); + + test("never probes libc off linux", () => { + for (const platform of ["darwin", "win32"]) { + expect( + findUnusableNodePtyReason({ + platform, + isMuslRuntime: () => { + throw new Error(`libc probed on ${platform}`); + }, + hasSourceBuild: () => false, + }), + ).toBeNull(); + } + }); +}); + +describe("formatNodePtyUnavailableMessage", () => { + test("includes the original failure and the rebuild hint", () => { + const message = formatNodePtyUnavailableMessage(new Error("no such file")); + expect(message).toContain("no such file"); + expect(message).toContain("compile node-pty from source"); + }); +}); diff --git a/src/utils/node-pty-loader.ts b/src/utils/node-pty-loader.ts new file mode 100644 index 0000000000..d054c439fb --- /dev/null +++ b/src/utils/node-pty-loader.ts @@ -0,0 +1,126 @@ +/** + * Guarded loader for node-pty. + * + * node-pty's install script (`scripts/prebuild.js`) only checks that + * `prebuilds/-/` *exists* — it never validates that the binary + * inside is usable, and its presence suppresses the `node-gyp rebuild` fallback. + * The linux-x64 prebuild is linked against glibc, which produces two distinct + * failures on systems the prebuild was not built for: + * + * - glibc older than the prebuild's floor (GLIBC_2.28): `require` throws a raw + * dynamic-link error. Catchable, but useless to the user as-is. + * - musl (Alpine): `require` *succeeds*, and the first `spawn()` call + * **segfaults the process**. Verified on node:22-alpine with + * node-pty@1.2.0-beta.14. Nothing in JS can catch that, so the only safe + * option is to refuse to load a glibc prebuild on a musl runtime. + * + * Both cases surface as a tagged error that callers detect with + * `isNodePtyUnavailableError` and handle by degrading (a pipe instead of a PTY) + * rather than crashing. + */ + +import { existsSync } from "node:fs"; +import { dirname, join } from "node:path"; + +const NODE_PTY_UNAVAILABLE_CODE = "LETTA_NODE_PTY_UNAVAILABLE"; + +const REBUILD_HINT = + "Reinstall with `npm_config_build_from_source=true` to compile node-pty from " + + "source (requires make, python3 and a C++ toolchain)."; + +function tagUnavailable(message: string, cause?: unknown): Error { + const error = new Error(message, cause === undefined ? undefined : { cause }); + (error as { code?: string }).code = NODE_PTY_UNAVAILABLE_CODE; + return error; +} + +export function formatNodePtyUnavailableMessage(cause: unknown): string { + const detail = cause instanceof Error ? cause.message : String(cause); + return `Failed to load node-pty: ${detail}\n${REBUILD_HINT}`; +} + +/** + * Runs `load` and normalizes any failure into a tagged error. Exported + * separately from `requireNodePty` so the failure path is testable without + * module mocking. + */ +export function loadNodePtyWith(load: () => unknown): unknown { + try { + return load(); + } catch (cause) { + throw tagUnavailable(formatNodePtyUnavailableMessage(cause), cause); + } +} + +/** + * Returns a reason string when node-pty must not be loaded at all, or null when + * loading is safe. Takes its probes as arguments so both branches are testable. + */ +export function findUnusableNodePtyReason(probes: { + platform: string; + isMuslRuntime: () => boolean; + hasSourceBuild: () => boolean; +}): string | null { + if (probes.platform !== "linux") return null; + if (!probes.isMuslRuntime()) return null; + // A locally compiled binding is linked against musl and is safe to use; only + // the shipped glibc prebuild is dangerous here. + if (probes.hasSourceBuild()) return null; + return ( + "node-pty's prebuilt binary is linked against glibc, but this system uses musl " + + "(Alpine). Calling into it segfaults the process, so the PTY is disabled.\n" + + REBUILD_HINT + ); +} + +/** + * musl builds of Node report no glibc version in their diagnostic report; glibc + * builds report e.g. "2.36". Same signal `detect-libc` uses. + */ +function isMuslRuntime(): boolean { + try { + const report = process.report?.getReport?.() as + | { header?: { glibcVersionRuntime?: string } } + | undefined; + if (!report?.header) return false; + return report.header.glibcVersionRuntime === undefined; + } catch { + return false; + } +} + +function hasSourceBuild(): boolean { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const packageJsonPath = require.resolve("node-pty/package.json"); + return existsSync( + join(dirname(packageJsonPath), "build", "Release", "pty.node"), + ); + } catch { + return false; + } +} + +let cachedUnusableReason: string | null | undefined; + +export function requireNodePty(): unknown { + if (cachedUnusableReason === undefined) { + cachedUnusableReason = findUnusableNodePtyReason({ + platform: process.platform, + isMuslRuntime, + hasSourceBuild, + }); + } + if (cachedUnusableReason) { + throw tagUnavailable(cachedUnusableReason); + } + // eslint-disable-next-line @typescript-eslint/no-require-imports + return loadNodePtyWith(() => require("node-pty")); +} + +export function isNodePtyUnavailableError(error: unknown): boolean { + return ( + error instanceof Error && + (error as { code?: string }).code === NODE_PTY_UNAVAILABLE_CODE + ); +} diff --git a/src/websocket/terminal-handler.ts b/src/websocket/terminal-handler.ts index a6b6281990..567d9e929e 100644 --- a/src/websocket/terminal-handler.ts +++ b/src/websocket/terminal-handler.ts @@ -11,6 +11,7 @@ import { existsSync } from "node:fs"; import * as os from "node:os"; import WebSocket from "ws"; +import { requireNodePty } from "@/utils/node-pty-loader"; const IS_BUN = typeof Bun !== "undefined"; @@ -209,8 +210,9 @@ function spawnNodePty( socket: WebSocket, ): TerminalSession { const terminalKey = getTerminalKey(connectionId, terminal_id); - // eslint-disable-next-line @typescript-eslint/no-require-imports - const pty = require("node-pty") as NodePtyModule; + // Throws a tagged, actionable error when the prebuilt binary is unloadable; + // handleTerminalSpawn forwards the message to the client as terminal_exited. + const pty = requireNodePty() as NodePtyModule; const handleData = makeOutputBatcher((data) => sendTerminalMessage(socket, { type: "terminal_output", terminal_id, data }),