Skip to content

Aggregate independent checks to report all failures at once - #138

Merged
alexkroman merged 5 commits into
mainfrom
claude/agent-repo-dev-tools-e9fdwy
Aug 13, 2026
Merged

Aggregate independent checks to report all failures at once#138
alexkroman merged 5 commits into
mainfrom
claude/agent-repo-dev-tools-e9fdwy

Conversation

@alexkroman

Copy link
Copy Markdown
Collaborator

What & why

scripts/check.sh runs many independent, read-only checks before the Swift build (swift-format, swiftlint, actionlint, zizmor, prettier, markdownlint, shellcheck, shfmt, ruff, pytest, site integrity, portability, and now settled-decision invariants). Under set -e, the first failure stops the run and hides all the rest — a single pending swift-format reflow would end the run before swiftlint, actionlint, zizmor, prettier, markdownlint, shellcheck, shfmt, ruff, and pytest had said anything.

This change introduces run_check() to record failures and keep going, collecting them all and reporting them together at the bottom. It's the same shape the checks already use internally (the dependency guard and sound catalog each tally violations and report in one pass), applied across the whole read-only block.

The fail-fast region (Swift build, coverage gate, sanitizers, app build, integration tests) remains unchanged — those have real dependencies, so continuing would produce noise, not findings.

Also adds scripts/check-invariants.sh, which mechanically enforces the "Settled decisions" table from AGENTS.md. It greps for constructs that reintroduce deliberately-removed patterns (AVAudioEngine capture, streaming paths, on-device models, etc.) and fails the health check rather than depending on review memory. Shape mirrors check-portability.sh: parallel pattern/advice arrays, per-rule self-test, and an escape hatch for false positives.

How it was tested

  • scripts/check.sh passes
  • scripts/check.sh --portable passes
  • scripts/check-invariants.sh --self-test passes
  • Verified the aggregation works by introducing a deliberate failure in one check and confirming all others still run and report together at the end
  • Verified the exit trap cleans up the app build log on all exit paths (normal, fail-fast, ^C)

https://claude.ai/code/session_01TACi7k2226no2tVcR3YJr1

Two gaps ported from the tooling in alexkroman/agent, both about a red run
telling you everything it knows instead of one thing at a time.

check.sh aggregates its read-only checks. Everything before `swift test` is
independent and non-mutating, so there was no ordering reason for the first
failure to hide the rest — but under a plain `set -e` it did: one pending
swift-format reflow ended the run before swiftlint, actionlint, zizmor,
prettier, markdownlint, shellcheck, shfmt, ruff and pytest had said anything.
That is the same complaint PR #128's reordering comment makes about the build
("their findings arrived one red run at a time"); reordering fixed only the half
that was the build's fault. `run_check` now records a failure and continues, and
an exit trap reports the tally. The trap rather than a block at the bottom is
load-bearing: the fail-fast region sits between the two, so `swift test` failing
under errexit would exit straight past a closing summary and throw away
everything already collected. swiftlint analyze and periphery are aggregated for
the same reason at higher cost — each takes minutes, and periphery runs its own
xcodebuild. The build, coverage gate, sanitizers and integration steps stay
fail-fast, where the dependency is real.

check-invariants.sh mechanizes the grep-decidable half of AGENTS.md's "Settled
decisions" table, which until now was enforced only for as long as a reviewer
remembered it. Twelve rules: AVAudioEngine/installTap capture, a streaming or
on-device path, a client-side cleanup pass, config.language_code, config.prompt,
a keystroke-typing injector, LSUIElement, a KeyboardShortcuts import, a
self-replacing updater, the production Keychain under Tests/, and deprecated
shims. Shape is check-portability.sh's, since both answer "does this tree
contain a construct we decided against?": parallel rule arrays, a --self-test
asserting each pattern still matches its own probe, and a `// invariant-ok:`
escape hatch. Two additions to that shape — negative probes drawn from real
lines that sit one character from a rule (the Accessibility prompt dictionary
against the "prompt" wire key, the test keychain against the production one),
and a check that every rule's scope matches tracked files, because a scope typo
kills a rule as quietly as a broken regex.

Scopes do real work: KeytermsWireTests contains
`#expect(object.keys.contains("language_code") == false)` — the invariant being
enforced — so that rule reads Sources/ only. Rules that cannot separate a
violation from correct code stay prose and stay in review; the filler-word
clause is the clearest case, since CleanupInstruction's rewrite instruction
legitimately names filler sounds.

Verified with scripts/check.sh --portable (green), and against deliberately
broken trees: five simultaneous failures all reported in one run, and a
recorded failure still reported when a simulated `swift test` abort ends the
run early.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TACi7k2226no2tVcR3YJr1
Comment thread .claude/skills/check/SKILL.md Outdated
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Dev build

Download Blurt.app — built from d1d43cb, Debug-Local,
ad-hoc signed. Expires in 14 days.

Installing it
cd ~/Downloads
unzip -o blurt-dev-build-pr-138.zip   # GitHub wraps every artifact in a zip
unzip -o Blurt-dev-d1d43cb.zip
find Blurt.app -exec xattr -c {} +   # clear quarantine: xattr lost -r in macOS 12.3
rm -rf /Applications/Blurt.app && cp -R Blurt.app /Applications/
open -a Blurt

It is ad-hoc signed and not notarized: Gatekeeper refuses to open it until
the quarantine flag is cleared, and macOS treats it as a different app from a
released Blurt, so you have to re-grant Microphone, Accessibility, and Input
Monitoring. Reinstall the release DMG
when you are done reviewing.

Expect that re-grant once per dev build, including a second build of this
same PR. TCC pins an Accessibility grant to the signature that took it, and an
ad-hoc signature is just a hash of the binary, so every build is a new app as far
as tccd is concerned. Blurt clears the orphaned grant at launch, which is what
keeps the Accessibility step from getting stuck on a Blurt row that is switched on
and still denied. If you are coming from a build old enough to predate that,
clear the grant yourself once:

tccutil reset Accessibility dev.alex.blurt

The gate shipped in the previous commit looks like it enforces AGENTS.md's
settled-decisions table, but nothing tied a rule to a row — and the rows move.
PR #132 rewrote the config.prompt and language entries three weeks ago. A rule
that outlives its row is the one failure this file cannot survive: it keeps
firing, keeps citing AGENTS.md, and keeps sounding authoritative while enforcing
a decision the project has reversed. That is strictly worse than the prose it
replaced — prose that no longer reflects the design gets read and ignored, a
gate that no longer reflects the design blocks the change that reflects it.

Each rule now carries a verbatim slice of its "Don't" cell and of its bullet in
the guardrails skill, and --self-test asserts both still resolve. The table
anchor is matched inside the table section only (sed-extracted), so a deleted
row cannot keep its pin alive by being mentioned in passing elsewhere in the
guide; a failed extraction reports itself as a moved heading rather than as
twelve simultaneously-unpinned rules. The skill anchors are deliberately not the
table's wording — the skill says the same things differently, and a pin that
assumed identical text would only be checking that someone had copy-pasted.

This also makes CLAUDE.md's "keep the two in agreement when you change either"
cost one grep instead of a reviewer's memory, for the twelve decisions that are
mechanized. The remaining rows stay a matter of care, and the docs now say which
is which.

The failure message asks for a decision rather than a patch: reworded row,
update the anchor; reversed decision, delete the rule with it.

Verified: deleting a row, rewording a row, dropping a skill bullet, and renaming
the table heading each fail --self-test with the right message; the clean tree
passes; scripts/check.sh --portable is green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TACi7k2226no2tVcR3YJr1
@alexkroman
alexkroman enabled auto-merge August 13, 2026 18:54
claude added 3 commits August 13, 2026 18:56
The tool list was a slash-separated run-on inside a parenthetical that had grown
another item, wrapping mid-clause and reading as a fragment. Same facts, same
tools, split into a list and a second paragraph.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TACi7k2226no2tVcR3YJr1
Two conflicts of substance, both from main's side moving under this branch.

scripts/check.sh: #136 deleted pr-dev-build.yml and corrected the actionlint
comment naming it; this branch had restructured that same block into a
run_check call. Kept both — main's wording, this branch's structure.

check-invariants.sh scoping: #137 moved BLURTENGINE.md to
Sources/BlurtEngine/README.md, and the ENGINE scope was the whole Sources
directory, which had been all Swift until then. The gate promptly flagged two
paragraphs of that document for saying "Do not replace this with a long-lived
AVAudioEngine/installTap graph" — prose about a rule read as a violation of it.
The whole-line-comment filter cannot help there: a Markdown paragraph is not a
comment in any language grep knows.

So the scopes now name file types rather than directories, which is what they
should have said in the first place: these rules are about code, and a directory
holds more than code. It also takes the app scope off the .png and .m4a
resources it had been grepping byte by byte.

The twelve anchors still resolve against main's rewritten AGENTS.md, so none of
the pinned rows moved in #136 or #137. Verified after the merge: --self-test
green, the scan clean, and the scan still bites on an injected AVAudioEngine in
Swift and an injected LSUIElement in Info.plist. scripts/check.sh --portable is
green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TACi7k2226no2tVcR3YJr1
The engine README carried a third copy of the settled-decisions list, after
AGENTS.md's table and the project-guardrails skill. Three copies is two chances
to drift, and this one had no mechanism keeping it honest: the rules pinned by
check-invariants.sh anchor to the AGENTS.md row and the skill bullet, so a
change here would have gone unnoticed by everything.

Nothing unique is lost. Every bullet was already covered — the Swift 6
concurrency rule near-verbatim in AGENTS.md's Conventions, dependency-freedom in
several places, and the rest are rows of the settled-decisions table. The
section stays, pointing at the single copy, because an embedder reading this
guide still needs to know the constraints exist.

The inline rationale under `MicCaptureProtocol` is deliberately kept: explaining
why capture is a fresh AVAudioRecorder per session is part of documenting that
seam, not a duplicate of the list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TACi7k2226no2tVcR3YJr1
@alexkroman
alexkroman added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit b7513b0 Aug 13, 2026
10 checks passed
@alexkroman
alexkroman deleted the claude/agent-repo-dev-tools-e9fdwy branch August 13, 2026 20:01
alexkroman pushed a commit that referenced this pull request Aug 13, 2026
The only conflict was the engine README's "Invariants — don't break these"
list. Both sides edited it: this branch added the CoreAudio dependency rule
and the transcription-steering note to the bullets, while main (#138) deleted
the bullets outright and replaced them with a pointer to AGENTS.md's Settled
decisions table plus the new scripts/check-invariants.sh.

Took main's side. The de-duplication is the point of #138 — check-invariants.sh
pins each mechanized rule to the table row it came from, which only works while
there is exactly one row to pin to. Keeping a second copy of the list here would
re-create the drift that change removed.

The branch's edits survive where they now belong: AGENTS.md:37 and :55 carry the
CoreAudio addition to the allowed-dependency rule, AGENTS.md:488 the steering
note, and .claude/skills/project-guardrails/SKILL.md:100 mirrors both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JU1Ln2MKMsF9PJf1FLMQRX
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants