feat(verify-pr): ship a one-command capture helper - #8114
Conversation
Fourth attempt at the same failure, and the first one aimed at the real cause. The score so far: #8016 captures were "Optionally … when text cannot carry the oracle" -> 0 images / 14 runs #8104 captures became budget item 4 in Scope selection -> 0 images / 1 run That last run is the one that settles it. Same PR (#7975), browser installed and working ("Install evidence browser: success"), and the verification got DEEPER — 64 assertions against 40, 53 tables against 31 — while still producing zero images. An agent reading the instruction, doing more work than before, and still not capturing is not an agent that missed the instruction. The cause is one I should have checked when I wrote #8016: the skill sent the agent to build node-pty -> xterm.js -> Playwright itself, and `node-pty` is not a dependency of this repo. It needs a native build. The `playwright` package is not a declared dependency either. So the documented route did not exist, and the incentive was entirely against trying it: authoring that pipeline risks failing and eats budget, while skipping costs nothing and is invisible. `scripts/verify-capture.mjs` makes a capture one command using deps that are already installed: node scripts/verify-capture.mjs --out evidence/01-ab.png \ --title 'A/B: the gate flips' -- node my-harness.mjs Command (or stdin) -> @xterm/headless parses the ANSI into a cell grid with colour and bold -> SVG -> sharp rasterises. No browser, no pseudo-terminal. A non-zero exit from the captured command still produces an image, because capturing a failing base arm is the normal case. The skill's dead route is removed and replaced with that command, and the budget line drops from ~5 minutes to ~2 because there is no pipeline to author. QWEN_VERIFY_CHROMIUM and its install stay for a future web-UI capture, but the terminal route no longer depends on them — gating on a browser the route does not use is how an absent browser turns into a skipped capture. Mutation-verified 7/7 against the real helper and the real PNGs: bare LF (staircase render), no event-loop turn (blank capture), dropped bold, dropped colour, no blank-row trimming, tolerating an empty capture, and dropped geometry validation. Two of those initially SURVIVED. The colour test compared "coloured and bold" against plain and asserted the bytes differ — which passes while EITHER attribute survives. That is the wrong-reason trap this skill warns about, met in the skill's own test file. Each attribute is now isolated against the same plain baseline (green-no-bold, bold-no-colour), and both mutations kill. 119/119 across both suites; prettier and eslint clean.
|
Thanks for the PR! Template looks good ✓ — all sections present, including the full Chinese translation. Problem: observed and well-documented, not theoretical. Four live Direction: aligned. This is internal process tooling that makes the verify-pr workflow actually produce evidence images. No CHANGELOG signal needed — it is not user-facing. Size: not applicable — no core paths touched. Everything is in Approach: the scope feels right and minimal. Five files, each necessary: the helper, its test suite, the skill-doc rewrite that points at it, the existing workflow-test assertions updated to match, and Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 各节齐全，含完整中文翻译。 问题：已观测且有充分记录，非理论性加固。四次 方向：对齐。这是让 verify-pr 工作流真正产出证据图的内部流程工具。无需 CHANGELOG 信号——非用户可见。 规模：不适用——未触及核心路径。全部改动在 方案：范围合理且最小。五个文件，各有必要：helper、测试套件、指向它的 skill 文档重写、适配后的现有工作流测试断言、以及把 风险：无升级风险信号——未匹配高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given the problem (the skill sends agents to assemble a browser pipeline that is not installed as a unit, so four runs produced zero images), I would write one standalone script that uses Comparison with the diff: clean and well-executed, and the five follow-up commits since the first review pass landed real improvements, each pinned by a test:
One thing I checked specifically: No correctness bugs, security holes, or regressions. No AGENTS.md violations — the script lives in Testing
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The ubuntu unit suite — the check that was still running on the previous head — now passes on The central technical claim — "one command produces a valid evidence PNG" — is substantiated by this suite, not taken on faith: every test runs the real helper as a subprocess and inspects the real PNG (signature bytes and decoded pixels, not file existence or mocks), covering colour/bold isolation, the grey fallback, black-on-black, CRLF, the truncation boundary, XML escaping, and the error exits. A suite that passed with the helper removed is not what is running here. The author's 7/7 mutation kill report is their own claim; the green Linux run is the independent signal, and it agrees. What this PR's CI cannot settle — and the author says so plainly — is the forward-looking process claim that agents will actually use the helper in future 中文说明代码审查独立方案： 鉴于问题（skill 让 agent 自行搭建一条未作为单元安装的浏览器管线，导致四次运行零张图），我会写一个独立脚本：用 与 diff 的比较： 简洁且执行良好；首轮 review 之后的五个后续 commit 各带来一项由测试钉住的真实改进：以 特别核查了一点： 未发现正确性 bug、安全漏洞或回归。无 AGENTS.md 违规。 测试ubuntu 单元测试——上一个 head 上仍在运行的检查——现已在 核心技术主张（"一条命令产出有效证据 PNG"）由该套件证实，而非凭信：每条测试都把真实 helper 作为子进程运行并检查真实 PNG（签名字节与解码像素，而非文件存在或 mock）。作者 7/7 变异致死是其自述；绿色的 Linux 运行是独立信号，二者一致。 本 PR 的 CI 无法验证的是前瞻性的流程主张——agent 在未来 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean across every stage; CI is green and I am approving now. This is the fourth attempt at getting evidence images out of The code is easy to maintain: one focused script, error messages that name the real mistake, and comments that explain the non-obvious why without narrating the obvious. The test suite is the standout — it runs the real helper and inspects real PNG bytes and pixels, and it caught its own wrong-reason trap (the colour/bold comparison that passed while either attribute survived). The five follow-up commits turned prior review feedback into pinned behaviour rather than prose. If I had to maintain this in six months, I would thank the author. The honest risk framing is appreciated and correct: nothing requires an image, so an agent may still skip, and the next lever (visibility/enforcement) is named but deliberately deferred until we see whether removing the obstacle was enough. The The ubuntu unit suite that was pending on the previous head now passes on 中文说明置信度：5/5 —— 每个阶段都干净；CI 已绿，现在批准。 这是第四次尝试让 代码易于维护：一个聚焦的脚本、点名真实错误的错误信息、以及解释非显而易见之"为什么"而不叙述显而易见之事的注释。测试套件是亮点——运行真实 helper 并检查真实 PNG 字节与像素，还抓到了自身的"因错误原因通过"陷阱。五个后续 commit 把此前的 review 反馈变成了钉住的行为而非文字。 诚实且正确的风险表述值得肯定：没有任何东西强制要图，agent 仍可能跳过；下一个手段（可见性/强制）已点名但刻意推迟，待看清移除障碍是否足够。 上一个 head 上仍在运行的 ubuntu 单元测试现已在 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Maintainer review — real local environment, PR head
|
| # | finding | severity |
|---|---|---|
| 1 | U+FE0F in captured output aborts the process — SIGTRAP, no PNG, no diagnostic |
blocker |
| 2 | The "Cause 1" dependency table does not reproduce; the deleted route runs | body correction |
| 3 | No TTY for the captured command — cannot capture an ink TUI | scope note |
| 4 | Output longer than --rows silently loses its header |
follow-up |
| 5 | --title longer than --cols clips silently |
nit |
| 6 | sharp is not declared at root, only hoisted |
nit |
1. Blocker — U+FE0F aborts the capture
⚠️ is U+26A0 U+FE0F. The variation selector makes Pango look for a colour-emoji font, fail, and call abort() inside librsvg — underneath sharp, in native code.
- Exit 133 = SIGTRAP. Bare
U+26A0alone is fine; addingU+FE0Fkills it. - No PNG written. No
verify-capture:line on stderr — the script's owntry/catchnever runs, because a nativeabort()is not a JS exception. The agent sees onlyPango-ERROR ... bailing out. - 79 tracked files contain
U+FE0F, including three that land directly in captured output:packages/cli/src/i18n/locales/en.js—⚠️ History gap:, real CLI outputintegration-tests/terminal-capture/scenario-runner.ts— prints⏹️,🎞️to stdout.github/scripts/serve-ab-diff.mjs—⚠️ _The PR-base baseline could not be built…_, an A/B harness
So the first round that captures the CLI, or the A/B diff harness, or a Tested on table, produces zero images again — with a worse signature than before, because a signal-killed process looks like a broken helper rather than a skipped step. That is round five failing on mechanics, which is precisely what this PR set out to remove.
Fix verified locally, one line beside the existing CRLF normalisation:
// U+FE0F sends Pango looking for a colour-emoji font; not finding one it
// calls abort(), which no try/catch here can see. The base codepoint renders.
term.write(raw.replace(/\uFE0F/g, '').replace(/\r?\n/g, '\r\n'));With that, the same input exits 0 and writes the PNG. Note also that no test in the suite feeds the helper a single non-ASCII byte — worth a case, since the mutation matrix is otherwise thorough.
2. The "Cause 1" table does not reproduce — and the deleted route runs
This matters more than a wording nit: it is the justification the next round will read, and the PR's own thesis is that #8016 shipped on an unverified claim about this very route.
| PR body claims | measured in this checkout |
|---|---|
node-pty — "not a dependency of this repo, and needs a native build" |
@lydell/node-pty@1.2.0-beta.10 is a root optionalDependency, with prebuilt binaries for darwin-arm64/x64, linux-x64, win32-arm64/x64. No native build. It opened a pty here on the first try. |
playwright — "not a declared dependency" |
Declared in packages/webui (^1.57.0) and integration-tests/terminal-capture (^1.50.0); resolves from root at 1.58.2. |
The line the PR deletes — Route: `terminal-capture` skill (node-pty → xterm.js → Playwright PNG) — points at .qwen/skills/terminal-capture/SKILL.md, a real skill backed by a 27 KB engine, and all four of its dependencies resolve from the repo root.
I ran that route end-to-end:
Arm A produced a valid 9 KB PNG in 1.5 s. One substitution: chromium's own download is blocked in this sandbox, so Playwright drove the system Chrome via channel:'chrome' — the browser binary is substituted, the four-stage pipeline is not. In CI the skill's QWEN_VERIFY_CHROMIUM=1 pre-installs chromium, so that arm had its browser too.
The real obstacle is a different one, and neither reworded instructions nor a new helper would have surfaced it: integration-tests/terminal-capture is not in root workspaces[], so its package.json is never installed as a unit — it works only because every dep happens to be hoisted from elsewhere. That is a genuine fragility, and worth writing down in place of the dependency table.
The conclusion still stands — a one-command helper at 0.3 s beats a browser pipeline at 1.5 s for rendering a table, and that is reason enough to ship it. It just doesn't need the "route did not exist" story, which is falsifiable and now falsified.
3. Scope note — no TTY, so no ink TUI
spawnSync gives the captured command isTTY=false and columns=undefined; the pty route gives isTTY=true, columns=40. FORCE_COLOR=1 recovers colour for tools that honour it, but not layout, and not a TUI that renders only under a terminal. This helper cannot capture the qwen CLI's own ink UI — which is what terminal-capture was built for.
That is a fair trade for A/B tables. But the PR removes the skill's only pointer to the route that can do it, so a future web-shell or CLI-rendering PR has nothing to find. Suggest keeping one line — "for a TUI or web-UI capture, see the terminal-capture skill" — rather than deleting it.
4. Output longer than --rows silently loses its header
A 44-line mutation matrix at the default --rows 40: scrollback: 0 means the tail is kept, so the title row, the column header, the rule and mutations 01–02 are gone — the render starts at mutation-03. The image looks complete, and stdout reports 39 rows with no mention that 5 lines were discarded. For an A/B table or a mutation matrix the header is exactly the part that carries meaning.
This is the helper's own rule turned on itself — "a blank capture is worse than none: it looks like evidence and shows nothing." A truncated capture has the same property. A one-line stderr warning when input lines exceed --rows would be enough.
5. --title clips silently
Width comes from --cols alone, so a title wider than cols × 8.4 px is cut with exit 0 and no warning. Measured: --cols 20 --title '<68 chars>' yields a 192 px canvas for a caption needing ~570 px. I hit this for real while producing image 05 above, and had to shorten the caption to fit. Either widen the canvas to the title or warn.
6. sharp is not declared at root
scripts/verify-capture.mjs is a root-level script that requires sharp, but root package.json does not declare it — only packages/core (^0.34.5) does, and root resolution works by hoisting. @xterm/headless is in root devDependencies. This is the same class of fragility the PR is fixing, one level down; a one-line root declaration removes it.
What verified clean
- 119/119, exactly as claimed —
verify-capture.test.js12/12,qwen-triage-workflow.test.js107/107. - Independent 7/7 mutation kill, baseline confirmed green first so a "killed" means something.
- The self-report about mutations 3 and 4 is accurate. I reverted the isolated assertions to a single "coloured and bold vs plain" comparison: both mutations survived. With this PR's isolated version, both die. The test rewrite is load-bearing, not cosmetic.
prettier --checkandeslintclean on all four changed files.- CJK renders correctly — I expected the fixed 8.4 px advance to overlap wide glyphs, but xterm assigns them two cells and the arithmetic works out. Not a defect.
- Cause 2 corroborated: 406
refs/heads/pr-assets/*refs exist andrefs/heads/pr-assetsdoes not, so the old bare-branch clone could never have succeeded. - The helper does what it says. Five captures, colour and bold intact, ~0.3 s each, no browser.
Not covered
- No Linux runner execution. macOS arm64 (Darwin 25.6.0), node v24.18.1, npm 11.16.0. The Linux
⚠️ in the PR body stands — and note finding 1 is likely worse on a bare Linux container, where colour-emoji fonts are less likely to be present than on macOS. - Arm A used system Chrome, not downloaded chromium (see above).
- I did not run
scenario-runner.tsitself, which needs a built CLI; arm A reproduced its documented four-stage architecture rather than that entrypoint. - Findings 3–6 are reported, not fixed.
Recommendation
Fix finding 1 with the one-line strip plus a non-ASCII test case, and correct the Cause 1 table in the body — then merge. Findings 3–6 are clean follow-ups, and finding 3 is worth a line in the skill before this lands so the TUI route stays discoverable.
中文版
维护者评审 —— 真实本地环境,PR head b260e369
结论基于真实 checkout 上的构建与运行,不是读 diff 得出的。下面每张图都由本 PR 所交付的 helper 自己生成,因此这份报告同时也是对交付物的一次实测。每个 harness 都在截图那一刻现场测量自己的结论,而非引用。
结论:修掉 1 个阻塞项即可合并。 helper 确实可用,测试是诚实的,变异结论在独立复跑下成立。但在现实输入上存在一个崩溃,它复现的正是本 PR 想要终结的那个失败;而且 PR body 里给出的病因经实测不成立。
| # | 发现 | 严重度 |
|---|---|---|
| 1 | 被捕获输出中的 U+FE0F 导致进程 abort —— SIGTRAP、无 PNG、无诊断信息 |
阻塞 |
| 2 | 「Cause 1」依赖表不成立;被删掉的那条路径实际能跑 | 需修正 body |
| 3 | 被捕获命令拿不到 TTY —— 无法截取 ink TUI | 范围说明 |
| 4 | 输出超过 --rows 时静默丢掉表头 |
后续项 |
| 5 | --title 超过 --cols 时静默裁剪 |
小问题 |
| 6 | sharp 未在 root 声明,仅靠 hoisting |
小问题 |
1. 阻塞项 —— U+FE0F 让截图进程 abort
⚠️ 是 U+26A0 U+FE0F。变体选择符让 Pango 去找彩色 emoji 字体,找不到便在 librsvg 内部调用 abort() —— 位于 sharp 之下的原生代码里。
- 退出码 133 = SIGTRAP。单独的
U+26A0没问题,加上U+FE0F就死。 - 不产出 PNG,stderr 上也没有
verify-capture:那一行 —— 脚本自己的try/catch根本不会执行,因为原生abort()不是 JS 异常。agent 只会看到Pango-ERROR ... bailing out. - 仓库中 79 个受版本控制的文件含
U+FE0F,其中三个会直接进入被捕获的输出:packages/cli/src/i18n/locales/en.js——⚠️ History gap:,真实 CLI 输出integration-tests/terminal-capture/scenario-runner.ts—— 向 stdout 打印⏹️、🎞️.github/scripts/serve-ab-diff.mjs—— A/B 对比 harness
也就是说,第一次去截 CLI、截 A/B harness、或截一张 Tested on 表格的运行,又会是0 张图 —— 而且比之前更难看:被信号杀掉的进程看起来像 helper 坏了,而不像跳过了一步。这就是第五次仍败在机械层面,恰恰是本 PR 想消除的。
已在本地验证的修法,就加在现有 CRLF 归一化旁边一行:
term.write(raw.replace(/\uFE0F/g, '').replace(/\r?\n/g, '\r\n'));加上之后同样的输入退出 0 并正常写出 PNG。另外:整个测试套件没有任何一条给 helper 喂过非 ASCII 字节 —— 变异矩阵在其他方面已经很扎实,这一条值得补上。
2. 「Cause 1」表格不成立 —— 而且被删掉的那条路径能跑
这比措辞问题更重要:它是下一轮会读到的依据,而本 PR 自己的论点正是「#8016 基于一个未经核实的、关于这条路径的判断就发了出去」。
| PR body 的说法 | 本 checkout 实测 |
|---|---|
node-pty ——「本仓库没有这个依赖,且需要原生编译」 |
@lydell/node-pty@1.2.0-beta.10 是 root optionalDependency,带 darwin-arm64/x64、linux-x64、win32-arm64/x64 预编译产物。不需要原生编译。 在这里一次就打开了 pty。 |
playwright ——「非声明依赖」 |
在 packages/webui(^1.57.0)和 integration-tests/terminal-capture(^1.50.0)中均有声明;从 root 解析到 1.58.2。 |
被删掉的那一行 —— Route: `terminal-capture` skill (node-pty → xterm.js → Playwright PNG) —— 指向 .qwen/skills/terminal-capture/SKILL.md,一个真实存在的 skill,背后是 27 KB 的引擎;而且它的四个依赖全部能从仓库 root 解析。
我把那条路径完整跑了一遍:arm A 在 1.5 秒内产出了合法的 9 KB PNG。 只有一处替换:本沙箱里 chromium 自身的下载被阻断,因此 Playwright 通过 channel:'chrome' 驱动系统 Chrome —— 被替换的是浏览器二进制,四级管线没有被替换。在 CI 中 skill 的 QWEN_VERIFY_CHROMIUM=1 会预装 chromium,所以那一侧同样有浏览器。
真正的障碍是另一个,而且无论改措辞还是换 helper 都不会让它暴露:integration-tests/terminal-capture 不在 root 的 workspaces[] 里,它的 package.json 从来没有被作为一个整体安装过 —— 它能用,纯粹因为每个依赖恰好都从别处被 hoist 上来了。这是真实的脆弱点,值得写下来,替换掉那张依赖表。
结论本身依然成立 —— 渲染一张表格,0.3 秒的一条命令胜过 1.5 秒的浏览器管线,这个理由足够支撑合并。它只是不需要「路径不存在」这个说法,而那个说法是可被推翻的,现在也已经被推翻。
3. 范围说明 —— 没有 TTY,因而截不了 ink TUI
spawnSync 给被捕获命令的是 isTTY=false、columns=undefined;pty 那条路径给的是 isTTY=true, columns=40。FORCE_COLOR=1 能为遵守它的工具挽回颜色,但挽不回布局,也挽不回只在终端下才渲染的 TUI。这个 helper 截不了 qwen CLI 自己的 ink 界面 —— 而那正是 terminal-capture 当初存在的理由。
对 A/B 表格来说这是合理的取舍。但本 PR 把 skill 中指向那条路径的唯一线索删掉了,于是将来的 web-shell 或 CLI 渲染类 PR 无处可查。建议保留一行 ——「需要 TUI 或 web UI 截图,见 terminal-capture skill」—— 而不是整段删除。
4. 输出超过 --rows 时静默丢掉表头
44 行的变异矩阵在默认 --rows 40 下:scrollback: 0 意味着保留的是尾部,于是标题行、列头、分隔线、以及 mutation 01–02 全部消失 —— 渲染是从 mutation-03 开始的。图看起来是完整的,stdout 报 39 rows,完全没提有 5 行被丢弃。而对 A/B 表格或变异矩阵而言,表头恰恰是承载含义的那部分。
这正是 helper 自己那条规则反过来作用在它身上 ——「空白截图比没有更糟:它看起来像证据,却什么都没显示。」 被截断的截图具有同样的性质。在输入行数超过 --rows 时向 stderr 输出一行警告就够了。
5. --title 静默裁剪
宽度只由 --cols 决定,因此超过 cols × 8.4 px 的标题会被裁掉,退出码为 0 且无任何警告。实测:--cols 20 --title '<68 字符>' 得到 192 px 的画布,而标题需要约 570 px。我在生成上面第 05 张图时真的撞上了这个问题,只能把标题缩短以适应画布。要么把画布放宽到容纳标题,要么给出警告。
6. sharp 未在 root 声明
scripts/verify-capture.mjs 是 root 层的脚本并 require 了 sharp,但 root package.json 并未声明它 —— 只有 packages/core(^0.34.5)声明了,root 能解析纯粹靠 hoisting。而 @xterm/headless 确实在 root 的 devDependencies 中。这与本 PR 正在修的问题属于同一类脆弱性,只是低了一层;在 root 加一行声明即可消除。
验证通过的部分
- 119/119,与声称完全一致 ——
verify-capture.test.js12/12,qwen-triage-workflow.test.js107/107。 - 独立复跑 7/7 变异全灭,并先确认基线为绿,这样「killed」才有意义。
- 关于变异 3 与 4 的自述属实。 我把隔离断言退回为单一的「带色且加粗 vs 纯文本」比较:两个变异都存活。换成本 PR 的隔离版本,两个都被杀死。这次测试重写是承重的,不是装饰。
prettier --check与eslint在四个改动文件上均干净。- CJK 渲染正常 —— 我原本预期固定 8.4 px 的步进会让宽字形重叠,但 xterm 给它们分配两个单元格,算术刚好对上。这不是缺陷。
- Cause 2 得到佐证: 存在 406 个
refs/heads/pr-assets/*ref,而refs/heads/pr-assets不存在,因此旧的裸分支 clone 确实永远不可能成功。 - helper 名副其实。 五张截图,颜色与粗体都在,每张约 0.3 秒,不需要浏览器。
未覆盖
- 未在 Linux runner 上执行。 环境为 macOS arm64(Darwin 25.6.0)、node v24.18.1、npm 11.16.0。PR body 里 Linux 的
⚠️ 依然有效 —— 并且请注意,发现 1 在裸 Linux 容器上很可能更糟,那里比 macOS 更不容易预装彩色 emoji 字体。 - arm A 使用的是系统 Chrome,而非下载的 chromium(见上文)。
- 我没有运行
scenario-runner.ts本身,它需要已构建的 CLI;arm A 复现的是它所记载的四级架构,而不是那个入口。 - 发现 3–6 只是报告,未作修复。
建议
用那一行剥离加一条非 ASCII 测试修掉发现 1,并修正 body 中的 Cause 1 表格 —— 然后即可合并。发现 3–6 适合作为干净的后续项;其中发现 3 值得在本 PR 落地前先在 skill 里补一行,以保持 TUI 路径可被发现。
Evidence images produced by scripts/verify-capture.mjs from this PR on macOS arm64, hosted on pr-assets/8114-maintainer-review following the repo's per-PR asset-branch convention. Harness sources and the mutation runner are reproducible from the commands described above.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| const colour = | ||
| cell.fg >= 0 && cell.fg < ANSI.length ? ANSI[cell.fg] : FG_DEFAULT; |
There was a problem hiding this comment.
[Suggestion] Any 256-color (SGR 38;5) or truecolor (SGR 38;2) foreground fails the < ANSI.length (16) guard and collapses to FG_DEFAULT, so a red FAIL and a green PASS render in the same grey. — Failure scenario (probe-confirmed): 256-color red (196) and green (46), and truecolor red/green, all fall through to the same #d4d4d4; under TERM=xterm-256color (inherited via ...process.env) a captured reporter that emits 256-color red/green produces an image whose two arms are indistinguishable, losing the at-a-glance distinction the tool exists to preserve. The SKILL.md also claims "colour and bold preserved" without qualification. Either map palette/RGB colours, or at minimum qualify the skill claim:
rasterises the cell grid with `sharp` — the 16 base ANSI colours and bold preserved, …and add a test piping 256-color red/green (e.g. ESC[38;5;196mFAIL ESC[38;5;46mPASS) asserting the two cells render differently.
中文说明
任何 256 色（SGR 38;5）或真彩（SGR 38;2）前景都会因不满足 < ANSI.length（16）的守卫而塌缩为 FG_DEFAULT，于是红色 FAIL 与绿色 PASS 渲染成同一种灰。失败场景（probe 已确认）：256 色红（196）与绿（46）、以及真彩红/绿，全都落到同一个 #d4d4d4；在 TERM=xterm-256color（经 ...process.env 继承）下，一个发出 256 色红/绿的被捕获 reporter 会产出两臂无法区分的图，丢失本工具赖以存在的“一眼可辨”的区别。SKILL.md 还无条件地声称 “colour and bold preserved”。要么映射调色板/RGB 颜色，要么至少对该声明加上限定（见上方），并新增一个管道输入 256 色红/绿、断言两个单元格渲染不同的测试。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Took the "at minimum qualify the skill claim" option rather than the full mapping: SKILL.md now reads "the 16 base ANSI colours and bold preserved (256-colour and truecolor fall back to the default grey)", so it no longer overclaims.
Declined the palette/RGB mapping and the "red vs green render differently" test. Per AGENTS.md Simplicity First this is out of scope for a capture-helper hardening round: the helper's primary use (A/B tables, mutation matrices) emits 16-colour ANSI under FORCE_COLOR, and a test asserting two different renders would require the very mapping this declines — it would fail until that feature exists. If a real capture needs 256-colour fidelity, a follow-up can add the palette map and the test together. Leaving this thread open so you can confirm the qualified claim is enough for now.
中文说明
采用了「至少对 skill 声明加限定」这一选项，而非完整映射：SKILL.md 现为「the 16 base ANSI colours and bold preserved (256-colour and truecolor fall back to the default grey)」，不再过度声称。
拒绝了调色板/RGB 映射与「红 vs 绿渲染不同」的测试。按 AGENTS.md 的 Simplicity First，这对一个 capture-helper 加固轮而言超出范围：本 helper 的主要用途（A/B 表格、变异矩阵）在 FORCE_COLOR 下发出 16 色 ANSI，而一个断言两种不同渲染的测试需要它所拒绝的映射本身——在该功能存在之前它会失败。若确有截图需要 256 色保真，后续 PR 可以把调色板映射与测试一并加上。保持本线程打开，以便你确认「加限定的声明」目前是否足够。
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
- Strip U+FE0F before rasterising: the emoji variation selector made Pango
abort() in native code (SIGTRAP, no PNG, no diagnostic) when no colour-emoji
font exists; the base codepoint renders. Add a non-ASCII regression test.
- Await xterm's write callback instead of a fixed 120ms sleep, so a large
capture is not read mid-parse and silently come out blank.
- Name a signal-killed child ("killed by SIGKILL") rather than "exited null".
- Warn on stderr when input is taller than --rows and the top is dropped.
- Correct the falsified "route did not exist" rationale: the browser pipeline's
deps do resolve from this repo; the real fragility is that
integration-tests/terminal-capture is not a root workspace. Keep a skill
pointer to terminal-capture for TUI/web-UI captures this helper cannot do.
- Qualify the colour claim (16 base ANSI colours; 256/truecolor fall back).
- Tests: feed both --cols and --rows to the geometry guard, exercise escapeXml,
flatten the SKILL.md assertion against reflow, and replace the platform-fragile
PNG byte-length check (flaked at 846B on Linux vs >1000B on macOS) with the
deterministic canvas geometry.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review round summary — PR #8114Addressed the maintainer's blocker and the actionable automated-reviewer suggestions in one focused commit ( Maintainer findings (@wenshao)
Automated reviewer inline suggestions
Additional fix found during verification (not in feedback)
Conflict notes
Verification
中文说明评审轮次总结 — PR #8114在一次聚焦的提交（ 维护者发现（@wenshao）
自动评审器行内建议
验证过程中发现的额外修复（不在反馈中）
冲突说明
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedrc:3682817646 — 256-colour / truecolor fallback testDecision: Implemented. The bounds guard at line 224 ( rc:3682817652 — Wrap-aware truncation warningDecision: Implemented. The warning counted newline-delimited lines, but xterm wraps lines wider than Changed the check to strip ANSI escape sequences from each line, compute Conflict notesNo conflicts ( Verification
中文说明已处理的评审反馈rc:3682817646 — 256 色 / 真彩色回退测试决定： 已实现。 第 224 行的边界守卫（ rc:3682817652 — 感知换行的截断警告决定： 已实现。 原来的警告仅统计以换行符分隔的行数，但 xterm 会将宽于 修改后的检查会先剥离每行的 ANSI 转义序列，计算每行的换行行数 冲突说明无冲突（ 验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
Maintainer verification — built and run locally on LinuxVerdict: the central claim holds and this is worth merging — but not quite as-is. The one-command capture works end-to-end on Linux with zero setup, both documented invocation forms run verbatim out of Every image in this report was produced by the helper in this PR, which is the strongest evidence available for it: the tool rendered the report about itself. Environment
Claims verified
Mutation testing — 7/7 PR claims reproduce, plus 4 of my ownEach mutation edits the real helper, runs the real 17-test suite, and restores.
The PR's mutation claim is sound. Mutation #11 is the finding: making the warning arithmetic correct breaks 2 tests — the suite pins the off-by-one rather than catching it. Finding 1 —
|
| 验证 head | 47fa5c193（全部 3 个 commit，含两轮 autofix） |
| 平台 | Linux x64 / Node 22 —— 正是 PR 标记为 ⚠️ 未测的平台 |
| 所需配置 | 无：@xterm/headless + sharp 已可解析；不需浏览器、不需原生编译 |
| 测试套件 | verify-capture.test.js 17/17 · qwen-triage-workflow.test.js 107/107 → 124/124 |
| prettier / eslint | 改动文件全部干净 |
| CI | Test (ubuntu-latest, Node 22.x) 通过，无失败检查。BLOCKED 仅因 REVIEW_REQUIRED |
已验证的主张
| 主张 | 结果 |
|---|---|
SKILL.md 中命令可逐字执行 |
✅ -- 形式与管道形式均一次成功 |
| 真实命令的颜色与粗体得以保留 | ✅ 实跑 vitest —— 绿色对勾、黄色耗时、暗色文本均正确渲染 |
| 命令非零退出仍出图 | ✅ exited 3 → PNG 已写出，capture 退出码 0 |
| 几何校验 | ✅ 拒绝 0、-5、abc、9999、3.5 |
| 空输出退出 1 | ✅ nothing to render |
| 自动创建父目录 | ✅ 写入不存在的 evidence/ 成功 |
| 可在 Linux 运行 | ✅ —— 这条补上了 PR 自己的 ⚠️ |
"402 个 refs/heads/pr-assets/*" |
✅ 独立复核成立，现为 413 个，故每 PR 分支方案确为承重设计 |
变异测试 —— PR 的 7/7 全部复现，另加我自己的 4 个
每个变异都修改真实 helper、跑真实的 17 条测试、然后还原。
PR 的变异主张站得住。变异 #11 才是发现所在：把警告的算术改正确反而打挂 2 条测试 —— 测试钉住的是这个 off-by-one，而不是在捕捉它。
发现 1 —— --rows 在默认行数下静默丢掉首行且不告警
重要。 以换行结尾的输出需要比其最后一行多一行空间：最后那个 CRLF 会把光标推出视口，滚掉一行。在 scrollback: 0 下这一行就没了。因此可用容量是 rows - 1，而防护判断的是 wrappedRows > opts.rows。
两个后果，均已复现：
- 当输入恰好等于
--rows行时（包括默认的 40），首行丢失且完全不告警。这正是该防护被加进来要防的"看起来完整、实则从中间开始"的图。 - 超过之后，告警会触发，但每次都少报恰好 1 行。
直接复现 —— seq 1 40 用默认 --rows，首行改名 HEADER-1：
$ seq 1 40 | sed 's/^1$/HEADER-1/' | node scripts/verify-capture.mjs --out c.png --cols 30
c.png 276x726 10126B 39 rows # 喂 40 行，渲染 39 行，无任何告警
产出的 PNG 从 2 开始，HEADER-1 静默消失。机制佐证：同样 4 行但不带结尾换行时，4 行全部可见。
发现 2 —— 两条测试钉住了错误的数字
同一根因。6 行配 --rows 4 渲染出 l4 l5 l6，实际丢掉了顶部 3 行，但 stderr 写的是 dropped the top 2，而测试在两处断言了这个字面量：
scripts/tests/verify-capture.test.js——warns when input is taller than --rowsscripts/tests/verify-capture.test.js——warns when wrapped lines exceed --rows
建议修复 —— 已在所有情形下验证正确
替换 scripts/verify-capture.mjs 中的防护：
const capacity = /\r?\n$/.test(raw) ? opts.rows - 1 : opts.rows;
if (wrappedRows > capacity) {
process.stderr.write(
`verify-capture: warning: input occupies ${wrappedRows} terminal rows; ` +
`--rows ${opts.rows} kept the last ${capacity} and dropped the top ` +
`${wrappedRows - capacity}\n`,
);
}打上此补丁后重跑 oracle，每一种情形下 says dropped 都等于 really dropped，含换行场景（--cols 10 --rows 4，三条 20 字符行 → 报 3，实际 3）。需要改动的测试仅上述两处断言：dropped the top 2 → dropped the top 3。
发现 3 —— emoji 回归测试在 CI 实际运行的平台上是空转的
次要。 变异 #8 删掉 U+FE0F 剥离后，Linux 上仍然 17/17 全过 —— Pango 的 abort() 是 macOS 特有的，所以 renders non-ASCII and emoji without aborting 恰恰在 CI 执行的平台上是个空操作。修复本身正确、应当保留，只是在 runner 上没有防护。变异 #10（删除 stripAnsi）同理存活 —— 没有测试喂入足够长的带色输入来改变折行计数。
发现 4 —— SGR 30 渲染成黑底黑字而消失
属性质说明，非阻断项。 ANSI[0] 是 #1e1e1e，画布 BG 也是 #1e1e1e，完全相同。任何黑色前景文本会整体消失，而不只是丢掉背景色。这是给彩色徽章写标签的常规做法，并不冷僻：vitest 自己的项目徽章就是 ESC[30m ESC[46m scripts，在我拍的第一张真实截图里它就是一片空白。
反显（SGR 7）与亮黑（SGR 90）都正常 —— 该问题仅限 SGR 30。PR 在 Risk & Scope 中已声明"无背景填充"，但实际后果比"丢失颜色"更重。加一行兜底（当映射色等于 BG 时回退到 FG_DEFAULT）即可保住这类标签的可读性。
未覆盖
- macOS 与 Windows —— 此处仅 Linux。macOS 的
U+FE0Fabort 采信 PR 的报告。 - 真正的验收标准 —— 实跑
/verify是否终于产出图片 —— 无法从本地构建观察。那要看下一次运行，它仍是这第四次尝试的真正检验。 - publish job 的托管路径；本报告图片由我手动托管在 fork 分支上。
建议
修掉发现 1–2（一处防护表达式 + 两处断言字符串）后合入。成本很低；不修的话，本 PR 新增的这道防护会在默认行数下静默放过它本该报出的那种失败。发现 3–4 可作为后续跟进。
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review round summaryBoth suggestions from the automated reviewer are addressed. rc:3683769427 —
|
…ur (#8114) The truncation guard compared wrapped rows against --rows, but newline-terminated output needs one row beyond its last line (the final CRLF scrolls it off the scrollback-less viewport), so input of exactly --rows lines — including the default 40 — lost its top line with no warning, and taller input under-reported the drop by one. Compare against a capacity of rows - 1 for newline-terminated input. Also lift SGR 30 foreground to the default grey: it mapped to #1e1e1e, identical to the canvas background, so black-foreground labels (e.g. vitest's project badge) vanished as black-on-black.
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #8114Addressed the maintainer verification report from @wenshao. Findings 1–2 (the Finding 1 —
|
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| // (a line wider than --cols occupies ceil(len / cols) terminal rows) so the | ||
| // warning also fires when wrapping, not just newlines, pushes past --rows. | ||
| const ESC = String.fromCharCode(27); | ||
| const stripAnsi = (s) => |
There was a problem hiding this comment.
[Suggestion] the clipping-warning row count diverges from xterm's actual row count: stripAnsi only strips CSI [0-9;] sequences (not private-mode ESC[?…, OSC, charset, or the U+FE0F the render path strips), and .length counts UTF-16 code units rather than terminal columns. — Failure scenario (probe-confirmed): 25 CJK characters (50 columns) at --cols 40 wrap into 2 rows in xterm but compute ceil(25/40)=1, so a real top-row scroll fires no warning — the silent clipping this warning exists to prevent; an OSC title sequence conversely fires a false "dropped the top N".
| const stripAnsi = (s) => | |
| // Count rows from xterm's own buffer (e.g. term.buffer.active.baseY + viewport) | |
| // instead of re-deriving from the raw string, so wrapping and wide chars match. |
中文说明
裁剪告警的行数统计与 xterm 的真实行数不一致：stripAnsi 只剥离 CSI [0-9;] 序列（不含私有模式 ESC[?…]、OSC、字符集，以及渲染路径会剥掉的 U+FE0F），且 .length 统计的是 UTF-16 码元而非终端列宽。失败场景（已用 probe 验证）：25 个 CJK 字符（50 列）在 --cols 40 下，xterm 折成 2 行，但这里算出 ceil(25/40)=1，于是真正发生的顶部滚动不触发告警——正是该告警要防的静默裁剪；反过来，一段 OSC 标题序列又会误报 "dropped the top N"。建议：改用 xterm 自身缓冲区（如 buffer.active.baseY 加视口）来计数，使折行与宽字符与真实渲染一致。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Declined — the suggested baseY + viewport approach does not work with scrollback: 0 because baseY is always 0 when there is no scrollback buffer. A proper fix requires either a large scrollback setting to count total rows from the buffer, or a Unicode-width library for CJK column widths — both exceed this PR's scope. The warning is best-effort; the rendered image is always correct since xterm handles wrapping natively. Recording as a known limitation for a follow-up.
中文说明
已拒绝——建议的 baseY + viewport 方案在 scrollback: 0 下不可用，因为没有回滚缓冲区时 baseY 始终为 0。正确的修复需要设置较大的 scrollback 以从缓冲区计数总行数，或引入 Unicode 宽度库处理 CJK 列宽——两者都超出本 PR 范围。该告警是尽力而为的诊断；渲染图像始终正确，因为 xterm 原生处理折行。记录为已知限制，留待后续跟进。
- Declare sharp as a root devDependency so the script does not rely on workspace hoisting from packages/core - Guard against TTY stdin hanging silently: check process.stdin.isTTY before readFileSync(0) and print usage immediately - Fix phantom blank row when stdout already ends with a newline: only insert a separator between stdout and stderr when stdout lacks a trailing newline - Strengthen 256-colour/truecolor test to decode pixels and assert the #d4d4d4 fallback grey is present - Add test for non-newline-terminated input that fits exactly --rows - Add test for the phantom blank row fix (console.log + stderr)
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedrc:3685368102 —
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| const how = | ||
| res.signal != null ? `killed by ${res.signal}` : `exited ${res.status}`; | ||
| process.stderr.write(`verify-capture: command ${how}\n`); |
There was a problem hiding this comment.
[Suggestion] The signal-killed branch (res.signal != null) has no test — only process.exit(3) (which sets status=3, signal=null) is exercised. — Failure scenario: if a future edit inverted the ternary (res.signal == null), a signal-killed child (SIGSEGV, SIGKILL) would print "command exited null" instead of "command killed by SIGSEGV", and no test would catch it. The PNG is unaffected; only the diagnostic message is wrong.
Suggested fix: add a test that spawns a child which kills itself by signal, e.g. node -e "process.kill(process.pid, 'SIGTERM')", and assert stderr contains killed by SIGTERM.
中文说明
信号终止分支（res.signal != null）没有测试——目前只测了 process.exit(3)（产生 status=3, signal=null）。失败场景：如果将来有人把三元条件反转（res.signal == null），被信号终止的子进程（如 SIGSEGV、SIGKILL）会输出 "command exited null" 而非 "command killed by SIGSEGV"，且没有测试能捕获该回归。PNG 不受影响，仅诊断信息错误。
建议修复：添加一个测试，生成一个自杀的子进程，如 node -e "process.kill(process.pid, 'SIGTERM')"，并断言 stderr 包含 killed by SIGTERM。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action takenThis round has no actionable feedback:
No code changes were made. 中文说明未采取任何操作本轮没有可处理的反馈：
未做任何代码改动。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds: the automated reviewer's non-Critical suggestions below are deferred and stay open for human follow-up — do not modify code, resolve threads, or reply on their behalf. Maintainer feedback defers only once its author has already had 2 regular feedback batches addressed in this window's Critical-only tail — an account can host an automated reviewer loop, so the brake keys on measured regeneration, not identity; authors at their budget, if any, are named below. (A maintainer can lift the mode itself:
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式：以上为自动评审的非 Critical 建议，予以延后、保持开放并留待人工跟进——不要为其修改代码、解决线程或代为回复。维护者的反馈仅在其本人于本窗口 Critical-only 阶段已被处理 2 批常规反馈之后才会被延后——账号可能挂着自动评审循环，因此刹车依据实测的再生频度而非身份；达到预算的作者（如有）在下方点名。（如需解除该模式，评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Maintainer verification, round 2 — built and run locally on Linux, head
|
| Head verified | 5ac69ae8 — all 9 commits, incl. the 3 rounds since my last report |
| Platform | Linux x64 · Node 22.22.2 — the platform the PR marks ⚠️ untested |
| Setup needed | none: @xterm/headless 5.5.0 + sharp 0.34.5 resolve as-is; no browser, no native build |
| Suites | verify-capture.test.js 23/23 · qwen-triage-workflow.test.js 107/107 → 130/130 |
| eslint | clean |
| prettier | one line over print width — see note 2 |
| CI at this head | green (Test (ubuntu-latest, Node 22.x) 12m41s) |
Status of my previous four findings
| # | finding | status |
|---|---|---|
| 1 | --rows drops the top line silently at the default row count |
fixed — pixel-verified below |
| 2 | two tests pinned the off-by-one | fixed — and the corrected numbers are now pinned |
| 3 | the emoji regression test is vacuous on Linux | still true, deferred by the PR — reproduced as SURVIVED below |
| 4 | SGR 30 renders black-on-black and vanishes |
fixed — decoded pixels now contain #d4d4d4, not #1e1e1e |
How I verified it, without trusting the instrument
The warning under test is an arithmetic claim about how much output was lost, so counting rows the same way the helper counts them would prove nothing. Ground truth here is measured from the image: render the same bytes twice — once uncapped, once at the row cap — then find which uncapped row band the capped PNG begins at. That index is how many top rows were really lost. Nothing in it reuses the helper's counting.
Three arms, same oracle: my last report's head, this head, and the fix I suggest below.
| arm | disagreements with the pixels |
|---|---|
47fa5c19 — my last report |
7 / 10 |
5ac69ae8 — this head |
3 / 10 |
| suggested fix below | 0 / 10 |
The four rows that flipped from ✗ to ✓ between the two heads are exactly Findings 1–2 and their wrapped variants. That part is settled: at the default --rows 40, 40 newline-terminated lines now warn, and the number it reports is the number that actually went missing.
New finding (not a blocker) — the warning still describes a string, not a terminal
The three remaining disagreements share one root cause. wrappedRows re-derives the row count from the raw bytes:
sum + Math.max(1, Math.ceil(stripAnsi(line).length / opts.cols))That models neither display width (a CJK cell is 2 columns wide), nor cursor motion (a bare \r returns to column 0, so a repainted progress bar is one row, not one per repaint), nor non-CSI escapes (the strip is CSI-only, so an OSC 8 hyperlink's URL is counted as visible text). Reproduced:
| input | really dropped | warning says |
|---|---|---|
| 30 CJK lines, 60 wide chars each (60 real rows) | 21 | silence |
30 lines with \r repaints |
0 | 111 |
| 30 lines with OSC 8 hyperlinks | 0 | 21 |
Two of these are phantom warnings: they tell the reader an image is truncated when it is complete. The CJK one is the silent kind — the same failure class that was the blocker last round, and not exotic in a repo whose verification reports are bilingual. Here it is, captured by the helper at its default --rows: the header line 00 表头：这一行必须出现在截图里 and ten assertions are gone, and stderr printed nothing.
I am calling this non-blocking rather than repeating last round's blocker call, because the trigger narrowed: Finding 1 fired on any 40-line ASCII capture at the default, which is the common case; this needs wide characters, \r, or OSC in the captured stream.
Suggested fix — ask the terminal instead of the string
The helper already owns a parser that knows all three things. A second, tall probe terminal answers the question directly:
const probe = new Terminal({
cols: opts.cols, rows: PROBE_ROWS, scrollback: 0, allowProposedApi: true,
});
await new Promise((r) => probe.write(normalised, r));
// The cursor sits on the row after the last line when input is
// newline-terminated, and that row counts against the viewport too.
const occupied = probe.buffer.active.cursorY + 1;
const dropped = Math.max(0, occupied - opts.rows);
if (dropped > 0) {
const atLeast = occupied >= PROBE_ROWS ? 'at least ' : '';
process.stderr.write(
`verify-capture: warning: input occupies ${atLeast}` +
`${rows.length + dropped} terminal rows; --rows ${opts.rows} kept the ` +
`last ${rows.length} and dropped the top ${dropped}\n`,
);
}It deletes stripAnsi, wrappedRows and the capacity special-case, trading that arithmetic for a direct measurement (+25 / −24 lines), and under it:
- the oracle goes 10/10, including all three cases above;
- the PR's 23 tests pass unchanged — no assertion churn at all, because the numbers it produces for plain ASCII are the ones the suite already pins;
- cost is one extra parse of bytes already in memory.
PROBE_ROWScaps the count, hence theat leastwording above that ceiling.
Does the suite defend its own fixes?
Each row reverts one behaviour at head and re-runs the 23 tests.
Six of eight are KILLED, including both fixes made in response to my last report. The two survivors are the expected ones and both are honest: the U+FE0F strip guards a macOS-only Pango abort() (my previous Finding 3, deferred with a recorded reason — still unprotected where CI actually runs), and the TTY-stdin guard added in 5ac69ae8 has no test because every test pipes stdin.
Claims re-checked at head
Eleven behaviour and pixel checks, decided by decoding the written PNG and reading exit codes — never by the helper's stdout.
The CRLF row carries its own counterfactual: with the fix, the three rendered rows share a left edge within 1 px; with the normalisation removed, the same input staircases to a 186 px spread. SGR 30 now paints at #bfbfbf/#d4d4d4 instead of vanishing into the background, which closes Finding 4.
Smaller notes
- The PR description's Cause 1 no longer matches the code. It says
node-ptyis "not a dependency of this repo, and needs a native build" and that theplaywrightpackage is "not a declared dependency". Neither holds:@lydell/node-ptyis a rootoptionalDependencyshipping a prebuiltlinux-x64binary — I spawned a real pty with it on this box — andplaywrightis declared inpackages/webuiandintegration-tests/terminal-capture. The header comment inverify-capture.mjsalready concedes this, andSKILL.mdwas corrected in a later round; only the PR body still carries the disproven version. The conclusion is untouched — the route still needs a browser, is slow, andintegration-tests/terminal-captureis not a root workspace, so it resolves only by hoisting — but please update the body before merge so the record matches what shipped. - prettier:
scripts/verify-capture.mjs:138(the TTY guard added in5ac69ae8) exceeds print width. CI runsprettier --write .with no diff check, so this can never fail CI — it is a one-line reformat, not a gate. - package-lock metadata: root
package.jsongainedsharp: ^0.34.5, but the lock'spackages[""]was not refreshed.npm ciaccepts it because the existing top-levelnode_modules/sharp@0.34.5satisfies the range — confirmed by a dry-run here and by green CI — so this is drift, not a break. FORCE_COLORchanges what you capture. The helper setsFORCE_COLOR=1/CLICOLOR_FORCE=1on the captured command. That is what makes captures colourful, but it means the command's output can differ from what you saw running it by hand — it silently broke my own mutation harness, whose regex overvitest's summary read-1 passeduntil I stripped SGR before matching. One line inSKILL.mdwould save the next person the same detour: if your harness parses its own subprocess output, strip ANSI first.
Not covered
- macOS and Windows — Linux only here; the macOS Pango behaviour is taken on report.
- The actual acceptance criterion — whether a live
/verifyrun now produces images — still cannot be observed from a local build. That remains the real test of this fourth attempt. - The publish job's hosting path; I hosted these five images manually on a fork branch.
Recommendation
Merge. The central claim holds on the untested platform with zero setup, the regressions I reported are fixed and now pinned, and nothing outstanding blocks the deliverable. My preference is to take the terminal-probe fix in this PR — it is contained, replaces arithmetic with a direct measurement, and needs no test edits — but it is equally fine as a follow-up. Note 1 (the stale Cause 1 table in the description) is worth correcting before merge, since that paragraph is the reasoning future rounds will inherit.
中文版
维护者验证（第 2 轮）—— 本地 Linux 真实构建与运行，head 5ac69ae8
结论：可以合入。 我上一份报告（47fa5c19）中的两个阻断项已真正修复，而且我这次是拿渲染出的像素去核对的，而不是用 helper 自己的算术 —— 上一轮出问题的恰恰就是那套算术。六轮 autofix 中做出的每一处修复，现在都有测试守住。剩下的是同一类的一个非阻断缺陷，我给出的修法已端到端验证，且不需要改动任何测试。
下面每一张图都由本 PR 的 helper 自己产出，因此这份报告同时也是对交付物的一次实跑检验。
环境
| 验证 head | 5ac69ae8 —— 全部 9 个 commit，含上次报告之后的 3 轮 |
| 平台 | Linux x64 · Node 22.22.2 —— 正是 PR 标记为 ⚠️ 未测的平台 |
| 所需配置 | 无：@xterm/headless 5.5.0 + sharp 0.34.5 直接可解析；不需浏览器、不需原生编译 |
| 测试套件 | verify-capture.test.js 23/23 · qwen-triage-workflow.test.js 107/107 → 130/130 |
| eslint | 干净 |
| prettier | 有一行超出行宽 —— 见小结 2 |
| 该 head 的 CI | 全绿（Test (ubuntu-latest, Node 22.x) 12m41s） |
上次四个发现的状态
| # | 发现 | 状态 |
|---|---|---|
| 1 | 默认行数下 --rows 静默丢掉首行 |
已修复 —— 下文以像素核实 |
| 2 | 两条测试钉住了 off-by-one | 已修复，且现在钉住的是正确数字 |
| 3 | emoji 回归测试在 Linux 上空转 | 依然成立，PR 已记录延后 —— 下文复现为 SURVIVED |
| 4 | SGR 30 黑底黑字而消失 |
已修复 —— 解码像素中出现 #d4d4d4 而非 #1e1e1e |
我如何在不信任仪器的前提下验证
被测的这条警告本身就是"丢了多少输出"的算术主张，所以用与 helper 相同的方式去数行，等于什么都没证明。这里的基准真值取自图像：同样的字节渲染两次 —— 一次不设上限，一次设行数上限 —— 然后找出受限的 PNG 是从不受限渲染的第几个行带开始的。这个下标就是真正被丢掉的顶部行数，全过程不复用 helper 的任何计数逻辑。
三个臂、同一套 oracle：上次报告的 head、本次 head、以及我建议的修法。
| 臂 | 与像素不符的用例 |
|---|---|
47fa5c19 —— 上次报告 |
7 / 10 |
5ac69ae8 —— 本次 head |
3 / 10 |
| 下文建议的修法 | 0 / 10 |
两个 head 之间由 ✗ 翻为 ✓ 的四行，正是发现 1–2 及其折行变体。这部分已经落定：默认 --rows 40 下、40 行以换行结尾的输入现在会告警，且报出的数字与真正丢失的行数一致。
新发现（非阻断）—— 这条警告描述的仍是字符串，不是终端
剩下三处不符同出一源。wrappedRows 是从原始字节反推行数的：
sum + Math.max(1, Math.ceil(stripAnsi(line).length / opts.cols))它既没有建模显示宽度（一个 CJK 字符占 2 列），也没有建模光标移动（裸 \r 回到第 0 列，因此反复重绘的进度条只占一行，而不是每次重绘一行），更没有建模非 CSI 转义（剥离逻辑只处理 CSI，于是 OSC 8 超链接里的 URL 被当作可见文本计入）。复现如下：
| 输入 | 实际丢失 | 警告所报 |
|---|---|---|
| 30 行 CJK，每行 60 个宽字符（实占 60 行） | 21 | 无任何输出 |
30 行含 \r 重绘 |
0 | 111 |
| 30 行含 OSC 8 超链接 | 0 | 21 |
其中两种是幽灵告警：图片本是完整的，却告诉读者它被截断了。CJK 那一种则是静默的 —— 与上一轮那个阻断项同属一类，而在一个验证报告需要双语的仓库里，它并不冷僻。上图即为 helper 在默认 --rows 下拍到的实况：表头 00 表头：这一行必须出现在截图里 连同十条断言一起消失，而 stderr 一个字也没打印。
之所以这次判为非阻断、而非重复上一轮的阻断结论，是因为触发面变窄了：发现 1 在任何 40 行 ASCII 的默认截图上都会发作，那是最常见的情形；而本次需要被截取的流中出现宽字符、\r 或 OSC。
建议修法 —— 去问终端，别去问字符串
helper 手里本来就有一个知道这三件事的解析器。用第二个"高"探针终端直接问它即可（代码见英文版）。
该改动删掉了 stripAnsi、wrappedRows 以及 capacity 特例，用一次直接测量取代那套算术（+25 / −24 行），效果：
- oracle 达到 10/10，含上述三种情形；
- PR 的 23 条测试原样全过 —— 没有任何断言需要改，因为它对纯 ASCII 给出的数字正是测试已经钉住的那些；
- 代价是对已在内存中的字节多解析一次。
PROBE_ROWS会给计数封顶，因此超出该上限时措辞为at least。
测试是否守得住自己的修复？
矩阵中每一行都在 head 上回退一处行为并重跑 23 条测试。八项中六项 KILLED，其中包含针对我上次报告做的两处修复。两个存活项都在预期之内、且都诚实：U+FE0F 剥离防的是 macOS 特有的 Pango abort()（即我上次的发现 3，已记录理由延后 —— 在 CI 真正运行的平台上仍无防护）；而 5ac69ae8 新增的 TTY-stdin 守卫没有测试，因为所有测试都是从管道喂 stdin 的。
在 head 上复核的各项主张
十一项行为与像素检查，判定依据是解码写出的 PNG 与进程退出码，而非 helper 自己的 stdout。
其中 CRLF 那一行自带反事实对照：有该修复时，三行渲染的左边缘相差 1 px；把归一化去掉后，同样的输入呈阶梯状、相差 186 px。SGR 30 现在以 #bfbfbf/#d4d4d4 着色而不再没入背景，发现 4 就此关闭。
若干小结
- PR 描述中的"病因 1"已与代码不符。 描述称
node-pty"本仓库没有这个依赖，且需要原生编译"、playwright包"非声明依赖"。两点都不成立：@lydell/node-pty是根optionalDependency且带linux-x64预编译产物 —— 我在这台机器上用它拉起了真实 pty；playwright则在packages/webui与integration-tests/terminal-capture中有声明。verify-capture.mjs的头部注释其实已经改口，SKILL.md也在后续轮次修正过，只有 PR 正文仍保留着已被证伪的版本。结论本身不受影响 —— 那条路径确实需要浏览器、确实慢，且integration-tests/terminal-capture不是 root workspace、仅靠 hoisting 才解析得到 —— 但请在合入前更新正文，让记录与实际交付一致。 - prettier：
scripts/verify-capture.mjs:138（5ac69ae8新增的 TTY 守卫）超出行宽。CI 执行的是prettier --write .且不做 diff 校验，因此它永远不会让 CI 变红 —— 属于一行格式化，不是门禁。 - package-lock 元数据：根
package.json新增了sharp: ^0.34.5，但 lock 的packages[""]未同步刷新。npm ci仍然接受，因为已有的顶层node_modules/sharp@0.34.5满足该范围 —— 本地 dry-run 与绿色 CI 均已确认 —— 所以这是元数据漂移，不是故障。 FORCE_COLOR会改变你截到的东西。 helper 会给被截取的命令设置FORCE_COLOR=1/CLICOLOR_FORCE=1。这正是截图有颜色的原因，但也意味着该命令的输出可能与你手动运行时看到的不同 —— 它就悄悄搞坏了我自己的变异 harness：其匹配vitest汇总行的正则一直读出-1 passed，直到我在匹配前先剥掉 SGR。在SKILL.md里加一句就能让下一个人少走这段弯路：如果你的 harness 要解析它自己子进程的输出，先剥 ANSI。
未覆盖
- macOS 与 Windows —— 此处仅 Linux；macOS 的 Pango 行为采信 PR 的报告。
- 真正的验收标准 —— 实跑
/verify是否终于产出图片 —— 依然无法从本地构建观察。 那仍是这第四次尝试的真正检验。 - publish job 的托管路径；本报告这五张图由我手动托管在 fork 分支上。
建议
合入。 核心主张在此前未测的平台上零配置成立，我报过的回归已修复并被测试钉住，没有任何未决项阻碍交付。我倾向于把"探针终端"这版修法一并放进本 PR —— 它足够内聚、以直接测量取代算术、且无需改测试 —— 但作为后续跟进同样可以接受。小结 1（描述中已过时的"病因 1"表格）建议在合入前订正，因为那段推理会被后续轮次继承下去。
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at 5ac69ae. The helper is sound where it matters. Command execution is injection-safe: spawnSync(cmd[0], cmd.slice(1)) with no shell:true, argv taken verbatim after --. The blank-capture claim is implemented better than advertised — the code awaits xterm's write callback (fires when the parser has drained), which is strictly stronger than the one-event-loop-turn the PR body describes. Non-zero child exit still captures, CRLF normalisation, trailing-blank-row trim, and empty-output exit-1 each have a test that spawns the real script and decodes actual PNG pixels via sharp — no reimplementation. A hanging child has no inner timeout (spawnSync without timeout), but the verify lane wraps the agent in timeout --kill-after plus a job-level limit, so the exposure is bounded by design. Dependency hygiene checks out: sharp added to root devDependencies was already in the lockfile at the base (grep count identical base vs HEAD), so npm ci is unaffected, and the workflow runs npm ci without --omit=dev before the helper can execute. The hosting fix is real and pinned from both sides: I confirmed the old --branch pr-assets clone was structurally impossible (402 refs/heads/pr-assets/* refs exist, so the bare ref cannot), the workflow now uses pr-assets/${PR_NUMBER}-verify with orphan-init, and tests pin the branch name in publish and cleanup plus reject the bare-leaf pattern. SKILL.md matches the implementation exactly (path, flags, colour-fallback wording, ~2-minute budget line), and the dead node-pty/Playwright route is gone with a test asserting its absence.
On the three open threads, checked against this HEAD: the 256/truecolor fallback (thread 1) is now explicitly documented in the code comment and SKILL.md and pinned by a pixel-decoding test — it's a deliberate, disclosed limitation rather than a silent bug, so I'd resolve it as declined-by-design (a follow-up could map 38;5/38;2 properly, but a grey render is still legible evidence). Threads 2 (clipping-warning row estimate vs private-mode/OSC/wide-char reality) and 3 (untested res.signal != null branch) both still stand — legitimate Suggestion-class hardening, warning-text accuracy and one missing test respectively, neither affecting the capture output itself; fine to take in this PR or defer, but the threads should be closed out explicitly either way.
Ran both touched suites locally at this HEAD: verify-capture 23/23, triage-workflow 107/107. CI green. No new findings.
4 participants
What this PR does
Ships
scripts/verify-capture.mjsso a/verifyevidence image is one command instead of a pipeline the agent has to build:node scripts/verify-capture.mjs --out evidence/01-ab.png \ --title 'A/B: the gate flips on noisy data' -- node my-harness.mjs(That image was produced by the helper in this PR — command →
@xterm/headlesscell grid → SVG →sharp. No browser, no pseudo-terminal.)Why — the fourth attempt, and the first aimed at the real cause
The #8104 run settles it. Same PR (#7975),
Install evidence browser: success, and the verification got deeper — 64 assertions vs 40, 53 tables vs 31 — while still producing zero images. An agent that read the instruction, did more work than before, and still did not capture is not an agent that missed the instruction.Cause 1 — the documented route did not exist
The skill sent the agent to build
node-pty→xterm.js→ Playwright itself:node-ptyplaywrightpackagenpx --yes)@xterm/headlesssharpSo the incentive was entirely one-way: authoring that pipeline risks failing and eats budget; skipping costs nothing and is invisible. I should have checked this when I wrote #8016 — I verified chromium installs and never verified the route the skill actually names. That is the "corroborate an instrument with something that does not use it" rule from this same skill, and I did not apply it to my own change.
Cause 2 — the hosting branch could not exist either
Found while hosting the image above. The old code did:
There are 402
refs/heads/pr-assets/*refs in this repo. Git cannot have a ref that is simultaneously a file and a directory, sorefs/heads/pr-assetscannot exist — a bare push to it is rejected withdirectory file conflict, which I reproduced. That clone always failed, the function returned, and a text-only report went out.This is a third independent reason images were 0/14, and it fired before wording ever mattered. It also means the autofix bot's switch to per-PR branches in
029beba3efwas load-bearing, not the cosmetic tidy-up I took it for at the time.What the helper does
Command (or stdin) →
@xterm/headlessparses ANSI into a cell grid with colour and bold → SVG →sharprasterises. Notable behaviours, each pinned by a test:write(). Without it the buffer is still empty and the capture comes out blank.The skill's dead route is replaced with the command, and the budget line drops from ~5 minutes to ~2 because there is no pipeline to author.
QWEN_VERIFY_CHROMIUMand its install stay for a future web-UI capture, but the terminal route no longer depends on them — gating on a browser the route does not use is how an absent browser becomes a skipped capture.Reviewer Test Plan
How to verify
12 tests, every one running the real helper and inspecting the real PNG (signature bytes, not file existence). A test that only checked the skill's wording would have passed through all four failed rounds.
Mutation-verified 7/7:
--cols 0/-5/abc/9999Mutations 3 and 4 initially SURVIVED. The colour test compared "coloured and bold" against plain and asserted the bytes differ — which passes while either attribute survives. That is precisely the wrong-reason trap this skill warns about, met in the skill's own test file. Each attribute is now isolated against the same plain baseline (green-no-bold, bold-no-colour) and both mutations kill.
119/119 across both suites; prettier and eslint clean.
Tested on
Linux:
@xterm/headlessis pure JS andsharpships prebuilt linux-x64 binaries thatnpm cialready installs for this repo, so the pipeline has no platform-specific step — but it has not executed on a runner yet. The acceptance check is the next/verifyrun: images per report going from 0 to a small purposeful number, with the A/B table carrying a filename beside it.Risk & Scope
evidence/was empty, so the absence is visible to the maintainer and to the next round's agent readingprevious-report.md. I would rather ship that after seeing whether removing the obstacle was enough.pr-assetsref finding above is reported, not fixed — the bot's per-PR scheme already handles it.Linked Issues
Fourth follow-up to #8016; supersedes the approach in #8104 (whose budget line is kept and re-costed). No issues closed.
中文说明
本 PR 做了什么
新增
scripts/verify-capture.mjs，让一张/verify证据图变成一条命令，而不是让 agent 自己搭一条管线：node scripts/verify-capture.mjs --out evidence/01-ab.png \ --title 'A/B: the gate flips on noisy data' -- node my-harness.mjs（上方那张图正是本 PR 的 helper 产出的——命令 →
@xterm/headless单元格网格 → SVG →sharp。不用浏览器，也不用伪终端。）为什么——第四次尝试，也是第一次对准真正的病因
#8104 那次运行是决定性的。同一个 PR（#7975）、
Install evidence browser: success、而且验证本身更深了——断言 64 对 40、表格 53 对 31——却仍然产出 0 张图。一个读了指令、比以前做得更多、却仍然不截图的 agent，不是"漏看了指令"的 agent。病因 1 —— 文档指定的路径根本不存在
skill 让 agent 自己搭
node-pty→xterm.js→ Playwright：node-ptyplaywright包npx --yes）@xterm/headlesssharp于是激励完全是单向的：自己搭那条管线有失败风险、要吃预算；而跳过零代价且不可见。我写 #8016 时就该查这一点——我验证了 chromium 能安装，却从未验证 skill 实际指定的那条路径。这正是这份 skill 里"当 oracle 是仪器时，要用不依赖它的机制佐证"那条规则，而我没有把它用在自己的改动上。
病因 2 —— 托管分支同样无法存在
这是在为上面那张图做托管时发现的。旧代码是：
本仓库有 402 个
refs/heads/pr-assets/*ref。git 不允许同一路径既是文件又是目录，因此refs/heads/pr-assets不可能存在——直接推送该 ref 会被拒绝并报directory file conflict，我已复现。这条 clone 永远失败，函数直接 return，发出纯文本报告。这是图片为 0/14 的第三个独立原因，而且在措辞问题起作用之前就已生效。这也意味着 autofix bot 在
029beba3ef中改为每 PR 分支是承重改动，而不是我当时以为的顺手整理。helper 做了什么
命令（或 stdin）→
@xterm/headless把 ANSI 解析成带颜色与粗体属性的单元格网格 → SVG →sharp栅格化。以下行为各有测试钉住：write()后让出一轮事件循环。 否则缓冲区仍为空，截图静默产出空白。skill 中那条死路径被替换为该命令，预算项从约 5 分钟降到 约 2 分钟，因为不再需要搭管线。
QWEN_VERIFY_CHROMIUM及其安装保留，供将来的 web UI 截图使用，但终端截图路径不再依赖它们——对一条并不使用浏览器的路径设置浏览器门槛，正是"浏览器缺失"变成"跳过截图"的原因。评审验证方案
12 条测试，每一条都运行真实的 helper 并检查真实的 PNG（签名字节，而非文件是否存在）。一条只检查 skill 措辞的测试，会在此前四次失败的轮次中全部通过。
7/7 变异验证：裸 LF（阶梯渲染）、去掉事件循环让出（空白截图）、去掉粗体、去掉全部颜色、不裁剪空行、容忍空截图、去掉几何校验。
变异 3 与 4 起初存活。 颜色测试比较的是"带颜色且加粗"与纯文本，断言字节不同——而这在任一属性存活时都会通过。这正是本 skill 所警告的"因错误原因通过"陷阱，出现在 skill 自己的测试文件里。现在每个属性各自与同一纯文本基线隔离比较（绿色不加粗、加粗不着色），两个变异均致死。
两个套件共 119/119 通过；prettier 与 eslint 干净。
测试平台
macOS ✅；Windows N/A；Linux⚠️ ——
@xterm/headless是纯 JS，sharp提供本仓库npm ci已安装的 linux-x64 预编译产物，因此该管线没有平台相关步骤——但尚未在 runner 上执行过。验收标准是下一次/verify运行：每份报告的图片数从 0 变成一个少而有目的的数值，且 A/B 表格旁带有文件名。风险与范围
evidence/为空"时在评论中写明，使这一缺失对维护者、以及对下一轮读previous-report.md的 agent 都可见。我更愿意先看清"移除障碍"是否已经足够，再上这一步。pr-assetsref 的发现是报告而非修复——bot 的每 PR 分支方案已经处理了它。关联 Issue
#8016 的第四次后续；取代 #8104 的做法（其预算项保留并重新估价）。不关闭任何 issue。