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 thelint-workflowsMakefile target are the canonical source of truth; this ADR records the rationale. Reviewer-facing instructions for companion previews live indocs/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:
- 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.ymltears companions down on close). The label read like state but behaved like a button. - Rebuilding a companion required a backend commit. The frontend repos'
mainmoves while a backend PR is open, and theircompanion.ymldeploy path builds from that repo'smain— so a companion built on day one keeps serving day-one frontend code until something re-dispatches. Something already did:deploy-preview.ymltriggers onsynchronize, and itscompanionsjob runs whenever the deploy succeeded and the event was notlabeled, re-dispatchingcompanion-deployfor whicheverpreview-*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 furtherlabeledevent 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 ispreview-pr-<N>withcancel-in-progress: ${{ github.event.action != 'labeled' }}, i.e. asynchronizecancels 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 grouppreview-pr-<N>withcancel-in-progress: false, butdeploy-preview.ymluses the identical group withcancel-in-progress: ${{ github.event.action != 'labeled' }}, and the incoming run's flag is what governs cancellation.reopenedis indeploy-preview.yml'stypes:, 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 givingdestroy-preview.ymlits 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.
- The principle is not yet applied everywhere — one pre-existing case is left open.
- Per-label concurrency group, never cancelled:
companion-teardown-pr-<N>-<label>withcancel-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
unlabeledpayload describes the moment of removal. The remove→re-add rebuild flow fires anunlabeledrun and alabeledrun, 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_NUMBERguard 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 guarddestroy-preview.ymlapplies). - The read is REST (
gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER"), deliberately notgh pr view --json labels.gh pr viewis a GraphQL call, and GraphQL's scope requirements are less certain than REST's, whereas/pulls/{n}is unambiguously covered by the job'spull-requests: read. Guessing wrong here would not have failed in one place: the same read was added todeploy-preview.yml'scompanionsjob, 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'sgrep -qxF, a JSON array ([.labels[].name]) for the deploy side'sjq 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 theifcondition), 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.
- The numeric
- Least-privilege token scope, declared at the job and nowhere else. The teardown job declares
permissions: pull-requests: readand nothing else — enough for the REST label read, and deliberately notcontents: read, because nothing in the job checks the repository out (add it back alongside any future checkout step).destroy-companion.ymltherefore carries no workflow-levelpermissions:block at all, and a comment abovejobs: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 toGH_TOKEN— the guard step usesgithub.token(the job-scoped default), the dispatch step the cross-repoREPO_DISPATCH_TOKEN, in its own stepenv:.deploy-preview.yml'scompanionsjob now carries the samepermissions: pull-requests: readfor the mirror-image reason — it reads live labels too, and previously ran withpermissions: {}. - 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, anddestroy-preview.ymlalready 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'scasemaps them to repos, but GitHub expressions compare strings case-insensitively whilecase(and the deploy side'sjq index(…)) are case-sensitive — so a variant likePreview-Webpasses theifand 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. Sodestroy-companion.ymlwrites a$GITHUB_STEP_SUMMARYblock on all three of its green exits — the re-add stand-down, the case-variant no-op, and the accepted dispatch — anddeploy-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.ymlalready handle thecompanion-destroyevent —destroy-preview.ymlhas 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-workflowsruns actionlint in a one-shot container against a read-only whole-repo mount (-v "$(CURDIR)":/repo:ro), with--network none --security-opt no-new-privilegesand-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.envand 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 thandocker-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.ymlalready 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 existingstaticjob (immediately after checkout, beforesetup-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 threeblacksmith-{2,4,8}vcpu-ubuntu-2404-armlabels (D100) are declared asself-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: 46SC2086+ 3SC2046quoting findings, 2SC2016(both aredeploy-preview.yml's AWS--queryJMESPath literals, where the single quotes are correct and the backticks inside them are JMESPath syntax, not command substitution — shellcheck cannot know that), and anSC2221/SC2222pair flagging a shadowedcasearm inci.yml's ownis_docs_only()classifier (the*.mdarm subsumes the later.github/PULL_REQUEST_TEMPLATE.mdarm; bothreturn 0, so it is dead redundancy, not a misclassification). They sit mostly indeploy-preview.yml(36 of 53), thensync-secrets.yml,destroy-preview.ymlanddeploy-production.yml(4 each),deploy-staging.ymlandci.yml(2 each) andbuild-image.yml(1). Clearing them is its own change — and not a pure re-quoting job, since theSC2016pair 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 asdocs/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:
| Caught | Workflow 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 caught | Typos 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-backofficenow 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 frontendmainon 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 owncompanion.ymljob 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 (bothcompanion.ymls own that comment row) and is deliberately out of scope here; tracked asdocs/tracking/devops-hardening.md§10. Until it lands,docs/preview-access-guide.mdtells 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
unlabeledand alabeledrun 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, anddeploy-preview.yml'scompanionsjob 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 ispreview-pr-<N>withcancel-in-progress: ${{ github.event.action != 'labeled' }}— the flag governs this run's behaviour, so asynchronizerun cancels an in-progress or pendinglabeledrun in the group and the re-add's dispatch is lost. It usually self-heals for free: the push's owncompanionsjob 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 succeeds —companionsneedsneeds.deploy.result == 'success', so a failed push-deploy means the cancelled rebuild silently never happens. Accepted as-is: excludinglabeledruns from cancellation is what stops a queued companion dispatch being killed, and excludingsynchronizefrom 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 —
companionsfirst requires this PR's backend preview to answer/healthwith 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 throughdestroy-preview.ymland 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.
companionscarriesgithub.event.pull_request.state == 'open'in itsif:, so adding or re-adding a label on a closed/merged PR is a silent no-op — the wholelabeledrun 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.shexits 1 when the Cloudflare delete returns anything but 200/404, and the following "Update backend PR comment" step has noif: always()— so a failed teardown posts nothing; and because that run isrepository_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_SUMMARYblock 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 asdocs/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_TOKENreaches exactly one step — in both workflows, and one of them was tightened here. Indestroy-companion.ymlthe cross-repo credential lives in the dispatch step's ownenv:from the start. Indeploy-preview.ymlit did not:companionsheldGH_TOKEN: ${{ secrets.REPO_DISPATCH_TOKEN }}at job scope, so the preceding/healthstep — which makes an outboundcurlto 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'senv:. Both label reads use the default token atpull-requests: readinstead. 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-basedif:clause only gates thelabeledevent, so on every other trigger the job runs whenever the deploy succeeded — including on PRs carrying nopreview-*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 redcompanionsjob. Mitigated but not eliminated by the 3 retries and thepull-requests: readscope 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 sharedbuild-imageand the scheduled cleanup), and adding a third companion label means touching three surfaces:deploy-preview.yml'scompanionsjob — the label→repo map on the deploy path (thejq index(…)tests).destroy-companion.yml— the label→repo map on the unlabel path (the jobif:allowlist plus thecase). Adding a label to the deploy side only yields a label that deploys but never tears down.destroy-preview.yml— not 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 acompanion-destroyto 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 aMIRROR SURFACEScomment 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
staticjob, 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.yamlis a maintained parity surface alongside D100'ssetup-phpextension 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
teardownjob insidedeploy-preview.yml. The obvious placement, and the reason it was rejected is concrete: that workflow's concurrency group cancels in-progress runs onsynchronize, so a push could cancel a queued teardown and leak an orphaned frontend worker. - Trusting the
unlabeledevent 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 allpreview-*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), asdocker-compose.ymldoes 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.