From 6b5443289286572c6a37836b05b9b8bfa6aa4d9b Mon Sep 17 00:00:00 2001 From: Elder Miranda de Andrade <187990529+redle82@users.noreply.github.com> Date: Sat, 15 Aug 2026 18:47:47 +0200 Subject: [PATCH] fix(canary): require human smoke confirmation on the all-green path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same root cause as land-and-deploy's Step 7a: canary's alert path already routes through AskUserQuestion (Phase 5), but the most common outcome — nothing broke across the whole monitoring window — never puts the live page in front of a human. Phase 6 prints "VERDICT: DEPLOY IS HEALTHY" straight from automated checks, and Phase 7 offers to promote that unconfirmed state into the new baseline other runs compare against. Phase 6 now opens the live URL and asks for an explicit pass/fail after monitoring completes, distinguishing `Human smoke: CONFIRMED` from `Human smoke: AWAITING` in the report and JSONL log — matching the verdict vocabulary land-and-deploy's Step 9 already uses. Phase 7's baseline-update offer is gated on CONFIRMED so an unconfirmed all-clear can't quietly become the new reference point. Part of the same fix wave as #2580 (land-and-deploy). Scoped this one to canary only after auditing the other ~50 skills for the same pattern (automated verdict of HEALTHY/VERIFIED with no human-confirmation gate on the quiet all-green path) — qa, design-review, ios-design-review, ios-qa, and benchmark all already report per-item evidence (verified/best-effort/ reverted/deferred, or objective metrics) rather than a global "it works in prod" claim, so they don't share this specific gap. Tested: bun run gen:skill-docs --dry-run reports FRESH, exit 0. Targeted bun test (skill-parser, skill-validation, gen-skill-docs, skill-size-budget): 757 pass / 1 fail, the fail is the same pre-existing package.json/VERSION mismatch already on main, unrelated to this diff. --- canary/SKILL.md | 31 ++++++++++++++++++++++++++++--- canary/SKILL.md.tmpl | 31 ++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/canary/SKILL.md b/canary/SKILL.md index 02091fced..85479d1ca 100644 --- a/canary/SKILL.md +++ b/canary/SKILL.md @@ -1023,10 +1023,30 @@ Per-Page Results: Alerts Fired: [N] (X critical, Y high, Z medium) Screenshots: .gstack/canary-reports/screenshots/ +Human smoke: [CONFIRMED (user said pass) / AWAITING (screen opened, no reply yet)] VERDICT: [DEPLOY IS HEALTHY / DEPLOY HAS ISSUES — details above] ``` +**A HEALTHY status from Phases 1-5 is the automated read, not the final word.** +Every alert path already routes through AskUserQuestion (Phase 5) — but the +quiet, all-green path never puts anything in front of the user, which means +the most common outcome (nothing broke) is also the one nobody ever actually +looks at. Before finalizing the report: + +```bash +open "" 2>/dev/null || xdg-open "" 2>/dev/null || echo "Couldn't auto-open — here's the URL: " +``` + +Tell the user: "Automated monitoring found nothing wrong across {N} checks over +{duration} — but that's a script's opinion, not a look. I've opened the live +page. Take a glance, then tell me pass or fail so I can close this out as +actually verified." If the user confirms, mark `Human smoke: CONFIRMED` in the +report. If this is a non-interactive/spawned session, or the user doesn't +reply before you need to finish the report, write `Human smoke: AWAITING` and +say so plainly — do not silently treat "no alerts fired" as "a human looked +and it's fine." + Save report to `.gstack/canary-reports/{date}-canary.md` and `.gstack/canary-reports/{date}-canary.json`. Log the result for the review dashboard: @@ -1036,17 +1056,22 @@ eval "$(~/.claude/skills/gstack/bin/gstack-slug 2>/dev/null)" mkdir -p ~/.gstack/projects/$SLUG ``` -Write a JSONL entry: `{"skill":"canary","timestamp":"","status":"","url":"","duration_min":,"alerts":}` +Write a JSONL entry: `{"skill":"canary","timestamp":"","status":"","human_smoke":"","url":"","duration_min":,"alerts":}` ### Phase 7: Baseline Update -If the deploy is healthy, offer to update the baseline: +If the deploy is healthy AND `Human smoke: CONFIRMED`, offer to update the baseline: -- **Context:** Canary monitoring completed. The deploy is healthy. +- **Context:** Canary monitoring completed. The deploy is healthy and you confirmed it looks right. - **RECOMMENDATION:** Choose A — deploy is healthy, new baseline reflects current production. - A) Update baseline with current screenshots - B) Keep old baseline +If the deploy is healthy but `Human smoke: AWAITING` (nobody confirmed yet), skip +this offer — don't bake an unconfirmed state into the baseline other runs will +compare against. Mention it once: "Once you've had a look, run `/canary +--baseline` if you want this state as the new reference point." + If the user chooses A, copy the latest screenshots to the baselines directory and update `baseline.json`. ## Important Rules diff --git a/canary/SKILL.md.tmpl b/canary/SKILL.md.tmpl index d1eb2950a..0967e5119 100644 --- a/canary/SKILL.md.tmpl +++ b/canary/SKILL.md.tmpl @@ -188,10 +188,30 @@ Per-Page Results: Alerts Fired: [N] (X critical, Y high, Z medium) Screenshots: .gstack/canary-reports/screenshots/ +Human smoke: [CONFIRMED (user said pass) / AWAITING (screen opened, no reply yet)] VERDICT: [DEPLOY IS HEALTHY / DEPLOY HAS ISSUES — details above] ``` +**A HEALTHY status from Phases 1-5 is the automated read, not the final word.** +Every alert path already routes through AskUserQuestion (Phase 5) — but the +quiet, all-green path never puts anything in front of the user, which means +the most common outcome (nothing broke) is also the one nobody ever actually +looks at. Before finalizing the report: + +```bash +open "" 2>/dev/null || xdg-open "" 2>/dev/null || echo "Couldn't auto-open — here's the URL: " +``` + +Tell the user: "Automated monitoring found nothing wrong across {N} checks over +{duration} — but that's a script's opinion, not a look. I've opened the live +page. Take a glance, then tell me pass or fail so I can close this out as +actually verified." If the user confirms, mark `Human smoke: CONFIRMED` in the +report. If this is a non-interactive/spawned session, or the user doesn't +reply before you need to finish the report, write `Human smoke: AWAITING` and +say so plainly — do not silently treat "no alerts fired" as "a human looked +and it's fine." + Save report to `.gstack/canary-reports/{date}-canary.md` and `.gstack/canary-reports/{date}-canary.json`. Log the result for the review dashboard: @@ -201,17 +221,22 @@ Log the result for the review dashboard: mkdir -p ~/.gstack/projects/$SLUG ``` -Write a JSONL entry: `{"skill":"canary","timestamp":"","status":"","url":"","duration_min":,"alerts":}` +Write a JSONL entry: `{"skill":"canary","timestamp":"","status":"","human_smoke":"","url":"","duration_min":,"alerts":}` ### Phase 7: Baseline Update -If the deploy is healthy, offer to update the baseline: +If the deploy is healthy AND `Human smoke: CONFIRMED`, offer to update the baseline: -- **Context:** Canary monitoring completed. The deploy is healthy. +- **Context:** Canary monitoring completed. The deploy is healthy and you confirmed it looks right. - **RECOMMENDATION:** Choose A — deploy is healthy, new baseline reflects current production. - A) Update baseline with current screenshots - B) Keep old baseline +If the deploy is healthy but `Human smoke: AWAITING` (nobody confirmed yet), skip +this offer — don't bake an unconfirmed state into the baseline other runs will +compare against. Mention it once: "Once you've had a look, run `/canary +--baseline` if you want this state as the new reference point." + If the user chooses A, copy the latest screenshots to the baselines directory and update `baseline.json`. ## Important Rules