From 02b8cf71e9ae298f2826ac1b7960623d8f0a9088 Mon Sep 17 00:00:00 2001 From: jun Date: Sat, 5 Sep 2026 06:01:18 +0900 Subject: [PATCH 1/2] docs(devlog): close out the 429 failover unit honestly --- .../090_outcome.md | 17 +++++++--- .../091_post_merge_audit.md | 32 +++++++++++++++++++ 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/devlog/_fin/260905_always_on_429_failover/090_outcome.md b/devlog/_fin/260905_always_on_429_failover/090_outcome.md index c942b6b908..3de38c8936 100644 --- a/devlog/_fin/260905_always_on_429_failover/090_outcome.md +++ b/devlog/_fin/260905_always_on_429_failover/090_outcome.md @@ -1,15 +1,24 @@ # 090 — Outcome -Shipped in three pull requests: +Shipped in eight pull requests: | PR | Merge | What | |---|---|---| | [#3495](https://github.com/lidge-jun/opencodex/pull/3495) | `56a084aa9` | the failover fix itself | | [#3499](https://github.com/lidge-jun/opencodex/pull/3499) | `26a2e512a` | GUI copy the fix invalidated | | [#3503](https://github.com/lidge-jun/opencodex/pull/3503) | `6edc56328` | a per-request store read #3495 introduced | - -The second and third were not planned. Both were found by auditing the merged result against -the tree rather than against the plan, and both are recorded in `091`. +| [#3512](https://github.com/lidge-jun/opencodex/pull/3512) | `c91c8c5b2` | rotator-set contract test | +| [#3517](https://github.com/lidge-jun/opencodex/pull/3517) | `9be23dc41` | the `inert` DTO marker, rescoped | +| [#3520](https://github.com/lidge-jun/opencodex/pull/3520) | `5d10a1900` | public docs, 8 locales | +| [#3523](https://github.com/lidge-jun/opencodex/pull/3523) | `69d35a736` | the last stale guide + re-gating guard | +| [#3526](https://github.com/lidge-jun/opencodex/pull/3526) | `99fc38c39` | a duplicated test file | + +**Only the first was planned.** Every other one came from auditing the merged result against +the tree rather than against the plan — the plan's own criteria were satisfied after #3495. +Two were defects the fix itself created (#3499, #3503), three were surfaces still describing the +old contract (#3517, #3520, #3523), one closed the structural gap that let this unit ship two +subset-rotator loops (#3512), and one cleaned up after a collision with concurrent maintainer +work (#3526). All are recorded in `091`. ## What changed diff --git a/devlog/_fin/260905_always_on_429_failover/091_post_merge_audit.md b/devlog/_fin/260905_always_on_429_failover/091_post_merge_audit.md index 8c1e360862..490040325d 100644 --- a/devlog/_fin/260905_always_on_429_failover/091_post_merge_audit.md +++ b/devlog/_fin/260905_always_on_429_failover/091_post_merge_audit.md @@ -55,3 +55,35 @@ resource pressure that was timing the job out. Rebasing onto it turned macOS gre **Rule:** when a rerun fails the same way twice, stop rerunning and check whether the base branch already carries the fix. A stale branch point is a cause, not a flake. + +## A merge I should not have made + +I merged #3523 on `gh pr checks` reporting five passes. The test and macOS jobs were still +**queued** — that command lists only the check runs GitHub has reported so far, so a partial set +reads exactly like a complete green one. A count of passes is not a statement that anything +finished. + +The post-merge run on `dev` then showed `ci failure`, which was a genuinely alarming way to find +out. It turned out to be cancellation by the maintainer's next merge two minutes later, not a +real failure — every job read `cancelled`, not `failure`. + +**Rule:** verify with the check-runs API and require zero `null` conclusions, not a pass count: + +```bash +gh api repos///commits//check-runs \ + --jq '[.check_runs[] | .conclusion] | group_by(.) | map({(.[0]//"null"): length}) | add' +``` + +A clean result looks like `{"skipped":3,"success":24}` — no `null` key at all. + +The near-miss paid for itself: sweeping `dev` afterwards found a real defect. #3511 and #3513 +landed concurrently, one moving `anthropic-quorum-cache.test.ts` into `tests/routing/` and the +other placing a copy in `tests/adapters/anthropic/`. Different paths, so git saw no conflict and +both survived — a byte-identical duplicate running the same six tests twice. Removed in #3526. + +## Reviewer credit + +CodeRabbit caught that the first Claude Code guide assertion was too weak: requiring the intro to +mention `429` and carry emphasis is satisfied by the **original stale sentence**, so a revert +would have passed. Each locale now bans the phrase pattern that actually attributed failover to +the pool, and the test was driven red against the restored sentence before committing. From 6a5607ee7e2f60882b49ee571894dec56eccf23b Mon Sep 17 00:00:00 2001 From: jun Date: Sat, 5 Sep 2026 06:02:12 +0900 Subject: [PATCH 2/2] test(hygiene): fail on two test files sharing a basename #3511 and #3513 landed the same day and independently relocated one file to different domain directories. Different paths, so git saw no conflict and both copies survived -- a byte-identical duplicate running its suite twice, invisible until someone listed the tree by hand. A duplicated basename is also how a real fix goes stale: an author edits one copy, CI keeps running both, and the stale one silently disagrees. Driven red against the exact duplicate before committing. --- tests/repo-hygiene.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/repo-hygiene.test.ts b/tests/repo-hygiene.test.ts index 899cf49cc2..47f7c3eb09 100644 --- a/tests/repo-hygiene.test.ts +++ b/tests/repo-hygiene.test.ts @@ -264,3 +264,27 @@ describe("devlog is tracked, with no submodule left behind", () => { expect(missing).toEqual([]); }); }); + +describe("test layout", () => { + test("no two test files share a basename", () => { + // Two reorganizations landed on the same day (#3511, #3513) and independently relocated the + // same file to different domain directories. Different paths, so git saw no conflict and both + // copies survived -- a byte-identical duplicate running its suite twice, invisible until + // someone listed the tree by hand. + // + // A duplicated basename is also how a real fix goes stale: an author edits one copy, CI keeps + // running both, and the stale one silently disagrees. Names are the only thing a human uses + // to find a test, so they have to be unique. + const byName = new Map(); + for (const path of trackedFiles()) { + if (!path.startsWith("tests/")) continue; + if (!path.endsWith(".test.ts") && !path.endsWith(".test.tsx")) continue; + const name = path.split("/").pop()!; + byName.set(name, [...(byName.get(name) ?? []), path]); + } + const duplicates = [...byName.entries()] + .filter(([, paths]) => paths.length > 1) + .map(([name, paths]) => `${name}: ${paths.join(", ")}`); + expect(duplicates).toEqual([]); + }); +});