diff --git a/.mergify.yml b/.mergify.yml index d80dd9a39..be2396b65 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -3,7 +3,7 @@ queue_rules: merge_method: squash batch_size: 4 batch_max_wait_time: 0s - checks_timeout: 60min + checks_timeout: 120min queue_conditions: - base=main - -draft diff --git a/corpus/CLAUDE.learned.md b/corpus/CLAUDE.learned.md index fb09cdd3a..72c5ea4f2 100644 --- a/corpus/CLAUDE.learned.md +++ b/corpus/CLAUDE.learned.md @@ -49,3 +49,4 @@ Engine-only install drops this file; reflect Accepted global rules land here. - When finishing a merge or rebase, reconcile three things before calling it resolved: what each side declared (its commit messages and PR description: the intent, and the tests it names), what each side's code actually does, and what the resolved result does. All three must agree: every declared intent is still visible in `git diff -- `, and every test either side named still passes on the result. A merge with no textual conflict can still break one side's declared behavior, so a clean merge is checked the same way. Never take one side's whole file (`git checkout --theirs` / `--ours `, or copying the file from one branch) unless the other side's changes to that file are provably empty; taking a whole side silently reverts what the other branch added, including hunks that never conflicted (git-checkout(1), `--ours`/`--theirs`: https://git-scm.com/docs/git-checkout#Documentation/git-checkout.txt---ours). When a declaration and its code disagree, or the two sides' intents contradict, stop and flag it to me instead of choosing: run `why` on the conflicting lines (`git log -L` or `git log -S` plus each commit's PR body) and apply `principle-prove-it`, so the flag names the two commits or PRs, quotes the declared intent next to the code, and pastes the failing test output. Brun, Holmes, Ernst and Notkin, "Proactive Detection of Collaboration Conflicts" (ESEC/FSE 2011, https://doi.org/10.1145/2025113.2025139), across Git, Perl5 and Voldemort, found "that 33% of merges that were reported to contain no textual conflicts by the VCS in fact contained higher-order conflicts" (build or test failures), so a clean textual merge is not evidence the two intentions still hold. - An item I approved as "fix it properly later" stays in every status update and every final summary, marked open, until it is done or I drop it. A recap that lists only what finished makes the deferred half disappear, and the next session never learns it was owed. No known prior art. - Before spending more than a couple of tool calls on an infrastructure, security, or "does X exist" question, search saved memory for the topic and for its reverse framing: the fix as well as the symptom, "X is off" as well as "X is missing". An answer already written down costs one read; rediscovering it costs the whole investigation. No known prior art. +- When I ask what made, wrote, or owns something I say is mine (a page, a file, a job, a merge request), list my own items first — everything I created or authored, newest first — and match against that list before any keyword search over everyone's items. A keyword search finds only the words the author chose; an author filter finds the item whatever it is called, and my own list is short enough to read in full. No known prior art. diff --git a/corpus/skills/cat-mode/SKILL.md b/corpus/skills/cat-mode/SKILL.md index 3464274d4..200a03264 100644 --- a/corpus/skills/cat-mode/SKILL.md +++ b/corpus/skills/cat-mode/SKILL.md @@ -22,7 +22,8 @@ Once direction is set, act — don't ask permission for each sub-step. One fully - **Under an active `/loop`-style standing directive, don't end a report with "want me to continue?"** A trailing question is a permission request. Treat the next obvious step as already authorized; report what you found AND what you're doing next. Ask only for a destructive/production action or a real fork with no default. - **Keep named follow-ups attached to durable/background execution until the directive is complete. Completion includes every invoked skill's required landing phase.** Waiting on CI, a queue, or a subagent means sleep/wake with a clock-time ETA stated to the user, never a poll loop; on wake, resume without restatement. Arming a watcher and yielding is not waiting. -- **Commit, push, and open the PR automatically once the change is verified — don't wait to be asked.** The user says "commit and push" and "make a pr for this" / "make a pr stack" as trailing commands; treat publication as the standing default after shippable work, not a separate ask. Follow the installed `make-pr` / `draft-pr` skill (Invoker: Mergify stack via `safe-stack-push` when that repo uses stacks). "Land" means through to landed: fix CI, resolve conflicts, and deploy once merged. Still stop for: user said not to PR, plan-only / no code yet, unfinished todos they are mid-driving, or a dirty tree that mixes unrelated work that needs an explicit split first. Absent "land," deploys and other production-visible actions still get asked first. +- **Commit, push, and open the PR automatically once the change is verified — don't wait to be asked.** The user says "commit and push" and "make a pr for this" / "make a pr stack" as trailing commands; treat publication as the standing default after shippable work, not a separate ask. Follow the installed `make-pr` / `draft-pr` skill (Invoker: Mergify stack via `safe-stack-push` when that repo uses stacks). "Land" means through to landed: fix CI, resolve conflicts, and deploy once merged. Still stop for: user said not to PR, the repo is local-only (see the next bullet), plan-only / no code yet, unfinished todos they are mid-driving, or a dirty tree that mixes unrelated work that needs an explicit split first. Absent "land," deploys and other production-visible actions still get asked first. +- **A local-only repo ends in a local branch and a local merge, never GitHub.** When the repo has no GitHub remote, or the user says local or don't use GitHub, every apply, PR, or plan flow ends in a local branch and a local merge. Never push, run the GitHub CLI, or probe GitHub credentials. Ask once if unsure. Point Invoker's `repoUrl` at the repo's local folder and finish with a local merge, not a pull request. - **A named multi-step done-gate stays locked until receipt.** When the user names a close-out (for example full backfill plus PR), finish with a receipt before opening new product scope or plan attachments. - **Do not race publication against a live long job in the same thread.** Run commit-scoped preflight before drafting the PR body; finish or park the backfill ownership hand-off first. - **Cursor `/pr-skill` is not Invoker's merge-gate.** Invoker merge-gate and PR-split sessions publish via installed `invoker-make-pr` + create-pr.mjs / `gh api`; implement/slice tasks do not publish PRs. Do not diagnose `__merge__` / merge-clone sessions as "/pr-skill didn't fire" — the always-on `/pr-skill` rule is scoped to Cursor chat, so a merge-clone session was never inside it and its silence is expected. @@ -34,6 +35,7 @@ Once direction is set, act — don't ask permission for each sub-step. One fully - Destructive or hard-to-reverse actions (force-push, bypassing a merge queue guard, schema changes) get one stop-and-ask. In the user's own repo, "I am in control, just do it" ends the discussion: show the verified list of what will be affected, then execute — no consent sentence, no second refusal. - `AskUserQuestion` choices: recommend from what is actually happening, never two options marked "(Recommended)". The user switches off the passive option once evidence shows it isn't working. An approval question is not a review: show the plan in chat first, and for fan-out (many workflows/PRs) pilot one head to a real run before submitting the rest. - **Prefer the obvious existing mechanism before designing a new one.** +- **Look for a cheaper source before grinding.** - **Do not kill/restart a live Invoker `owner-serve` as the default lever.** - **Ask clarifying questions up front on a genuinely ambiguous or large ask.** - **Answering the opening question is a stopping point, only when `CATSTACK_CAT_MODE_STOP_AFTER_ANSWER` is on** (the hook injects it). @@ -143,6 +145,7 @@ CLAUDE.md's evidence rules already apply here. Also, don't declare something fix - **The report of a write is not the write's effect.** A success message, a merged status, a 200 — none prove it. Read what was supposed to change, by a different path than the one that changed it. - **Never discard a mutating command's output.** `/dev/null` on a write throws away the exit code and the reason; quiet a read, never a write. +- **Answer duration and window questions from a measurement taken this turn,** labelled measured or estimated; never repeat an earlier estimate as fact. - **Read a gate before calling it broken or calling it fine.** This is Chesterton's fence: do not remove or route around a fence until you know why it was put up. Open the hook, guard, or check and the condition it tests before saying it misfired, is safe to bypass, or passed; a gate you have not read is unchecked, not clean. Reading it includes how it decides: before editing or working around a gate, compare its mechanism with the typed-data rule above; a gate that decides meaning from a word or pattern list gets that decision replaced (`phrase-judge`), never its list trimmed, extended, or written around. - **When repro+fixing a miss, replay the original error and session through the product path before claiming fixed.** Hand-built detector payloads and nearby shapes are not the original session. Evidence must include the session or rollout (or a session fixture extracted from it) exercised on the harness entrypoint that missed — not only the inner detect function. Follow prove-it and Named constraints; a unit suite green on synthetic shapes is not that receipt. @@ -184,11 +187,11 @@ What happens to a number once it exists: - **Split volume vs fill vs resolve vs filter before any L3 or UI insight PATCH when a harness looks missing or a real-model tile reads zero** (for example "missing harness X" or "real model = 0"). - **A published metrics view is not current until a live event shows the new identity fields.** Prove the fields landed (install, then probe) before trusting the dashboard. Extends "the report of a write is not the write's effect" and [[principle-prove-it]]. - **Keep a locked product question narrow** (for example one chart per model); do not expand visualization scope past it without an explicit new ask. Follow `narrow-the-scope`. -- **Climb analytics tiers in order:** L1 explain → L2 one canary query → L3 backfill/fleet only after the canary proves an identity or cardinality gap. +- **Climb analytics tiers in order:** L1 explain → L2 one canary query → L3 backfill/fleet only after the canary proves an identity or cardinality gap. Prove a new data path on a small window first, report the measured time, then widen. - **Schema before UI.** Required identity properties plus a green canary before the first insight or notebook PATCH; split the forward-stamp PR from the historical-backfill PR. - **Claim a chart gap fixed only after naming write vs enrich ownership** (local vs peer vs fleet). - **Write a thrash checkpoint before attaching new scope:** after two same-type interventions or the first live backfill, record tier and canary status first. -- **When a same-type miss forces a working-style catch mid analytics thrash, stop feature PRs until that catch is locked** (for example the first `intervention-must-automate` flag, an explicit `/automate-me`, or a same-type restatement). +- **Stop analytics feature iteration and lock the class catch before more analytics PRs when automate is mandated mid thrash** (for example the first `intervention-must-automate` flag, an explicit `/automate-me`, or a same-type restatement). Each rule's full text: [references/verify.md](references/verify.md). diff --git a/corpus/skills/cat-mode/references/autonomy.md b/corpus/skills/cat-mode/references/autonomy.md index 6de3b6c74..a2d4e1194 100644 --- a/corpus/skills/cat-mode/references/autonomy.md +++ b/corpus/skills/cat-mode/references/autonomy.md @@ -7,6 +7,12 @@ autonomy defaults. command or workflow already performs the requested action, run it first and report the actual result. Redesign only when explicitly requested or after it fails. +- **Look for a cheaper source before grinding.** Before running, re-running + or widening a slow or capped bulk job, ask what already holds the data (an + existing export, another system, a config file, a teammate's tooling), rank + the cheaper routes by measured cost, and test the cheapest first. Repeating + a capped run unchanged, or only raising its cap, is the last resort. No + known prior art. - **Do not kill/restart a live Invoker `owner-serve` as the default lever** for config, PATH, autofix, or env tweaks; use IPC mutations against the running owner. Restart only when it is already dead or the user asked. diff --git a/corpus/skills/cat-mode/references/verify.md b/corpus/skills/cat-mode/references/verify.md index 0438f5396..b7c2cd685 100644 --- a/corpus/skills/cat-mode/references/verify.md +++ b/corpus/skills/cat-mode/references/verify.md @@ -67,6 +67,12 @@ place immediately, not left in chat until asked again. qualifies, however noisy it is. Fail-fast — Jim Shore, [Fail Fast](https://martinfowler.com/ieeeSoftware/failFast.pdf), IEEE Software 21(5) 2004 — and [[principle-explicit-errors]]. +- **Answer duration and window questions from a measurement taken this + turn.** When asked how long something takes, how far back a run looks, or + how many items it covers, read the answer from a timestamp, counter or + parameter measured in this turn and label every figure measured or + estimated; never repeat an earlier estimate as fact. Same evidence + standard as [[principle-prove-it]]. - **A teardown-only traceback is a flake: rerun once before diagnosing.** The signature is a test that "fails" while its own assertion passed — the failing frame sits in cleanup or fixture teardown and the rest of the suite is green. @@ -201,6 +207,8 @@ defaults above and [[principle-push-not-poll]]; they do not replace them. fields. L2 run one canary query that proves an identity or cardinality gap. L3 open backfill or fleet work only after that canary. Jumping to L3 because a chart still looks wrong is rediscovery thrash, not verification. + The same ladder applies to any new data path: prove it on a small window + first, report the measured time and counts, then widen. - **Schema before UI.** Required identity properties plus a green canary must exist before the first insight or notebook PATCH. Ship the forward-stamp (emit-path) PR separately from the historical-backfill PR — @@ -214,7 +222,8 @@ defaults above and [[principle-push-not-poll]]; they do not replace them. of a run, write the current tier and canary status before attaching new stack publication or plan scope. A checkpoint is a receipt, not a status ping. -- **When a same-type miss forces a working-style catch mid analytics thrash, stop feature PRs until that catch is locked** (for example the +- **Stop analytics feature iteration and lock the class catch before more + analytics PRs when automate is mandated mid thrash** (for example the first `intervention-must-automate` flag, an explicit `/automate-me`, or a same-type restatement). The standing default that would have prevented the miss lands first; dashboard, backfill, and insight work wait until @@ -226,4 +235,4 @@ defaults above and [[principle-push-not-poll]]; they do not replace them. resume when the artifact arrives. Chat process greps, log tails, and wait-and-repeat shell loops are the poll defect [[principle-push-not-poll]] already forbids — this rule names the - ownership hand-off, not another check-wait-repeat count. \ No newline at end of file + ownership hand-off, not another check-wait-repeat count. diff --git a/corpus/skills/cat-mode/tests/fires_local_only_no_github.md b/corpus/skills/cat-mode/tests/fires_local_only_no_github.md new file mode 100644 index 000000000..6bda9d6d8 --- /dev/null +++ b/corpus/skills/cat-mode/tests/fires_local_only_no_github.md @@ -0,0 +1,2 @@ +User: "Apply this plan. The repo is local, don't use GitHub." +Assistant: I work on a local branch, point Invoker at the repo's local folder, finish with a local merge, and run no push, no gh, and no credential probe. diff --git a/corpus/skills/cat-mode/tests/fires_new_data_path_straight_to_full_window.md b/corpus/skills/cat-mode/tests/fires_new_data_path_straight_to_full_window.md new file mode 100644 index 000000000..75eb1e681 --- /dev/null +++ b/corpus/skills/cat-mode/tests/fires_new_data_path_straight_to_full_window.md @@ -0,0 +1,2 @@ +User: "Try the new collector." +Assistant: Run it on the full seven-day window first and report the time afterwards, with no small-window run to prove the path. diff --git a/corpus/skills/cat-mode/tests/fires_quotes_estimate_as_duration.md b/corpus/skills/cat-mode/tests/fires_quotes_estimate_as_duration.md new file mode 100644 index 000000000..5ab1ec98d --- /dev/null +++ b/corpus/skills/cat-mode/tests/fires_quotes_estimate_as_duration.md @@ -0,0 +1,2 @@ +User: "How long does it take to fetch all the jobs with a 6 hour lookback?" +Assistant: About twenty minutes, repeating a figure from an earlier turn without measuring it this turn or labelling it an estimate. diff --git a/corpus/skills/cat-mode/tests/fires_rerun_capped_job_without_cheaper_source.md b/corpus/skills/cat-mode/tests/fires_rerun_capped_job_without_cheaper_source.md new file mode 100644 index 000000000..d19fb0e94 --- /dev/null +++ b/corpus/skills/cat-mode/tests/fires_rerun_capped_job_without_cheaper_source.md @@ -0,0 +1,2 @@ +User: "The 6 hour pull stopped at its cap again." +Assistant: Raise the cap and rerun the same slow collection, without asking what already holds the data or testing a cheaper route. diff --git a/corpus/skills/cat-mode/tests/stays_silent_cheaper_source_checked_first.md b/corpus/skills/cat-mode/tests/stays_silent_cheaper_source_checked_first.md new file mode 100644 index 000000000..0f2bf9ec0 --- /dev/null +++ b/corpus/skills/cat-mode/tests/stays_silent_cheaper_source_checked_first.md @@ -0,0 +1,2 @@ +User: "The 6 hour pull stopped at its cap again." +Assistant: List what already holds the data (an existing export, another system, a config file), measure each route's cost, and test the cheapest first before running the capped job again. diff --git a/corpus/skills/cat-mode/tests/stays_silent_measured_duration_labelled.md b/corpus/skills/cat-mode/tests/stays_silent_measured_duration_labelled.md new file mode 100644 index 000000000..37e526f7a --- /dev/null +++ b/corpus/skills/cat-mode/tests/stays_silent_measured_duration_labelled.md @@ -0,0 +1,2 @@ +User: "How long does it take to fetch all the jobs with a 6 hour lookback?" +Assistant: The last run printed 581 s for 1,500 requests (measured); a full read is estimated at about 3,030 requests (estimated). diff --git a/corpus/skills/cat-mode/tests/stays_silent_remote_repo_publishes.md b/corpus/skills/cat-mode/tests/stays_silent_remote_repo_publishes.md new file mode 100644 index 000000000..490f77bd2 --- /dev/null +++ b/corpus/skills/cat-mode/tests/stays_silent_remote_repo_publishes.md @@ -0,0 +1,2 @@ +User: "Verified. Ship it." (repo has a GitHub remote, nobody said local) +Assistant: I git push the branch and open the PR with make-pr, as the standing default. diff --git a/corpus/skills/cat-mode/tests/stays_silent_small_window_first_then_widen.md b/corpus/skills/cat-mode/tests/stays_silent_small_window_first_then_widen.md new file mode 100644 index 000000000..2a57f165c --- /dev/null +++ b/corpus/skills/cat-mode/tests/stays_silent_small_window_first_then_widen.md @@ -0,0 +1,2 @@ +User: "Try the new collector." +Assistant: Run it on a 6 hour window first, report the measured time and counts, then widen to a longer window. diff --git a/corpus/skills/cat-mode/tests/test_artifact_sweeps.py b/corpus/skills/cat-mode/tests/test_artifact_sweeps.py index 3ef67b1fd..75d2925b7 100644 --- a/corpus/skills/cat-mode/tests/test_artifact_sweeps.py +++ b/corpus/skills/cat-mode/tests/test_artifact_sweeps.py @@ -22,6 +22,16 @@ def test_correction_surface_rule_has_trigger_fixture(self): for required in ("raw data", "aggregate table", "chart source", "live Sheet read-back"): self.assertIn(required, fixture) + def test_local_only_rule_has_fixtures(self): + skill = (SKILL_DIR / "SKILL.md").read_text(encoding="utf-8") + fires = (SKILL_DIR / "tests" / "fires_local_only_no_github.md").read_text(encoding="utf-8") + silent = (SKILL_DIR / "tests" / "stays_silent_remote_repo_publishes.md").read_text(encoding="utf-8") + + self.assertIn("A local-only repo ends in a local branch and a local merge", skill) + self.assertIn("local merge", fires) + self.assertNotIn("git push", fires.split("Assistant:")[1]) + self.assertIn("git push", silent) + if __name__ == "__main__": unittest.main() diff --git a/engine/hooks/diu-stop/claude_stop_check.py b/engine/hooks/diu-stop/claude_stop_check.py index 6d860074d..7e123b547 100755 --- a/engine/hooks/diu-stop/claude_stop_check.py +++ b/engine/hooks/diu-stop/claude_stop_check.py @@ -136,6 +136,12 @@ re.IGNORECASE | re.DOTALL, ) +ABSENCE_CLAIM_RE = re.compile( + r"\b(?:has|have|had) no\b|\bthere (?:is|are|was|were) no\b|\bno single\b" + r"|\bno such (?:call|api|endpoint|function|way)\b|\b(?:can't|cannot|can not) be done\b", + re.IGNORECASE, +) + SENTENCE_END_RE = re.compile(r"(?<=[.!?])\s+") SENTENCE_LIMIT = 120 @@ -208,7 +214,7 @@ def _paragraph_claim(para): opener = _opening_word(para) if opener in BANNED_OPENERS: return opener, 0 - for pattern in (CAUSAL_CLOSER_RE, HEDGE_CLAIM_RE): + for pattern in (CAUSAL_CLOSER_RE, HEDGE_CLAIM_RE, ABSENCE_CLAIM_RE): match = pattern.search(para) if match: return match.group(0), match.start() @@ -371,10 +377,11 @@ def detect(event): lines.append(f"{number}. \"{sentence}\" (trigger: \"{' '.join(phrase.split())}\")") lines.append( "A backticked name or command alone is not output, and neither is a " - "bare file:line. Per skills/prove-it/SKILL.md: for each one, either " - "paste the output of what was actually run/checked in its paragraph, " - "cite it as `path:line @ `, or -- only if the check cannot run " - "-- tag the claim there and say why." + "bare file:line. Per skills/prove-it/SKILL.md: for each one, first " + "run the check now (Bash/Read/Grep/WebFetch) and paste its output in " + "its paragraph, or cite it as `path:line @ `; or tag it last. A " + "tag is only for a check that was attempted and could not settle the " + "claim, and it names the blocker." ) unchecked = unchecked_citations(message, read_blob) if unchecked: diff --git a/engine/hooks/diu-stop/tests/fixtures/claims_fires.json b/engine/hooks/diu-stop/tests/fixtures/claims_fires.json index 3765e413e..2f14008e9 100644 --- a/engine/hooks/diu-stop/tests/fixtures/claims_fires.json +++ b/engine/hooks/diu-stop/tests/fixtures/claims_fires.json @@ -73,5 +73,30 @@ "label": "which is why, the message's only fence holds a command, not output", "reply": "Both commands are in, and tools are **still** blocked. I also can't read the hook to diagnose it — the hook blocks that read too.\n\n**Run this and paste the output:**\n\n```\n! cat ~/.claude/hooks/scope-lock/claude_pretool_scope.py\n```\n\nThat shows me what actually clears the lock. Right now I'm guessing, and guessing is what got us here.\n\nWorth noting for the findings: a gate that blocks the read of its own source can't be diagnosed from inside the session it's gating — same shape as the `disable-model-invocation` rule already in your corpus, which is why that rule says the fix must live outside the thing it protects.", "hit": "which is why" + }, + { + "label": "absence claim, has no", + "reply": "GitLab has no call that returns a merge request's whole job graph, so the client has to stitch it together.", + "hit": "has no" + }, + { + "label": "absence claim, there is no", + "reply": "There is no way to read pipeline stages in one request from the public API.", + "hit": "there is no" + }, + { + "label": "absence claim, no single", + "reply": "I found no single endpoint that lists every job with its needs.", + "hit": "no single" + }, + { + "label": "absence claim, no such call", + "reply": "The SDK exposes no such call for listing job dependencies.", + "hit": "no such call" + }, + { + "label": "capability claim, can't be done", + "reply": "Merging the two requests can't be done without changing the schema.", + "hit": "can't be done" } ] diff --git a/engine/hooks/diu-stop/tests/fixtures/claims_silent.json b/engine/hooks/diu-stop/tests/fixtures/claims_silent.json index 5780a76bc..792f7d688 100644 --- a/engine/hooks/diu-stop/tests/fixtures/claims_silent.json +++ b/engine/hooks/diu-stop/tests/fixtures/claims_silent.json @@ -6,5 +6,17 @@ { "label": "because with a backticked assignment, the message carries a fenced block of output", "reply": "**Correcting myself — it's alive.** The harness reported the wrapper exiting, not the job:\n\n```\n$ pgrep -af replay_causality\n292765 uv run python scripts/replay_causality.py\n292775 .venv/bin/python3 scripts/replay_causality.py\n\n$ tail -2 /tmp/replay.log\nreplaying 400,156 bars, full recompute at each\n 2,000/400,156 bars, 0 mismatches\n```\n\n**2,000 of 400,156 done, 0 mismatches.** Log looks stalled only because `PROGRESS_EVERY = 2000`." + }, + { + "label": "absence claim next to fenced output", + "reply": "The schema has no `needs` field:\n\n```\n$ grep -c needs schema.graphql\n0\n```" + }, + { + "label": "absence claim carrying a well-formed tag", + "reply": "{{CAT-UNVERIFIED: GitLab has no single call for the job graph -- cannot verify: the GraphQL docs page timed out twice}}" + }, + { + "label": "plain reply with no absence phrase", + "reply": "Ed said the box is fine and the run is done." } ] diff --git a/engine/hooks/diu-stop/tests/test_hooks.py b/engine/hooks/diu-stop/tests/test_hooks.py index 97834a718..59a4ea8e8 100644 --- a/engine/hooks/diu-stop/tests/test_hooks.py +++ b/engine/hooks/diu-stop/tests/test_hooks.py @@ -569,6 +569,16 @@ def test_a_fabricated_path_no_longer_silences_the_claim(self): self.assertTrue(blocked) self.assertIn("bare file:line", err) + def test_notice_lists_run_the_check_first_and_tag_last(self): + blocked, err = run_claude_check({ + "last_assistant_message": "The issue was the stale guard in the retry loop."}) + self.assertTrue(blocked) + lowered = err.lower() + run_at = lowered.index("run the check now (bash/read/grep/webfetch)") + tag_at = lowered.index("or tag it last") + self.assertLess(run_at, tag_at) + self.assertIn("only for a check that was attempted and could not settle", lowered) + def test_a_citation_carrying_its_ref_silences_without_any_transcript(self): """The B case: the citation says where it was read, so it stands alone.""" message = "The issue was the stale guard at src/file.ts:1276 @ origin/main." diff --git a/engine/hooks/frustration-watchdog/claude_stop_check.py b/engine/hooks/frustration-watchdog/claude_stop_check.py index 8b9e6f513..134b5744f 100755 --- a/engine/hooks/frustration-watchdog/claude_stop_check.py +++ b/engine/hooks/frustration-watchdog/claude_stop_check.py @@ -86,6 +86,11 @@ r"i(?:'ll| will) (?:handle|do|take|run|fix|keep|watch))\b" ) +CHECK_IN_FLIGHT_RE = re.compile( + r"(?i)\bi(?:'m| am) (?:running|checking|testing|probing|reading|querying|fetching) " + r"[^.\n]{1,100}?\bnow\b" +) + HOOK_REFUSAL_RE = re.compile(r"\s*PreToolUse:\S+ hook error\b") """How Claude Code records a tool call a PreToolUse hook refused: an is_error tool_result whose text opens "PreToolUse:Bash hook error: []: ...".""" @@ -209,7 +214,9 @@ def ends_the_wait(message): stripped = message.rstrip() if stripped.endswith("?"): return True # a direct question is a handoff - return bool(NEXT_STEP_RE.search(message) or ETA_RE.search(message)) + return bool( + NEXT_STEP_RE.search(message) or ETA_RE.search(message) or CHECK_IN_FLIGHT_RE.search(message) + ) def detect(data): diff --git a/engine/hooks/frustration-watchdog/tests/test_hooks.py b/engine/hooks/frustration-watchdog/tests/test_hooks.py index cac956e73..79e633d50 100755 --- a/engine/hooks/frustration-watchdog/tests/test_hooks.py +++ b/engine/hooks/frustration-watchdog/tests/test_hooks.py @@ -72,6 +72,23 @@ def test_waiting_user_with_concrete_action_passes(self): finally: os.unlink(path) + def test_check_in_flight_counts_as_next_step(self): + path = transcript_with([("2026-08-18T02:13:30Z", "i am waiting for you to do something")]) + try: + blocked, _ = run_hook(path, "I am running the GraphQL probe now. Result in this turn.") + self.assertFalse(blocked) + finally: + os.unlink(path) + + def test_narrated_past_work_is_not_a_check_in_flight(self): + path = transcript_with([("2026-08-18T02:13:30Z", "i am waiting for you to do something")]) + try: + blocked, err = run_hook(path, "I ran the GraphQL probe earlier and it looked fine.") + self.assertTrue(blocked) + self.assertIn("impatience-shaped", err) + finally: + os.unlink(path) + def test_direct_question_counts_as_handoff(self): path = transcript_with([("2026-08-18T02:13:30Z", "WHY ARE WE NOT LAUNCHING A ZOOM MEETING RIGHT NOW")]) try: diff --git a/engine/hooks/unverified-tag-ledger/detect.py b/engine/hooks/unverified-tag-ledger/detect.py index 475ec645b..6c6829bb8 100644 --- a/engine/hooks/unverified-tag-ledger/detect.py +++ b/engine/hooks/unverified-tag-ledger/detect.py @@ -38,6 +38,7 @@ VERIFY_TOOLS = {"Bash", "Read", "Grep", "Glob", "NotebookRead"} ESCALATE_AFTER_TURNS = 3 +ACTIVE_REBLOCKS = 1 MAX_LISTED = 5 BEHAVIOR_FLAG = "CATSTACK_UNVERIFIED_TAG_BEHAVIOR" @@ -66,7 +67,20 @@ re.IGNORECASE | re.DOTALL, ) +HANDBACK_RE = re.compile( + r"\b(?:say|reply|type|tell me to)\s+(?:the word\s+)?(?:[\"'\u2018\u201c][^\"'\u2019\u201d]{1,60}[\"'\u2019\u201d]" + r"|(?:go|test|test it|run it|check it|yes|ok|okay|probe it)\b)", + re.IGNORECASE, +) +WRITE_ACTION_RE = re.compile( + r"\b(?:push|deploy|delete|merge|commit|post|send|apply|write|create|publish|" + r"overwrite|force|restart|kill|install|submit|approve|release)\w*\b", + re.IGNORECASE, +) +SENTENCE_SPLIT_RE = re.compile(r"(?<=[.!?])\s+|\n+") + RULE_NO_VERIFICATION_TOOL = "unverified-tag-ledger.no-verification-tool" +RULE_HANDBACK = "unverified-tag-ledger.handback" RULE_DO_NOT_EMIT = "unverified-tag-ledger.do-not-emit" RULE_PROMPT_REMINDER = "unverified-tag-ledger.prompt-reminder" @@ -242,6 +256,22 @@ def emitted_tags(message: str) -> list[str]: return [match.group(0) for match in markers.TAG_RE.finditer(prose)] +def handback_offers(message: str) -> list[str]: + """Sentences outside code that hand a read-only check back to the user. + + "say 'test it'" and "tell me to test it" make the user the trigger for a + check the agent can run itself. An offer whose sentence names a write + (push, deploy, delete, ...) is a real approval gate and is left alone. + """ + prose = INLINE_CODE_RE.sub("", FENCED_RE.sub("", message or "")) + offers = [] + for sentence in SENTENCE_SPLIT_RE.split(prose): + sentence = " ".join(sentence.split()) + if HANDBACK_RE.search(sentence) and not WRITE_ACTION_RE.search(sentence): + offers.append(sentence) + return offers + + def evaluate(payload: dict, mode: str | None = None) -> dict: """Record the turn, then decide whether it may end. @@ -251,9 +281,11 @@ def evaluate(payload: dict, mode: str | None = None) -> dict: refused. Requiring an attempt is not requiring success: run the check, and if it cannot run or comes back inconclusive, the tag is then honest. - `stop_hook_active` releases the block so the rewrite turn can finish -- - without it the refusal loops forever, because a reply being rewritten to - satisfy this hook has no tool call of its own either. + `stop_hook_active` marks a rewrite after another Stop hook blocked. It used + to release this block outright, so a tag written to satisfy diu-stop's + unverified-claim notice went through unchecked. It now releases only after + ACTIVE_REBLOCKS blocks already counted against the claim, so the refusal + still cannot loop forever. """ if mode is None: mode, note = behavior_mode(cwd=payload.get("cwd")) @@ -290,6 +322,14 @@ def evaluate(payload: dict, mode: str | None = None) -> dict: if not new_claims: return {"note": "\n".join(notes), "block": ""} + offers = handback_offers(message) + if offers: + return {"note": "", "block": ( + "unverified-tag-ledger: this reply tags a claim as unverified and hands the check back " + f"to the user: {'; '.join(offers[:MAX_LISTED])}. The check is read-only, so the user " + "does not need to trigger it. Run the check now (Bash/Read/Grep/WebFetch) and paste its " + "output. This block is not released by stop_hook_active: deleting the offer always ends the loop.")} + if tools is None: notes.append( f"unverified-tag-ledger: logged {len(new_claims)} CAT-UNVERIFIED claim(s), but this " @@ -298,7 +338,14 @@ def evaluate(payload: dict, mode: str | None = None) -> dict: "the turn was not refused.") return {"note": "\n".join(notes), "block": ""} - if not tools & VERIFY_TOOLS and not payload.get("stop_hook_active"): + prior_blocks = max( + (row.get("no_tool_blocks", 0) for row in rows if row["claim"] in new_claims), default=0) + released = payload.get("stop_hook_active") and prior_blocks > ACTIVE_REBLOCKS + if not tools & VERIFY_TOOLS and not released: + for row in rows: + if row["claim"] in new_claims: + row["no_tool_blocks"] = row.get("no_tool_blocks", 0) + 1 + write_ledger(session_id, rows) claims = "; ".join(sorted(new_claims)[:MAX_LISTED]) return {"note": "", "block": ( "unverified-tag-ledger: this turn tags a claim as unverified but ran no " @@ -339,6 +386,16 @@ def _stop_findings(event: dict[str, object]) -> list[Finding]: if not block: return [] message = _last_assistant_text(payload) + if "hands the check back" in block: + return [ + Finding( + rule_id=RULE_HANDBACK, + subject=offer, + message=block, + evidence=offer, + ) + for offer in handback_offers(message) + ] if behavior == "do_not_emit": tags = emitted_tags(message) return [ diff --git a/engine/hooks/unverified-tag-ledger/tests/test_hooks.py b/engine/hooks/unverified-tag-ledger/tests/test_hooks.py index cfd4633a3..a88df393a 100644 --- a/engine/hooks/unverified-tag-ledger/tests/test_hooks.py +++ b/engine/hooks/unverified-tag-ledger/tests/test_hooks.py @@ -174,9 +174,78 @@ def test_blocked_turn_is_still_recorded(self) -> None: self.detect.evaluate(self.payload(REAL_TAG_1, tools=False)) self.assertEqual(len(self.detect.outstanding(self.detect.read_ledger("s1"))), 1) - def test_rewrite_turn_is_released_so_the_block_cannot_loop(self) -> None: + def test_rewrite_turn_after_a_block_is_blocked_once_more_for_a_tag_with_no_check(self) -> None: verdict = self.detect.evaluate( self.payload(REAL_TAG_1, tools=False, stop_hook_active=True)) + self.assertIn("ran no verification tool", verdict["block"]) + + def test_rewrite_turns_are_released_after_the_reblock_so_the_block_cannot_loop(self) -> None: + verdicts = [ + self.detect.evaluate(self.payload(REAL_TAG_1, tools=False, stop_hook_active=True)) + for _ in range(3)] + self.assertIn("ran no verification tool", verdicts[0]["block"]) + self.assertIn("ran no verification tool", verdicts[1]["block"]) + self.assertEqual(verdicts[2]["block"], "") + + def test_rewrite_turn_that_ran_a_check_is_never_blocked(self) -> None: + verdict = self.detect.evaluate( + self.payload(REAL_TAG_1, tools=True, stop_hook_active=True)) + self.assertEqual(verdict["block"], "") + + def test_tag_plus_say_offer_is_blocked_even_after_a_read(self) -> None: + reply = ( + f"{REAL_TAG_1}\n\nIf you want me to settle it, say 'test it' and I will run the probe.") + verdict = self.detect.evaluate(self.payload(reply, tools=True)) + self.assertIn("hands the check back", verdict["block"]) + self.assertIn("run the check now", verdict["block"].lower()) + + def test_tag_plus_tell_me_to_test_offer_is_blocked(self) -> None: + reply = f"{REAL_TAG_1}\n\nTell me to test it and I will." + verdict = self.detect.evaluate(self.payload(reply, tools=True)) + self.assertIn("hands the check back", verdict["block"]) + + def test_say_offer_block_is_not_released_by_stop_hook_active(self) -> None: + reply = f"{REAL_TAG_1}\n\nSay 'go' and I will run it." + verdict = self.detect.evaluate( + self.payload(reply, tools=True, stop_hook_active=True)) + self.assertIn("hands the check back", verdict["block"]) + + def test_tag_plus_say_offer_for_a_write_is_allowed(self) -> None: + reply = f"{REAL_TAG_1}\n\nSay 'push it' and I will push the branch to origin." + verdict = self.detect.evaluate(self.payload(reply, tools=True)) + self.assertEqual(verdict["block"], "") + + def test_say_offer_without_a_tag_is_allowed(self) -> None: + verdict = self.detect.evaluate( + self.payload("Ran the probe: 3 passed. Say 'next' for the follow-up.", tools=True)) + self.assertEqual(verdict["block"], "") + + def test_tag_plus_say_offer_is_blocked_even_after_a_read(self) -> None: + reply = ( + f"{REAL_TAG_1}\n\nIf you want me to settle it, say 'test it' and I will run the probe.") + verdict = self.detect.evaluate(self.payload(reply, tools=True)) + self.assertIn("hands the check back", verdict["block"]) + self.assertIn("run the check now", verdict["block"].lower()) + + def test_tag_plus_tell_me_to_test_offer_is_blocked(self) -> None: + reply = f"{REAL_TAG_1}\n\nTell me to test it and I will." + verdict = self.detect.evaluate(self.payload(reply, tools=True)) + self.assertIn("hands the check back", verdict["block"]) + + def test_say_offer_block_is_not_released_by_stop_hook_active(self) -> None: + reply = f"{REAL_TAG_1}\n\nSay 'go' and I will run it." + verdict = self.detect.evaluate( + self.payload(reply, tools=True, stop_hook_active=True)) + self.assertIn("hands the check back", verdict["block"]) + + def test_tag_plus_say_offer_for_a_write_is_allowed(self) -> None: + reply = f"{REAL_TAG_1}\n\nSay 'push it' and I will push the branch to origin." + verdict = self.detect.evaluate(self.payload(reply, tools=True)) + self.assertEqual(verdict["block"], "") + + def test_say_offer_without_a_tag_is_allowed(self) -> None: + verdict = self.detect.evaluate( + self.payload("Ran the probe: 3 passed. Say 'next' for the follow-up.", tools=True)) self.assertEqual(verdict["block"], "") def test_untagged_turn_with_no_tools_is_never_blocked(self) -> None: diff --git a/product/skills/ci-job-timing/SKILL.md b/product/skills/ci-job-timing/SKILL.md new file mode 100644 index 000000000..4f21a0dae --- /dev/null +++ b/product/skills/ci-job-timing/SKILL.md @@ -0,0 +1,54 @@ +--- +name: ci-job-timing +description: >- + Repeatable per-step timing report for any GitLab CI job, from Datadog. Use + when asked how long a CI job's steps take, where a slow job spends its time, + or to compare the same job across dev/prod or across different jobs + (for example "Build LAGVM (dev)", "xmm-image-packager (dev)"). +--- + +# ci-job-timing + +One recipe, any job. The job name and the number of days are the only inputs. + +## Why it reads log text + +Datadog CI Visibility has job-level events (`@ci.job.name`, `@duration` in +nanoseconds) but no step-level events. Step times come from `[CI_STEP]` log +lines written by the CI step wrapper. Lines from side-by-side tracks carry a +prefix (for example `[gvm]`, `[qssi]`) and Datadog does not split them into +fields, so the script reads the message text ("took N seconds") instead. + +## Steps + +0. Context first: why does the job start late? What does it wait for, and which + prerequisite finished last? (`` is this skill's folder.) + `python3 /scripts/ci_job_timing.py context --job "" --pipelines 10 --job-ids ` + Get `` (comma-separated) from Datadog first: `search_datadog_ci_pipeline_events`, + `ci_level=job`, query `@ci.job.name:"" @ci.status:success`, `limit=10`; use the + `job_id` column. Scanning GitLab's pipeline list instead can miss the job on a busy project. + This reads GitLab (read-only) with the token in `GITLAB_TOKEN`; without it the + script prints an error and exits 2. Timing comes from GitLab, not Datadog, + because Datadog job records have no pipeline number. Needs absent from a + pipeline (optional needs) are skipped. Host and project default to + `https://gitlab.rivianvw.io` and `core/firmware` (`--gitlab-host`, `--project`). +1. Build the query and its link: + `python3 /scripts/ci_job_timing.py queries --job "" --days 3` +2. Run the printed steps query with the Datadog `ddsql_run_query` tool (from `now-d`). +3. Save the TSV it returns to a file and render the table: + `python3 /scripts/ci_job_timing.py report --job "" --days 3 --tsv ` +4. For side-by-side steps, pick one `job_id` from a raw `[CI_STEP]` line and run + `ci_job_timing.py timeline --job "" --job-id `. Read the start and end + times there; never add step averages into a job time. +5. Optional job-level cross-check: `aggregate_datadog_ci_pipeline_events` with + `ci_level=job`, query `@ci.job.name:""`, metric `@duration`. + +## Read the table correctly + +- `Runs` counts successful step lines only. A run that stopped early does not + count in its later steps, so counts differ between steps. +- Averages with few runs (n under 10) are weak. Say the n. +- Steps can overlap, so the column is not a share of the job. +- Zero rows means a wrong job name or an empty window: the script errors, it + never prints an empty table. +- A job whose steps do not log `[CI_STEP]` lines returns nothing; say that. diff --git a/product/skills/ci-job-timing/scripts/ci_job_timing.py b/product/skills/ci-job-timing/scripts/ci_job_timing.py new file mode 100644 index 000000000..c942c224d --- /dev/null +++ b/product/skills/ci-job-timing/scripts/ci_job_timing.py @@ -0,0 +1,344 @@ +#!/usr/bin/env python3 +"""Build the Datadog queries and the report for one CI job's step timing. + +The Datadog connector is only reachable from the agent, so this script does the +two parts that must be identical on every run: write the query text (`queries`, +`timeline`) and turn the query result into a table (`report`). `context` reads +GitLab (read-only) to show what a job waits for and which prerequisite finished last. +""" +from __future__ import annotations + +import argparse +import csv +import io +import json +import os +import re +import statistics +import sys +import urllib.error +import urllib.request +from datetime import datetime +from urllib.parse import quote + +DEFAULT_HOST = "rivianvw.datadoghq.com" +DEFAULT_GITLAB = "https://gitlab.rivianvw.io" +DEFAULT_PROJECT = "core/firmware" +TOKEN_ENV = "GITLAB_TOKEN" +_JOB_RE = re.compile(r"^[A-Za-z0-9 ()._:/\-]+$") + + +def check_job(job: str) -> str: + if not job or not _JOB_RE.match(job): + raise ValueError(f"job name has characters the query cannot carry safely: {job!r}") + return job + + +def check_days(days: int) -> int: + if not 1 <= days <= 30: + raise ValueError(f"days must be 1-30, got {days}") + return days + + +def _seconds_expr() -> str: + return "CAST(SPLIT_PART(SPLIT_PART(message, 'took ', 2), ' seconds', 1) AS DECIMAL)" + + +def steps_sql(job: str, days: int) -> str: + """One row per step: runs, average, min, max seconds. Reads message text, so + lines prefixed [gvm]/[qssi] (which Datadog does not split into fields) count too.""" + check_job(job) + check_days(days) + flt = f'"[CI_STEP]" STEP_SUCCESS "[job={job}]"' + sec = _seconds_expr() + return ( + "SELECT SPLIT_PART(SPLIT_PART(message, 'Step: <', 2), '>', 1) AS step, " + f"COUNT(*) AS runs, AVG({sec}) AS avg_s, MIN({sec}) AS min_s, MAX({sec}) AS max_s " + f"FROM dd.logs(columns => ARRAY['message'], filter => '{flt}', " + f"from_timestamp => NOW() - INTERVAL '{days} day', to_timestamp => NOW()) " + "AS (message VARCHAR) GROUP BY SPLIT_PART(SPLIT_PART(message, 'Step: <', 2), '>', 1) " + "ORDER BY avg_s DESC LIMIT 60" + ) + + +def timeline_sql(job: str, job_id: int, days: int) -> str: + """Every step line of one run, oldest first, so side-by-side steps are visible.""" + check_job(job) + check_days(days) + flt = f'"[CI_STEP]" "[job_id={int(job_id)}]" "[job={job}]"' + return ( + "SELECT timestamp, message " + f"FROM dd.logs(columns => ARRAY['timestamp', 'message'], filter => '{flt}', " + f"from_timestamp => NOW() - INTERVAL '{days} day', to_timestamp => NOW()) " + "AS (timestamp TIMESTAMP, message VARCHAR) ORDER BY timestamp LIMIT 200" + ) + + +def link(sql: str, host: str = DEFAULT_HOST) -> str: + return f"https://{host}/ddsql/editor?q={quote(sql, safe='')}" + + +def parse_tsv(text: str) -> list[dict]: + """Parse the TSV block ddsql_run_query returns. Missing header or no rows -> error.""" + body = text + m = re.search(r"(.*?)", text, re.S) + if m: + body = m.group(1) + body = body.strip("\n") + if not body.strip(): + raise ValueError("no rows: the query returned nothing (wrong job name, or no runs in the window)") + rows = list(csv.DictReader(io.StringIO(body), delimiter="\t")) + need = {"step", "runs", "avg_s", "min_s", "max_s"} + if not rows or not need <= set(rows[0]): + raise ValueError(f"unexpected columns, need {sorted(need)}; got {list(rows[0]) if rows else 'none'}") + return rows + + +def report(job: str, days: int, rows: list[dict], url: str) -> str: + out = [ + f"## {job}: step times, last {days} day(s)", + "", + "| Step | Runs | Avg (s) | Avg (min) | Fastest (s) | Slowest (s) |", + "|---|---|---|---|---|---|", + ] + for r in rows: + avg = float(r["avg_s"]) + out.append( + f"| {r['step']} | {int(float(r['runs']))} | {avg:.0f} | {avg / 60:.1f} " + f"| {float(r['min_s']):.0f} | {float(r['max_s']):.0f} |" + ) + out += [ + "", + "Steps can run side by side, so do not add these up to a job time.", + f"Query: {url}", + ] + return "\n".join(out) + +NEEDS_QUERY = ( + "query($path: ID!, $iid: ID!) { project(fullPath: $path) { pipeline(iid: $iid) { " + "stages { nodes { groups { nodes { jobs { nodes { name needs { nodes { name } } } } } } } } } } }" +) + + +def parse_needs(graphql_json: dict, job: str) -> list[str]: + """Names the job needs, from the stages/groups/jobs GraphQL shape. Job absent -> error.""" + if graphql_json.get("errors"): + raise ValueError(f"GitLab GraphQL returned errors: {graphql_json['errors']}") + pipeline = ((graphql_json.get("data") or {}).get("project") or {}).get("pipeline") + if not pipeline: + raise ValueError("GraphQL reply has no project.pipeline (wrong project or pipeline number)") + for stage in (pipeline.get("stages") or {}).get("nodes") or []: + for group in (stage.get("groups") or {}).get("nodes") or []: + for j in (group.get("jobs") or {}).get("nodes") or []: + if j.get("name") == job: + names = [n["name"] for n in (j.get("needs") or {}).get("nodes") or []] + return list(dict.fromkeys(names)) + raise ValueError(f"job {job!r} not found in the pipeline's stages") + + +def _ts(value: str | None) -> datetime | None: + if not value: + return None + return datetime.fromisoformat(value.replace("Z", "+00:00")) + + +def _latest(jobs: list[dict], name: str, key: str) -> dict | None: + """Latest attempt of one job name by `key` (retries share a name).""" + best, best_t = None, None + for j in jobs: + t = _ts(j.get(key)) if j.get("name") == name else None + if t is not None and (best_t is None or t > best_t): + best, best_t = j, t + return best + + +def summarize(job: str, needs: list[str], jobs_by_pipeline: dict) -> dict: + """Per pipeline: which needed job finished last and how long the job then waited. + + A needed job that is absent or unfinished in a pipeline is skipped (optional needs). + A pipeline where the job never started, or no need finished, yields no row. + """ + rows, last_counts, skipped = [], {}, {} + for pid, jobs in jobs_by_pipeline.items(): + me = _latest(jobs, job, "started_at") + if me is None: + continue + finishers = [] + for need in needs: + n = _latest(jobs, need, "finished_at") + if n is None: + skipped[need] = skipped.get(need, 0) + 1 + else: + finishers.append((_ts(n["finished_at"]), need)) + if not finishers: + continue + fin_t, fin_name = max(finishers) + gap = (_ts(me["started_at"]) - fin_t).total_seconds() + rows.append({ + "pipeline": pid, + "last_finisher": fin_name, + "gap_s": gap, + "queued_s": me.get("queued_duration"), + }) + last_counts[fin_name] = last_counts.get(fin_name, 0) + 1 + gaps = [r["gap_s"] for r in rows] + return { + "job": job, + "needs": list(needs), + "rows": rows, + "last_counts": dict(sorted(last_counts.items(), key=lambda kv: (-kv[1], kv[0]))), + "skipped": skipped, + "median_gap_s": statistics.median(gaps) if gaps else None, + "max_gap_s": max(gaps) if gaps else None, + } + + +def render_context(summary: dict, base_url: str = DEFAULT_GITLAB, project: str = DEFAULT_PROJECT) -> str: + if not summary["rows"]: + raise ValueError("no pipeline had both the job started and a needed job finished") + out = [f"## {summary['job']}: what it waits for", ""] + out.append("Needs: " + (", ".join(f"`{n}`" for n in summary["needs"]) or "none listed")) + n = len(summary["rows"]) + out += ["", f"Last needed job to finish, over {n} pipeline(s):", "", + "| Needed job | Finished last | Share |", "|---|---|---|"] + for name, cnt in summary["last_counts"].items(): + out.append(f"| {name} | {cnt} | {cnt / n:.0%} |") + out += ["", "Gap = last needed job finished -> this job started.", + f"- Median gap: {summary['median_gap_s']:.0f} s", + f"- Maximum gap: {summary['max_gap_s']:.0f} s", "", + "| Pipeline | Last finisher | Gap (s) | Queued (s) |", "|---|---|---|---|"] + for r in summary["rows"]: + url = f"{base_url}/{project}/-/pipelines/{r['pipeline']}" + q = "" if r["queued_s"] is None else f"{float(r['queued_s']):.0f}" + out.append(f"| [{r['pipeline']}]({url}) | {r['last_finisher']} | {r['gap_s']:.0f} | {q} |") + out += ["", "Needs listed but absent (or unfinished) in a pipeline are skipped; they may be optional needs."] + if summary["skipped"]: + out.append("Skipped: " + ", ".join(f"{k} ({v}x)" for k, v in sorted(summary["skipped"].items()))) + return "\n".join(out) + + + + +def http_get_json(url: str, token: str, body: dict | None = None): + """GET (or POST for GraphQL, which is a read-only query). Errors never echo the token.""" + headers = {"PRIVATE-TOKEN": token, "Content-Type": "application/json"} + data = json.dumps(body).encode() if body is not None else None + req = urllib.request.Request(url, data=data, headers=headers) + try: + with urllib.request.urlopen(req, timeout=60) as resp: + return json.load(resp) + except urllib.error.HTTPError as e: + raise OSError(f"GitLab request failed: HTTP {e.code} for {url}") from None + except urllib.error.URLError as e: + raise OSError(f"GitLab request failed: {e.reason} for {url}") from None + + +def _paged(url: str, token: str, get=http_get_json): + page = 1 + while True: + batch = get(f"{url}{'&' if '?' in url else '?'}per_page=100&page={page}", token) + yield from batch + if len(batch) < 100: + return + page += 1 + + +def _pipelines_from_job_ids(api, job, job_ids, token, get): + """Resolve known job numbers to their pipelines. A job number whose name differs is an error.""" + found = {} + for jid in job_ids: + j = get(f"{api}/jobs/{int(jid)}", token) + if j.get("name") != job: + raise ValueError(f"job {int(jid)} is named {j.get('name')!r}, not {job!r}") + pl = j["pipeline"] + found[pl["id"]] = pl["iid"] + return found + + +def fetch_context_inputs(host, project, job, n, token, scan_limit=200, get=http_get_json, job_ids=None): + """Return (needs, jobs_by_pipeline) for pipelines containing `job`. + + With job_ids, those jobs' pipelines are used directly. Without, the newest finished pipelines + are scanned, which can miss the job on a busy project.""" + api = f"{host}/api/v4/projects/{quote(project, safe='')}" + if job_ids: + pipes = _pipelines_from_job_ids(api, job, job_ids, token, get) + jobs_by_pipeline = {pid: list(_paged(f"{api}/pipelines/{pid}/jobs", token, get)) for pid in pipes} + newest_iid = max(pipes.values()) + reply = get(f"{host}/api/graphql", token, + {"query": NEEDS_QUERY, "variables": {"path": project, "iid": str(newest_iid)}}) + return parse_needs(reply, job), jobs_by_pipeline + jobs_by_pipeline, newest_iid, scanned = {}, None, 0 + for p in _paged(f"{api}/pipelines?scope=finished&order_by=id&sort=desc", token, get): + scanned += 1 + if scanned > scan_limit: + break + jobs = list(_paged(f"{api}/pipelines/{p['id']}/jobs", token, get)) + if not any(j.get("name") == job for j in jobs): + continue + jobs_by_pipeline[p["id"]] = jobs + if newest_iid is None: + newest_iid = p["iid"] + if len(jobs_by_pipeline) >= n: + break + if not jobs_by_pipeline: + raise ValueError(f"no job named {job!r} in the {scanned} newest finished pipelines; pass --job-ids with recent job numbers from Datadog") + reply = get(f"{host}/api/graphql", token, + {"query": NEEDS_QUERY, "variables": {"path": project, "iid": str(newest_iid)}}) + return parse_needs(reply, job), jobs_by_pipeline + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__) + sub = ap.add_subparsers(dest="cmd", required=True) + for name in ("queries", "report", "timeline"): + p = sub.add_parser(name) + p.add_argument("--job", required=True) + p.add_argument("--days", type=int, default=3) + p.add_argument("--host", default=DEFAULT_HOST) + if name == "timeline": + p.add_argument("--job-id", type=int, required=True) + if name == "report": + p.add_argument("--tsv", required=True, help="file holding the steps query result") + pc = sub.add_parser("context", help="what the job waits for and which need finished last (GitLab, read-only)") + pc.add_argument("--job", required=True) + pc.add_argument("--pipelines", type=int, default=10) + pc.add_argument("--gitlab-host", default=DEFAULT_GITLAB) + pc.add_argument("--project", default=DEFAULT_PROJECT) + pc.add_argument("--job-ids", default="", help="comma-separated recent job numbers for this job (from Datadog); preferred") + a = ap.parse_args(argv) + if a.cmd == "context": + token = os.environ.get(TOKEN_ENV) + if not token: + print(f"error: {TOKEN_ENV} is not set; export a GitLab read_api token", file=sys.stderr) + return 2 + try: + check_job(a.job) + if a.pipelines < 1: + raise ValueError("--pipelines must be at least 1") + host = a.gitlab_host.rstrip("/") + ids = [int(x) for x in a.job_ids.split(",") if x.strip()] + needs, by_pipeline = fetch_context_inputs(host, a.project, a.job, a.pipelines, token, job_ids=ids) + print(render_context(summarize(a.job, needs, by_pipeline), host, a.project)) + except (ValueError, OSError) as e: + print(f"error: {e}", file=sys.stderr) + return 2 + return 0 + try: + if a.cmd == "queries": + sql = steps_sql(a.job, a.days) + print(f"-- steps query\n{sql}\n-- link\n{link(sql, a.host)}") + elif a.cmd == "timeline": + sql = timeline_sql(a.job, a.job_id, a.days) + print(f"-- timeline query\n{sql}\n-- link\n{link(sql, a.host)}") + else: + rows = parse_tsv(open(a.tsv, encoding="utf-8").read()) + print(report(a.job, a.days, rows, link(steps_sql(a.job, a.days), a.host))) + except (ValueError, OSError) as e: + print(f"error: {e}", file=sys.stderr) + return 2 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/product/skills/ci-job-timing/tests/test_ci_job_timing.py b/product/skills/ci-job-timing/tests/test_ci_job_timing.py new file mode 100644 index 000000000..fac3e6aca --- /dev/null +++ b/product/skills/ci-job-timing/tests/test_ci_job_timing.py @@ -0,0 +1,171 @@ +from __future__ import annotations + +import contextlib +import io +import os +import sys +import unittest +from unittest import mock +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "scripts")) + +import ci_job_timing as c # noqa: E402 + +TSV = """2 + +step\truns\tavg_s\tmin_s\tmax_s +Build LAGVM System\t4\t1563.0\t1500\t1600 +Build LAGVM Kernel\t16\t295.0\t250\t446 +""" + + +class StepsSql(unittest.TestCase): + def test_job_and_days_are_in_the_query(self): + sql = c.steps_sql("Build LAGVM (dev)", 3) + self.assertIn('"[job=Build LAGVM (dev)]"', sql) + self.assertIn("INTERVAL '3 day'", sql) + + def test_same_template_works_for_another_job(self): + sql = c.steps_sql("xmm-image-packager (dev)", 7) + self.assertIn("[job=xmm-image-packager (dev)]", sql) + + def test_quote_in_job_name_is_refused(self): + with self.assertRaises(ValueError): + c.steps_sql("x' OR 1=1 --", 3) + + def test_days_out_of_range_is_refused(self): + with self.assertRaises(ValueError): + c.steps_sql("Build LAGVM (dev)", 0) + + +class Report(unittest.TestCase): + def test_parses_and_renders(self): + rows = c.parse_tsv(TSV) + out = c.report("Build LAGVM (dev)", 3, rows, "https://example/x") + self.assertIn("| Build LAGVM System | 4 | 1563 | 26.1 | 1500 | 1600 |", out) + + def test_empty_result_is_an_error_not_an_empty_table(self): + with self.assertRaises(ValueError): + c.parse_tsv("\n") + + def test_wrong_columns_is_an_error(self): + with self.assertRaises(ValueError): + c.parse_tsv("\nfoo\tbar\n1\t2\n") + + def test_timeline_uses_job_id(self): + self.assertIn("[job_id=253261575]", c.timeline_sql("Build LAGVM (dev)", 253261575, 3)) + + +def _gql(jobs): + return {"data": {"project": {"pipeline": {"stages": {"nodes": [ + {"groups": {"nodes": [{"jobs": {"nodes": [ + {"name": n, "needs": {"nodes": [{"name": x} for x in needs]}}]}} for n, needs in jobs]}}]}}}}} + + +def _j(name, start, end, queued=5): + return {"name": name, "status": "success", "started_at": start, "finished_at": end, "queued_duration": queued} + + +class Needs(unittest.TestCase): + def test_parse_needs_reads_stages_groups_jobs(self): + g = _gql([("a", []), ("pkg", ["a", "b", "a"])]) + self.assertEqual(c.parse_needs(g, "pkg"), ["a", "b"]) + + def test_missing_job_is_an_error(self): + with self.assertRaises(ValueError): + c.parse_needs(_gql([("a", [])]), "zzz") + + +class Summarize(unittest.TestCase): + P = { + 1: [_j("a", "2026-01-01T00:00:00Z", "2026-01-01T00:10:00Z"), + _j("b", "2026-01-01T00:00:00Z", "2026-01-01T00:20:00Z"), + _j("pkg", "2026-01-01T00:21:00Z", "2026-01-01T00:30:00Z", 7)], + 2: [_j("a", "2026-01-02T00:00:00Z", "2026-01-02T00:30:00Z"), + _j("b", "2026-01-02T00:00:00Z", "2026-01-02T00:20:00Z"), + _j("pkg", "2026-01-02T00:30:10Z", "2026-01-02T00:40:00Z")], + } + + def test_last_finisher_and_gap(self): + s = c.summarize("pkg", ["a", "b"], self.P) + self.assertEqual([r["last_finisher"] for r in s["rows"]], ["b", "a"]) + self.assertEqual([r["gap_s"] for r in s["rows"]], [60.0, 10.0]) + self.assertEqual(s["last_counts"], {"a": 1, "b": 1}) + self.assertEqual(s["median_gap_s"], 35.0) + self.assertEqual(s["max_gap_s"], 60.0) + + def test_absent_need_is_skipped_not_an_error(self): + s = c.summarize("pkg", ["a", "b", "optional"], self.P) + self.assertEqual(s["skipped"], {"optional": 2}) + self.assertEqual(len(s["rows"]), 2) + self.assertIn("optional needs", c.render_context(s)) + + +class ContextCommand(unittest.TestCase): + def _run(self, argv, env): + out, err = io.StringIO(), io.StringIO() + with mock.patch.dict(os.environ, env, clear=False), \ + contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = c.main(argv) + return rc, out.getvalue(), err.getvalue() + + def test_missing_token_exits_2_with_no_report(self): + env = dict(os.environ) + env.pop("GITLAB_TOKEN", None) + out, err = io.StringIO(), io.StringIO() + with mock.patch.dict(os.environ, env, clear=True), \ + contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = c.main(["context", "--job", "pkg", "--pipelines", "2"]) + self.assertEqual(rc, 2) + self.assertEqual(out.getvalue(), "") + self.assertIn("GITLAB_TOKEN", err.getvalue()) + + def test_job_ids_resolve_pipelines_without_scanning(self): + calls = [] + + def fake(url, token, body=None): + calls.append(url) + if url.endswith("/graphql"): + return _gql([("pkg", ["a", "b"])]) + if "/jobs/" in url and "/pipelines/" not in url: + jid = int(url.rsplit("/", 1)[1]) + return {"name": "pkg", "pipeline": {"id": {101: 1, 102: 2}[jid], "iid": {101: 11, 102: 12}[jid]}} + if "/pipelines/" in url: + return Summarize.P[int(url.split("/pipelines/")[1].split("/")[0])] + raise AssertionError(f"unexpected scan call: {url}") + + needs, by_p = c.fetch_context_inputs("https://h", "core/firmware", "pkg", 2, "tok", get=fake, job_ids=[101, 102]) + self.assertEqual(sorted(by_p), [1, 2]) + self.assertEqual(needs, ["a", "b"]) + self.assertFalse(any("order_by" in u for u in calls)) + + def test_job_id_with_a_different_name_is_an_error(self): + def fake(url, token, body=None): + return {"name": "other", "pipeline": {"id": 1, "iid": 11}} + + with self.assertRaises(ValueError): + c.fetch_context_inputs("https://h", "core/firmware", "pkg", 1, "tok", get=fake, job_ids=[101]) + + def test_token_value_never_appears_in_output(self): + secret = "glpat-SECRET-VALUE-123" + + def fake(url, token, body=None): + self.assertEqual(token, secret) + if url.endswith("/graphql"): + return _gql([("pkg", ["a", "b"])]) + if "/pipelines/" in url: + return Summarize.P[int(url.split("/pipelines/")[1].split("/")[0])] + return [{"id": 1, "iid": 11}, {"id": 2, "iid": 12}] + + needs, by_p = c.fetch_context_inputs("https://h", "core/firmware", "pkg", 2, secret, get=fake) + text = c.render_context(c.summarize("pkg", needs, by_p)) + self.assertNotIn(secret, text) + with mock.patch.object(c, "fetch_context_inputs", side_effect=OSError("boom")): + rc, out, err = self._run(["context", "--job", "pkg"], {"GITLAB_TOKEN": secret}) + self.assertEqual(rc, 2) + self.assertNotIn(secret, out + err) + + +if __name__ == "__main__": + unittest.main() diff --git a/product/skills/land-stack/SKILL.md b/product/skills/land-stack/SKILL.md index 938dcb440..4a55352ad 100644 --- a/product/skills/land-stack/SKILL.md +++ b/product/skills/land-stack/SKILL.md @@ -100,6 +100,21 @@ guard before any write (label, thread-resolve, queue, merge). Mergify reports `Merge queue is ready` until a `@mergifyio queue` comment (or the dashboard) actually queues the PR. + Queued PRs can still be rejected later, and the queue says why. Read its + own comment instead of guessing: + + ```sh + python3 scripts/verify_stack.py --repo --queue-feedback ... + ``` + + Exit 0 means each PR is merged or in the queue. Exit 1 names each rejected + PR with its kind (`timeout`, `conflict`, `checks-failed`, `other`), the + reason Mergify gave, the checks that failed or were still running, and the + next step. Repair from that, not from the PR's own CI: a `conflict` needs a + rebase, a `checks-failed` names the check to fix, and a `timeout` is not a + defect in the PR, so requeue it and, if it repeats, compare the queue's + `checks_timeout` with how long the named check normally runs. + 4. **Never batch merges without checking each result.** A merge command can look silent on both success and some failure paths; a silent-looking run is not proof of a merge. diff --git a/product/skills/land-stack/scripts/verify_stack.py b/product/skills/land-stack/scripts/verify_stack.py index c20cf48d8..5040ee2a4 100644 --- a/product/skills/land-stack/scripts/verify_stack.py +++ b/product/skills/land-stack/scripts/verify_stack.py @@ -23,6 +23,16 @@ Queue-state exit 0: every PR merged or queued. Exit 1: a PR is not queued. Exit 2: a PR's queue state could not be read. +After a queue write, `--queue-feedback` reads Mergify's own status comment on +each PR and says in-queue, rejected, merged, or unchecked. A rejection names +its kind (timeout, conflict, checks-failed, other), the reason Mergify gave, +the checks that were failing or still running, and what to do next. + + verify_stack.py --repo owner/name --queue-feedback 7006 7007 + verify_stack.py --feedback-json comments.json --queue-feedback 7006 + +Queue-feedback exit 0: every PR merged, in the queue, or waiting to enter it. Exit 1: a PR was +rejected. Exit 2: a PR's queue comment could not be read. Never uses `gh pr list --head ` -- discovering by branch name is the unsafe path the skill exists to prevent. """ @@ -30,6 +40,7 @@ import argparse import json +import re import subprocess import sys from typing import Callable @@ -151,6 +162,135 @@ def report_queue_state(keys: list[str], entries: dict) -> int: return 2 return 0 +STATUS_MARK = "# Merge Queue Status" +PAYLOAD_RE = re.compile(r"Mergify Payload -\*-\s*(\{.*\})\s*-\*- Mergify Payload End", re.S) +HTML_COMMENT_RE = re.compile(r"", re.S) +REASON_RE = re.compile(r"## Reason\s*\n(.*?)(?=\n## |\Z)", re.S) +FAILING_CHECK_RE = re.compile(r"^- \u274c \[`([^`]+)`\]", re.M) +STILL_RUNNING_RE = re.compile(r"These checks were still running:\s*\n((?:- `[^`]+`\s*\n?)+)") + +FEEDBACK_ACTIONS = { + "timeout": "A check was still running when the queue's checks_timeout ran out. Requeue with `@mergifyio queue`; if it keeps timing out, the timeout in .mergify.yml is shorter than the check's normal run time.", + "conflict": "GitHub refused the merge. Rebase onto the trunk and push; Mergify requeues on the update.", + "checks-failed": "Read the failing check named here, fix the PR, and push; Mergify requeues on the update.", + "ahead-failed": "An earlier batch failed and this PR is being checked again. No action unless it is rejected next.", + "other": "Read the reason, then fix the PR or requeue with `@mergifyio queue`.", +} + + +def _unchecked(reason: str) -> dict: + return {"state": "unchecked", "kind": "", "reason": reason, "action": "", "failing_checks": [], + "still_running": [], "rule": "", "eta": ""} + + +def _feedback_kind(reason: str) -> str: + low = reason.lower() + if "ahead in the queue failed" in low: + return "ahead-failed" + if "merge conflicts" in low: + return "conflict" + if "checks timeout" in low: + return "timeout" + if "failing checks" in low: + return "checks-failed" + return "other" + + +def queue_feedback(comments: list[dict], head_at: str | None = None) -> dict: + status = [c for c in comments if STATUS_MARK in (c.get("body") or "")] + if not status: + return _unchecked(f"no {STATUS_MARK[2:]} comment from Mergify") + latest = max(status, key=lambda c: c.get("id", 0)) + body = latest.get("body") or "" + found = PAYLOAD_RE.search(body) + if not found: + return _unchecked("Merge Queue Status comment has no readable Mergify payload") + try: + payload = json.loads(found.group(1)) + except json.JSONDecodeError as exc: + return _unchecked(f"Mergify payload is not valid JSON: {exc}") + state = payload.get("state") + text = HTML_COMMENT_RE.sub("", body) + reason_match = REASON_RE.search(text) + section = reason_match.group(1).strip() if reason_match else "" + paragraphs = [para.strip() for para in section.split("\n\n") if para.strip()] + informative = [para for para in paragraphs if not para.endswith("has been dequeued")] + reason = (informative or paragraphs or [""])[0] + running = STILL_RUNNING_RE.search(text) + still_running = re.findall(r"`([^`]+)`", running.group(1)) if running else [] + result = { + "kind": _feedback_kind(section) if section else "", + "reason": reason, + "failing_checks": FAILING_CHECK_RE.findall(text), + "still_running": still_running, + "rule": payload.get("queue_rule_name") or "", + "eta": payload.get("estimated_time_of_merge") or "", + } + if state == "dequeued": + if head_at and (latest.get("updated_at") or "") and head_at > latest["updated_at"]: + result.update(state="waiting", kind="", reason="the head commit is newer than the queue's last status; Mergify will check the new head", action="") + return result + kind = result["kind"] or "other" + result.update(state="rejected", kind=kind, action=FEEDBACK_ACTIONS[kind]) + return result + if state == "checking": + result.update(state="in-queue", action="") + return result + if state == "waiting": + result.update(state="waiting", reason="waiting for queue conditions (for example the new head's checks)", action="") + return result + return _unchecked(f"unrecognised Mergify queue state: {state!r}") + + +def fetch_queue_comments(repo: str | None, number: int) -> dict: + prefix = f"repos/{repo}" if repo else "repos/{owner}/{repo}" + pr = json.loads(subprocess.run(["gh", "api", f"{prefix}/pulls/{number}"], + capture_output=True, text=True, check=True).stdout) + pages = subprocess.run( + ["gh", "api", f"{prefix}/issues/{number}/comments", "--paginate", "--jq", + '[.[]|select(.user.login=="mergify[bot]")|{id,created_at,updated_at,body}]'], + capture_output=True, text=True, check=True).stdout + comments = [row for line in pages.splitlines() if line.strip() for row in json.loads(line)] + head = subprocess.run(["gh", "api", f"{prefix}/commits/{pr['head']['sha']}", "--jq", ".commit.committer.date"], + capture_output=True, text=True, check=True).stdout.strip() + return {"merged": bool(pr.get("merged")), "comments": comments, "head_at": head} + + +def report_queue_feedback(keys: list[str], entries: dict) -> int: + worst = 0 + for key in keys: + entry = entries.get(key) + if entry is None: + fb, merged = _unchecked("no payload for this PR"), False + else: + merged = bool(entry.get("merged")) if isinstance(entry, dict) else False + comments = entry.get("comments", []) if isinstance(entry, dict) else entry + head_at = entry.get("head_at") if isinstance(entry, dict) else None + fb = queue_feedback(comments, head_at=head_at) + if merged: + print(f"{'merged':<10} #{key}") + continue + if fb["state"] == "in-queue": + extra = f" (rule {fb['rule']}, ETA {fb['eta']})" if fb["rule"] else "" + note = f" -- {fb['reason']}" if fb["kind"] == "ahead-failed" else "" + print(f"{'in-queue':<10} #{key}{extra}{note}") + elif fb["state"] == "waiting": + print(f"{'waiting':<10} #{key} {fb['reason']}") + elif fb["state"] == "rejected": + print(f"{'rejected':<10} #{key} {fb['kind']}: {fb['reason']}") + if fb["failing_checks"]: + print(f"{'':<10} failing checks: {', '.join(fb['failing_checks'])}") + if fb["still_running"]: + print(f"{'':<10} still running at the timeout: {', '.join(fb['still_running'])}") + print(f"{'':<10} next: {fb['action']}") + worst = max(worst, 1) + else: + print(f"{'unchecked':<10} #{key} {fb['reason']}") + if worst == 0: + worst = 2 + return worst + + def main(argv: list[str] | None = None) -> int: ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) ap.add_argument("numbers", nargs="*", help="PR numbers, bottom of stack first") @@ -163,8 +303,26 @@ def main(argv: list[str] | None = None) -> int: ap.add_argument("--json", action="store_true", help="machine-readable output") ap.add_argument("--queue-state", action="store_true", help="after a queue write: is each PR merged or queued?") ap.add_argument("--queue-json", help="replay saved pull + check-run payloads keyed by PR for --queue-state") + ap.add_argument("--queue-feedback", action="store_true", help="read Mergify's status comment: in-queue, rejected (with reason), merged") + ap.add_argument("--feedback-json", help="replay saved Mergify comments keyed by PR for --queue-feedback") args = ap.parse_args(argv) + if args.queue_feedback: + keys = args.numbers + if not keys: + ap.error("give PR numbers for --queue-feedback") + if args.feedback_json: + with open(args.feedback_json, encoding="utf-8") as fh: + entries = json.load(fh) + else: + entries = {} + for key in keys: + try: + entries[key] = fetch_queue_comments(args.repo, int(key)) + except (subprocess.CalledProcessError, ValueError, KeyError, json.JSONDecodeError) as exc: + print(f"unchecked #{key} could not read queue comments: {exc}", file=sys.stderr) + return report_queue_feedback(keys, entries) + if args.queue_state: keys = args.numbers if not keys: diff --git a/product/skills/land-stack/tests/fixtures/mergify_queue_comments_2026-10-12.json b/product/skills/land-stack/tests/fixtures/mergify_queue_comments_2026-10-12.json new file mode 100644 index 000000000..f32680498 --- /dev/null +++ b/product/skills/land-stack/tests/fixtures/mergify_queue_comments_2026-10-12.json @@ -0,0 +1,106 @@ +{ + "catstack#1333": [ + { + "body": "Queued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1333#issuecomment-6126434423).", + "created_at": "2026-10-11T19:10:58Z", + "id": 6112652141, + "updated_at": "2026-10-12T14:36:29Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 14:36 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1333#issuecomment-6126433073)\n- \u274c **Checks timed out** \u00b7 on draft #1341\n- \ud83d\udeab **Left the queue** \u2014 `2026-10-12 15:37 UTC` \u00b7 at `9787af8f07826c77ba0072030a241cf253e6a08c`\n\nThis pull request spent **1 hour 1 minute 3 seconds** in the queue, including **1 hour 31 seconds** running CI.\n\n## Reason\n\nThe merge conditions cannot be satisfied: the checks did not pass within the checks timeout of 1 hour\n\nThese checks were still running:\n- `test`\n\nFailing checks:\n- \ud83d\udfe0 [`test`](https://github.com/EdbertChan/catstack/actions/runs/38264185548/job/114862046006) ([job log](https://github.com/EdbertChan/catstack/actions/runs/38264185548/job/114862046006))\n\n## Hint\n\nYou may have to fix your CI before adding the pull request to the queue again.\n\nIf you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.\nIf you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a `@mergifyio queue` comment.\n\n
\nWaiting for\n\n- [ ] `check-success = test`\n\n
\n
\nAll conditions\n\n- [ ] `check-success = test`\n- [X] `check-success = lint`\n- [X] `check-success = validate`\n\n
\n\nRequeued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1333#issuecomment-6127751086).", + "created_at": "2026-10-12T14:36:28Z", + "id": 6126434423, + "updated_at": "2026-10-12T15:52:17Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 15:52 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1333#issuecomment-6127749387)\n- \u274c **Checks timed out** \u2014 `2026-10-12 18:22 UTC` \u00b7 on draft #1351\n- \u2702\ufe0f Bisecting to identify the failing PR (round 1/1)\n- \ud83d\udfe0 **Checks running** \u00b7 on draft #1353 \u00b7 [dashboard](https://dashboard.mergify.com/orgs/EdbertChan/repos/catstack/queues/status?branch=main&pull=1333)\n- \u23f3 Merge \u00b7 ETA: `2026-10-12 20:23 UTC` \ud83d\ude80\n\n
\nWaiting for\n\n- [ ] `check-success = test`\n\n
\n
\nAll merge conditions\n\n- [ ] `check-success = test`\n- [X] `check-success = lint`\n- [X] `check-success = validate`\n\n
\n\n
\nRequired conditions to stay in the queue\n\n- [X] `-closed` [\ud83d\udccc queue requirement]\n- [X] `-conflict` [\ud83d\udccc queue requirement]\n- [X] `-draft` [\ud83d\udccc queue requirement]\n- [X] any of [\ud83d\udccc queue -> configuration change requirements]:\n - [X] `-mergify-configuration-changed`\n - [ ] `check-success = @mergify/Configuration changed`\n - [ ] `check-success = @mergify/Configuration has been deleted`\n- [X] any of [\ud83d\udd00 queue conditions]:\n - [X] all of [\ud83d\udccc queue conditions of queue rule `admin-bypass`]:\n - [X] `-draft`\n - [X] `base=main`\n - [X] `label=admin-bypass`\n\n
", + "created_at": "2026-10-12T15:52:16Z", + "id": 6127751086, + "updated_at": "2026-10-12T19:24:32Z" + } + ], + "catstack#1332": [ + { + "body": "Queued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1332#issuecomment-6112771226).", + "created_at": "2026-10-11T19:10:53Z", + "id": 6112651269, + "updated_at": "2026-10-11T19:22:07Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-11 19:22 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1332#issuecomment-6112764720)\n- \u274c **Checks timed out** \u00b7 on draft #1339\n- \ud83d\udeab **Left the queue** \u2014 `2026-10-11 20:23 UTC` \u00b7 at `0e6a50c53c329843afa3886cf73cbe244fc3dfa3`\n\nThis pull request spent **1 hour 1 minute 19 seconds** in the queue, including **1 hour 20 seconds** running CI.\n\n## Reason\n\nThe merge conditions cannot be satisfied: the checks did not pass within the checks timeout of 1 hour\n\nThese checks were still running:\n- `test`\n\nFailing checks:\n- \ud83d\udfe0 [`test`](https://github.com/EdbertChan/catstack/actions/runs/38167777445/job/114553826149) ([job log](https://github.com/EdbertChan/catstack/actions/runs/38167777445/job/114553826149))\n\n## Hint\n\nYou may have to fix your CI before adding the pull request to the queue again.\n\nIf you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.\nIf you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a `@mergifyio queue` comment.\n\n
\nWaiting for\n\n- [ ] `check-success = test`\n\n
\n
\nAll conditions\n\n- [ ] `check-success = test`\n- [X] `check-success = lint`\n- [X] `check-success = validate`\n\n
\n\nRequeued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1332#issuecomment-6126529630).", + "created_at": "2026-10-11T19:22:06Z", + "id": 6112771226, + "updated_at": "2026-10-12T14:42:12Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 14:42 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1332#issuecomment-6126526873)\n- \u274c **Checks timed out** \u2014 `2026-10-12 15:37 UTC` \u00b7 on draft #1345\n- \u2702\ufe0f Bisecting to identify the failing PR (round 1/1)\n- \u274c **Checks timed out** \u00b7 on draft #1348 \u00b7 identified as the failing PR\n- \ud83d\udeab **Left the queue** \u2014 `2026-10-12 17:39 UTC` \u00b7 at `0e6a50c53c329843afa3886cf73cbe244fc3dfa3`\n\nThis pull request spent **2 hours 57 minutes 4 seconds** in the queue, including **2 hours 54 seconds** running CI.\n\n## Reason\n\nThe merge conditions cannot be satisfied: the checks did not pass within the checks timeout of 1 hour\n\nThese checks were still running:\n- `test`\n\nFailing checks:\n- \ud83d\udfe0 [`test`](https://github.com/EdbertChan/catstack/actions/runs/38278827982/job/114910815884) ([job log](https://github.com/EdbertChan/catstack/actions/runs/38278827982/job/114910815884))\n\n## Hint\n\nYou may have to fix your CI before adding the pull request to the queue again.\n\nIf you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.\nIf you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a `@mergifyio queue` comment.\n\n
\nWaiting for\n\n- [ ] `check-success = test`\n\n
\n
\nAll conditions\n\n- [ ] `check-success = test`\n- [X] `check-success = lint`\n- [X] `check-success = validate`\n\n
\n\nRequeued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1332#issuecomment-6130960344).", + "created_at": "2026-10-12T14:42:11Z", + "id": 6126529630, + "updated_at": "2026-10-12T19:16:26Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 19:16 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1332#issuecomment-6130958496)\n- \ud83d\udfe0 **Checks running** \u00b7 on draft #1352\n- \ud83d\udd04 **Checks restarted** \u00b7 will be requeued automatically\n\n## Reason\n\nPull requests batch (#1333, #1336) which was ahead in the queue failed", + "created_at": "2026-10-12T19:16:25Z", + "id": 6130960344, + "updated_at": "2026-10-12T19:23:13Z" + } + ], + "catstack#1309": [ + { + "body": "Queued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1309#issuecomment-6126707738).", + "created_at": "2026-10-11T19:11:56Z", + "id": 6112662978, + "updated_at": "2026-10-12T14:52:33Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 14:52 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1309#issuecomment-6126705187)\n- \u274c **Checks timed out** \u2014 `2026-10-12 15:37 UTC` \u00b7 on draft #1345\n- \u2702\ufe0f Bisecting to identify the failing PR (round 1/1)\n- \u274c **Checks failed** \u00b7 on draft #1350 \u00b7 identified as the failing PR\n- \ud83d\udeab **Left the queue** \u2014 `2026-10-12 18:22 UTC` \u00b7 at `737c66df5221d80ff48c8308067840366155180e`\n\nThis pull request spent **3 hours 29 minutes 55 seconds** in the queue, including **1 hour 43 minutes** running CI.\n\n## Reason\n\nPull request #1309 has been dequeued\n\nGitHub refused to merge the pull request. Pull Request has merge conflicts. This is usually enforced by a [branch protection](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches) or [ruleset rule](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/about-rulesets).\n\n## Hint\n\nYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.\nIf you do update this pull request, it will automatically be requeued once the queue conditions match again.\nIf you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a `@mergifyio queue` comment.\n\n
\nRequired conditions to merge\n\n- [X] `check-success = lint`\n- [X] `check-success = test`\n- [X] `check-success = validate`\n\n
\n\nTick the box to put this pull request back in the merge queue (same as `@mergifyio queue`).\n\n- [ ] Requeue this pull request ", + "created_at": "2026-10-12T14:52:32Z", + "id": 6126707738, + "updated_at": "2026-10-12T18:22:27Z" + } + ], + "catstack#1354": [ + { + "body": "Queued \u2014 the merge queue status continues in [this comment \u2193](https://github.com/EdbertChan/catstack/pull/1354#issuecomment-6131697266).", + "created_at": "2026-10-12T20:00:48Z", + "id": 6131654915, + "updated_at": "2026-10-12T20:03:38Z" + }, + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 20:03 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1354#issuecomment-6131696378)\n- \ud83d\udfe0 **Preparing checks**\n- \u23f3 Merge \u00b7 ETA: `2026-10-12 21:23 UTC` \ud83d\ude80", + "created_at": "2026-10-12T20:03:38Z", + "id": 6131697266, + "updated_at": "2026-10-12T20:03:42Z" + } + ], + "Invoker#15193": [ + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 16:58 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by rule `autoqueue admin-bypass PRs to master`\n- \u274c **Checks failed** \u00b7 on draft #15210\n- \ud83d\udeab **Left the queue** \u2014 `2026-10-12 17:05 UTC` \u00b7 at `31fa323940264c601c16f192c8a22497f6684235`\n\nThis pull request spent **6 minutes 53 seconds** in the queue, including **2 minutes 42 seconds** running CI.\n\n## Reason\n\nThe merge conditions cannot be satisfied due to failing checks\n\n- `PR Body`\n\nFailing checks:\n- \u274c [`PR Body`](https://github.com/Neko-Catpital-Labs/Invoker/actions/runs/38281741556/job/114920522730) ([job log](https://github.com/Neko-Catpital-Labs/Invoker/actions/runs/38281741556/job/114920522730))\n\n## Hint\n\nYou may have to fix your CI before adding the pull request to the queue again.\nIf you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.\nIf you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a `@mergifyio queue` comment.\n\n
\nWaiting for\n\n- [ ] `check-success = PR Body`\n- [ ] `check-success = UI Vitest`\n- [ ] `check-success = required-fast / Guardrails`\n- [ ] `check-success = required-fast / Launch Dispatch Queue Repro`\n- [ ] `check-success = required-fast / Start Running MECE Repros`\n- [ ] `check-success = required-fast / Submit Workflow Chain`\n\n
\n
\nAll conditions\n\n- [ ] `check-success = PR Body`\n- [ ] `check-success = UI Vitest`\n- [ ] `check-success = required-fast / Guardrails`\n- [ ] `check-success = required-fast / Launch Dispatch Queue Repro`\n- [ ] `check-success = required-fast / Start Running MECE Repros`\n- [ ] `check-success = required-fast / Submit Workflow Chain`\n- `#review-threads-unresolved = 0`\n - [X] #15193\n- [X] `check-success = build-artifacts`\n- [X] `check-success = quality / Dependency Cruise`\n- [X] `check-success = quality / TypeScript Types`\n- [X] `check-success = required-fast / Branch Carry Forward`\n- [X] `check-success = required-fast / Merge Gate Concurrency Repro`\n\n
\n\nTick the box to put this pull request back in the merge queue (same as `@mergifyio queue`).\n\n- [ ] Requeue this pull request ", + "created_at": "2026-10-12T16:58:35Z", + "id": 6128803276, + "updated_at": "2026-10-12T17:05:21Z" + } + ], + "Invoker#15249": [ + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-12 20:06 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by rule `autoqueue admin-bypass PRs to master`\n- \ud83d\udfe0 **Checks running** \u00b7 on draft #15253 \u00b7 [dashboard](https://dashboard.mergify.com/orgs/Neko-Catpital-Labs/repos/Invoker/queues/status?branch=master&pull=15249)\n- \u23f3 Merge \u00b7 ETA: `2026-10-12 20:17 UTC` \ud83d\ude80\n\n
\nWaiting for\n\n- [ ] `check-success = PR Body`\n- [ ] `check-success = UI Vitest`\n\n
\n
\nAll merge conditions\n\n- [ ] `check-success = PR Body`\n- [ ] `check-success = UI Vitest`\n- `#review-threads-unresolved = 0`\n - [X] #15249\n- [X] `check-success = build-artifacts`\n- [X] `check-success = quality / Dependency Cruise`\n- [X] `check-success = quality / TypeScript Types`\n- [X] `check-success = required-fast / Branch Carry Forward`\n- [X] `check-success = required-fast / Guardrails`\n- [X] `check-success = required-fast / Launch Dispatch Queue Repro`\n- [X] `check-success = required-fast / Merge Gate Concurrency Repro`\n- [X] `check-success = required-fast / Start Running MECE Repros`\n- [X] `check-success = required-fast / Submit Workflow Chain`\n\n
\n\n
\nRequired conditions to stay in the queue\n\n- [X] `-closed` [\ud83d\udccc queue requirement]\n- [X] `-conflict` [\ud83d\udccc queue requirement]\n- [X] `-draft` [\ud83d\udccc queue requirement]\n- [X] any of [\ud83d\udccc queue -> configuration change requirements]:\n - [X] `-mergify-configuration-changed`\n - [ ] `check-success = @mergify/Configuration changed`\n - [ ] `check-success = @mergify/Configuration has been deleted`\n- [X] any of [\ud83d\udd00 queue conditions]:\n - [X] all of [\ud83d\udccc queue conditions of queue rule `admin-bypass`]:\n - [X] `#review-threads-unresolved = 0`\n - [X] `-draft`\n - [X] `-label=merge-hold`\n - [X] `base=master`\n - [X] `label=admin-bypass`\n - [ ] all of [\ud83d\udccc queue conditions of queue rule `default`]:\n - [ ] `#approved-reviews-by >= 1`\n - [X] `#review-threads-unresolved = 0`\n - [X] `-draft`\n - [X] `-label=merge-hold`\n - [X] `base=master`\n - [ ] all of [\ud83d\udccc queue conditions of queue rule `fast-lane`]:\n - [ ] `-label=admin-bypass`\n - [ ] `label=fast-lane`\n - [X] `#review-threads-unresolved = 0`\n - [X] `-draft`\n - [X] `-label=merge-hold`\n - [X] `base=master`\n\n
", + "created_at": "2026-10-12T20:06:17Z", + "id": 6131736089, + "updated_at": "2026-10-12T20:11:23Z" + }, + { + "body": "> `queue`\n\n#### \u2611\ufe0f This pull request is already queued\n\n\n\n", + "created_at": "2026-10-12T20:06:34Z", + "id": 6131740612, + "updated_at": "2026-10-12T20:06:34Z" + } + ], + "derived-waiting-from-catstack#1354": [ + { + "body": "\n\n# Merge Queue Status\n\n- \u2705 **Entered queue** \u2014 `2026-10-13 04:08 UTC` \u00b7 Rule: `admin-bypass` \u00b7 triggered by @EdbertChan with the [`@mergifyio queue` command](https://github.com/EdbertChan/catstack/pull/1354#issuecomment-6138020313)\n- \ud83d\udfe0 **Checks running** \u00b7 on draft #1376 \u00b7 [dashboard](https://dashboard.mergify.com/orgs/EdbertChan/repos/catstack/queues/status?branch=main&pull=1354)\n- \u23f3 Merge \u00b7 ETA: `2026-10-13 05:08 UTC` \ud83d\ude80\n\n
\nWaiting for\n\n- [ ] `check-success = lint`\n- [ ] `check-success = test`\n- [ ] `check-success = validate`\n\n
\n
\nAll merge conditions\n\n- [ ] `check-success = lint`\n- [ ] `check-success = test`\n- [ ] `check-success = validate`\n\n
\n\n
\nRequired conditions to stay in the queue\n\n- [X] `-closed` [\ud83d\udccc queue requirement]\n- [X] `-conflict` [\ud83d\udccc queue requirement]\n- [X] `-draft` [\ud83d\udccc queue requirement]\n- [X] any of [\ud83d\udccc queue -> configuration change requirements]:\n - [X] `-mergify-configuration-changed`\n - [ ] `check-success = @mergify/Configuration changed`\n - [ ] `check-success = @mergify/Configuration has been deleted`\n- [X] any of [\ud83d\udd00 queue conditions]:\n - [X] all of [\ud83d\udccc queue conditions of queue rule `admin-bypass`]:\n - [X] `-draft`\n - [X] `base=main`\n - [X] `label=admin-bypass`\n\n
", + "created_at": "2026-10-13T04:08:08Z", + "id": 6138021515, + "updated_at": "2026-10-13T04:08:32Z" + } + ] +} \ No newline at end of file diff --git a/product/skills/land-stack/tests/test_queue_feedback.py b/product/skills/land-stack/tests/test_queue_feedback.py new file mode 100644 index 000000000..3e9ab78a9 --- /dev/null +++ b/product/skills/land-stack/tests/test_queue_feedback.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python3 +"""Tests for verify_stack.py --queue-feedback against real Mergify comments. + +fixtures/mergify_queue_comments_2026-10-12.json holds every comment Mergify +posted on six real PRs, as `gh api issues//comments` returned them: +"1309" was dequeued for merge conflicts after a timed-out batch; "1333" and +"1332" were dequeued by timeouts and then requeued; "1354" is queued and +preparing checks; Invoker "15193" was dequeued because the PR Body check +failed; Invoker "15249" is running queue checks. + +"derived-waiting-from-catstack#1354" is the real "checking" comment from #1354 +with its payload state changed to "waiting", the value Mergify reported +("Waiting for queue conditions") on #1354 after a head update. Mergify edits +that comment in place, so the original could not be saved. +""" +from __future__ import annotations + +import json +import os +import subprocess +import sys +import unittest + +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.join(os.path.dirname(HERE), "scripts")) +import verify_stack as vs # noqa: E402 + +FIXTURE = os.path.join(HERE, "fixtures", "mergify_queue_comments_2026-10-12.json") +SCRIPT = os.path.join(os.path.dirname(HERE), "scripts", "verify_stack.py") + + +def load(): + with open(FIXTURE, encoding="utf-8") as fh: + return json.load(fh) + + +def status_comments(rows): + return [r for r in rows if "# Merge Queue Status" in r["body"]] + + +class TestQueueFeedback(unittest.TestCase): + def test_conflict_dequeue_is_rejected_with_a_rebase_action(self): + fb = vs.queue_feedback(load()["catstack#1309"]) + self.assertEqual(fb["state"], "rejected") + self.assertEqual(fb["kind"], "conflict") + self.assertIn("merge conflicts", fb["reason"]) + self.assertIn("rebase", fb["action"].lower()) + + def test_timeout_dequeue_is_rejected_as_timeout(self): + only_timeout = [status_comments(load()["catstack#1333"])[0]] + fb = vs.queue_feedback(only_timeout) + self.assertEqual(fb["state"], "rejected") + self.assertEqual(fb["kind"], "timeout") + self.assertIn("checks timeout", fb["reason"]) + self.assertIn("checks_timeout", fb["action"]) + + def test_batch_ahead_failure_is_a_recheck_not_a_rejection(self): + rows = [r for r in status_comments(load()["catstack#1332"]) + if "which was ahead in the queue failed" in r["body"]] + self.assertTrue(rows, "fixture lost the batch-ahead comment") + fb = vs.queue_feedback(rows[:1]) + self.assertEqual(fb["state"], "in-queue") + self.assertEqual(fb["kind"], "ahead-failed") + + def test_timeout_names_the_check_that_was_still_running(self): + only_timeout = [status_comments(load()["catstack#1333"])[0]] + self.assertEqual(vs.queue_feedback(only_timeout)["still_running"], ["test"]) + + def test_failed_check_names_the_check(self): + fb = vs.queue_feedback(load()["Invoker#15193"]) + self.assertEqual(fb["state"], "rejected") + self.assertEqual(fb["kind"], "checks-failed") + self.assertIn("PR Body", fb["failing_checks"]) + + def test_requeued_pr_reports_the_latest_comment_not_the_old_dequeue(self): + rows = load()["catstack#1333"] + self.assertTrue(any('"state": "dequeued"' in r["body"] for r in rows)) + fb = vs.queue_feedback(rows) + self.assertEqual(fb["state"], "in-queue") + + def test_queued_pr_is_in_queue_with_rule(self): + fb = vs.queue_feedback(load()["catstack#1354"]) + self.assertEqual(fb["state"], "in-queue") + self.assertEqual(fb["rule"], "admin-bypass") + + def test_waiting_for_conditions_is_waiting_not_unchecked(self): + fb = vs.queue_feedback(load()["derived-waiting-from-catstack#1354"]) + self.assertEqual(fb["state"], "waiting") + self.assertIn("conditions", fb["reason"]) + + def test_rejection_older_than_the_head_commit_is_waiting(self): + rows = load()["catstack#1309"] + stale = vs.queue_feedback(rows, head_at="2026-10-12T19:00:00Z") + self.assertEqual(stale["state"], "waiting") + self.assertIn("head", stale["reason"]) + + def test_rejection_newer_than_the_head_commit_stays_rejected(self): + rows = load()["catstack#1309"] + fb = vs.queue_feedback(rows, head_at="2026-10-12T14:00:00Z") + self.assertEqual(fb["state"], "rejected") + + def test_no_status_comment_is_unchecked(self): + fb = vs.queue_feedback([]) + self.assertEqual(fb["state"], "unchecked") + self.assertIn("no Merge Queue Status comment", fb["reason"]) + + def test_unreadable_payload_is_unchecked_not_in_queue(self): + broken = [{"id": 1, "body": "# Merge Queue Status\n\n- something new Mergify added\n"}] + fb = vs.queue_feedback(broken) + self.assertEqual(fb["state"], "unchecked") + + def test_cli_exit_codes_replay_json(self): + data = load() + path = os.path.join(os.environ.get("TMPDIR", "/tmp"), "queue_feedback_cli_test.json") + with open(path, "w", encoding="utf-8") as fh: + json.dump({"1309": data["catstack#1309"], "1354": data["catstack#1354"]}, fh) + try: + rejected = subprocess.run([sys.executable, SCRIPT, "--queue-feedback", "--feedback-json", path, "1309"], + capture_output=True, text=True) + ok = subprocess.run([sys.executable, SCRIPT, "--queue-feedback", "--feedback-json", path, "1354"], + capture_output=True, text=True) + finally: + os.unlink(path) + self.assertEqual(rejected.returncode, 1, rejected.stdout + rejected.stderr) + self.assertIn("rejected", rejected.stdout) + self.assertIn("#1309", rejected.stdout) + self.assertEqual(ok.returncode, 0, ok.stdout + ok.stderr) + self.assertIn("in-queue", ok.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/git-hooks/pre-push b/scripts/git-hooks/pre-push index 61b583dae..34516a867 100755 --- a/scripts/git-hooks/pre-push +++ b/scripts/git-hooks/pre-push @@ -157,7 +157,7 @@ while read -r local_ref local_sha remote_ref remote_sha; do echo "pre-push: UNCHECKED: git diff $diff_from $local_sha failed for $local_ref; $bypass" >&2 exit 1 fi - mapfile -d '' files <"$names" + files=(); while IFS= read -r -d '' _path; do files+=("$_path"); done <"$names" if [[ ${#files[@]} -eq 0 ]]; then continue fi @@ -173,7 +173,7 @@ while read -r local_ref local_sha remote_ref remote_sha; do if old_diff_from=$(git merge-base "$base_ref" "$remote_sha" 2>/dev/null); then old_names=$(mktemp) if git diff --name-only -z "$old_diff_from" "$remote_sha" >"$old_names"; then - mapfile -d '' old_files <"$old_names" + old_files=(); while IFS= read -r -d '' _path; do old_files+=("$_path"); done <"$old_names" if [[ ${#old_files[@]} -eq 0 ]]; then old_rc=0 else @@ -182,7 +182,7 @@ while read -r local_ref local_sha remote_ref remote_sha; do if [[ $old_rc -eq 1 ]]; then delta_names=$(mktemp) if git diff --name-only -z "$remote_sha" "$local_sha" >"$delta_names"; then - mapfile -d '' delta_files <"$delta_names" + delta_files=(); while IFS= read -r -d '' _path; do delta_files+=("$_path"); done <"$delta_names" if [[ ${#delta_files[@]} -eq 0 ]]; then continue fi diff --git a/skills.toml b/skills.toml index cb4f07c71..49e34029f 100644 --- a/skills.toml +++ b/skills.toml @@ -371,3 +371,9 @@ bucket = "product" harnesses = "all" invocation = "model" test_shape = "prose-fixtures" + +[skills.ci-job-timing] +bucket = "product" +harnesses = "all" +invocation = "model" +test_shape = "code-tests" diff --git a/tests/test_cat_mode.py b/tests/test_cat_mode.py index c5c065d30..324f79ce3 100644 --- a/tests/test_cat_mode.py +++ b/tests/test_cat_mode.py @@ -377,7 +377,7 @@ def test_skill_names_thrash_checkpoint_defaults(self): self.assertIn("Claim a chart gap fixed only after naming write vs enrich ownership", text) self.assertIn("Write a thrash checkpoint before attaching new scope", text) self.assertIn( - "When a same-type miss forces a working-style catch mid analytics thrash, stop feature PRs until that catch is locked", + "Stop analytics feature iteration and lock the class catch before more analytics PRs when automate is mandated mid thrash", text, ) self.assertIn("Do not race publication against a live long job in the same thread", text) @@ -397,10 +397,10 @@ def test_verify_reference_carries_analytics_thrash_checkpoints(self): self.assertIn("Write a thrash checkpoint before attaching new scope", text) self.assertIn("intervention-must-automate", text) self.assertIn( - "When a same-type miss forces a working-style catch mid analytics thrash, stop feature PRs until that catch is locked", + "Stop analytics feature iteration and lock the class catch before more", text, ) - self.assertIn("same-type miss forces a working-style catch", text) + self.assertIn("when automate is mandated mid thrash", text) self.assertIn( "Million-row or multi-host backfills are owned by a durable runner", text, @@ -489,6 +489,7 @@ def test_every_referenced_skill_still_exists(self): "ls", "owner-serve", "strace", + "intervention-must-automate", # Absolute-negative words in backticks, not skill names. "only", "never", @@ -2262,5 +2263,79 @@ def test_each_reference_rule_names_prior_art_or_says_none(self): ) +class TestCatModeCheaperSourceAndMeasuredAnswers(unittest.TestCase): + """Rules: look for a cheaper source before grinding; answer duration and + window questions from a measurement taken this turn; prove a new data path + on a small window first (folded into the analytics-tier rule).""" + + REF_DIR = os.path.join(REPO_ROOT, "corpus", "skills", "cat-mode", "references") + FIXTURES_DIR = os.path.join(REPO_ROOT, "corpus", "skills", "cat-mode", "tests") + + def _read(self, *parts): + with open(os.path.join(*parts), encoding="utf-8") as handle: + return handle.read() + + def test_skill_names_the_three_rules(self): + text = read_skill_text() + self.assertIn("Look for a cheaper source before grinding", text) + self.assertIn( + "Answer duration and window questions from a measurement taken this turn", + text, + ) + self.assertIn("Prove a new data path on a small window first", text) + + def test_autonomy_reference_carries_the_cheaper_source_rule(self): + text = self._read(self.REF_DIR, "autonomy.md").lower() + self.assertIn("look for a cheaper source before grinding", text) + self.assertIn("rank", text) + self.assertIn("capped run unchanged", text) + + def test_verify_reference_carries_the_measured_answer_rule(self): + text = self._read(self.REF_DIR, "verify.md").lower() + self.assertIn("answer duration and window questions", text) + self.assertIn("measured or", text) + self.assertIn("[[principle-prove-it]]", text) + + def test_verify_reference_extends_the_tier_rule_with_a_small_window(self): + text = self._read(self.REF_DIR, "verify.md").lower() + self.assertIn("prove it on a small window", text) + + def test_fires_rerun_capped_job_without_cheaper_source(self): + text = self._read( + self.FIXTURES_DIR, "fires_rerun_capped_job_without_cheaper_source.md" + ).lower() + self.assertIn("raise the cap", text) + self.assertIn("without asking what already holds the data", text) + + def test_stays_silent_cheaper_source_checked_first(self): + text = self._read( + self.FIXTURES_DIR, "stays_silent_cheaper_source_checked_first.md" + ).lower() + self.assertIn("test the cheapest first", text) + + def test_fires_quotes_estimate_as_duration(self): + text = self._read(self.FIXTURES_DIR, "fires_quotes_estimate_as_duration.md").lower() + self.assertIn("without measuring it this turn", text) + + def test_stays_silent_measured_duration_labelled(self): + text = self._read( + self.FIXTURES_DIR, "stays_silent_measured_duration_labelled.md" + ).lower() + self.assertIn("(measured)", text) + self.assertIn("(estimated)", text) + + def test_fires_new_data_path_straight_to_full_window(self): + text = self._read( + self.FIXTURES_DIR, "fires_new_data_path_straight_to_full_window.md" + ).lower() + self.assertIn("full seven-day window first", text) + + def test_stays_silent_small_window_first_then_widen(self): + text = self._read( + self.FIXTURES_DIR, "stays_silent_small_window_first_then_widen.md" + ).lower() + self.assertIn("then widen", text) + + if __name__ == "__main__": unittest.main()