test(knobs): find a stray number by shape, not by its name #876

Merged

coilysiren merged 1 commit from aos/claude/hs68-knob-guard into main

2026-08-17 02:32:18 +00:00

coilyco-ops commented

2026-08-17 02:29:39 +00:00

Member

The sweep in #835 put every drifted number back in internal/community/config.go and left TestEveryTuningNumberLivesInConfigGo behind them. This closes the gap in that guard, which is the half of #829 that was not done.

The problem

The guard matched a candidate by its name, against ^(max|min|default)[A-Z] and nine suffixes. It reported a stray only when the author happened to spell it one of those ways.

That failed twice on the very sweep it was written for. #835's own comment records four of seven numbers found only after its regex was widened. And after the widening it was still silent on one:

// commandmcps.go, before this branch
const mcpsReplyBudget = 1800

A send budget under Discord's 1990-rune interaction bound. A cap by AGENTS.md's own list, outside config.go, settable by nobody, and named neither max* nor any of the nine.

A pattern that gets widened every time someone names a number a new way is not holding a line. It is describing the names already used.

The change

The guard parses the package with go/ast and reports every package-level numeric const and var outside config.go. A number is a stray unless elsewhereByDesign names it with a reason. A local inside a function body still never reaches it, which is why workers := 1 in jobrunner.go is not swept up.

Verified against the three shapes the old regex could not see, by dropping a probe file into the package and running the test:

zzstray_probe.go:5 oddlyNamedBudget      // no max/min/default, no matching suffix
zzstray_probe.go:7 typedStray            // `var typedStray int = 42`
zzstray_probe.go:9 octalStray            // `const octalStray = 0o755`

All three reported. All three were invisible before. The probe was removed.

mcpsReplyBudget moves into the table as SIRENS_ECHO_MCPS_REPLY_BUDGET, defaulting to 1800, and docs/sirens-echo-knobs.md regenerates through just knobs.

The exemptions, and the trade

Inverting the default costs a written reason for every genuine non-knob. There are seven, in three families:

Floors that decide what something is. minNormalizedIDDigits, minEncodedGuardBytes, opaqueSecretRunes. Lowering one changes what counts as an identifier or a credential, not how much of a match is allowed.
File modes. scratchPermissions, scratchFilePermissions, workspacePermissions. Text-only is enforced by denying the execute bit, so a deployment able to grant it could undo the property.
A sentinel. unboundedReply is the absence of a ceiling rather than one.

The first two families were already correct; three of them survived on naming luck rather than on a recorded decision. Now they have a sentence a reviewer can disagree with, which is the deliberate half of the trade: an exemption is something someone wrote, where a pattern miss is silence that reads exactly like a pass.

Docs

docs/sirens-echo-knob-guard.md is new and carries the reasoning. docs/sirens-echo-tuning.md points at it and stays under its line cap. AGENTS.md's Where numbers live gains one sentence, since that is the file agents actually read.

No docs/FEATURES.md entry: this is validation hardening, which that file's own rule excludes.

Not in scope

Step 2 of #361, the collapse. It stays consult and Kai's, and it is easier to judge now that the inventory cannot be quietly incomplete.

just gate passes: build, policy-check, vet, test, test-skips, pre-commit.

closes #829

The sweep in #835 put every drifted number back in `internal/community/config.go` and left `TestEveryTuningNumberLivesInConfigGo` behind them. This closes the gap in that guard, which is the half of #829 that was not done. ## The problem The guard matched a candidate by its **name**, against `^(max|min|default)[A-Z]` and nine suffixes. It reported a stray only when the author happened to spell it one of those ways. That failed twice on the very sweep it was written for. #835's own comment records four of seven numbers found only after its regex was widened. And after the widening it was still silent on one: ```go // commandmcps.go, before this branch const mcpsReplyBudget = 1800 ``` A send budget under Discord's 1990-rune interaction bound. A cap by AGENTS.md's own list, outside `config.go`, settable by nobody, and named neither `max*` nor any of the nine. A pattern that gets widened every time someone names a number a new way is not holding a line. It is describing the names already used. ## The change The guard parses the package with `go/ast` and reports every package-level numeric `const` and `var` outside `config.go`. A number is a stray unless `elsewhereByDesign` names it with a reason. A local inside a function body still never reaches it, which is why `workers := 1` in `jobrunner.go` is not swept up. Verified against the three shapes the old regex could not see, by dropping a probe file into the package and running the test: ``` zzstray_probe.go:5 oddlyNamedBudget // no max/min/default, no matching suffix zzstray_probe.go:7 typedStray // `var typedStray int = 42` zzstray_probe.go:9 octalStray // `const octalStray = 0o755` ``` All three reported. All three were invisible before. The probe was removed. `mcpsReplyBudget` moves into the table as `SIRENS_ECHO_MCPS_REPLY_BUDGET`, defaulting to 1800, and `docs/sirens-echo-knobs.md` regenerates through `just knobs`. ## The exemptions, and the trade Inverting the default costs a written reason for every genuine non-knob. There are seven, in three families: - **Floors that decide what something *is*.** `minNormalizedIDDigits`, `minEncodedGuardBytes`, `opaqueSecretRunes`. Lowering one changes what counts as an identifier or a credential, not how much of a match is allowed. - **File modes.** `scratchPermissions`, `scratchFilePermissions`, `workspacePermissions`. Text-only is enforced by denying the execute bit, so a deployment able to grant it could undo the property. - **A sentinel.** `unboundedReply` is the absence of a ceiling rather than one. The first two families were already correct; three of them survived on naming luck rather than on a recorded decision. Now they have a sentence a reviewer can disagree with, which is the deliberate half of the trade: an exemption is something someone wrote, where a pattern miss is silence that reads exactly like a pass. ## Docs `docs/sirens-echo-knob-guard.md` is new and carries the reasoning. `docs/sirens-echo-tuning.md` points at it and stays under its line cap. `AGENTS.md`'s **Where numbers live** gains one sentence, since that is the file agents actually read. No `docs/FEATURES.md` entry: this is validation hardening, which that file's own rule excludes. ## Not in scope Step 2 of #361, the collapse. It stays `consult` and Kai's, and it is easier to judge now that the inventory cannot be quietly incomplete. `just gate` passes: build, policy-check, vet, test, test-skips, pre-commit. closes #829

coilyco-ops added 1 commit

2026-08-17 02:29:39 +00:00

test(knobs): find a stray number by shape, not by its name

ci / test (pull_request) Successful in 47s

Details

ci / publish-echo-image (pull_request) Has been skipped

Details

ci / publish-observed (pull_request) Has been skipped

Details

ci / image-build (pull_request) Successful in 25s

Details

03a57aca90

The guard matched a candidate against `^(max|min|default)[A-Z]` and nine
suffixes, so it reported a stray only when the author happened to spell it that
way. It missed four of the seven numbers the sweep it was written for
recovered, and it stayed silent on `mcpsReplyBudget` after that pattern was
widened, because a send budget is neither shape.

It now parses the package and reports every package-level numeric `const` and
`var` outside `config.go`, exempt only when `elsewhereByDesign` names it with a
reason. A local inside a function still never reaches it.

`mcpsReplyBudget` moves into the table as `SIRENS_ECHO_MCPS_REPLY_BUDGET`. The
five numbers that are correctly outside `config.go` get their reason written
down: three floors that decide what counts as an identifier or a credential,
three file modes that enforce text-only by denying the execute bit, and one
sentinel for a transport with no ceiling.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

coilyco-ops requested review from coilysiren

2026-08-17 02:29:39 +00:00

coilyco-ops referenced this pull request

2026-08-17 02:29:58 +00:00

Do a sweep to make sure every numeric configuration is in internal/community/config.go, with the classic env var override, update READMEmd to point at that file #829