Hold the production lock across production's verification and rollbacks

[Babissimo]

ClickUp: 1.1 Hold the production lock across production smoke, E2E and their rollbacks

Stacked on #590 (1.4). Review that first; this PR's own changes are the two commits above it.

Why

Only deploy-production and its deploy-failure rollback took production-deploy. GitHub takes and releases a concurrency group per job. So a following run's production deploy could start while this run's smoke tests and E2E were still testing production, and a rollback from either would then act on that deploy.

Seen on 2026-09-23: run 35841886234's "Deploy to production" began at 09:26:53, while run 35841881231's "Playwright E2E (production)" ran until 09:26:59.

What changes

1. Production's smoke and E2E rollbacks get 15 minutes.

• rollback.sh can rebuild the image it restores. The deploy-failure rollbacks allow 15 minutes for that, but these two set no command_timeout, so the ssh action's 10-minute default could cut one off part way.
• A test now covers every ssh step, in any workflow, that runs rollback.sh. Each must allow at least 15 minutes and fit in its job's cap after the capped steps ahead of it. Every deploying workflow must also still show such a step, so one written in an unrecognised form can't drop out of the check unnoticed.

2. One lock across production's whole chain.

• The move. Production's deploy, both suites and all three rollbacks move into .github/workflows/production-deploy-verify.yml. One Production job in ci.yml calls it, holding production-deploy with cancel-in-progress: false, as staging's chain already works.
• Why that holds the lock. A calling job isn't complete until every job it called has finished, so the next run's production deploy waits for this run's verification and any rollback.
• Staging isn't held. It keeps staging-deploy, so a following run's staging chain still runs while this run verifies production. Only production-to-production is serialised.
• Only the wiring changes.
  • No job inside takes a group of its own, since the caller holds it for them.
  • APP_DIR and the Playwright image arrive as inputs.
  • The main-push gate is repeated on every job that can reach production.
  • The new file declares the same read-only token as the other workflows.
• RADAR_API_KEY stays optional. The called workflow doesn't require it, which matches how the jobs behaved in ci.yml: if it's unset, the smoke suite warns that its keyed checks are unverified, and the chain still starts.
• Why a separate file. It's a separate file rather than staging-deploy-verify.yml parameterised by environment, because the two deploy scripts and smoke suites still differ in substance. The single on-box deploy script (1.6) is where they converge.
• Pointers updated. ONBOARDING, the runbook, docs/architecture.md and every comment that sent a reader to ci.yml for the production jobs now name the new file.

What the tests pin

• test_deploy_workflows.py:
  • each environment's chain is one called workflow, triggered only by that call, under one calling job holding its group;
  • nothing inside a chain takes a group;
  • no workflow reaches a droplet key outside its chain, except tower-service-contract's read-only staging probe;
  • every job that reaches production is gated to a push to main;
  • a push run's own group is unique to the run, and the two environments' groups differ.
• test_smoke_sends_radar_key.py: it reads the moved production smoke suite. It also finds each chain's caller in ci.yml and checks the caller passes that environment's own key, since a called workflow's secret is whatever its caller passes under that name.

What to expect after merge

• Job names: jobs in the run view gain a prefix, such as "Production / Deploy to production" and "Production / Production smoke tests".
• Cancelled Production jobs: during a burst of merges one is expected, as a cancelled Staging job already is. One run holds the group, one waits, and a third cancels the waiting one. The run that replaces it deploys a superset.
• Waiting: a following run's production deploy now waits for this run's smoke tests and E2E. That's about a minute at the median, and only when two merges land close together.

Assumption shared with staging

GitHub holds the calling job's group until every job in the called workflow has finished, including an always() rollback that runs after a cancel. Staging has relied on this since #381, and nothing in a PR run can exercise it.

Overlap

#625 also edits e2e/playwright.config.ts and e2e/specs/dashboard.spec.ts. This PR only changes comments in those files, and git merges the two cleanly in either order.

Verification

• YAML comparison: each moved job, compared with its original in ci.yml at Run production's E2E rollback in its own job, away from npm #590's head, differs only in the wiring listed above.
• Controls, each confirmed to fail before its fix or under a deliberate break:
  • the timeout test failed on the two production steps before commit 1;
  • raising the smoke step's cap so the rollback no longer fits in its job fails it;
  • giving push runs a shared per-ref group fails the run-group test;
  • a group on a job inside a chain fails test_no_job_in_a_chain_takes_a_lock_of_its_own.
• Workflow tests: the pins, permissions and deploy-workflow tests and their neighbours pass (331), and so does the backend suite with CI's flags. pre-commit is clean.
• Not exercised yet: the deploy chain runs only on a push to main, so its first real run is the first merge after this lands.

🤖 Generated with Claude Code

[Babissimo]

I checked both of the review's main findings, and neither holds.

"inputs is not available in a called workflow's top-level env:". It is. GitHub's context-availability table allows github, secrets, inputs and vars in workflow-level env (https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability).

actionlint encodes the same table, and I checked it against a scratch workflow_call workflow with a top-level env holding two lines:

• APP_DIR: ${{ inputs.app_dir }} passes;
• WRONG: ${{ steps.nothing.outputs.here }} fails with context "steps" is not allowed here. available contexts are "github", "inputs", "secrets", "vars".

The same actionlint passes this PR's workflows in the lint job.

The comment in staging-deploy-verify.yml is about the other direction: the caller's workflow-level env (ci.yml's APP_DIR) does not cross into a called workflow. That is why the directory arrives as an input, which production-deploy-verify.yml then restates once.

"A Playwright image failure now skips the whole production deploy". It already did. staging has needed playwright-image since before this PR, in #590's ci.yml and here alike: needs: [changes, backend-tests, backend-coverage, lint, web-build, docker-build, env-parity, tower-service-contract, playwright-image].

A failed image build therefore skips staging, and deploy-production needed staging, so production was never deployed on such a push. Listing changes and playwright-image on the production job adds no gate. They are there so that job can read their outputs for its with: inputs.

The minor notes. _runs_rollback could reuse _commands() for its comment filter. It is one startswith("#") check, though, and not worth a restack and rerun of this PR and #649. test_deploy_workflows.py has no helper that finds a workflow's caller, since its table names the callers, so _caller() duplicates nothing there.

[Babissimo]

Rebased with #590 onto main at ad12bbd, over #644 and #651. Head: 834ad0b.

What that changed here:

• Run the E2E suites in the runner's Chrome, not Playwright's container #644 removed the playwright-image job, so production-deploy-verify.yml no longer takes a playwright_image input and the Production job no longer needs that job. APP_DIR is now the only input; the description's "APP_DIR and the Playwright image arrive as inputs" is superseded. This also retires the earlier review point about the playwright-image need.
• The moved e2e-prod is Run production's E2E rollback in its own job, away from npm #590's runner-Chrome job, Read Chrome's version in the staging E2E job instead of launching it #651's note on the Chrome launch included. Compared with Run production's E2E rollback in its own job, away from npm #590's ci.yml, it differs only in comments: pointers that now name staging-deploy-verify.yml and ci.yml rather than "the staging job", and a note on the repeated main-push gate. The other moved jobs differ only in the wiring the description lists.

Verification on this head: the same tests with test_smoke_sends_radar_key.py pass (338). Each control still fails its test: a group on e2e-prod inside the chain, the E2E rollback left at the ssh action's 10-minute default, and push runs sharing a per-ref group. pre-commit is clean.

[claude]

Claude finished @jehanazad's task in 9m 16s —— View job

---

Review complete

• Read trigger comment and gather PR context
• Check the two prior review findings against the author's rebuttal / rebase
• Run a fresh code review pass on the PR diff
• Post final review

This sync is a merge from main (834ad0b → 927b98d); diffing 834ad0b..HEAD shows it pulled in ~97 unrelated backend/dashboard files but touched none of this PR's own files (workflows, deploy scripts, tests, docs). So there's nothing new in this PR's actual content since the last review pass — I re-checked the two previously-raised points plus did a fresh look at the rest.

Prior findings, re-checked

inputs in production-deploy-verify.yml's workflow-level env: (line 50). Still doesn't hold. GitHub's context availability table lists inputs as available at a reusable workflow's top-level env:, this is distinct from the caller's workflow-level env (which doesn't cross the workflow_call boundary — correctly handled here by threading app_dir through as an input). The author verified this against actionlint directly, and the repo's own lint job passes against this file. Confirmed again by reading the current file; nothing here changed in this sync.

Playwright-image gating. Moot — there's no playwright-image job anywhere in the current workflow set, as the author noted after rebasing onto #644.

One new, narrow finding from this pass

test_every_rollback_outlasts_a_rebuild's "ahead" math undercounts steps without their own timeout-minutes. backend/tests/test_deploy_workflows.py:384:

ahead = sum(int(step.get("timeout-minutes", 0)) for step in steps[:index])
assert ahead + int(timeout[:-1]) <= _job(workflow, job)["timeout-minutes"]

A step with no declared timeout-minutes contributes 0 here, but it isn't actually bounded at 0 — unbounded, it can run up to the job's full remaining cap. In production-smoke-tests (.github/workflows/production-deploy-verify.yml:428, job cap 40m), the leading actions/checkout step has no step-level timeout, so the test computes ahead = 5 + 15 = 20 and declares 20 + 15 ≤ 40 safe — the checkout step's own uncapped runtime isn't in that budget at all. If checkout stalls unusually long (slow runner pickup, infra hiccup), the rollback step can still get squeezed against the job's 40-minute wall before it finishes rebuilding — the exact "cut off part way" failure this PR's goal #1 sets out to prevent, with the new test still green.

This is narrow (checkout actions rarely take 20+ minutes) and not something introduced by the diff's logic error so much as a gap in what the test proves versus what its comment claims ("fit in its job's cap after the capped steps ahead of it" — a step with no cap isn't actually capped). Worth a look, not blocking: fixing it would mean asserting every step ahead of a rollback declares its own timeout-minutes, or budgeting a step's uncapped risk some other way.

Rest of the diff

Spot-checked the e2e comment-only changes (playwright.config.ts, dashboard.spec.ts) against origin/main — confirmed comment/wording only (ci.yml → production-deploy-verify.yml references), matching the PR's stated #625 overlap claim.

No other issues found.
· Branch