Public: anyone can read this; the crew can change it.
forge.json run may be a compound command: cd sub && bun server.ts starts #10308
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10294-forge-json-run-may-be-a-compound-command → main 1 commit fork
4 files
changed +114
−3
src/apps/process.ts +6 −2
| @@ -20,6 +20,7 @@ import { isFeatureVarName } from "./feature_env.ts"; | ||
| 20 | 20 | import { err, ok, type Result } from "../kernel/result.ts"; |
| 21 | 21 | import { spellings, stripForgeDirs } from "../runs/paths.ts"; |
| 22 | 22 | import { seconds } from "./timing.ts"; |
| 23 | import { notFoundHint, shellCommand } from "./run_command.ts"; | |
| 23 | 24 | import { cellInstall, meteredInstall, prefillInstallCache, withLock } from "../checks/install.ts"; |
| 24 | 25 | import { |
| 25 | 26 | commandPid, |
| @@ -562,7 +563,7 @@ export async function startProcess( | ||
| 562 | 563 | }, |
| 563 | 564 | log, |
| 564 | 565 | ); |
| 565 | const command = `exec ${manifest.run}`; | |
| 566 | const command = shellCommand(manifest.run); | |
| 566 | 567 | const env = appEnv({ |
| 567 | 568 | port, |
| 568 | 569 | dataDir: input.dataDir, |
| @@ -652,7 +653,10 @@ export async function startProcess( | ||
| 652 | 653 | // The exit can be seen before the pumps deliver its last lines, the |
| 653 | 654 | // address-in-use one among them; a pipe a child holds open is not waited on. |
| 654 | 655 | await Promise.race([drained, Bun.sleep(DRAIN_CAP_MS)]); |
| 655 | const tail = recent.length > 0 ? ` — ${failureReason(recent)}` : ""; | |
| 656 | const hint = notFoundHint(recent); | |
| 657 | const tail = | |
| 658 | (recent.length > 0 ? ` — ${failureReason(recent)}` : "") + | |
| 659 | (hint === undefined ? "" : ` (${hint})`); | |
| 656 | 660 | return { |
| 657 | 661 | error: { stage: ready.error.stage, message: ready.error.message + tail }, |
| 658 | 662 | portTaken: ready.error.exited && recent.some((line) => ADDRESS_IN_USE_RE.test(line)), |
src/apps/run_command.ts added +32
| @@ -0,0 +1,32 @@ | ||
| 1 | /** | |
| 2 | * How a manifest's `run` (or `tty`) becomes a shell command. A simple | |
| 3 | * command is `exec`ed, so the app replaces the shell and signals reach it | |
| 4 | * directly. A compound one (`cd sub && bun server.ts`) is run as written: | |
| 5 | * `exec cd sub && …` would exec `cd`, which is no program. The whole tree | |
| 6 | * is still stopped — by process group, or by the cell's tree walk. | |
| 7 | */ | |
| 8 | ||
| 9 | /** Whether the command uses a shell operator, a subshell, or spans lines. */ | |
| 10 | export function isCompound(run: string): boolean { | |
| 11 | return /&&|\|\||[;|&\n()`]|\$\(/.test(run); | |
| 12 | } | |
| 13 | ||
| 14 | /** The shell text to spawn for a manifest command. */ | |
| 15 | export function shellCommand(run: string): string { | |
| 16 | return isCompound(run) ? run : `exec ${run}`; | |
| 17 | } | |
| 18 | ||
| 19 | const NOT_FOUND_RE = /exec: ([^\s:]+): not found/; | |
| 20 | ||
| 21 | /** When the shell says `exec: X: not found`, the fix, else undefined. */ | |
| 22 | export function notFoundHint(lines: readonly string[]): string | undefined { | |
| 23 | for (const line of lines) { | |
| 24 | const found = NOT_FOUND_RE.exec(line); | |
| 25 | if (found === null) continue; | |
| 26 | const word = found[1] ?? ""; | |
| 27 | return ["cd", "export", "source", ".", "set", "umask"].includes(word) | |
| 28 | ? `"${word}" is a shell builtin, not a program: chain it with a program, as in "cd sub && bun server.ts"` | |
| 29 | : `"${word}" is not installed or not on PATH: name an installed program in forge.json "run"`; | |
| 30 | } | |
| 31 | return undefined; | |
| 32 | } |
src/apps/tty.ts +2 −1
| @@ -18,6 +18,7 @@ | ||
| 18 | 18 | * one. |
| 19 | 19 | */ |
| 20 | 20 | |
| 21 | import { shellCommand } from "./run_command.ts"; | |
| 21 | 22 | import { killTree, openCell, type Cell } from "./contain.ts"; |
| 22 | 23 | import type { App as Forge } from "../app.ts"; |
| 23 | 24 | import { getAppBySlug, type AppRecord } from "../core/apps.ts"; |
| @@ -200,7 +201,7 @@ export async function runAppTty( | ||
| 200 | 201 | user === undefined ? "" : canWrite(forge.db, repo, user) ? "admin" : "user"; |
| 201 | 202 | // The terminal is held exactly as the app is (`apps/contain.ts`). |
| 202 | 203 | const cell = live.cell === undefined ? undefined : openCell(live.cell); |
| 203 | const program = startTerminal(`exec ${live.tty}`, { | |
| 204 | const program = startTerminal(shellCommand(live.tty), { | |
| 204 | 205 | terminal, |
| 205 | 206 | env, |
| 206 | 207 | cwd: live.dir, |
test/apps-compound-run.test.ts added +74
| @@ -0,0 +1,74 @@ | ||
| 1 | import { expect, test } from "bun:test"; | |
| 2 | import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { startProcess } from "../src/apps/process.ts"; | |
| 6 | import { isCompound, notFoundHint, shellCommand } from "../src/apps/run_command.ts"; | |
| 7 | import type { AppManifest } from "../src/apps/manifest.ts"; | |
| 8 | ||
| 9 | const manifest = (run: string): AppManifest => | |
| 10 | ({ run, install: "", health: "/", ready: 8 }) as AppManifest; | |
| 11 | ||
| 12 | const start = (dir: string, run: string, lines: string[] = []) => | |
| 13 | startProcess({ | |
| 14 | dir, | |
| 15 | dataDir: dir, | |
| 16 | manifest: manifest(run), | |
| 17 | env: "production", | |
| 18 | url: "http://app.test", | |
| 19 | commitOid: "0".repeat(40), | |
| 20 | forgeUrl: "http://forge.test", | |
| 21 | identityKey: "key", | |
| 22 | log: (l) => void lines.push(l), | |
| 23 | signal: new AbortController().signal, | |
| 24 | }); | |
| 25 | ||
| 26 | test("only a simple command is exec'ed", () => { | |
| 27 | // Invariant: operators are never prefixed with exec. | |
| 28 | for (const run of ["cd a && b", "a || b", "a; b", "a | b", "a\nb"]) { | |
| 29 | expect(isCompound(run)).toBe(true); | |
| 30 | expect(shellCommand(run)).toBe(run); | |
| 31 | } | |
| 32 | expect(shellCommand("bun server.ts")).toBe("exec bun server.ts"); | |
| 33 | }); | |
| 34 | ||
| 35 | test("exec: cd: not found names the fix", () => { | |
| 36 | expect(notFoundHint(["/bin/sh: 1: exec: cd: not found"])).toContain("cd sub && bun"); | |
| 37 | expect(notFoundHint(["/bin/sh: 1: exec: nope: not found"])).toContain("not installed"); | |
| 38 | expect(notFoundHint(["fine"])).toBeUndefined(); | |
| 39 | }); | |
| 40 | ||
| 41 | test("a compound run starts, answers, and is stopped with its whole tree", async () => { | |
| 42 | // Invariant: `cd sub && server` starts as written and stop() ends the server. | |
| 43 | const dir = await mkdtemp(join(tmpdir(), "forge-compound-")); | |
| 44 | try { | |
| 45 | await mkdir(join(dir, "sub")); | |
| 46 | await writeFile( | |
| 47 | join(dir, "sub", "server.ts"), | |
| 48 | "Bun.serve({port:Number(process.env.PORT),hostname:'127.0.0.1',fetch:()=>new Response('ok')})", | |
| 49 | ); | |
| 50 | const started = await start(dir, "cd sub && bun server.ts"); | |
| 51 | expect(started.ok).toBe(true); | |
| 52 | if (!started.ok) return; | |
| 53 | const { handle } = started.value; | |
| 54 | const res = await fetch(`http://127.0.0.1:${handle.port}/`); | |
| 55 | expect(await res.text()).toBe("ok"); | |
| 56 | await handle.stop(); | |
| 57 | await handle.exited; | |
| 58 | await expect(fetch(`http://127.0.0.1:${handle.port}/`)).rejects.toThrow(); | |
| 59 | } finally { | |
| 60 | await rm(dir, { recursive: true, force: true }); | |
| 61 | } | |
| 62 | }, 30_000); | |
| 63 | ||
| 64 | test("a missing program's start failure names the fix", async () => { | |
| 65 | const dir = await mkdtemp(join(tmpdir(), "forge-notfound-")); | |
| 66 | try { | |
| 67 | const started = await start(dir, "nonexistent-prog-xyz"); | |
| 68 | expect(started.ok).toBe(false); | |
| 69 | if (started.ok) return; | |
| 70 | expect(started.error.message).toContain("not installed or not on PATH"); | |
| 71 | } finally { | |
| 72 | await rm(dir, { recursive: true, force: true }); | |
| 73 | } | |
| 74 | }, 30_000); |