Skip to main content

D103 — Companion preview labels are a desired-state toggle, and workflows are linted by actionlint (RESOLVED)

Architecture decision record. Status, thematic clusters, and how to record a new ADR: the decision log index. Two decisions shipped in one change to the delivery pipeline: the preview-* label lifecycle, and a workflow-lint gate that guards the workflows implementing it. The workflow files (.github/workflows/destroy-companion.yml, deploy-preview.yml, destroy-preview.yml, ci.yml) and the lint-workflows Makefile target are the canonical source of truth; this ADR records the rationale. Reviewer-facing instructions for companion previews live in docs/preview-access-guide.md; the operational view of the lint gate is on Testing & the gates.

Context

Companion previews. A backend preview environment (https://pr-<N>.preview.dev-stables.xyz) can carry a companion frontend preview, requested by putting a label on the backend PR: preview-web or preview-backoffice. Adding the label makes deploy-preview.yml's companions job repository_dispatch a companion-deploy to the matching frontend repo (stablesxyz/stables-core-web / stables-core-web-backoffice), whose companion.yml builds a …-bepr-<N> worker pointed at this PR's backend.

Two gaps followed from labels being handled on labeled only:

  1. The label was a one-way switch. Removing it did nothing, so an unwanted companion could only be removed by closing the PR (destroy-preview.yml tears companions down on close). The label read like state but behaved like a button.
  2. Rebuilding a companion required a backend commit. The frontend repos' main moves while a backend PR is open, and their companion.yml deploy path builds from that repo's main — so a companion built on day one keeps serving day-one frontend code until something re-dispatches. Something already did: deploy-preview.yml triggers on synchronize, and its companions job runs whenever the deploy succeeded and the event was not labeled, re-dispatching companion-deploy for whichever preview-* labels are present. So any push to the backend PR already rebuilt the companion. That was the only re-trigger, which is the actual gap: it is unavailable when you have nothing to push, and it couples a frontend refresh to a backend commit for no reason. (No further labeled event can fire while the label is already present, so the label offered no path of its own.)

Workflow linting. The delivery pipeline is now substantial CI/CD logic expressed in YAML — a change-detection classifier (D101), a compose-stack test job on third-party runners (D100), preview deploy/teardown, and now a fifth preview-lifecycle workflow. Workflow mistakes are expensive because they are only discovered by pushing: an invalid activity type, a typo'd steps.<id>.outputs.* reference, or an unknown runs-on: label costs a round-trip through GitHub — or worse, silently never fires. Nothing in the repo validated the workflows before push.

Decision 1 — Removing a preview-* label tears the companion down

A dedicated destroy-companion.yml workflow runs on pull_request: [unlabeled] and dispatches companion-destroy to the frontend repo matching the removed label, making the label a desired-state toggle rather than a one-way switch. Remove + re-add therefore becomes a second way to rebuild a companion against the frontend repo's current main — the first being a push to the backend PR, which already re-dispatched (gap 2 above) but is unavailable with nothing to push.

  • A separate workflow file, not another job in deploy-preview.yml — for concurrency safety. deploy-preview.yml's concurrency group is preview-pr-<N> with cancel-in-progress: ${{ github.event.action != 'labeled' }}, i.e. a synchronize cancels in-flight runs of that group. A teardown job living in that workflow would share the group, so a push landing while the teardown was queued could kill it — leaking an orphaned frontend worker with nothing left to point at. A separate workflow gets its own group and cannot be collateral damage.
    • The principle is not yet applied everywhere — one pre-existing case is left open. destroy-preview.yml (the PR-close teardown) uses group preview-pr-<N> with cancel-in-progress: false, but deploy-preview.yml uses the identical group with cancel-in-progress: ${{ github.event.action != 'labeled' }}, and the incoming run's flag is what governs cancellation. reopened is in deploy-preview.yml's types:, so reopening a PR while its close-triggered teardown is still running can cancel that teardown mid-flight — the same class of leak this decision avoided for the unlabel path. Pre-existing, not introduced by this change, and not fixed here; recorded so the rule above is not read as universally enforced. Fixing it means giving destroy-preview.yml its own concurrency group (which then needs thinking through against the deploy/teardown ordering that sharing the group currently provides), so it is a change of its own.
  • Per-label concurrency group, never cancelled: companion-teardown-pr-<N>-<label> with cancel-in-progress: false. Per-label so removing both labels back-to-back tears both down concurrently instead of queueing; never-cancel because a cancelled teardown is exactly the orphaned-worker leak above.
  • A live-label re-add guard, because the event payload is a snapshot. The unlabeled payload describes the moment of removal. The remove→re-add rebuild flow fires an unlabeled run and a labeled run, and if the teardown lands after the redeploy it would delete the companion that was just built. So the first step re-reads the live labels and stands down when the label is back. The guard is fail-closed: after 3 attempts (5s apart) it errors out rather than dispatching a possibly-stale destroy. It is retried at all for the same reason the deploy side is — a single transient API blip must not redden a teardown, because a red run skips the destroy and leaves the inverse leak: label gone, companion still live.
    • The numeric PR_NUMBER guard runs before the FIRST use of the value, not just before the cross-repo dispatch. The value is GitHub-supplied, so this is belt-and-braces — but the first thing interpolating it is now the label-read API URL, so the guard precedes that; the dispatch step re-validates before it reaches another repo's workflow (the same guard destroy-preview.yml applies).
    • The read is REST (gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER"), deliberately not gh pr view --json labels. gh pr view is a GraphQL call, and GraphQL's scope requirements are less certain than REST's, whereas /pulls/{n} is unambiguously covered by the job's pull-requests: read. Guessing wrong here would not have failed in one place: the same read was added to deploy-preview.yml's companions job, so an insufficient scope would have reddened every teardown and every preview deploy. Both sides read .labels[].name; they differ only in shape — a newline list for the teardown's grep -qxF, a JSON array ([.labels[].name]) for the deploy side's jq index(…).
    • The deploy side additionally asserts the payload is a JSON array, and fails closed otherwise. A zero exit is not sufficient there: an empty or non-array payload makes every jq index(…) test silently miss (jq's own non-zero exit is swallowed by the if condition), so the job would dispatch nothing while staying green — "label added, no companion, no error", the exact silent-success state this read exists to prevent. The shape check turns that into a red run.
  • Least-privilege token scope, declared at the job and nowhere else. The teardown job declares permissions: pull-requests: read and nothing else — enough for the REST label read, and deliberately not contents: read, because nothing in the job checks the repository out (add it back alongside any future checkout step). destroy-companion.yml therefore carries no workflow-level permissions: block at all, and a comment above jobs: says why: a job-level block replaces the workflow default rather than extending it, so with a single job a workflow-level declaration would be dead config that reads as if it were in force. The two steps differ only in which credential they bind to GH_TOKEN — the guard step uses github.token (the job-scoped default), the dispatch step the cross-repo REPO_DISPATCH_TOKEN, in its own step env:. deploy-preview.yml's companions job now carries the same permissions: pull-requests: read for the mirror-image reason — it reads live labels too, and previously ran with permissions: {}.
  • No state == 'open' guard (unlike the deploy path, which requires an open PR): a second destroy is a harmless no-op in the frontend repo, and destroy-preview.yml already tears companions down on close — so refusing to act on a closing PR would only create a gap.
  • The label→repo fallback separates two look-alike cases. The job-level if: allowlists the two labels and the dispatch step's case maps them to repos, but GitHub expressions compare strings case-insensitively while case (and the deploy side's jq index(…)) are case-sensitive — so a variant like Preview-Web passes the if and reaches the fallback arm. That is not a companion label (nothing ever deployed for it), so the arm re-tests the lowercased label: a case variant stands down green, while a label that is genuinely in the allowlist but unmapped — real drift, which would leak a companion — exits non-zero. Blanket-erroring would redden runs on harmless case variants; blanket-exiting-zero would hide the leak.
  • Every green exit explains itself in the run summary, on both sides. A ::notice:: is only visible by expanding a step, and all of these runs otherwise look like a green job that did nothing. So destroy-companion.yml writes a $GITHUB_STEP_SUMMARY block on all three of its green exits — the re-add stand-down, the case-variant no-op, and the accepted dispatch — and deploy-preview.yml's dispatch step now emits a ::notice:: plus a summary when nothing was dispatched, which was previously silent-green and indistinguishable from a successful dispatch without reading the log. The notable case for that last one is a label removed during the deploy, where the live read correctly finds it gone and the teardown is authoritative.
  • No frontend-side change was required. Both frontend repos' companion.yml already handle the companion-destroy event — destroy-preview.yml has been dispatching it on PR close since the preview pipeline landed; this decision only adds a second trigger for the same contract.

The PR comment posted by deploy-preview.yml now states the toggle semantics on its Frontends row ("remove it to tear that companion down; remove + re-add (or just push) to rebuild it from the frontend's current main"), so the affordance is discoverable where reviewers actually look.

Decision 2 — make lint-workflows gates the workflows with pinned actionlint

Lint every workflow with rhysd/actionlint, pinned by image digest and invoked through a make target that CI runs verbatim.

  • make lint-workflows runs actionlint in a one-shot container against a read-only whole-repo mount (-v "$(CURDIR)":/repo:ro), with --network none --security-opt no-new-privileges and -shellcheck=. It is the one make target that deliberately does not run in the app container, which has no actionlint. The sandboxing is not ceremony: the mount spans the whole repo (actionlint auto-discovers .github/ from the root), which locally includes the gitignored .env and the full git history — so the container gets no network, no privilege escalation, no write access and no secrets.
  • Pinned by digest, not tag. This is third-party code executing in CI, so it follows the same rule as the SHA-pinned actions in .github/workflows/* rather than docker-compose.yml's version-tag convention for dev-only official images. A tag on a personal Docker Hub namespace is repointable by its publisher, which would silently break the property this gate exists to have: a green run locally is a green run in CI. The version (1.7.12) is kept as a comment beside the digest; the two are updated together.
  • A pinned image via the make target, not a third-party GitHub Action. ci.yml already records that its change classifier is hand-rolled bash specifically to keep the supply-chain surface unchanged (D101); adding a marketplace action to lint the workflows would undo that for a convenience.
  • Wired as the FIRST step of ci.yml's existing static job (immediately after checkout, before setup-php). It needs no PHP, so a malformed workflow fails in seconds instead of after the Composer install. No new job, so no new required-status-check name to configure.
  • Runner labels are declared in .github/actionlint.yaml. actionlint only knows GitHub's own hosted labels, so the three blacksmith-{2,4,8}vcpu-ubuntu-2404-arm labels (D100) are declared as self-hosted-runner.labels; a new runner size needs a row.
  • The bundled shellcheck pass is OFF (-shellcheck=) — a deliberate, tracked deferral. With the pass enabled the workflows report 53 pre-existing findings, all audited as non-behavior-affecting: 46 SC2086 + 3 SC2046 quoting findings, 2 SC2016 (both are deploy-preview.yml's AWS --query JMESPath literals, where the single quotes are correct and the backticks inside them are JMESPath syntax, not command substitution — shellcheck cannot know that), and an SC2221/SC2222 pair flagging a shadowed case arm in ci.yml's own is_docs_only() classifier (the *.md arm subsumes the later .github/PULL_REQUEST_TEMPLATE.md arm; both return 0, so it is dead redundancy, not a misclassification). They sit mostly in deploy-preview.yml (36 of 53), then sync-secrets.yml, destroy-preview.yml and deploy-production.yml (4 each), deploy-staging.yml and ci.yml (2 each) and build-image.yml (1). Clearing them is its own change — and not a pure re-quoting job, since the SC2016 pair and the shadowed arm need their own treatment. With the pass off the baseline is zero findings, so the gate is green from day one and can never be "temporarily ignored". Tracked as docs/tracking/devops-hardening.md §8.

What the gate does and does not catch

Verified against actionlint 1.7.12 while adding the gate — the honest boundary matters, because a gate people over-trust is worse than no gate:

CaughtWorkflow schema errors; invalid event names and activity types; unknown runs-on: labels (unless declared in .github/actionlint.yaml); bad steps.<id>.outputs.* and other typed-context references; malformed ${{ }} expressions
Not caughtTypos in github.event.* property access — that context is loosely typed, so github.event.label.nmae passes clean. Also, with the shellcheck pass off, nothing inside run: bodies

It earned its place during development: the config was first written to the repo root as .actionlint.yaml, which actionlint silently ignores — the Blacksmith labels kept erroring until the file moved to .github/actionlint.yaml. A lint gate that is quiet about its own ignored config is a good argument for keeping the digest-pinned invocation identical everywhere.

Consequences

  • preview-web / preview-backoffice now mean what they look like. Label present → companion exists; label absent → it does not. Reviewers can drop a companion without closing the PR, and remove + re-add is a documented rebuild that needs no backend commit — a long-lived backend PR can pick up frontend main on demand, where before it had to push to get the same effect.
  • The rebuild mechanism ships; rebuild observability does not — tracked, not delivered. The motivating problem was frontend staleness, and nothing in this change makes staleness visible or a rebuild confirmable. The row the frontend repos write into the Companion Frontend Previews comment is derived only from the backend PR number (| stables-core-web | https://bepr-<N>-web… |) — no SHA, no timestamp — so a rebuild rewrites a byte-identical row; on the fast remove→re-add path the teardown stands down so the row never disappears either; and the old build keeps serving during the rebuild (whose ceiling is the frontend repo's own companion.yml job timeout — 15 minutes at the time of writing; nothing in this repo pins it), so a working URL proves nothing. A reviewer therefore cannot detect the staleness that motivates a rebuild, nor confirm the rebuild happened, from the backend PR. The fix is frontend-repo work (both companion.ymls own that comment row) and is deliberately out of scope here; tracked as docs/tracking/devops-hardening.md §10. Until it lands, docs/preview-access-guide.md tells reviewers the honest thing: ask a developer to check the frontend repo's Actions run.
  • A rebuild is inherently a race, resolved by re-reading live state. Fast remove→re-add fires both an unlabeled and a labeled run with no ordering guarantee between workflows; correctness comes from both sides consulting the live labels rather than their own payload — the teardown stands down when the label is back, and deploy-preview.yml's companions job re-reads live labels so a stale snapshot cannot resurrect a companion the teardown already removed. That narrows the window rather than mathematically closing it, and anything else added to this lifecycle must keep the property. Closing it is available to the operator, at a price: both frontend repos put every companion dispatch for a backend PR in one never-cancelled concurrency group (companion-<backend_pr>, cancel-in-progress: false), so re-adding the label only after the teardown check has gone green provably queues the rebuild behind the destroy — no race, but a guaranteed gap with nothing serving until the rebuild finishes. Both recipes are documented in the README's action table. When the window is lost the end state is: label present, companion absent, no row in the Companion Frontend Previews comment — and nothing reconciles it. There is no loop re-asserting label ⇔ companion, so the state persists for the PR's lifetime until someone removes + re-adds the label again or pushes to the PR.
  • A push can swallow a requested rebuild. deploy-preview.yml's concurrency is preview-pr-<N> with cancel-in-progress: ${{ github.event.action != 'labeled' }} — the flag governs this run's behaviour, so a synchronize run cancels an in-progress or pending labeled run in the group and the re-add's dispatch is lost. It usually self-heals for free: the push's own companions job re-reads live labels and dispatches for every label present, which is the rebuild that was asked for. But it self-heals only if that deploy succeedscompanions needs needs.deploy.result == 'success', so a failed push-deploy means the cancelled rebuild silently never happens. Accepted as-is: excluding labeled runs from cancellation is what stops a queued companion dispatch being killed, and excluding synchronize from cancelling would give up the "latest push wins" property the preview pipeline depends on.
  • The rebuild has a failure branch, and the asymmetry is deliberate. The teardown is unconditional; the re-add is not — companions first requires this PR's backend preview to answer /health with 200 (3 attempts, 5s apart) and otherwise exits 1. So on a PR whose backend preview is broken, or was already reaped by the 48h idle cleanup (which routes through destroy-preview.yml and therefore also destroys companions while leaving the labels in place), remove → companion gone; re-add → red job, nothing rebuilt, label present with no companion and no automatic retry. Gating the teardown on backend health symmetrically was rejected: it would block cleanup exactly when the backend is broken — the case where a leaked frontend worker pointing at a dead backend is most likely and least useful — which is worse than the current behaviour. The health gate's own error message therefore states the residual state ("the companion was NOT rebuilt; if you removed the label to rebuild it, you currently have no companion"), and the README's action table and the reviewer guide both document the recovery (get the backend healthy, then remove + re-add again).
  • The add direction requires an open PR; the teardown direction deliberately does not. companions carries github.event.pull_request.state == 'open' in its if:, so adding or re-adding a label on a closed/merged PR is a silent no-op — the whole labeled run skips, a neutral check with no error and no notice. That is the intended shape (a closed PR has no backend preview to point at), and it is the mirror of the teardown having no state guard so that closing a PR still cleans up; the cost is that the one direction that silently does nothing is the one a user might expect feedback from.
  • Failures are loud up to the dispatch, and invisible after it. On this side the guard's fail-closed read and the unmapped-label arm turn genuine ambiguity into a red run — the opposite doctrine to D101's fail-open classifier, and deliberately so: skipping a test run wrongly wastes credits, but skipping a teardown wrongly leaks a live environment. The boundary is the dispatch. That step goes green the moment GitHub accepts the repository_dispatch; it cannot observe the teardown. In the frontend repo, scripts/delete-worker.sh exits 1 when the Cloudflare delete returns anything but 200/404, and the following "Update backend PR comment" step has no if: always() — so a failed teardown posts nothing; and because that run is repository_dispatch-triggered it attaches to no check on this PR. Net: label removed → green check here → companion possibly still live. The workflow mitigates this with a $GITHUB_STEP_SUMMARY block that says the step only confirms acceptance, links the frontend repo's Actions tab, and states that success shows up as the row disappearing from the Companion Frontend Previews comment. That is a pointer, not a signal — the real gap is tracked as docs/tracking/devops-hardening.md §9, whose fix belongs in the frontend repos.
  • Label-only actors can now tear a companion down. GitHub's triage role can add and remove labels without write access, so this change hands it a destroy capability it did not have. Judged acceptable: it is symmetric with the create capability triage already had (adding the label deploys a companion), the blast radius is one ephemeral frontend Worker behind Cloudflare Access, and it is trivially re-creatable by re-adding the label.
  • REPO_DISPATCH_TOKEN reaches exactly one step — in both workflows, and one of them was tightened here. In destroy-companion.yml the cross-repo credential lives in the dispatch step's own env: from the start. In deploy-preview.yml it did not: companions held GH_TOKEN: ${{ secrets.REPO_DISPATCH_TOKEN }} at job scope, so the preceding /health step — which makes an outbound curl to the preview URL and has no cross-repo business at all — also ran holding a PAT with write access to both frontend repos. This change moves it into the "Dispatch per label" step's env:. Both label reads use the default token at pull-requests: read instead. So the preview pipeline's cross-repo token surface narrows even as the pipeline gains a workflow.
  • The deploy path picks up a new GitHub API dependency, in its happy case. companions' label-based if: clause only gates the labeled event, so on every other trigger the job runs whenever the deploy succeeded — including on PRs carrying no preview-* label at all. It now performs a live-label API read on all of those runs, where previously (permissions: {}, reading the event payload) it made no API call and could not fail for an API reason. So a preview deploy that requests no companion can now end in a red companions job. Mitigated but not eliminated by the 3 retries and the pull-requests: read scope being the narrowest one that works; the honest statement is that this is a new external dependency on the deploy path's happy case, accepted because the alternative — trusting a snapshot taken ~8 minutes earlier — is the resurrection bug the read exists to prevent. (Pre-filtering the job on the payload's labels would avoid the call on unlabelled PRs, but would reintroduce the snapshot for the case that matters — a label added while the deploy was running.)
  • Five preview-lifecycle workflows now exist (deploy-preview, destroy-preview, destroy-companion, plus the shared build-image and the scheduled cleanup), and adding a third companion label means touching three surfaces:
    1. deploy-preview.yml's companions job — the label→repo map on the deploy path (the jq index(…) tests).
    2. destroy-companion.yml — the label→repo map on the unlabel path (the job if: allowlist plus the case). Adding a label to the deploy side only yields a label that deploys but never tears down.
    3. destroy-preview.ymlnot a label→repo map: it holds the unconditional frontend-repo list on the PR-close path (for repo in stables-core-web stables-core-web-backoffice), which knows nothing about labels and dispatches a companion-destroy to every frontend repo regardless. A new companion repo must be added to that list or its companion survives PR close; a new label for an already-listed repo needs nothing here. The three are cross-referenced by a MIRROR SURFACES comment on the deploy side.
  • Workflow mistakes are now caught before push, for whoever runs the target — and unavoidably in CI, on the free GitHub-hosted static job, so it costs no Blacksmith credits and adds seconds. Note the gate lints .github/workflows/**, which is classified as code by the D101 docs-only skip gate, so a workflow-only change still runs the full pipeline.
  • .github/actionlint.yaml is a maintained parity surface alongside D100's setup-php extension list: adopting a new Blacksmith runner size without declaring its label reddens CI.
  • The shellcheck deferral is a known hole in run: bodies until §8 of the devops-hardening tracker is closed. Treat the gate as schema/expression validation, not shell review.

Alternatives rejected

  • A teardown job inside deploy-preview.yml. The obvious placement, and the reason it was rejected is concrete: that workflow's concurrency group cancels in-progress runs on synchronize, so a push could cancel a queued teardown and leak an orphaned frontend worker.
  • Trusting the unlabeled event payload. Simpler, and wrong for the flow this change exists to enable: a remove→re-add rebuild would tear down the companion it had just rebuilt, intermittently, depending on run ordering.
  • Cancelling in-progress teardowns (cancel-in-progress: true), or one group for all preview-* labels. Both trade a leaked live environment for a marginally tidier run list.
  • Requiring pull_request.state == 'open'. Would skip teardown exactly when a PR is being closed, duplicating nothing useful — the close path already dispatches a destroy, and a repeat destroy is a no-op.
  • Gating the teardown on backend health, to make the two directions symmetric. It would remove the "label re-added but nothing rebuilt" end state by refusing the teardown too — at the cost of blocking cleanup precisely when the backend preview is broken or already reaped, which is when an orphaned frontend worker is most likely and least useful. A companion that cannot be torn down is worse than a companion that cannot be rebuilt, so the asymmetry stays and is documented instead (Consequences above; README action table; reviewer guide).
  • A marketplace actionlint action. Convenient, but it re-opens the supply-chain surface D101 went out of its way to keep closed, and it cannot be run locally as the same command.
  • Pinning the actionlint image by version tag (rhysd/actionlint:1.7.12), as docker-compose.yml does for its service images. Rejected because the two cases are not alike: compose pins official images for a dev-only stack, whereas this is third-party code executing in CI — the SHA-pinning rule already applied to every action in .github/workflows/*. A tag in a personal Docker Hub namespace can be repointed by its publisher, which would break both the supply-chain posture and "green locally = green in CI" without any diff to notice.
  • Landing the gate with shellcheck enabled and an ignore list. An ignore list institutionalizes the 53 findings and rots; disabling the whole pass keeps the deferral visible in one place (the Makefile comment) and tracked in one place (devops-hardening §8), with a zero baseline to defend.
  • Installing actionlint into the app image. Grows the dev image (and every CI pull) for a tool that lints YAML outside the application; a pinned throwaway container is the cheaper seam.

← Decision log index