e9f73273281fac5307e17a0d31c6be6350016ee7
60 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3d1430110c |
Narrow the fmt gate's exit contract to what it actually maintains
ChatGPT Codex on PR #159: both formatter invocations end `|| rc=1`, so any failure after the rustfmt probe was reported as "misformatted" although no verdict was produced. Correct, and the narrower form of the bug this gate exists for. Measured: an unparseable source exits 1, identical to a diff; an unknown rustfmt.toml option is only a warning (exit 0). Narrowed rather than re-classified, because telling a diff from a parse error means parsing rustfmt's prose, which is the check-adjacent-to-its-subject this repo keeps getting wrong. The line the split needs is "did the gate run": exit 1 names a file and blames the tree; exit 2 names none and blames the invocation, and every exit-2 cause is checked above the loop. Two more rivals injected: deleting the `cargo fmt --version` probe, and turning `|| rc=1` into `|| return 2`. The first exposed a vacuous test -- a cargo stub that failed for every argument also breaks `cargo metadata`, so the run exited 2 on the metadata arm and the probe could be deleted with the suite green. The stub now delegates every subcommand but `fmt`, which is the shape a missing rustfmt component actually has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
cb3afb559e |
Split a fmt usage error from a fmt verdict across both workspace roots
improvise-b4ks. From the repo root `cargo fmt -p opfs-contract -- --check`
exits 1 with "package `opfs-contract` is not a member of the workspace" and
never runs --check, so a caller reading $? cannot tell "misformatted" from
"nothing was checked". Reproduced at
|
||
|
|
cb6866b2c6 |
fix(scripts): one shell and one environment for every checker (improvise-f9qf)
`scripts/pr-reviews-test.sh` passed 19/19 locally and 2/19 in CI on the same commit, because a multi-assignment `local` is not left-to-right on bash 5 while macOS's bash 3.2 tolerates it. The obvious half of the fix is pinning the shell. The half that decides the design is not. .envrc DOES already put a nix bash 5.3.15 on PATH, which is CI's semantics -- and it is not enough. `pkgs.mkShell` exports `name`, `out`, `outputs`, `system`, `shell`, `builder`, `stdenv`, `phases` and about twenty more generically-named variables that CI does not have, so a script reading one by accident under `set -u` is SILENT in the dev shell and fatal in CI. That is exactly what happened: the broken clause read `$name`, and in the dev shell `$name` was `nix-shell-env`. Measured at 051f541^ on one machine: macOS /bin/bash 3.2 19 passed, 0 failed direnv exec . ./scripts/pr-reviews-test.sh 19 passed, 0 failed the same, with `name` unset 2 passed, 17 failed Two reasons for green, one of them the correct interpreter. A dev shell is a third environment, not a reproduction of CI. A nix shebang was measured and declined, on availability rather than speed: warm, `nix shell nixpkgs#bash --command true` is 61.8ms against an ambient 5.1ms, and `nix-shell -p bash --run true` -- the manpage form -- is 675.2ms, 132x, inside git's merge machinery for beads-merge-driver.sh. But only the `kb` job installs nix; build, test, wasm, js, harness-lint and browser have none, so a nix shebang needs an installer step on three more jobs to reach a bash those runners already ship. zsh is not a shebang change and the runner does not have it either. Both declined in context/code/tooling.md section 5. What lands instead pins interpreter and environment together, in one place: scripts/checkers.sh is the one entry point for every check_the_checker, used verbatim by CI and by the local gate. It pins a bash >= 5 (ambient first, nix only when the ambient one is older, exit 3 when neither), scrubs the environment to an allowlist, and refuses any shell shebang in scripts/ that is not `#!/usr/bin/env bash`. The pinned bash goes on the front of PATH so inner invocations get it too -- which is where the fault actually lived. The second undetected split is closed with it: the four `#!/bin/sh` and one `#!/usr/bin/env sh` scripts are now bash, and the six places that ran a script through an explicit `sh` -- overriding the callee's own shebang, which is the same disease -- now invoke it directly. Verification, injected against the real script: restoring pr-reviews' multi-assignment `local` leaves `sh scripts/pr-reviews-test.sh` at 22/22 and `direnv exec . ./scripts/pr-reviews-test.sh` at 22/22, while `./scripts/checkers.sh pr-reviews` reports 2 passed, 20 failed, exit 1 -- matching the 2/19 CI actually reported. scripts/checkers-test.sh is twelve cases, each injecting rather than asserting; the pair that carries it runs one fixture both ways, so the `set -u` case is a test of the runner and not of `set -u`. The old local gate was `for t in scripts/*-test.sh; do sh "$t"; done`, which overrides every shebang with /bin/sh -- so it ran every suite under bash 3.2 in sh-mode regardless of what the file said. It is replaced by `./scripts/checkers.sh`. Workspace suite unchanged at 1689 passing over 38 binaries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
051f54111b |
fix(scripts): a local is not left-to-right on bash 5, and --paginate emits one array per page
Both found by ChatGPT Codex on PR #133, both invisible on macOS, both arriving as exit 2 with no output -- jq's answer to an invalid --argjson, so a shell fault at the top of the script wears jq's exit code. `local n="$1" name="$2" f="$DIR/$n.$name.json"` creates every name before running the assignments on bash 5, so $n is unbound in the third clause and set -u kills the shell. The dead command substitution then hands --argjson an empty string. Works on macOS bash 3.2: the suite passed 19/19 locally and 2/19 in CI on the same commit. Each local now gets its own statement. `gh api --paginate` emits one JSON array PER PAGE, so a comment thread past the page size is several concatenated documents that --argjson rejects the same way. Both fetch paths now flatten through one pages_to_array (`jq -s 'add // []'`), which is a no-op on a single page. The fixture path shares that helper, which is the only reason a fixture can test it: a fixture holding a single array would exercise a shape the live fetch never produces. Three cases added, and removing the helper from the fixture path fails exactly those three, the wait case with the live exit-2 signature. Suite is 22 cases. No PR in this repo has yet had enough inline comments to reach a second page, so nothing but the test will see that one before a user does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
a47310d50f |
fix(scripts): pr-reviews reads all three review surfaces (improvise-xgra)
It printed review BODIES only and was blind to inline review comments -- the surface a human reviewer is most likely to use. On PR #122 the user reviewed with the body "Generally good, added comments inline." and five substantive rulings underneath it; pr-reviews printed the one-liner, and an agent reported the PR as awaiting review while the answers sat on it. pulls/{n}/comments and issues/{n}/comments join pulls/{n}/reviews, all three interleaved in time order, each inline comment carrying its path and line (falling back to original_line, which is what an outdated comment has left). The exit contract is unchanged: 0 means something exists against the current head, 1 means the wait elapsed, 4 is usage. An inline comment at the head now satisfies --wait -- it carries a commit_id and is exactly the case that failed. An issue-level comment does not: it names no commit and so makes no claim about which tree was read. The usage's exit-code warning is widened from "do not pipe into head/tail" to the real rule: nothing may follow the script in the same `;` list either, because a trailing `echo "exit=$?"` makes the status the echo's. scripts/pr-reviews-test.sh is the check_the_checker, fixture-driven and needing no network, wired into CI's kb job beside pr-watch-test.sh and release-preflight-test.sh. Five faults were injected and watched to fail; context/test-evidence/tooling.md records which case caught each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
56632430db | Merge pull request #122 from fiddlerwoaroof/docs/adr-004-faces | ||
|
|
6034a216e1 |
docs(site): publish the two new context docs, and de-number two stale counts
The `build` job failed, correctly: `scripts/build-doc-site.mjs` requires every `context/**/*.md` to be either in the SECTIONS manifest or a named omission (improvise-res9's cure), and the two files this branch added were neither. Both are now published, beside the TUI in each section. While there: `context/code/doc-site.md` asserted 'the seven `context/code/*.md` documents' when there were already twelve, and '501 entries' when the checker reports 801. Both were stale before this branch and this branch makes them staler, so they are de-numbered rather than re-asserted — a count in prose that nothing checks is a fact with no owner, and the shape that stops drifting is not having one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
9191718612 |
docs(adr): answer the Codex review — Indexed is a fourth arm, selection is per-surface
Three findings, all correct, all applied. 1. A contextual selection face cannot be resolved by `resolve(Face)`. Clause 3 fixed that signature while the vocabulary described one `selection` face "parameterised by the surface's accent" — the same Face cannot yield the grid's Cyan and the formula panel's Yellow. Resolved the way clause 2 already works: `selection` is a BASE face and each surface has a derived one. A context argument was rejected explicitly, because it is a second input three front ends would each have to supply correctly, which is the three-derivations hazard one level down. 2. `Indexed` could not be "a detail". The ADR defined Colour as exactly Default | Named | Rgb, promised the two existing Indexed greys would still render exactly, and said the shipped theme used only the first two. Folding an index into Rgb changes its meaning AND breaks that rule. `Indexed(u8)` is now a fourth arm, the browser resolves 16-255 through the xterm cube and ramp, and the shipped-theme rule is restated over the line that actually matters: palette-relative versus absolute. The shipped theme contains no `Rgb`. The rejected-alternatives entry now also says that dropping `Rgb` entirely is a coherent version of this ADR, since that would make the regression unrepresentable rather than defaulted-against. 3. "Build all bundles" was failing: the ADR was neither published nor a named omission. Added to SECTIONS in scripts/build-doc-site.mjs. The checked-in site/docs output is deliberately NOT regenerated here — assemble.sh builds into its own output directory, and site/ is a periodic snapshot rather than a per-commit artifact. Also corrects the hex count: nine stray values in style.css, six light and three dark, not six. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
1ac2a13b6c |
Merge main (#120 ui/shell-polish) into feat/oujb-web-in-release
#120 restyled the page and rewrote #backend-notice as a <details>; <header>'s markup is untouched, so #build-version lands where it was written. What the merge actually has to answer is the CSS, and the stamp gate after it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
7d12ada45d |
docs: strip the tag inventory; a verified fact still has a shelf life
v0.1.0-rc3 no longer exists anywhere -- not local, not gh, not origin, not gitea. Verified here independently before changing anything; the releases are untouched (rc1, rc2, and there never was an rc3 release). THREE MEASUREMENTS OF ONE FACT IN ONE SESSION, and only the first was careless: 1. present on all three remotes and locally (from aa4e901's commit message) 2. on gh alone (git ls-remote, this branch) 3. nowhere at all (git ls-remote, now) Reading 1 was asserted rather than asked, and correcting it was right. Readings 2 and 3 were each taken correctly, by two different people, and they disagree -- because the tag was deleted in between. So the lesson this section reached for was too weak. It is not "ask rather than assert"; it is that A REMOTE-SHAPED FACT HAS A SHELF LIFE EVEN AFTER YOU ASK. A verified answer decays, and prose cannot distinguish a stale line from a live one, so a reader following RELEASING.md at 2am has no way to tell which they are holding. WHAT CHANGES. The present-tense inventory goes, everywhere I put it: RELEASING.md's "It is on gh only; origin and gitea carry rc1 and rc2", the same clause's "the mirrors did not agree", tooling.md's "rc3 is on gh alone", and "pushed to gh" in both tooling.md and the script header -- that last one named a specific remote too, and we do not actually know which remotes the push reached. rc3 survives as a plainly historical example: it was pushed while every crate still said 0.1.0-rc2, dist refused, no release was built. Nothing about where it is now. WHAT STAYS. The instruction to ask -- `git tag -l`, `git ls-remote --tags <remote>` -- because that is the part that ages well: it tells the reader to look rather than to believe. And the record that aa4e901's claim was wrong when checked, which is durable precisely because its subject is a fixed text. Note which half of the earlier correction survived and which did not: "the commit message was wrong" is permanent; "the tag is on gh alone" was true and is now false. The keeper is the one whose subject cannot change. Both files now say explicitly that they record no inventory, so the next editor knows the absence is deliberate rather than an omission to helpfully fill in. The principle never decayed and is untouched: a wrong tag is left dead and the next version cut; a published tag is not moved. No version bumped, no tag cut, no tag deleted by me. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
90ccfb0154 |
feat: attach the assembled site to every release, and let the page name its build
improvise-oujb. A tagged release carried four native tarballs and nothing for the web; the site shipped only by ci.yml's pages-deploy, on a push to main. Pages keeps tracking main (user, 2026-09-22: "I like Pages showing the latest main for now"), so this is a CAPTURE, not a second deployment: the tarball is a self-hostable copy of the site as of the tag and a rollback target, not what a visitor to the published site is running. pages-deploy is untouched. Mechanism is dist's own [[dist.extra-artifacts]], so it survives `dist init`: `dist plan` under the pinned 0.30.4 lists improvise-web.tar.gz beside the four platform tarballs and `dist generate` leaves release.yml byte-identical. The build command is scripts/build-web-release.sh, which calls web/build.sh and web/assemble.sh -- the same two scripts ci.yml and a developer run, so there is no second way to build the bundle. Its toolchain pins come from scripts/web-toolchain-versions.env, which ci.yml's build job now reads too. `github-build-setup` was measured and does NOT help: it injects into build-local-artifacts only, which is the wrong job for a global artifact. The page now names the build it was assembled from in #build-version, stamped by web/assemble.sh with the same token as the bundles and verified by the same scripts/assemble-stamp.sh gate. An unassembled tree honestly reads "unstamped". Verified by running `dist build --artifacts=global` locally: the script ran, the site assembled, and dist placed improvise-web.tar.gz in target/distrib. Not verified: a real tag push, which would require cutting a tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
708d335fa7 |
ci: the preflight repair relocated its blind spot; close it
REVIEW OF THE PREVIOUS COMMIT, NOT NEW MACHINERY. The diagnosis it states was re-verified in both directions before anything here was written: `dist plan --tag=v0.1.0-rc2` gets past the tag/version check (stopping at the pinned cargo-dist-version, as expected) and `--tag=v0.1.0-rc3` reproduces the CI error verbatim, naming `--tag=v0.1.0-rc2` as the fix in its own help text. The workflow was never broken. Both claimed fault injections also reproduce exactly: the comparison replaced by `if false;`, and `expected` derived from the tag, each produce four failures and exit 1 -- exit code read unpiped, since a pager's status would mask it. WHAT THE TEST COULD NOT SEE. Disabling the dirty-tree guard outright left all six checks green and the script exiting 0. That guard is not decoration: it is what stops a tag naming a commit that was never tested, and its failure is irreversible in the same way the version mismatch is. The cause is the previous repair. Moving the negative cases to a clean throwaway checkout is what made the tag comparison reachable at all -- and it thereby removed the only circumstance in which the test could observe the tree guard. The guard whose PRECEDENCE caused the original vacuity became the one thing the suite could not see. The vacuity and its repair are not two events; the repair relocated the blind spot rather than closing it. So the new check is aimed the other way: a tag that MATCHES the version, run against a dirtied checkout, so the comparison passes and a refusal can only be the tree. Verified discriminating -- with the guard disabled it fails with exit 1, and the two faults are independent in both directions (comparison disabled leaves the tree check passing, guard disabled leaves the three tag checks passing), so the seventh check is not a restatement of the other six. Also recorded, rather than guarded: the `dist plan` second opinion can be disabled with the suite green, because every input the test feeds it has already been refused by the textual check. That follows from the arm being optional by design -- pinning it would mean installing a dist matching cargo-dist-version, the very mismatch the script tolerates on purpose. Stated so the silence is not mistaken for coverage. Two corrections of fact. The CI step lives in the `kb` job beside pr-watch-test.sh, where this repo keeps its check_the_checkers -- not in `test`; tooling.md now says so. And the awk version parser was probed rather than hardened: `version.workspace = true` yields empty, which the root path refuses loudly and the per-crate loop skips; a non-first [package] block and build metadata both parse correctly; a trailing comment on the version line mis-parses but fails CLOSED and prints the mis-parsed value in the refusal. No manifest in this workspace has one, so nothing was changed for it. No version bumped, no tag cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
aa4e901bd2 |
ci: a release preflight, because the tag is already public when CI refuses
THE WORKFLOW WAS NEVER BROKEN. v0.1.0-rc3 was pushed to all three remotes while every crate still said 0.1.0-rc2; dist matches a tag against the root package version, refused with "This workspace doesn't have anything for dist to Release!", and the four jobs behind `plan` skipped. Proved in both directions locally: `dist plan --tag=v0.1.0-rc2` gets past the check and `--tag=v0.1.0-rc3` reproduces the CI error exactly. So this changes no trigger and no job graph. What it adds is a check that runs BEFORE the irreversible step. dist named the right answer in its own error text; the problem is when that answer arrives -- a tag is public and immutable by the time CI reads it, so the repair is a new version rather than an edit. scripts/release-preflight.sh refuses a dirty tree, compares the intended tag against the root package version, runs `dist plan` as a second opinion when dist is on PATH (and says so when it is not, since an absent dist must not read as a pass), and prints the tag-and-push commands only once it passes. THE TEST'S FIRST VERSION WAS VACUOUS AND THE INJECTION IS WHAT SHOWED IT. release-preflight-test.sh originally asserted only "did it refuse". Every negative case still passed with the tag comparison replaced by `if false;`, because the script refuses a DIRTY WORKING TREE first and this repo's tree is almost always dirty -- bd auto-exports .beads/issues.jsonl on nearly every command. Four of five checks were answering "is the tree dirty" while claiming to answer "does it catch a bad tag". It now runs the negative cases against a clean throwaway checkout AND asserts the refusal names the version mismatch, so a refusal for the wrong reason fails. Two rival faults were injected against the repaired version -- the comparison disabled, and `expected` derived from the tag itself -- and each produces four failures and exit 1. That is design-principles section 7's "a check that looks like the answer, answering a neighbouring question", found inside a checker written to prevent that class, within an hour of writing it. Recorded in context/code/tooling.md. No version is bumped and no tag is cut here, per the user's instruction to make the workflow work without releasing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
c5e77591d3 |
tooling: a merge driver for .beads/issues.jsonl
issues.jsonl is a passive export whose rows are ordered by insertion, so two sides that file issues collide in the same region even when the issues are disjoint. Git line-merges it and produces conflict markers INSIDE the JSONL, which breaks bd's auto-export silently -- observed today, where a stash-pop left `<<<<<<< Updated upstream` in the file and auto-export was skipping with "invalid character '<'" until the file was regenerated. The driver does no merge logic of its own, because bd already has the semantics: `bd import` skips any row whose updated_at is not strictly newer than the local one, creates ids it has not seen, and keeps the local row on a tie. So importing BOTH sides cannot move the database backwards, and the file is then regenerated from it. The database is the truth; the file is derived. Deliberately not reimplemented here: field-level reconciliation. `bd conflicts show/resolve` already does that against the Dolt working set, which is the real sync path -- a second copy against the text export would have to track bd's schema by hand. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
787e354b9b |
doc-site: name context/code/pr-review.md as an omission
The `build` job fails on main and on every branch merged from it: doc-site: context/code/pr-review.md is neither published nor a named omission NOT a merge loss -- scripts/build-doc-site.mjs in this branch is byte-identical to main's, and neither lists the file. PR #96 added context/code/pr-review.md without a manifest line, which is exactly the gate improvise-res9 added: a doc added without a line publishes nothing and says nothing, so the generator refuses. Omitted rather than published, and the line is cloud-agents.md's rather than tooling.md's. tooling.md explains checkers a reader of the code will meet; pr-review.md is about THIS REPOSITORY'S PROCESS -- where a Codex review appears, how to tell it from our own agents' comments, why absence is not approval. Nobody visiting a website is about to read a PR thread. Both homes updated, because an omission is restated beside the manifest AND in doc-site.md section 2, and the prose there counted them ("Neither is referenced...") so it went stale at three. This strays into scripts/, which is Wave N's ground. Flagged in the hand-back; it is a one-line data entry either way if the orchestrator would rather publish it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
d7bfcbbd93 | Merge gh/main (PR #96) into orchestrator main | ||
|
|
c73df4b5a1 |
kb: a pipe discards the exit code you meant to read
CI / Build all bundles (push) Has been cancelled
CI / Test and lint (host) (push) Has been cancelled
CI / wasm32 check (browser bundle crates) (push) Has been cancelled
CI / Browser JS unit tests (node --test) (push) Has been cancelled
CI / Knowledge base consistency (push) Has been cancelled
CI / Lint the harness crate (push) Has been cancelled
CI / Browser harness (OPFS contract + shell) (push) Has been cancelled
CI / Deploy to GitHub Pages (push) Has been cancelled
Fourth occurrence in one campaign, and this one was in a tool I wrote TO BE the reliable answer. scripts/pr-reviews --wait correctly exited 1 on a timeout, and my invocation ended in '| tail -20; echo', so the harness reported 0 — a review window that had EXPIRED read as a review that had ARRIVED, which is the dangerous direction. The earlier three: pr-watch reading tr's status through a pipe; a probe reporting EXIT=0 from a checker that had exited 1; and PIPESTATUS in zsh, where the array is lowercase pipestatus — the two shells differ, which is its own trap. The script's usage now says not to pipe it. Closes improvise-711n (,.2f yielded zero decimals — the spelling our own docs show) and improvise-x1ns (the preflight undercount ChatGPT Codex found on a PR I had already merged unread). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
67905d9440 |
scripts: pr-reviews, because twenty PRs merged with their reviews unread
Prints a PR's reviews and says whether each targets the CURRENT head or an earlier one, because that changes what you do: a finding against an old commit may be fixed or still live, and both have happened this week. --wait blocks until a review lands against the head. Observed latency is ten to fifteen minutes after a push, so a merge four minutes later races the review that was about to arrive. When it times out that is a decision to merge unreviewed, not a clean bill. The detection rule is placement, not authorship: Codex posts under the owner's account, the same one our agents use, and ENDS with its attribution where ours OPEN with theirs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
7bc26f80d5 |
Review round 1: three checks that did not check what they named
All three found by the adversarial review of
|
||
|
|
383e33a1a0 |
Refuse a duplicate test-evidence heading (improvise-hsoe)
--check-evidence verified that a heading names a test that RUNS, and a duplicate names one just as well, so both copies pass. PR #92 shipped two entries under one heading; the older described a fixture the test no longer had. Refuse rather than warn, on the improvise-rm4s precedent, naming the file and every line in file order. Keyed by the RESOLVED test, so `### f` beside `### m::tests::f` collides too -- the case rm4s's qualification repair creates. Six SELF_TEST_DUPLICATES cases, including the three clean arms: two different tests sharing a leaf are ambiguous and not duplicate, prose repeats freely, and two headings naming two tests are fine. Resolves the two tui.md duplicates: the older column-header copy called THE_MEMBERSHIP_CORRECT_SPANS a live tripwire three hundred lines above the entry recording its removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
0d847e9464 |
Review fixes: name the cap in the dispatch assertion, unstale the skill
Two findings from running /code-review on this branch. The dispatch self-test's dirty arm asserted the run failed and named `f.rs`, which is adjacent to the question -- a failure for any other reason naming that file would have satisfied it. It now requires the cap's own message as well, so the assertion names the question it means. And .claude/skills/code-review/SKILL.md still told reviewers the flag "selects rather than adds, so --check-evidence never runs the comment cap", which this branch makes false. Grepped for every copy of the claim; that was the only one outside context/. A doc that describes the script it gates is code, and leaving it is the gotcha-48 failure exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
da564dc4c1 |
Make --check-evidence additive, so no clean exit skips the comment cap
scripts/check-comment-runs.py early-returned on args.check_evidence, so the flagged run answered the evidence question and said nothing at all about the comment-run cap. The flagged form is the expensive one -- it needs a compiled workspace -- which makes it the one a human runs alone; three agents hit this independently in one campaign, and in one case a local gate went green while CI went red on a three-line doc comment (improvise-ijql). The cap moves into check_cap(), and --check-evidence now runs it first and returns the worse of the two verdicts. The ticket's cheaper alternative -- keep the early return and print a line saying the cap did not run -- is declined and the reason is recorded: a provenance line qualifies an answer the run gave, where a disclaimer qualifies one it did not, while still exiting 0. Exit 0 is what CI branches on and what a human reads as clean, so the rule is stated over the exit code: a tree with a long comment run must not pass any invocation that can exit 0. The guard is _dispatch_self_test, which runs the script as a subprocess against a temporary tree -- the only level at which this is visible, since every unit in the file was correct and the defect was in main()'s dispatch. Both arms were demonstrated by injection: restoring the early return fails "DISPATCH: --check-evidence: exited 0 over a four-line comment run", and forcing the cap to always report a violation fails the clean arm for both invocations, which is what stops an always-fail from satisfying the first. Measured: the cap adds 0.29s to a 6.5s --check-evidence run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
3542d6f717 |
fix(tooling): a leaf is not an identity, and a convenience flag was a bypass
Both blockers from the adversarial review of #83, both in the mode this PR rewrote, both producing a pass while the gate did not do what its prose said. 1. cargo_test_names reduced every listed path to its last segment. Six leaf names collide in this workspace and two are cited in evidence, so a heading was satisfiable by a test in another module answering another question -- renaming ui::project::tests::a_load_that_fails_is_not_a_rebase, the one its entry describes, left the checker at exit 0 with the ui::host namesake answering for it. Paths are kept whole; a bare leaf resolves only when unique, and an ambiguous one is REFUSED with both candidates named. _RUST_NAME accepts `::`, so a qualified heading is a claim rather than prose, and a citation is matched on the leaf so qualifying a heading cannot orphan its own test. The two colliding entries are qualified. Neither half suffices alone: rename one of a colliding pair and the leaf becomes unique again, which is why qualification is the fix and the refusal is what forces it. 2. --rust-tests called cargo_test_names with one argument, so the --ignored subtraction never ran, and it printed a byte-identical success line. It now requires --rust-tests-ignored (one alone exits 2) and the line names its source. Also, from the same review: a missing context/test-evidence/ exited 0 while 170 citations pointed at nothing (early return, now gone); the doc-site "runs last" check accepted a trailing `echo` that merely NAMED the generator, so it asks for an invocation; bundle_freedom gains an arm for a transitive `import "../app.js"`, which reaches the bundle while naming nothing forbidden; and two prose overclaims are corrected -- the prose-heading count is a convenience rather than a guard (git diff is the mechanism), and the ratio rule says "above the run MEDIAN", which is defensible where "above the others" is false against second place. scripts/check-ci-graph.py makes claim 6's rule a gate: pages-deploy must `needs` every other job in ci.yml. The rule was stated in the last commit and a zz-new-check: job added during review was still ungated with nothing noticing. README now states the rule and lists no jobs, the second copy being what went stale twice. Every fault re-injected and observed; figures in context/test-evidence/tooling.md rather than only here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
1d4d9107cf |
fix(tooling): three findings from reviewing this branch against itself
1. A self-test case that was a duplicate wearing different clothes. "a heading cargo's listing does not carry is an orphan however the source reads" passed on an empty listing -- which is the first case in the table, already. Once defined comes from cargo, the Rust beside a fixture is not read at all, so four cases for the four source shapes would be four copies of one assertion dressed as coverage. Deleted; the evidence for those shapes is the real-tree injection, and context/code/tooling.md now says which half a fixture can carry and which it cannot. This is exactly the shape improvise-84wp's prose, applied two commits ago, tells a reviewer to look for. 2. assemble-stamp.sh grepped its patterns as REGEXES, inherited from the inline version it was extracted from. The `.` in improvise_wasm_client.js?v=abc matches any character, so a stamp corrupted in that position read as present -- a pass in the one direction a guard must not fail in. grep -F, plus a case proved by injection: dropping the -F flips it. 3. A missing cargo raised a traceback out of --check-evidence. It now names the flag that takes a saved listing instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
1c3e1bd968 |
feat(doc-site): the published set is derived, and the generator must run last (improvise-res9)
The manifest only ever grows, so a doc added without a line in it published nothing and said nothing: build green, link check green, doc simply absent. The generator now walks context/**/*.md plus docs/design-notes.md, subtracts the manifest, and fails unless the remainder is exactly the OMITTED map -- checked in BOTH directions, because an entry naming a file nobody can find asserts a reason for nothing. Filed as latent; it was live. context/code/kb-tool.md had been unpublished, unargued and unnoticed since it was added, while doc-site.md's own table says context/code/*.md is published. It is published now, with context/code/tooling.md and context/test-evidence/tooling.md: 23 pages to 26. Second gate, same file: the generator reads web/assemble.sh and refuses unless its own invocation is that script's last command. Its outward-link check resolves ../index.html and ../app/ against the filesystem, so a line appended below it would make that check pass against a half-built tree. Both run at module scope, before any page is generated, so a manifest error is answered before the 48 outward-link failures a bare run reports -- a missing doc is actionable wherever you ran the generator, and the two are unrelated diagnoses. Three faults injected and observed; evidence in context/test-evidence/tooling.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
8af37d44a1 |
feat(ci): two conventions become gates (improvise-w8ih, improvise-4ktt)
The stamp gate is scripts/assemble-stamp.sh, taking FILE PATTERN pairs, and scripts/assemble-stamp-test.sh is its check_the_checker. Deleting the final `test -z "$missing"` makes four of its eight cases fail; before the extraction that same deletion was invisible, both misses printing while assemble.sh exited 0. It is a separate script because exercising it inside assemble.sh would need a fabricated web/pkg, which is a second answer to "what does the site assemble from" (improvise-vvl3). Wired into the `build` job, first, because it needs nothing and guards a step in that job. web/tests/bundle_freedom.test.js refuses a flat web/tests/ file that names the bundle directory -- the constraint keeping the `js` job at seconds rather than ~4m30s, stated in prose and checked by nothing. It scans its own filename too, so it cannot become the place a violation hides, and its companion asserts web/tests/bundle/ still holds a test that DOES use the bundle: a scoping rule is uninteresting if nothing falls outside it. Both faults were injected and watched to fail; evidence in context/test-evidence/tooling.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
7945221eed |
fix(kb): two comments claimed things the code does not do (improvise-9d5k)
kb-test case 10 said it was where same_key is discriminated. Reverting same_key to unification leaves case 10 PASSING and fails the wildcard rm and case 17 instead -- a rule keys on its whole clause, so the `:-` term cannot unify with probe_fact(a) and conflicts never fires there. Re-demonstrated by injection before rewriting the comment. The site is guarded; the explanation was wrong. replace's refusal said it would add the rules "beside the ones already there", which is false when the predicate has none. The behaviour was always right and fail-safe; only that clause of the sentence was wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
61473ef4d3 |
fix(tooling): evidence headings are diffed against cargo, not scraped (improvise-rm4s, improvise-9za9)
Four shapes let a heading name something that never runs: a test inside a raw string literal, one behind an unenabled cfg feature, one behind cfg(target_arch = "wasm32"), and one behind #[ignore]. All four cleared the scrape; injecting them together made the previous checker print "543 entries, all name a test that exists", exit 0. --check-evidence now takes the Rust names from `cargo test --workspace -- --list` minus its `--ignored` twin. All 539 live headings resolve against a real listing, so the stricter source costs nothing today. rust_tests is rust_citations: the scrape survives only for the reverse direction, where a false positive is loud. Prose headings become a third kind, counted and printed, so retiring a claim by rewriting it as prose moves a number instead of passing in silence (improvise-9za9). No gate can read prose for truth; the reviewer is the mechanism, and the diff is what to read. context/code/tooling.md is new and is the file four docstrings in check-comment-runs.py already pointed at. design-principles §7 stopped saying the check runs in one direction, which improvise-lyyc had already made false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
cd094b6dfb |
fix(kb): the dj1l cases discriminate, and a replaced rule cannot silently become an add
Five findings from the second review round, and the first is this PR's own standard applied to its own P1. **The two fixes improvise-dj1l names had no discriminating test.** Reverting `has_key` to unification, or dropping `canon_key` from the duplicate gate, left kb-test at 27 passed. Case 11 was carried by `check_total` refusing with a DIFFERENT message, and case 10 by the other half of the fix. Both now assert which refusal fired (`said()`), plus two cases aimed at each half: a rm key with a variable in a non-final position must be refused as ABSENT rather than matching everything, and a keyed FACT whose key holds a variable must ADD without excluding the siblings it would unify with. Each proved by reverting exactly one fix and watching exactly that case fail. **Keying a rule on its whole clause made `replace` on an edited rule silently become `add`** -- both arms live, the stale one still firing, and `check_total` cannot see it because `NO + NF` is correct arithmetic for an add. Reachable on the 15 `kb_error/1` arms and the `kb_text/2` rules, so editing the check improvise-2f97 adds is the first thing to hit it. `check_rule_replace` refuses and names the route (rm, then add); the negative arm -- replace of a rule present verbatim -- is tested, or the refusal would just be "replace never works on rules". **The `decision(` arm was dead code.** `cited_name` took the first field, and the repo spells `decision(<ticket>, <name>)`, whose ticket id fails `name_shaped/1` on the hyphen -- so that marker could never fire for a real citation. A second `cited_name` clause takes the second field. All three markers are now exercised. Minor, from the same round: `_JS_TEST` no longer accepts `test.only`, which disables every other test in its file, with a case asserting a heading cannot be satisfied by one; and `node-test-test.sh`'s last case no longer duplicates an earlier one. Filed rather than fixed: improvise-w8ih, extracting `web/assemble.sh`'s gate so its own absence is visible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
0b7a2351b5 |
fix(tooling): a barren file is refused by node's own report, not by grepping for test(
Two findings from the PR #69 review, one class: a lexical `test(` scan is satisfied by things that execute no assertion -- a commented-out call, the words in a string, `test.skip`/`test.todo`, and an empty `test.describe`. scripts/node-test.sh drops the scan and reads node's report instead. `# pass` is 0 for a skip-only file, a todo-only file and an empty suite; and a file that registered no test is reported as one passing test WHOSE NAME IS THE FILE PATH, which is both how a barren file is detected and why `# tests` alone reads as the file count. That naming convention is an assumption about node, so the self-test pins it: a node that stops doing it fails the case rather than letting the guard go quiet. Six cases added, including a barren file hiding among passing ones. --check-evidence had the parallel hole: _JS_TEST accepted `.skip`, `.todo` and `.describe`, so a quoted heading could be satisfied by disabled coverage or by an empty suite. Now runnable leaves only (`.only` included), with three cases asserting that a skipped, todo or suite-only name leaves the heading orphaned while a test nested in a suite still satisfies one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
50de0a8fd2 |
fix(kb): keys are compared as variants, and a cited clause cannot be silently retired (improvise-dj1l, improvise-2f97)
dj1l (P1, found by review of PR #68 after it merged). kb-merge.pl matched clause keys by UNIFICATION, which both lost and corrupted clauses. check_self_consistent compared keys with ==, but findall hands back fresh variables, so two clauses with variant heads were never == and passed the duplicate gate; foldl_merge then matched by unification and the second removed the first. Reproduced: 536 before, 2 in delta, 537 after. Worse, adding `probe_fact(X) :- doctrine(X,_).` over a kb holding probe_fact(a) and probe_fact(b) deleted probe_fact(a) AND wrote `probe_fact(a):-doctrine(a,A)` -- the match bound X, so the clause written was not the clause supplied. The round-trip verify cannot see that: it compares the file against an intended list already corrupted by the same unification. same_key/2 (copy_term + numbervars) in has_key, conflicts and absent; canonicalize before duplicate detection; a RULE keys on the whole clause, since one head shape with different bodies is ordinary Prolog; and check_total/2 asserts the arithmetic the tool already PRINTED beside the delta size -- "2 in delta, 537 after" was visible in its own output twice and nothing compared them. 2f97. kb_error(dangling_citation/3): a name cited inside another clause's text must resolve as a doctrine/2, invariant/2 or decision/3, so retiring a clause can no longer dangle every clause naming it. The token rule is narrow on purpose -- a name-shaped token directly inside doctrine(, invariant( or decision( -- so prose cannot false-positive. kb_error/1 is a gate predicate, so this was written with KB_ALLOW_GATE=1 deliberately: the refusal exists so a write cannot disarm its own gate, and an intentional change to the gate is supposed to be a greppable act rather than an impossible one. Seven cases added to kb-test.sh, each proved by injection: two variant-headed rules both land; a non-ground rule deletes nothing and is written as supplied; a duplicate key is still refused; an injected dropping fold is caught by the total; a cited clause cannot be retired; and the citation arm fires on a name that resolves to nothing without firing on one that resolves. scripts/kb gains a KB_MERGE seam so a broken merge can be injected. One pre-existing case moved off doctrine(bug_first), which the new arm correctly refuses to retire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
2eb46122d3 |
fix(tooling): --check-evidence sees #### headings, JS tests, and the reverse direction (improvise-5ray, improvise-lyyc)
Two blind spots with one consequence: an evidence file could promise a guard
that was not there, and a test could point at prose that did not exist.
5ray: headings were collected with `startswith("### ")` and resolved against
Rust fn names only. context/test-evidence/clients.md uses `#### "name"` for
every browser test precisely BECAUSE a `###` naming a Playwright test would
have been reported as an orphan -- so the convention that kept the checker
quiet was the same one that made it blind. Now `###` and `####` alike, decided
by SHAPE: a bare identifier is a Rust test, a quoted string is a JS test
(harness/tests and web/tests), anything else is prose naming nothing.
lyyc: the check was one-directional. A test whose doc comment names
context/test-evidence must now appear there. Deliberately only that narrower
obligation -- most tests have no evidence and should not be forced to. Neither
direction can see a STALE entry; that stays discipline.
Each half found a real instance on its first run. A `#### "? opens help..."`
heading outlived the test
|
||
|
|
ea347ccab4 |
fix(ci): a node test step with an unmatched glob is a gate that gates nothing (improvise-vz54)
Under bash an unmatched glob survives as a literal; node 22+ globs the literal itself, matches nothing, reports 0 tests and exits 0. Both CI node steps -- the `js` job and the browser job's bundle suite -- were exposed: rename or move either directory and the step goes green having asserted nothing. Reproduced under `bash -e`, which is the shell CI names; zsh ABORTS on an unmatched glob, so the failure is invisible from a developer's shell. The glob becomes the script's rather than the shell's. scripts/node-test.sh expands it, refuses an empty match, and also refuses a matched file that declares no test -- checked LEXICALLY, because node reports such a file as one passing test named after the file, so the `# tests N` a minimum-count assertion would read is the FILE count and is satisfied by the offender. Measured on node 24.20. scripts/node-test-test.sh proves each refusal by injecting its fault, including that the wrapper does not buy its refusals by swallowing a real failure, and runs in the `js` job: a check wired into nothing is a convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
fc6d229dda |
kb: design against the model before implementing, and fix the key rule it exposed
AGENTS.md has always said Prolog is a design tool for reasoning about tickets; it was the least-used line in the file, because kb.pl was only ever written retrospectively. `kb check --with <design.pl>` merges proposed facts into a throwaway candidate and runs kb_consistent over the union, so a design can be refused before any code exists. Nothing is written either way. The model already derives rather than records -- illegal_dependency, may_use, depends_transitively, portable, snapshot_safe, view_state_admits, record_obligation, and eleven kb_error classes. Three proposals exercised it in one sitting: one added dependency edge produced SIX layering violations through the transitive closure; a proposal to declare decision/3 functional was refused, correctly, because improvise-907 carries two decisions; and a mis-ordered layer/2 argument was named rather than accepted. Trying to model a design found a real defect in the tool. Keys were the functor plus all arguments but the last, which is right for doctrine(Name,Text) and wrong for any RELATION: crate_depends_on(A,B) keyed on A, so a crate depending on two crates read as one key twice and was unaddable. It cannot be read off the KB's functional/1 -- that keys an arity-3 predicate on its first argument, right for crate/3 and wrong for decision/3 -- so key_arity/2 is a table, and anything unlisted keys on the whole head, which is the safe direction. kb-test.sh is 20 cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
eaaf9d935a |
kb: a write cannot disarm its own gate, and cannot silently lose another's
Second review pass on PR #68. All three findings real; the first two reproduced by injection before fixing. The candidate is validated by CONSULTING it, so the thing being checked got to influence the check. Three ways through, all demonstrated: - `:- halt(0).` as a delta -- fixed last round by refusing directives - REMOVING kb_consistent: it then throws existence_error, and tpl exits 0 when a goal throws before reaching halt, so the gate passed by not existing. The error was printed and the tool still reported success. - REPLACING it with the fact `kb_consistent.`, which passes forever after. CHECK_GOAL now catches, so a throw exits non-zero, and writes touching the gate predicates are refused unless KB_ALLOW_GATE makes it deliberate. That list is hand-maintained and a new gate predicate is unprotected until named -- stated rather than hidden. Writes were read/validate/mv with nothing in between, so two agents -- ordinary here, waves run in parallel -- would both read the same file, both validate, and the later mv would discard the earlier addition. The lost-update class this tool exists to remove, reintroduced in the tool. Closed by stamping content at read time and refusing to commit if it moved, plus an mkdir lock. The stamp is the part that matters; the lock only makes the collision rare. AGENTS.md's top-level instruction still said to save facts through the MCP. The wave section had been fixed; the authoritative line had not. kb-test.sh is 17 cases. The race is deterministic via KB_TEST_RACE. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
661ca76e6d |
kb: close the directive bypass, and package both tools as skills
Review on PR #68 found two real holes, both reproduced by injection before fixing and both now regression-tested: - A DIRECTIVE is callable, so the callable check let one through -- and the candidate is validated by CONSULTING it, so a delta of `:- halt(0).` exited the check with success before kb_consistent ran. The gate passed by never running, and the directive landed in kb.pl. Directives and queries are now refused by shape. - Keys were checked only against the EXISTING file, so two clauses sharing a previously-unused key clashed with nothing and the merge dropped the first -- add discarding part of its own payload. Visible in the tool's own output ("2 in delta" against one clause added) and nothing compared those numbers. Also from the review: nothing ran kb-test.sh. A suite that gates the only writer of kb.pl but is invoked by no workflow is doctrine(an_advisory_gate_is_not_a_gate) in the PR that adds it. Both suites now run in the kb job. AGENTS.md now leads with the rule instead of describing the retired procedure first -- the review read against the earlier head, but it was half right at the later one too: a skimming reader followed the batching paragraph. Both tools are packaged as skills, because the failure is not that the scripts are missing, it is that agents do not know they exist and write their own. Three hand-rolled PR watchers in one session all reported green while wrong, which is what scripts/pr-watch and its eight fixture cases exist to stop. One of those cases -- a head that moves mid-wait -- was untestable until a subshell bug in the fixture seam was fixed: fetch ran inside "$(...)", so its counter never advanced and every poll re-read the first fixture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
805bae7a15 |
kb: a clause can be retired, and the docs stop naming the MCP
Three gaps in the tool as first written, each found by asking what it could not do rather than by running it: - There was no way to REMOVE a clause. Every correction could add or overwrite, so a fact that went false could only be rewritten, never dropped. `kb rm` takes keys and refuses when any is absent -- a typo'd key would otherwise report success and leave the clause in place. - `doctrine(kb_is_maintained_through_the_mcp)` is retired for kb_is_maintained_by_scripts_kb. The NAME was the claim that went false, which is the repair opfs_is_run_but_bindings_is_not needed for the same reason. The replacement records why nobody should go back, and that READING through the MCP is unaffected. - AGENTS.md said kb.pl was "MCP-maintained, whole-predicate replacement" and told agents to strip host_rpc shims after each save. Both described the retired mechanism. This is a_fact_stated_twice_is_changed_by_grep landing inside the PR that depends on that doctrine being true. Also fixes a printed value that was an unevaluated expression: `rm` reported "530-529 removed" rather than 1. kb-test.sh is 11 cases now, both new ones proved by injection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
3405fc4def |
kb: retire the Prolog MCP for a tool that costs the clause, not the predicate
loadProgram REPLACES a whole predicate, so adding one clause meant re-sending every existing clause of it through an agent's context window. Measured on the wave-3 batch: ~113KB read and ~113KB written to deliver ~25KB of new text. That sets the behaviour -- writing becomes a thing you batch and do rarely -- and the batch is where a short payload silently drops the rest of a predicate, which has cost this repo 22 decisions and 19 doctrines once already. scripts/kb reads clauses as DATA and never consults them, so the cost of adding a clause is the clause. Canonical form is read_term in, portray_clause out -- measured, not designed: all 495 existing facts round-trip byte-identically and the form is a fixed point in one pass. Rules move to one line, which is why the reformat lands here rather than earlier: the MCP would re-expand them on its next save and the two would thrash. trealla moves into the devShell. Reaching it through `nix shell` per invocation took the tool's first real use past a two-minute timeout for a write that takes 0.42s with tpl on PATH. Four guards, each added because injecting its fault showed the tool accepting what it should refuse. None was found by reading: - an unparseable delta dropped trealla into its REPL, which HANGS with a terminal and EXITS 0 without one -- a refusal that reads as acceptance - `-g main,halt` exits 0 on a throw AND on a failure, because a goal that fails never reaches the halt beside it; halt(2) alone exits 2, so the default is the hazard. The goal now guarantees a halt on every path (0/2/1) - `42.` parses, is not callable, and landed as a fact whose head is a number; kb_consistent has no opinion about stray terms - fmt discarded the merge's exit code and would have committed a failed canonicalization Plus the two that were designed in: the result is read back and required to be the clauses intended (variant equality -- == compares variable identity and reports every rule as different), and kb_consistent runs on the CANDIDATE, so a bad delta leaves the knowledge base untouched rather than needing a revert. scripts/kb-test.sh proves all nine by injection. context/code/kb-tool.md carries the argument, including what it does not reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH |
||
|
|
05589676f8 |
fix(docs): the nav toggle is named "Pages", not "trigram for heaven, Pages"
`::before` content participates in accessible-name computation, so the hamburger glyph was part of the toggle's name and a screen reader would read it out. The CSS alt-text form `content: "\2630 " / ""` suppresses the glyph from the name while still painting it. Taken from the reviewer's optional polish, and verified here rather than accepted: at 390 the checkbox's accessible name is exactly "Pages" and its `checked` state flips false -> true on Space, so the collapsed/expanded state is announced rather than merely the control's existence. The glyph still renders -- computed `::before` content is present and the label lays out at 358px. At 1280 there is still no checkbox in the tree at all. This is the same principle the previous commit wrote down, one step further: focusability is necessary and not sufficient, because a control a screen reader can reach but cannot name, or whose state it cannot read, is still broken. The instrument that observes that is the accessible name and the checked flag, not the tab order that was enough for the last fix. Firefox ignores the alt-text form and falls back to reading the content, which is exactly today's behaviour, so the change is an improvement in Chromium and WebKit and a no-op elsewhere. Gates: 1378 rust, 26 node --test, 16 playwright (3 skipped, webkit OPFS), all three check-comment-runs modes, 3462 site links / 0 broken, 0 nested anchors, 0 of 23 pages overflow at 320/390/1280, nav operability unchanged at 390/900/ 901/1280. improvise-djoo Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
c0b786cf0d |
fix(docs): the phone nav toggle needs a keyboard, not a mouse
A regression the previous commit introduced, found in review. The checkbox driving the collapsed nav carried `hidden`, which is `display:none`, which removes it from the tab order AND from the accessibility tree. A <label> is not focusable on its own -- the checkbox it labels is what carries focus -- so at <=900px the navigation could be clicked and reached no other way, and the accessibility snapshot contained neither the checkbox nor any of the 25 links. The toggle is now visually hidden but focusable at narrow widths, and display:none at wide ones. Keeping display:none on the wide side matters: merely un-hiding it puts focus on an invisible control whose label is display:none, which is its own wart. Measured at 390, 900, 901 and 1280, because the regression lives on one side of a media query. At 390 and 900 the tab order is brand -> navtoggle, the toggle is focusable, the a11y tree carries a checkbox node, and Space takes visible nav links 1 -> 26. At 901 and 1280 the toggle is display:none, not focusable, absent from the a11y tree, and tab runs straight through the 26 links -- desktop identical to before. 0 of 23 pages scroll horizontally at 320, 390 and 1280, so the absolutely-positioned 1px input introduces nothing. WHAT IS WORTH MORE THAN THE FIX, and why doc-site.md gains a paragraph rather than the code gaining a comment: my probe for the previous commit counted VISIBLE nav links. That answers "does operating the toggle reveal the links" -- correct and complete for a mouse -- and is silent on whether any non-mouse route exists. It could not have seen this, however sharp the visibility instrument was. Last round's lesson was that a measurement which cannot distinguish the two states is not a measurement; this round adds the half it was missing: a measurement of the wrong PROPERTY is not a measurement either. checkVisibility fixed the visibility instrument; it does not turn a visibility count into an operability check. The instrument here is tab order, .focus() and the accessibility snapshot. Gates: 1378 rust, 26 node --test, 16 playwright (3 skipped, webkit OPFS), all three check-comment-runs modes, 3462 site links / 0 broken, 0 nested anchors, 0 of 23 pages overflow at 320/390/1280. improvise-djoo Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
41f694f535 |
fix(docs): collapse the phone nav, and stop the header comment lying
Round 2 of review. Three notes and the mobile-nav call. THE NAV, and the correction that prompted it. I had called the 25-link sidebar wall pre-existing and declined to fix it. That was wrong: the entire CSS is new in this PR, so the wall ships for the first time here and it is mine. Every phone page opened on ~1500px of nav before any content. The mechanism is a checkbox and not <details>, and the reason is measured rather than assumed. <details> was the obvious control, and it collapses correctly on a phone -- but the sidebar layout has to force it OPEN, and a closed <details> hides its content through ::details-content, which `display` cannot override. Chromium showed the desktop sidebar collapsed to one link with its toggle hidden: unusable. So the hiding is ours, in a media query, where overriding it is just cascade. Measured at both widths: 390px shows 1 link and the toggle, with the h1 at 189px instead of ~1500; 1280px shows all 26 links with no toggle and the h1 at 40px, unchanged; and the phone toggle really expands 1 -> 26 on click. Note the first probe was wrong -- getBoundingClientRect reports a height for content-visibility-hidden children, so it read 26 visible in both states and would have passed the broken version. checkVisibility() is what separates them. The same argument applies to the table of contents, which is equally new here: repo-map opened on 1397px of TOC on a phone. Capped at 45vh so it scrolls; 1397 -> 924, desktop identical at 652. THE HEADER COMMENT. The generator stopped running standalone when the link check began resolving ../index.html and ../app/ on disk, and its header still advertised a default OUT_DIR that now exits 1. That is gotcha 48's third paragraph exactly -- a fact stated in two places and updated in one, with the copy that was updated being doc-site.md and the copy someone about to run the script actually reads being the header. It now says so, keeps the strictness (the hole it replaced was counting those links as checked while skipping them), and records the other thing the reorder changed: the DIRECTION of a failure, from an obviously-broken tree with no app/ to a complete-looking one with a partial docs/. And one documented subset: a \| inside a code span in a table cell keeps its backslash where GFM would unescape it. Splitting is suppressed inside code spans, so the escape is never needed there and never consumed. No corpus instance; recorded as a decision rather than left to be rediscovered. Gates: 1378 rust, 26 node --test, 16 playwright (3 skipped, webkit OPFS), all three check-comment-runs modes, 3460 site links / 0 broken, 0 nested anchors, 0 of 23 pages overflow at 390 and 1280. improvise-djoo Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1a0f04856e |
fix(docs): the review's four rendering defects, and the check that could see one
Review of PR #62 found four live defects and four smaller ones. The most valuable is the first, because of what it says about the rest. A TOC entry wrapping a heading whose rendered text already contained a link emitted <a href="#anchor"><a href="github...">...</a></a>. The parser closes the outer anchor early, so the entry became an empty unclickable stub and clicking "src/draw.rs" in the TOC navigated to GitHub. 19 occurrences over 10 pages. Both hrefs resolved perfectly, so the link check, the whole-site link check and a screenshot pass all missed it -- it answers "does every href resolve" and this is not that question. So the fix is two things: the TOC keeps a tag-free variant of each heading, AND a new check asserts no <a> appears inside another <a>. Proved by reverting the first half: 66 nested anchors reported, assembly exits 1. Clicking that TOC entry now lands on #src-draw-rs. esc() escaped & < > but not the quote it was interpolated into title="..." with. Five shipped bead titles contain one, and Chromium parsed the resulting tag as having three junk attributes. Now escapes " and ' as well. Today's corpus being friendly is not a reason to interpolate tracker text unescaped. Seven pages overflowed at 390px by up to 92%. The reviewer attributed it to the TOC; the TOC was real but not sufficient, and the first fix measured clean by getBoundingClientRect while the page still overflowed, because the heading text overflows its box without widening it. Only per-element scrollWidth names the culprit: hundreds of snake_case test names as headings. overflow-wrap on the headings and on .toc a, plus dropping display:table from .tickets, which had been cancelling the mobile overflow-x guard. 0 of 23 pages overflow at 390 and 1280 now, measured. table() split rows on every pipe before inline parsing, so `|Time Period|` -- this repo's own quoting syntax, in a corpus whose subject is that syntax -- was shredded into extra columns. A row that still parses, so no column count could see it. Now walked, respecting code spans and \|. Proved with a fixture: 9 tds with the fix, 12 with the old splitter reinstated. The four smaller ones: the link check counted 48 links it never verified behind a comment promising they were "checked below" when there was no below -- those are the links leaving the docs tree, and they are now resolved against the filesystem, which is why assemble.sh runs the generator LAST, once its siblings exist. The gotcha jump bar's hardcoded 48 is derived from the emitted anchors, because it failed closed on deletion and silently on addition, and the list only grows. context/code/doc-site.md cited improvise-xxxx and so broke the audit the surrounding paragraph describes; the repo-wide audit is now 189 ids, all resolving. And the Conventions paragraph's own example path now goes through inline(), so the sentence demonstrates the linking instead of asserting it. The ticket index keeps all 180 rows, open bugs and titles included, by the user's ruling: the data is already public in .beads/issues.jsonl, the page only makes it legible, and filtering it while the lede links the raw JSONL would be theatre. Gates: 1378 rust tests, 26 node --test, 16 playwright (3 skipped, webkit OPFS), all three check-comment-runs modes, 3460 site links / 0 broken, 0 nested anchors, 189 bead ids all resolving. improvise-djoo Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
04d8c1ff9a |
fix(docs): a source link names a directory as a tree, and . is not a path
Two ways the path linker could emit a URL that resolves by luck or not at all, both found by reading the generated links back rather than by the link check, which only sees the site's own pages. A code span naming a directory -- `crates/gp-tree/`, `context/test-evidence`, and about a dozen more -- was linked under /blob/. GitHub redirects that to /tree/ today, so it worked, but the correctness of ~30 links rested on a redirect nobody here controls. The kind is now read off the filesystem at generation time, which is the same source the existence check already uses, and the trailing slash is dropped. And a code span of punctuation alone passed the path guard: `.` is matched by both the character-class test and the "contains a slash or a dot" test, exists on disk, and is a directory, so it became a link to the repository root from the middle of a sentence. A path now has to contain at least one alphanumeric. Verified with a 40-hex token: 195 source links, zero of them `blob/main`, no blob link ending in a slash, no tree link naming a file, and no link to `.`. improvise-djoo Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
f1011d7f6f |
feat(docs): publish the context/ prose as a site folded into Pages
The landing page is the site root and the app is /app/; the prose now sits at /docs/, assembled by the same web/assemble.sh the Pages job and the browser harness both run, so what ships and what is tested are one tree. Published: design-notes, design-principles, repo-map, the three ADRs, the seven context/code/ arguments, and the 501 test-evidence entries as their own clearly-labelled section. Not published: context/plan.md, which is an agent prompt about a launch rather than a description of the program, and context/cloud-agents.md, which is addressed to nobody who visits a website. Neither is referenced by a published document outside a code fence, so leaving them out dangles nothing. The reasoning is context/code/doc-site.md, which is itself published. Two things the corpus needed that a generator would not have given it. The ~190 improvise-xxxx citations resolve to a generated ticket index built from the already-public .beads/issues.jsonl rather than to an invented URL that would 404; an id the export does not yet carry stays plain text. And a code span that is wholly a repo path links to GitHub at the assembled ref, but only when the file exists on disk, so a stale citation degrades to text. The generator link-checks its own output and exits non-zero on a break, and assemble.sh runs under set -eu, so a dead cross-reference fails the assembly rather than shipping. It earned that on its first two runs: it caught a fence-close regex comparing a three-character marker to a one-character string, which swallowed every document from its first code block onward, and a list parser that ended the gotcha list at the first blank line, emitting 1 of the 48 anchors the jump strip links to. Proved by injection afterwards -- renaming the anchors reports 48 missing anchors, pointing the ticket links at an ungenerated page reports 763 missing pages. No framework, no node_modules for the site, no build step beyond one zero-dependency node script, matching the landing page's posture. improvise-djoo Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
96adc66030 |
fix: the attribute handling was still a line scanner, and carried two holes
Adversarial review of the previous commit: scan() survived 16 literal-shape
attacks, and both HIGH findings were in the code that did NOT get rewritten.
The attribute handling still asked the RAW LINE the questions this commit's
whole argument says a raw line cannot answer.
- Bracket depth was counted over the raw line, so `#[cfg(feature = "[")]`
set attr_depth=1 with no reset path and exempted EVERY REMAINING LINE IN THE
FILE. The raw-string spelling of it is the exact defect class this rewrite
was for: scan() knew those bytes were string content and line 150 asked the
line instead. The tree has 14 `#[arg(...)]` and 3 `#[command(...)]` clap
attributes whose help strings are free text.
- `s.startswith("#[doc")` is an UNANCHORED PREFIX, so `#[doc(hidden)]` counted
as a comment and flagged correct code. Fourth instance of that class in this
campaign, eleven lines above the comment naming the previous three.
Also from the same review: `#![doc = "..."]` is the inner spelling of a doc
comment and was never counted (unlimited free crate doc); an inner attribute
between two `//!` blocks split one crate doc in half, where the rule exists to
separate a crate doc from an ITEM doc — so it now ends a run only where the doc
KIND changes; and a block comment closing on a line with code got one free line.
Attributes are now asked of the scanner: `scan()` returns the code-only text,
and depth is counted over that. `_is_attribute` is deleted — dead since the
logic moved inline, and an orphaned explanation is what --check-evidence exists
to catch one directory over.
The coverage finding was the important one. Two of scan()'s four literal
mechanisms had NO self-test case: reverting char-literal or plain-string
handling left the suite green. Both are load-bearing in opposite directions —
without char literals, `'"'` opens a string that eats the file (false negative);
without plain strings, a multi-line string holding `//` lines is a false
positive, the CI-breaking direction. Both now have cases, and so does every
shape above. The attribute-depth mechanism needed a fixture with code AFTER the
attribute, because comments inside one are counted either way: the first
fixture I wrote passed under its own injection.
Self-test: 17 violations caught, 9 legitimate shapes allowed. Every mechanism
proved discriminating by reverting it alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
a6451363ca |
fix: replace the line scanner with a real one; three more bypasses were open
Adversarial review demonstrated that the previous fix closed one class and
left three, then reintroduced the bug this file's own history is about.
- A ``` fence in a PLAIN `//` comment earned the doctest exemption. rustdoc
runs no test there ("running 0 tests"), so two lines exempted anything after
them. The cheapest bypass yet, and `--list-doctests` in the same file already
had it right: one of the two was wrong and it was this one.
- A block comment opened after code on the same line was invisible, since only
a line STARTING with /* was seen. That is the most natural way to write one.
- Nested block comments closed at the first */ and the rest read as code.
- And the false positive: `in_block` had no reset path, so a /* inside a raw
string swallowed the rest of the file. Latent — no .rs file starts a line
with /* today, so the block fix had zero effect on this repo — but 8 files
hold raw strings and this repo's habit is Rust fixtures inside them. The
first such fixture breaks CI with a diagnosis pointing at a string.
A line-based scanner cannot decide any of these, so `scan()` now walks
characters, tracking nested block comments, strings, raw strings (with hashes)
and char literals. `#![...]` ends a run where `#[...]` stays transparent,
because a crate doc and an item doc separated by an inner attribute are two
blocks a reader does not meet as one. `#[doc = "..."]` counts: it is the one
attribute that is a comment.
The self-test carries all of it: 12 bypasses, 6 shapes that must stay legal.
Each new fix was proved discriminating by injecting it alone.
One of those injections caught a VACUOUS test of my own: with raw-string
handling disabled, a plain-string fallback reached the same verdict, so the
fixture proved nothing. It now contains r#"x"y, which a non-raw parse closes
early — so the fault is visible. That is the defect this repo is named for,
found in the test written to prevent it.
Closes improvise-mbmf, which PR #45 claimed to close and did not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
|
53ffe9f193 |
fix: close three demonstrated bypasses of the comment cap, and self-test it
Adversarial review of PR #44 constructed all three, each compiling clean with no warnings, each passing the checker. Block comments were invisible: the scanner only ever tested startswith("//"), so a six-line /* */ passed — and /** */ is a DOC comment that rustdoc renders exactly as /// does, making `/**` the cheapest way to write the long argument the rule exists to forbid. There are zero block comments in the tree today, which is the only reason this was unexploited; #44 is the change that gives anyone a reason to look. Fence state was per-FILE, so one unclosed ``` exempted every later comment in the file. A dangling doctest would fail the build in a lib target, but doctests do not run for bins, tests/, examples/ or harness/ — all default scan roots. Fence state now belongs to the comment block: any non-comment line clears it. Attributes terminated a run, so `#[inline]` between two pairs of doc lines split one argument of four into two legal runs of two. The module docstring had always promised attributes are "ignored"; the code disagreed with it, and disagreed in the direction that passes. Closing the last of those found two real violations the old checker could not see. crates/improvise-command/src/command/cmd/text_buffer.rs carried TWO doc comments straddling #[derive(Debug)] — both attach to the struct, so rustdoc rendered both; a leftover from the comment wave. transport.rs had a three-line run split by #[allow]. A checker with ten silent successes and no test of its own is the thing it warns about, so --self-test runs every bypass this file has been fixed for plus three shapes that must stay legal. Proved discriminating by reverting the attribute fix: it fails and names that case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
338d0287d2 |
fix(checker): anchor the test attribute — #[cfg(test)] contains 'test'
Seventh silent success, found by CC-command on PR #41, and the THIRD instance in this campaign of one class: matching a substring where an exact token was meant. 'test' in s also matched '#[cfg(test)]', so every cfg-gated fixture was accepted as a test by --check-evidence — a gate written to reject exactly those. Two plain fixtures cleared it: Host::model and Sessions::apply_caught_up. The first instance was the bd dead-ID audit's unanchored exclusion list, where a real ticket improvise-gp12 would have been silently excluded because it starts with a crate name. The second was --check-evidence accepting any fn. This is the third. Now matched by exact attribute path (test, tokio::test, async_std::test, wasm_bindgen_test). Fault-injected: a #[cfg(test)] fn under a ### heading is reported as an orphan, a #[test] fn is not. Re-ran over the five merged units: 215 entries still pass, so none of them relied on the hole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
590b20ba2e |
fix(checker): fence state and exemption are two facts, not one
Sixth silent success, and I introduced it fixing the fourth. Found by review. Deciding 'are we inside a fence' and 'is this fence exempt' from one flag meant a ```text block's BARE CLOSING ``` looked runnable and opened an exemption -- so prose after a non-runnable fence evaded the cap entirely. Reproduced: four prose lines after a closed ```text block reported clean. They are now tracked separately, and the delimiters belong to the block they bound, so a runnable fence's closing ``` is exempt too -- otherwise it lands in the prose run and a genuine two-line doc reads as three. Fault-injected both directions: prose after a closed ```text block is a violation; a runnable block with one prose line either side is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
1045345959 |
fix(checker): a proptest! block ends, and so should the flag that accepts it
Fifth silent success in this file, found by review on PR #40. in_proptest was set on the first 'proptest!' line and NEVER CLEARED, so every later fn in that file -- helpers included -- was accepted as a test by --check-evidence. A file with one property test made the whole file's functions eligible, which is exactly the hole the any-fn fix was meant to close. Now the block's braces are tracked and the flag leaves when they balance. Fault-injected: a helper defined after a proptest! block is reported as an orphan, while the #[test] inside the block still passes. Five silent successes in one small checker in one day, every one failing in the direction that looks like passing, and none found by the person who wrote it. The gate and the thing it gates were written by the same hand and nobody injected a fault -- which is the wave's own finding turned on its tooling. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |