fix(ci): typecheck the celilo hook scripts, and fix the 18 errors behind it #54

Merged

forgejo-admin merged 1 commit from typecheck-celilo-scripts into main

2026-08-18 06:14:07 +00:00

forgejo-admin commented

2026-08-17 22:21:28 +00:00

Owner

Closes #52. Done in the order agreed: fix the errors, then add the tsconfig, then wire it into the gate.

celilo/scripts/ had no tsconfig.json and was in none of the four projects bun run typecheck compiles. The code that runs against the fleet during install, health check, backup and restore had never been through tsc. bun test runs it, but bun strips types without checking them, so a green test:hooks proved nothing about types.

The three that were real

These are live code, not test scaffolding, and they're the point of the issue.

1 & 2 — two Browser types (health-check.ts:410, smoke-handler.ts:100). Root cause was a version skew, not a cast: celilo/scripts pinned playwright-core at ~1.55.1 while the root pinned ~1.60.0. scripts/smoke/spa-smoke.ts types its browser parameter against the root copy; the hook launched one from its own bundled copy. Two Browsers, one call, and 1.60 added bind/unbind.

Fixed by aligning the pin to ~1.60.0 rather than by loosening a type. Nothing chose 1.55.1 deliberately — it's the version root was on when the pin was written. Aligning also puts the production smoke check on the same playwright the e2e suite validates against, which it should have been all along.

Note this ships a runtime dependency change. celilo/scripts/node_modules is bundled into the .netapp, so merging this eventually puts playwright-core 1.60.0 on the fleet — and auto_upgrade is on for lunacycle with a 15m poll, so CD will take it. The health-check hook is what launches that browser, so a bad bump shows up as a failed deploy rather than silent drift.

3 — Fetcher vs typeof fetch (health-check.ts:459). ApiSmokeOptions.fetchImpl was typed typeof fetch, which carries preconnect. The injectable fetchers the seam exists for — celilo's Fetcher, test doubles — are plain functions without it. So the health-check hook could not pass its own injected fetch into its own smoke helper: a DI seam that rejected the thing it was built to accept. Now typed as the call signature runApiSmoke actually uses.

The fifteen that were drift

Spies that had stopped matching the interfaces they name: FsOps gained writeFile, LunacycleHealthCheckDeps and LunacycleSetupWebDeps gained secrets, IdpCapability gained list_tokens/revoke_token.

Was anything silently passing? No — and I checked rather than assumed. Each hook genuinely never calls the missing member, so the tests were testing what they claimed; they just no longer described the type. Rather than take that on faith I stubbed FsOps.writeFile on the backup/restore spies and the two idp methods to throw instead of no-op. All 62 hook tests still pass, which is now evidence rather than an assertion.

The secrets drift did surface something real, though it's a coverage hole rather than a false pass: every call site in health-check.test.ts passes secrets: {}, so every test takes the !botPassword skip branch and the entire mint-token → SPA/API/WS smoke path has no unit coverage at all. Out of scope here, filed as #53 with the seams that make it testable.

The tsconfig

Inlines celilo's modules/tsconfig.scripts.base.json — a standalone module repo cannot extend a file that lives inside the celilo checkout. Two deliberate choices carried over or added:

No paths for @celilo/capabilities (celilo's own comment explains why): the hooks must resolve it from this directory's package.json, exactly as they do on the fleet. Mapping it elsewhere would typecheck against code the module never runs.
Type packages come from the root install via typeRoots, not from celilo/scripts/package.json. That directory's node_modules is bundled into the .netapp and shipped to the fleet, where nothing ever runs tsc — @types/* there would be dead weight on every deploy.

The gate

typecheck now ends in typecheck:hooks, which installs celilo/scripts' deps before running tsc -p celilo/scripts, mirroring what test:hooks already does. That makes it work on a fresh checkout regardless of where it sits in pr-validate's step order, so no workflow reordering was needed — verified by deleting celilo/scripts/node_modules and running it cold.

Proved it fails. Appended const x: number = "not a number"; to health-check.ts: bun run typecheck exits 2 and names the file and line. Removed it: exits 0. (Checked the exit code, not just the printed output — a gate that prints errors and exits 0 is not a gate.)

Gates

lint — exit 0
typecheck — exit 0, now including 16 hook files that had never been compiled
test:unit — 117 pass, 0 fail, 3 todo
test:hooks — 62 pass, 0 fail
test:seed — 1 pass, 0 fail

Upstream

The scaffolder gap behind this is filed as celilo#881 — scaffold_module emits no scripts/tsconfig.json, so every operator-owned module starts un-typecheckable and celilo only warns about it.

🤖 Generated with Claude Code

Closes #52. Done in the order agreed: fix the errors, then add the tsconfig, then wire it into the gate. `celilo/scripts/` had no `tsconfig.json` and was in none of the four projects `bun run typecheck` compiles. The code that runs against the fleet during install, health check, backup and restore had never been through `tsc`. `bun test` runs it, but bun strips types without checking them, so a green `test:hooks` proved nothing about types. ## The three that were real These are live code, not test scaffolding, and they're the point of the issue. **1 & 2 — two `Browser` types (`health-check.ts:410`, `smoke-handler.ts:100`).** Root cause was a version skew, not a cast: `celilo/scripts` pinned `playwright-core` at `~1.55.1` while the root pinned `~1.60.0`. `scripts/smoke/spa-smoke.ts` types its `browser` parameter against the root copy; the hook launched one from its own bundled copy. Two `Browser`s, one call, and 1.60 added `bind`/`unbind`. Fixed by aligning the pin to `~1.60.0` rather than by loosening a type. Nothing chose 1.55.1 deliberately — it's the version root was on when the pin was written. Aligning also puts the production smoke check on the same playwright the e2e suite validates against, which it should have been all along. **Note this ships a runtime dependency change.** `celilo/scripts/node_modules` is bundled into the `.netapp`, so merging this eventually puts playwright-core 1.60.0 on the fleet — and `auto_upgrade` is on for lunacycle with a 15m poll, so CD will take it. The health-check hook is what launches that browser, so a bad bump shows up as a failed deploy rather than silent drift. **3 — `Fetcher` vs `typeof fetch` (`health-check.ts:459`).** `ApiSmokeOptions.fetchImpl` was typed `typeof fetch`, which carries `preconnect`. The injectable fetchers the seam exists for — celilo's `Fetcher`, test doubles — are plain functions without it. So the health-check hook could not pass its own injected fetch into its own smoke helper: a DI seam that rejected the thing it was built to accept. Now typed as the call signature `runApiSmoke` actually uses. ## The fifteen that were drift Spies that had stopped matching the interfaces they name: `FsOps` gained `writeFile`, `LunacycleHealthCheckDeps` and `LunacycleSetupWebDeps` gained `secrets`, `IdpCapability` gained `list_tokens`/`revoke_token`. **Was anything silently passing?** No — and I checked rather than assumed. Each hook genuinely never calls the missing member, so the tests were testing what they claimed; they just no longer described the type. Rather than take that on faith I stubbed `FsOps.writeFile` on the backup/restore spies and the two idp methods to **throw** instead of no-op. All 62 hook tests still pass, which is now evidence rather than an assertion. The `secrets` drift did surface something real, though it's a coverage hole rather than a false pass: **every** call site in `health-check.test.ts` passes `secrets: {}`, so every test takes the `!botPassword` skip branch and the entire mint-token → SPA/API/WS smoke path has no unit coverage at all. Out of scope here, filed as #53 with the seams that make it testable. ## The tsconfig Inlines celilo's `modules/tsconfig.scripts.base.json` — a standalone module repo cannot extend a file that lives inside the celilo checkout. Two deliberate choices carried over or added: - **No `paths` for `@celilo/capabilities`** (celilo's own comment explains why): the hooks must resolve it from this directory's `package.json`, exactly as they do on the fleet. Mapping it elsewhere would typecheck against code the module never runs. - **Type packages come from the root install** via `typeRoots`, not from `celilo/scripts/package.json`. That directory's `node_modules` is bundled into the `.netapp` and shipped to the fleet, where nothing ever runs `tsc` — `@types/*` there would be dead weight on every deploy. ## The gate `typecheck` now ends in `typecheck:hooks`, which installs `celilo/scripts`' deps before running `tsc -p celilo/scripts`, mirroring what `test:hooks` already does. That makes it work on a fresh checkout regardless of where it sits in `pr-validate`'s step order, so no workflow reordering was needed — verified by deleting `celilo/scripts/node_modules` and running it cold. **Proved it fails.** Appended `const x: number = "not a number";` to `health-check.ts`: `bun run typecheck` exits **2** and names the file and line. Removed it: exits **0**. (Checked the exit code, not just the printed output — a gate that prints errors and exits 0 is not a gate.) ## Gates - `lint` — exit 0 - `typecheck` — exit 0, now including 16 hook files that had never been compiled - `test:unit` — 117 pass, 0 fail, 3 todo - `test:hooks` — 62 pass, 0 fail - `test:seed` — 1 pass, 0 fail ## Upstream The scaffolder gap behind this is filed as celilo#881 — `scaffold_module` emits no `scripts/tsconfig.json`, so every operator-owned module starts un-typecheckable and celilo only warns about it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

forgejo-admin added 1 commit

2026-08-17 22:21:28 +00:00

fix(ci): typecheck the celilo hook scripts, and fix the 18 errors behind it

pr-validate / validate (pull_request) Successful in 39s

Details

2116470cdd

celilo/scripts had no tsconfig.json and was in none of the four projects
`bun run typecheck` compiles. The code that runs against the fleet during
install, health check, backup and restore had never been through tsc. `bun test`
runs it, but bun strips types without checking them, so a green test:hooks
proved nothing about types.

Three of the eighteen were in live code, not test scaffolding:

- celilo/scripts pinned playwright-core ~1.55.1 while the root pinned ~1.60.0,
  so the Browser the health-check hook launched was a different type from the
  Browser scripts/smoke/spa-smoke.ts accepts. Aligned to ~1.60.0, which also
  puts the production smoke check on the same playwright the e2e suite
  validates against. This changes a runtime dependency bundled into the .netapp.
- ApiSmokeOptions.fetchImpl was typed `typeof fetch`, which carries preconnect.
  The injectable fetchers the seam exists for — celilo's Fetcher, test doubles —
  are plain functions without it, so the health-check hook could not pass its
  own injected fetch to its own smoke helper. Now typed as the call signature
  the function actually uses.

The other fifteen were spies drifted from the interfaces they name: FsOps gained
writeFile, LunacycleHealthCheckDeps and LunacycleSetupWebDeps gained secrets,
IdpCapability gained list_tokens/revoke_token. Nothing was silently passing —
each hook genuinely never calls the missing member, which the throwing stubs on
FsOps.writeFile and the idp methods now prove rather than assume. The secrets
drift did expose a real coverage hole, filed separately as #53.

The tsconfig inlines celilo's modules/tsconfig.scripts.base.json (a standalone
module repo cannot extend a file inside the celilo checkout) and deliberately
keeps no `paths` for @celilo/capabilities, so the hooks resolve it from their
own package.json exactly as they do on the fleet. Type packages come from the
root install rather than this directory's package.json: celilo/scripts/
node_modules is bundled into the .netapp, where nothing ever runs tsc.

`typecheck:hooks` installs celilo/scripts' deps first, mirroring test:hooks, so
it works on a fresh checkout regardless of step order in pr-validate. Verified
the gate fails: a deliberate `const x: number = "s"` in health-check.ts exits 2,
and clean exits 0.

Closes #52

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

forgejo-admin referenced this pull request

2026-08-17 22:42:16 +00:00

test(celilo): cover the health_check smoke path, which had no tests at all #55

forgejo-admin referenced this pull request

2026-08-17 22:46:10 +00:00

chromium.launch() is unguarded — a browser-cache miss crashes health_check instead of recording a check #56

forgejo-admin referenced this pull request

2026-08-17 22:46:34 +00:00

The OIDC login bypasses the hook's injected fetch — the seam exists but is not in the path #57

forgejo-admin merged commit 09e931dbdc into main

2026-08-18 06:14:07 +00:00

forgejo-admin referenced this pull request from a commit