From dc9e1253539cb1d3028d7b43c24b12959e9cbe5b Mon Sep 17 00:00:00 2001 From: SanHsien <34234698+SanHsien@users.noreply.github.com> Date: Sun, 23 Aug 2026 15:30:03 +0800 Subject: [PATCH] =?UTF-8?q?fix(usage,responses):=20=E5=85=A9=E8=99=95?= =?UTF-8?q?=E8=AE=80=E5=8F=96=E6=B2=92=E6=9C=89=E4=B8=8A=E9=99=90=EF=BC=8C?= =?UTF-8?q?=E4=B8=A6=E6=8A=8A=20PR/issue=20=E6=9F=A5=E6=B3=95=E6=94=B9?= =?UTF-8?q?=E6=88=90=20--state=20all?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 引用上游 dev 上的兩支修正，兩者的缺陷都在本 fork 實查確認： - #2395：readUsageEntriesIncrementally() 只擋檔案縮小，沒擋一次 append 太多。 兩次輪詢之間灌進來多少就整批讀多少並逐行解析，繞過 maxReadBytes 這個唯一的 記憶體上限——而且是在帳本長最快的時候繞過。補 4 行守衛交還給 full-tail reader。 - #2398 前半：bounded-body 的兩個進入點在 signal 已 aborted 時先 throw，此時 還沒掛 reader，原始 body 沒被結束，fetch 背後的 stream 會留著一個被拒絕的 read。補 cancelBodyWithoutWaiting()。不引用後半的 readDisplaySafeErrorText 重構（依賴 dev 的新介面），觸發條件記在 docs/fork/UPSTREAM.md。 兩條測試取自上游同 PR 並改寫；把 src 改動 stash 掉重跑確認都會紅。 查法缺口：先前查 PR/issue 用 --state open，看不到未合併就關閉的項目——那正是 上游拒收但可能對本 fork 有價值的一類。改成 --state all，水位 PR #2433、 issue #2434。同時把 fork-hygiene 裡寫死 reviewed_date 的斷言改成日期格式， 否則每做完一次審查那條測試就變紅。 typecheck 乾淨、privacy:scan 通過、fork-hygiene 18 pass、bounded-body 27 pass、 api-usage 25 pass。 Co-Authored-By: Claude Opus 5 --- docs/fork/DECISIONS.md | 16 ++++++++++++ docs/fork/UPSTREAM.md | 49 ++++++++++++++++++++++++++++++++++++ src/lib/bounded-body.ts | 24 +++++++++++++++--- src/usage/log.ts | 5 ++++ tests/api-usage.test.ts | 34 +++++++++++++++++++++++++ tests/bounded-body.test.ts | 25 ++++++++++++++++++ tests/fork-hygiene.test.ts | 6 ++++- tools/upstream_baseline.json | 6 ++--- 8 files changed, 157 insertions(+), 8 deletions(-) diff --git a/docs/fork/DECISIONS.md b/docs/fork/DECISIONS.md index 519787fbe9..14ff55469a 100644 --- a/docs/fork/DECISIONS.md +++ b/docs/fork/DECISIONS.md @@ -1,5 +1,21 @@ # 維護決策 +## 2026-08-23：PR／issue 一律 `--state all`，並在 `dev` 上選擇性引用 + +**決定**：修正 2026-08-22 那條「上游的 PR 不是本 fork 的審查單位」。審查單位仍是 `main` 上的 +release commit，但增加兩條： + +1. 查 PR 與 issue **一律用 `--state all`**。未合併就關閉的 PR 永遠不會經由 commit 路徑抵達， + 而那正是「上游拒收、但可能對本 fork 有價值」的一類。 +2. 已合併但只在 `dev` 的修正，若**本 fork 現在就會痛**，就選擇性引用，不等 release。 + +**理由**：`dev` 目前領先 `main` 69 個 commit，實測 `#2398` 的 merge commit 在 `dev` 不在 `main` +——「合併後會隨 release 進 `main`」在時間上可能是好幾週。本輪依此引用了兩支「讀取沒有上限」的 +修正（`#2395`、`#2398` 前半），兩者的缺陷都在本 fork 的程式碼裡實查確認過。 + +**維持不變**：不逐筆採用未合併的 open PR。那是提案，不是上游已接受的變更；採用等於接手維護一份 +還會變動的補丁。逐項證據與觸發條件記在 [`UPSTREAM.md`](UPSTREAM.md)。 + ## 2026-08-22：上游的 PR 與分支不是本 fork 的審查單位，issue 只追 platform **決定**：不逐筆評估上游的 open PR（43 個，全部 base 在 `dev`）與分支（71 個，多為那些 PR 的 diff --git a/docs/fork/UPSTREAM.md b/docs/fork/UPSTREAM.md index 66a94b727c..95612b1bd3 100644 --- a/docs/fork/UPSTREAM.md +++ b/docs/fork/UPSTREAM.md @@ -135,3 +135,52 @@ upstream/main...upstream/dev` 回報 **25 / 58**——`dev` 已經領先 `main` 安全性，以及本檔 issue 表裡已登記為「等上游修」的項目。 3. 挑中的先確認它只依賴 `main` 已有的檔案（`git cat-file -e upstream/main:`）， 再 cherry-pick，並記下未來同步時會衝突的檔案。 + +## 2026-08-23（第二輪）：`--state all` 補查，並引用兩支「無上限讀取」修正 + +### 先修一個查法上的缺口 + +上一輪查 PR／issue 用的是 `--state open`。那看不到**未合併就關閉**的項目，而那正是「上游拒收、 +但可能對本 fork 有價值」的一類——已合併的項目遲早會經由 commit 路徑抵達，被關掉的永遠不會。 +本輪起一律 `--state all`。 + +用 `--state all` 重查水位（PR `#2383`／issue `#2379`）之後的增量：**33 個 PR、55 個 issue**。 +其中 12 個 PR 已合併，但**合併進的是 `dev` 不是 `main`**（實測：`#2398` 的 merge commit +`383279cd2` 在 `upstream/dev`、不在 `upstream/main`；`dev` 目前領先 `main` 69 個 commit）。 +本 fork 的取用點是 `main`，所以這 12 筆**還沒**被 commit 水位涵蓋——判準仍是上一節那條： +**`dev` 上有沒有本 fork 現在就會痛的修正**。 + +### 已引用（兩支，都是「讀取沒有上限」） + +| 來源 | 本 fork 的實查證據 | 移植內容 | +| --- | --- | --- | +| [PR #2395](https://github.com/lidge-jun/opencodex/pull/2395) `fix(usage): bound incremental append reads`（`3611850c5`） | `src/usage/log.ts` 的 `readUsageEntriesIncrementally()` 只擋「檔案縮小」，沒有擋「一次 append 太多」。兩次輪詢之間灌進來的位元組不論多大，都會走增量路徑整批讀進來並逐行解析——**繞過 `maxReadBytes` 這個唯一的記憶體上限，而且是在帳本長最快的時候繞過**。 | 4 行守衛：`size - retained.coveredThroughBytes > maxReadBytes` 時回 `null`，交還給有上限的 full-tail reader。 | +| [PR #2398](https://github.com/lidge-jun/opencodex/pull/2398) `fix(responses): bound upstream error body reads`（`383279cd2`）的**前半** | `src/lib/bounded-body.ts` 的兩個進入點（`readBoundedResponseBytes:118`、`readBoundedResponseBody:211`）在 signal 已經 aborted 時**先 throw 再說**，此時還沒有 reader 掛上去，原始 body 因此沒有被結束——fetch 背後的 stream 會把一個被拒絕的 read 留著。 | 新增 `cancelBodyWithoutWaiting()`，兩個進入點在 throw 之前先結束 body。 | + +**驗證**：兩支各補一條測試（取自上游同一個 PR 的測試，改寫成本 fork 的檔案結構）。把 src 改動 +stash 掉重跑，兩條都紅（`bounded-body` 1 fail、`api-usage` 1 fail），確認測試有牙齒；改動放回 +全綠（`bounded-body` 27 pass、`api-usage` 25 pass）。 + +**不引用 #2398 的後半**（`readDisplaySafeErrorText`，`src/server/responses/core.ts` +65 行）：那是 +錯誤路徑的重構，依賴 `dev` 上的 `bounded-body` 新介面，硬移植等於自行改寫。**觸發條件**：隨 +release 進 `main`，或本線出現「上游錯誤內容被原樣顯示」的實例。 + +### 其餘 31 個 PR + +- **已合併進 `dev` 的另外 10 筆**：`devlog:` 工作紀錄與 `fix(zcode)`／`fix(auth)`／`fix(gui)` 等， + 逐條看過主旨與檔案，都不是本 fork 現在會痛的路徑；等 release。 +- **open／closed-未合併的 21 筆**：`fix(xai)`、`fix(codex)`、`fix(catalog)`、`feat(test)` 這類， + base 都在 `dev`，且改的是本 fork 沒有啟用的 provider 路徑或上游自己的測試工具鏈。它們是**提案**， + 不是上游已接受的變更；採用未合併的提案等於接手維護一份上游還沒定案的補丁。**觸發條件**：合併 + 進 `dev` 且屬「本 fork 現在會痛」那一類，或隨 release 進 `main`。 + +### 55 個 issue + +以「會不會改變本 fork 要驗什麼」為判準逐條掃過標題。多數是 provider 目錄、xAI／Kiro／OpenRouter +的路由行為與上游自己的 App 問題。沒有一條指向本 fork 已知的 Windows 行為缺口——`#2292`（Windows +model picker）那條已於本檔上一節引用 `a3bbcdb0` 解決。 + +### 水位 + +- commit：`6ae83b1`（`6ae83b1..upstream/main` 仍為 0） +- PR：**#2433**、issue：**#2434**（首次以 `--state all` 查過） diff --git a/src/lib/bounded-body.ts b/src/lib/bounded-body.ts index 1d761dd07e..0b6f921f55 100644 --- a/src/lib/bounded-body.ts +++ b/src/lib/bounded-body.ts @@ -103,6 +103,16 @@ function cancelWithoutWaiting(reader: ReadableStreamDefaultReader, r } } +function cancelBodyWithoutWaiting(body: ReadableStream, reason?: unknown): void { + // A signal can already be aborted before a reader is attached. Settle the original + // body anyway, or a fetch-backed stream keeps a rejected read alive in that gap. + try { + void body.cancel(reason).catch(() => undefined); + } catch { + // A locked or non-conforming stream may throw synchronously from cancel(). + } +} + /** * Consume the original response body as raw bytes under a strict memory ceiling. * @@ -115,9 +125,12 @@ export async function readBoundedResponseBytes( options: BoundedBytesOptions, ): Promise { const signal = options.signal; - if (signal?.aborted) throw signal.reason; - const body = response.body; + if (signal?.aborted) { + if (body) cancelBodyWithoutWaiting(body, signal.reason); + throw signal.reason; + } + if (!body) return { bytes: new Uint8Array(0), oversized: false }; const reader = body.getReader(); @@ -208,9 +221,12 @@ export async function readBoundedResponseBody( options: BoundedBodyOptions = {}, ): Promise { const signal = options.signal; - if (signal?.aborted) throw signal.reason; - const body = response.body; + if (signal?.aborted) { + if (body) cancelBodyWithoutWaiting(body, signal.reason); + throw signal.reason; + } + if (!body) { return { text: "", diff --git a/src/usage/log.ts b/src/usage/log.ts index 66654b8b74..bb7225c513 100644 --- a/src/usage/log.ts +++ b/src/usage/log.ts @@ -927,6 +927,11 @@ async function readUsageEntriesIncrementally( // A shrink means truncation or replacement-in-place; the retained rows may no // longer correspond to file contents, so refuse to extend them. if (size < retained.coveredThroughBytes) return null; + // Retained-state reuse is only an optimization. A burst larger than the configured + // window must re-anchor through the bounded full-tail reader instead of reading and + // parsing every byte appended since the previous poll -- otherwise the one path that + // exists to cap memory is bypassed exactly when the ledger grows fastest. + if (size - retained.coveredThroughBytes > maxReadBytes) return null; // Verify the retained REGION is unchanged before anything is reused. Identity keeps // dev/ino/birthtime, and an append and an in-place rewrite both move mtime/ctime // forward, so only the bytes themselves settle it. diff --git a/tests/api-usage.test.ts b/tests/api-usage.test.ts index 58a38a232b..4041d3af70 100644 --- a/tests/api-usage.test.ts +++ b/tests/api-usage.test.ts @@ -578,6 +578,40 @@ describe("GET /api/usage", () => { } }); + test("an append burst larger than the byte window falls back to a bounded full read", async () => { + // Retained-state reuse is an optimization, not a second reader. Without the size + // guard a burst bigger than the window is read and parsed in full through the + // incremental path -- bypassing the only thing that caps memory here, and doing so + // exactly when the ledger is growing fastest. Taken from upstream #2395. + const now = Date.now(); + const maxReadBytes = 512; + const row = (id: string): string => `${JSON.stringify({ + requestId: id, + timestamp: now, + provider: "openai", + model: "gpt-5.5", + status: 200, + durationMs: 1, + usageStatus: "reported", + usage: { inputTokens: 1, outputTokens: 1 }, + totalTokens: 2, + })}\n`; + const path = join(testDir, "usage.jsonl"); + writeFileSync(path, row("seed")); + + await usageLogModule.readUsageSnapshotForManagement(maxReadBytes); + const parsedBeforeBurst = usageReadCacheStatsForTests().parsedLines; + appendFileSync(path, Array.from({ length: 100 }, (_, index) => row(`burst-${index}`)).join("")); + + const snapshot = await usageLogModule.readUsageSnapshotForManagement(maxReadBytes); + const stats = usageReadCacheStatsForTests(); + expect(stats.fullReads).toBe(2); + expect(stats.tailReads).toBe(0); + expect(stats.parsedLines - parsedBeforeBurst).toBe(snapshot.entries.length); + expect(snapshot.entries.length).toBeLessThan(100); + expect(snapshot.entries.some(entry => entry.requestId === "burst-99")).toBe(true); + }); + test("appends to an over-window ledger stay incremental and bounded", async () => { const now = Date.now(); writeFixture(now); diff --git a/tests/bounded-body.test.ts b/tests/bounded-body.test.ts index f1ce47df55..0bf1b0f54f 100644 --- a/tests/bounded-body.test.ts +++ b/tests/bounded-body.test.ts @@ -312,6 +312,31 @@ describe("readBoundedResponseBody", () => { } }); + test("an already-aborted signal still settles the original response body", async () => { + // The early return threw before a reader was ever attached, so a fetch-backed + // stream kept a rejected read alive in that gap. Taken from upstream #2398. + let cancelReason: unknown; + const body = new ReadableStream({ + pull() { return new Promise(() => {}); }, + cancel(reason) { cancelReason = reason; }, + }, { highWaterMark: 0 }); + const response = new Response(body); + const controller = new AbortController(); + const reason = { code: "already-stopped" }; + controller.abort(reason); + + let caught: unknown; + try { + await readBoundedResponseBody(response, { signal: controller.signal }); + } catch (error) { + caught = error; + } + await Promise.resolve(); + + expect(caught).toBe(reason); + expect(cancelReason).toBe(reason); + }); + test("parent abort wins when EOF settles in the same turn", async () => { const parent = new AbortController(); const reason = new Error("same-turn cancel"); diff --git a/tests/fork-hygiene.test.ts b/tests/fork-hygiene.test.ts index 8def1a0bf4..ba412e054c 100644 --- a/tests/fork-hygiene.test.ts +++ b/tests/fork-hygiene.test.ts @@ -134,7 +134,11 @@ describe("upstream checker", () => { expect(baseline.repo.endsWith("opencodex.git")).toBe(true); expect(baseline.branch).toBe("main"); expect(baseline.reviewed_through).toHaveLength(40); - expect(baseline.reviewed_date).toBe("2026-08-22"); + // Not a frozen value: this date moves every time the upstream review runs, and + // pinning it turns a completed review into a red test. What matters is that it + // is a real ISO date and that the decisions record carries it -- the test below + // checks the second half. + expect(baseline.reviewed_date).toMatch(/^\d{4}-\d{2}-\d{2}$/); }); test("workflow is scheduled and fails on unreviewed commits", async () => { diff --git a/tools/upstream_baseline.json b/tools/upstream_baseline.json index ab05d49bbf..1c4c9b7139 100644 --- a/tools/upstream_baseline.json +++ b/tools/upstream_baseline.json @@ -2,7 +2,7 @@ "repo": "https://github.com/lidge-jun/opencodex.git", "branch": "main", "reviewed_through": "6ae83b1f189c353935d4977bb01227484fbdb52b", - "reviewed_date": "2026-08-22", - "reviewed_issue_through": 2379, - "reviewed_pr_through": 2383 + "reviewed_date": "2026-08-23", + "reviewed_issue_through": 2434, + "reviewed_pr_through": 2433 }