ci: bring up what init generates and assert the database persists - #192
Merged
Conversation
Every other job asserts the generated files as strings, so nothing executed them. A compose file could be well-formed and wrong and still pass CI, review, and the conformance matrix, which is how the postgres:18 mount regression in #164 shipped: the harness's own postgres mounts no volume, so it exercised the in-image default rather than the broken mount. Adds scripts/scaffold-smoke.sh and a workflow that runs it on every PR. It scaffolds with --local --yes --auth=docker --admin=none, brings up db and auth from the generated compose, writes a row, recreates the container with the volume kept, and reads it back. Bring-up alone is not the assertion. A volume mounted where the image does not store data leaves a database that starts, reports healthy, and silently writes to the container layer, so the read-back is what catches that class. The scaffold step retries with backoff: init downloads the template registry, the templates archive and the auth .env.example, and this job is about the generated compose rather than GitHub's availability. Closes #166
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
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.
Closes #166
The gap
Every job in CI asserts the generated files as strings. Nothing executed them, so a compose file could be well-formed and wrong and still pass
Build and unit test, passverify / verify, pass review, and merge. That is how #164 shipped: the harness's own postgres service mounts no volume, so the conformance matrix exercised the in-image default rather than the broken mount, and it was only found when someone randocker compose upby hand.What this adds
scripts/scaffold-smoke.sh, run by a new workflow on every PR:init smoke --local --yes --email=... --auth=docker --admin=nonedocker compose up -d --wait db authdown(volume kept),upagain, read it backdown -v, always, via a trapOnly
dbandauthcome up. Both have healthchecks, so--waitmeans something, and neither needs annpm install. Bringing upapiandwebwould mostly measure the templates and roughly triple the runtime; that is the "first cut" the issue suggests.Verified against three real scenarios
The issue asks for confirmation that a deliberate regression fails the job, on the grounds that a smoke test which cannot fail is worse than none. Run locally against Docker:
mainPASS, exit 0/var/lib/postgresql/data)up --waitB is the one that justifies the design. Bring-up passes completely, four containers report healthy, and the failure only appears at the read-back:
Worth correcting one detail in the issue, though: it predicts C would also start and silently not persist. On
postgres:18the container actually refuses to start, andup --waitcatches it. The persistence check is still what earns its place, because it is the only thing that catches B.Two properties of the generated compose this surfaced
Both are pre-existing and neither is changed here, but they shaped the script:
container_name.seamless-dbandseamless-authare pinned, so-p <project>does not isolate a run. A leftover container produces a raw Docker name conflict, so the script preflights and says what is wrong. It deliberately does not remove containers it did not create; on a developer's machine those are their work.verify.SMOKE_EXTRA_COMPOSElets a developer supply an override to run it beside an existing stack, which is how A, B and C above were exercised on a machine already using 5432.Flakiness
initfetches the template registry, the templates archive, and the auth.env.example. That failed intermittently during development, always recovering, so the scaffold step retries up to four times with exponential backoff. This job should fail for what the compose file does, not for GitHub being briefly unavailable.On the
seamless checkexit codeThe issue offers two branches: give
checka non-zero exit, or have the smoke job assert independently. This takes the second, so the job does not depend on that decision.The first is still worth doing,
checkis what anyone would reach for in a health-check script and it always exits 0, so I am sending it as a separate PR adding--strictrather than mixing a user-facing CLI flag into a CI change. It touches all thirteen check sites and deserves its own review.Verification
npm run buildandnpm testpass. AGENTS.md documents the job, why bring-up is not the assertion, and how to run it locally.