Skip to content

docs(testing): pin which layer runs every suite, and check it (#749) - #755

Merged
Birfy merged 1 commit into
mainfrom
feat/749-testing-layers
Oct 22, 2026
Merged

docs(testing): pin which layer runs every suite, and check it (#749)#755
Birfy merged 1 commit into
mainfrom
feat/749-testing-layers

Conversation

@Birfy

@Birfy Birfy commented Oct 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Written-by: Dev2

Fixes #749

Tip SHA: 8b559c1

Reproduce

Nothing in the repository said which test belongs to which gate, so the answer to "is this covered before it merges?" was only readable from the YAML: a new *.test.mjs that no workflow globs runs nowhere and nobody notices, and a new workflow that gates nothing is indistinguishable from one that does. On main, add misc/example.test.js (or .github/workflows/fresh-gate.yml) and nothing fails; with this PR the check exits 1 and names the file.

What changed

  • docs/testing-layers.json (new): the manifest. Three layer definitions (L1 commit, L2 merge, L3 self-test) with what each must catch, what it must not depend on, its budget and its command; then the two kinds of row:
    • suite rows (test-file suites) — core, plugins/social, plugins/tree, plugins/timeline, the repo scripts, interop, each with the layer that runs it and the glob it owns; plus none rows with a reason for patches/ds-bot/files/** (generated copies, compared by ds-bot-patches check), ds-bot/test/** (pristine upstream, never run directly) and dev/live-e2e/test/** (offline unit tests for the live harness that no CI gate runs today).
    • workflow rows — all 11 files in .github/workflows/, each with a kind, its layer and whether it gates a merge. The kind is the acceptance's second kind of row: gate runs the suites or judges a queue entry (test.yml, merge-gate.yml); workflow-only is a layer that carries no test file of its own — docker.yml (L2), live-e2e.yml (L3), nightly.yml, label-hygiene.yml; not-a-layer is a workflow that runs no tests and gates nothing (review-gate, risk-tier, controller, audit-merges, release), with the reason written out.
  • docs/testing-layers.md (new): the three layers in prose (what each must catch, must not depend on, budget, command, what runs it), the rule that a case may only enter a layer that can run it, an L3 pointer to docs/self-test.md — Self-test case catalog with stable ids — to land as docs/self-test.md (current cases, gaps, planned) #742 owns that file and this PR neither creates nor rewrites it — and the two generated tables.
  • .github/scripts/testing-layers.mjs (new): --check and --write. It fails on a test file no suite row covers, on a workflow with no row, on a row naming a workflow that does not exist, on an unknown layer or workflow kind, on a gate row without a role, on a workflow-only row that says neither what it guarantees nor why it is not a layer, on a none row without a reason, and on a doc whose generated block no longer matches the manifest. Workflow-only rows are validated against the workflow file itself, which is exactly the row a test-path check could never verify.
  • .github/scripts/testing-layers.test.mjs (new): 7 cases — the real repository classifies everything; dropping the plugins/timeline row names its test file; dropping the docker row names docker.yml; a new unclassified test file and a new workflow are both named; a none row without a reason, an unknown layer, an unknown workflow kind, a workflow-only row with no trigger and a stale doc are reported; the doc's tables equal renderTables(manifest); the glob matcher covers one segment, many segments and a trailing **.
  • .github/workflows/test.yml: one step in package — node .github/scripts/testing-layers.mjs --check. No new required status check and no new workflow: package already aggregates into tests-ok, so L2 stays exactly the checks main requires today.

What I ran

At this tip 8b559c11f9675a0c236c1551023f1d9bfc819c5c (base 1bda9b2a, 5 files, +651/−0):

  • node .github/scripts/testing-layers.mjs --check → 307 test file(s) in 11 workflow(s) / every test file and every workflow is classified, exit 0.
  • node --test .github/scripts/*.test.mjs → 220 tests, 220 pass, 0 fail. Baseline without the new test file in the same tree: 213/213.
  • node --test .github/scripts/testing-layers.test.mjs → 7/7.
  • arch-docs: OK (5 changed file(s)); changelog.mjs check → 23 fragments well formed (no fragment here: nothing a user sees changes); all 11 workflows parse (js-yaml), including the edited test.yml.
  • The previous tip of this branch (195d5240) had already run the new package step in CI: package and merge-gate green there, so the check does run in the job as wired.

Red without the source change

Both required reds run against the real manifest and repository, not only in the fixture, and both are re-run on this tip:

  • Delete the plugins/timeline suite row → exit 1, and all 11 test files of that suite are named (unclassified test file: plugins/timeline/test/alerts.test.js, …).
  • Delete the docker workflow row → exit 1, unclassified workflow: .github/workflows/docker.yml — the workflow-only row that owns no test path at all.
  • Restore either → exit 0 again.
  • In the test file the same two reds are pinned as fixtures, plus a new misc/*.test.js, a new .github/workflows/fresh-gate.yml, an unknown workflow kind and a workflow-only row with no trigger, so a regression fails package even if the manifest is edited but the check stops detecting.

Not run

  • actionlint is not installed on this box; the YAML parse and the existing package lint step stand in for it, and CI runs the real actionlint on this PR.
  • The new package step itself only runs in CI — locally I ran the identical command by hand.
  • npm run verify as a whole and the core/plugins suites: this diff touches no core/, plugins/ or patches/ file, so those suites cannot be affected; ds-bot-patches check is likewise untouched.

Risk

T3: .github/** is T3 in .github/agent/risk-rules.json, and this adds a step to the package job. Green tests-ok at the tip plus a PASS at this tip, then the queue.

What the change can break: the new package step fails the job when the manifest and the repository disagree, which is the point — but a false positive would block every pull request, so the check is narrow (it names the file, it does not judge content) and its own 7 cases pin both directions. Nothing else in test.yml changes, no permissions change, no new workflow, no new required check, and the manifest is data: deleting a row makes the check red rather than making a suite silently disappear.

The one judgement a reviewer should look at: dev/live-e2e/test/** is classified none rather than L1. Those cases are offline (a fake adapter, no model), so they belong in L1, but no CI gate runs them today and moving them into L1 is out of scope here. The row's reason says exactly that, and the L3 workflow row (live-e2e.yml) is separate.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 22, 2026
@Birfy Birfy self-assigned this Oct 22, 2026
@Birfy
docs(testing): pin which layer runs every suite, and check it (#749)
8b559c1 
Nothing said which test belonged to which gate, so a suite could run
everywhere or nowhere. `docs/testing-layers.json` now maps every test
file to L1 (commit), L2 (merge) or `none` with a reason, and gives every
workflow in `.github/workflows/` a row with its kind, its layer and
whether it gates a merge. `docs/testing-layers.md` explains the three
layers and generates its tables from the manifest.

The two kinds of row are both enforced. A suite row owns test paths; a
workflow row is checked against the workflow file that exists, which is
the row a test-path check could never verify. Within a workflow row the
kind says what it is: `gate` runs the suites or judges an entry,
`workflow-only` is a layer with no test file of its own (`docker`, the
compat matrix, `live-e2e`, `label-hygiene`), and `not-a-layer` is a
workflow that runs no tests, with the reason written out.

`.github/scripts/testing-layers.mjs --check` fails on a test file or a
workflow nobody classified, and on a doc that no longer matches the
manifest; the `package` job runs it, so a new `*.test.mjs` cannot land
unclassified.
@Birfy
Birfy force-pushed the feat/749-testing-layers branch from 195d524 to 8b559c1 Compare October 22, 2026 10:59
@Birfy

Birfy commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 8b559c1
Written-by: Reviewer

The two required reds hold on the real manifest, not only in the fixtures, and the workflow-only row is checked against the workflow file as asked. CI at this tip is 9/9 green and mergeable: clean; the new step sits inside the existing package job, so no new required check appears.

Two non-blocking notes, neither about this diff's behaviour:

  • A manifest row is not checked against "does its glob still match a file". A none row whose path is typo'd, or whose directory is deleted later, stays silent — nothing becomes unclassified, so the check passes. Only layers and runs/role are validated structurally. Worth a problems.push for rows whose paths match nothing, if this list is expected to survive the directories it names.
  • globMatch('**/*.test.js', 'x.test.js') is false (the ** expands to .* and the pattern then requires a slash). No row uses that shape today; it would bite whoever writes a root-level catch-all row.

The count moved from the body's 307 to 309 at this tip: the rebase onto 1bda1b2a brought core/test/approval-waiting.test.js and one more test file with it, and the check classified them without a manifest edit — which is the behaviour we want. No prose hardcodes the number.

Ran:

  • node .github/scripts/testing-layers.mjs --check → 309 test file(s) in 11 workflow(s), every test file and every workflow classified, exit 0
  • TZ=UTC node --test .github/scripts/testing-layers.test.mjs → 7/7
  • three cuts on the real docs/testing-layers.json, each restored and re-checked to exit 0: delete the plugins/timeline suite row → exit 1, names all 11 plugins/timeline/test/*.test.js; delete the docker workflow row → exit 1, unclassified workflow: .github/workflows/docker.yml; add .github/workflows/zzz-probe.yml → exit 1, names it
  • node .github/scripts/testing-layers.mjs --write on a copy of the doc → byte-identical, so the generated block is stable and --check will not flap

Not run: the whole .github/scripts/*.test.mjs set (the ds-bot-patches cases need a .git; package is green at this tip), npm run verify, actionlint.

@Birfy
Birfy added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit e15d14c Oct 22, 2026
9 checks passed
@Birfy
Birfy deleted the feat/749-testing-layers branch October 22, 2026 11:04
@Birfy Birfy mentioned this pull request Oct 22, 2026
Merged
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 23, 2026
@Birfy
feat(labels): mark a pull request pass when the verdict was a PASS (T…
6d943d8 
…hinkFlowLab#739) (ThinkFlowLab#760)

Written-by: Dev2

Fixes ThinkFlowLab#739

Tip SHA: f61f459

## Reproduce

A merged pull request keeps no trace of the verdict that let it in.
`agent-pr` stays on the list, and the conclusion lives only in a comment
thread: to answer "was this reviewed?" you open the pull request and
scroll. On `main` today there is no `pass` label at all
(`.github/labels.json` has 12 entries), so `gh pr list --label pass`
answers nothing, and after a branch is deleted the review record is
unreadable from the list.

## What changed

- `.github/labels.json`: a `pass` label (`#006B75`, "Reviewer PASS
verdict at the merged head; set by pass-label, never removed"), so
`sync-labels.mjs` creates it. The count assertion in
`sync-labels.test.mjs` moves from 12 to 13 and `pass` joins the names
the test requires.
- `.github/scripts/pass-label.mjs` (new): `verdictOf(body, { writers,
association })` reads a conclusion with `parseReport` from
`review-gate.mjs` — the parser the gates already share — and returns it
only when the verdict is `PASS`, the comment's `author_association` is
`OWNER`/`MEMBER`/`COLLABORATOR`, and the `Written-by:` name is in the
policy's `writers`. A body whose first line is `Correction from …` never
counts, whatever `Verdict:` it quotes. `planComment` decides one event;
`planBackfill` decides the merged ones from raw API pages; `main` runs
either mode against `gh`.
- `.github/workflows/pass-label.yml` (new): on `issue_comment` (created)
it labels the pull request the conclusion was posted on, and
`workflow_dispatch` runs `--backfill`. The comment body is only parsed
and never run; the code that runs is checked out from `main`, so an
untrusted comment cannot bring code with it. Permissions are `contents:
read`, `issues: write`, `pull-requests: write`.
- `docs/testing-layers.json` + the generated `docs/testing-layers.md`:
`pass-label.yml` gets its workflow row (`workflow-only`, gates nothing),
so the layer check ThinkFlowLab#755 added stays honest about the new workflow.
- The label is **only ever added**. A later INC on a newer tip, or a
moved tip, does not remove it — the label records that a PASS was given,
and `hold` stays the one manual stop. A test asserts the script contains
no removal call at all.

## What I ran

At this tip `f61f4595b5f3c06183623ac30d3e029b8c136ba7` (base `e15d14c3`,
7 files, +303/−2):

- `node --test .github/scripts/*.test.mjs` → **226 tests, 226 pass, 0
fail** (baseline before this diff: 219).
- `node --test .github/scripts/pass-label.test.mjs` → **6/6**: a PASS
labels; INC, `Correction from …`, a non-member association and an
unknown `Written-by:` name never label; a `Replaces:` PASS labels like
any PASS; a second run adds nothing; backfill picks the
merged-already-on-a-PASS one and leaves the INC-only, already-labelled,
still-open and PASS-at-another-tip rows alone; a 12-character tip names
the head.
- `node .github/scripts/testing-layers.mjs --check` → `310 test file(s)
in 12 workflow(s)`, all classified (the new workflow and the new test
file are both listed).
- `node scripts/ds-bot-patches.mjs check` → three ok (untouched by this
diff).
- `docs/testing-layers.md` regenerated with `--write`.

## Red without the source change

- Delete the `Correction` guard in `verdictOf` → the correction case
fails, because `parseReport` finds the `Verdict: PASS` the correction
quotes.
- Drop the `writer`/`writers` check → the unknown-`Written-by:` case
fails (and a passer-by with `association: NONE` still fails on its own
rule).
- Drop the `already labelled` check in `planComment` → the "second run
adds nothing" case fails.
- Drop the `namesHead` check in `planBackfill` → the PASS-at-another-tip
row is labelled too, so the backfill case sees `[11, 15]` instead of
`[11]`.
- Add a `DELETE` call to the script → the "closed record" case fails.

## Not run

- The two workflows themselves have never executed: `pass-label.yml`
does not exist on `main` yet. The first trigger is the next PASS comment
after merge; `workflow_dispatch` runs the backfill, which I did **not**
run from here — labelling 40+ merge records is the workflow's job, not a
by-hand write.
- `actionlint` is not installed on this box; `package` runs the real one
on this PR.
- `npm run verify` as a whole; nothing under `core/`, `plugins/` or
`patches/` changes.

## Risk

**T3** (`.github/**` and `docs/testing-layers.json`), so green
`tests-ok` at the tip plus a PASS at this tip, then the queue.

The write is a single `POST /issues/<n>/labels` with one label, from a
workflow that has `issues: write` and nothing else. Three things could
go wrong and each is bounded: a mistake labels a pull request `pass`; a
pass-through of untrusted text would at worst add a label (it cannot
remove anything, and it never gates a merge — `merge-gate` reads the
conclusion itself at the entry); and the backfill could label a pull
request whose merged head carried a PASS for the wrong reason — the tip
check reduces that to a PASS that names the merged head, which is the
record the label is supposed to keep. If the label turns out to be
unwanted on an old pull request, removing it is a human action, like
`hold`.
@Birfy Birfy removed the agent-pr Pull request opened by an agent label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Birfy Birfy

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Lay out the test layers: what runs at commit, at merge, and at self-test

1 participant

@Birfy