ci: worker-smoke gate on every PR + label-gated per-PR full-stack environments#90
Draft
xBalbinus wants to merge 15 commits into
Draft
ci: worker-smoke gate on every PR + label-gated per-PR full-stack environments#90xBalbinus wants to merge 15 commits into
xBalbinus wants to merge 15 commits into
Conversation
The existing ci job typechecks and runs vitest against a better-sqlite3 shim, so DOs, real D1 semantics, migrations, middleware and routing were never exercised pre-merge. scripts/ci-worker-smoke.sh (modeled on otel-e2e.sh) builds @valet/shared+@valet/sdk, generates plugin registries, sed-substitutes wrangler.toml placeholders into a throwaway config, applies all D1 migrations + seeds the test API token into an isolated --persist-to dir, boots wrangler dev (wrangler 4 via npx — the repo 3.x pin lacks local workflows waitForEvent), polls /health, and runs tests/smoke/api.test.ts. agent-*.test.ts are excluded (they need a real Modal backend). Cleanup kills only the PID tree it started. api.test.ts's workflow-sync payload predates dag/v1 validation on /api/workflows/sync and 400s against any current worker; updated to the minimal valid DAG from scripts/smoke-dag.sh.
packages/runner and packages/plugin-{github,gmail,slack,telegram} have
passing suites but were missing from the root projects list, so 'pnpm
test' (and CI) silently skipped them. plugin-google-workspace is left
out: its labels-guard classification tests fail on main
(docs.find_text_index is classified in two categories).
The port pre-check ran after `trap cleanup EXIT` was registered, and cleanup() ends with a port-scoped `lsof | kill -9` sweep. On the refuse-to-start path (port held by a foreign process) the trap therefore SIGKILLed the very process the script had just declined to steal the port from — reproduced locally against a python http.server. Moving the check above the mktemp/trap block means the refusal path creates no state and arms no trap, so the sweep's invariant (anything on $PORT is ours) actually holds. Also in ci.yml: cache ~/.npm/_npx keyed on the smoke script so the wrangler@4 npx download isn't an uncached registry dependency in a required PR gate, and place worker-smoke before the ci job so the new hunk doesn't anchor at EOF where parallel branches append theirs.
Template the hard-coded Cloudflare Workflows binding name as
${WORKFLOW_NAME} (account-scoped — N per-PR workers would collide) with
a default that keeps the generated dev/prod config byte-identical.
pr-deploy stamps a full stack (Worker + D1 + R2 + Workflow + Pages) from
PROJECT_NAME, running migrations BEFORE the first worker deploy because
the minutely cron fires as soon as the worker exists. Secrets go up
non-interactively via stdin: fresh ENCRYPTION_KEY per env, passthrough
GOOGLE_*/GITHUB_* from the caller. pr-destroy refuses PROJECT_NAMEs
without '-pr-' so it can never touch dev/prod.
/api/workflows/sync validates data as a dag/v1 definition since the interpreter landed; the legacy steps[] payload has been failing with VALIDATION_ERROR against any current worker.
Adding the 'preview-env' label stamps valet-pr-<N> (worker + D1 + R2 + Workflow + Pages), runs the api.test.ts smoke suite against it with the seeded token, and comments the URLs on the PR. Destroy fires on close AND on label removal — teardown is mandatory because every per-PR worker runs the minutely cron. Both workflows share the pr-env-<N> concurrency group so a destroy cancels an in-flight deploy.
Live teardown showed 'wrangler delete' leaves the account-scoped
Workflow behind ('wrangler workflows list' still showed
valet-pr-9999-wfi). The pinned wrangler 3.114 has no 'workflows delete',
so remove it via the REST DELETE endpoint when CLOUDFLARE_API_TOKEN is
present (CI always is). Endpoint verified live against the orphan.
The default pull_request checkout ref (refs/pull/N/merge) does not exist for unmergeable PRs and can vanish right after close — checkout would fail and the teardown never run, leaking a cron-billing env. Teardown needs nothing from the PR's code, so pin to main.
pr-destroy tolerated every deletion failure (needed for 'already gone' idempotence) which also swallowed real API failures — a worker that survives teardown keeps billing the minutely cron with a green exit. Verify the worker is actually gone at the end and exit non-zero if not. pr-deploy regenerated ENCRYPTION_KEY on every run, but D1 survives synchronize redeploys — rotation invalidated JWT sessions and orphaned credentials encrypted under the old key. Generate only when the secret is absent (verified live: redeploy over a live env keeps the key).
The catch-all placeholder scrub turned the newly-templated
${WORKFLOW_NAME} into name = "" in wrangler.e2e.toml. The worker
still boots, but the Workflows binding pointed at a nonexistent name
and any future e2e touching WORKFLOW_INTERPRETER would fail
confusingly.
GOOGLE_CLIENT_ID/SECRET are not part of deploy-dev.yml's secret set (dev's worker secrets were bootstrapped once, not per-deploy) and are currently absent from the dev environment — deploy.sh silently skips empty passthrough values, so the env would deploy green with no Google OAuth secrets. Surface a workflow warning instead. GITHUB_* cannot be Actions secrets at all (reserved prefix), documented inline.
Both this branch and ci/boot-worker-smoke fix the same stale pre-dag/v1 workflow-sync payload; identical content lets the branches merge cleanly in either order.
|
Preview deployment: https://pr-90.dev-valet-turnkey-client.pages.dev |
The worker-smoke script predates the templated Workflows binding name; without an explicit substitution the catch-all sed blanked it to name = "". Mirrors the otel-e2e.sh substitution.
xBalbinus
changed the title
Author
|
Converting to draft — deprioritized for now; will revisit after the current platform sprint. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI test environments: worker-smoke gate + label-gated per-PR full-stack envs
One story in two layers — today only the frontend gets a per-PR preview (wired to the shared dev worker running main), so backend changes are invisible pre-merge. This PR closes that gap at both depths:
worker-smokeCI job boots the real worker locally in CI — DOs, real D1 semantics, all migrations, Hono middleware — and runs the API smoke suite.preview-envlabel): a full ephemeralvalet-pr-Nstack (worker + D1 + R2 + Workflows + Pages) deployed to Cloudflare, smoke-tested, URL-commented, and destroyed on close/unlabel.Layer 1 —
worker-smokeon every PRscripts/ci-worker-smoke.sh(modeled onotel-e2e.sh): registry gen → build shared+sdk → sed wrangler.toml placeholders into a throwaway config →wrangler@4.107.0 devon :8794 with hermetic--persist-tostate → apply all 25 migrations → seed the test API token → poll/health→ runtests/smoke/api.test.ts. Agent-dispatching smoke tests (agent-*.test.ts) are excluded — they need a real Modal backend (that's what Layer 2 + shared dev Modal is for).Also fixes
tests/smoke/api.test.ts(its workflow-sync payload predated dag/v1 validation and 400'd against any current worker) and addspackages/runner+ green plugin suites to the root vitest projects (they silently didn't run in CI).plugin-google-workspaceleft out: 2 pre-existing failures on main — needs its own fix.Layer 2 — per-PR ephemeral environments
wrangler.toml: the one hard-coded account-scoped name (valet-workflow-interpreter) templated as${WORKFLOW_NAME}; dev/prod generated configs verified byte-identical before/afterdeploy.sh:pr-deploy/pr-destroy(+make deploy-pr-env/destroy-pr-env); migrate before deploy (doesn't copy cmd_all's old ordering bug); freshENCRYPTION_KEYon first deploy, kept on redeploys (D1 persists across pushes); teardown verifies the worker is actually gone and removes the orphaned per-PR Workflow via REST.github/workflows/deploy-pr-env.yml(labeled/synchronize/reopened, same-repo, concurrency per PR) +destroy-pr-env.yml(closed/unlabeled; checkout pinned tomainso teardown survives unmergeable PRs)docs/deployment.md§ Per-PR preview environments (incl. the minutely-cron cost note that makes teardown mandatory)Evidence
${WORKFLOW_NAME}substitution the merge exposed; review caught the EXIT trap SIGKILLing a foreign process on the port-occupied path — reproduced live, fixed, re-verifiedvalet-pr-9999cycles on the real CF account: D1 created + 25 migrations → worker deployed →/health200 → seeded-token API calls → smoke green → destroy verified (deployments list errors, novalet-pr-9999-db, R2/Pages gone); redeploy cycle idempotentworker-smokejob end-to-endBefore first real use
The dev GitHub environment is missing
GOOGLE_CLIENT_ID/GOOGLE_CLIENT_SECRET(verified absent at env + repo level — dev's worker secrets were set out-of-band). The deploy job warns rather than fails, but add them (or accept 500s on Google-touching paths in PR envs). First labeled run should be watched.Before the first labeled run
preview-envto any PR until this merges —destroy-pr-env.ymlchecks outmain, which has nopr-destroypath yet, so teardown would fail and leak the env (deploy runs from the PR branch and would succeed).preview-envlabel (doesn't exist in the repo yet).GOOGLE_CLIENT_ID/GOOGLE_CLIENT_SECRETto the dev GitHub environment (deploy warns rather than fails without them).CLOUDFLARE_API_TOKENscopes to cover D1/R2/Pages create+delete and Workflows delete — insufficient scope surfaces on the first labeled PR.pr-destroytolerates a non-empty R2 bucket (wrangler refuses to delete it), so a PR env that wrote to R2 leaks its bucket — storage-only, the cron-billing worker is verified deleted.