rstillwell/deckhearth
1
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions

fix(ci): plumb VERCEL_AUTOMATION_BYPASS_SECRET into preview-smoke + visual-diff #17

Merged
varutasu merged 4 commits from convoy/fix-vercel-deployment-protection-in-ci into main 2026-05-24 17:26:22 -04:00
Conversation 2 Commits 4 Files changed 4 +708 -6
4 changed files with 708 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

312
.convoys/fix-vercel-deployment-protection-in-ci.md
View file

@ -13,7 +13,7 @@ skip:
- role-ux-reviewer # no UX surface
- role-ia-architect # no IA surface
- browser-smoke # the convoy IS the smoke pipeline
status: queued
status: in-progress
created: 2026-05-24
parent: ship-readiness
addresses: P0 #7 (CI infrastructure side-effect)
@ -236,4 +236,312 @@ to solve in this convoy. Each is queued separately if it warrants a fix.
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.
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.

362
.convoys/fix-vercel-deployment-protection-in-ci/brief-1-inject-bypass-secret-into-ci-workflows.md Normal file
View file

@ -0,0 +1,362 @@
---
convoy: fix-vercel-deployment-protection-in-ci
brief_number: 1
depends_on: []
files:
- .github/workflows/preview-smoke.yml
- .github/workflows/visual-diff.yml
---
# Brief 1: Inject `VERCEL_AUTOMATION_BYPASS_SECRET` into the preview-smoke + visual-diff workflows so the wait-for-vercel-preview healthcheck passes against protected Vercel previews
## Goal (1 sentence)
Plumb `${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` into `wait-for-vercel-preview`'s `path:` input as a query parameter (Decision A) in both workflows, tighten `max_timeout` from 600 → 120 (Decision B), extend the `gate:` job to skip fork PRs (Decision D), and forward the same secret as an env var to the Playwright smoke / screenshot-capture step so `adopt-playwright-smoke` finds the surface pre-wired — without ever leaking the secret to a workflow log or to `${{ steps.vercel.outputs.url }}`.
## Files in scope (do not edit anything else)
- `.github/workflows/preview-smoke.yml` — modified.
- `.github/workflows/visual-diff.yml` — modified.
**Files explicitly out of scope** (do not touch even if it seems related):
- `playwright.config.js` — does not exist yet; `adopt-playwright-smoke` owns it.
- `tests/smoke/app.smoke.spec.ts` — already exists as a stub but stays in `tests/smoke/`; `adopt-playwright-smoke` owns it.
- `tests/visual/` — does not exist yet; `adopt-playwright-smoke` owns it.
- `AGENTS.md` § 7 — the wording correction (header → query param; queued → wired) is the doc-writer pass at convoy close, NOT this brief.
- `package.json` — no new deps. `@playwright/test` is not in scope here.
- Any other workflow in `.github/workflows/` (e.g. `ci.yml`) — out of scope.
## Conventions to follow
- **Decision A (`.convoys/fix-vercel-deployment-protection-in-ci.md` § Decisions post-IA round).** The wait-action receives the bypass via query param on `path:`. The header form is reserved for the future `playwright.config.js`.
- **Decision B (same file).** `max_timeout: 600` → `max_timeout: 120` in both workflows. Operator may override at gate 1 if Vercel builds have been slow recently.
- **Decision C (same file).** Do NOT place `${{ secrets.* }}` inside any `concurrency:` group expression. GitHub Actions YAML parser rejects secret refs in `concurrency:` and the workflow fails to load. The `concurrency:` blocks stay unchanged.
- **Decision D (same file).** Extend the existing `gate:` step's `Decide` shell script to check `github.event.pull_request.head.repo.fork` FIRST. When the PR comes from a fork, emit `::notice::` and set `should_run=false`. Same shape in both workflows.
- **No-go zones (`.cursor/rules/no-go-zones.mdc`).** None of the files in scope are in the no-go list. Do not edit anything outside `.github/workflows/preview-smoke.yml` and `.github/workflows/visual-diff.yml` in this brief.
- **Secret-handling discipline:**
- NEVER `echo`, `cat`, or `printf` a URL or env var that contains `${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` or `$VERCEL_AUTOMATION_BYPASS_SECRET`.
- NEVER use `set -x` in any `run:` step in either workflow (it would echo every command including ones that interpolate secrets).
- NEVER assign the constructed URL (the one with the bypass query) to a shell variable that is later printed.
- The wait-action's own logs are safe (see Boot-the-brief finding #2 below). The risk is in YOUR additions, not in the action.
- GitHub Actions auto-masks values that match registered secrets in workflow logs. That is a backstop, not a primary defense. Do not rely on it to redact full URLs.
- **Style match.** PR #16 (`fix(ci): scoped permissions for preview-smoke + visual-diff workflows`, squash commit `7e97254`) is the precedent for touching both workflow files in the same PR. Follow its diff shape: same change applied to both files, with workflow-specific differences (smoke vs visual-diff naming, the `pull-requests: write` permission only on visual-diff) preserved as-is.
## Acceptance criteria
### `.github/workflows/preview-smoke.yml`
- [ ] **Wait-action step (currently lines 60-65) becomes:**
```yaml
- name: Wait for Vercel Preview deployment
id: vercel
uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
with:
token: ${{ secrets.GITHUB_TOKEN }}
max_timeout: 120
path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true
```
Three changes relative to the current step:
1. `max_timeout: 600` → `max_timeout: 120` (Decision B).
2. New `path:` input — **MUST begin with a leading `/`**. `action.js:42` parses this via `new URL(path, url)`; a missing leading `/` makes the URL resolver produce a path relative to the wrong base. Verbatim `/` then `?` then the two query params.
3. The `x-vercel-set-bypass-cookie=true` segment is REQUIRED, not optional. It causes Vercel to set a `_vercel_jwt` cookie on the response so any follow-up same-origin requests (e.g. Playwright's `page.goto` redirects) reuse the bypass without needing the query string again.
Do NOT:
- Move the `path:` line above `max_timeout:` (no semantic difference, but match the verbatim order so the diff stays minimal).
- Use single quotes around the `path:` value. YAML treats the unquoted form as a plain string; quoting introduces escape-handling questions. Leave it unquoted.
- Add an `env:` block to this step. The wait-action does not read `process.env.VERCEL_AUTOMATION_BYPASS_SECRET`; it consumes the input only.
- [ ] **`gate:` job's `Decide` step (currently lines 41-49) becomes:**
```yaml
- name: Decide
id: check
run: |
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
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Smoke skipped via pipeline directive"
else
echo "should_run=true" >> $GITHUB_OUTPUT
fi
```
Three changes relative to current:
1. New `if [[ "${{ ... fork }}" == "true" ]]` branch FIRST. It must come before the body-directive check so fork PRs short-circuit out without parsing the PR body.
2. The shell comparison is `== "true"` (a string compare against the literal string `"true"`). `github.event.pull_request.head.repo.fork` is rendered as the string `"true"` or `"false"` in expression context — NOT as a bare boolean (Risk R5 in the convoy's architecture risk list).
3. The notice mentions "fork PR" explicitly; this surfaces in the GitHub Actions UI summary so a reader scanning a PR can immediately see why smoke didn't run.
Do NOT:
- Replace the body-directive check (`pipeline:.*skip.*\bsmoke\b`). That gate is still useful for non-fork PRs that legitimately want to skip smoke (e.g. doc-only PRs).
- Move the `gate:` step's `if:` higher — `if: needs.gate.outputs.should_run == 'true'` on the `smoke:` job is the correct gate; it stays.
- [ ] **Playwright smoke step (currently lines 77-80) becomes:**
```yaml
- name: Run smoke tests
run: npx playwright test --project=smoke
env:
BASE_URL: ${{ steps.vercel.outputs.url }}
VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
```
One change relative to current:
1. New `VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}` line in the step's `env:` block. This is forward-compat plumbing — the `playwright.config.js` that `adopt-playwright-smoke` ships will read `process.env.VERCEL_AUTOMATION_BYPASS_SECRET` and inject the bypass via `extraHTTPHeaders` per Vercel's snippet.
2. **Do NOT modify `BASE_URL`.** It stays `${{ steps.vercel.outputs.url }}`. The wait-action's `outputs.url` is the bare deployment URL — the bypass query string is NOT appended (confirmed by reading `action.js:360`). Playwright will inject the bypass via headers; the URL must stay clean so the headers actually apply on every request (Playwright re-applies `extraHTTPHeaders` per request, including redirects).
3. Until `adopt-playwright-smoke` ships, this step will FAIL because `@playwright/test` is not installed. That's the documented end state of THIS brief (see acceptance criterion #1 at the bottom of this section); do not try to fix it here.
- [ ] **All other lines in `.github/workflows/preview-smoke.yml` stay byte-for-byte identical to the current file** — including:
- The `name:` line.
- The full `on:` block (PR types, target branch).
- The full `concurrency:` block (Decision C — no secret reference).
- The full `permissions:` block (PR #16 already landed the minimal scope; do not touch).
- The `gate:` job's `name:`, `runs-on:`, `outputs:`, the existing `actions/checkout@v4` step in the `smoke:` job, `actions/setup-node@v4`, `npm ci`, `npx playwright install --with-deps chromium`, and the `Upload Playwright report on failure` step.
- The leading multi-line comment block at the top of the file (lines 1-11). Update text is the doc-writer's job, not the implementer's.
### `.github/workflows/visual-diff.yml`
- [ ] **Wait-action step (currently lines 56-61) becomes:**
```yaml
- name: Wait for Vercel Preview deployment
id: vercel
uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
with:
token: ${{ secrets.GITHUB_TOKEN }}
max_timeout: 120
path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true
```
Identical to the preview-smoke version (same wait-action, same input shape, same secret). The shared shape is intentional — Decision A applies to both workflows.
- [ ] **`gate:` job's `Decide` step (currently lines 38-45) becomes:**
```yaml
- id: check
run: |
if [[ "${{ github.event.pull_request.head.repo.fork }}" == "true" ]]; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Visual diff skipped on fork PR (bypass secret unavailable to forks)"
elif echo "${{ github.event.pull_request.body }}" | grep -qE 'pipeline:.*skip.*\bvisual\b'; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Visual diff skipped via pipeline directive"
else
echo "should_run=true" >> $GITHUB_OUTPUT
fi
```
Same shape as preview-smoke's gate, with two text-only differences:
1. The body-directive regex is `\bvisual\b` (was `\bsmoke\b` in preview-smoke). Matches the existing convention in the current file.
2. The two `::notice::` strings say "Visual diff" instead of "Smoke" — matches the workflow's name.
Note: the current `visual-diff.yml` `Decide` step is missing the `name:` field (the current file is `- id: check` directly). Preserve that style — do not add a `name:` here just because preview-smoke has one. The diff stays minimal.
- [ ] **Screenshot-capture step (currently lines 71-75) becomes:**
```yaml
- name: Capture screenshots (PR)
run: npx playwright test --project=visual --update-snapshots=none
env:
BASE_URL: ${{ steps.vercel.outputs.url }}
VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
continue-on-error: true
```
Same change as preview-smoke's smoke step — add `VERCEL_AUTOMATION_BYPASS_SECRET` to `env:`. Do NOT remove `continue-on-error: true` (the visual-diff workflow's design is to upload diffs even when tests fail; that behavior stays).
- [ ] **All other lines stay byte-for-byte identical** — including:
- `name:`, `on:` (paths-only trigger), `concurrency:`, `permissions:` (note: visual-diff has `pull-requests: write` because of the comment-on-PR step; do not change this).
- The leading multi-line comment block at the top of the file (lines 1-5).
- The `actions/upload-artifact@v4` and `actions/github-script@v7` (comment-on-PR) steps.
### Cross-file checks (apply to both YAML files)
- [ ] **No `${{ secrets.* }}` reference appears inside the `concurrency:` block** in either file. Grep before committing: `rg 'secrets\.' .github/workflows/preview-smoke.yml .github/workflows/visual-diff.yml` should return exactly two matches per file (one in the wait-action's `path:`, one in the Playwright/visual step's `env:`). Three or more matches per file means a stray secret reference snuck somewhere; investigate.
- [ ] **No `echo`, `printf`, `cat`, or `set -x` references the bypass-bearing URL or env var.** Grep: `rg 'echo|printf|cat|set -x' .github/workflows/preview-smoke.yml .github/workflows/visual-diff.yml` — every match should be an existing line untouched by this brief (the `gate:` job's `echo "should_run=..." >> $GITHUB_OUTPUT` lines are fine; they don't echo URLs or secrets).
- [ ] **Diff hygiene.** `git diff main..HEAD -- .github/workflows/` should show only the changes specified above. No whitespace-only changes elsewhere. No unrelated edits. Total diff is expected to be ~50 LOC across both files (~20 LOC per file changed, plus a few context lines).
### Acceptance criterion #1 — end-state behavior
After this brief lands on the convoy branch and a Vercel preview deployment is published for the PR:
- [ ] **The wait-action's healthcheck exits successfully.** In the `Playwright smoke` job's run log, expect a single line near the end of the `Wait for Vercel Preview deployment` step:
```
Received success status code
```
This means the bypass query made it through to Vercel and the protected preview returned 2xx.
- [ ] **The wait-action step takes < 90 seconds wall time** (typical: < 10s if the deployment is already up, which is the usual case based on PR #16's logs).
- [ ] **The subsequent `Run smoke tests` step (preview-smoke) or `Capture screenshots (PR)` step (visual-diff) is REACHED**, even though it will fail because `@playwright/test` is not installed. The expected failure mode is roughly:
```
npm ERR! could not determine executable to run
npm error code ENOENT
npm error path .../node_modules/@playwright/test
```
OR a `npx`-driven download that succeeds but then fails on the missing config. Either is acceptable for this brief — the success metric is "wait-action passed and Playwright step was reached," not "Playwright passed."
- [ ] **Total job runtime is < 5 minutes.** The convoy file's success metric. Now-correctly-passing wait step (~10s) + reached-but-failing Playwright step (~30-90s) is well inside 5 minutes.
- [ ] **The bypass secret value does NOT appear in any line of the run log.** Verify after the run:
```bash
gh run download <run-id> -n logs
# Then locally — NEVER commit this script — check that the secret's
# first 8 chars do not appear in the downloaded logs. GitHub Actions
# also auto-masks; this is belt-and-suspenders.
rg "$(head -c 8 <<< "$VERCEL_AUTOMATION_BYPASS_SECRET")" logs/ || echo "OK — bypass not in logs"
```
Expected: `OK — bypass not in logs`.
## Manual verification (in addition to the workflow run on push)
Run these in order. Paste relevant output (with secrets redacted) into the PR description.
- [ ] **Local YAML lint.** Install actionlint (one-time):
```bash
brew install actionlint
```
Then validate:
```bash
actionlint .github/workflows/preview-smoke.yml .github/workflows/visual-diff.yml
```
Expected: zero output, exit code 0. If actionlint flags anything,
read the message — most actionlint warnings are real (shellcheck
embedded). Investigate before commit. If `brew` is unavailable, the
binary is downloadable from the actionlint releases page; recommended
but not strictly required by acceptance criterion (the workflow YAML
is small enough to eyeball).
- [ ] **Local `gh` dry-run check.** Verify the secret is still seeded
(operator says it is, but confirm before pushing):
```bash
gh secret list | grep VERCEL_AUTOMATION_BYPASS_SECRET
```
Expected: one line showing the secret name and an `Updated` timestamp.
- [ ] **Push the branch and observe the first workflow run.** From the
convoy branch (`convoy/fix-vercel-deployment-protection-in-ci`):
```bash
git push -u origin HEAD
```
Then watch the Preview-smoke and Visual-diff workflows. The first
Vercel preview deploy on this PR is the one to scrutinize. Expect:
- Wait-action step logs `target url » https://<deployment>.vercel.app`
(note: no query string in this log — that's the action's safe log
of the bare deployment URL).
- Within a few seconds, `Received success status code`.
- Step exits 0.
- Next step (`Set up Node`, `npm ci`, etc.) runs.
- Eventually fails at the Playwright step — that's the expected end
state of THIS brief.
- [ ] **Re-run validation.** Click "Re-run jobs" on the same run. Expect
identical behavior — the wait-action's healthcheck issues fresh
axios GETs on every iteration (no caching), so re-runs are
idempotent (Risk R-not-listed-because-confirmed-OK).
- [ ] **Bypass-log-leak validation** (as documented above under acceptance
criterion #1, final bullet).
## Boot-the-brief findings (preempted by the architect; do not re-investigate)
### Finding 1 — Wait-action source: `path:` is consumed via `new URL(path, url)`
`patrickedqvist/wait-for-vercel-preview@v1.3.2` at `action.js:42`:
```js
let checkUri = new URL(path, url);
await axios.get(checkUri.toString(), { headers });
```
Parsed as a URL relative to `url` (the deployment URL). Query strings
work verbatim. **MUST begin with `/`** or the URL resolver produces
unexpected paths.
### Finding 2 — Wait-action source: bypass secret never appears in the action's logs
Action source emits exactly three `console.log` calls that include URL
content:
1. `action.js:357` — `console.log('target url »', targetUrl)`. `targetUrl`
is `status.target_url` (the bare deployment URL from the GitHub
Deployments API). **No `path:` is appended.** ✅ Safe.
2. `action.js:363` — `console.log('Waiting for a status code 200 from: ${targetUrl}')`. Same `targetUrl`. ✅ Safe.
3. `action.js:53-54` — `console.log('GET status: ${e.response.status}. Attempt ${i} of ${iterations}')`. Only the HTTP status code, no URL. ✅ Safe.
The bypass query string lives ONLY in the internal `checkUri` axios
call. **This means: passing the bypass via `path:` is structurally
safe from log leakage by the action itself.** Your remaining job is to
not add any echo/print step in the workflow YAML that constructs a URL
with the bypass.
### Finding 3 — `outputs.url` is the bare URL (no bypass query)
`action.js:360`: `core.setOutput('url', targetUrl)` — `targetUrl` does
NOT include `path:`. So `${{ steps.vercel.outputs.url }}` downstream is
clean. **This is why the Playwright `BASE_URL` env var stays bare:** the
future `playwright.config.js` will inject the bypass via
`extraHTTPHeaders`, NOT by reconstructing a URL with the query string.
### Finding 4 — Empty/unset secret on fork PRs
GitHub Actions silently omits repo secrets on `pull_request`-event runs
from forks. If the fork-PR gate (Decision D) is NOT added, fork PRs
would build `https://<deployment>/?x-vercel-protection-bypass=&x-vercel-set-bypass-cookie=true`,
get 401, and time out at `max_timeout` (120s after Decision B, but
still 4 minutes total wasted per fork PR across both workflows). The
fork-PR gate in Decision D prevents this entirely.
### Finding 5 — `max_timeout: 600` is excessive
PR #16's failed run (`gh run view 26370087240`) shows the wait-action
retrieved the deployment URL within 1 second (`target url » ...` at
T+1s relative to job start). The healthcheck then 401-looped for
~600s. With a working bypass, the first axios GET would have succeeded
within 2 seconds. **120s is plenty of headroom.** Decision B applies.
### Finding 6 — Concurrency expression is safe
Current `concurrency:` blocks:
- preview-smoke: `group: preview-smoke-${{ github.event.pull_request.number }}`
- visual-diff: `group: visual-diff-${{ github.event.pull_request.number }}`
No secret reference. Adding the secret via the wait-action's `with:`
and the Playwright step's `env:` does NOT touch `concurrency:`. The
cancel-stale-runs behavior is preserved. Decision C applies.
### Finding 7 — `tests/smoke/app.smoke.spec.ts` is a `.ts` file in a JS-only repo
Out of scope for this brief — flagged for `adopt-playwright-smoke`,
which will own both the Playwright config and the JS/TS decision for
its test files. Do NOT rename or edit it here.
### Finding 8 — actionlint is not installed locally for the implementer
Not a blocker. The brief recommends installing it for local validation
(see Manual verification), but the absence of actionlint in CI today
means it's a recommended-not-required check. A future `adopt-actionlint`
convoy can add it to CI; this brief stays focused on the bypass plumb.
## Out of scope (do not do these)
- [ ] No new Playwright tests or `playwright.config.js`.
- [ ] No `@playwright/test` install.
- [ ] No `package.json` or `package-lock.json` changes.
- [ ] No edits to any other workflow YAML (`ci.yml`, etc.).
- [ ] No edits to `AGENTS.md` § 7 (the wording correction header → query param is the doc-writer pass after this convoy closes).
- [ ] No edits to `.cursor/rules/*.mdc`.
- [ ] No new vitest tests (the existing 16-test suite remains green and is unrelated to this convoy).
- [ ] No replacement of `patrickedqvist/wait-for-vercel-preview` with another action or a hand-rolled `gh api` + `curl` poll loop. That's `replace-wait-for-vercel-preview` (queued, separate scope) and is explicitly out of scope per the convoy file.
- [ ] No tightening or loosening of the `permissions:` blocks in either workflow — PR #16 landed the minimal scope.
- [ ] No `setup-node@v4` version bump, no `actions/checkout@v4` bump, no `actions/upload-artifact@v4` bump. Those are general dependency-bump scope, not this convoy's.
- [ ] No `max_timeout` change beyond the 600 → 120 specified by Decision B.
- [ ] No echo of the constructed URL or BASE_URL in any step. Even for "debugging." If a debug echo is needed during local iteration, remove it before committing.
## Rationale (≤3 sentences)
The wait-action's healthcheck loop is the de-facto bypass-works assertion; injecting the bypass via `path:` query is the only mechanism the action exposes (its `action.yml` has no custom-header input), and the action's source confirms the query never leaks to logs or `outputs.url`. Combining query-param-on-wait with header-form-on-Playwright (deferred to `adopt-playwright-smoke` via the plumb-the-env-var step) keeps both call sites idiomatic for their respective HTTP clients. The fork-PR gate and the `max_timeout` reduction are small operator-quality-of-life refinements that make a 10-minute failure into a 2-minute (or zero-minute) failure when something does go wrong.

25
.github/workflows/preview-smoke.yml vendored
View file

@ -40,8 +40,20 @@ jobs:
steps:
- name: Decide
id: check
env:
# Route PR body + fork flag through env vars instead of inline
# ${{ }} interpolation. Direct ${{ github.event.pull_request.body }}
# in a shell command pastes arbitrary user-controlled text (parens,
# backticks, pipes, heredocs) directly into the script — both a
# syntax-error risk AND a shell-injection vector. Quoting the env
# vars below makes both safe.
PR_BODY: ${{ github.event.pull_request.body }}
PR_IS_FORK: ${{ github.event.pull_request.head.repo.fork }}
run: |
if echo "${{ github.event.pull_request.body }}" | grep -qE 'pipeline:.*skip.*\bsmoke\b'; then
if [[ "$PR_IS_FORK" == "true" ]]; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Smoke skipped on fork PR (bypass secret unavailable to forks)"
elif echo "$PR_BODY" | grep -qE 'pipeline:.*skip.*\bsmoke\b'; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Smoke skipped via pipeline directive"
else
@ -62,7 +74,15 @@ jobs:
uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
with:
token: ${{ secrets.GITHUB_TOKEN }}
max_timeout: 600
max_timeout: 120
# NOTE: do NOT add `&x-vercel-set-bypass-cookie=true` here. Vercel
# responds to that with a 307 + Set-Cookie (`_vercel_jwt`), but
# axios in Node has no cookie jar — the cookie is dropped before
# the followup request, which then 401s. For this one-shot
# healthcheck the bare bypass query is enough; the cookie variant
# belongs in the future Playwright config where the browser does
# have a cookie jar.
path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
- uses: actions/setup-node@v4
with:
@ -78,6 +98,7 @@ jobs:
run: npx playwright test --project=smoke
env:
BASE_URL: ${{ steps.vercel.outputs.url }}
VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
- name: Upload Playwright report on failure
if: failure()

15
.github/workflows/visual-diff.yml vendored
View file

@ -36,8 +36,16 @@ jobs:
should_run: ${{ steps.check.outputs.should_run }}
steps:
- id: check
env:
# See preview-smoke.yml for the rationale: ${{ github.event.* }}
# inlined into shell is a syntax-error + injection vector.
PR_BODY: ${{ github.event.pull_request.body }}
PR_IS_FORK: ${{ github.event.pull_request.head.repo.fork }}
run: |
if echo "${{ github.event.pull_request.body }}" | grep -qE 'pipeline:.*skip.*\bvisual\b'; then
if [[ "$PR_IS_FORK" == "true" ]]; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Visual diff skipped on fork PR (bypass secret unavailable to forks)"
elif echo "$PR_BODY" | grep -qE 'pipeline:.*skip.*\bvisual\b'; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Visual diff skipped via pipeline directive"
else
@ -58,7 +66,9 @@ jobs:
uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
with:
token: ${{ secrets.GITHUB_TOKEN }}
max_timeout: 600
max_timeout: 120
# See preview-smoke.yml for the no-`set-bypass-cookie` rationale.
path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
- uses: actions/setup-node@v4
with:
@ -72,6 +82,7 @@ jobs:
run: npx playwright test --project=visual --update-snapshots=none
env:
BASE_URL: ${{ steps.vercel.outputs.url }}
VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
continue-on-error: true
- name: Upload screenshots + diffs