feat(dogfood): sweep every shipped interface with the deterministic toolchain, and ban hand-rolled argv parsing - #2524
Closed
noahgift wants to merge 2 commits into
Closed
feat(dogfood): sweep every shipped interface with the deterministic toolchain, and ban hand-rolled argv parsing#2524noahgift wants to merge 2 commits into
noahgift wants to merge 2 commits into
Conversation
…ommands, 34 routes, 9 MCP tools — deterministically
WHAT WAS MISSING
Neither existing skill covered the shipped surface. Measured before writing this:
.claude/skills/apr-dogfood/SKILL.md (828 lines)
references 26 distinct `apr` subcommands out of 103
occurrences of mcp / MCP / /v1/ / curl / endpoint / route / chat-completions: 0
.claude/skills/pre-release/SKILL.md
same: 0
The 0.63.0 audit that probed 104 CLI commands, 9 MCP tools and 45 routes was
done BY HAND. It was never reproducible.
scripts/dogfood_surfaces.sh covers all three interface kinds across every
binary the workspace builds, and its receipt is byte-identical across runs.
ENUMERATED AT RUNTIME, NEVER FROM A LIST
binaries cargo build --message-format=json (executables cargo REPORTS)
apr subcommands apr --help
HTTP routes the ("GET","/path",handler) table in api/router.rs
MCP tools const NAME in aprender-mcp/src/tools/ + the contract
A written-down list is the defect this repo keeps finding: the falsification
spec asserts "exactly 36 top-level commands" and now finds 0 because the enum
moved file; CLAUDE.md has claimed 77, 103 and 111. Grepping source is no better
-- a regex over clap Subcommand enums reports 0 subcommands for `simular`, which
IS a clap-derive CLI. Only the binary knows what the binary accepts.
Every enumeration is vacuity-guarded: too few items FAILS, because a sweep over
a shrunken universe otherwise reports a clean pass.
A PASS MUST EXCLUDE AN OUTCOME
`--help` exiting 0 is not a pass -- a binary that prints nothing also exits 0.
Each binary must ALSO reject an unknown flag, which is what catches a parser
that is not parsing. The 0.63.0 audit found tests asserting is_ok() on invalid
input; those lock the defect in.
Skips are counted, never silent, and a run skipping more than MAX_SKIP_PCT
FAILS -- the require_model! defect, where 30 call sites `return` early and
report ok.
TWO BUGS IN THIS SCRIPT, FOUND AND FIXED WHILE WRITING IT
1. It parsed `apr --help` with `^[[:space:]]+[a-z]`, which scraped WRAPPED
DESCRIPTION lines: `apr yet)`, `apr clip.wav`, `apr existing` were all
reported as subcommands and the count came out 114 against a real 105. clap
indents a subcommand by exactly two spaces; descriptions wrap far deeper.
2. It built binary paths from `cargo metadata`'s target_directory. In a worktree
that reports /mnt/nvme-raid0/targets/aprender while cargo actually writes to
<worktree>/target/debug -- .cargo/config.toml holds the redirect and is
gitignored, so it exists in the main checkout and not in a worktree. The
script was probing binaries built from a DIFFERENT TREE. Now it asks cargo
which executables it produced. This is the repo's own binary-pinning
doctrine, and the first version violated it.
Its route enumeration was also wrong once: globbing every "/..." string literal
reported 284 routes. It reads the route table now, and gets 34.
WHAT THE FIRST RUN FOUND (all confirmed by hand)
aprender-train-lora PANICKED on any argument
trueno-zram PANICKED on any argument
Both declared a short option twice -- `-m` for `model` AND `method`, `-p` for
`pages` AND `pattern`. clap's check is #[cfg(debug_assertions)], so RELEASE
builds do not panic; they ship the ambiguity. Verified on a release build:
$ aprender-train-lora plan --help
-m, --model <MODEL> Model size in parameters ...
-m, --method <METHOD> Fine-tuning method ...
Two arguments claiming one short flag, in the binary `cargo install` produces.
Fixed by making the colliding argument long-only in each; the short was never
usable, and `-m`/`-p` now bind unambiguously.
STILL RED, deliberately left for a decision (they are in the receipt):
aprender-compute-xtask --help exits 1 (hand-rolled env::args() parsing)
aprender-zram-generator --help produces 0 bytes, and accepts an unknown flag
at exit 0 -- it is a systemd generator taking normal_dir/early_dir/late_dir
positionally, so an unrecognised flag is treated as a DIRECTORY PATH
VERIFICATION
--self-test 3/3, including the row where a permissive CLI is CAUGHT
--twice byte-identical receipts
full sweep pass=198 fail=3 skip=1 (skip 0%)
bash -n rc=0
bashrs reports 7 errors, all the documented false-positive classes on embedded
python/awk (SC1078 x4, SC1028, SC1035, SC2296); `bash -n` is clean. NOTE for
sequencing: scripts/check_shell_lint_ratchet.sh (#2511, not yet on main)
baselines the repo-wide bashrs error count, so it will need a re-baseline when
both land.
Refs #2503
…oolchain, and ban hand-rolled argv parsing
WHAT WAS MISSING
Neither existing skill covered the shipped surface. Measured before writing this:
.claude/skills/apr-dogfood/SKILL.md (828 lines)
references 26 distinct `apr` subcommands out of 103
occurrences of mcp / MCP / /v1/ / curl / endpoint / route: 0
.claude/skills/pre-release/SKILL.md
same: 0
The 0.63.0 audit that probed 104 CLI commands, 9 MCP tools and 45 routes was
done BY HAND and was never reproducible.
scripts/dogfood_surfaces.sh covers all three interface kinds across every binary
the workspace builds, and its receipt is byte-identical across runs.
pass=209 fail=0 skip=1 (skip 0%) rc=0
--twice: DETERMINISTIC, byte-identical receipts
IT USES THE DETERMINISTIC TOOLCHAIN, IT DOES NOT REIMPLEMENT IT
pv contract validation (never yq, never a python YAML walk)
bashrs shell quality (never shellcheck)
probar endpoint testing (never a hand-rolled curl loop)
pmat code search / quality (never grep for discovery)
Each is asserted PRESENT with its version rather than skipped-if-missing: a
sweep that silently drops its verification tools reports a clean pass having
checked less, which is the vacuous-scan defect the script exists to avoid.
The first draft violated this. It parsed contracts/apr-mcp-tool-schemas-v1.yaml
with python and counted `tools:` entries by hand -- muda by CLAUDE.md's explicit
rule, AND redundant, because FALSIFY-MCP-008 already asserts byte-identity
between the codegen constants and the live tools/list response at four layers.
Reimplementing a weaker version of an existing falsifier is the opposite of
dogfooding. It is `pv validate` now, plus `pv lint contracts/` over the whole
directory. The live endpoint probe runs `probar llm test`, not curl. The script
holds itself to the rule it enforces: it bashrs-lints its own source.
ENUMERATED AT RUNTIME, NEVER FROM A LIST
binaries cargo build --message-format=json (executables cargo REPORTS)
apr subcommands apr --help
HTTP routes the ("GET","/path",handler) table in api/router.rs
MCP tools const NAME in aprender-mcp/src/tools/
A written-down list is the defect this repo keeps finding: the falsification
spec asserts "exactly 36 top-level commands" and now finds 0 because the enum
moved file; CLAUDE.md has claimed 77, 103 and 111. Grepping source is no better
-- a regex over clap Subcommand enums reports 0 subcommands for `simular`, which
IS a clap-derive CLI. Only the binary knows what the binary accepts. Every
enumeration is vacuity-guarded: too few items FAILS.
A PASS MUST EXCLUDE AN OUTCOME
`--help` exiting 0 is not a pass -- a binary that prints nothing also exits 0.
Each binary must ALSO reject an unknown flag, which catches a parser that is not
parsing. Skips are counted, never silent, and a run skipping more than
MAX_SKIP_PCT FAILS.
HAND-ROLLED PARSERS: FIXED AND BANNED
scripts/check_no_hand_rolled_parsers.sh bans the CONSTRUCT, structurally and
ratcheted. It is complementary to the behavioural probe: the probe catches
today's broken parsers, the ban stops one returning. Self-test 3/3, including
two false-positive controls (a clap CLI that also calls env::args() must NOT be
flagged).
Four were hand-rolled; this converts three to clap derive (aprender-ptx-debug is
#2520). Baseline 4 -> 1.
aprender-compute-xtask --help exited 1
aprender-qa-certify apr-qa-readme-sync
aprender-zram-generator --help printed 0 BYTES and an unknown flag was
ACCEPTED at exit 0 -- so a typo'd flag was treated
as one of its DIRECTORY arguments. It is a systemd
generator; the three positional dirs are preserved
exactly, and --help now explains the protocol.
WHAT THE FIRST RUN FOUND
aprender-train-lora PANICKED on any argument
trueno-zram PANICKED on any argument
Both declared a short option twice -- `-m` for `model` AND `method`, `-p` for
`pages` AND `pattern`. clap's check is #[cfg(debug_assertions)], so RELEASE
builds do not panic; they ship the ambiguity. Verified on a release build,
`aprender-train-lora plan --help` listed BOTH `-m, --model` and `-m, --method`.
Fixed by making the colliding argument long-only in each.
TWO BUGS IN THIS SCRIPT, FOUND AND FIXED WHILE WRITING IT
1. It parsed `apr --help` with `^[[:space:]]+[a-z]`, scraping WRAPPED
DESCRIPTION lines: `apr yet)`, `apr clip.wav`, `apr existing` were reported
as subcommands and the count read 114 against a real 105.
2. It built binary paths from `cargo metadata`'s target_directory. In a worktree
that reports /mnt/nvme-raid0/targets/aprender while cargo writes to
<worktree>/target/debug -- .cargo/config.toml holds the redirect and is
gitignored. The script was probing binaries built from a DIFFERENT TREE. It
asks cargo now. The repo's own binary-pinning doctrine, violated by the first
draft.
Route enumeration was wrong once too: globbing every "/..." string literal
reported 284 routes. Reading the route table gives 34.
VERIFICATION
dogfood --self-test 3/3 (incl. permissive-CLI caught)
hand-rolled ban --self-test 3/3 (incl. 2 false-positive controls)
dogfood --twice byte-identical receipts
full sweep pass=209 fail=0 skip=1, rc=0
pv lint contracts/ 0 errors
cargo test (3 converted crates) 226 passed, 0 failed
cargo clippy --all-targets 0 errors
cargo fmt --all --check rc=0
NOTE for sequencing: scripts/check_shell_lint_ratchet.sh (#2511, not yet on
main) baselines the repo-wide bashrs error count; these two new scripts add 13
(all documented false-positive classes on embedded python/awk -- `bash -n` is
clean on both), so it needs a re-baseline when both land.
Refs #2503
Contributor
Author
|
Superseded by #2527, which consolidates this with the other guard PRs. All four inserted into the same |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was missing
Neither existing skill covered the shipped surface. Measured before writing this:
apr-dogfood(828 lines)pre-releasemcp/MCP/v1/,curl,endpoint,routeaprsubcommands referencedThe 0.63.0 audit that probed 104 CLI commands, 9 MCP tools and 45 routes was done by hand. It was never reproducible.
It uses the deterministic toolchain — it does not reimplement it
Each is asserted present with its version, not skipped-if-missing — a sweep that silently drops its verification tools reports a clean pass having checked less.
The first draft violated this. It parsed
contracts/apr-mcp-tool-schemas-v1.yamlwith python and countedtools:entries by hand — muda by CLAUDE.md's explicit rule, and redundant, because FALSIFY-MCP-008 already asserts byte-identity between the codegen constants and the livetools/listresponse at four layers. Reimplementing a weaker version of an existing falsifier is the opposite of dogfooding. It'spv validatenow, pluspv lint contracts/. The live endpoint probe runsprobar llm test. The script bashrs-lints its own source.Enumerated at runtime, never from a list
cargo build --message-format=jsonaprsubcommandsapr --help("GET","/path",handler)tableconst NAMEinaprender-mcp/src/tools/A written-down list is the defect this repo keeps finding — the falsification spec asserts "exactly 36 top-level commands" and finds 0 because the enum moved file; CLAUDE.md has claimed 77, 103, and 111. Grepping source is no better: a regex over clap
Subcommandenums reports 0 subcommands forsimular, which is a clap-derive CLI. Every enumeration is vacuity-guarded.A pass must exclude an outcome.
--helpexiting 0 isn't a pass — a binary printing nothing also exits 0. Each binary must also reject an unknown flag.Hand-rolled parsers: fixed and banned
check_no_hand_rolled_parsers.shbans the construct, structurally and ratcheted — complementary to the behavioural probe. Self-test 3/3 including two false-positive controls (a clap CLI that also callsenv::args()must not be flagged).Four were hand-rolled; three converted here (
aprender-ptx-debugis #2520). Baseline 4 → 1.aprender-zram-generatorwas the sharp one:--helpprinted 0 bytes and an unknown flag was accepted at exit 0 — so a typo'd flag was treated as one of its directory arguments. It's a systemd generator; the three positional dirs are preserved exactly and--helpnow explains the protocol.What the first run found
aprender-train-loraandtrueno-zrampanicked on any argument. Each declared a short option twice —-mfor bothmodelandmethod,-pfor bothpagesandpattern. clap's check is#[cfg(debug_assertions)], so release builds don't panic — they ship the ambiguity. Verified on a release build,plan --helplisted both-m, --modeland-m, --method.Two bugs in this script, found while writing it
apr --helpwith loose indentation, scraping wrapped description lines —apr yet),apr clip.wavcounted as subcommands; 114 vs a real 105.cargo metadata'starget_directory. In a worktree that reports/mnt/nvme-raid0/...while cargo writes to<worktree>/target/debug—.cargo/config.tomlholds the redirect and is gitignored. It was probing binaries from a different tree — the repo's own binary-pinning doctrine, violated by my first draft. It asks cargo now.Route enumeration was wrong once too: globbing every
"/..."literal gave 284; reading the table gives 34.Verification
--self-test--self-test--twicepv lint contracts/cargo test(3 converted crates)--all-targetscargo fmt --checkSequencing note:
check_shell_lint_ratchet.sh(#2511, not yet on main) baselines the repo-wide bashrs count; these two scripts add 13, all documented false-positive classes on embedded python/awk —bash -nis clean on both. Needs a re-baseline when both land.Refs #2503