Skip to content

ci: add pull-request CI for the release codepath - #17

Merged
luhe19001 merged 4 commits into
mainfrom
hlu/eng-1320
Aug 5, 2026
Merged

ci: add pull-request CI for the release codepath#17
luhe19001 merged 4 commits into
mainfrom
hlu/eng-1320

Conversation

@luhe19001

@luhe19001 luhe19001 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Context

The repository had no CI. Nothing checked a pull request, and nothing verified that the release build — the one that embeds the built frontend into the Go binary via go:embed — still produced a working executable. A broken release was only discoverable by hand.

This adds both, with the release path exercised the same way a person would run it rather than approximated by a separate set of steps.

Changes

.github/workflows/ci.yml — three jobs, on every pull request, on pushes to main, and on manual dispatch. In-progress runs are cancelled for pull requests but not for main.

Job Runner What it does
Frontend ubuntu npm ci, lint, format check, tests, production build
Collector ubuntu make check (gofmt + go vet), make test
Release macOS make release, worktree-clean check, smoke test, dev build, dev smoke test, artifact upload

The Release job is the point of the change. It runs the same make release a human release uses, then boots the resulting binary and asserts its behavior over HTTP. Step order matters: the embedded binary is copied aside before make build overwrites bin/coslash, so the uploaded artifact is the embedded one and not the bare development build.

collector/scripts/smoke.sh boots a built binary under an empty HOME and checks its HTTP contract in two modes.

  • Embedded/ serves the application document; a /assets/*.js file referenced by that document resolves; /coslash survives a direct hit rather than 404ing; a missing asset returns 404 rather than falling back to the SPA document; an unrouted /api path returns 404 rather than the application document; /api/sessions returns JSON; --version is not dev.
  • Bare/ returns 503 and /api/sessions still answers.

The 404 and 503 assertions lock in the behavior from 0cb5405, which was previously only verifiable by hand.

make check, make smoke, make smoke-dev give CI stable entry points instead of shell inlined into YAML, and stay usable locally.

The collector now builds on non-darwin platforms. The new Linux job caught this immediately: Birthtimespec exists only on darwin's syscall.Stat_t, so internal/vendors/claude/fork.go had never compiled anywhere else — nothing had ever tried. fileCreationTime already fell back to ModTime() when the Stat_t type assertion failed, so the intent was present; it simply could not compile where the field is absent. The darwin-only read now sits behind a build-tagged birthtime helper. No behavior change on darwin — the expression is unchanged — and fork ordering only uses birthtime to break ties between transcripts.

CoslashPage.tsx reformatted so the new formatting gate is not red on its first run. Whitespace and Tailwind class ordering only; nothing renders differently.

Plus a CI badge in the README.

Test

All three jobs pass: Frontend 23s, Collector 23s, Release 48s.

The uploaded artifact is the embedded binary — the one failure mode that could ship behind a green check. Downloaded coslash-macos (10.1 MB), --version printed a real commit SHA rather than dev, and booting it under an empty HOME gave / → 200 with <title>coSlash and one id="root" div, /api/sessions → 200. A 503 there would have meant the wrong binary was uploaded.

The frontend build step earns its slot. With a deliberate type error present, npm run lint, npm run format:check, vitest run, and vite build all exit 0; only npm run build fails, with TS2322. vite build passing on its own confirms why — esbuild strips types without checking them, so tsc -b is doing the work. It uses the same script the release path invokes, so both check the identical build.

Every CI step also replayed locally, in the workflow's order, on darwin-arm64 / Node 23.5.0 / Go 1.26.5: make check, make test, make release (worktree clean afterwards), make smoke, make build, make smoke-dev, and the full frontend sequence from a cold node_modules. GOOS=linux go build ./... and go vet ./... pass on amd64 and arm64 after the portability fix.

Not yet done: deliberately breaking each gate to watch it go red — though the gofmt/vet gate proved itself for real via the Linux bug above, and the formatting gate was genuinely red before the CoslashPage.tsx fix — and the smoke script's port-fallback and argument-validation edge cases.

Note for reviewers

go test ./... currently has no test files, so that step passes vacuously. The wiring is correct and picks up tests as soon as they exist, and smoke.sh is doing the real behavioral verification in the meantime, but a green Collector check should not be read as coverage. internal/web's routing rules are pure functions of an fs.FS and are the obvious first candidate.

Adds a three-job GitHub Actions workflow that runs on every pull
request, on pushes to main, and on manual dispatch:

- Frontend (ubuntu): npm ci, lint, format check, tests
- Collector (ubuntu): gofmt + go vet via a new `make check`, go test
- Release (macOS): the real `make release`, then a check that staging
  the frontend leaves the worktree clean, then a smoke test of the
  resulting binary, then the same for a dev binary, then artifact upload

The release job is the point of the change: it exercises the same
embedded-frontend build path a human release uses, rather than
approximating it.

scripts/smoke.sh boots a built binary under an empty HOME and asserts
its HTTP contract in two modes. Embedded: / serves the app document, a
referenced /assets/*.js resolves, /coslash survives a direct hit, a
missing asset is a 404 rather than the SPA fallback, an unrouted /api
path is a 404 rather than the app document, /api/sessions returns JSON,
and --version is not "dev". Bare: / is 503 and /api/sessions still
answers. The 404 and 503 assertions lock in the behavior from 0cb5405.

Also reformats CoslashPage.tsx, which was the only file failing
prettier and would have made the new format gate red on its first run.
The change is whitespace and Tailwind class ordering only.
`go vet ./...` on the new ubuntu CI job could not compile the collector:
Birthtimespec exists only on darwin's syscall.Stat_t, so
internal/vendors/claude/fork.go failed to build on Linux.

fileCreationTime already fell back to ModTime when the Stat_t assertion
failed, so the intent was there — it just could not compile where the
field is absent. Move the darwin-only read behind a build-tagged
birthtime helper and keep the existing fallback for everything else.

Fork ordering only uses birthtime to break ties between transcripts, and
the collector ships for macOS, so the fallback costs nothing in practice.
No behavior change on darwin: the expression is unchanged.

Verified: `make check` and `make test` pass natively, and
GOOS=linux (amd64 and arm64) now builds and vets clean.
The production build only ran inside `make stage` on the macOS Release
job, so a TypeScript error failed Release while Frontend stayed green —
a source error reported as release-infrastructure breakage, on the
slowest and most expensive runner.

Runs `npm run build`, the same script the release path invokes, rather
than a separate typecheck script, so both paths check the identical
build. Placed last because it is the slowest gate (~3s locally against
under a second for the others) and nothing earlier depends on it.

Verified the step earns its place: with a deliberate type error present,
lint, format:check, and `vitest run` all exit 0 and only `npm run build`
fails (TS2322). `vite build` alone also exits 0 — esbuild strips types
without checking them, so `tsc -b` is doing the work.

No worktree impact: both tsconfigs set noEmit and point tsBuildInfoFile
into node_modules/.tmp, so the Release job's clean gate is unaffected.
@luhe19001 luhe19001 added the enhancement New feature or request label Aug 4, 2026

@milanshen milanshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two recommended workflow changes before merge.

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml Outdated
@luhe19001
luhe19001 merged commit 8198937 into main Aug 5, 2026
3 checks passed
This was referenced Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants