smoke-handler's celiloGet returns "key = value", so every post-deploy smoke URL is malformed #58

New issue

Open

opened 2026-08-17 22:56:06 +00:00 by forgejo-admin · 0 comments

forgejo-admin commented

2026-08-17 22:56:06 +00:00

Owner

celiloGet() in celilo/scripts/smoke-handler.ts:146-162 shells out to celilo module config get <module> <key> and treats stdout as the bare value. It is not. celilo module config get returns `${key} = ${formatted}` (apps/celilo/src/cli/commands/module-config.ts:365 in the celilo repo), and it never sets rawOutput: true.

Verified against the live fleet just now:

$ celilo module config get lunacycle idp_base_url
idp_base_url = https://auth.lunacycle.net

$ celilo module config get lunacycle auto_upgrade
auto_upgrade = false

So every value the handler reads carries a key = prefix, and every URL built from one is garbage:

domain → "domain = lunacycle.net"
wwwHost = `www.${domain}` → "www.domain = lunacycle.net"
authentikUrl → "idp_base_url = https://auth.lunacycle.net"

The first fetch() — mintAccessToken — then fails with fetch() URL is invalid, which is the error recorded against the lunacycle.smoke-after-deploy bus subscriber.

Two things that hide it

The guard is defeated by the format it was meant to catch. if (!domain) throw new Error('lunacycle smoke: domain not configured …') exists to fail loudly on missing config — but "domain = lunacycle.net" is truthy, so it passes and the failure surfaces much later as an unrelated-looking fetch error.

stripCliDecoration does not help and its comment says why it should. It only strips ANSI escapes (if (!raw.includes('\x1b')) return raw;) and does nothing about the key = prefix. Its comment reasons: "As a child of bun /path/handler.ts, stdout is a pipe → rawOutput-mode commands write the raw value." Two problems — module config get is not a rawOutput command, and per celilo's cli/index.ts:2568, rawOutput "no longer selects a different destination" anyway.

Blast radius: the post-deploy smoke has never run

celilo events list-failed shows 8 failed deliveries for lunacycle.smoke-after-deploy, most recent from tonight's 1.4.0+1 deploy. It only fires on deploy.completed.lunacycle, so 8 failures is plausibly every deploy it has ever seen.

Critically, this is invisible from the health side: the 15-minute health_check hook is a different code path that receives config through injected deps rather than shelling out, and it passes (smoke_spa: SPA fetched data and rendered (tRPC 200, 6 /api requests, 5404ms)). So the fleet looks healthy while the post-deploy smoke has silently never executed — the failure lives only in the event bus's failed-delivery table.

Scope

celilo/scripts/smoke-handler.ts:146-162 — celiloGet().
:164-171 — stripCliDecoration(); its stated premise about rawOutput is wrong and the comment should go with the fix.
:58 (domain), :81 (idp_base_url), and the botPassword secret read — check whether celilo module secret get has the same shape.
Every URL derived from those: wwwHost, authentikUrl, redirectUri, and the API/WS smoke targets.

Acceptance

celiloGet returns the value only, parsing the documented key = value shape (or uses a machine-readable mode if celilo grows one) — and does not silently accept a value it failed to parse.
A value that arrives in an unexpected shape fails loudly rather than flowing into a URL; the !domain guard actually catches malformed config.
celilo module secret get's output shape is confirmed rather than assumed.
Recurrence gate: a unit test feeding celiloGet's parser the real "idp_base_url = https://auth.lunacycle.net" string and asserting it yields https://auth.lunacycle.net. Prove it fails against the current implementation first — it currently returns the whole line.
Re-run the handler after a deploy and confirm lunacycle.smoke-after-deploy records a success, not a failed delivery.

Found while investigating 2279 failed bus deliveries on celilo-mgr after the celilo v1.0.0 rollout; this subscriber was the freshest failing one.
Same family as lunacycle#57 (a seam declared but not wired): here a parsing contract is assumed but never verified. Neither is visible to any existing gate.
Consider asking celilo for a value-only output mode for module config get — that would remove the parsing entirely rather than making this handler better at guessing.

`celiloGet()` in `celilo/scripts/smoke-handler.ts:146-162` shells out to `celilo module config get <module> <key>` and treats stdout as the **bare value**. It is not. `celilo module config get` returns `` `${key} = ${formatted}` `` (`apps/celilo/src/cli/commands/module-config.ts:365` in the celilo repo), and it never sets `rawOutput: true`. Verified against the live fleet just now: ``` $ celilo module config get lunacycle idp_base_url idp_base_url = https://auth.lunacycle.net $ celilo module config get lunacycle auto_upgrade auto_upgrade = false ``` So every value the handler reads carries a `key = ` prefix, and every URL built from one is garbage: - `domain` → `"domain = lunacycle.net"` - `wwwHost` = `` `www.${domain}` `` → `"www.domain = lunacycle.net"` - `authentikUrl` → `"idp_base_url = https://auth.lunacycle.net"` The first `fetch()` — `mintAccessToken` — then fails with **`fetch() URL is invalid`**, which is the error recorded against the `lunacycle.smoke-after-deploy` bus subscriber. ## Two things that hide it **The guard is defeated by the format it was meant to catch.** `if (!domain) throw new Error('lunacycle smoke: domain not configured …')` exists to fail loudly on missing config — but `"domain = lunacycle.net"` is truthy, so it passes and the failure surfaces much later as an unrelated-looking fetch error. **`stripCliDecoration` does not help and its comment says why it should.** It only strips ANSI escapes (`if (!raw.includes('\x1b')) return raw;`) and does nothing about the `key = ` prefix. Its comment reasons: *"As a child of `bun /path/handler.ts`, stdout is a pipe → rawOutput-mode commands write the raw value."* Two problems — `module config get` is not a rawOutput command, and per celilo's `cli/index.ts:2568`, `rawOutput` "no longer selects a different destination" anyway. ## Blast radius: the post-deploy smoke has never run `celilo events list-failed` shows 8 failed deliveries for `lunacycle.smoke-after-deploy`, most recent from tonight's 1.4.0+1 deploy. It only fires on `deploy.completed.lunacycle`, so 8 failures is plausibly every deploy it has ever seen. Critically, this is **invisible from the health side**: the 15-minute `health_check` hook is a different code path that receives config through injected deps rather than shelling out, and it passes (`smoke_spa: SPA fetched data and rendered (tRPC 200, 6 /api requests, 5404ms)`). So the fleet looks healthy while the post-deploy smoke has silently never executed — the failure lives only in the event bus's failed-delivery table. ## Scope - `celilo/scripts/smoke-handler.ts:146-162` — `celiloGet()`. - `:164-171` — `stripCliDecoration()`; its stated premise about rawOutput is wrong and the comment should go with the fix. - `:58` (`domain`), `:81` (`idp_base_url`), and the `botPassword` secret read — check whether `celilo module secret get` has the same shape. - Every URL derived from those: `wwwHost`, `authentikUrl`, `redirectUri`, and the API/WS smoke targets. ## Acceptance - [ ] `celiloGet` returns the value only, parsing the documented `key = value` shape (or uses a machine-readable mode if celilo grows one) — and does not silently accept a value it failed to parse. - [ ] A value that arrives in an unexpected shape **fails loudly** rather than flowing into a URL; the `!domain` guard actually catches malformed config. - [ ] `celilo module secret get`'s output shape is confirmed rather than assumed. - [ ] Recurrence gate: a unit test feeding `celiloGet`'s parser the real `"idp_base_url = https://auth.lunacycle.net"` string and asserting it yields `https://auth.lunacycle.net`. Prove it fails against the current implementation first — it currently returns the whole line. - [ ] Re-run the handler after a deploy and confirm `lunacycle.smoke-after-deploy` records a success, not a failed delivery. ## Related - Found while investigating 2279 failed bus deliveries on celilo-mgr after the celilo v1.0.0 rollout; this subscriber was the freshest failing one. - Same family as lunacycle#57 (a seam declared but not wired): here a parsing contract is assumed but never verified. Neither is visible to any existing gate. - Consider asking celilo for a value-only output mode for `module config get` — that would remove the parsing entirely rather than making this handler better at guessing.