Skip to content

Commit b7513b0

Browse files
alexkromanclaude
andauthored
Aggregate independent checks to report all failures at once (#138)
* Report every independent check failure, and gate the settled decisions 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 * Pin each invariant rule to the prose it enforces 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 * Tighten the portable-subset paragraph in the check skill 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 * Drop the duplicated invariants list from the engine guide 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e577f19 commit b7513b0

7 files changed

Lines changed: 651 additions & 151 deletions

File tree

.claude/skills/check/SKILL.md

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
name: check
3-
description: Verify the repo is green by running scripts/check.sh — the same full health check CI runs (swift test + coverage gate, sanitizers, xcodegen drift, app build, swift-format/swiftlint/periphery/prettier/markdownlint/shellcheck/shfmt, site deployability, ruff + pytest over evals/). Use before claiming a change builds, passes, or is ready to commit/PR. Bakes in the macOS-only guard so a Linux/web sandbox flags "verify on a Mac" instead of fabricating a green result; there, scripts/check.sh --portable runs the platform-independent subset (docs/site/scripts/workflows).
3+
description: Verify the repo is green by running scripts/check.sh — the same full health check CI runs (swift test + coverage gate, sanitizers, xcodegen drift, app build, swift-format/swiftlint/periphery/prettier/markdownlint/shellcheck/shfmt, site deployability, settled-decision invariants, ruff + pytest over evals/). Its read-only checks report together, so a red run names every failure at the bottom rather than stopping at the first. Use before claiming a change builds, passes, or is ready to commit/PR. Bakes in the macOS-only guard so a Linux/web sandbox flags "verify on a Mac" instead of fabricating a green result; there, scripts/check.sh --portable runs the platform-independent subset (docs/site/scripts/workflows).
44
---
55

66
# check — is this green?
@@ -27,19 +27,20 @@ What you CAN run there is the portable subset:
2727
scripts/check.sh --portable
2828
```
2929

30-
It runs the repo-integrity guards (dependencies, sound catalog, site, shell
31-
portability) then
32-
actionlint / zizmor / prettier / xmllint / markdownlint / shellcheck / shfmt /
33-
ruff (lint + format check) / pytest over `evals/` / `release.test.sh` (plus `swift-format lint` and `swiftlint lint` if Linux
34-
builds are on `PATH` — under the default web network policy they are not).
35-
That fully verifies docs, site, scripts, eval, and workflow changes. It is **not**
36-
"green" in the CI sense: the entire Swift side is skipped, and the closing
37-
line says so. For Swift changes, push and watch `check.yml` instead — its
38-
`compile` job reports a broken test build in ~2 minutes, and `format-patch`
30+
It runs the repo-integrity guards — dependencies, sound catalog, site, shell
31+
portability, and settled decisions — then actionlint, zizmor, prettier, xmllint,
32+
markdownlint, shellcheck, shfmt, ruff (lint + format check), pytest over
33+
`evals/`, and `release.test.sh`. `swift-format lint` and `swiftlint lint` join
34+
them when Linux builds are on `PATH`; under the default web network policy they
35+
are not.
36+
37+
That fully verifies docs, site, scripts, eval, and workflow changes. It is
38+
**not** "green" in the CI sense: the entire Swift side is skipped, and the
39+
closing line says so. For Swift changes, push and watch `check.yml` instead —
40+
its `compile` job reports a broken test build in ~2 minutes, and `format-patch`
3941
publishes the exact `swift-format` reflow as an artifact so you don't have to
40-
reproduce it by hand. In
41-
Claude Code on the web, the `SessionStart` hook installs the portable
42-
linters automatically.
42+
reproduce it by hand. In Claude Code on the web, the `SessionStart` hook
43+
installs the portable linters automatically.
4344

4445
Quick preflight:
4546

@@ -62,6 +63,11 @@ Everything source-only runs first, then everything that builds. That ordering is
6263
deliberate: reversed, a compile error means the cheap checks are
6364
never reached and their findings arrive on the next 11-minute run instead.
6465

66+
The read-only checks (steps 1–5, plus 12) also don't stop each other: each one
67+
runs, failures are tallied, and the run ends with a single `error: N check(s)
68+
failed:` list naming all of them. So one pending `swift-format` reflow no longer
69+
hides every lint finding behind it — expect to fix a batch, not a queue.
70+
6571
1. repo-integrity guards: no external SPM dependencies; sound-catalog
6672
integrity (every `SoundPackCatalog` voice has both cue files, no orphans, no
6773
duplicate or reserved ids); and site integrity (`scripts/check-site.sh`
@@ -70,7 +76,15 @@ never reached and their findings arrive on the next 11-minute run instead.
7076
no unreferenced assets). All run in `--portable` too
7177
2. shell portability (`scripts/check-portability.sh`): GNU-only idioms in
7278
`scripts/*.sh` and `.claude/hooks/*.sh`, which run on BSD userland (Mac, CI)
73-
as well as GNU (Linux sandbox). `--portable` too
79+
as well as GNU (Linux sandbox). Then settled decisions
80+
(`scripts/check-invariants.sh`): the grep-decidable subset of AGENTS.md's
81+
[Settled decisions](../../../AGENTS.md#settled-decisions--dont-reintroduce-these)
82+
table — `AVAudioEngine` capture, a streaming or on-device path, a client-side
83+
cleanup pass, `LSUIElement`, a keystroke-typing injector, the production
84+
Keychain in tests. Each rule is pinned to its table row and its
85+
`project-guardrails` bullet, so editing the prose without revisiting the rule
86+
fails here rather than leaving a gate that enforces a reversed decision. Both
87+
`--portable` too
7488
3. `swift-format lint --strict`, then `swiftlint lint --strict` (warnings are
7589
failures) — both source-only
7690
4. actionlint / zizmor (workflow security) / prettier / xmllint / markdownlint /
@@ -102,6 +116,9 @@ never reached and their findings arrive on the next 11-minute run instead.
102116
- **Any non-zero exit** → not green. Report the failing step and its output
103117
verbatim; do not soften ("mostly passes") or claim success. Fix, then re-run
104118
the _full_ `check.sh` — a single-file `swift test --filter` is not green.
119+
A red run reports every independent failure it found, so read the closing
120+
`error: N check(s) failed:` list and fix all of them before re-running —
121+
stopping at the first one wastes the aggregation.
105122
- A `note: <tool> not installed; skipping` line means coverage of that check is
106123
_missing_, not satisfied. On a dev Mac, run `scripts/bootstrap.sh` to install
107124
the toolchain rather than accepting skips.

.claude/skills/project-guardrails/SKILL.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@ These are settled decisions. Don't reintroduce them; if a task seems to require
1010
one, stop and ask the user first. This is the fast "don't" list; AGENTS.md's
1111
"Settled decisions" table is the fuller reference and the source of truth.
1212

13+
Many of these are also enforced mechanically — `scripts/check-invariants.sh`
14+
(run by `check.sh`, including `--portable`) greps for the constructs that give
15+
each one away, so reintroducing one fails the health check rather than depending
16+
on this list being read. Those rules also pin a verbatim slice of the bullet
17+
they come from in this file, so rewording or deleting one fails the gate until
18+
someone decides whether the rule survives the edit — edit these entries
19+
knowing that, and fix the anchor in the same change. Treat that as a backstop, not the boundary: the rules
20+
it can't express are still here, still binding, and the reasons in this file are
21+
what let you tell an intended exception from a mistake. Never silence a finding
22+
with `// invariant-ok:` to get a build green — that marker is for a line that is
23+
genuinely correct, and reaching for it means it's time to stop and ask.
24+
1325
## Audio
1426

1527
- **No `AVAudioEngine` / `installTap` capture path.** `MicCapture` uses

AGENTS.md

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ App/Blurt/
6969
Shared/ UITestIdentifiers.swift — compiled into BOTH app and UI-test targets
7070
BlurtUITests/ XCUITest bundle (see Tests)
7171
Tests/BlurtEngineTests/ Swift Testing suites; Stubs/ holds the seam doubles
72-
scripts/ check.sh, check-site.sh, bootstrap.sh, dev-build.sh, uitest.sh, leaks.sh,
73-
release*.sh
72+
scripts/ check.sh, check-site.sh, check-portability.sh, check-invariants.sh,
73+
bootstrap.sh, dev-build.sh, uitest.sh, leaks.sh, release*.sh
7474
hand-run maintainer tools — no automated caller, invoked by a
7575
human, so "nothing references it" here does NOT mean dead code:
7676
serve-site.sh (preview site/ locally), screenshot.swift +
@@ -327,6 +327,24 @@ In Claude Code on the web, a `SessionStart` hook installs the portable linters a
327327
Each was tried the other way and reverted. If a task seems to require one, stop and ask first.
328328
(`.claude/skills/project-guardrails` is the compressed version of this list.)
329329

330+
`scripts/check-invariants.sh` mechanizes the subset of this table a regex can decide, so those
331+
entries fail `check.sh` rather than relying on a reviewer's memory: `AVAudioEngine`/`installTap`
332+
capture, a streaming or on-device path, a client-side cleanup pass (`StylerProtocol`, an LLM
333+
Gateway/LeMUR client), `config.language_code`, `config.prompt`, a keystroke-typing injector,
334+
`LSUIElement`, a `KeyboardShortcuts` import, a self-replacing updater, the production Keychain
335+
under `Tests/`, and `@available(*, deprecated)` shims. The SPM-dependency row has its own guard in
336+
`check.sh` (it parses `project.yml`'s `packages:` block), and the `.pbxproj` row is covered by the
337+
xcodegen drift check plus a Claude `PreToolUse` hook. The rest — the filler-word clause, the
338+
context-widening rule, the `/Applications` install path — stay prose, because no pattern separates
339+
them from correct code that reads the same. Add a rule when you find one that can.
340+
341+
**Editing a mechanized row breaks the build on purpose.** Each of those rules pins a verbatim slice
342+
of its row here (and of its bullet in the guardrails skill), so deleting or rewording one fails
343+
`check-invariants.sh --self-test`. That is the intended behaviour, not an obstacle to route around:
344+
a rule that outlives its row keeps firing and keeps citing this table while enforcing something the
345+
project has already reversed. Reworded the row? Update the anchor. Reversed the decision? Delete the
346+
rule along with the row.
347+
330348
| Don't | Because |
331349
| ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
332350
| Add an external SPM dependency to the engine | Dependency-free by rule (biggest supply-chain risk); a `check.sh` guard fails on `.package(` in `Package.swift` or a `url:`/`github:` package in `project.yml`. Extend `BlurtEngine` instead. |

CLAUDE.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ globs go in one invocation so `hook-lib.sh` and `release-lib.sh` are in the inpu
4848
portable subset. Load it before claiming a change builds or passes.
4949
- **`project-guardrails`** — the compressed "don't do this" list. `AGENTS.md`'s
5050
[Settled decisions](./AGENTS.md#settled-decisions--dont-reintroduce-these) table is the fuller
51-
reference and the source of truth; keep the two in agreement when you change either.
51+
reference and the source of truth; keep the two in agreement when you change either. For the
52+
twelve decisions `scripts/check-invariants.sh` mechanizes, that agreement is no longer on your
53+
honour: each rule pins a verbatim slice of its table row _and_ its skill bullet, so deleting or
54+
rewording either fails the gate's `--self-test` until someone decides whether the rule survives
55+
the edit. The rest of the list is still yours to keep in sync.
5256
- **`release`** — the ship pipeline (build → sign → notarize → staple → DMG → GitHub). User-invoked
5357
only: it has real, hard-to-undo side effects.
5458
- **`launch-metrics`** — read-only launch KPI snapshot from the GitHub API.

Sources/BlurtEngine/README.md

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,4 @@ Two ways out, neither taken yet: drop it from the public product (its own target
280280

281281
## Invariants — don't break these
282282

283-
Each of these was tried the other way and reverted; the longer stories are in [AGENTS.md](../../AGENTS.md) and the source comments:
284-
285-
- **No external SPM dependencies in the engine.** Foundation/Security/AVFoundation only.
286-
- **No streaming STT, no local models, no client-side LLM cleanup pass.** One dictation request per utterance is the architecture; the cleanup rewrite is server-side (the request's `llm` block), and transcription steering belongs in `ConversationContext`.
287-
- **No `AVAudioEngine`/`installTap` capture path.** Fresh `AVAudioRecorder` per session, resolved at record time.
288-
- **Paste is always clipboard-based** (save → write → ⌘V → settle → restore), with the copied-to-clipboard degradation for lost targets.
289-
- **No English-pinning or filler-word clauses**, no reviving `config.prompt`, and no widening the conversation context past the recent dictations and the prior chunk — the app name, window title, field label and selected text stay on the machine. Key terms go on `config.word_boost`, never back into the context turns.
290-
- **Actors own state** (`DictationSession`, `KeyInjector`, `MicCapture`); the stateless API client stays a `Sendable` struct. Keep new code Swift 6 strict-concurrency clean.
283+
Each was tried the other way and reverted, and they bind engine code as much as the app's. The list is deliberately not repeated here: it lives once, in [AGENTS.md's Settled decisions](../../AGENTS.md#settled-decisions--dont-reintroduce-these) table, alongside the engine conventions those rules rest on (dependency-free by rule; actors own state; the stateless API client stays a `Sendable` struct; new code Swift 6 strict-concurrency clean). `scripts/check-invariants.sh` mechanizes the subset a regex can decide and fails `check.sh` on them, so a good number are enforced rather than remembered — and it pins each rule to the table row it came from, which only works while there is one row to pin to.

0 commit comments

Comments
 (0)