ci: give the api-surface build the tb-addon backend URLs - #140
Conversation
The API-surface gate's Build step runs `pnpm -r build`, which reaches apps/tb-addon, whose build fail-closes without PKG_URL/CRYPTIFY_URL/POSTGUARD_WEBSITE_URL. It is now the fourth build step in this file to need them. #135 and #137 were each green in isolation and broke in combination: #135 added this build step before the tb-addon import existed, and #137 added the app that makes the step's preconditions stricter. Neither PR's CI could see the other. Delivery on main has been failing since both merged, which also skips the Release job — so this blocks publishing.
|
Thanks for the detailed writeup, @rubenhensen — I'm on it. Starting a review of this PR now, focusing on the guard-test pattern from postguard#272 and whether it properly closes the gap that let #135/#137 combine into a broken |
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Correct fix, and I reproduced both halves locally: pnpm -r build with the three vars unset fails at apps/tb-addon/build.mjs:14, and it passes with them set. All 16 checks are green. Approving so main's Delivery stops blocking the release.
Two things worth folding in before merge, neither blocking.
The api-surface job can drop the env block entirely. Details in the inline comment. pnpm api:gate reads exactly one build artifact, so pnpm --filter @e4a/pg-js build passes with all three vars unset. That takes this job out of the recurring class you describe, without waiting for the guard-test pattern from postguard#272.
If the recursive build stays, the block is now duplicated four times in integration.yml (lines 41, 89, 135, 182), with the production URLs hardcoded as fallbacks in each. A URL change becomes a four-place edit in one file, and each future build step is another chance to forget. A single job-level or workflow-level env: collapses all four; a per-job env: is the form I would trust without first checking context availability. delivery.yml:62 needs to keep its own copy, since pnpm release genuinely builds the addon, so that one is not duplication.
Separate note on CLAUDE.md:107: it still says api:gate is NOT yet a CI step and that a maintainer has to apply the job patch from #135. #135 did land it, at integration.yml:63-96, which is the job this PR edits. CLAUDE.md is auto-loaded agent context, so as written it tells the next agent to run api:gate locally on the belief that CI does not. Worth correcting in this PR.
|
Approved #140 — no blockers, so I reproduced the diagnosis locally: Three notes went along with the approval, not blocking this PR:
Agreed the guard-test pattern from postguard#272 is the right long-term fix for this class of cross-PR breakage. |
Both non-blocking points from review. api-report.mjs reads exactly packages/pg-js/dist/index.d.mts and nothing from apps/*, so the gate never needed a workspace-wide build. Filtering to `pnpm --filter @e4a/pg-js build` removes this job from the class where a new app's build preconditions silently become this workflow's — which is a better outcome than adding the fourth copy of the workaround — and drops a website + addon build the Node/Bun/Deno lanes already cover. Verified with all three vars unset: filtered build exits 0 and the gate passes. The three remaining recursive builds now inherit one workflow-level env block instead of carrying three copies with the production URLs hardcoded in each. delivery.yml keeps its own copy, since `pnpm release` genuinely does build the addon.
|
Took your suggestion over my own fix — it is the better one, and I verified the claim rather than trusting it: That is a genuinely better outcome than what I pushed. My fix added the fourth copy of a workaround for an invariant that lives in another file; yours removes the job from that class entirely, and makes it cheaper by dropping a website and addon build the Node, Bun and Deno lanes already cover. Also collapsed the duplication you flagged: the three remaining recursive builds now inherit one workflow-level Both verified locally: the api-surface path passes with no env set, and the recursive lanes still build with the inherited values. |
1 participant
Main's
Deliveryhas been red since #135 and #137 both merged, which skips theReleasejob — so this blocks publishing.Cause: the API-surface gate's
Buildstep runspnpm -r build, which now reachesapps/tb-addon, whose build deliberately fail-closes withoutPKG_URL/CRYPTIFY_URL/POSTGUARD_WEBSITE_URL. This is the fourth build step inintegration.ymlto need them; I patched the other three when the addon landed, anddelivery.ymlafter review caught it there.Why no CI caught it: #135 and #137 were each green in isolation and broke in combination. #135 added this build step before the addon existed; #137 added the app that makes the step's preconditions stricter. Neither PR's checks could see the other's change.
Verified locally:
pnpm -r buildwith the vars unset reproduces main's failure, and passes with them set.The generalisable fix is the guard-test pattern in encryption4all/postguard#272 — a gate asserting its own wiring, rather than each new build step having to remember an invariant that lives in a different file. This class will recur on B4 and B5.
Part of encryption4all/postguard#247.