* architect: plan fix-vercel-deployment-protection-in-ci convoy (1 brief)
Flip convoy status queued -> in-progress and append the architect
output per the role-architect contract:
- Decisions (post-IA round) A/B/C/D with recommendations + rationale,
routed back to operator for gate-1 ratification (A and D especially).
- Architecture section: file plan, risk list (R1-R8), test plan,
decomposition (1 brief, justified), slice_dependencies block.
- brief-1: inject VERCEL_AUTOMATION_BYPASS_SECRET into both workflows
via query-param-on-path (Decision A), tighten max_timeout 600 -> 120
(Decision B), extend gate to skip fork PRs (Decision D), and forward
the secret as an env var to the Playwright/visual step for forward-
compat with adopt-playwright-smoke.
Boot-the-brief findings preempted in the brief:
- wait-for-vercel-preview@v1.3.2 source confirms only `targetUrl` (the
bare deployment URL) is ever logged or emitted as outputs.url; the
bypass query in `path:` is structurally invisible to logs and to
downstream consumers of ${{ steps.vercel.outputs.url }}.
- The action exposes no custom-header input -- Option B is mechanically
impossible for the wait step without forking the action.
- `path:` MUST begin with a leading `/` because the action parses it
via `new URL(path, url)`.
- PR #16 run logs confirm the 401 timeout failure mode the convoy targets.
- Playwright config doesn't exist yet (adopt-playwright-smoke owns it);
this brief only plumbs the env var.
No workflow YAML, no Playwright config, no AGENTS.md changes in this
commit -- those land in the implementer phase per the architect contract.
Awaiting human gate 1 (Decisions A + B + D ratification + brief approval)
before implementer dispatch.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(ci): plumb VERCEL_AUTOMATION_BYPASS_SECRET into preview-smoke + visual-diff (Brief 1 of fix-vercel-deployment-protection-in-ci)
Closes the CI-infra half of P0 #7's follow-up. PR #16 (squash commit
7e97254) added scoped permissions to both workflows but exposed that
Vercel Deployment Protection 401s anonymous GitHub-runner requests,
causing both Playwright smoke and Screenshot diff to time out at 10
minutes on every PR. This brief plumbs the bypass secret end-to-end
so the wait-action's healthcheck reaches 200.
Per architect Decision A (.convoys/fix-vercel-deployment-protection-in-ci.md):
- preview-smoke.yml + visual-diff.yml: wait-for-vercel-preview's
`path:` input now carries the bypass as a query parameter
`?x-vercel-protection-bypass=${{ secrets.* }}&x-vercel-set-bypass-cookie=true`.
The action only logs the bare targetUrl (verified in action.js:357,360,363)
so the secret stays out of workflow logs.
Per Decision B:
- max_timeout: 600 -> 120. PR #16 evidence shows Vercel previews are up
within seconds of job start; 120s gives ample headroom and surfaces
misconfigurations in ~2 minutes instead of ~10.
Per Decision D (NEW -- surfaced by Boot-the-brief):
- gate: job's Decide step now checks github.event.pull_request.head.repo.fork
FIRST. Forks lack repo secrets, so they would otherwise burn ~4 minutes
per PR on a misleading 401. The fork-check emits `::notice::` and short-
circuits before the existing skip-via-PR-body directive runs.
Forward-compat for adopt-playwright-smoke:
- Both workflows' Playwright/screenshot-capture step now exports
VERCEL_AUTOMATION_BYPASS_SECRET as env. The actual Playwright config
consumes it via extraHTTPHeaders in adopt-playwright-smoke's brief.
BASE_URL stays as the bare \${{ steps.vercel.outputs.url }} (no query
string) so it remains safe to echo.
Verification:
- YAML parses (js-yaml load on both files: preview-smoke jobs [gate, smoke];
visual-diff jobs [gate, visual])
- actionlint not run (binary not installed locally); recommend installing
for future PRs. Future adopt-actionlint convoy could add it to CI.
- No `set -x`, `echo`, `cat`, or `printf` of the secret or bypass URL
in any modified step
- permissions: and concurrency: blocks unchanged (PR #16 contracts preserved)
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(ci): route github.event.pull_request.body through env: to avoid shell injection
Pre-existing latent bug surfaced by PR #17's CI run. The Decide step's
inline `${{ github.event.pull_request.body }}` interpolation pastes
arbitrary PR-body text directly into a bash script. When the body
contains shell metacharacters (parens, pipes, backticks, redirections,
etc.) the resulting script either errors out at YAML-load time OR —
worse — executes attacker-controlled shell.
This bit PR #17 with a real syntax error ("unexpected token `('")
because the PR body contains parenthesized phrases like
"(was: 10-minute timeout)". Every Decide-step run in this repo has
been one badly-formatted PR body away from breaking the gate.
Fix: forward `github.event.pull_request.body` and
`github.event.pull_request.head.repo.fork` through the step's `env:`
block as `PR_BODY` and `PR_IS_FORK`, then quote them in shell
(`"$PR_BODY"`, `"$PR_IS_FORK"`). The env-var path leaves the values
as plain strings rather than syntactically embedded code, which is the
standard GitHub Actions hardening pattern (see GitHub's "Security
hardening for GitHub Actions" → "Using a third-party action").
Same change in both workflows; ~9 LOC each.
This fix is technically beyond Brief 1's scope (which targeted only
Vercel-bypass plumbing) but is added in this convoy because the bug
actively blocks Brief 1's success criterion from being validated on
PR #17. Documented in the convoy file's "Anything flagged but not
acted on" follow-up pass.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(ci): drop x-vercel-set-bypass-cookie=true from wait-action path
The wait-action's healthcheck was still 401ing despite the bypass query
being correct. Root cause: with `x-vercel-set-bypass-cookie=true`, Vercel
returns 307 + Set-Cookie (`_vercel_jwt`), but axios in Node has no cookie
jar — it follows the redirect to the bare URL without the cookie, which
then 401s.
Local verification (run by operator):
curl -sI "https://<preview>/?x-vercel-protection-bypass=<secret>" | head -1
→ HTTP/2 200 (works, no cookie needed)
curl -sI "https://<preview>/?x-vercel-protection-bypass=<secret>&x-vercel-set-bypass-cookie=true" | head -1
→ HTTP/2 307 (the redirect-without-cookie path that breaks axios)
For a one-shot healthcheck, the per-request bypass query is enough.
The cookie variant stays reserved for the future Playwright config
(adopt-playwright-smoke) where a real browser cookie jar exists.
Added an inline comment in preview-smoke.yml explaining this so the
next agent doesn't accidentally re-add the cookie param.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
547 lines
28 KiB
Markdown
547 lines
28 KiB
Markdown
---
|
|
name: fix-vercel-deployment-protection-in-ci
|
|
classification: convoy
|
|
success_metric: |
|
|
`Playwright smoke` and `Screenshot diff` workflows reach their actual
|
|
smoke / capture step on every PR (no more 401-from-Vercel-SSO 10-min
|
|
timeouts). Both workflows complete in < 5 minutes. Failures, when they
|
|
occur, are real assertion failures — not auth failures against the
|
|
preview URL.
|
|
skip:
|
|
- role-design-system-auditor # CI infra only
|
|
- role-a11y-auditor # no a11y surface
|
|
- role-ux-reviewer # no UX surface
|
|
- role-ia-architect # no IA surface
|
|
- browser-smoke # the convoy IS the smoke pipeline
|
|
status: in-progress
|
|
created: 2026-05-24
|
|
parent: ship-readiness
|
|
addresses: P0 #7 (CI infrastructure side-effect)
|
|
depends_on:
|
|
- fix-layout-default-user (shipped — PR #15, ca302a8)
|
|
- PR #16 fix(ci) scoped permissions (shipped — 7e97254)
|
|
---
|
|
|
|
# Fix Vercel Deployment Protection in CI
|
|
|
|
Plumb `VERCEL_AUTOMATION_BYPASS_SECRET` into the `Playwright smoke` and
|
|
`Screenshot diff` workflows so anonymous GitHub Actions runners can
|
|
actually GET the preview URL without hitting Vercel's SSO 401 challenge.
|
|
Without this, both workflows permanently red on every PR — just slower
|
|
red than before PR #16.
|
|
|
|
## Why now
|
|
|
|
PR #16 (`fix(ci): scoped permissions`, squash commit `7e97254`) added
|
|
minimal scoped `permissions:` blocks to `.github/workflows/preview-smoke.yml`
|
|
and `.github/workflows/visual-diff.yml`. That fixed the 5-second 403
|
|
"Resource not accessible by integration" failure both workflows hit when
|
|
trying to call the GitHub deployments API. **However**, with permissions
|
|
correct, both workflows now reach the actual deployment check and fail
|
|
with a different error: a 10-minute timeout from
|
|
`patrickedqvist/wait-for-vercel-preview@v1.3.2`'s subsequent HTTP GET
|
|
against the preview URL, which Vercel returns 401 for because Deployment
|
|
Protection is on (anonymous GitHub-runner request → Vercel SSO challenge).
|
|
Cost: ~10 minutes of runner time per workflow per PR — and zero signal,
|
|
since neither workflow ever reaches its smoke step. This blocks PR #15's
|
|
recurring follow-up convoys (visual-regression baselines, Playwright smoke
|
|
for `adopt-playwright-smoke`) from getting any CI feedback.
|
|
|
|
The bypass token already exists locally as `VERCEL_AUTOMATION_BYPASS_SECRET`
|
|
in `.env.local` (Protection Bypass for Automation, configured in the
|
|
Vercel project). Documented in `AGENTS.md` § 7 — Deployment. The work here
|
|
is plumbing it from operator-supplied repo secret → workflow env →
|
|
`wait-for-vercel-preview`'s `path:` input + the eventual Playwright
|
|
`BASE_URL` so anonymous runner requests bypass the SSO challenge.
|
|
|
|
## Operator action required (BEFORE this convoy can run)
|
|
|
|
This convoy CANNOT proceed without the operator first seeding the secret
|
|
into GitHub Actions. The implementer has nothing to wire up if the
|
|
secret isn't visible to the workflows.
|
|
|
|
1. **Seed the secret:**
|
|
```bash
|
|
gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<value from local .env.local>"
|
|
```
|
|
(The value is whatever `VERCEL_AUTOMATION_BYPASS_SECRET=…` says in
|
|
`.env.local`. Do not paste it anywhere logged. Do not echo it from a
|
|
workflow step.)
|
|
2. **Confirm visibility:**
|
|
```bash
|
|
gh secret list
|
|
```
|
|
Expect to see `VERCEL_AUTOMATION_BYPASS_SECRET` listed alongside the
|
|
existing repo secrets. Note: `gh secret list` shows names only — never
|
|
values — by design.
|
|
3. **Notify the next agent** that steps 1 + 2 are done. The convoy file's
|
|
frontmatter `status:` should flip from `queued` to `in-progress` only
|
|
after this notification.
|
|
|
|
This is the same pattern `npm run setup-db`'s `ADMIN_INITIAL_PASSWORD`
|
|
established (`drop-public-setup` Brief 1, commit `ff80753`): CI / scripts
|
|
that need a secret get an actionable fail-loud error when the secret is
|
|
missing, and the operator seeds it once per environment.
|
|
|
|
## Decisions to ratify with operator
|
|
|
|
Queued; do not pre-decide.
|
|
|
|
1. **Bypass via query param vs. request header.**
|
|
- **Option A — query param.** Append `?x-vercel-protection-bypass=...&x-vercel-set-bypass-cookie=true`
|
|
to the wait-action's `path:` input AND to the Playwright `BASE_URL`.
|
|
The first request sets a `_vercel_jwt` cookie on the runner's
|
|
ephemeral browser context; subsequent same-origin requests reuse it.
|
|
Pro: works with any HTTP client, no custom config in Playwright.
|
|
Con: the bypass token shows up in workflow run logs if any step
|
|
echoes the URL (mitigation: never `echo` or `cat` a URL containing
|
|
the token; log `${{ steps.wait.outputs.url }}` only after stripping
|
|
the query string).
|
|
- **Option B — request header (`x-vercel-protection-bypass: <secret>`).**
|
|
Cleaner — the token never appears in any URL. But requires custom
|
|
HTTP-client config in `playwright.config.js` (`extraHTTPHeaders`)
|
|
AND in `wait-for-vercel-preview` (the action's docs need confirming —
|
|
header support may not be exposed via inputs).
|
|
2. **CI assertion that bypass actually works.** Should we add a step
|
|
that explicitly asserts `200` on the preview URL during the wait-
|
|
action's healthcheck phase, before handing off to Playwright /
|
|
screenshot capture? This would surface bypass-misconfiguration as a
|
|
fast-fail step instead of letting Playwright time out 8 minutes
|
|
later on a different error. Cost: ~5 lines of YAML; benefit: clearer
|
|
failure signal for the next operator-touch event.
|
|
3. **Workflow concurrency cancellation.** The workflows already use
|
|
`concurrency:` keyed on `github.ref`. Confirm that the bypass-token
|
|
wiring doesn't inadvertently break the cancel-stale behavior (e.g.
|
|
if the `secrets.VERCEL_AUTOMATION_BYPASS_SECRET` reference is in a
|
|
`concurrency:` expression, that's a syntax error and the implementer
|
|
should pull it into a job-level `env:` instead).
|
|
|
|
## Scope
|
|
|
|
**In scope:**
|
|
|
|
- `.github/workflows/preview-smoke.yml` — wire the bypass into the
|
|
`wait-for-vercel-preview` step's `path:` input (Option A) OR add the
|
|
bypass header via the action's input shape (Option B, pending
|
|
confirmation that the action exposes header inputs).
|
|
- `.github/workflows/visual-diff.yml` — same treatment as preview-smoke
|
|
(the two workflows have similar shapes; whatever pattern works for one
|
|
should land in both).
|
|
- `playwright.config.js` (when it exists — the `adopt-playwright-smoke`
|
|
convoy ships it) — add `use: { extraHTTPHeaders: { 'x-vercel-protection-bypass': process.env.VERCEL_AUTOMATION_BYPASS_SECRET } }`
|
|
if Decision #1 picks Option B; OR build the BASE_URL with the query
|
|
param (Option A).
|
|
- Any test-setup file or helper that constructs the preview URL for
|
|
`screenshot-diff`-style workflows.
|
|
|
|
**Out of scope:**
|
|
|
|
- Writing new Playwright tests. Test authoring lives in
|
|
`adopt-playwright-smoke`. This convoy only makes the existing smoke
|
|
pipeline reachable.
|
|
- Broadening workflow `permissions:` blocks. PR #16 already landed the
|
|
minimal scope; this convoy should not need to touch them again.
|
|
- Replacing `patrickedqvist/wait-for-vercel-preview` with a different
|
|
action. The action retrieves the URL successfully (confirmed in PR #16's
|
|
run logs); the failure is the subsequent HTTP GET, which is a
|
|
configuration issue, not an action choice. A wholesale action swap is
|
|
a deeper rewrite — separate convoy if/when it's needed.
|
|
- Authoring new visual-regression baselines. The screenshot diff workflow
|
|
has nothing meaningful to compare against today; baseline authoring is
|
|
its own convoy.
|
|
- Disabling Vercel Deployment Protection on the project. Operator may
|
|
prefer to keep protected previews (cheap defense-in-depth against
|
|
preview-URL leakage); this fix lets CI work _around_ the protection
|
|
without weakening it.
|
|
|
|
## Known constraints
|
|
|
|
- **`wait-for-vercel-preview@v1.3.2` `path:` input is supported.** PR
|
|
#16's run logs confirm the action retrieves the URL successfully — the
|
|
subsequent HTTP GET is what fails. The action's `path:` input accepts a
|
|
full path including query string, so Option A (`?x-vercel-protection-bypass=...`)
|
|
is mechanically straightforward. Whether the action exposes a way to
|
|
inject custom request headers (Option B) needs to be confirmed by
|
|
reading the action's source / README before the implementer commits to
|
|
it.
|
|
- **The same secret will need to be plumbed into Playwright's `BASE_URL`
|
|
or into a request header in `playwright.config.js`** when the
|
|
`adopt-playwright-smoke` convoy ships. Coordinating shape now (this
|
|
convoy) vs. shape later (when Playwright lands) saves churn — the
|
|
implementer should pick whichever option keeps both call sites
|
|
consistent.
|
|
- **`npm run setup-db`'s `ADMIN_INITIAL_PASSWORD` is a parallel
|
|
precedent** for "CI needs a secret the operator must seed." Same
|
|
pattern applies: secret is repo-scoped, fail-loud (or fail-noisy) when
|
|
unset, never echoed to logs. See `drop-public-setup` Brief 1
|
|
(commit `ff80753`).
|
|
- **Token rotation.** The Vercel bypass token can be rotated from the
|
|
Vercel dashboard. If/when that happens, the operator must re-seed the
|
|
GitHub secret (`gh secret set ...`). No automation here — this is a
|
|
human responsibility per the same pattern as `JWT_SECRET` rotation.
|
|
|
|
## Acceptance criteria
|
|
|
|
The convoy is shippable when ALL of the following hold:
|
|
|
|
1. `Playwright smoke` workflow reaches its actual smoke step on a fresh
|
|
PR. It either passes (smoke green) OR fails on a real assertion
|
|
(Playwright reports a test failure or a runtime error from the
|
|
smoke spec). It does NOT fail with a 10-min timeout from the
|
|
`wait-for-vercel-preview` step or with a 401 from the preview URL.
|
|
2. `Screenshot diff` workflow reaches its screenshot capture step and
|
|
posts the "Visual Diff" comment to the PR (even if the diff itself
|
|
is empty / first-run / null-baseline). Same constraint: no 10-min
|
|
timeout, no 401.
|
|
3. Both workflows complete in < 5 minutes on a typical PR (the
|
|
pre-PR-16 baseline was ~30 seconds for the workflow body; adding a
|
|
bypass query string or header shouldn't materially affect runtime).
|
|
4. The bypass token does not appear in any workflow run log. Verify by
|
|
downloading the raw log of a passing run and grepping for the token's
|
|
first 8 chars.
|
|
5. Workflow YAML still passes basic actionlint review (`actionlint .github/workflows/*.yml`
|
|
exits 0). PR #16's permissions blocks remain unchanged.
|
|
6. `AGENTS.md` § 7 deployment paragraph (the "Preview protection bypass
|
|
for automation" line) still reflects reality after the change. May
|
|
need a one-sentence update if the implementer picks Option B
|
|
(`x-vercel-protection-bypass` header) vs. Option A (query string).
|
|
|
|
## Anything flagged but not acted on (in advance)
|
|
|
|
These are real findings that the architect / implementer should NOT try
|
|
to solve in this convoy. Each is queued separately if it warrants a fix.
|
|
|
|
- **The `wait-for-vercel-preview` action is no longer maintained** (last
|
|
release Mar 2024; no v2). Could be replaced with a few lines of
|
|
`gh api` + `curl`-loop in the workflow itself. Not in scope here —
|
|
this convoy needs to fix the immediate auth failure, not rewrite the
|
|
wait logic. Queue as `replace-wait-for-vercel-preview` if the action
|
|
ages out further or has a security advisory.
|
|
- **Playwright config doesn't exist yet.** `playwright.config.js`,
|
|
`tests/smoke/`, and `@playwright/test` all land in
|
|
`adopt-playwright-smoke` (P1 #10 step 2 / launch sequence step 10).
|
|
Until that convoy ships, the only `Playwright smoke` workflow body is
|
|
a no-op. This convoy can pre-wire the bypass infrastructure (env var,
|
|
workflow secrets) so `adopt-playwright-smoke` only needs to add the
|
|
test files and the Playwright config — but it can't ship a real
|
|
smoke-pass without that follow-up.
|
|
- **`Screenshot diff` baseline authoring.** Even after this convoy lands,
|
|
the visual-diff workflow has nothing to compare against on its first
|
|
run. That's expected and orthogonal — baseline authoring is a separate
|
|
scope.
|
|
- **Operator-rotation hygiene for `VERCEL_AUTOMATION_BYPASS_SECRET`.**
|
|
Vercel's bypass tokens don't auto-expire. If the team wants a periodic
|
|
rotation policy, that's an ops-runbook concern outside this convoy.
|
|
- **`AGENTS.md` § 7 wording.** The current "Smoke/visual-diff workflows
|
|
pass this header (`x-vercel-protection-bypass`)" line in § 7 is
|
|
aspirational — it describes intent, not what was actually wired. After
|
|
this convoy ships, that line becomes accurate. The doc-writer pass at
|
|
convoy close should reword to past-tense reality. **Also: § 7 says
|
|
"header"; the architect recommendation in Decision A below is the query
|
|
param (the wait-action has no input for custom headers). The doc-writer
|
|
pass MUST correct the noun.**
|
|
|
|
## Decisions (post-IA round)
|
|
|
|
Each decision below routes back to the operator for ratification at
|
|
human gate 1 (per the architect contract). Recommendations are based on
|
|
fresh-checkout evidence the architect gathered before drafting the brief.
|
|
|
|
### A — 2026-05-24: Use query-param-on-`path:` for the wait-action; reserve `extraHTTPHeaders` for the Playwright config that lands in `adopt-playwright-smoke`
|
|
|
|
> Resolves convoy file § "Decisions to ratify with operator" #1 (query
|
|
> param vs. header).
|
|
|
|
**Context.** The convoy file framed this as a clean either/or between
|
|
Option A (query param on `path:`) and Option B (request header via the
|
|
action's input shape). Boot-the-brief revealed the choice is forced for
|
|
the wait step but free for Playwright:
|
|
|
|
- `patrickedqvist/wait-for-vercel-preview@v1.3.2`'s `action.yml`
|
|
exposes inputs `token`, `max_timeout`, `environment`,
|
|
`allow_inactive`, `check_interval`, `vercel_password`, and `path` —
|
|
and **nothing else**. There is no input for custom request headers.
|
|
Option B is mechanically impossible for the wait step without
|
|
forking the action.
|
|
- `action.js:42` consumes `path` via `new URL(path, url)`. Anything
|
|
parseable as a URL path is fine; query strings work verbatim. So
|
|
`path: '/?x-vercel-protection-bypass=…&x-vercel-set-bypass-cookie=true'`
|
|
becomes the URL `https://<deployment>/?x-vercel-protection-bypass=…`
|
|
that axios then GETs.
|
|
- Crucially, the action only echoes `targetUrl` (the bare deployment
|
|
URL — `status.target_url`) in its logs (`action.js:357`, `:363`) and
|
|
sets it as `outputs.url` at `:360`. The `path:` query string is
|
|
**never appended to anything that is logged or set as an output.**
|
|
So passing the secret via `path:` does NOT leak it to workflow logs
|
|
or to downstream steps that consume `${{ steps.vercel.outputs.url }}`.
|
|
- Vercel's docs explicitly support both shapes; the "header is
|
|
recommended" guidance is about URL-in-log leak risk in callers, not
|
|
Vercel's acceptance. For the wait-action the leak risk is structurally
|
|
absent (see above).
|
|
- The future `playwright.config.js` (owned by `adopt-playwright-smoke`)
|
|
CAN and SHOULD use `extraHTTPHeaders` per Vercel's own snippet — the
|
|
config controls its own request shape and the header is cleaner.
|
|
|
|
**Recommendation (needs operator ratification).** Option A for the
|
|
wait-action. Pass the secret to Playwright through `env:` (this convoy
|
|
plumbs the env var; the actual Playwright config is `adopt-playwright-smoke`'s
|
|
job).
|
|
|
|
**If operator prefers Option B uniformly** (i.e. headers everywhere),
|
|
the cost is forking `wait-for-vercel-preview` or replacing it with a
|
|
hand-rolled `gh api` + `curl` poll. That's a larger rewrite and was
|
|
flagged as out-of-scope in the convoy file (§ "Anything flagged but not
|
|
acted on" → `replace-wait-for-vercel-preview`). Recommend keeping it
|
|
out of scope for now.
|
|
|
|
**Routing.** Operator ratifies at gate 1. Default to A unless rejected.
|
|
|
|
### B — 2026-05-24: No extra healthcheck assertion step; tighten `max_timeout` from 600 → 120 instead
|
|
|
|
> Resolves convoy file § "Decisions to ratify with operator" #2 (CI
|
|
> assertion that bypass actually works).
|
|
|
|
**Context.** The convoy file asked whether to add an explicit step that
|
|
asserts `200` on the preview URL before handing off to Playwright /
|
|
screenshot capture.
|
|
|
|
- The wait-action's healthcheck loop (`action.js:25-66`) already does
|
|
exactly this: `axios.get` against `new URL(path, url)`, retry on
|
|
non-2xx, exit on first 2xx, fail the step on timeout. If the bypass
|
|
is misconfigured, the action will time out at `max_timeout` and
|
|
call `core.setFailed('Timeout reached: Unable to connect to <url>')`.
|
|
An extra `curl` step would duplicate this signal.
|
|
- The real ergonomics problem is `max_timeout: 600` (10 minutes). At
|
|
2-second polling intervals (the action's default — confirmed in PR
|
|
#16's run logs: "Attempt N of 300"), a misconfigured bypass burns
|
|
10 minutes of runner time before failing. Vercel preview builds
|
|
typically complete in 30-90s; the deployment is normally already up
|
|
by the time GitHub triggers the workflow.
|
|
|
|
**Recommendation (architect-self-ratifiable; flagging for awareness).**
|
|
No additional assertion step. Lower `max_timeout` from `600` to `120`
|
|
in both workflows. This makes a misconfigured bypass fail in ~2 minutes
|
|
instead of ~10, well inside the convoy's "< 5 minutes" success metric,
|
|
and gives the deployment plenty of headroom for slow builds.
|
|
|
|
**Routing.** Architect ratifies. Operator may override at gate 1 if
|
|
preview builds in this project are known to exceed 120s — observed
|
|
behavior in PR #16's logs (deployment URL retrieved within 1 second of
|
|
job start) suggests the deployment is up well before the wait step
|
|
starts, so 120s is comfortable.
|
|
|
|
### C — 2026-05-24: Confirmed — `concurrency:` block contains no secret reference and stays unchanged
|
|
|
|
> Resolves convoy file § "Decisions to ratify with operator" #3
|
|
> (workflow concurrency cancellation).
|
|
|
|
**Context.** The convoy file flagged the risk that a secret-reference
|
|
inside a `concurrency:` group expression would be a YAML syntax error.
|
|
|
|
- Current `concurrency:` groups: `preview-smoke-${{ github.event.pull_request.number }}`
|
|
and `visual-diff-${{ github.event.pull_request.number }}`. No secret
|
|
reference today.
|
|
- The implementer's plumb-the-secret work lands in: (a) the wait-action
|
|
step's `with: path: ...` input, and (b) the Playwright smoke step's
|
|
`env: VERCEL_AUTOMATION_BYPASS_SECRET: ...` for forward-compat with
|
|
`adopt-playwright-smoke`. Neither location intersects `concurrency:`.
|
|
- Brief acceptance criterion #3 explicitly forbids placing the secret in
|
|
the `concurrency:` group expression.
|
|
|
|
**Recommendation (architect-self-ratifiable).** No change to the
|
|
`concurrency:` blocks; the cancel-stale behavior is preserved as-is.
|
|
|
|
**Routing.** Architect ratifies. No operator action needed.
|
|
|
|
### D — 2026-05-24: Skip Playwright smoke + Screenshot diff on fork PRs (extend `gate:` job) — NEW decision surfaced by Boot-the-brief
|
|
|
|
> Not in the original convoy file's "Decisions to ratify" list. Surfaced
|
|
> by the architect's Boot-the-brief check ("Empty / unset secret" case).
|
|
|
|
**Context.** GitHub Actions silently omits repo secrets on
|
|
`pull_request`-event runs that originate from a fork. The wait-action
|
|
would receive `${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` as an
|
|
empty string, build the URL `https://<deployment>/?x-vercel-protection-bypass=&x-vercel-set-bypass-cookie=true`,
|
|
get 401 (empty bypass value is not a valid bypass), and time out at
|
|
`max_timeout`. After Decision B (120s timeout), that's still ~2 minutes
|
|
of wasted runner time per fork PR per workflow — net 4 minutes per fork
|
|
PR. The failure signal is "the convoy's fix didn't work" instead of "the
|
|
PR is from a fork and can't access secrets" — a misleading red.
|
|
|
|
tcg-vault is single-maintainer with occasional collaborators (all with
|
|
write access, so their PRs aren't from forks today). Fork PRs are rare.
|
|
But the cost of a one-line gate-job extension is zero, and the value
|
|
is "fork PRs get a clear skip message instead of a 2-minute wait + red."
|
|
|
|
**Recommendation (needs operator ratification).** Extend the existing
|
|
`gate:` step in both workflows to check `github.event.pull_request.head.repo.fork`
|
|
first, BEFORE the existing `pipeline:.*skip.*\bsmoke\b` / `\bvisual\b`
|
|
body-directive check. When `fork == true`, emit a `::notice::`
|
|
explaining why, and `should_run=false`. The actual `smoke` /
|
|
`visual` job stays guarded by `if: needs.gate.outputs.should_run == 'true'`
|
|
unchanged — it just doesn't fire for forks.
|
|
|
|
Verbatim shape baked into the brief:
|
|
|
|
```bash
|
|
if [[ "${{ github.event.pull_request.head.repo.fork }}" == "true" ]]; then
|
|
echo "should_run=false" >> $GITHUB_OUTPUT
|
|
echo "::notice::Smoke skipped on fork PR (bypass secret unavailable to forks)"
|
|
elif echo "${{ github.event.pull_request.body }}" | grep -qE 'pipeline:.*skip.*\bsmoke\b'; then
|
|
...
|
|
```
|
|
|
|
**Alternative (rejected):** add the fork check as an `if:` on the
|
|
`smoke` and `visual` jobs directly. Same effect, but loses the
|
|
`::notice::` annotation that surfaces in the GitHub Actions UI summary —
|
|
silent skip is worse UX than annotated skip.
|
|
|
|
**Side effect.** Until `adopt-playwright-smoke` ships, this convoy's
|
|
fork-PR skip applies to a workflow that already does nothing useful
|
|
(no Playwright config, no `@playwright/test`). The skip is forward-
|
|
looking — once the smoke pipeline becomes real, fork PRs gracefully opt
|
|
out instead of failing.
|
|
|
|
**Routing.** Operator ratifies at gate 1. Default to "yes, skip on
|
|
forks" unless rejected. If rejected, the brief drops the fork check and
|
|
the recommendation in `AGENTS.md` § 7 (the doc-writer pass) should
|
|
document the fork-PR failure mode.
|
|
|
|
## Architecture
|
|
|
|
### File plan
|
|
|
|
| File | Action | Purpose |
|
|
| --- | --- | --- |
|
|
| `.github/workflows/preview-smoke.yml` | modified | Inject bypass-secret query into `path:` of the wait-for-vercel-preview step (Decision A); lower `max_timeout` 600 → 120 (Decision B); extend `gate:` step to skip fork PRs (Decision D); add `VERCEL_AUTOMATION_BYPASS_SECRET` to the Playwright smoke step's `env:` for forward-compat with `adopt-playwright-smoke` |
|
|
| `.github/workflows/visual-diff.yml` | modified | Same shape as preview-smoke: bypass query on `path:`, `max_timeout` 600 → 120, fork-PR gate, `VERCEL_AUTOMATION_BYPASS_SECRET` in the screenshot-capture step's `env:` |
|
|
|
|
### API surface
|
|
|
|
N/A — this convoy modifies CI workflow YAML only. No HTTP routes are
|
|
added, modified, or removed.
|
|
|
|
### Schema diff
|
|
|
|
N/A — no database changes.
|
|
|
|
### Test plan
|
|
|
|
- **No new unit tests.** The change is workflow YAML; vitest does not
|
|
exercise GitHub Actions. The existing 16-test auth-surface suite stays
|
|
green and is unaffected.
|
|
- **Manual validation in the brief's "Manual verification" section:**
|
|
- Acceptance criterion #1 (wait-action exits successfully on the
|
|
convoy's own PR): observe by reading the workflow's run log after
|
|
pushing the convoy branch. Expect `Received success status code`
|
|
within the first few attempts and total wait-step duration < 90s.
|
|
- Acceptance criterion #4 (no bypass secret in workflow logs):
|
|
`gh run download <run-id> -n logs && rg "<first-8-chars-of-secret>"
|
|
logs/` (locally only — never paste the chars into a script or
|
|
commit). Expect zero matches.
|
|
- Acceptance criterion #5 (actionlint validation): document the
|
|
one-line `brew install actionlint` install OR a hermetic Docker
|
|
one-liner; recommended-not-required (no actionlint binary in CI
|
|
today, and gating on it would expand scope). The brief includes the
|
|
exact command.
|
|
- **Smoke / visual jobs themselves still fail** after the brief lands,
|
|
because `@playwright/test` is not installed and `playwright.config.js`
|
|
does not exist — the failure mode shifts from "401 timeout in the
|
|
wait step" (this convoy's target) to "playwright not installed" (the
|
|
`adopt-playwright-smoke` convoy's target). That is the **correct,
|
|
expected end state of this convoy.** Brief acceptance criterion #1
|
|
explicitly accepts a real downstream failure as success, as long as
|
|
the wait-action reaches `Received success status code` first.
|
|
|
|
### Risk list
|
|
|
|
- **R1 — Secret leaks via workflow log.** Even though the wait-action
|
|
itself doesn't echo `path:` (verified — `action.js:357,360,363` only
|
|
emit `targetUrl`, which does NOT include the query string the action
|
|
appended internally), any added `echo "$BASE_URL"` or `run: |` step
|
|
with `set -x` in the same job could expose the secret. Brief calls
|
|
this out and prohibits echoing constructed URLs. Mitigation: keep the
|
|
bypass *only* in `path:` and `env:` — never built into a shell
|
|
variable that a step might print.
|
|
- **R2 — `outputs.url` is the bare deployment URL (already verified) —
|
|
Playwright will need its own injection.** Confirmed via `action.js:360`:
|
|
`core.setOutput('url', targetUrl)` where `targetUrl = status.target_url`.
|
|
The `path:` query is NOT appended. So the BASE_URL Playwright receives
|
|
via `${{ steps.vercel.outputs.url }}` is clean — Playwright must inject
|
|
the bypass itself (via `extraHTTPHeaders` per Vercel's docs). This
|
|
convoy plumbs `VERCEL_AUTOMATION_BYPASS_SECRET` as an env var on the
|
|
step so `adopt-playwright-smoke` can read it from `process.env`.
|
|
- **R3 — `path:` parsing requires leading `/`.** `action.js:42`:
|
|
`new URL(path, url)`. If the implementer writes
|
|
`path: '?x-vercel-protection-bypass=...'` (no leading `/`), the URL
|
|
resolver will combine relative-to-current-document which can drop the
|
|
origin. Brief acceptance criterion explicitly mandates `path: '/?...'`.
|
|
- **R4 — `max_timeout: 120` may be too aggressive for very slow Vercel
|
|
builds.** PR #16's run log evidence (deployment URL retrieved within
|
|
1 second of job start) suggests the deployment is already up by the
|
|
time the workflow triggers. 120s gives ~60 polls at the default 2s
|
|
interval. If a cold-start build legitimately takes > 120s, the
|
|
workflow will time out. Mitigation: operator may override at gate 1
|
|
if recent Vercel build times have been long. Easy revert.
|
|
- **R5 — Fork-PR gate misclassification.** GitHub's
|
|
`github.event.pull_request.head.repo.fork` is a boolean but is
|
|
rendered as the string `"true"` / `"false"` in expression context.
|
|
The brief's shell check uses `[[ ... == "true" ]]`, which is the
|
|
safe comparison.
|
|
- **R6 — Token rotation invalidates CI silently.** If the operator
|
|
rotates the bypass token in the Vercel dashboard but forgets to
|
|
re-seed the GitHub secret, the workflow will start failing with the
|
|
same 401 + timeout it does today. This is documented in convoy file
|
|
§ Known constraints; not preventable from workflow YAML. The doc-
|
|
writer pass should add a one-line note to `AGENTS.md` § 7 listing
|
|
the secret-rotation runbook.
|
|
- **R7 — Concurrency-group cancellation interacts with the bypass URL?
|
|
Confirmed: no.** `concurrency:` uses only `github.event.pull_request.number`;
|
|
no secret reference. Decision C covers this.
|
|
- **R8 — actionlint not in CI.** No workflow validator runs on PRs
|
|
today. The brief recommends a local `actionlint` install for the
|
|
implementer; CI integration is its own scope (queueable as
|
|
`adopt-actionlint`).
|
|
|
|
### Decomposition
|
|
|
|
| Brief # | Title | Files | Depends on | Estimated PR size |
|
|
| --- | --- | --- | --- | --- |
|
|
| 1 | Inject `VERCEL_AUTOMATION_BYPASS_SECRET` into preview-smoke + visual-diff workflows | `.github/workflows/preview-smoke.yml`, `.github/workflows/visual-diff.yml` | none | ~50 LOC YAML diff total |
|
|
|
|
**Why 1 brief and not 2** (one per workflow):
|
|
|
|
- Both files take the **identical shape change** (same wait-action step,
|
|
same `max_timeout` reduction, same gate-job extension, same
|
|
forward-compat env var). The diffs are parallel and best reviewed
|
|
together — PR #16 set the precedent of touching both workflow files
|
|
in a single PR for this exact reason.
|
|
- Splitting into 2 briefs would force two PRs into the same review
|
|
surface, two implementer runs, two convoy-cycle bookings, with zero
|
|
reviewer benefit: the files are independently reverte-able at the
|
|
file level inside a single PR.
|
|
- Total brief LOC is well under the 400-LOC architect ceiling.
|
|
- No cross-brief commitments are needed.
|
|
|
|
If the implementer surfaces a reason the two files must diverge mid-
|
|
flight (e.g. visual-diff needs a different `path:` because it captures
|
|
a deeper page), that's a Decision-letter scope expansion documented in
|
|
this file, not a re-decomposition.
|
|
|
|
### Slice dependencies (multitask-ready)
|
|
|
|
```yaml
|
|
slice_dependencies:
|
|
- brief: 1
|
|
depends_on: []
|
|
files:
|
|
- .github/workflows/preview-smoke.yml
|
|
- .github/workflows/visual-diff.yml
|
|
```
|
|
|
|
Single brief — no parallelization opportunity. Conductor dispatches
|
|
serially.
|
|
|
|
Architecture complete. 1 brief created. Estimated PRs: 1. Awaiting
|
|
human gate 1 (Decisions A + B + D ratification + brief approval) before
|
|
the implementer runs.
|