Skip to content

Commit 9310cb1

Browse files
odenirdevclaude
andcommitted
fix(cli): real gen:versions CI gate + truthful press build exit codes
Closes the BASE/CLI review's two "looks done but isn't" defects. CI gate (DONE #4): `gen:versions --check` was a silent no-op. It filtered on `@ogs-tech/press-cli` — no such package (it's `@ogs-tech/create-press`), and `pnpm --filter` exits 0 when it matches nothing. Even with the right name it ran AFTER `pnpm build`, whose cli build script is `gen:versions && tsc` (write mode), which regenerated the file and masked any drift. Fixed the filter and moved the step above `pnpm build`, so a stale versions.generated.ts now fails CI. press build (DONE #2): a failing subprocess (strapi/next build) collapsed to a generic exit 1. `util/run.ts` now rejects with a SubprocessError carrying the real code/signal and bin/press.ts re-exits with it — matching dev.ts's crash-aware guarantee. dev's pre-boot seed/sync-types become truthful for free. Robustness/cleanup gaps from the review are tracked as follow-ups in docs/superpowers/plans/2026-07-11-base-cli-followups.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 03a2ed9 commit 9310cb1

6 files changed

Lines changed: 140 additions & 5 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@ogs-tech/press-web': patch
3+
---
4+
5+
fix(cli): `press build` surfaces the failing subprocess's real exit code
6+
7+
`util/run.ts` now rejects with a `SubprocessError` carrying the child's real
8+
`code` (and `signal`), and `bin/press.ts` re-exits with it instead of a blanket
9+
`1`. Before, any failing step in `press build` (`strapi build` / `next build`)
10+
collapsed to exit `1`, weakening the "truthful failure" guarantee that `dev.ts`
11+
already upholds via `waitForReadyOrExit` — a CMS build failing with a specific
12+
code was indistinguishable from any other error in CI. `press dev`'s pre-boot
13+
`seed`/`sync-types` steps become truthful for free (same `run()` helper). No
14+
behavior change on success (exit 0).

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@ jobs:
2020

2121
- run: pnpm install --frozen-lockfile
2222

23+
# Guard the committed versions.generated.ts BEFORE `pnpm build` runs — the cli's
24+
# build script is `gen:versions && tsc` (WRITE mode), which would regenerate the
25+
# file in the runner's tree and mask a stale commit. Must stay above `pnpm build`.
26+
- run: pnpm --filter @ogs-tech/create-press gen:versions --check
27+
2328
- run: pnpm build
2429

2530
- run: pnpm -r --if-present typecheck
2631

2732
- run: pnpm -r test
2833

29-
- run: pnpm --filter @ogs-tech/press-cli gen:versions --check
30-
3134
- run: pnpm pack:check
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# BASE/CLI — tracked follow-ups (2026-07-11)
2+
3+
Surfaced by the BASE/CLI closing review. These are **not** blockers for closing
4+
the CRM task — the DONE criteria are met (see the review summary). They are the
5+
robustness/cleanup gaps we consciously deferred. Each is independently shippable
6+
as its own PR (do not bundle — separate concerns).
7+
8+
The task-closing PR fixed only the two "truthful failure" defects:
9+
- CI `gen:versions --check` gate made real (wrong `--filter` name + step ordered
10+
after `pnpm build`, which regenerated the file before the check could see drift).
11+
- `press build` now surfaces the subprocess's real exit code (`SubprocessError`
12+
in `util/run.ts` + `bin/press.ts`), matching `dev.ts`.
13+
14+
---
15+
16+
## 1. End-to-end + orchestration tests
17+
18+
**Gap.** The two most important commands — `devCommand` (`packages/web/src/commands/dev.ts`)
19+
and `buildCommand` (`build.ts`) — have **no direct tests**. Their primitives
20+
(`waitForReadyOrExit`, `watchSchema`, `materialize`, `run`) are well covered, but
21+
the orchestration itself (6-step boot sequence, `killAll`, SIGINT/SIGTERM
22+
handlers, the `cms.exit`/`web.exit` race, the `shuttingDown` flag) is validated
23+
only by manual `pnpm dev` dogfooding. Likewise, `@ogs-tech/create-press` has no
24+
test that runs the **published binary** (`bin/create-press.js` → `dist/cli.js` →
25+
real `pnpm install`); `scaffold()` is tested in isolation and only exercised from
26+
TS source via `scripts/create-playground.ts`.
27+
28+
**Do.**
29+
- Unit-level orchestration tests for `dev.ts`/`build.ts` by injecting a spawn/`run`
30+
seam (mock children like `wait-for-ready-or-exit.test.ts`'s `FakeChild`): assert
31+
the boot order, that a cms crash aborts before booting web, that SIGINT exits 0,
32+
and that an unexpected child exit re-exits with its real code.
33+
- A Playwright/e2e smoke (already named as planned in the root README): scaffold a
34+
temp project via the built binary, `press dev`, assert `:1337` + `:3000` serve
35+
and `shared/types/generated.ts` is written.
36+
37+
**Acceptance.** `dev.ts`/`build.ts` have coverage; a CI smoke boots a scaffolded
38+
project end-to-end.
39+
40+
## 2. Documentation drift + dead code
41+
42+
**Gap.**
43+
- **`press.config.ts` naming.** CLAUDE.md and the README describe build-time
44+
anchors as living in `press.config.ts` at the repo root, but the scaffold emits
45+
`packages/web/config.ts` (its header comment even says `// press.config.ts`).
46+
No generated file is literally named/located `press.config.ts`. There is also a
47+
stray `press.config.ts` at this monorepo's root with no functional link to the
48+
CLI. Harmless, but it misleads anyone auditing literally.
49+
- **`VERSIONS.pressCli` dead field.** Computed and tested in `compute-versions.ts`
50+
but never consumed by `scaffold.ts` (a scaffolded project never depends on
51+
`@ogs-tech/create-press`, by design).
52+
- **Stale `packages/cli/dist/`.** Contains artifacts from a prior architecture
53+
(`dist/commands/{dev,build,deploy}.js`, `dist/materialize.js`, …) with no `src/`
54+
counterpart. `dist/` is gitignored so publish is unaffected, but it confuses a
55+
manual `node bin/create-press.js` without a fresh rebuild.
56+
57+
**Do.** Reconcile the docs to the real path (or rename the emitted file — decide
58+
which is canonical); drop `pressCli` from `ScaffoldVersions` (or wire it if a use
59+
appears); note that `dist/` must be rebuilt before manual bin inspection.
60+
61+
**Acceptance.** Docs match the scaffold output; no computed-but-unused version field.
62+
63+
## 3. Derive `NEXT_PIN` instead of hardcoding
64+
65+
**Gap.** `compute-versions.ts:41` hardcodes `NEXT_PIN = '^15.1.0'` — the only
66+
scaffold pin not derived from a real manifest (react/strapi come from engine
67+
`devDependencies`; the follow-up is already noted in-code). `@ogs-tech/press-web`
68+
declares only an open `next: '>=15'` peer range, useless as a pin.
69+
70+
**Do.** Add `next` to `@ogs-tech/press-web` `devDependencies` (the version the
71+
engine is actually tested against) and derive the pin via `requireDevDep`, exactly
72+
like react/strapi. Removes the last manual knob and closes the drift loop for Next.
73+
74+
**Acceptance.** `computeVersions` derives `next` from a manifest; no `NEXT_PIN`
75+
constant.

‎packages/web/bin/press.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
#!/usr/bin/env tsx
22
import { run } from '../src/runtime-cli';
3+
import { SubprocessError } from '../src/util/run';
34

45
run(process.argv).catch((err) => {
56
console.error(err?.message ?? err);
6-
process.exit(1);
7+
// Surface the failing subprocess's REAL exit code (never a blanket 1) so build
8+
// failures are truthful in CI; a signal-killed child (code null) still exits non-zero.
9+
process.exit(err instanceof SubprocessError ? (err.code ?? 1) : 1);
710
});

‎packages/web/src/util/run.test.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { run, SubprocessError } from './run';
3+
4+
describe('run', () => {
5+
it('resolves when the command exits 0', async () => {
6+
await expect(run('node', ['-e', 'process.exit(0)'])).resolves.toBeUndefined();
7+
});
8+
9+
it('rejects with a SubprocessError that carries the REAL exit code', async () => {
10+
// The whole point of press build/dev "truthful failures": a non-zero
11+
// subprocess must surface ITS code, never a generic 1.
12+
const err = await run('node', ['-e', 'process.exit(3)']).catch((e) => e);
13+
expect(err).toBeInstanceOf(SubprocessError);
14+
expect((err as SubprocessError).code).toBe(3);
15+
});
16+
17+
it('rejects (not resolves) when the command cannot be spawned', async () => {
18+
const err = await run('this-binary-does-not-exist-press', []).catch((e) => e);
19+
expect(err).toBeInstanceOf(Error);
20+
});
21+
});

‎packages/web/src/util/run.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,23 @@ export interface RunOptions {
99
env?: Record<string, string | undefined>;
1010
}
1111

12+
/**
13+
* A subprocess that exited non-zero, carrying the REAL exit code (and signal, if
14+
* killed). bin/press.ts re-exits with this code so `press build` surfaces the
15+
* failing tool's own code instead of a generic 1 — the same "truthful failure"
16+
* guarantee dev.ts already gives via waitForReadyOrExit.
17+
*/
18+
export class SubprocessError extends Error {
19+
constructor(
20+
readonly command: string,
21+
readonly code: number | null,
22+
readonly signal: NodeJS.Signals | null = null,
23+
) {
24+
super(`${command} exited ${code ?? `(signal ${signal})`}`);
25+
this.name = 'SubprocessError';
26+
}
27+
}
28+
1229
/** Runs a command, inheriting stdio, and resolves on exit 0 (rejects otherwise). */
1330
export function run(cmd: string, args: string[], opts: RunOptions = {}): Promise<void> {
1431
return new Promise((resolve, reject) => {
@@ -18,8 +35,10 @@ export function run(cmd: string, args: string[], opts: RunOptions = {}): Promise
1835
stdio: 'inherit',
1936
});
2037
child.on('error', reject);
21-
child.on('exit', (code) =>
22-
code === 0 ? resolve() : reject(new Error(`${cmd} ${args.join(' ')} exited ${code}`)),
38+
child.on('exit', (code, signal) =>
39+
code === 0
40+
? resolve()
41+
: reject(new SubprocessError(`${cmd} ${args.join(' ')}`, code, signal)),
2342
);
2443
});
2544
}

0 commit comments

Comments
 (0)