Run production build in CI - #153
Conversation
Independent code review trailRound 1
Round 2
Round 3
Unresolved after 3 rounds: none — every finding was either fixed or explicitly out of scope (packaging/ |
|
Re: "Fix CI failure" — the only red check here is Folding the Once #156 merges to |
|
The only red check here is |
… line-number pointer, document XDG_CONFIG_HOME scope, capture stderr
Independent code review trail (fixing CI: audit + build step)Fix applied: merged Round 1:
Round 2:
Round 3 (final round — 3-round cap reached):
Unresolved after 3 rounds: the smoke test still exercises Follow-up suggestions
|
What changed
CI ran lint, typecheck (against
tsconfig.test.json), andtest:ci, but nevernpm run build— thetsc && tsc-aliasstep that produces the artifact actually published to npm.tsconfig.json(production build) has differentinclude/paths/rootDirthantsconfig.test.json(typecheck), so a break reachable only through the real build compile could pass CI green and ship broken to npm.This adds:
npm run buildas a CI step, so production-build-only compile breakages fail the PR.dist/index.js --helpand asserts its output, so a build that compiles but produces a broken/empty entry point (e.g. atsc-aliasmisconfiguration that silently fails to rewrite@/...path aliases) is also caught. This step:grep, rather than piping directly intogrep -q— GitHub Actions runs steps withoutpipefail, so a direct pipe would letgrep's exit code mask a non-zero exit from the CLI itself.XDG_CONFIG_HOMEto an isolated temp directory, sincesrc/libs/config.tsinitializes aConfstore at module load (before the--helpbranch is even reached), so the step doesn't depend on/pollute the runner's real config directory.Why not more
An independent review round also flagged that the published npm tarball currently has no
filesallowlist and would ship withoutdist/, and that testingdist/index.jsdirectly (rather than through the packedbinentry) doesn't catch that. Both are out of scope here — PR #152 (issue #146) is already adding aprepackbuild hook andfilesallowlist for exactly that, and this PR was scoped to stay out ofpackage.json's publish/dist handling per the issue's instructions.Where to view
https://github.com/neonpixels-studio/markpost-cli/blob/agent/ci-run-build/.github/workflows/ci.yml
Closes #147