From 57210f8aef693796538bbe6baf8c0f1386ab4931 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 17:23:56 +0900 Subject: [PATCH 1/3] docs(agents): record test-gate regression and stale-PR merge mechanics --- AGENTS.md | 35 +++++++++++++++++++++++++++++++++++ CLAUDE.md | 23 +++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index abc12d0221..6ea99b702d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -65,3 +65,38 @@ The materialization contract is also covered by [`docs/doctoring/exact-artifact- invalidates earlier checks and reviews. Never self-approve, dismiss reviews, force-push, disable a security gate, or use admin bypass for product or security changes. + +## Test-gate regressions and stale-PR merges + +- A red required check on your PR is not proof that your diff caused it. No workflow runs + the full `tests/` suite unconditionally on a push to `main`: the only unrestricted + full-suite run lives in `opencode-review-dispatch.yml`, which triggers on + `repository_dispatch` from a pull request, and the two quality workflows that do watch + `main` are each path-filtered to their own slice. A suite-breaking merge therefore lands + invisibly and then fails every later pull request regardless of that request's own diff. +- Reproduce a suspect failure on a clean baseline before repairing it. Run + `git worktree add /tmp/baseline --detach`, then `cd /tmp/baseline` + and run `python3 -m pytest tests -q`; that takes roughly four minutes and needs no + virtualenv. You must `cd` into the worktree: over thirty test files read repository files + through working-directory-relative paths such as `Path(".github/workflows/...")`, so + pointing pytest at the baseline directory from your own checkout silently tests your tree + and reports a green baseline that proves nothing. Baseline the pull request's actual base + or merge-base rather than `origin/main` once `main` has moved past it. If the failure + reproduces on the baseline it is pre-existing: repair it as its own pull request and name + the change that introduced it. +- When you change a workflow file or a `scripts/ci/` module, grep the whole `tests/` tree + for every literal you touched — event-type strings, cron expressions, environment-variable + names, tuple members, pinned digests — not only the obviously named sibling test. A change + can satisfy one oracle and still leave a second, independent one stale. +- `tests/test_pr_review_autofix_nvidia_nim_contract.py` pins the exact `git hash-object` + digest of `.github/workflows/opencode-review-dispatch.yml`. Any byte change to that + workflow makes the pin stale and fails a required gate for every open pull request — + reverts included, because a revert restores the original bytes while the pin stays on the + reverted value. Recompute it with + `git hash-object .github/workflows/opencode-review-dispatch.yml`; + `tests/test_opencode_rust_coverage_toolchain_contract.py` re-derives the same constant by + regular expression, so correcting the single declaration fixes both. +- Production code under `scripts/ci/` branches on `GITHUB_ACTIONS`, and pytest inherits that + variable in CI, so a failure class exists that cannot reproduce locally. Before calling a + scheduler change clean, run the affected tests both ways, including + `GITHUB_ACTIONS=true python3 -m pytest `. diff --git a/CLAUDE.md b/CLAUDE.md index e519e150d8..e1342d8be4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -161,3 +161,26 @@ repeatable compile command. cross-repo references as `owner/repo#num` or full URLs; durable knowledge in the repo/Project, not private memory; one roadmap phase at a time) are defined in `docs/CWL-MASTER-CONTEXT.md` §7 and apply here. +- **`interrogate` counts private helpers in this repo.** `[tool.interrogate]` in `pyproject.toml` + sets only `exclude = ["tests"]` and `fail-under = 100`. `ignore-private` is not set and the tool + defaults it off, so `_helper` and `__helper` both count toward the 100% requirement. (Even when it + is set it covers only double-underscore names; single-underscore needs `--ignore-semiprivate`.) + Sibling repositories differ — `contextual-orchestrator` enables six `ignore-*` flags and does skip + them — so do not carry a docstring habit across repositories. CI never runs interrogate repo-wide: + the quality workflows run it against explicit file lists, and `opencode-review-dispatch.yml` runs + it advisory-only behind `|| true`. +- **A stale PR's conflict scope is a snapshot, not a property of the PR.** `main` took 548 commits + across 106 merges in the last seven days, touching roughly ninety files a day, concentrated in + `.github/workflows/`, `scripts/ci/`, and `docs/doctoring/` — exactly where conflicts land. Re-run + the merge yourself immediately before resolving; a scope measured hours earlier can be several + times larger or smaller than the real one. Note that this repository mixes squash merges and merge + commits, so `git merge-base --is-ancestor` cannot tell you whether a PR's delta reached `main`; + compare content instead. +- **Nothing validates Markdown structure.** There is no markdownlint, remark, or mermaid check in any + workflow, and `ARCHITECTURE.md` (five mermaid diagrams) is read by no test, so a conflict + resolution that splits a fenced block into two fragments ships silently and renders the diagram + source as a plain code block. After resolving a conflict in a document containing fenced blocks, + re-read the whole enclosing section rather than the diff hunk, and confirm each block has one + opening fence carrying its language tag and one closing fence. Do not check by counting fences — a + split leaves four where there were two. The damage can also arrive inherited, from an earlier + commit on the same branch or from the autofix flow's conflict-marker resolution. From 0ff7ed20e3aef8b822d37a43c8262fc52a46ea12 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 17:30:47 +0900 Subject: [PATCH 2/3] docs(agents): tighten test-gate rules after adversarial review Applied fixes from three independent critique lenses run against the draft: - Narrowed "a red required check" to the suite gates this procedure actually diagnoses; Semgrep/CodeQL/Strix/Scorecard are a different diagnosis. - Replaced the absence claim about push-to-main full-suite runs with the checkable one: those workflows exist but are paths-filtered, so a pairing broken outside their declared paths lands with no full-suite run. - Cut point-in-time merge-velocity figures, which read as false during any quiet period, in favour of the mechanism they were illustrating. - Replaced the CI-inventory assertion about markdown linting with the durable in-repo fact: no test parses fenced blocks. - Made the content-hash pin discoverable via grep instead of enumerated, and warned that most forty-hex literals under tests/ are commit/action pins that hash-object would corrupt. - Stated the interrogate rule first and the configuration second, since the config is a line a future PR can flip. Added the two-dot/three-dot diff rule: gh pr diff is already three-dot, so the mass-revert illusion belongs to two-dot git diff, not to gh. Verified: python3 -m pytest tests -q -> 2883 passed, 1 skipped, 21 subtests. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 34 ++++++++++++++++++++-------------- CLAUDE.md | 46 +++++++++++++++++++++++----------------------- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ab0e612384..a5c72cf12c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,12 +102,13 @@ history, never the organization's actual state. ## Test-gate regressions and stale-PR merges -- A red required check on your PR is not proof that your diff caused it. No workflow runs - the full `tests/` suite unconditionally on a push to `main`: the only unrestricted - full-suite run lives in `opencode-review-dispatch.yml`, which triggers on - `repository_dispatch` from a pull request, and the two quality workflows that do watch - `main` are each path-filtered to their own slice. A suite-breaking merge therefore lands - invisibly and then fails every later pull request regardless of that request's own diff. +- A red `tests`, coverage, or `interrogate` gate on your pull request is not proof that your + diff caused it. Full-suite execution on a push to `main` is not guaranteed: the workflows + that run `pytest tests` on push are `paths:`-filtered, so a pairing broken outside their + declared paths reaches `main` with no full-suite run. The breakage then surfaces on the + next pull request whose review dispatch does run the suite, and fails it regardless of + that request's own diff. This procedure covers the suite gates only; a red Semgrep, + CodeQL, Strix, or Scorecard check is a different diagnosis. - Reproduce a suspect failure on a clean baseline before repairing it. Run `git worktree add /tmp/baseline --detach`, then `cd /tmp/baseline` and run `python3 -m pytest tests -q`; that takes roughly four minutes and needs no @@ -122,14 +123,19 @@ history, never the organization's actual state. for every literal you touched — event-type strings, cron expressions, environment-variable names, tuple members, pinned digests — not only the obviously named sibling test. A change can satisfy one oracle and still leave a second, independent one stale. -- `tests/test_pr_review_autofix_nvidia_nim_contract.py` pins the exact `git hash-object` - digest of `.github/workflows/opencode-review-dispatch.yml`. Any byte change to that - workflow makes the pin stale and fails a required gate for every open pull request — - reverts included, because a revert restores the original bytes while the pin stays on the - reverted value. Recompute it with - `git hash-object .github/workflows/opencode-review-dispatch.yml`; - `tests/test_opencode_rust_coverage_toolchain_contract.py` re-derives the same constant by - regular expression, so correcting the single declaration fixes both. +- Read a stale pull request's own changes with a three-dot diff — + `git diff ...` — or with `gh pr diff`, which is already three-dot. A two-dot + `git diff ` renders everything the base gained since the fork point as though + this branch deleted it, so an untouched branch reads as a mass revert. +- Content-hash pins exist under `tests/`; find them before editing a workflow. Run + `grep -rn 'hash-object' tests/` — today that is the `git hash-object` pin of + `.github/workflows/opencode-review-dispatch.yml`. Any byte change to a pinned file makes + its constant stale and fails a required gate for every open pull request, reverts included, + because a revert restores the original bytes while the pin stays on the reverted value. + Recompute only with `git hash-object `, and only for constants you have confirmed are + blob pins: most forty-hex literals under `tests/` are commit or action pins, and + recomputing those corrupts them. A second contract re-derives the dispatch pin by regular + expression from the first, so keep the assignment on one line and correct it in one place. - Production code under `scripts/ci/` branches on `GITHUB_ACTIONS`, and pytest inherits that variable in CI, so a failure class exists that cannot reproduce locally. Before calling a scheduler change clean, run the affected tests both ways, including diff --git a/CLAUDE.md b/CLAUDE.md index d0569c64e9..4d471b9cbe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -177,26 +177,26 @@ repeatable compile command. longer than it is. Querying `status=success` and `status=failure` directly cuts through the churn to the most recent real conclusion of each kind. Those are historical signals about pipeline liveness only — they never substitute for exact-current-head evidence on the PR you are acting on. -- **`interrogate` counts private helpers in this repo.** `[tool.interrogate]` in `pyproject.toml` - sets only `exclude = ["tests"]` and `fail-under = 100`. `ignore-private` is not set and the tool - defaults it off, so `_helper` and `__helper` both count toward the 100% requirement. (Even when it - is set it covers only double-underscore names; single-underscore needs `--ignore-semiprivate`.) - Sibling repositories differ — `contextual-orchestrator` enables six `ignore-*` flags and does skip - them — so do not carry a docstring habit across repositories. CI never runs interrogate repo-wide: - the quality workflows run it against explicit file lists, and `opencode-review-dispatch.yml` runs - it advisory-only behind `|| true`. -- **A stale PR's conflict scope is a snapshot, not a property of the PR.** `main` took 548 commits - across 106 merges in the last seven days, touching roughly ninety files a day, concentrated in - `.github/workflows/`, `scripts/ci/`, and `docs/doctoring/` — exactly where conflicts land. Re-run - the merge yourself immediately before resolving; a scope measured hours earlier can be several - times larger or smaller than the real one. Note that this repository mixes squash merges and merge - commits, so `git merge-base --is-ancestor` cannot tell you whether a PR's delta reached `main`; - compare content instead. -- **Nothing validates Markdown structure.** There is no markdownlint, remark, or mermaid check in any - workflow, and `ARCHITECTURE.md` (five mermaid diagrams) is read by no test, so a conflict - resolution that splits a fenced block into two fragments ships silently and renders the diagram - source as a plain code block. After resolving a conflict in a document containing fenced blocks, - re-read the whole enclosing section rather than the diff hunk, and confirm each block has one - opening fence carrying its language tag and one closing fence. Do not check by counting fences — a - split leaves four where there were two. The damage can also arrive inherited, from an earlier - commit on the same branch or from the autofix flow's conflict-marker resolution. +- **Do not assume `interrogate` skips private helpers.** `[tool.interrogate]` here sets no + `ignore-*` flags and the tool defaults them off, so a docstring-less `_helper` or `__helper` in + `scripts/ci/` counts against the 100% gate — it is the stricter docstring check, not the laxer + one. Sibling repositories configure this differently (`contextual-orchestrator` enables six + `ignore-*` flags and does skip them), so read the target repo's `pyproject.toml` rather than + carrying a docstring habit across repositories. Note also that `ignore-private` would cover only + double-underscore names; single-underscore needs `ignore-semiprivate`. +- **A stale PR's conflict scope is a snapshot, not a property of the PR.** Any advance of the base + between measuring the conflicts and resolving them invalidates the list, and base advances land in + the same directories conflicts do (`.github/workflows/`, `scripts/ci/`, `docs/doctoring/`). Scope + grows as often as it shrinks — a branch that merged cleanly can become conflicted with no change + to the branch at all — so re-run the merge yourself immediately before resolving and treat any + earlier measurement, including your own from minutes ago, as expired. Resolving against a stale + smaller scope silently leaves conflicts unhandled. +- **No test parses fenced code blocks.** The doc-contract tests match exact prose in specific files; + none of them check Markdown structure, and `ARCHITECTURE.md` (five mermaid diagrams) is read by no + test at all. A conflict resolution that splits a fenced block into two fragments therefore ships + green, rendering the diagram source as a plain code block. After resolving a conflict in a + document containing fenced blocks, re-read the whole enclosing section rather than the diff hunk, + and confirm each block has one opening fence carrying its language tag and one matching closing + fence. Do not check by counting fences — a split leaves four where there were two, so an even + count proves nothing. The damage can also arrive inherited, from an earlier commit on the same + branch or from the autofix flow's conflict-marker resolution. From e280c4fbc875da0e9d994bb23fcd237aa393ad8b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 5 Sep 2026 17:33:00 +0900 Subject: [PATCH 3/3] docs(agents): name the real categories of forty-hex literal in tests/ Counted them rather than characterising from memory: of 17 distinct forty-hex literals across 14 test files, exactly one is a blob pin. The rest are pinned action SHAs, a vendored-revision pin, synthetic fixture heads, and assertions that a SHA appears in a document. "Commit or action pins" undersold that variety; naming the categories lets an agent recognise which constant it is holding before reaching for hash-object. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a5c72cf12c..91d7d93a49 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,10 +132,13 @@ history, never the organization's actual state. `.github/workflows/opencode-review-dispatch.yml`. Any byte change to a pinned file makes its constant stale and fails a required gate for every open pull request, reverts included, because a revert restores the original bytes while the pin stays on the reverted value. - Recompute only with `git hash-object `, and only for constants you have confirmed are - blob pins: most forty-hex literals under `tests/` are commit or action pins, and - recomputing those corrupts them. A second contract re-derives the dispatch pin by regular - expression from the first, so keep the assignment on one line and correct it in one place. + Recompute only with `git hash-object `, and only for a constant you have confirmed is + a blob pin. Nearly every other forty-hex literal under `tests/` is something else — a + pinned action SHA, a vendored-revision pin, a synthetic fixture head, or an assertion that + a SHA appears in a document — and pointing `hash-object` at any of those produces a wrong + value that breaks what it replaces. A second contract re-derives the dispatch pin by + regular expression from the first, so keep the assignment on one line and correct it in one + place. - Production code under `scripts/ci/` branches on `GITHUB_ACTIONS`, and pytest inherits that variable in CI, so a failure class exists that cannot reproduce locally. Before calling a scheduler change clean, run the affected tests both ways, including