Smoke-surface-contract convention
Owner: sre-agent
Last incident: 2026-08-09 (docs/incidents/2026-08-09-tickets-e2e-smoke-mailto-drift.md, #4429)
Last reviewed: 2026-08-10 (#4449 — this doc + the guard it documents were added by this card)
Why this exists
PR #4335 (#4262, merged 2026-07-24) redesigned tickets.raxx.app/new-conversation
from a server-rendered form into a static mailto-only contact page, and correctly
updated docs/ops/runbooks/freescout.md in the same PR. It did not touch
scripts/ops/tickets_e2e_smoke.py, which asserted against the old page shape. The
drift went undetected for 13 days: the daily smoke cron kept reporting green-ish
results (a false-positive PASS at Step 3, masked by an independently-broken Step 4)
until an unrelated event incidentally triggered a full manual pipeline run and
surfaced the failure. See the RCA for the full timeline.
The root cause wasn't a person forgetting something — it was that nothing in CI cross-references "a PR changes a surface" against "the smoke script that asserts on that surface got reviewed." This doc describes the lightweight convention that closes that gap, and how to apply it to a new smoke script.
The convention
Any script under scripts/ops/*.py that smoke-tests a surface (an HTML page, a
JSON API response shape, a config file — anything with a "shape" that can silently
change out from under the script's assertions) should do two things:
1. Declare a SURFACE_CONTRACT module-level dict
SURFACE_CONTRACT = {
"watches": [
# repo-relative paths whose content defines the surface's shape.
# Empty list if the surface is hosted elsewhere (e.g. a vendor's own
# API — see tickets_cleanup_smoke.py for that case) with no in-repo
# source file to watch.
"terraform/freescout/assets/new-conversation/index.html",
],
"known_shapes": ["server_form", "mailto_only"], # human-readable, for docs/review
"on_unrecognized_shape": "fail loud — see <function that implements it>",
}
watches is read by scripts/ci/check_smoke_surface_contracts.py (an AST-based
scan — it does not import the scanned scripts, so it has no dependency on their
runtime requirements). That CI check runs on every PR (smoke-surface-drift-check
in .woodpecker/ci-pr.yaml) and prints a non-blocking warning when a watched
path changes without the owning smoke script also changing in the same diff.
It is advisory by design (issue #4449's explicit AC: "doesn't need to block the
PR"). A human still decides whether the smoke script's assertions need updating —
the check just makes sure that decision gets made instead of skipped silently. The
.github/PULL_REQUEST_TEMPLATE.md "Smoke-tested surface checklist" section is the
same reminder in checklist form, for PRs a human reviews without CI's help (or
where the watched file isn't yet declared in any SURFACE_CONTRACT).
2. Fail loud on an unrecognized surface shape at runtime — never silently pass or skip
This is the more important half, and it does not depend on the CI check above being wired up correctly, kept current, or even existing. A smoke script's own runtime behavior is the last line of defense: when it encounters a surface shape it does not recognize, it must produce an unambiguous FAIL (or an explicit, clearly-labeled SKIP with a reason — never a silent PASS or a result indistinguishable from "everything is fine").
Two reference implementations, both in this PR:
tickets_e2e_smoke.py,check_new_ticket_form()— recognizes exactly two page shapes (server_form,mailto_only). A page matching neither is a hard FAIL with a body snippet in the detail message, not a silent SKIP. (This function already had this property before #4449 — it's the pattern to copy for new HTML-surface smokes.)tickets_cleanup_smoke.py,_validate_conversations_shape()— added by #4449. Before this, a FreeScout API response missing the expected_embedded.conversationsHAL structure was silently treated as "zero conversations found," reportingFound 0 conversations. Nothing to clean up.with exit code 0 — indistinguishable from a genuinely empty inbox. That is exactly the false-confidence failure mode the RCA above describes, just in a different script. It now raisesSurfaceShapeError(aRuntimeErrorsubclass), whichrun_cleanup()reports via the sameABORT:path as a 401 or network error.
The test for "did I implement this correctly": if the surface you're smoke-testing were deleted entirely tomorrow, would your script still print something that looks like success? If yes, it doesn't fail loud yet.
Applying this to a new smoke script
- Identify the surface(s) the script asserts against, and whether they have an
in-repo source file (
watchesnon-empty) or are hosted entirely by a third party (watches: [], e.g. a vendor's REST API response shape). - Add the
SURFACE_CONTRACTdict near the script's other module-level constants. - Audit every assertion the script makes against that surface. For each one, ask: what happens if the surface's shape changes to something this script doesn't recognize? If the answer is "it still passes" or "it silently reports zero/empty/nothing-to-do," restructure it to fail loud instead — see the two reference implementations above.
- No further wiring is required —
check_smoke_surface_contracts.pyscansscripts/ops/*.pyforSURFACE_CONTRACTat check time, so a new script with awatchesentry is picked up automatically on the next PR.
Non-goals
- This is not a schema-validation framework. There is no shared base class,
no YAML schema registry, no generic "surface" abstraction. Each smoke
script's shape-recognition logic stays local to that script, matching how
these are already single-file, dependency-light CLI scripts by design (see
the "kept duplicated, not shared" note on
_cf_access_service_token_headersin bothtickets_e2e_smoke.pyandtickets_cleanup_smoke.py). - The CI check does not block merges. If a future incident shows the
advisory-only posture is insufficient, that's a decision for the operator —
file a new
type:reliabilitycard rather than silently hardening this one. - This does not replace the daily/weekly cron smokes themselves as the primary detection mechanism — it only closes the specific "PR changed the surface, nobody thought to check the smoke" gap. A cron that isn't running reliably is a separate problem (tracked separately per the RCA's action item #2, #4450).
References
- RCA:
docs/incidents/2026-08-09-tickets-e2e-smoke-mailto-drift.md - CI check:
scripts/ci/check_smoke_surface_contracts.py, wired assmoke-surface-drift-checkin.woodpecker/ci-pr.yaml - PR template:
.github/PULL_REQUEST_TEMPLATE.md§Smoke-tested surface checklist - Reference implementations:
scripts/ops/tickets_e2e_smoke.py(check_new_ticket_form),scripts/ops/tickets_cleanup_smoke.py(_validate_conversations_shape) - Issue: #4449 (this convention's origin card), #4429 (the incident)