Skip to content

test(specs): check the network-denial step count against the workflow - #254

Merged
mbeacom merged 2 commits into
mainfrom
test/network-denial-count
Oct 1, 2026
Merged

mbeacom merged 2 commits into
mainfrom
test/network-denial-count

Conversation

@mbeacom

@mbeacom mbeacom commented Oct 1, 2026

Copy link
Copy Markdown
Owner

What and why

This closes ADR-0040's last open decision, action item 7: should the clean-clone-builds network-denial step count be derived rather than restated in four places?

I'm settling it as checked, not derived. The count appears in four hand-written places in specs/010-catalog-backstage/:

  • observed-failing-register.md §4.6;
  • the clean-clone-offline README;
  • FR-050 in spec.md;
  • T093 in tasks.md.

ADR-0040 keeps specs/ out of machine rewriting, so generating these sentences would contradict the record the item belongs to. And the failure was never the prose: twice, a step was added to the job and nobody re-counted.

scripts/network-denial-count.test.ts parses the job from ci.yml and:

  • requires bun test to be the only post-install step not run through scripts/run-network-denied.ts;
  • fails when any of the four documents states a count other than the workflow's (16 of 17 today);
  • fails when a document stops stating a count it can read, so rewording a sentence can't silently turn this into a check of nothing.

ADR-0040 item 7 is ticked with the outcome, and §4.6 now says the count is checked and why.

Checklist

  • Commits are DCO signed off.
  • An ADR action item is closed with its outcome recorded; the decision itself is unchanged.
  • Schema: n/a
  • packages/ci/dist: n/a
  • Observed failing before it counted (ADR-0016):
    • a duplicated wrapped step in clean-clone-builds fails all four documents;
    • T093 edited to "15 of the 17" fails that file;
    • FR-050's sentence reworded away fails that file through the "still states a count" rule.
  • The scripts/ suite (1,113 tests), adr lint, check:stale-refs, and typecheck pass.

Notes for reviewers

  • gate-integrity flags this because it adds a file under scripts/.
  • Adding a fifth restatement means adding its path to STATEMENTS in the test. The test can't discover new restatements on its own, the same hand-maintained-scope trade-off ADR-0040 accepts for the prose guard.
  • ADR-0040's item 5 (a public Markdown inventory formatter) stays open by design, until a second adopter asks or the reviewBy date.

ADR-0040 action item 7 asked whether the clean-clone-builds network-denial
count should be derived rather than restated in four places. I am
settling it as checked, not derived. The four statements are
hand-written requirement and evidence prose in specs/010-catalog-backstage,
and ADR-0040 keeps specs/ out of machine rewriting. What actually went
wrong, twice, was a step added to the job without a re-count.

scripts/network-denial-count.test.ts parses clean-clone-builds. It
requires bun test to be the only post-install step not run through
run-network-denied.ts, and it fails when observed-failing-register.md
§4.6, the clean-clone-offline README, FR-050, or T093 states a different
count, or stops stating one it can read. The last rule keeps a reworded
sentence from turning the check into a check of nothing.

Observed failing (ADR-0016) three ways: a duplicated wrapped step in the
job (all four documents fail), T093 edited to 15 (that file fails), and
FR-050's sentence reworded away (that file fails).

ADR-0040 item 7 is ticked with the outcome, and §4.6 records why the
count is now checked.

Signed-off-by: Mark Beacom <m@beacom.dev>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 02:09
@mbeacom mbeacom added the gate-change-acknowledged A maintainer has seen and accepted this PR's change to the CI gate surface (ADR-0035) label Oct 1, 2026
@mbeacom mbeacom self-assigned this Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Decisions governing this change

  • 0001 — Record architecture decisions as versioned markdown in git
    • via path: docs/adr/**
  • 0007 — Isolate integrations as optional adapters and build only against public surfaces
    • via path: .github/workflows/**
  • 0010 — Use Bun as the package manager and test runner while publishing Node-targeted artifacts
    • via path: .github/workflows/**
  • 0017 — Keep dependency audit scope explicit and release-scoped
    • via path: .github/workflows/ci.yml
  • 0025 — Ship badges as recipes over existing output, not a new CLI surface
    • via path: .github/workflows/ci.yml
  • 0032 — Publish one lockstep OCI image after the coordinated release succeeds
    • via path: .github/workflows/ci.yml
  • 0035 — Execute the gates that certify a pull request from the default branch
    • via path: .github/workflows/**
    • via path: scripts/**
  • 0040 — Keep derived surfaces in lockstep with three mechanisms matched to three classes of drift
    • via path: .github/workflows/ci.yml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The checker can falsely classify unwrapped steps and omits an existing live count restatement.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds a guard that synchronizes documented network-denial counts with the CI workflow.

Changes:

  • Parses clean-clone-builds and validates four documentation files.
  • Records the checked-not-derived decision in ADR-0040.
  • Documents the new guard in the evidence register.

Decision-context MCP verification was unavailable.

File Description
scripts/​network-denial-count.test.ts Adds workflow/count checks.
docs/​adr/​0040-keep-derived-surfaces-in-lockstep-with-three-mechanisms-matched-to-three-classes.md Closes action item 7.
specs/​010-catalog-backstage/​evidence/​observed-failing-register.md Records the checking mechanism.

Comment thread scripts/network-denial-count.test.ts Outdated
Comment thread scripts/network-denial-count.test.ts
…e command

Two review findings on #254.

The check counted a clean-clone-builds step as network-denied whenever
its run mentioned run-network-denied.ts anywhere. So
`bun run build && echo scripts/run-network-denied.ts` counted, and so
did `wrapper -- a && curl ...`, where the shell runs curl outside the
wrapper. A step now counts only when its run starts with the wrapper
and has no shell operator (&&, ||, ;, |, &, a newline, a backtick, or
$(...)) outside quotes. The existing `sh -c '... && ...'` steps still
count, because their operators run inside the wrapped shell. A unit
test covers both sides, and it failed against the old substring check.

The comment on the bun test step in ci.yml restated "16 of the 17",
a fifth copy that the check didn't read. I removed the number rather
than parsing it: the comment now points to §4.6 and to this test. The
test also fails if an "N of the M" count or an ordinal step count
reappears in ci.yml, and it failed against the old comment.

Signed-off-by: Mark Beacom <m@beacom.dev>
@github-actions github-actions Bot removed the gate-change-acknowledged A maintainer has seen and accepted this PR's change to the CI gate surface (ADR-0035) label Oct 1, 2026
@mbeacom mbeacom added the gate-change-acknowledged A maintainer has seen and accepted this PR's change to the CI gate surface (ADR-0035) label Oct 1, 2026
@mbeacom
mbeacom enabled auto-merge (squash) October 1, 2026 02:34
@mbeacom
mbeacom merged commit a90d75b into main Oct 1, 2026
22 of 23 checks passed
@mbeacom
mbeacom deleted the test/network-denial-count branch October 1, 2026 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate-change-acknowledged A maintainer has seen and accepted this PR's change to the CI gate surface (ADR-0035)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants