From 98ae355156363052e84bef7a43ac68029fc4e552 Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 02:26:55 +0000 Subject: [PATCH 01/12] docs(agents): require an oracle, not a model of one, for deciding predicates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #283's ca-trust guard merged with two approvals and independent verification of every blocker, and is wrong in both directions on main — a CRL in a bundle voids the whole file, and our CA relabelled TRUSTED CERTIFICATE passes a check node's loader would fail. Neither reviewer ever fed the guard a realistic bundle. The suite was green because it exercised a hand-copied twin: mutating the shipped guard to accept unconditionally left it at 12/12. Adds the rule and its two corollaries — the oracle must be the same API production calls, and a test must be proven to reach the shipped code before it counts as evidence — plus the phrasing rule for universal claims like "conservative, never permissive". Widened past trust decisions to any predicate that predicts another program, per AI Team Lead: git push --dry-run against a branch ruleset is the same failure with a different oracle. --- AGENTS.md | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 6c0764a9..19caf7ff 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,6 +150,75 @@ When several reviewers are on one PR, the classes are what make their findings cheap to reconcile — *Measured* from one and *Read* from another on the same point is a signal, not a contradiction. +## Predicates That Predict Another Program + +**When a change decides trust, admission, or rejection — or more +generally when any predicate's job is to predict another program's +behavior — the review must run adversarial inputs through the real +decision path and compare against the oracle production actually uses. +A code read is not sufficient evidence for a claim about which inputs +it accepts.** + +Two corollaries. They are the load-bearing part; the rule above them +would not have caught the case that produced it. + +- **A. The oracle must be the *same API* production calls, not merely + a real one.** A real check through the wrong entry point is a green + test that certifies nothing. +- **B. Before relying on a test as evidence, verify it reaches the + shipped code — mutate the code and confirm the test fails.** + +**"We have tests" is the most common form the reassurance takes, and +it is the one that failed here.** Measured on merged `23346ac9`: +mutating the launcher's CA guard to accept unconditionally left +`test/proxy-forward-ca.test.mjs` passing **12/12**, because the test +exercised a hand-copied twin of the logic rather than the shipped +function. Both reviewers counted that suite as reassurance. + +### Phrasing + +Claims of the form *"conservative, never permissive"* / *"fails +closed"* / *"cannot accept X"* are universal quantifications over an +input space. State them **Measured with the input set named**, or +downgrade to a floor: *"these shapes were checked; the set is not +known to be complete."* + +### Why this rule exists + +PR #283's `ca-trust` guard merged with two approvals and independent +verification of every blocker. It is wrong in **both** directions, and +both reproduce on `main`: + +- **False reject** — `new X509Certificate(block)` runs on every PEM + block and the throw escapes; one CRL in a bundle voids the whole + file. Rejection is not the safe direction: the fallback drops every + sibling CA, which is the failure the contract exists to prevent. +- **False accept** — `X509Certificate` ignores the PEM label, so our + CA relabelled `TRUSTED CERTIFICATE` yields byte-identical DER and + passes, while node's loader skips any block not labelled exactly + `CERTIFICATE`. + +We were not careless. We measured a write→rename race at 0.88 ms over +5,000 iterations, grepped the rendezvous path, checked file modes — +**and never fed the guard a realistic bundle.** The shape to watch for +is *verifying the checkable parts and reasoning about the deciding +part*, and it is invisible from inside because the deciding function +usually looks readable. + +Round counts on that one predicate: 5 rounds, 3 parties, each finding +shapes the last missed. When that pattern appears, stop asking whether +reviewers were diligent and ask whether the design is a model of an +oracle that already exists. + +### Where else it applies + +The CA guard is one instance. The unifying property is that **the +oracle exists and we chose to model it instead of calling it.** Also +in this class: `git push --dry-run` as a test of a branch ruleset (it +reports success against a ruleset the server never consulted — read +`gh api repos///rulesets` instead); a schema pre-check ahead of +a strict parse; any validator predicting a downstream parser. + ## Anti-Bloat Lens (no-directive PRs) The global baseline's bloat bar is *"larger than the directive's From 2ccfc7142ea8f2da17c0110cd61d16d96d803571 Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:37:03 +0000 Subject: [PATCH 02/12] docs(agents): read the project's own history before reviewing a diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five rounds on the CA guard argued node CA-loader semantics. The client stopped being node at CC v2.1.113 — documented in AGENTS.md, README.md, and CHANGELOG.md, and the reason the NODE_OPTIONS preload died and this proxy exists. Every round had it available; none consulted it. Also adds the expectations rule: mutating the code to prove a test reaches it is not enough when the test asserts what another program does — the expected value must have come from that program. A shape-table row recorded the predicate's own behaviour as the expectation and five green rounds re-certified it. --- AGENTS.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 19caf7ff..5bc4b2ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -210,6 +210,38 @@ shapes the last missed. When that pattern appears, stop asking whether reviewers were diligent and ask whether the design is a model of an oracle that already exists. +### Read the README before reviewing the code + +**When a change depends on what some other program does — a runtime, a +client, an upstream API — read this repo's own README and CHANGELOG +history for that program before reviewing the diff.** The project's +accumulated knowledge of it lives there, and a reviewer who skips it +re-derives from the code alone and will re-derive wrong. + +Concretely: five review rounds on the CA guard argued about node's CA +loader semantics. The client stopped being node at CC v2.1.113 — the +Bun binary switch — which is documented in this file, in +`README.md`, in `CHANGELOG.md`, and is the reason the `NODE_OPTIONS` +preload was abandoned and this proxy exists in its current form. Every +round had that available and none consulted it. The launcher comment at +`bin/claude-via-proxy.mjs:329` still says *"never that **Node** will +verify a given leaf with it."* + +The failure is not that the fact was hidden. It is that reviewing a +diff invites reasoning from the diff, and project history is exactly +the context a diff does not carry. + +### The expectations are part of what gets checked + +Corollary B says mutate the code to prove the test reaches it. That is +not sufficient on its own: **when a test asserts what another program +does, the expectation itself must have come from that program.** + +A row in the CA guard's shape table recorded the predicate's own +behaviour as the expected value. Every round compared the code to the +table; no round compared the table to node. Five rounds re-certified a +wrong expectation, and the suite was green throughout. + ### Where else it applies The CA guard is one instance. The unifying property is that **the From 421ea0ef5609a14303861cca407fc12ef113e391 Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:09:41 +0000 Subject: [PATCH 03/12] docs(agents): look at CI, and say which runtime you measured on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every approval on #296 — two reviewers, three label applications — was granted while CI was cancelled or still running. Nobody looked. The matrix was sitting on a defect that hangs the suite forever on two of three supported runtimes. The local runs did not catch it either, and could not have: the suite passed 1543/1543 across thirteen runs on node 24, and both reviewers ran it too. On node 20 all 36 CA tests pass and the process never exits. package.json declares engines >=18; npx node@20 --test takes a minute. Both rules land in Evidence Class because that is the failure: the measurement was real, honestly reported, and certified nothing about two thirds of the supported surface — the same family as running a genuine handshake through an API production does not call. --- AGENTS.md | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5bc4b2ce..87aca6b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,6 +128,49 @@ This binds every reviewer — Codex, the implementation agent's own maintainer comments, and any third model added later. A finding relayed without its class is not usable by the next reader. +### Look at CI before approving + +**Check the status rollup on the head you are approving. Cancelled, +pending, queued, and absent are all "not green."** Say which you saw. + +This is the cheapest control available and the easiest to skip, because +a PR page shows a review box and a checks box and only one of them asks +for your attention. + +Measured on #296: every approval on that PR — two reviewers, three +label applications — was granted while CI was either **cancelled** or +**still running**. Nobody looked. The matrix was at that moment sitting +on a defect that made the suite hang forever on two of the three +supported runtimes. + +Approving ahead of CI is sometimes right; a fork PR whose workflow needs +maintainer authorisation cannot go green before someone acts. Then say +so — *"approved with CI pending, on the following local run"* — so the +next reader knows the checkmark was not part of the evidence. + +### A local run is evidence only for the runtime it ran on + +**State the runtime beside the count.** `1543/1543` is not a result; +`1543/1543 on node v24.11.1` is. + +When the package declares an `engines` range, exercise the **floor** +before approving, not just whatever is on your PATH. A version-specific +defect is invisible to any number of runs on one version — repetition +measures flakiness, not portability. + +Measured on #296: the full suite passed 1543/1543 across thirteen local +runs on node 24, and both independent reviewers ran it too. On node 20 +all 36 CA tests pass and **the process never exits** — a child process +handle that node 24 reaps and node ≤20 does not. `npx node@20 --test` +took under a minute and would have caught it. `package.json` declares +`engines: >=18`. + +This is an Evidence Class failure of exactly the shape this section +exists for: the measurement was real, honestly reported, and certified +nothing about two thirds of the supported surface. Same family as +running a genuine TLS handshake through the API production does not +call. + ### Why this rule exists On PR #270 the reviewer **endorsed** a claim about agent-id availability From 0a92e8dc81e5b72c2fc0a99be4abdae3eaa5bd4d Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:34:30 +0000 Subject: [PATCH 04/12] docs(agents): anchor the launcher citation to the commit that carried it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #296 rewrote the guard, so bin/claude-via-proxy.mjs:329 no longer holds the comment the rule quotes. Cite 23346ac — the merged commit where the defect lived — and quote it in full, since the point is that the function documented its own limitation and five rounds read past it. A live line number in a rule about verifying citations was going to rot on the next touch of that file. The historical anchor cannot. --- AGENTS.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 87aca6b1..22a7b35f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -266,9 +266,13 @@ loader semantics. The client stopped being node at CC v2.1.113 — the Bun binary switch — which is documented in this file, in `README.md`, in `CHANGELOG.md`, and is the reason the `NODE_OPTIONS` preload was abandoned and this proxy exists in its current form. Every -round had that available and none consulted it. The launcher comment at -`bin/claude-via-proxy.mjs:329` still says *"never that **Node** will -verify a given leaf with it."* +round had that available and none consulted it. The launcher said so +about itself, too: until #296 replaced it, the guard carried the comment +*"Still only a pre-flight guard, not proof… never that **Node** will +verify a given leaf with it. Only a handshake shows that, and the +launcher does not perform one"* (`23346ac:bin/claude-via-proxy.mjs`, +since rewritten). Five rounds read past a function that documented its +own limitation. The failure is not that the fact was hidden. It is that reviewing a diff invites reasoning from the diff, and project history is exactly From ad8e28a5ce7db86f922f0a90a6d72e15d54cddc6 Mon Sep 17 00:00:00 2001 From: "vsits-codex-review-agent[bot]" <279859562+vsits-codex-review-agent[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:39:14 +0000 Subject: [PATCH 05/12] docs(code-review): add PR 300 round 1 codex review --- docs/code-reviews/pr-300-round-1-codex.md | 32 +++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 docs/code-reviews/pr-300-round-1-codex.md diff --git a/docs/code-reviews/pr-300-round-1-codex.md b/docs/code-reviews/pr-300-round-1-codex.md new file mode 100644 index 00000000..5f9b5e64 --- /dev/null +++ b/docs/code-reviews/pr-300-round-1-codex.md @@ -0,0 +1,32 @@ +# Review: PR #300 + +Date: 2026-08-04 +Reviewed: `AGENTS.md` at `0a92e8dc81e5b72c2fc0a99be4abdae3eaa5bd4d` +Round: 1 +Label applied: changes-requested + +## What Is Correct +- Measured: the new predicate rule itself is justified by the historical test gap. I checked merged `23346ac9`, installed that tree's dependencies in a disposable checkout, ran `node --test test/proxy-forward-ca.test.mjs` (`12/12` pass), then mutated the inline launcher guard in `bin/claude-via-proxy.mjs` to accept unconditionally and reran the same file; it still passed `12/12`. That supports [AGENTS.md:214-219](AGENTS.md#L214) as written. +- Read + Measured: the historical launcher quote is anchored correctly now. `git show 23346ac9:bin/claude-via-proxy.mjs` contains the quoted comment at the cited commit, so the switch from a live line number to a historical commit anchor in [AGENTS.md:270-274](AGENTS.md#L270) is the right repair. +- Read: the repo-history premise for the Bun switch is supported. The same `v2.1.113` / Bun fact appears in [README.md:7](README.md#L7), [README.md:694](README.md#L694), [CHANGELOG.md:471](CHANGELOG.md#L471), and [AGENTS.md:52](AGENTS.md#L52), so [AGENTS.md:264-268](AGENTS.md#L264) is grounded in repo artifacts rather than in reviewer memory. +- Read: the pre-existing agent-id figures in [AGENTS.md:176-180](AGENTS.md#L176) are sourceable to the directive history at [docs/directives/proxy-session-budget-attribution.md:102](docs/directives/proxy-session-budget-attribution.md#L102) and [docs/directives/proxy-session-budget-attribution.md:104](docs/directives/proxy-session-budget-attribution.md#L104). I did not independently rerun those external log queries here, so I am treating them as sourced, not freshly re-measured. +- Measured: the runtime-floor paragraph is directionally supported. `package.json` declares `engines.node: ">=18"` at [package.json:25](package.json#L25), and the #296 issue thread records `node 24.11.1 1543/1543, exits clean` plus `node 20.20.2 36/36 pass, NEVER EXITS` in the discussion that led to these rules. + +## Blockers +- Measured + Read: the new CI paragraph overstates the historical record and is false as written. [AGENTS.md:140-143](AGENTS.md#L140) says that on PR #296 "every approval on that PR ... was granted while CI was either cancelled or still running." `gh api repos/cnighswonger/claude-code-cache-fix/pulls/296/reviews --paginate` and `gh pr view 296 --json statusCheckRollup` show two final approvals on `2026-08-04`: `vsits-codex-review-agent[bot]` at `19:13:15Z` and `cnighswonger` at `19:18:28Z`, both after the matrix had completed successfully at `17:56Z`. The rule is sound; the example is not. A document about evidence discipline cannot keep a measured example that fails measurement. +- Read: several of the added justifications state reviewer thought-processes as facts when the artifacts only show outcomes. "Nobody looked" in [AGENTS.md:142](AGENTS.md#L142), "none consulted it" in [AGENTS.md:268-269](AGENTS.md#L268), and "Five rounds read past" in [AGENTS.md:274](AGENTS.md#L274) are not mechanically verifiable from repo or GitHub artifacts. At most, the artifacts show approvals before green, missing mention of the Bun switch, or arguments focused on node semantics. That distinction matters because this PR is adding rules about evidence classes and falsifiability; it should not itself rely on mind-reading. Rephrase these to observable claims. + +## What Needs Attention +- Read: the CI escape hatch in [AGENTS.md:146-149](AGENTS.md#L146) is acceptable only because it requires the reviewer to say explicitly that CI was pending and to name the local run. Keep that reporting requirement if you revise the section; without it, the exception would become too permissive. +- Read: anti-bloat is mostly fine here. The added sections are not redundant with the earlier Evidence Class text; the predicate/oracle rule, README-history rule, and expectation-source rule each close a distinct failure mode. My concern is accuracy, not size. + +## Bloat / Non-Functional +None. + +## Recommendations +- Rewrite the #296 CI example to the narrower claim the artifacts actually support. For example: earlier approvals/labels were granted while CI was cancelled or running, but the final approvals on August 4, 2026 landed after green. That still teaches the rule without misstating the timeline. +- Replace mind-reading phrases with observable ones: "the reviews did not cite the check status," "the review thread argued node-loader semantics despite the Bun switch being documented," or similar. +- If you want the agent-id figures to carry "Measured" weight in this file rather than merely sourced weight, add a pointer to the underlying query artifact or log note. Right now they are traceable, but not reproducible from this branch alone. + +## Bottom Line +Revise. Most of the new rules survive scrutiny, and the strongest new example — the `23346ac9` mutation staying green at `12/12` — reproduced exactly. But one of the added measured examples is false as written, and several surrounding sentences assert reviewer mental state rather than observable evidence. For a PR whose whole point is to raise the bar on verification language, those are blocking problems. — Codex review From 7053366daa60180f6267744fed68a63281a5cf46 Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:43:26 +0000 Subject: [PATCH 06/12] docs(agents): fix the CI example, and stop asserting what reviewers thought Codex round 1 found the CI paragraph false as written. Verified: on #296, four of six approvals landed against a cancelled or still-running matrix, but the final two on 5e6a2e04 came after CI went green at 17:56Z. "Every approval" did not survive measurement. Replaced with the per-head table, which teaches the rule and includes the head where it was followed. A document about evidence discipline cannot keep an example that fails its own standard. Also replaced three unfalsifiable claims about reviewer mental state with what the artifacts actually show: "no round's written findings mention the Bun switch" rather than "none consulted it"; "no round's findings quote or answer it" rather than "five rounds read past"; and "both reviews cited the pass count, neither established it reached the shipped guard" rather than "counted that suite as reassurance". Same lesson, checkable. --- AGENTS.md | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 22a7b35f..7aa4b446 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,11 +137,21 @@ This is the cheapest control available and the easiest to skip, because a PR page shows a review box and a checks box and only one of them asks for your attention. -Measured on #296: every approval on that PR — two reviewers, three -label applications — was granted while CI was either **cancelled** or -**still running**. Nobody looked. The matrix was at that moment sitting -on a defect that made the suite hang forever on two of the three -supported runtimes. +Measured on #296, from the review and workflow timestamps: + +``` +head CI approvals green first? +82c9f27e cancelled 11:06Z 10:46Z, 15:18Z no +4a32d142 never completed 16:39Z, 17:18Z no +5e6a2e04 success 17:56Z 19:13Z, 19:18Z yes +``` + +Four of the six approvals landed against a matrix that was cancelled or +still running, and no review on those two heads cited a check status. +`4a32d142` was approved twice while its run sat `in_progress` — on a +defect that made the suite hang forever on two of the three supported +runtimes, which is why that run never finished. The last head is what +the rule looks like when it is followed. Approving ahead of CI is sometimes right; a fork PR whose workflow needs maintainer authorisation cannot go green before someone acts. Then say @@ -216,7 +226,8 @@ it is the one that failed here.** Measured on merged `23346ac9`: mutating the launcher's CA guard to accept unconditionally left `test/proxy-forward-ca.test.mjs` passing **12/12**, because the test exercised a hand-copied twin of the logic rather than the shipped -function. Both reviewers counted that suite as reassurance. +function. Both reviews cited the suite's pass count; neither established +that it reached the shipped guard. ### Phrasing @@ -261,18 +272,19 @@ history for that program before reviewing the diff.** The project's accumulated knowledge of it lives there, and a reviewer who skips it re-derives from the code alone and will re-derive wrong. -Concretely: five review rounds on the CA guard argued about node's CA -loader semantics. The client stopped being node at CC v2.1.113 — the -Bun binary switch — which is documented in this file, in -`README.md`, in `CHANGELOG.md`, and is the reason the `NODE_OPTIONS` -preload was abandoned and this proxy exists in its current form. Every -round had that available and none consulted it. The launcher said so -about itself, too: until #296 replaced it, the guard carried the comment -*"Still only a pre-flight guard, not proof… never that **Node** will -verify a given leaf with it. Only a handshake shows that, and the -launcher does not perform one"* (`23346ac:bin/claude-via-proxy.mjs`, -since rewritten). Five rounds read past a function that documented its -own limitation. +Concretely: five review rounds on the CA guard argued node CA-loader +semantics, and **no round's written findings mention the Bun switch.** +The client stopped being node at CC v2.1.113, which is documented in +this file, in `README.md`, in `CHANGELOG.md`, and is the reason the +`NODE_OPTIONS` preload was abandoned and this proxy exists in its +current form. + +The guard also documented its own limitation. Until #296 replaced it, +it carried the comment *"Still only a pre-flight guard, not proof… +never that **Node** will verify a given leaf with it. Only a handshake +shows that, and the launcher does not perform one"* +(`23346ac:bin/claude-via-proxy.mjs`, since rewritten). No round's +findings quote or answer it. The failure is not that the fact was hidden. It is that reviewing a diff invites reasoning from the diff, and project history is exactly From 6bd9f12f26b1f80a2c91ee494d706a70b26222fb Mon Sep 17 00:00:00 2001 From: "vsits-codex-review-agent[bot]" <279859562+vsits-codex-review-agent[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:53:56 +0000 Subject: [PATCH 07/12] docs(reviews): add PR 300 round 2 Codex review --- docs/code-reviews/pr-300-round-2-codex.md | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 docs/code-reviews/pr-300-round-2-codex.md diff --git a/docs/code-reviews/pr-300-round-2-codex.md b/docs/code-reviews/pr-300-round-2-codex.md new file mode 100644 index 00000000..83bb4537 --- /dev/null +++ b/docs/code-reviews/pr-300-round-2-codex.md @@ -0,0 +1,30 @@ +# Review: PR #300 + +Date: 2026-08-04 +Reviewed: `AGENTS.md` at `7053366daa60180f6267744fed68a63281a5cf46` +Round: 2 +Label applied: changes-requested + +## What Is Correct +- Measured: the revised CI table in `AGENTS.md:140-154` is materially better than round 1 and its cells match the GitHub artifacts I checked. `gh api repos/cnighswonger/claude-code-cache-fix/commits//check-runs` shows `82c9f27e` had `test (18)` / `test (20)` cancelled at `11:06Z`, `4a32d142` had `test (18)` / `test (20)` still `in_progress` at the two approval times `16:39Z` and `17:18Z`, and `5e6a2e04` had all three test jobs completed successfully by `17:56Z` before the two approvals at `19:13Z` and `19:18Z`. +- Measured: the narrower CI prose at `AGENTS.md:149-154` now tracks that table instead of repeating the false “every approval” claim from round 1. I confirmed six approval events on PR #296 via `gh api repos/cnighswonger/claude-code-cache-fix/pulls/296/reviews --paginate`. +- Read: the third row does not undercut the rule. Keeping one compliant head in the example helps because it shows the contrast the rule is trying to enforce rather than leaving the section as a pile of failures with no positive comparator. +- Measured: PR #300 itself is reviewable on this head. `gh pr checks 300 --repo cnighswonger/claude-code-cache-fix` is green on `7053366d`, and `gh api graphql` for review threads returned no open threads to resolve. + +## Blockers +- Read + Measured: `AGENTS.md:275-287` still makes two artifact claims that the written review record does not support. First, “**no round's written findings mention the Bun switch**” is false as written against the review bodies you are invoking: the approved PR #296 review at `5e6a2e04` says the head “adds the missing Bun/BoringSSL veto.” Second, “**No round's findings quote or answer it**” is not safe: PR #283 round 1 explicitly says the new tests “verify the guard against real TLS authorization outcomes,” which is at least an attempted answer to the comment’s “Only a handshake shows that” limitation, even if it later proved to be the wrong oracle. `quote` is grep-able; `answer` is both semantic and, on this record, substantively false. Because this section is itself a rule about falsifiable claims, it needs another wording pass that stays inside what the review texts demonstrably say. + +## What Needs Attention +- Read: if you want to preserve the point, I would split the current sentence into claims with different evidence standards. “No #283 review body mentions the Bun switch” is cheaply checkable. “No round quoted the launcher comment” is also checkable. The broader “or answer it” formulation is where the evidence slips. + +## Bloat / Non-Functional +- None. The delta is a tight wording correction in one document, and the retained table is proportionate because it replaces an earlier incorrect universal claim with inspectable evidence. + +## Recommendations +- Replace `AGENTS.md:275-287` with phrasing limited to the artifacts you can actually cite, for example by separating “did not mention the Bun switch” from the weaker claim that the reviews reasoned from the guard/tests rather than from the project-history context. +- Keep the third CI row. It strengthens the section by showing the compliant case on the same PR timeline. + +## Bottom Line +Revise once more. The round-1 blockers about the CI example and reviewer mental-state assertions are mostly fixed, and the CI table now stands up to direct verification. But one replacement paragraph still overstates what the historical review bodies did not say, in exactly the document that tells reviewers not to do that. I would merge this after one more tightening pass on `AGENTS.md:275-287`, not before. + +— Codex review From db9477fdf72be2c062f685a6a3fe704cf0d5079f Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:56:50 +0000 Subject: [PATCH 08/12] docs(agents): say what the #283 reviews actually said, not what they missed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 2 found the round-1 fix also overclaimed. Both blockers verified before accepting: "no round's written findings mention the Bun switch" was false — the #296 approval at 5e6a2e04 credits the head with adding "the missing Bun/BoringSSL veto". My original grep was case-insensitive /bun/, which matches "bundle"; with word boundaries the real count on #296 is 1 of 7. "No round's findings quote or answer it" was also false. #283 round 1 credits the new tests with verifying "the guard against real TLS authorization outcomes" — the handshake gap WAS noticed. It was answered through tls.connect({ca}), which is not the API the launcher uses. That is a better lesson than the one I wrote: the limitation was read, and answered with the wrong oracle. Scoped to #283, where 0 of 3 reviews mention Bun or BoringSSL, and counted honestly — one of those three has an empty body, so "every one of them reasons about X509Certificate" would have been a third overclaim. --- AGENTS.md | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7aa4b446..1fe71285 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -272,19 +272,27 @@ history for that program before reviewing the diff.** The project's accumulated knowledge of it lives there, and a reviewer who skips it re-derives from the code alone and will re-derive wrong. -Concretely: five review rounds on the CA guard argued node CA-loader -semantics, and **no round's written findings mention the Bun switch.** -The client stopped being node at CC v2.1.113, which is documented in -this file, in `README.md`, in `CHANGELOG.md`, and is the reason the -`NODE_OPTIONS` preload was abandoned and this proxy exists in its -current form. - -The guard also documented its own limitation. Until #296 replaced it, -it carried the comment *"Still only a pre-flight guard, not proof… -never that **Node** will verify a given leaf with it. Only a handshake -shows that, and the launcher does not perform one"* -(`23346ac:bin/claude-via-proxy.mjs`, since rewritten). No round's -findings quote or answer it. +Concretely, and measured against the review bodies themselves: **no +review on #283 mentions Bun or BoringSSL** (0 of 3 — two written Codex +rounds and one empty-bodied approval), while both written rounds reason +about node's `X509Certificate` and node's CA loader. The client stopped +being node at CC v2.1.113, which is documented in this file, in +`README.md`, in `CHANGELOG.md`, and is the reason the `NODE_OPTIONS` +preload was abandoned and this proxy exists in its current form. + +The guard documented its own limitation too. It carried the comment +*"Still only a pre-flight guard, not proof… never that **Node** will +verify a given leaf with it. Only a handshake shows that, and the +launcher does not perform one"* (`23346ac:bin/claude-via-proxy.mjs`, +replaced by #296). That comment was **partly answered and partly +misread**: #283 round 1 credits the new tests with verifying "the guard +against real TLS authorization outcomes," so the handshake gap was +noticed — but the handshake in question ran through `tls.connect({ca})`, +which is not the API the launcher uses. The limitation was read, and +answered with the wrong oracle. + +The runtime fact stayed unexamined for three further rounds, until it +was measured directly against the shipped binary. The failure is not that the fact was hidden. It is that reviewing a diff invites reasoning from the diff, and project history is exactly From 0e8ac7e9840a1da9fadd364cba1908276d4a987f Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:01:36 +0000 Subject: [PATCH 09/12] docs(agents): make the last two soft numbers countable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-audit ahead of round 3, on the two claims Codex has not flagged yet but which fail the same standard as the three he did. "5 rounds, 3 parties" was stale and uncountable — it predates four more reviews. Now "10 formal reviews across #283 and #296 (3 + 7)", with the gh api command that produces it. "1543/1543 across thirteen local runs" cited a number only I can attest to; nothing in the artifacts records how many times I ran it. Repetition was never the point — the runtime was. Restated as the version, plus the fact that makes it bite: CI covers 18/20/22, so node 24 is the one runtime the matrix does not cover, and it is the one everybody measured on. Also corrected the node 20 mechanism. It is not "a handle node 24 reaps and node 20 does not" — that was my first-pass diagnosis and #296 landed a better one: a positive control the tests depend on cannot be established below v22.15, so execution never reaches the teardown. --- AGENTS.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1fe71285..431ba6ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,12 +168,14 @@ before approving, not just whatever is on your PATH. A version-specific defect is invisible to any number of runs on one version — repetition measures flakiness, not portability. -Measured on #296: the full suite passed 1543/1543 across thirteen local -runs on node 24, and both independent reviewers ran it too. On node 20 -all 36 CA tests pass and **the process never exits** — a child process -handle that node 24 reaps and node ≤20 does not. `npx node@20 --test` -took under a minute and would have caught it. `package.json` declares -`engines: >=18`. +Measured on #296, and reproducible from the PR thread: the full suite +passed `1543/1543` on node v24.11.1 — repeatedly, and for both +independent reviewers. On node v20.20.2 all 36 CA tests pass and **the +process never exits**, because a positive control the tests depend on +cannot be established below v22.15. `npx node@20 --test` took under a +minute and would have caught it. `package.json` declares +`engines: >=18`, and the CI matrix is 18/20/22 — so the runtime that +everyone measured on was the one runtime CI does not cover. This is an Evidence Class failure of exactly the shape this section exists for: the measurement was real, honestly reported, and certified @@ -259,10 +261,11 @@ is *verifying the checkable parts and reasoning about the deciding part*, and it is invisible from inside because the deciding function usually looks readable. -Round counts on that one predicate: 5 rounds, 3 parties, each finding -shapes the last missed. When that pattern appears, stop asking whether -reviewers were diligent and ask whether the design is a model of an -oracle that already exists. +Round counts on that one predicate: **10 formal reviews across #283 and +#296** (3 + 7, countable via `gh api .../pulls//reviews`), by three +parties, each round finding shapes the last missed. When that pattern +appears, stop asking whether reviewers were diligent and ask whether the +design is a model of an oracle that already exists. ### Read the README before reviewing the code From d03645cf32e31ddafe9f711307da02611464bf7a Mon Sep 17 00:00:00 2001 From: "vsits-codex-review-agent[bot]" <279859562+vsits-codex-review-agent[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:05:33 +0000 Subject: [PATCH 10/12] docs(code-review): add PR 300 round 3 Codex review --- docs/code-reviews/pr-300-round-3-codex.md | 31 +++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 docs/code-reviews/pr-300-round-3-codex.md diff --git a/docs/code-reviews/pr-300-round-3-codex.md b/docs/code-reviews/pr-300-round-3-codex.md new file mode 100644 index 00000000..a4cd6549 --- /dev/null +++ b/docs/code-reviews/pr-300-round-3-codex.md @@ -0,0 +1,31 @@ +Codex review: + +# Review: PR #300 + +Date: 2026-08-04 +Reviewed: `AGENTS.md` at `0e8ac7e9840a1da9fadd364cba1908276d4a987f` +Round: 3 +Label applied: changes-requested + +## What Is Correct +- Measured: the two numeric repairs called out in the dispatch are materially better than the prior wording and mostly survive direct verification. `AGENTS.md:171-178` now names `node v24.11.1` instead of the unartifacted "thirteen runs," `package.json:25-27` declares `engines: >=18`, and `.github/workflows/test.yml:12-15` confirms the CI matrix is `18/20/22`. +- Read: the corrected node-20 mechanism is now aligned with the merged test file rather than with the earlier stale diagnosis. `test/proxy-forward-ca.test.mjs:105-109` documents that `tls.getCACertificates` arrives in `v22.15`, and the current suite gates positive-control rows on that capability instead of assuming lower runtimes can establish the same premise. +- Measured: the formal-review count in `AGENTS.md:264-265` is right as far as it goes. `gh api repos/cnighswonger/claude-code-cache-fix/pulls/283/reviews --paginate` returns `3` reviews, and the same call for `pulls/296/reviews` returns `7`, for `10` total formal reviews across the two PRs. +- Measured: PR #300 itself is green and thread-clean on this head. `gh pr checks 300 --repo cnighswonger/claude-code-cache-fix` reports `test (18)`, `test (20)`, `test (22)`, GitGuardian, and Snyk all passing at `0e8ac7e`, and `gh api graphql ... reviewThreads ...` returned no open review threads. +- Read: the document is not mush. After three tightening rounds, the added sections still read as rules with concrete examples rather than as qualification piled on qualification; I did not find a section that has become unreadable through over-hedging. + +## Blockers +- Measured + Read: `AGENTS.md:264-266` still overclaims the very review history it cites. The sentence says the `10 formal reviews across #283 and #296` were "by three parties, each round finding shapes the last missed." The same `gh api .../pulls//reviews` artifacts named in the sentence show only **two** formal reviewers across those reviews: `vsits-codex-review-agent[bot]` and `cnighswonger`. They also show that not every round "found shapes the last missed": three of the ten reviews are empty-bodied approvals, and the clean approval rounds on `#283` and `#296` are explicitly reporting no blockers. The `10` and `(3 + 7)` counts are now correct; the rest of the sentence is not. In a rule about countable claims, that remains merge-blocking. + +## What Needs Attention +- Read: I did not find another section that should be removed entirely. The CI rule, runtime rule, oracle rule, README-history rule, and expectation-source rule are all directionally sound for this repo. My remaining concern is accuracy of the review-history example, not that the guidance itself is bad. + +## Bloat / Non-Functional +- Measured: none in this round. Since `db9477f`, the production delta is a wording-only correction inside one document, and the added review artifact files are required by repo policy rather than by product surface. + +## Recommendations +- Measured + Read: tighten `AGENTS.md:264-266` to the claims the cited artifacts actually support. For example: keep `10 formal reviews across #283 and #296 (3 + 7)` and drop or restate the rest as something observable, such as "across two formal reviewers, with multiple later rounds still uncovering new defects." +- Read: keep the narrower corrected mechanism in `AGENTS.md:173-178`. That paragraph is stronger now because it points at runtime capability and CI coverage rather than at a private run count or an abandoned root-cause guess. + +## Bottom Line +Revise once more. The round-3 self-audit did fix two real problems: the review-count sentence is now partially countable, and the node-20 paragraph now points at the right class of mechanism. But one sentence still turns a correct numeric count into two unsupported claims about who reviewed and what every round did. Because this PR is codifying how to write review evidence, it should not merge with that overclaim still in place. — Codex review From 4df663c021aea8fb566a582aabea2be4c310837d Mon Sep 17 00:00:00 2001 From: "vsits-proxy-builder[bot]" <279815601+vsits-proxy-builder[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:08:03 +0000 Subject: [PATCH 11/12] docs(agents): cut the round-count sentence instead of patching it again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex round 3: the fixed count was right (10) and the same sentence still carried two false claims — "three parties" (measured: 2 review authors, 4 cnighswonger + 6 codex) and "each round finding shapes the last missed" (4 of the 10 reviews have empty bodies; several are clean approvals). Third failure on one sentence. The count was never load-bearing — the argument is that a function kept producing defects after review signed off on it, which needs no arithmetic. Replaced with that, verified: #283 was approved twice and merged before the false accept/reject were reproduced; #296 was approved at 4a32d142 before the node-hang was found. Also fixed a fourth instance of the same shape that no round had flagged, in the expectations rule: "Every round compared the code to the table; no round compared the table to node. Five rounds re-certified a wrong expectation." Now says what the record shows — it survived every review that reached it, and was found by @codeslake running the table against the real loader, reported on #296 2026-08-03. --- AGENTS.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 431ba6ad..338213eb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -261,11 +261,11 @@ is *verifying the checkable parts and reasoning about the deciding part*, and it is invisible from inside because the deciding function usually looks readable. -Round counts on that one predicate: **10 formal reviews across #283 and -#296** (3 + 7, countable via `gh api .../pulls//reviews`), by three -parties, each round finding shapes the last missed. When that pattern -appears, stop asking whether reviewers were diligent and ask whether the -design is a model of an oracle that already exists. +That predicate shipped in #283 and was still being corrected in #296 — +two PRs, and the defects above were found after both had been approved. +When a single function keeps producing new defects after review has +signed off on it, stop asking whether reviewers were diligent and ask +whether the design is a model of an oracle that already exists. ### Read the README before reviewing the code @@ -308,9 +308,10 @@ not sufficient on its own: **when a test asserts what another program does, the expectation itself must have come from that program.** A row in the CA guard's shape table recorded the predicate's own -behaviour as the expected value. Every round compared the code to the -table; no round compared the table to node. Five rounds re-certified a -wrong expectation, and the suite was green throughout. +behaviour as the expected value. It survived every review that reached +it, and was found only when @codeslake ran the table against the real +loader while building #296's oracle — reported on that PR. The suite was +green throughout. ### Where else it applies From 2bc2fea7afcd94c5bf216582d71d34a587ef1e5a Mon Sep 17 00:00:00 2001 From: "vsits-codex-review-agent[bot]" <279859562+vsits-codex-review-agent[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:19:27 +0000 Subject: [PATCH 12/12] docs(reviews): add PR 300 round 4 Codex review --- docs/code-reviews/pr-300-round-4-codex.md | 33 +++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 docs/code-reviews/pr-300-round-4-codex.md diff --git a/docs/code-reviews/pr-300-round-4-codex.md b/docs/code-reviews/pr-300-round-4-codex.md new file mode 100644 index 00000000..c2f331f9 --- /dev/null +++ b/docs/code-reviews/pr-300-round-4-codex.md @@ -0,0 +1,33 @@ +# Review: PR #300 + +Date: 2026-08-04 +Reviewed: `AGENTS.md` at `4df663c021aea8fb566a582aabea2be4c310837d` +Round: 4 +Label applied: approved-by-codex-agent + +## What Is Correct +- Measured: PR #300 is green on the head under review. `gh pr view 300 --json statusCheckRollup` on `4df663c0` shows `test (18)`, `test (20)`, `test (22)`, GitGuardian, and Snyk all successful on 2026-08-04. +- Measured: there are no open review threads left to resolve. `gh api graphql` for `reviewThreads` on PR #300 returned an empty `nodes` array. +- Read: the round-3 blocker is fixed cleanly. The review-history paragraph at `AGENTS.md:264-268` now says only what the artifacts support, and dropping the "three parties / each round" clause materially improves the document's credibility. +- Read: the added rules are still usable rather than over-qualified. `AGENTS.md:156-184`, `AGENTS.md:208-323`, and `AGENTS.md:325-369` read as concrete reviewer instructions with named failure modes, not as hedge piles that collapse into "it depends." +- Read: the CI rule's fork-PR escape hatch is honest, not a loophole. `AGENTS.md:156-159` does not waive the check-status discipline; it requires the reviewer to say explicitly that CI was pending and to name the local evidence instead. That is the right tradeoff for this repo's maintainer-authorized fork workflows. +- Read: the runtime rule scales for this repo. `AGENTS.md:161-184` does not require reviewers to enumerate runtimes gratuitously; it requires them to name the runtime when they use a pass count as evidence, and to hit the floor when the package claims an `engines` range. Given this repo's documented node-version failures, that is signal, not ritual. +- Read: the predicate/oracle rule is good advice as scoped. `AGENTS.md:208-323` is triggered when a predicate is trying to predict another program's admission or rejection behavior and repeatedly insists on the production oracle when one exists. It does not tell contributors to replace ordinary parsing with subprocesses in general; the "Where else it applies" paragraph explicitly frames the common property as "the oracle exists and we chose to model it instead of calling it." +- Read: the README/history rule is also sound as written. `AGENTS.md:270-302` says to read the repo's accumulated knowledge before reviewing a diff whose correctness depends on external program behavior; it does not say README text outranks the code under review. In this repo, where runtime changes and upstream-client facts are load-bearing and easy to miss from a diff alone, that rule is proportionate. + +## Blockers +None. + +## What Needs Attention +- Read: `AGENTS.md:297-302` is accurate but slightly more abstract than the surrounding sections. If you want one more cut without changing meaning, "The failure is not that the fact was hidden..." paragraph is the first removable candidate; the concrete Bun / BoringSSL example above it already carries the lesson. +- Read: `AGENTS.md:196-206` is useful, but the last sentence about reconciling findings is closer to meta-commentary than rule text. It is harmless, not wrong; I would trim there before trimming any of the concrete examples. + +## Bloat / Non-Functional +- Read: no blocking bloat remains in the round-4 delta. The added process text is longer than average repo guidance, but here the length is buying specific failure cases, explicit evidence standards, and concrete counterexamples that the earlier shorter baseline did not cover. + +## Recommendations +- Read: merge as written. The document is now on the right side of the accuracy/utility line, and the remaining possible cuts are editorial rather than corrective. +- Read: if you want a shorter post-merge pass, cut `AGENTS.md:297-302` first and consider compressing `AGENTS.md:196-206` to the two bullet points. Those are the only spots where removing text would preserve the binding force of the rules. + +## Bottom Line +Read + Measured: approve. On the axes that mattered for round 4, the document now holds up: the new rules are usable, they are good advice for this repo rather than cargo-cult process, and the one remaining historical overclaim from round 3 has been removed instead of patched into another arguable sentence. I would not spend another review round trying to shave this further before merge. — Codex review