From 3b31a522d9707307f91e8118dfe58bb095782aff Mon Sep 17 00:00:00 2001 From: jwfing Date: Tue, 18 Aug 2026 17:36:08 -0700 Subject: [PATCH 1/3] =?UTF-8?q?feat:=20insta=20build=20=E2=80=94=20pre-pus?= =?UTF-8?q?h=20verification=20(plan=20+=20Dockerfile=20+=20static=20checks?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agents (and humans) currently learn a deploy would fail only after pushing a remote Fly build. `insta build [dir]` answers it locally and offline, with no login: the detection plan (builder, install/build/start commands, port with rationale, .env.example keys), the Dockerfile that would be used (the user's, or nixpacks-generated via `nixpacks build --out` — static, no Docker daemon), and checks that each carry a next action. --json for agents, --explain to include the Dockerfile content, exit 1 on a failed verdict. nixpacks is auto-installed best-effort (mirrors ensureFlyctl); without it the command degrades to Dockerfile-only verification. Co-Authored-By: Claude Fable 5 --- .claude/skills/developing-insta-cli/SKILL.md | 3 +- src/commands/build.ts | 247 +++++++++++++++++++ src/index.ts | 8 + src/nixpacks.ts | 98 ++++++++ test/build.test.ts | 147 +++++++++++ test/nixpacks.test.ts | 98 ++++++++ 6 files changed, 600 insertions(+), 1 deletion(-) create mode 100644 src/commands/build.ts create mode 100644 src/nixpacks.ts create mode 100644 test/build.test.ts create mode 100644 test/nixpacks.test.ts diff --git a/.claude/skills/developing-insta-cli/SKILL.md b/.claude/skills/developing-insta-cli/SKILL.md index 89bf590a..5419695c 100644 --- a/.claude/skills/developing-insta-cli/SKILL.md +++ b/.claude/skills/developing-insta-cli/SKILL.md @@ -26,9 +26,10 @@ npx tsx src/index.ts --help # run the CLI from source | `index.ts` | commander program — registers every command | | `api.ts` | typed platform-API client (auth headers, token refresh, error mapping) | | `config.ts` | global `~/.insta/config.json` (apiUrl + tokens + user) · project `./.insta/project.json` (projectId / orgId / current branch) | -| `commands/` | one file per command group: `auth` `org` `project` `services` `branch` `secrets` `deploy` `compute` `upgrade` `metrics` (+`logs`) `billing` `govern` (policy/approvals) `manifest` `observe` | +| `commands/` | one file per command group: `auth` `org` `project` `services` `branch` `secrets` `build` `deploy` `compute` `upgrade` `metrics` (+`logs`) `billing` `govern` (policy/approvals) `manifest` `observe` | | `observe/` | local `insta observe` hook — `scanner.ts` (AWS/GitHub/Stripe/LLM/DB cred detection), `hook.ts`, `install.ts`, `report.ts` (→ platform event ingest) | | `flyctl-build.ts` | source-directory deploy build glue (Fly build context) | +| `nixpacks.ts` | nixpacks glue for `insta build` — plan detection + Dockerfile generation (no Docker daemon) | | `ensure-skills.ts` | installs/refreshes the agent skills into the user's project | | `util.ts` | shared helpers | diff --git a/src/commands/build.ts b/src/commands/build.ts new file mode 100644 index 00000000..253ba532 --- /dev/null +++ b/src/commands/build.ts @@ -0,0 +1,247 @@ +// `insta build` — pre-push verification of a source directory: the detection plan (what would +// build), the Dockerfile that would be used (yours, or nixpacks-generated), and static checks. +// Entirely local and offline: no login, no project link, nothing pushed or deployed. Phase 1 is +// static-only — no Docker daemon involved. +import { resolve, join } from 'node:path' +import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs' +import { info, printJson, die } from '../util.js' +import { dockerfileExposedPort } from './deploy.js' +import { nixpacksPlan, nixpacksGeneratedDockerfile, ensureNixpacks, quietRunner, type NixpacksPlan } from '../nixpacks.js' +import type { BuildRunner } from '../flyctl-build.js' + +export type BuildCheck = { + id: string + severity: 'critical' | 'warning' | 'info' + status: 'pass' | 'fail' | 'skip' + title: string + detail?: string + nextAction?: string +} + +export type BuildReport = { + dir: string + plan: { + builder: 'dockerfile' | 'nixpacks' | null + providers: string[] + installCommand?: string + buildCommand?: string + startCommand?: string + port?: number + portRationale: string + envKeys: string[] + } + dockerfile: { source: 'user' | 'nixpacks' | null; path?: string; content?: string } + checks: BuildCheck[] + verdict: 'deployable' | 'needs-attention' | 'failed' +} + +// A failed critical sinks the build; a failed warning deserves attention; skips are not failures. +export function computeVerdict(checks: BuildCheck[]): BuildReport['verdict'] { + const failed = checks.filter((c) => c.status === 'fail') + if (failed.some((c) => c.severity === 'critical')) return 'failed' + if (failed.length > 0) return 'needs-attention' + return 'deployable' +} + +// Port resolution mirrors deploy.ts: an explicit --port wins, else the Dockerfile's EXPOSE. The +// rationale string is part of the output — every plan line says why (the `fly launch` pattern). +export function inferPort(flag: string | undefined, dockerfile: string | undefined): { port?: number; rationale: string } { + if (flag) return { port: Number(flag), rationale: '--port flag' } + const exposed = dockerfile ? dockerfileExposedPort(dockerfile) : undefined + if (exposed) return { port: exposed, rationale: `Dockerfile EXPOSE ${exposed}` } + return { port: undefined, rationale: 'not detected — deploy defaults to 8080' } +} + +// Keys the app expects, from .env.example — surfaced so an agent can `insta secrets set` them +// before the first deploy instead of discovering missing config from runtime crashes. +export function envKeysFromDotEnvExample(content: string): string[] { + const keys: string[] = [] + for (const line of content.split('\n')) { + if (line.trim().startsWith('#')) continue + const key = /^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=/.exec(line)?.[1] + if (key) keys.push(key) + } + return keys +} + +const CONTEXT_WARN_BYTES = 100 * 1024 * 1024 +const WALK_CAP = 50_000 // entries; beyond this the size is reported as a floor + +type ContextStats = { totalBytes: number; nodeModulesBytes: number; hasNodeModules: boolean; nodeModulesIgnored: boolean } + +function contextStats(dir: string): ContextStats { + const ignoreFile = join(dir, '.dockerignore') + const ignoreLines = existsSync(ignoreFile) + ? readFileSync(ignoreFile, 'utf8').split('\n').map((l) => l.trim()).filter((l) => l && !l.startsWith('#')) + : [] + const nodeModulesIgnored = ignoreLines.some((l) => ['node_modules', 'node_modules/', '/node_modules', '**/node_modules'].includes(l)) + let totalBytes = 0 + let nodeModulesBytes = 0 + let hasNodeModules = false + let seen = 0 + const walk = (d: string, inNodeModules: boolean) => { + let entries: string[] + try { entries = readdirSync(d) } catch { return } + for (const name of entries) { + if (seen++ > WALK_CAP) return + if (name === '.git') continue + const p = join(d, name) + let st + try { st = statSync(p) } catch { continue } + const isNm = inNodeModules || name === 'node_modules' + if (name === 'node_modules' && st.isDirectory()) hasNodeModules = true + if (st.isDirectory()) walk(p, isNm) + else { + totalBytes += st.size + if (isNm) nodeModulesBytes += st.size + } + } + } + walk(dir, false) + return { totalBytes, nodeModulesBytes, hasNodeModules, nodeModulesIgnored } +} + +const mb = (bytes: number): string => `${(bytes / 1024 / 1024).toFixed(1)} MB` + +export async function buildReport( + dirArg: string, + opts: { port?: string }, + deps: { runner: BuildRunner; nixpacksAvailable: boolean }, +): Promise { + const dir = resolve(process.cwd(), dirArg) + const userDockerfilePath = join(dir, 'Dockerfile') + const hasUserDockerfile = existsSync(userDockerfilePath) + + let dockerfile: BuildReport['dockerfile'] = { source: null } + let np: NixpacksPlan | null = null + let dockerfileDetail = '' + if (hasUserDockerfile) { + dockerfile = { source: 'user', path: userDockerfilePath, content: readFileSync(userDockerfilePath, 'utf8') } + dockerfileDetail = 'using the Dockerfile in the directory' + } else if (!deps.nixpacksAvailable) { + dockerfileDetail = 'no Dockerfile in the directory, and nixpacks is not installed to generate one' + } else { + np = await nixpacksPlan(dir, deps.runner) + if (!np) { + dockerfileDetail = 'no Dockerfile, and nixpacks matched no provider for this directory' + } else { + const generated = await nixpacksGeneratedDockerfile(dir, deps.runner) + if (generated) { + dockerfile = { source: 'nixpacks', content: generated } + dockerfileDetail = `generated by nixpacks (providers: ${np.providers.join(', ') || 'none'})` + } else { + dockerfileDetail = 'nixpacks detected the app but could not generate a Dockerfile' + } + } + } + + const builder: BuildReport['plan']['builder'] = hasUserDockerfile ? 'dockerfile' : np ? 'nixpacks' : null + const { port, rationale } = inferPort(opts.port, dockerfile.content) + const envExample = join(dir, '.env.example') + const envKeys = existsSync(envExample) ? envKeysFromDotEnvExample(readFileSync(envExample, 'utf8')) : [] + + const checks: BuildCheck[] = [] + checks.push({ + id: 'dockerfile', + severity: 'critical', + status: dockerfile.source ? 'pass' : 'fail', + title: 'Dockerfile', + detail: dockerfileDetail, + ...(dockerfile.source ? {} : { nextAction: `add a Dockerfile at ${userDockerfilePath}, or install nixpacks (https://nixpacks.com/docs/install) so insta can generate one` }), + }) + + if (builder === 'dockerfile') { + const hasCmd = /^\s*(CMD|ENTRYPOINT)\s/im.test(dockerfile.content ?? '') + checks.push({ + id: 'start-command', + severity: 'warning', + status: hasCmd ? 'pass' : 'fail', + title: 'start command', + detail: hasCmd ? 'Dockerfile has a CMD/ENTRYPOINT' : 'no CMD or ENTRYPOINT in the Dockerfile — the base image must supply one', + ...(hasCmd ? {} : { nextAction: 'add a CMD (or ENTRYPOINT) so the image starts your app' }), + }) + } else if (builder === 'nixpacks') { + checks.push({ + id: 'start-command', + severity: 'critical', + status: np?.startCommand ? 'pass' : 'fail', + title: 'start command', + detail: np?.startCommand ?? 'nixpacks found no start command — the built image would not run', + ...(np?.startCommand ? {} : { nextAction: 'define one (e.g. a package.json "start" script, or a Procfile)' }), + }) + } else { + checks.push({ id: 'start-command', severity: 'critical', status: 'skip', title: 'start command', detail: 'skipped — no builder' }) + } + + checks.push({ + id: 'port', + severity: 'warning', + status: port !== undefined ? 'pass' : 'fail', + title: 'port', + detail: port !== undefined ? `${port} (${rationale})` : rationale, + ...(port !== undefined ? {} : { nextAction: 'pass --port (or add EXPOSE to the Dockerfile) — a port mismatch is the #1 deploy mistake' }), + }) + + const ctx = contextStats(dir) + const shipsNodeModules = ctx.hasNodeModules && !ctx.nodeModulesIgnored + const tooBig = ctx.totalBytes > CONTEXT_WARN_BYTES + checks.push({ + id: 'context', + severity: 'warning', + status: shipsNodeModules || tooBig ? 'fail' : 'pass', + title: 'build context', + detail: shipsNodeModules + ? `node_modules (${mb(ctx.nodeModulesBytes)}) would ship in the ${mb(ctx.totalBytes)} build context` + : `${mb(ctx.totalBytes)}${tooBig ? ' — large contexts make remote builds slow' : ''}`, + ...(shipsNodeModules || tooBig ? { nextAction: 'add a .dockerignore (node_modules, build artifacts, secrets)' } : {}), + }) + + return { + dir, + plan: { builder, providers: np?.providers ?? [], installCommand: np?.installCommand, buildCommand: np?.buildCommand, startCommand: np?.startCommand, port, portRationale: rationale, envKeys }, + dockerfile, + checks, + verdict: computeVerdict(checks), + } +} + +const MARK = { pass: '✓', fail: '✗', skip: '·' } as const + +export function renderReport(r: BuildReport, explain: boolean): string[] { + const lines: string[] = [] + lines.push(`plan for ${r.dir}:`) + lines.push(` builder: ${r.plan.builder ?? 'none'}${r.plan.providers.length ? ` (providers: ${r.plan.providers.join(', ')})` : ''}`) + if (r.plan.installCommand) lines.push(` install: ${r.plan.installCommand}`) + if (r.plan.buildCommand) lines.push(` build: ${r.plan.buildCommand}`) + if (r.plan.startCommand) lines.push(` start: ${r.plan.startCommand}`) + lines.push(` port: ${r.plan.port ?? '?'} (${r.plan.portRationale})`) + if (r.plan.envKeys.length) lines.push(` env keys (.env.example): ${r.plan.envKeys.join(', ')}`) + lines.push('checks:') + for (const c of r.checks) { + const mark = c.status === 'fail' && c.severity !== 'critical' ? '⚠' : MARK[c.status] + lines.push(` ${mark} ${c.title}${c.detail ? ` — ${c.detail}` : ''}`) + if (c.status === 'fail' && c.nextAction) lines.push(` → ${c.nextAction}`) + } + if (explain && r.dockerfile.content) { + lines.push(`dockerfile (${r.dockerfile.source}):`) + for (const l of r.dockerfile.content.trimEnd().split('\n')) lines.push(` ${l}`) + } + lines.push(`verdict: ${r.verdict}`) + return lines +} + +export async function build(dirArg: string | undefined, opts: { explain?: boolean; json?: boolean; port?: string }): Promise { + const dir = dirArg ?? '.' + const abs = resolve(process.cwd(), dir) + if (!existsSync(abs)) die(`no such directory: ${abs}`) + // Only reach for nixpacks when there is no Dockerfile to verify. + const nixpacksAvailable = existsSync(join(abs, 'Dockerfile')) ? false : await ensureNixpacks() + const report = await buildReport(dir, opts, { runner: quietRunner, nixpacksAvailable }) + if (opts.json) { + // Dockerfile content is included with --explain; without it the report stays small. + printJson(opts.explain ? report : { ...report, dockerfile: { ...report.dockerfile, content: undefined } }) + } else { + for (const line of renderReport(report, !!opts.explain)) info(line) + } + if (report.verdict === 'failed') process.exitCode = 1 +} diff --git a/src/index.ts b/src/index.ts index e2a918a3..fc6a474a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -17,6 +17,7 @@ import { resolveServiceArgs, serviceArgsDeps } from './resolve-service.js' import * as regions from './commands/regions.js' import * as secretsCmd from './commands/secrets.js' import { deploy } from './commands/deploy.js' +import { build } from './commands/build.js' import * as computeCmd from './commands/compute.js' import * as dbCmd from './commands/db.js' import * as storageCmd from './commands/storage.js' @@ -165,6 +166,13 @@ sec.command('unset ').description('Remove a user secret') sec.command('tree').description('Show secrets as project → branch → service → secrets').option('--json') .action(guard((o) => secretsCmd.secretsTree(o))) +// ---- build (pre-push verification — local, offline, deploys nothing) ---- +program.command('build [dir]').description('Verify a source directory would build before deploying: detection plan + the Dockerfile that would be used (yours, or nixpacks-generated) + static checks. Local and offline — no login needed, nothing pushed. Exit 1 when the verdict is failed') + .option('--explain', 'include the Dockerfile content in the output') + .option('--port

', 'port the app listens on (else the Dockerfile EXPOSE)') + .option('--json') + .action(guard((dir, o) => build(dir, o))) + // ---- deploy ---- program.command('deploy [dir]').description('Deploy a source directory (built remotely on Fly) or a prebuilt --image to a branch compute group') .option('--image ', 'prebuilt container image to deploy (instead of a source dir)').option('--branch ').option('--group ').option('--port

') diff --git a/src/nixpacks.ts b/src/nixpacks.ts new file mode 100644 index 00000000..beae7145 --- /dev/null +++ b/src/nixpacks.ts @@ -0,0 +1,98 @@ +// nixpacks glue for `insta build`: framework detection (`nixpacks plan`) and Dockerfile +// generation (`nixpacks build --out`) — both static, neither touches a Docker daemon. +// Same injectable-runner pattern as flyctl-build.ts. +import { spawn } from 'node:child_process' +import { mkdtempSync, readFileSync, rmSync, existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { info } from './util.js' +import type { BuildRunner } from './flyctl-build.js' + +export type NixpacksPlan = { + providers: string[] + installCommand?: string + buildCommand?: string + startCommand?: string +} + +// Capture-only runner: plan output is parsed (not shown), and a wedged binary must not hang the +// command — kill after 30s and let the caller degrade. +export const quietRunner: BuildRunner = (cmd, args, opts) => + new Promise((resolve) => { + const child = spawn(cmd, args, { cwd: opts.cwd, env: opts.env, stdio: ['ignore', 'pipe', 'pipe'] }) + let output = '' + const timer = setTimeout(() => child.kill('SIGKILL'), 30_000) + child.stdout?.on('data', (b) => { output += b.toString() }) + child.stderr?.on('data', (b) => { output += b.toString() }) + child.on('error', (err) => { clearTimeout(timer); resolve({ code: -1, output: `${output}\n${err.message}` }) }) + child.on('close', (code) => { clearTimeout(timer); resolve({ code: code ?? -1, output }) }) + }) + +export function parseNixpacksPlan(text: string): NixpacksPlan | null { + try { + const j = JSON.parse(text) + // Real plans (nixpacks ≥1.x) leave `providers` empty and name the matched provider(s) in the + // NIXPACKS_METADATA build variable instead. + const listed = Array.isArray(j.providers) ? j.providers : [] + const meta = typeof j.variables?.NIXPACKS_METADATA === 'string' + ? j.variables.NIXPACKS_METADATA.split(',').map((s: string) => s.trim()).filter(Boolean) + : [] + return { + providers: listed.length ? listed : meta, + installCommand: j.phases?.install?.cmds?.join(' && ') || undefined, + buildCommand: j.phases?.build?.cmds?.join(' && ') || undefined, + startCommand: j.start?.cmd || undefined, + } + } catch { + return null + } +} + +export async function nixpacksPlan(dir: string, run: BuildRunner = quietRunner): Promise { + const { code, output } = await run('nixpacks', ['plan', dir], { cwd: dir, env: process.env as Record }) + if (code !== 0) return null + return parseNixpacksPlan(output) +} + +// `nixpacks build --out

` generates .nixpacks/Dockerfile and skips Docker entirely. The out +// dir is a temp dir so the user's source tree stays clean (the platform writes into the source +// dir because it builds from a scratch clone — a local verify must not). +export async function nixpacksGeneratedDockerfile(dir: string, run: BuildRunner = quietRunner): Promise { + const out = mkdtempSync(join(tmpdir(), 'insta-nixpacks-')) + try { + const { code } = await run('nixpacks', ['build', dir, '--out', out], { cwd: dir, env: process.env as Record }) + if (code !== 0) return null + const generated = join(out, '.nixpacks', 'Dockerfile') + return existsSync(generated) ? readFileSync(generated, 'utf8') : null + } finally { + rmSync(out, { recursive: true, force: true }) + } +} + +// Best-effort: make the nixpacks CLI available (mirrors ensureFlyctl). Returns availability — +// `insta build` degrades to Dockerfile-only checks when it stays missing. +export async function ensureNixpacks(): Promise { + const ok = (cmd: string, args: string[], inherit = false) => + new Promise((resolve) => { + const p = spawn(cmd, args, { stdio: inherit ? 'inherit' : 'ignore' }) + p.on('error', () => resolve(false)) + p.on('close', (code) => resolve(code === 0)) + }) + try { + if (await ok('nixpacks', ['--version'])) return true + if (process.platform === 'darwin' && (await ok('brew', ['--version']))) { + info('nixpacks not found — installing with `brew install nixpacks` (one-time)…') + if (await ok('brew', ['install', 'nixpacks'], true)) return ok('nixpacks', ['--version']) + } + if (process.platform === 'linux') { + info('nixpacks not found — installing (one-time)…') + if (await ok('sh', ['-c', 'curl -fsSL https://nixpacks.com/install.sh | bash'], true)) { + if (await ok('nixpacks', ['--version'])) return true + } + } + info('nixpacks not found — install it for framework detection: https://nixpacks.com/docs/install') + return false + } catch { + return false + } +} diff --git a/test/build.test.ts b/test/build.test.ts new file mode 100644 index 00000000..a32a4086 --- /dev/null +++ b/test/build.test.ts @@ -0,0 +1,147 @@ +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, it, expect } from 'vitest' +import { + computeVerdict, inferPort, envKeysFromDotEnvExample, buildReport, renderReport, + type BuildCheck, +} from '../src/commands/build.js' +import type { BuildRunner } from '../src/flyctl-build.js' + +const check = (over: Partial): BuildCheck => + ({ id: 'x', severity: 'info', status: 'pass', title: 't', ...over }) + +describe('computeVerdict', () => { + it('deployable when nothing failed (skips are not failures)', () => { + expect(computeVerdict([check({}), check({ status: 'skip', severity: 'critical' })])).toBe('deployable') + }) + it('needs-attention on a failed warning', () => { + expect(computeVerdict([check({ status: 'fail', severity: 'warning' })])).toBe('needs-attention') + }) + it('failed on a failed critical (beats warnings)', () => { + expect(computeVerdict([ + check({ status: 'fail', severity: 'warning' }), + check({ status: 'fail', severity: 'critical' }), + ])).toBe('failed') + }) +}) + +describe('inferPort', () => { + it('an explicit --port wins', () => { + expect(inferPort('3000', 'EXPOSE 5000')).toEqual({ port: 3000, rationale: '--port flag' }) + }) + it('falls back to the Dockerfile EXPOSE', () => { + expect(inferPort(undefined, 'FROM x\nEXPOSE 5000')).toEqual({ port: 5000, rationale: 'Dockerfile EXPOSE 5000' }) + }) + it('reports the platform default when nothing declares a port', () => { + expect(inferPort(undefined, undefined)).toEqual({ port: undefined, rationale: 'not detected — deploy defaults to 8080' }) + }) +}) + +describe('envKeysFromDotEnvExample', () => { + it('extracts keys, ignoring comments, blanks, and export prefixes', () => { + expect(envKeysFromDotEnvExample('DATABASE_URL=postgres://x\n# API note\n\nexport API_KEY="y"\nnot a kv line\n')) + .toEqual(['DATABASE_URL', 'API_KEY']) + }) +}) + +// A runner that answers both nixpacks calls: `plan ` prints a plan, `build --out ` +// writes the generated Dockerfile. +const nixpacksFake = (planJson: string, dockerfile = 'FROM node:18\nCMD ["npm","start"]\n'): BuildRunner => + async (_cmd, args) => { + if (args[0] === 'plan') return { code: 0, output: planJson } + const out = args[args.indexOf('--out') + 1] + mkdirSync(join(out, '.nixpacks'), { recursive: true }) + writeFileSync(join(out, '.nixpacks', 'Dockerfile'), dockerfile) + return { code: 0, output: 'saved' } + } + +const PLAN = JSON.stringify({ + providers: ['node'], + phases: { install: { cmds: ['npm ci'] }, build: { cmds: ['npm run build'] } }, + start: { cmd: 'npm run start' }, +}) + +describe('buildReport', () => { + it('user Dockerfile: dockerfile builder, EXPOSE port, no nixpacks invocation', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'Dockerfile'), 'FROM node:20\nEXPOSE 3000\nCMD ["npm","start"]\n') + const runner: BuildRunner = async () => { throw new Error('nixpacks must not run when a Dockerfile exists') } + const r = await buildReport(dir, {}, { runner, nixpacksAvailable: true }) + expect(r.plan.builder).toBe('dockerfile') + expect(r.dockerfile.source).toBe('user') + expect(r.plan.port).toBe(3000) + expect(r.plan.portRationale).toBe('Dockerfile EXPOSE 3000') + expect(r.verdict).toBe('deployable') + }) + + it('no Dockerfile: nixpacks detects, generates, and the report carries the plan + content', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'package.json'), '{"name":"app","scripts":{"start":"node i.js"}}') + writeFileSync(join(dir, '.env.example'), 'DATABASE_URL=postgres://x\n') + const r = await buildReport(dir, { port: '3000' }, { runner: nixpacksFake(PLAN), nixpacksAvailable: true }) + expect(r.plan.builder).toBe('nixpacks') + expect(r.plan.providers).toEqual(['node']) + expect(r.plan.startCommand).toBe('npm run start') + expect(r.plan.envKeys).toEqual(['DATABASE_URL']) + expect(r.dockerfile.source).toBe('nixpacks') + expect(r.dockerfile.content).toContain('FROM node:18') + expect(r.verdict).toBe('deployable') + }) + + it('no Dockerfile and no nixpacks: critical failure with a next action', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + const runner: BuildRunner = async () => ({ code: 1, output: 'unused' }) + const r = await buildReport(dir, {}, { runner, nixpacksAvailable: false }) + expect(r.verdict).toBe('failed') + expect(r.dockerfile.source).toBeNull() + const df = r.checks.find((c) => c.id === 'dockerfile') + expect(df?.status).toBe('fail') + expect(df?.severity).toBe('critical') + expect(df?.nextAction).toContain('Dockerfile') + }) + + it('nixpacks plan without a start command is a critical failure', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'package.json'), '{"name":"app"}') + const noStart = JSON.stringify({ providers: ['node'], phases: { install: { cmds: ['npm ci'] } } }) + const r = await buildReport(dir, { port: '3000' }, { runner: nixpacksFake(noStart), nixpacksAvailable: true }) + expect(r.checks.find((c) => c.id === 'start-command')?.status).toBe('fail') + expect(r.verdict).toBe('failed') + }) + + it('warns when node_modules would ship in the build context', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'Dockerfile'), 'FROM node:20\nEXPOSE 3000\nCMD ["x"]\n') + mkdirSync(join(dir, 'node_modules', 'left-pad'), { recursive: true }) + writeFileSync(join(dir, 'node_modules', 'left-pad', 'index.js'), 'x'.repeat(1024)) + const runner: BuildRunner = async () => { throw new Error('unused') } + const r = await buildReport(dir, {}, { runner, nixpacksAvailable: true }) + expect(r.checks.find((c) => c.id === 'context')?.status).toBe('fail') + expect(r.verdict).toBe('needs-attention') + }) + + it('a .dockerignore covering node_modules silences the context warning', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'Dockerfile'), 'FROM node:20\nEXPOSE 3000\nCMD ["x"]\n') + writeFileSync(join(dir, '.dockerignore'), 'node_modules\n.git\n') + mkdirSync(join(dir, 'node_modules'), { recursive: true }) + const runner: BuildRunner = async () => { throw new Error('unused') } + const r = await buildReport(dir, {}, { runner, nixpacksAvailable: true }) + expect(r.checks.find((c) => c.id === 'context')?.status).toBe('pass') + }) +}) + +describe('renderReport', () => { + it('prints plan lines with rationale, check marks, and the verdict', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'package.json'), '{}') + const r = await buildReport(dir, { port: '3000' }, { runner: nixpacksFake(PLAN), nixpacksAvailable: true }) + const text = renderReport(r, false).join('\n') + expect(text).toContain('nixpacks') + expect(text).toContain('--port flag') + expect(text).toContain('verdict: deployable') + expect(text).not.toContain('FROM node:18') // Dockerfile content only with --explain + expect(renderReport(r, true).join('\n')).toContain('FROM node:18') + }) +}) diff --git a/test/nixpacks.test.ts b/test/nixpacks.test.ts new file mode 100644 index 00000000..8f683aa9 --- /dev/null +++ b/test/nixpacks.test.ts @@ -0,0 +1,98 @@ +import { mkdtempSync, mkdirSync, writeFileSync, existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, it, expect } from 'vitest' +import { parseNixpacksPlan, nixpacksPlan, nixpacksGeneratedDockerfile } from '../src/nixpacks.js' +import type { BuildRunner } from '../src/flyctl-build.js' + +// The documented `nixpacks plan` output shape (providers + phases + start). +const PLAN_JSON = JSON.stringify({ + providers: ['node'], + buildImage: 'ghcr.io/railwayapp/nixpacks:latest', + phases: { + setup: { nixPkgs: ['nodejs-18_x'] }, + install: { cmds: ['npm ci'], dependsOn: ['setup'] }, + build: { cmds: ['npm run build'], dependsOn: ['install'] }, + }, + start: { cmd: 'npm run start' }, +}) + +describe('parseNixpacksPlan', () => { + it('extracts providers, install/build commands, and the start command', () => { + expect(parseNixpacksPlan(PLAN_JSON)).toEqual({ + providers: ['node'], + installCommand: 'npm ci', + buildCommand: 'npm run build', + startCommand: 'npm run start', + }) + }) + + it('tolerates missing phases and start (partial detection)', () => { + expect(parseNixpacksPlan(JSON.stringify({ providers: ['staticfile'] }))).toEqual({ + providers: ['staticfile'], + installCommand: undefined, + buildCommand: undefined, + startCommand: undefined, + }) + }) + + it('returns null on non-JSON output', () => { + expect(parseNixpacksPlan('error: no providers matched')).toBeNull() + }) + + it('falls back to NIXPACKS_METADATA when the providers array is empty (nixpacks ≥1.x real output)', () => { + const real = JSON.stringify({ + providers: [], + variables: { CI: 'true', NIXPACKS_METADATA: 'node,python' }, + phases: { install: { cmds: ['npm i'] } }, + start: { cmd: 'npm run start' }, + }) + expect(parseNixpacksPlan(real)?.providers).toEqual(['node', 'python']) + }) +}) + +describe('nixpacksPlan', () => { + it('runs `nixpacks plan ` and parses the JSON it prints', async () => { + const seen: { cmd?: string; args?: string[] } = {} + const run: BuildRunner = async (cmd, args) => { + seen.cmd = cmd; seen.args = args + return { code: 0, output: PLAN_JSON } + } + const plan = await nixpacksPlan('/some/app', run) + expect(seen.cmd).toBe('nixpacks') + expect(seen.args).toEqual(['plan', '/some/app']) + expect(plan?.startCommand).toBe('npm run start') + }) + + it('returns null when nixpacks exits non-zero (caller degrades gracefully)', async () => { + const run: BuildRunner = async () => ({ code: 1, output: 'Error: no providers found' }) + expect(await nixpacksPlan('/some/app', run)).toBeNull() + }) +}) + +describe('nixpacksGeneratedDockerfile', () => { + it('runs `nixpacks build --out ` and reads the generated Dockerfile', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-src-')) + const seen: { args?: string[] } = {} + const run: BuildRunner = async (cmd, args) => { + seen.args = args + // nixpacks writes /.nixpacks/Dockerfile; the fake does the same + const out = args[args.indexOf('--out') + 1] + mkdirSync(join(out, '.nixpacks'), { recursive: true }) + writeFileSync(join(out, '.nixpacks', 'Dockerfile'), 'FROM node:18\nCMD ["npm","start"]\n') + return { code: 0, output: 'saved output to directory' } + } + const content = await nixpacksGeneratedDockerfile(dir, run) + expect(seen.args?.slice(0, 2)).toEqual(['build', dir]) + expect(seen.args).toContain('--out') + expect(content).toContain('FROM node:18') + // the temp out dir is cleaned up — nothing is written into the user's source dir + expect(existsSync(join(dir, '.nixpacks'))).toBe(false) + }) + + it('returns null when generation fails', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-src-')) + const run: BuildRunner = async () => ({ code: 1, output: 'boom' }) + expect(await nixpacksGeneratedDockerfile(dir, run)).toBeNull() + }) +}) From dc072ed87c97451dc20ca3638cd748cd2d054bfa Mon Sep 17 00:00:00 2001 From: jwfing Date: Tue, 18 Aug 2026 21:17:53 -0700 Subject: [PATCH 2/3] =?UTF-8?q?review=20round=201:=20keep=20--json=20stdou?= =?UTF-8?q?t=20pure=20=E2=80=94=20probe=20nixpacks,=20never=20install=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verifier no longer auto-installs nixpacks (no curl|bash / brew from a command advertised as local and offline, and no stdout chatter corrupting the --json agent contract on the fresh-machine path): nixpacksAvailable() is a silent --version probe and the report's nextAction carries the install hint. Also from review: --port validated as an integer in 1..65535, dockerignored node_modules no longer counted in the context size, the walk cap marks the stats truncated (conservative warn instead of a silent undercount), and a non-directory argument dies cleanly. contextCheck/jsonReport extracted pure and covered (10 new tests). Co-Authored-By: Claude Fable 5 --- src/commands/build.ts | 82 +++++++++++++++++++++++++++---------------- src/nixpacks.ts | 34 ++++-------------- test/build.test.ts | 72 ++++++++++++++++++++++++++++++++++++- test/nixpacks.test.ts | 14 +++++++- 4 files changed, 142 insertions(+), 60 deletions(-) diff --git a/src/commands/build.ts b/src/commands/build.ts index 253ba532..7d349cff 100644 --- a/src/commands/build.ts +++ b/src/commands/build.ts @@ -6,7 +6,7 @@ import { resolve, join } from 'node:path' import { existsSync, readFileSync, readdirSync, statSync } from 'node:fs' import { info, printJson, die } from '../util.js' import { dockerfileExposedPort } from './deploy.js' -import { nixpacksPlan, nixpacksGeneratedDockerfile, ensureNixpacks, quietRunner, type NixpacksPlan } from '../nixpacks.js' +import { nixpacksPlan, nixpacksGeneratedDockerfile, nixpacksAvailable, quietRunner, type NixpacksPlan } from '../nixpacks.js' import type { BuildRunner } from '../flyctl-build.js' export type BuildCheck = { @@ -46,7 +46,11 @@ export function computeVerdict(checks: BuildCheck[]): BuildReport['verdict'] { // Port resolution mirrors deploy.ts: an explicit --port wins, else the Dockerfile's EXPOSE. The // rationale string is part of the output — every plan line says why (the `fly launch` pattern). export function inferPort(flag: string | undefined, dockerfile: string | undefined): { port?: number; rationale: string } { - if (flag) return { port: Number(flag), rationale: '--port flag' } + if (flag) { + const port = /^\d+$/.test(flag.trim()) ? Number(flag.trim()) : NaN + if (!Number.isInteger(port) || port < 1 || port > 65535) throw new Error(`--port must be an integer between 1 and 65535, got: ${flag}`) + return { port, rationale: '--port flag' } + } const exposed = dockerfile ? dockerfileExposedPort(dockerfile) : undefined if (exposed) return { port: exposed, rationale: `Dockerfile EXPOSE ${exposed}` } return { port: undefined, rationale: 'not detected — deploy defaults to 8080' } @@ -65,11 +69,13 @@ export function envKeysFromDotEnvExample(content: string): string[] { } const CONTEXT_WARN_BYTES = 100 * 1024 * 1024 -const WALK_CAP = 50_000 // entries; beyond this the size is reported as a floor +const WALK_CAP = 50_000 // entries; hitting it marks the stats truncated (size becomes a floor) -type ContextStats = { totalBytes: number; nodeModulesBytes: number; hasNodeModules: boolean; nodeModulesIgnored: boolean } +export type ContextStats = { totalBytes: number; nodeModulesBytes: number; hasNodeModules: boolean; nodeModulesIgnored: boolean; truncated: boolean } -function contextStats(dir: string): ContextStats { +// Sizes what would actually ship: a dockerignored node_modules is skipped, not counted. (Only the +// node_modules pattern is honored — full .dockerignore glob semantics aren't reimplemented here.) +export function contextStats(dir: string, cap = WALK_CAP): ContextStats { const ignoreFile = join(dir, '.dockerignore') const ignoreLines = existsSync(ignoreFile) ? readFileSync(ignoreFile, 'utf8').split('\n').map((l) => l.trim()).filter((l) => l && !l.startsWith('#')) @@ -78,18 +84,22 @@ function contextStats(dir: string): ContextStats { let totalBytes = 0 let nodeModulesBytes = 0 let hasNodeModules = false + let truncated = false let seen = 0 const walk = (d: string, inNodeModules: boolean) => { let entries: string[] try { entries = readdirSync(d) } catch { return } for (const name of entries) { - if (seen++ > WALK_CAP) return + if (seen++ >= cap) { truncated = true; return } if (name === '.git') continue const p = join(d, name) let st try { st = statSync(p) } catch { continue } + if (name === 'node_modules' && st.isDirectory()) { + hasNodeModules = true + if (nodeModulesIgnored) continue // excluded from the context — don't count it + } const isNm = inNodeModules || name === 'node_modules' - if (name === 'node_modules' && st.isDirectory()) hasNodeModules = true if (st.isDirectory()) walk(p, isNm) else { totalBytes += st.size @@ -98,11 +108,31 @@ function contextStats(dir: string): ContextStats { } } walk(dir, false) - return { totalBytes, nodeModulesBytes, hasNodeModules, nodeModulesIgnored } + return { totalBytes, nodeModulesBytes, hasNodeModules, nodeModulesIgnored, truncated } } const mb = (bytes: number): string => `${(bytes / 1024 / 1024).toFixed(1)} MB` +export function contextCheck(ctx: ContextStats): BuildCheck { + const shipsNodeModules = ctx.hasNodeModules && !ctx.nodeModulesIgnored + const tooBig = ctx.totalBytes > CONTEXT_WARN_BYTES + const size = `${mb(ctx.totalBytes)}${ctx.truncated ? '+' : ''}` + const detail = shipsNodeModules + ? `node_modules (${mb(ctx.nodeModulesBytes)}) would ship in the ${size} build context` + : ctx.truncated + ? `over ${WALK_CAP.toLocaleString('en-US')} files — scan truncated, ${size} is a floor` + : `${size}${tooBig ? ' — large contexts make remote builds slow' : ''}` + const bad = shipsNodeModules || tooBig || ctx.truncated + return { + id: 'context', + severity: 'warning', + status: bad ? 'fail' : 'pass', + title: 'build context', + detail, + ...(bad ? { nextAction: 'add a .dockerignore (node_modules, build artifacts, secrets)' } : {}), + } +} + export async function buildReport( dirArg: string, opts: { port?: string }, @@ -182,19 +212,7 @@ export async function buildReport( ...(port !== undefined ? {} : { nextAction: 'pass --port (or add EXPOSE to the Dockerfile) — a port mismatch is the #1 deploy mistake' }), }) - const ctx = contextStats(dir) - const shipsNodeModules = ctx.hasNodeModules && !ctx.nodeModulesIgnored - const tooBig = ctx.totalBytes > CONTEXT_WARN_BYTES - checks.push({ - id: 'context', - severity: 'warning', - status: shipsNodeModules || tooBig ? 'fail' : 'pass', - title: 'build context', - detail: shipsNodeModules - ? `node_modules (${mb(ctx.nodeModulesBytes)}) would ship in the ${mb(ctx.totalBytes)} build context` - : `${mb(ctx.totalBytes)}${tooBig ? ' — large contexts make remote builds slow' : ''}`, - ...(shipsNodeModules || tooBig ? { nextAction: 'add a .dockerignore (node_modules, build artifacts, secrets)' } : {}), - }) + checks.push(contextCheck(contextStats(dir))) return { dir, @@ -230,18 +248,20 @@ export function renderReport(r: BuildReport, explain: boolean): string[] { return lines } +// Dockerfile content is included with --explain; without it the report stays small. +export function jsonReport(report: BuildReport, explain: boolean): BuildReport { + return explain ? report : { ...report, dockerfile: { ...report.dockerfile, content: undefined } } +} + export async function build(dirArg: string | undefined, opts: { explain?: boolean; json?: boolean; port?: string }): Promise { const dir = dirArg ?? '.' const abs = resolve(process.cwd(), dir) - if (!existsSync(abs)) die(`no such directory: ${abs}`) - // Only reach for nixpacks when there is no Dockerfile to verify. - const nixpacksAvailable = existsSync(join(abs, 'Dockerfile')) ? false : await ensureNixpacks() - const report = await buildReport(dir, opts, { runner: quietRunner, nixpacksAvailable }) - if (opts.json) { - // Dockerfile content is included with --explain; without it the report stays small. - printJson(opts.explain ? report : { ...report, dockerfile: { ...report.dockerfile, content: undefined } }) - } else { - for (const line of renderReport(report, !!opts.explain)) info(line) - } + if (!existsSync(abs) || !statSync(abs).isDirectory()) die(`no such directory: ${abs}`) + // Only probe for nixpacks when there is no Dockerfile to verify. The probe is silent and never + // installs anything — stdout must stay pure for --json, and a verifier must stay offline. + const available = existsSync(join(abs, 'Dockerfile')) ? false : await nixpacksAvailable() + const report = await buildReport(dir, opts, { runner: quietRunner, nixpacksAvailable: available }) + if (opts.json) printJson(jsonReport(report, !!opts.explain)) + else for (const line of renderReport(report, !!opts.explain)) info(line) if (report.verdict === 'failed') process.exitCode = 1 } diff --git a/src/nixpacks.ts b/src/nixpacks.ts index beae7145..f40b31d4 100644 --- a/src/nixpacks.ts +++ b/src/nixpacks.ts @@ -5,7 +5,6 @@ import { spawn } from 'node:child_process' import { mkdtempSync, readFileSync, rmSync, existsSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { info } from './util.js' import type { BuildRunner } from './flyctl-build.js' export type NixpacksPlan = { @@ -69,30 +68,11 @@ export async function nixpacksGeneratedDockerfile(dir: string, run: BuildRunner } } -// Best-effort: make the nixpacks CLI available (mirrors ensureFlyctl). Returns availability — -// `insta build` degrades to Dockerfile-only checks when it stays missing. -export async function ensureNixpacks(): Promise { - const ok = (cmd: string, args: string[], inherit = false) => - new Promise((resolve) => { - const p = spawn(cmd, args, { stdio: inherit ? 'inherit' : 'ignore' }) - p.on('error', () => resolve(false)) - p.on('close', (code) => resolve(code === 0)) - }) - try { - if (await ok('nixpacks', ['--version'])) return true - if (process.platform === 'darwin' && (await ok('brew', ['--version']))) { - info('nixpacks not found — installing with `brew install nixpacks` (one-time)…') - if (await ok('brew', ['install', 'nixpacks'], true)) return ok('nixpacks', ['--version']) - } - if (process.platform === 'linux') { - info('nixpacks not found — installing (one-time)…') - if (await ok('sh', ['-c', 'curl -fsSL https://nixpacks.com/install.sh | bash'], true)) { - if (await ok('nixpacks', ['--version'])) return true - } - } - info('nixpacks not found — install it for framework detection: https://nixpacks.com/docs/install') - return false - } catch { - return false - } +// Quiet probe — no install, no output. `insta build` advertises itself as local and offline, so +// unlike deploy's ensureFlyctl it must never download anything or write to stdout (which would +// corrupt --json); when nixpacks is missing the command degrades to Dockerfile-only checks and +// the report's nextAction says how to install it. +export async function nixpacksAvailable(run: BuildRunner = quietRunner): Promise { + const { code } = await run('nixpacks', ['--version'], { cwd: '.', env: process.env as Record }) + return code === 0 } diff --git a/test/build.test.ts b/test/build.test.ts index a32a4086..b61719a4 100644 --- a/test/build.test.ts +++ b/test/build.test.ts @@ -4,7 +4,8 @@ import { join } from 'node:path' import { describe, it, expect } from 'vitest' import { computeVerdict, inferPort, envKeysFromDotEnvExample, buildReport, renderReport, - type BuildCheck, + contextStats, contextCheck, jsonReport, + type BuildCheck, type ContextStats, } from '../src/commands/build.js' import type { BuildRunner } from '../src/flyctl-build.js' @@ -36,6 +37,75 @@ describe('inferPort', () => { it('reports the platform default when nothing declares a port', () => { expect(inferPort(undefined, undefined)).toEqual({ port: undefined, rationale: 'not detected — deploy defaults to 8080' }) }) + it('rejects a --port that is not an integer in 1..65535 (a verifier must not bless bad input)', () => { + expect(() => inferPort('abc', undefined)).toThrow(/1.*65535/) + expect(() => inferPort('0', undefined)).toThrow(/1.*65535/) + expect(() => inferPort('70000', undefined)).toThrow(/1.*65535/) + expect(() => inferPort('80.5', undefined)).toThrow(/1.*65535/) + }) +}) + +describe('contextStats', () => { + it('excludes a dockerignored node_modules from the shipped size', () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-ctx-')) + writeFileSync(join(dir, '.dockerignore'), 'node_modules\n') + writeFileSync(join(dir, 'app.js'), 'x'.repeat(100)) + mkdirSync(join(dir, 'node_modules'), { recursive: true }) + writeFileSync(join(dir, 'node_modules', 'big.js'), 'x'.repeat(10_000)) + const ctx = contextStats(dir) + expect(ctx.hasNodeModules).toBe(true) + expect(ctx.nodeModulesIgnored).toBe(true) + expect(ctx.totalBytes).toBeLessThan(10_000) // the ignored tree does not count + }) + + it('flags truncation when the walk cap is hit instead of silently undercounting', () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-ctx-')) + for (const n of ['a', 'b', 'c', 'd']) writeFileSync(join(dir, `${n}.txt`), 'x') + expect(contextStats(dir, 2).truncated).toBe(true) + expect(contextStats(dir).truncated).toBe(false) + }) +}) + +describe('contextCheck', () => { + const base: ContextStats = { totalBytes: 1024, nodeModulesBytes: 0, hasNodeModules: false, nodeModulesIgnored: false, truncated: false } + it('passes a small clean context', () => { + expect(contextCheck(base).status).toBe('pass') + }) + it('fails when node_modules would ship', () => { + const c = contextCheck({ ...base, hasNodeModules: true, nodeModulesBytes: 5_000_000 }) + expect(c.status).toBe('fail') + expect(c.detail).toContain('node_modules') + }) + it('fails an oversized context even without node_modules', () => { + const c = contextCheck({ ...base, totalBytes: 200 * 1024 * 1024 }) + expect(c.status).toBe('fail') + expect(c.detail).toContain('slow') + }) + it('fails conservatively when the scan was truncated (size is a floor, not a fact)', () => { + const c = contextCheck({ ...base, truncated: true }) + expect(c.status).toBe('fail') + expect(c.detail).toContain('truncated') + }) +}) + +describe('jsonReport', () => { + it('strips Dockerfile content without --explain and keeps it with — stdout stays parseable JSON either way', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + writeFileSync(join(dir, 'package.json'), '{}') + const r = await buildReport(dir, {}, { runner: nixpacksFake(PLAN), nixpacksAvailable: true }) + const bare = JSON.parse(JSON.stringify(jsonReport(r, false))) + expect(bare.dockerfile.content).toBeUndefined() + expect(bare.dockerfile.source).toBe('nixpacks') + const full = JSON.parse(JSON.stringify(jsonReport(r, true))) + expect(full.dockerfile.content).toContain('FROM node:18') + }) + + it('is pure on the nixpacks-missing path too (the agent contract must survive a fresh machine)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'insta-build-')) + const runner: BuildRunner = async () => { throw new Error('nixpacks must not be invoked when unavailable') } + const r = await buildReport(dir, {}, { runner, nixpacksAvailable: false }) + expect(JSON.parse(JSON.stringify(jsonReport(r, false))).verdict).toBe('failed') + }) }) describe('envKeysFromDotEnvExample', () => { diff --git a/test/nixpacks.test.ts b/test/nixpacks.test.ts index 8f683aa9..e3a76c71 100644 --- a/test/nixpacks.test.ts +++ b/test/nixpacks.test.ts @@ -2,7 +2,7 @@ import { mkdtempSync, mkdirSync, writeFileSync, existsSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { describe, it, expect } from 'vitest' -import { parseNixpacksPlan, nixpacksPlan, nixpacksGeneratedDockerfile } from '../src/nixpacks.js' +import { parseNixpacksPlan, nixpacksPlan, nixpacksGeneratedDockerfile, nixpacksAvailable } from '../src/nixpacks.js' import type { BuildRunner } from '../src/flyctl-build.js' // The documented `nixpacks plan` output shape (providers + phases + start). @@ -70,6 +70,18 @@ describe('nixpacksPlan', () => { }) }) +describe('nixpacksAvailable', () => { + it('probes `nixpacks --version` quietly — true on 0, false otherwise (never installs anything)', async () => { + const seen: { cmd?: string; args?: string[] } = {} + const up: BuildRunner = async (cmd, args) => { seen.cmd = cmd; seen.args = args; return { code: 0, output: 'nixpacks 1.41.0' } } + expect(await nixpacksAvailable(up)).toBe(true) + expect(seen.cmd).toBe('nixpacks') + expect(seen.args).toEqual(['--version']) + const down: BuildRunner = async () => ({ code: -1, output: 'spawn nixpacks ENOENT' }) + expect(await nixpacksAvailable(down)).toBe(false) + }) +}) + describe('nixpacksGeneratedDockerfile', () => { it('runs `nixpacks build --out ` and reads the generated Dockerfile', async () => { const dir = mkdtempSync(join(tmpdir(), 'insta-build-src-')) From ffcd98aafde7885e4f2c290ea298efa8d9753b2e Mon Sep 17 00:00:00 2001 From: jwfing Date: Tue, 18 Aug 2026 21:22:06 -0700 Subject: [PATCH 3/3] review round 2: reject --port '' explicitly; truncation message counts entries, not files Co-Authored-By: Claude Fable 5 --- src/commands/build.ts | 4 ++-- test/build.test.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/commands/build.ts b/src/commands/build.ts index 7d349cff..65d1a41b 100644 --- a/src/commands/build.ts +++ b/src/commands/build.ts @@ -46,7 +46,7 @@ export function computeVerdict(checks: BuildCheck[]): BuildReport['verdict'] { // Port resolution mirrors deploy.ts: an explicit --port wins, else the Dockerfile's EXPOSE. The // rationale string is part of the output — every plan line says why (the `fly launch` pattern). export function inferPort(flag: string | undefined, dockerfile: string | undefined): { port?: number; rationale: string } { - if (flag) { + if (flag !== undefined) { const port = /^\d+$/.test(flag.trim()) ? Number(flag.trim()) : NaN if (!Number.isInteger(port) || port < 1 || port > 65535) throw new Error(`--port must be an integer between 1 and 65535, got: ${flag}`) return { port, rationale: '--port flag' } @@ -120,7 +120,7 @@ export function contextCheck(ctx: ContextStats): BuildCheck { const detail = shipsNodeModules ? `node_modules (${mb(ctx.nodeModulesBytes)}) would ship in the ${size} build context` : ctx.truncated - ? `over ${WALK_CAP.toLocaleString('en-US')} files — scan truncated, ${size} is a floor` + ? `over ${WALK_CAP.toLocaleString('en-US')} entries — scan truncated, ${size} is a floor` : `${size}${tooBig ? ' — large contexts make remote builds slow' : ''}` const bad = shipsNodeModules || tooBig || ctx.truncated return { diff --git a/test/build.test.ts b/test/build.test.ts index b61719a4..86e413b3 100644 --- a/test/build.test.ts +++ b/test/build.test.ts @@ -42,6 +42,7 @@ describe('inferPort', () => { expect(() => inferPort('0', undefined)).toThrow(/1.*65535/) expect(() => inferPort('70000', undefined)).toThrow(/1.*65535/) expect(() => inferPort('80.5', undefined)).toThrow(/1.*65535/) + expect(() => inferPort('', undefined)).toThrow(/1.*65535/) // explicit-but-empty is a mistake, not an omission }) }) @@ -85,6 +86,7 @@ describe('contextCheck', () => { const c = contextCheck({ ...base, truncated: true }) expect(c.status).toBe('fail') expect(c.detail).toContain('truncated') + expect(c.detail).toContain('entries') // the cap counts entries (dirs included), not files }) })