From eab8177d3ea69d16b4aa8e4e213f2207d56b7223 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 08:32:02 +0000 Subject: [PATCH] fix(ci-queue-watch): a blind watchdog must be RED, not green MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The watchdog warned on a missing FLEET_READ_PAT, set an output, and gated its observation step on that output. So a scheduled run with no token skipped the work and the JOB WENT GREEN. It did that ten times. Every one green. Two of them — 07:20Z and 07:57Z — while the fleet was 40 hours into an outage the watchdog exists to detect. Green read as "the fleet is fine"; it actually meant "I looked at nothing". That is the exact vacuous pass this tool was built to find, reproduced inside the detector. The original reasoning was half right, and that half is preserved verbatim in the comment: an environmental gap must not masquerade as a FLEET OUTAGE. What was wrong was the conclusion. The fix is not to go green, it is to fail with a DIFFERENT SIGNAL — this error says the watchdog is blind, ci_queue_watch.py's stall error says the fleet is stuck, and they must never be confusable or the red is as uninformative as the green was. There is no legitimate no-token run of this job: the job-level `if` excludes pull_request, leaving schedule (default branch, full secrets) and workflow_dispatch (requires write access). A missing secret here is misconfiguration, never a transient. scripts/ci_queue_watch.py still exits 0 with no token and test_no_token_skips_cleanly still pins that — correct for a CLI run by hand, where there is no credential to misconfigure. The difference is RUN CONTEXT, which only the workflow knows. The comment says not to "align" them. Three tests, asserted against the REAL workflow file rather than a fixture, because a fixture would have to encode the structure under test and could agree with the bug — the failure mode that let a hardcoded default_branch survive twelve green tests elsewhere in this repo. Mutation-verified, each mutation caught by the assertion written for it: revert guard to warn-and-flag -> 2 failures restore `if: steps.tok...` -> 1 failure drop the outage disclaimer -> 1 failure restored -> 18/18 OK, workflow byte-identical Worth recording: my first attempt at adding these tests silently deleted them — a dedupe of the `__main__` block cut out the class I had just inserted, and the suite reported 18 tests as 15 and went green. Caught by counting, not by the suite. The lesson generalises: a green suite is evidence only if you know what it ran. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GaPa3JgrVNtWrGvqQEAEqv --- .github/workflows/ci-queue-watch.yml | 40 ++++++++++----- scripts/__tests__/test_ci_queue_watch.py | 63 +++++++++++++++++++++++- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci-queue-watch.yml b/.github/workflows/ci-queue-watch.yml index d2cf7056..333fd537 100644 --- a/.github/workflows/ci-queue-watch.yml +++ b/.github/workflows/ci-queue-watch.yml @@ -66,24 +66,42 @@ jobs: # Reading another repository's Actions queue needs a token with fleet # scope; the job's own GITHUB_TOKEN is scoped to this repository alone and - # would report a fleet of one. No token means this watchdog cannot do its - # job -- say so loudly and exit 0. Never red a scheduled run on a missing - # secret: an environmental condition must not look like a fleet outage, - # which is the same inversion the actor gate caused in a2a-maintain. - - name: Check for a fleet-scoped token - id: tok + # would report a fleet of one. + # + # THIS STEP USED TO WARN AND EXIT 0, AND THAT WAS THE DEFECT. The job below + # was gated on its output, so a scheduled run with no token skipped the + # observation and the job went GREEN. It did that ten times, every one + # green, including runs at 07:20Z and 07:57Z while the fleet was 40 hours + # into an outage. Green read as "the fleet is fine"; it actually meant "I + # looked at nothing" -- the exact vacuous pass this watchdog exists to + # detect, reproduced inside the detector. + # + # The original reasoning was half right, and that half is preserved: an + # environmental gap must not masquerade as a FLEET OUTAGE. The resolution + # is not to go green, it is to fail with a DIFFERENT SIGNAL. This error + # says the watchdog is blind. ci_queue_watch.py's stall error says the + # fleet is stuck. If those two were confusable the red would be as + # uninformative as the green was -- hence the distinct title and wording. + # + # There is no legitimate no-token run of this job. The job-level `if` + # excludes pull_request, leaving schedule (always the default branch, full + # secrets) and workflow_dispatch (requires write access). A missing secret + # here is misconfiguration, never a transient. + # + # scripts/ci_queue_watch.py deliberately still exits 0 with no token, and + # test_no_token_skips_cleanly pins that: for a CLI run by hand, no + # credential means nothing to misconfigure. The difference is RUN CONTEXT, + # which only this workflow knows. Do not "align" them. + - name: Require a fleet-scoped token env: FLEET_READ_PAT: ${{ secrets.FLEET_READ_PAT }} run: | if [ -z "$FLEET_READ_PAT" ]; then - echo "::warning::FLEET_READ_PAT is not set — the watchdog can only see this repo, so it is skipping. Set a PAT with read access to Actions across the fleet (scope: repo, or fine-grained Actions:read on the Fuze* repos)." - echo "ok=false" >> "$GITHUB_OUTPUT" - else - echo "ok=true" >> "$GITHUB_OUTPUT" + echo "::error title=Watchdog not configured::FLEET_READ_PAT is unset, so this watchdog can see only its own repository and cannot observe the fleet at all. This is a CONFIGURATION failure and NOT a fleet outage — do not read it as one. Fix: add a PAT with Actions:read across the Fuze* repos as the FLEET_READ_PAT secret on this repository." + exit 1 fi - name: Observe the fleet CI queue - if: steps.tok.outputs.ok == 'true' env: GITHUB_TOKEN: ${{ secrets.FLEET_READ_PAT }} STALL: ${{ inputs.stall_minutes || '30' }} diff --git a/scripts/__tests__/test_ci_queue_watch.py b/scripts/__tests__/test_ci_queue_watch.py index 17f2a474..cd0d0094 100644 --- a/scripts/__tests__/test_ci_queue_watch.py +++ b/scripts/__tests__/test_ci_queue_watch.py @@ -175,5 +175,66 @@ def test_source_excludes_zero_runner_id(self): "never picked anything up reports recent starts and reads healthy") +class BlindWatchdogMustBeRedNotGreen(unittest.TestCase): + """The WORKFLOW-level half of the same defect, invisible to the tests above. + + scripts/ci_queue_watch.py exiting 0 with no token is correct, and + test_no_token_skips_cleanly pins it. The bug lived one layer up: the workflow + warned, set an output, and gated the observation step on it — so a scheduled + run with no token skipped the work and the JOB went green. Ten consecutive + green runs observed nothing, two of them while the fleet was 40 hours into an + outage. + + These assertions read the REAL workflow file, not a fixture. A fixture would + have to encode the structure under test and could therefore agree with the + bug — the failure mode that let a hardcoded default_branch survive twelve + green tests elsewhere in this repo. + """ + + WORKFLOW = os.path.join(REPO, ".github", "workflows", "ci-queue-watch.yml") + + def _watch_job(self): + try: + import yaml + except ImportError: # pragma: no cover + self.skipTest("PyYAML unavailable") + with open(self.WORKFLOW, encoding="utf-8") as fh: + return yaml.safe_load(fh)["jobs"]["watch"] + + def _guard(self): + steps = self._watch_job()["steps"] + g = [s for s in steps if "token" in (s.get("name") or "").lower()] + self.assertEqual(len(g), 1, "expected exactly one token guard step") + return g[0] + + def test_missing_token_fails_the_job_rather_than_skipping_it(self): + """The regression: the guard must exit non-zero, not set a flag.""" + run = self._guard().get("run", "") + self.assertIn("exit 1", run, + "the token guard must FAIL when FLEET_READ_PAT is unset — " + "warning and continuing is what produced ten green runs " + "that had observed nothing") + self.assertNotIn("GITHUB_OUTPUT", run, + "the guard must not export a skip flag; that flag is how " + "the observation step got bypassed silently") + + def test_no_step_is_gated_on_token_presence(self): + """A leftover `if:` would re-open the hole even with the guard failing.""" + for s in self._watch_job()["steps"]: + self.assertNotIn("steps.tok", str(s.get("if", "")), + f"step {s.get('name')!r} is still gated on a token " + f"output — the observation must be unconditional " + f"once the guard has passed") + + def test_blindness_and_stall_are_distinguishable_signals(self): + """A red indistinguishable from a fleet stall is as useless as the green + was. The guard must name itself a CONFIGURATION failure.""" + run = self._guard().get("run", "") + self.assertIn("::error", run) + self.assertIn("NOT a fleet outage", run, + "the error must explicitly disclaim the fleet-outage " + "reading, or an operator acts on the wrong incident") + + if __name__ == "__main__": - unittest.main() + unittest.main(verbosity=2)