Repository navigation
fix(acp-bridge): deflake the exhaustive UTF-16 byte-estimate test (#10529) #10532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a623924
add1021
9da9722
ed03500
ccbcde4
a8394bd
ac4b2a4
66882c2
50447e2
62162ce
18aaa8f
964fb72
8cae3ac
3f56e9a
323281f
4c06d54
049ef40
5dcf629
488a1d9
4e8d80b
75ea637
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,12 +9,19 @@ import { estimateJsonStringBytes } from './json-string-bytes.js'; | |
|
|
||
| describe('estimateJsonStringBytes', () => { | ||
| it('matches JSON.stringify UTF-8 bytes for every UTF-16 code unit', () => { | ||
| const mismatches: Array<{ code: number; got: number; want: number }> = []; | ||
| for (let code = 0; code <= 0xffff; code++) { | ||
| const value = String.fromCharCode(code); | ||
|
Comment on lines
+12
to
14
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified as real; escalated — the fix is PR metadata, which this round cannot write. Confirmed against this checkout, not just the report:
Why no commit: the finding's own remediation is "PR-metadata bookkeeping only, no code change", and @wenshao's round-2 verification says "drop Applying it needs one Title, minus the trailing reference: Both body sections, with the closing keyword removed from each — GitHub parses closing keywords anywhere in the body, so removing one leaves the certification live: ## Linked Issues
Related to #10529 — deliberately **without** a closing keyword. That issue's recorded CI failure was in the `.github/scripts` helper suite `autofix-status-heartbeat.test.mjs` (subtest `stamps each tick in flight around the gh call and clears it after`), run by a separate `Run .github/scripts helper tests` step; in that run the step executing this PR's test was skipped, so this change is not its fix. The issue was closed as stale on 2026-08-31.
Restores the ECS-conditional `testTimeout` knob from #10552 as the authoritative per-test timeout for this package.## 关联 Issue
与 #10529 相关——刻意**不使用**关闭关键字。该 issue 记录的 CI 失败发生在 `.github/scripts` 辅助测试套件 `autofix-status-heartbeat.test.mjs`(子测试 `stamps each tick in flight around the gh call and clears it after`),由独立的 `Run .github/scripts helper tests` 步骤执行;在那次运行中,执行本 PR 测试的步骤被跳过,因此本改动不是它的修复。该 issue 已于 2026-08-31 以 stale 关闭。
恢复了 #10552 中按 ECS 条件生效的 `testTimeout` 开关,使其重新成为本包权威的单测超时。The remaining stale sections the finding names ("What this PR does", Test Plan step 3, Evidence, Risk) still describe the superseded 60s timeout raise. A complete paste-ready replacement body describing the assert-once change that actually ships is in this round's report comment on the PR. Thread intentionally left open: the finding is not resolved in code, so resolving it would hide an unaddressed Critical. 经复核属实;已上报——修复对象是 PR 元数据,本轮无法写入。 对照当前 checkout 确认(不只是照抄报告):
为何没有提交:该发现自身写明的修复方式是 "仅 PR 元数据操作,无需代码改动",@wenshao 的第 2 轮验证也写明 "drop 要落实它,只需一条 标题去掉末尾的 issue 引用: 两处正文章节都要移除关闭关键字——GitHub 会解析正文任意位置的关闭关键字,只移除一处,认证仍然有效。(两段替换文本见上方英文区块中的 该发现点名的其余过时章节("What this PR does"、Test Plan 第 3 步、Evidence、Risk)仍在描述已被替换的 60 秒超时上调。一份可直接整段粘贴、描述实际提交的 assert-once 改动的完整替换正文,已放在本轮发到 PR 上的报告评论中。 线程刻意保持开放:该发现并未在代码中被解决,关闭它会掩盖一条未处理的 Critical。 |
||
| expect(estimateJsonStringBytes(value, Number.MAX_SAFE_INTEGER)).toBe( | ||
| Buffer.byteLength(JSON.stringify(value)), | ||
| ); | ||
| const got = estimateJsonStringBytes(value, Number.MAX_SAFE_INTEGER); | ||
| const want = Buffer.byteLength(JSON.stringify(value)); | ||
| if (got !== want) mismatches.push({ code, got, want }); | ||
| } | ||
| expect({ count: mismatches.length, first: mismatches.slice(0, 5) }).toEqual( | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Disposition: escalated to a maintainer — not resolved in code, so this thread stays open. Re-verified at head
New evidence this round, which supports the finding rather than weakening it. Since this branch's merge-base ( One observation for whoever owns #10547, stated as a code-shape fact and not as a verified defect: the subtest this finding names ( The ask is unchanged (comment On the red 中文说明处置:上报维护者 —— 未在代码中解决,因此该线程保持开放。 以下均在 head
本轮的新证据,它支持而非削弱该发现。 自本分支 merge-base( 一条给 #10547 负责人的观察,作为代码形态事实陈述,而非已核实的缺陷:该发现点名的子测试( 请求事项不变(评论 关于本轮反馈中点名的红色检查
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Round 18: confirmed again at HEAD Re-verified locally this round rather than carried over:
The finding is real and undisputed. What has not changed in 18 rounds is that its only fix is a GitHub metadata write this loop cannot make, re-verified this round from the workflow source and the environment rather than asserted:
Retargeting the fix at the heartbeat suite is also unavailable ( For the maintainer — two commands, then merge. @wenshao already reached this conclusion twice (ic:5503235459 §Recommendation 1, ic:5509928882 §6 and §Recommendation 1: "merge the code … but rewrite the PR body first"), and the code side is done: the assert-once rewrite plus the bounded
The round-18 review's own exit recommendation is 中文说明第 18 轮:已在 HEAD 本轮在本地重新核实,而非沿用旧结论:
该发现属实且无争议。18 轮以来未曾改变的是:它唯一的修复是一次本循环无法执行的 GitHub 元数据写入——本轮已从 workflow 源码与运行环境重新核实,而非凭空断言:
把修复重新对准入心跳套件同样不可行( 给维护者——两条命令,然后合入。 @wenshao 已两次得出同一结论(ic:5503235459 建议 1;ic:5509928882 第 6 节与建议 1:"合入代码……但请先重写 PR 正文"),而代码侧已经完成:ic:5509928882 第 5 节要求的只断言一次改写与受限的
第 18 轮评审自身给出的出口建议是 |
||
| { | ||
| count: 0, | ||
| first: [], | ||
| }, | ||
| ); | ||
| }); | ||
|
|
||
| it('matches JSON.stringify for paired surrogates and mixed escaping', () => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escalated — not implementable in-tree. This thread stays open.
Confirmed and not disputed. In run 33274358974 the
Run .github/scripts helper testsstep failed on theautofix-status-heartbeat loopsubtest andRun tests and generate reports— the only step that executesnpm run test:ci, and therefore the only one that ever runs the acp-bridge suite — was skipped. SoFixes #10529is a false certification on the record. Maintainer @wenshao reached the same conclusion independently from the run's own step data (§1 and §6 of their two verification reports).Neither remediation this finding offers is available to this loop:
Fixes #10529linkage and the(#10529)title reference is a PR title / body / branch write. This loop has no GitHub credentials and performs no network writes — the workflow owns them. That is the same boundary this thread has been escalated against since round 1..github/scripts/autofix-status-heartbeat.test.mjs..github/is CI machinery this PR was never about, which this loop may not modify, and it is outside the PR's footprint, which the gate rejects. It is also moot: the heartbeat race was already fixed on main by3483836895 fix(ci): stop heartbeat mint-skip test racing loop startup (#10527)— verified an ancestor of this head — and Main CI failed: Qwen Code CI on 02f2101f4305 #10529 has been closed since 2026-08-31.The maintainer's instruction is already on the record and is bookkeeping, not code: "merge the code change, but drop or downgrade the
Fixes #10529link first", and in round 2 "rewrite 'What this PR does', 'Reviewer Test Plan', 'Evidence' and 'Risk & Scope' around the assert-once change, and dropFixes #10529(a plain reference is fine)". What remains is executing that metadata write.Question for the maintainer: will you — or the workflow — apply those metadata edits (demote
Fixes #10529and the title reference to plain references, rewrite the body around the assert-once change) so this can merge? Re-running this loop will keep returning the same report: there is no edit inside this PR's one-file footprint that closes this finding.Code state for the merge decision, re-verified this round at head
488a1d9a29(a base-update merge carrying no source or test edits): unchanged and green.packages/acp-bridge34 files / 1919 tests passed in 20.6 s; the changed file 5 tests in 49 ms; eslint--max-warnings 0and prettier clean;grep -c '60_000'on the changed file is 0, so #10552's ECS-conditionaltestTimeoutknob is authoritative for this test again. The diff is the assert-once rewrite plus the bounded mismatch collector from @wenshao's round-2 §5. Full detail in this round's report.中文说明
升级 —— 无法在代码树内实施。本线程保持开放。
已确认,不作反驳。在 run 33274358974 中,
Run .github/scripts helper tests步骤失败于autofix-status-heartbeat loop子测试,而Run tests and generate reports—— 唯一执行npm run test:ci、也就是唯一会运行 acp-bridge 套件的步骤 —— 被跳过。因此Fixes #10529是记录上的一次错误认证。维护者 @wenshao 对照该 run 自己的步骤数据独立得出同样结论(见其两份验证报告的 §1 与 §6)。该发现给出的两种修复方式,本循环都无权执行:
Fixes #10529关联与(#10529)标题引用 属于 PR 标题 / 正文 / 分支的写入。本循环没有 GitHub 凭据,也不执行任何网络写入 —— 这些由 workflow 负责。这与该线程自第 1 轮起持续升级所依据的边界是同一个。.github/scripts/autofix-status-heartbeat.test.mjs。.github/属于本 PR 从未涉及的 CI 机制,本循环不得修改;它也超出本 PR 的 footprint,会被门禁拒绝。而且已无意义:该心跳竞争问题已由 main 上的3483836895 fix(ci): stop heartbeat mint-skip test racing loop startup (#10527)修复(已验证为本 head 的祖先),且 Main CI failed: Qwen Code CI on 02f2101f4305 #10529 自 2026-08-31 起已关闭。维护者的指令已在记录上,且属记账而非代码:"合入代码改动,但先去掉或降级
Fixes #10529关联";第 2 轮补充 "把 'What this PR does'、'Reviewer Test Plan'、'Evidence' 与 'Risk & Scope' 围绕只断言一次的改动重写,并去掉Fixes #10529(保留为普通引用即可)"。剩下的就是执行这次元数据写入。给维护者的问题: 是否由您(或 workflow)来应用这些元数据修改(把
Fixes #10529与标题引用降级为普通引用,并围绕只断言一次的改动重写正文),以便本 PR 可以合入?重跑本循环只会持续返回同样的报告:在本 PR 单文件的 footprint 之内,不存在能关闭该发现的编辑。供合入决定使用的代码状态,本轮已在 head
488a1d9a29(一次不含源码或测试改动的 base 更新合并)重新验证:未改动且为绿。packages/acp-bridge34 个文件 / 1919 个测试通过,耗时 20.6 秒;改动文件 5 个测试、49 毫秒;eslint--max-warnings 0与 prettier 均干净;对改动文件执行grep -c '60_000'为 0,因此 #10552 的 ECS 条件testTimeout旋钮对该测试重新具有权威性。该 diff 就是只断言一次的重写,外加 @wenshao 第 2 轮 §5 的有界不匹配收集器。完整细节见本轮报告。