feat(image): put facility on the PATH in the api image - #58
Merged
Conversation
The image already carries the CLI so operator commands can run as one-shot tasks inside the VPC, which is the only place the database is reachable in the reference deployment. But it could only be reached as a path into the image, so the runbook's bootstrap step spells `node cli/bin/facility.mjs instance bootstrap` — in the middle of a JSON container override, at the step where a mistake means every sign-in fails with `not_invited`. Now it is `facility instance bootstrap`, the way it reads everywhere else. A wrapper in /usr/local/bin rather than a symlink: the checked-in bin is not executable, and an ECS command override runs without a shell to resolve it. The build-time guard now goes through the wrapper, so the PATH entry cannot rot silently. Verified against the built image: `facility …` as a command, as an entrypoint with no shell — which is how ECS invokes it — and the runbook's exact bootstrap against a real database, creating the organization and then reporting created=false on a second run.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3023dc1d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The bootstrap step binds the first organization, owner, and installation, and a command the image cannot resolve fails it — then every sign-in, with `not_invited`. Nothing caught the two halves drifting apart: revert the Dockerfile hunk and the suite stays green while the published runbook keeps telling operators to run `facility instance bootstrap`. The runbook test now reads the override out of the runbook and the api stage out of the Dockerfile, and asserts they name the same executable. It fails on either half of this pull request being reverted. The build guard moves to exec form, which resolves the name the way the container runtime does for an ECS command override, with no shell in between — so the guard exercises the same lookup the runbook depends on, instead of `sh -c`'s. Verified against the built api image: as an ECS command override with `--entrypoint facility`, and with the runbook's own command array, which reaches the CLI and stops at the missing DATABASE_URL. Addresses: #58 (comment)
javiertoledo
added a commit
that referenced
this pull request
Aug 2, 2026
…ecks (#59) * ci(release): pass the repository visibility the publication policy checks `scripts/release.mjs` reads the repository visibility from GITHUB_REPOSITORY_VISIBILITY and refuses to publish unless it is `public`. The runner exposes no such variable of its own, so the two npm-side steps that never passed it saw it unset and read a public repository as private. The job-level `if: github.event.repository.visibility == 'public'` hid this: it is evaluated by Actions, from the event payload, so the job started correctly and then the policy inside it refused. `images.yml` already passes the variable explicitly, which is why image publication was never affected. First observed on the v0.3.1 run for #58, which failed closed at `package-release` — nothing published, no tag. Fixing only ci.yml would have moved the same failure into publish-npm's revalidation, so both steps get it. Verified by calling `validateReleasePolicy` with the release's own inputs: unset reproduces `npm publishing is disabled until the repository is public`, and `public` validates. * test(release): fail when a validate step drops the visibility binding The publication policy is unit-tested and the workflows are asserted on at length, but nothing connected the two: the integration test hands GITHUB_REPOSITORY_VISIBILITY straight to the CLI, so it exercises the policy while bypassing the wiring, and deleting either binding left both suites green. That is exactly how the v0.3.1 run reached `package-release` before refusing. Assert the binding on every step that runs `release.mjs validate`, in both workflows, and add the unset case to the denial table — unset is the mode CI actually hit, and it is indistinguishable from a private repository to the policy. Verified by removing each binding in turn: both make this test fail. Addresses: #59 (comment)
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.
A small, real improvement — and deliberately the one that cuts the first automatic release, so we watch the whole pipeline run once with both of us looking at it.
The change
The api image already carries the CLI, because
facility instance bootstrapneeds the database and in the reference deployment the database accepts connections only from the service security group — a one-shot ECS task is the only place it can run. But it could only be reached as a path into the image, so the runbook spells:in the middle of a JSON container override, at the step where getting it wrong means every sign-in fails with
not_invited. Now it is"facility","instance","bootstrap", ….A wrapper in
/usr/local/binrather than a symlink, for two reasons that are easy to trip over: the checked-in bin is not executable, and an ECS command override runs without a shell to resolve it. The build-time guard now goes through the wrapper, so the PATH entry cannot rot silently — if it breaks, the image build fails rather than the deployment.Verified against the built image
docker run … facility instance bootstrap --help— the command resolves with no path;docker run --entrypoint facility …— how ECS invokes it, no shell involved;{"ok":true,"created":true,…}, then"created":falseon a second run.Guards, lint and the 86 script tests pass.
What merging this does
decide-releasereadsfeat(image): …sincev0.3.0and computes 0.3.1 — a patch, since pre-1.0 only a breaking change is a minor. Then: acceptance, stamp and pack, publish to npm with the bootstrap token, publish the six images, and finally write thev0.3.1tag with its release notes.Which leaves one thing for a maintainer right after: the six GHCR packages are created private, and making each public is a one-time manual step.