diff --git a/AGENTS.md b/AGENTS.md index 9d16e12c..43be4068 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -263,6 +263,7 @@ These cross-cutting decisions span multiple files. Reading individual chip docs - **Never write a commit body with `git commit -m` in this harness.** The shell is zsh, so **backticks are command substitution** and `` is an input redirect. A v2.4.0 message documenting three `gh` invocations lost all three to substitution and emitted `no such file or directory: owner` from a literal `repos///...` — the commit succeeded with a mangled body reading "recorded, because it is reusable: RETURNS THE PULL REQUEST". This project's house style puts command examples in commit bodies routinely, so `-m` is structurally the wrong tool: write the message to a file and use `git commit -F`. After amending, grep the message for each phrase that was supposed to survive. - **A test that reimplements its subject is testing itself, and it will agree forever.** v2.4.0 hit this in a test written *for a review finding*: it declared a local `fn strip(t) { t.trim_start_matches([' ', '*']) }` and asserted against that, so deleting the production stripping came back **NOT CAUGHT**. Only the mutation pass could see it — the test passed, read correctly, and covered nothing. The fix is the one this release needed three times over: **extract the decision into a named item both the code and the test call.** The other two were `atomic_write`'s injectable rename predicate (with it hard-wired, the exhaustion branch is unreachable on Unix and a mutation returning `Ok(())` for a save that never happened went uncaught) and `TimelineWatch` (`DebuggerOverlay::new` needs a window and a wgpu device, so nothing living only inside it is unit-testable). In all three the code **read** as testable beforehand. - **Never byte-slice in a panic or format path.** `&text[at..(at + 24).min(text.len())]` panics when the offset lands inside a multi-byte character, and these documents are full of em-dashes and arrows — so the audit would crash *while formatting the diagnostic*, replacing the message explaining the real failure with a char-boundary error about the reporting code. **A diagnostic that can crash the diagnosis is worse than none**, because the failure it exists to explain becomes harder to read than if the excerpt were omitted. Use `s.chars().take(n).collect::()`. +- **Two separately-verified facts do not verify the claim you assemble from them — and `AGENTS.md` is a notes file, not an oracle.** v2.4.1's published release notes carried a section headed "a claim v2.3.9 made that v2.3.9 did not ship", asserting that v2.3.9's release body *described* `release_anchor_audit.rs` while the tag did not contain it. **False, and it shipped to users.** Both inputs were real and each was checked: the file genuinely is absent from the tag (`git cat-file -e v2.3.9:crates/rustynes-test-harness/tests/release_anchor_audit.rs` fails), and v2.3.9 genuinely is where the anchor drift was found. The conjunction was never checked, and it was **one command away** — `gh release view v2.3.9 --json body`. The body says the eight drifted documents "All now read v2.3.9", a MANUAL re-synchronisation which IS in its tag, and mentions no audit, no gate, no "15 anchors" anywhere. A hand fix followed by a later commit adding the mechanism is ordinary sequencing, not an overclaiming release note. **Two propagation mechanisms matter more than the instance.** First, the sentence was written into `AGENTS.md` during the v2.4.0 work and the v2.4.1 summary was then drafted FROM `AGENTS.md` — so an unverified claim entering this file is laundered into a verified one by the next release that quotes it; treat every factual assertion here as re-checkable, and re-check it before it reaches a user-facing surface. Second, the claim had the shape this project rewards — a recursive irony, "the same failure one level up" — and that shape is exactly what suppresses the check, because it reads as an insight rather than as an assertion needing evidence. **The bar: before writing that a document, release, commit or person SAYS something, open it and grep for the words.** Corrected in #431, and retracted in place rather than deleted, because the claim was an accusation against a shipped release and silently removing it would erase the record that it was made. - **Verify a reviewer's claim before writing the fix, especially when their other findings were right.** On #427 a reviewer stated `starts_with("[workspace.package]")` also matches `[workspace.package.metadata]`. Plausible, a real class of bug, and the fix plus a commit body describing "the regression I introduced" were written before it was tested. It is **false**: the literal ends with `]` and the sub-table has `.` there, so the match is `false`; injecting such a sub-table and running the audit reads `2.3.9` under both forms. The same reviewer's two other findings that pass were both correct — which is exactly what makes the third easy to wave through. Adopt the change if it is better anyway (it was), but write down what is *true*, not a fix for a bug that never existed. - **The libretro "let-chains are unstable" review claim is FALSE and has now been raised seven times.** `if let Some(x) = e && cond` is stable in **edition 2024**, which this workspace uses on a pinned stable 1.96.0; the identical construct has been in `libretro_info_audit.rs` on `main` since v2.3.5; and CI's `fmt + clippy + rustdoc` job has compiled it green at five distinct SHAs. Do not "fix" it. Refute with the edition, the existing site, and the green lint job **on the current SHA** rather than by reference to earlier ones. - **The workspace cannot carry a SemVer pre-release version.** Setting `[workspace.package] version = "2.3.9-rc.1"` fails before any test runs: `failed to select a version for the requirement rustynes-apu = "^2.0.0" / candidate versions found which didn't match: 2.3.9-rc.1`. A caret requirement does not match a pre-release, so every intra-workspace dependency would have to be rewritten first. Relevant when reasoning about version parsing — `release_anchor_audit.rs` guards the case anyway, and its `version_core` is tested directly because the manifest route is unreachable.