feat(cli): add TUI image display tool - #8217
Conversation
E2E test reportManual observations during development
Automated verification on the final rebased branch
The targeted coverage includes TUI-only registration, workspace/path and PNG validation, restored-result safety, native placeholder generation, Warp exclusion, Still pending
|
github-actions
Bot
added
the
review/self-reported
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: this is a feature request (linked issue #8216, self-reported) rather than a bug fix. The motivation is genuine — repositories routinely contain image assets, and the current TUI has no way to preview them without leaving the session. The issue is well-specified with clear scope boundaries (PNG-only, capability-gated, explicit failure reporting). Direction: aligned. An interactive coding assistant benefits from keeping visual inspection in-workflow, and the tool is deliberately display-only (no image bytes enter model context). The capability-gated rendering with explicit failure reporting is the right posture. CHANGELOG has no direct terminal-image-preview reference, but the area is active — multiple image-handling fixes in recent releases confirm images are a relevant surface. Size: 963 production logic lines (359 core, 602 CLI, 2 web-shell) + 1268 test lines + 79 docs + 11 patch. Core production is under the 500-line escalation threshold. Total production is under the 1000-line advisory. The core touch points (config, tools, agents, fork-subagent) are each necessary for the feature: tool registration, config plumbing, and fork/subagent exclusion. Approach: the scope feels right for the stated goal. One new tool, one renderer, one TUI component — plus the fork/subagent plumbing needed to keep the tool main-session-only while preserving prompt-cache parity. The renderer reuses existing Mermaid infrastructure ( Risk: no elevated risk signals — none of the changed files match the high-risk path patterns from the revert-history analysis. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个功能请求（关联 issue #8216，作者自行提交），而非 bug 修复。动机是真实的——代码仓库中普遍包含图片资源，而当前 TUI 无法在会话内预览。Issue 规格清晰，范围边界明确（仅 PNG、能力判断、明确失败报告）。 方向：对齐。交互式编码助手受益于将视觉检查留在工作流内，且该工具刻意只做展示（图片字节不进入模型上下文）。能力判断加明确失败报告是正确的姿态。CHANGELOG 没有直接的终端图片预览引用，但该领域活跃——近期版本中有多项图片处理修复。 规模：963 行生产逻辑（359 核心、602 CLI、2 web-shell）+ 1268 行测试 + 79 行文档 + 11 行补丁。核心生产行低于 500 行升级阈值，总生产行低于 1000 行建议阈值。核心触点（config、tools、agents、fork-subagent）均为功能所需。 方案：范围与目标匹配。一个新工具、一个渲染器、一个 TUI 组件——加上保持工具仅限主会话同时维护 prompt-cache 一致性的 fork/subagent 管道。渲染器复用了现有 Mermaid 基础设施而非构建平行工具，这很好。 风险：无升级风险信号——变更文件均未命中 revert 历史分析中的高风险路径模式。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: for adding terminal image previews, I would create a core tool that validates the path and returns a small structured display value (keeping image bytes out of model history), a CLI-side renderer that detects terminal capabilities and falls back gracefully, and a TUI component that renders synchronously for Ink's Static region. Registration would be gated to interactive mode only, and subagents would be excluded. Comparison with the diff: the PR's approach matches this closely and goes further in one important way — the fork/subagent handling preserves the parent's No critical blockers found. Specific observations:
Files changed (30 of 30 shown)
TestingFinal CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The ubuntu test suite and web-shell visual capture are still running. macOS and Windows tests are skipped (likely gated on the ubuntu suite passing first). No failures so far, but the primary test evidence is not yet available. Sandboxed verification would settle the rendering claims: 中文说明代码审查独立方案： 对于添加终端图片预览，我会创建一个核心工具来验证路径并返回小型结构化显示值（不让图片字节进入模型历史），一个 CLI 端渲染器来检测终端能力并优雅降级，以及一个为 Ink Static 区域同步渲染的 TUI 组件。注册仅限交互模式，子代理被排除。 与 diff 的比较： PR 的方案与此高度一致，并在一个重要方面走得更远——fork/subagent 处理在 fork 的工具注册表中保留了父级的 未发现关键阻塞项。安全姿态扎实（工作区路径验证、PNG 签名校验、大小限制、cmd.exe 元字符拒绝、子进程环境净化）。复用良好（从 mermaidImageRenderer 导出五个函数而非重复实现）。缓存有界。同步渲染正确。 测试ubuntu 测试套件和 web-shell 视觉捕获仍在运行。目前无失败，但主要测试证据尚不可用。 沙盒验证可以确认渲染声明： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — solid, well-tested feature with clean architecture; CI still running and terminal rendering claims rest on the author's manual testing. Stepping back: this PR does one thing well. It adds a display-only image preview tool to the interactive TUI, with proper capability detection, graceful degradation, and explicit failure reporting. The architecture is clean — core validates and returns structured data, CLI renders, and the two never mix image bytes into model context. The fork/subagent handling is the most complex part, but it solves a real problem (prompt-cache parity) with a well-documented three-layer approach. Going back to my independent proposal: the PR matches it and exceeds it in the fork cache-parity handling, which I wouldn't have thought of initially. The reuse of Mermaid renderer infrastructure is exactly right — no parallel utilities, no duplicated logic. The test suite is comprehensive (50 renderer tests, tool tests, component tests, fork integration tests) and covers the security boundaries I care about (workspace traversal, cmd.exe injection, env leakage, oversized files). The reservations are honest ones: the ubuntu test suite hasn't finished, so I don't have CI confirmation that the 1268 test lines actually pass. And the rendering behavior — Kitty placement, chafa output, Warp exclusion — is inherently terminal-specific and can't be verified from a diff. The author tested on macOS with Kitty, Ghostty, and Warp, which covers the primary paths, but Linux and Windows remain unvalidated (the author marks them If I had to maintain this in six months, I'd thank the author — the design doc, the bounded caches, the explicit failure modes, and the test coverage make this straightforward to reason about. The code doesn't try too hard; it solves the stated problem without speculative generality. Not blocking, but worth noting for the maintainer: the Approval deferred until CI lands green on 中文说明置信度：4/5 —— 实现扎实、测试充分、架构清晰；CI 仍在运行，终端渲染声明依赖作者的手动测试。 退一步看：这个 PR 做好了一件事。它为交互式 TUI 添加了一个仅展示的图片预览工具，具备正确的能力检测、优雅降级和明确失败报告。架构清晰——核心验证并返回结构化数据，CLI 渲染，两者都不将图片字节混入模型上下文。fork/subagent 处理是最复杂的部分，但它用文档齐全的三层方案解决了一个真实问题（prompt-cache 一致性）。 与我的独立方案相比：PR 与之匹配并在 fork cache-parity 处理上超越了我最初的想法。Mermaid 渲染器基础设施的复用完全正确。测试套件全面，覆盖了我关心的安全边界。 保留意见是诚实的：ubuntu 测试套件尚未完成，渲染行为本质上是终端特定的，无法从 diff 中验证。作者在 macOS 上用 Kitty、Ghostty 和 Warp 进行了测试，覆盖了主要路径，但 Linux 和 Windows 仍未验证。 不阻塞，但值得维护者注意： 批准推迟至 CI 在 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
⏸️ Deferring to @wenshao — this one needs a human call, not an auto-merge. The code review is clean (no blockers; the security posture and test coverage are strong), but two things put it outside what the gate should approve on its own:
The specific design question worth your eye: the fork machinery keeps the Not requesting changes — I found nothing to reject. Just handing the merge decision to you. |
|
@qwen-code /resolve |
| const stdout = execFileSync( | ||
| 'chafa', | ||
| [ | ||
| '--animate=off', |
There was a problem hiding this comment.
[Critical] The chafa fallback spawns a subprocess synchronously inside React render. renderTerminalImage is synchronous and its non-Kitty path calls renderWithChafa → execFileSync('chafa', …, { timeout: 8000 }); TerminalImage.tsx:42 invokes it inside a React.useMemo, i.e. during render. — Failure scenario: on any non-Kitty/Ghostty terminal (tmux / SSH / most TERM — the common case, where supportsKittyImageProtocol returns false), displaying a display_image result blocks the Ink event loop (input handling and repaint) for the entire chafa run, up to 8 s. The useMemo deps include availableTerminalHeight and contentWidth, so a terminal resize re-triggers the blocking spawn. The sibling mermaidImageRenderer.ts deliberately keeps external processes outside React render via an async renderer, and MermaidDiagram.tsx proves the async useEffect+setState pattern works in the same Static region — the design doc's 'an async update would be dropped in Static' rationale does not hold.
Fix (spans this file + TerminalImage.tsx): add an async variant that uses a promisified execFile for the chafa path, resolve it in useEffect+setState mirroring MermaidDiagram.tsx, and keep the sync renderTerminalImage only for tests/headless.
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Still escalated — this needs a maintainer design call, not an auto-fix.
The prescribed fix (an async execFile resolved via useEffect+setState, mirroring MermaidDiagram.tsx) was implemented and tested, and it breaks the feature: completed tool rows render in Ink's append-only <Static> region, which renders each item once on mount and drops later state updates. A <Static>-wrapped TerminalImage driven by the async renderer produced an empty frame for the kitty/ansi/unavailable cases. MermaidDiagram survives this only because it always renders a synchronous wireframe fallback and treats the native image as an enhancement; TerminalImage has no fallback — the image is the content — so the same pattern renders nothing once committed to history. This is the rationale documented in docs/design/tui-display-image-tool.md. The async change was reverted to avoid shipping a regression.
The viable fixes are architectural:
- (a) keep the bounded synchronous render (status quo — now a single render rather than two, since the probe no longer spawns chafa); or
- (b) pre-render asynchronously during
execute()and carry the sized output in the display payload, so the component renders synchronously from pre-computed data (changes theTerminalImageDisplaydata model and sizing flow).
Question for the maintainer: which direction do you want — accept the bounded sync render (a), or take the data-model change to pre-render off the render path (b)? Leaving this thread open for that decision.
中文说明
仍为升级状态——这需要维护者做设计决策，而非自动修复。
我们已实现并测试了 prescribed 方案（仿照 MermaidDiagram.tsx、经 useEffect+setState 解析的异步 execFile），结果破坏了功能：已完成的工具行渲染在 Ink 只追加的 <Static> 区域，该区域在挂载时对每个条目只渲染一次，之后的状态更新会被丢弃。用 <Static> 包裹、由异步渲染器驱动的 TerminalImage 在 kitty/ansi/unavailable 三种情况下都得到空帧。MermaidDiagram 之所以不受影响，是因为它始终同步渲染一个线框兜底、并把原生图片仅作为增强；而 TerminalImage 没有兜底——图片本身就是内容——因此同样的模式在提交进历史后会渲染为空。这正是 docs/design/tui-display-image-tool.md 中记录的设计依据。为避免引入回归，已回退该异步改动。
可行的修复属于架构层面：
- (a) 保留有界的同步渲染（现状——由于探测不再触发 chafa，现在是一次渲染而非两次）；或
- (b) 在
execute()期间异步预渲染，并把已定尺寸的输出放进 display 载荷，使组件从预计算数据同步渲染（这会改动TerminalImageDisplay数据模型与定尺寸流程）。
给维护者的问题： 你倾向哪个方向——接受有界的同步渲染 (a)，还是接受数据模型改动以在渲染路径之外预渲染 (b)？该线程保持打开以待决策。
There was a problem hiding this comment.
Escalated to maintainer. The async-vs-sync tradeoff is a design call, as the reviewer noted. Converting renderTerminalImage from execFileSync to async execFile inside the React component requires decisions on: (1) loading-state UX while chafa runs, (2) resize cancellation/debouncing to avoid stale spawns, (3) useMemo → useEffect + useState lifecycle change.
Recommendation: move the chafa spawn into a useEffect with an AbortController, debounce on resize, show a one-line placeholder while pending. The Kitty path (pure computation) stays in useMemo. ~50-line change to TerminalImage.tsx + terminal-image-renderer.ts, but it touches the component's render contract.
Leaving this thread open for the maintainer's decision.
中文说明
已升级至维护者。 异步与同步的权衡是设计决策，正如审阅者所指出的。将 renderTerminalImage 从 execFileSync 转为异步 execFile 需要以下决策：(1) chafa 运行时的加载状态 UX，(2) resize 时的取消/防抖以避免过时的启动，(3) useMemo → useEffect + useState 生命周期变更。
建议：将 chafa 启动移入带 AbortController 的 useEffect，resize 时防抖，等待期间显示单行占位符。Kitty 路径（纯计算）保留在 useMemo 中。约 50 行变更，但涉及组件渲染契约。
保持此线程开放，等待维护者决定。
Combine the display_image execution restriction from the TUI image display tool with main's ask_user_question restriction (buildForkExecutionAllowlist + EXCLUDED_TOOLS_FOR_SUBAGENTS). A fork now keeps the parent's display_image declaration for prompt-cache parity but can execute neither display_image nor ask_user_question. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Root causeMain commit Textual or semanticSemantic — both sides rewrote the same allowlist computation. The resolution composes both filters: executionAllowedTools: resolveForkExecutionAllowedTools(
parentToolNames,
buildForkExecutionAllowlist(this.params.fork_tools, declaredExecutionToolNames),
agentConfig.getToolRegistry().getAllToolNames(),
),
Load-bearing
Could not verifyNo build/typecheck/tests run (per instructions). I hand-traced the resolution against the auto-merged 中文说明根因：main 的 语义冲突：双方重写了同一段白名单计算，解决方式为组合两种过滤（代码见上）——先 关键约束：顺序为先 无法验证：按指令未运行 build/typecheck/测试，已手工对照合并后的 |
|
Qwen Code did not run conflict resolution for this request. PR #8217 does not currently have merge conflicts with main. |
|
@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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
Local re-verification at
|
| PR head | 8f8b984f0 (13 commits behind main, merges cleanly, mergeable: MERGEABLE) |
| OS / arch | macOS 26.6 (Darwin 25.6.0), arm64 |
| Node | v24.18.1 |
| Terminal | Ghostty — a dedicated fullscreen instance per case, captured by CGWindow id |
| Fallback renderer | chafa 1.18.2 (Homebrew) |
| Model | qwen3.8-max-preview, real API, --approval-mode yolo |
Method — how the runs were driven, and one launcher trap worth knowing
Two harnesses, both against the worktree build:
- PTY harness —
@lydell/node-ptyspawns the TUI at a fixed 120×40, injects the prompt, and records every raw byte the TUI writes. This is what makes the Kitty-protocol claims below checkable rather than visual. - Screenshot harness — a dedicated fullscreen Ghostty per case, located by
CGWindowListCopyWindowInfotitle match and captured withscreencapture -l <window-id>.
scripts/cli-entry.js prefers a managed npm installation under ~/.qwen/updates/npm over the local dist/. On a machine with a released build installed, node scripts/cli-entry.js silently runs that build, not your worktree — display_image is simply absent from /tools and the model reports the tool does not exist. Invoke node packages/cli (what npm start does) instead. My first round of runs was invalid for exactly this reason.
Fixtures generated with a pure-Node PNG encoder: 32×32, 64×48, 1200×400, 800×800, 400×1200, a 5.50 MiB valid PNG, a 9.6 MiB oversized PNG, a non-PNG file named .png, and a 16-byte truncated PNG.
1. Build, typecheck, lint, tests
| Check | Result |
|---|---|
npm install && npm run build |
pass |
npm run typecheck |
pass |
npm run lint |
pass (exit 0) |
cli — terminal-image-renderer, TerminalImage, ToolMessage |
92 passed / 3 files |
core — display-image, tools, agent, fork-subagent, background-agent-resume |
332 passed / 5 files |
core — config, coreToolScheduler |
pass |
2. Native Kitty path in Ghostty — verified at the byte level
The model called display_image and Ghostty placed the PNG inline.
Decoding the recorded APC stream, for each fixture the transmitted payload is byte-identical to the source file and the placement geometry is exact:
| fixture | control keys | cells (c×r) | payload | identical to source |
|---|---|---|---|---|
small-32x32.png |
a=T f=100 U=1 |
4×2 | 267 B, PNG 32×32 | ✅ |
square-800x800.png |
a=T f=100 U=1 |
48×24 | 8199 B, PNG 800×800 | ✅ |
landscape-1200x400.png |
a=T f=100 U=1 |
72×12 | 4749 B, PNG 1200×400 | ✅ |
portrait-400x1200.png |
a=T f=100 U=1 |
16×24 | 7910 B, PNG 400×1200 | ✅ |
Every row preserves the source ratio (1:1 → 48×24, 3:1 → 72×12, 1:3 → 16×24), stays within the 72×24 cap, and the 32×32 image stays 4×2 in a 120-column terminal — no upscaling. Placeholder rows drawn match the declared r, and every row has exactly c placeholder cells.
3. Warp gate — placeholders bypassed, and colour is now correct
With TERM_PROGRAM=WarpTerminal, the recorded stream contains 0 Kitty APC sequences and 0 U+10EEEE placeholder cells — the glyph-spew this PR set out to fix. Instead: 28,890 block glyphs carried by 24,454 256-colour SGR runs.
That last number is the point: the previous round reported the fallback preview had no colour at all. The --colors=256 fix in 7ede190f works.
4. No renderer — explicit failure, no false success
TERM_PROGRAM=WarpTerminal with chafa off PATH. The tool row is red, and the model states plainly that nothing was shown:
x DisplayImage {"file_path":".../square-800x800.png"}
The image was not displayed: No compatible native image protocol was detected, and chafa is not installed.
5. Validation — all rejected, zero bytes emitted
Each case emitted 0 Kitty APC sequences and 0 placeholder cells, so no file bytes and no untrusted terminal data escaped:
| input | error |
|---|---|
non-PNG named .png |
Only PNG images are supported by display_image: … |
| 16-byte truncated PNG (valid signature) | Only PNG images are supported by display_image: … |
| path outside the workspace | Image path is outside the current workspace: … |
| 9.6 MiB PNG | Image exceeds the 8388608 byte display limit: … |
6. Registration gating
| context | display_image registered |
|---|---|
| interactive TUI | ✅ present in /tools as DisplayImage |
headless (-p) |
❌ absent — model replied TOOL_NOT_REGISTERED |
| subagent | ❌ absent — subagent reported "display_image is a main-agent-only tool… neither in their tool list nor fetchable as a deferred tool", and 0 graphics bytes were emitted |
Finding (non-blocking): the full PNG is re-transmitted on every re-render
Not a correctness bug, and I don't think it should hold the merge — but it is worth a follow-up.
TerminalImage guards repeat writes with a writtenSequence ref, which only lives as long as one mount — so the guard holds within a mount but not across remounts, and the observed counts line up with the component being remounted (live row → Static row, then once per resize). Measured on the recorded stream:
| scenario | transmissions of the same image | stream written |
|---|---|---|
| 5.50 MiB PNG, no resize | 2 | 16.9 MB |
| same, plus 3 terminal resizes | 5 (2 + one per resize) | 40.7 MB |
Every transmission carries the same image id (8746658), the same 64×24 shape, and byte-identical data — so the render cache is doing its job (no re-encode), but the bytes still go out to the TTY each time. Kitty already holds that id; the resends are pure waste.
At the 8 MiB cap this is roughly 11 MB of base64 per redraw. On a slow PTY, or a session with several images scrolled back, resizing the window would push a lot of data.
A shape-and-id-keyed "already transmitted this terminal session" set — outliving the component mount, next to the existing renderCache — would collapse all of these to one transmission and let the placeholder cells alone drive redraws.
中文版本
在 8f8b984 上的本地复验 —— 真实 Ghostty、真实模型、字节级校验
这是对我上一轮验证的跟进。上一轮唯一的问题（chafa 回退无颜色）已修复并确认生效。本轮针对当前 head 重跑了 reviewer 测试计划，并新增了对 TUI 实际写入终端字节流的解码校验。
结论：测试计划中的每一项都可复现。 发现一个不阻塞合并的效率问题，列在最后。
环境
| PR head | 8f8b984f0（落后 main 13 个提交，可干净合并，mergeable: MERGEABLE） |
| 系统 / 架构 | macOS 26.6（Darwin 25.6.0），arm64 |
| Node | v24.18.1 |
| 终端 | Ghostty —— 每个用例一个独立全屏实例，按 CGWindow id 精确截图 |
| 回退渲染器 | chafa 1.18.2（Homebrew） |
| 模型 | qwen3.8-max-preview，真实 API，--approval-mode yolo |
验证方法 —— 以及一个值得注意的 launcher 陷阱
两套 harness，均针对 worktree 构建：
- PTY harness —— 用
@lydell/node-pty以固定 120×40 拉起 TUI，注入 prompt，并记录 TUI 写出的每一个原始字节。下文关于 Kitty 协议的结论因此是可核查的，而非仅凭肉眼。 - 截图 harness —— 每个用例一个独立全屏 Ghostty，通过
CGWindowListCopyWindowInfo按标题定位窗口，再用screencapture -l <window-id>精确捕获。
scripts/cli-entry.js 会优先加载 ~/.qwen/updates/npm 下的 managed npm 安装，而不是本地 dist/。在装有已发布版本的机器上，node scripts/cli-entry.js 实际运行的是那个构建而非你的 worktree —— 此时 /tools 里根本没有 display_image，模型也会说该工具不存在。应改用 node packages/cli（即 npm start 的做法）。我第一轮的运行结果正是因此作废。
测试素材用纯 Node 的 PNG 编码器生成：32×32、64×48、1200×400、800×800、400×1200、一个 5.50 MiB 的合法 PNG、一个 9.6 MiB 超限 PNG、一个后缀为 .png 的非 PNG 文件，以及一个 16 字节的截断 PNG。
1. 构建、类型检查、Lint、测试
| 检查项 | 结果 |
|---|---|
npm install && npm run build |
通过 |
npm run typecheck |
通过 |
npm run lint |
通过（exit 0） |
cli —— terminal-image-renderer、TerminalImage、ToolMessage |
92 通过 / 3 个文件 |
core —— display-image、tools、agent、fork-subagent、background-agent-resume |
332 通过 / 5 个文件 |
core —— config、coreToolScheduler |
通过 |
2. Ghostty 原生 Kitty 路径 —— 字节级验证
模型调用 display_image 后，Ghostty 内联放置了该 PNG。
解码记录下来的 APC 流后，每个素材的传输载荷都与源文件逐字节一致，放置几何也完全准确：
| 素材 | 控制键 | 单元格 (c×r) | 载荷 | 与源文件一致 |
|---|---|---|---|---|
small-32x32.png |
a=T f=100 U=1 |
4×2 | 267 B，PNG 32×32 | ✅ |
square-800x800.png |
a=T f=100 U=1 |
48×24 | 8199 B，PNG 800×800 | ✅ |
landscape-1200x400.png |
a=T f=100 U=1 |
72×12 | 4749 B，PNG 1200×400 | ✅ |
portrait-400x1200.png |
a=T f=100 U=1 |
16×24 | 7910 B，PNG 400×1200 | ✅ |
每一行都保持了源图比例（1:1 → 48×24，3:1 → 72×12，1:3 → 16×24），均未超过 72×24 上限；32×32 的小图在 120 列终端里仍是 4×2 —— 没有被放大。绘制的 placeholder 行数与声明的 r 一致，且每行恰好有 c 个 placeholder 单元格。
3. Warp gate —— placeholder 已绕开，且颜色现已正确
设置 TERM_PROGRAM=WarpTerminal 后，记录到的字节流中包含 0 个 Kitty APC 序列、0 个 U+10EEEE placeholder 单元格 —— 正是本 PR 要解决的乱码问题。取而代之的是 28,890 个块状字形，由 24,454 段 256 色 SGR 承载。
最后这个数字是重点：上一轮测得的是 0 段颜色。7ede190f 中的 --colors=256 修复确实生效了。
4. 无可用渲染器 —— 明确失败，不会假装成功
TERM_PROGRAM=WarpTerminal 且 chafa 不在 PATH 中。工具行标红，模型也明确说明图片并未展示：
x DisplayImage {"file_path":".../square-800x800.png"}
The image was not displayed: No compatible native image protocol was detected, and chafa is not installed.
5. 校验 —— 全部拒绝，且未输出任何字节
以下每个用例都产生了 0 个 Kitty APC 序列、0 个 placeholder 单元格，因此既没有文件字节、也没有不可信终端数据泄漏：
| 输入 | 错误 |
|---|---|
后缀为 .png 的非 PNG |
Only PNG images are supported by display_image: … |
| 16 字节截断 PNG（签名合法） | Only PNG images are supported by display_image: … |
| 工作区外的路径 | Image path is outside the current workspace: … |
| 9.6 MiB PNG | Image exceeds the 8388608 byte display limit: … |
6. 注册门控
| 场景 | 是否注册 display_image |
|---|---|
| 交互式 TUI | ✅ /tools 中存在，显示为 DisplayImage |
headless（-p） |
❌ 不存在 —— 模型回复 TOOL_NOT_REGISTERED |
| subagent | ❌ 不存在 —— subagent 明确回报 "display_image is a main-agent-only tool… neither in their tool list nor fetchable as a deferred tool"，且未产生任何图形字节 |
发现（不阻塞合并）：每次重绘都会重传整张 PNG
这不是正确性缺陷，我也不认为它应该拦住合并，但值得后续跟进。
TerminalImage 用 writtenSequence ref 防止重复写入，但该 ref 的生命周期仅限于一次 mount —— 因此该保护在单次 mount 内有效，跨 remount 则失效；实测次数也与组件被 remount 的次数吻合（实时行 → Static 行，之后每次 resize 各一次）。基于记录字节流的实测：
| 场景 | 同一张图的传输次数 | 写出的字节流 |
|---|---|---|
| 5.50 MiB PNG，不 resize | 2 | 16.9 MB |
| 同上，外加 3 次终端 resize | 5（2 次 + 每次 resize 各 1 次） | 40.7 MB |
每一次传输携带的都是相同的 image id（8746658）、相同的 64×24 形状、逐字节相同的数据 —— 说明 render cache 是生效的（没有重新编码），但这些字节仍然被再次送往 TTY。Kitty 已经持有该 id，重发纯属浪费。
按 8 MiB 上限计算，每次重绘大约是 11 MB 的 base64。在较慢的 PTY 上，或者会话中有多张图片滚动在上方时，调整窗口大小会推送相当可观的数据量。
一个以 id 与形状为键、生命周期超出组件 mount 的"本终端会话已传输过"集合（放在现有 renderCache 旁边）就能把这些收敛为一次传输，让 placeholder 单元格独自驱动重绘。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Unresolved, please confirm: [Critical] Synchronous chafa render inside React render (packages/cli/src/ui/utils/terminal-image-renderer.ts + packages/cli/src/ui/components/TerminalImage.tsx, comment 3691124431): a residual first-render and cold-cache session-restore synchronous execFileSync (up to 8s Ink event-loop block) remains in the code. The maintainer-endorsed bounded LRU render cache addresses the resize amplifier (a within-clamp resize reuses the cache key and does not re-spawn), and the maintainer's own re-verification at 8f8b984 — whose source is unchanged at this head — is positive with only a non-blocking efficiency finding; but this is an escalated maintainer design call (async-vs-sync timing rework deemed out of scope), so it cannot be independently certified as resolved here. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未决，请确认：[Critical] Synchronous chafa render inside React render (packages/cli/src/ui/utils/terminal-image-renderer.ts + packages/cli/src/ui/components/TerminalImage.tsx, comment 3691124431): a residual first-render and cold-cache session-restore synchronous execFileSync (up to 8s Ink event-loop block) remains in the code. The maintainer-endorsed bounded LRU render cache addresses the resize amplifier (a within-clamp resize reuses the cache key and does not re-spawn), and the maintainer's own re-verification at 8f8b984 — whose source is unchanged at this head — is positive with only a non-blocking efficiency finding; but this is an escalated maintainer design call (async-vs-sync timing rework deemed out of scope), so it cannot be independently certified as resolved here. 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
Re-verified at
|
Check at e4921a0 |
Result |
|---|---|
npm install && npm run build && npm run typecheck |
pass |
core — display-image, tools, fork-subagent, config |
523 passed |
cli — terminal-image-renderer, TerminalImage, ToolMessage |
92 passed |
Ghostty native, square-800x800.png |
48×24 cells, payload byte-identical to source ✅ |
Warp gate (TERM_PROGRAM=WarpTerminal) |
0 Kitty APC, 0 U+10EEEE placeholders, 27,508 256-colour SGR runs ✅ |
Everything from the previous report still holds — including the screenshots, which show unchanged behaviour.
The re-transmission finding is unchanged and still open: the 5.50 MiB PNG is transmitted 2× at rest and 5× after three terminal resizes, same image id 8746658, same 64×24 shape, byte-identical payload each time. Still non-blocking from my side.
One housekeeping note: the branch is now 22 commits behind main (was 13). Still MERGEABLE.
中文版本
在 e4921a0 上的复验 —— 本次更新不改变行为，此前结论继续成立
针对新增的两个 commit（00cf11d 在 Warp 中直接渲染图片 → e4921a0 禁用 Warp 原生图片）重新检查。两者互相抵消：相对我此前验证的 head（8f8b984），净差异不涉及任何产品代码。
docs/design/tui-display-image-tool.md | 5 +++--
patches/ink+7.0.3.patch | 11 +++++------
git diff 8f8b984..e4921a0 -- packages/为空。patches/ink+7.0.3.patch的改动是重新生成该文件所致：hunk 头、index哈希，以及一个行尾换行标记。只比较+/-的实质内容行，两个版本逐字节一致（各 461 行） —— 打完补丁后的 Ink 产物没有变化。这一点值得明确说明，因为该补丁覆盖output.js与render-node-to-output.js，正是我此前那个发现所处的 Static/渲染路径。
即便如此，我仍然重新构建并实跑，而不是仅凭 diff 推断：
在 e4921a0 上的检查 |
结果 |
|---|---|
npm install && npm run build && npm run typecheck |
通过 |
core —— display-image、tools、fork-subagent、config |
523 通过 |
cli —— terminal-image-renderer、TerminalImage、ToolMessage |
92 通过 |
Ghostty 原生路径，square-800x800.png |
48×24 单元格，载荷与源文件逐字节一致 ✅ |
Warp gate（TERM_PROGRAM=WarpTerminal） |
0 个 Kitty APC、0 个 U+10EEEE placeholder、27,508 段 256 色 SGR ✅ |
上一份报告中的全部结论继续成立，截图同样有效（行为未变）。
重传问题原样存在，仍未处理：5.50 MiB 的 PNG 静态下传输 2 次，经过 3 次终端 resize 后为 5 次，每次都是相同的 image id 8746658、相同的 64×24 形状、逐字节相同的载荷。在我这边仍属不阻塞合并的问题。
另有一项事务性提醒：该分支目前落后 main 22 个提交（此前为 13）。仍为 MERGEABLE。
Re-verified at
|
Check at 913923d |
Result |
|---|---|
terminal-image-renderer.test.ts |
21 passed (was 20) |
Mutation: containsCmdShellMetacharacters → return false |
2 tests fail, including the new one — guard removal is caught |
| Source restored, re-run | 21 passed, tree clean |
The new test drives the full renderTerminalImage path under a mocked win32 platform with a real .CMD shim on PATH and a & in the filename, and asserts the render is refused before the spawn. That's the right level.
Nothing else changed, so the verification at 8f8b984 and the e4921a0 re-check both still stand — including the still-open re-transmission finding (5.50 MiB PNG sent 2× at rest, 5× after three resizes, identical id and payload each time). Non-blocking from my side.
Branch is now 23 commits behind main; still MERGEABLE.
中文版本
在 913923d 上的复验 —— 纯测试改动，且新测试并非空测试
新增一个 commit 913923d test(cli): cover chafa shell path guard：在 terminal-image-renderer.test.ts 中 +43 行，未触碰任何产品代码（git diff e4921a0..913923d -- packages/core packages/cli/src/ui/utils/terminal-image-renderer.ts 为空）。
这个补充很到位——它正好补上了我本会提出的那个缺口。containsCmdShellMetacharacters 防的是一个真实的命令注入面（以 shell: true 拉起 .cmd/.bat 形式的 chafa shim 时，Node 会不加引号地转发参数），而此前只覆盖了这个判定函数本身，没有覆盖经由 renderTerminalImage 的完整路径。
我确认了该测试确有价值，而不只是"能通过"：
在 913923d 上的检查 |
结果 |
|---|---|
terminal-image-renderer.test.ts |
21 通过（此前 20） |
变异测试：把 containsCmdShellMetacharacters 改为 return false |
2 个测试失败，含新增的这个——移除防护会被捕获 |
| 还原源码后重跑 | 21 通过，工作区干净 |
新测试在 mock 成 win32 的环境下走完整的 renderTerminalImage 路径：PATH 上放一个真实的 .CMD shim，文件名中带 &，断言在 spawn 之前就拒绝渲染。这个覆盖层次是对的。
其余没有任何变化，因此在 8f8b984 上的验证与在 e4921a0 上的复检均继续成立——包括那个仍未处理的重传问题（5.50 MiB 的 PNG 静态下传 2 次，3 次 resize 后 5 次，每次 id 与载荷完全相同）。在我这边仍属不阻塞合并。
分支目前落后 main 23 个提交；仍为 MERGEABLE。
…Zhang/qwen-code into dragon/tui-display-image
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedFinding (non-blocking): the full PNG is re-transmitted on every re-render — Implemented@wenshao's byte-level re-verification at Although flagged as non-blocking and "worth a follow-up", this is squarely in scope for this image-display PR and mirrors the existing bounded Changes
Tests
No conflict resolution was required ( Verification
The touched behavior (Kitty transmission dedup across remounts) is exercised directly by the unit tests above rather than only through the bundled CLI, so a focused integration run was not required. No settings source changed, so 中文说明已处理的评审反馈发现（不阻塞合并）：每次重绘都会重传整张 PNG —— 已实现@wenshao 在 尽管该问题被标注为不阻塞合并、「值得后续跟进」，但它完全属于本图片展示 PR 的范围，且与现有的有界 改动
测试
无需解决冲突（ 验证
被改动的行为（跨 remount 的 Kitty 传输去重）由上述单元测试直接覆盖，而非只能通过打包后的 CLI 验证，因此不需要单独的集成测试。未改动任何 settings 源文件，故无需运行 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 /trigage |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 69 passed · 0 failed · 69 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：69 通过 · 0 失败 · 69 总计 Verification reportPR #8217 —
|
| input / context | head result (oracle) |
|---|---|
| valid 1×1 PNG | structured terminal_image display; no base64 bytes in result; llmContent = "does not provide the image contents" |
| relative path | throws must be absolute |
| path outside workspace | throws outside the current workspace |
| missing file | FILE_NOT_FOUND |
| directory | TARGET_IS_DIRECTORY |
| FIFO (not regular file) | TARGET_NOT_REGULAR_FILE |
GIF bytes in .png |
INVALID_TOOL_PARAMS |
| size = 8 MiB + 1 | FILE_TOO_LARGE |
| 8-byte signature-only (truncated) | INVALID_TOOL_PARAMS |
executed inside runInForkContext |
EXECUTION_DENIED ("main agent") |
valid PNG, renderSupport={available:false} |
EXECUTION_FAILED; llmContent + returnDisplay = "The image was not displayed"; returnDisplay is not a display object |
Table 1b — single-guard mutants (behavior flips ⇒ guard load-bearing):
| mutant (guard disabled) | head | mutant | load-bearing? |
|---|---|---|---|
truncation (bytesRead !== 24) |
rejects 8-byte file | accepts it | ✅ (valid PNG still accepted → mutation-specific) |
| 8 MiB size cap | FILE_TOO_LARGE |
no longer FILE_TOO_LARGE |
✅ |
| fork-execution ban | EXECUTION_DENIED |
no longer denied | ✅ |
| explicit renderer-failure | EXECUTION_FAILED |
claims success | ✅ |
Every gate is pinned: 15 head assertions + 5 mutant-flip assertions = 20/20 pass.
Central claim 2 — chafa executable-lookup hardening (arbitrary-code-execution A/B)
The highest-value probe. A hostile repo can drop node_modules/.bin/chafa; if the renderer resolves chafa off an unfiltered PATH, that binary runs with the user's privileges. Harness harness-renderer.mjs builds a workspace whose node_modules/.bin/chafa is a malicious script that writes a PWNED marker, plus a benign chafa on a normal PATH dir, then calls the real renderTerminalImage end-to-end (real execFileSync child processes; the fake binaries are #!/usr/bin/env node scripts and node's bin dir is appended to the passed PATH only so env resolves the interpreter — node's dir holds no chafa, so it cannot affect lookup). The oracle is whether the PWNED marker file appears plus the resolved path from findExecutable. Raw log: logs/renderer.result.json. Capture: 02-renderer-security-ab.png.
Table 2 — chafa hardening A/B:
| cell | findExecutable('chafa') resolves to |
end-to-end render spawns | PWNED marker |
|---|---|---|---|
| HEAD (hardened) | benign PATH chafa |
benign chafa (record file confirms argv + cwd) | absent ✅ |
| MUTANT (skip-block disabled) | malicious ws/node_modules/.bin/chafa |
malicious chafa | created ✅ |
HEAD + QWEN_CODE_MERMAID_ALLOW_LOCAL_RENDERERS=1 |
malicious local chafa (opt-in escape hatch) | — | — |
The mutant's PWNED marker is the positive control that makes the head's "no marker" meaningful: the harness can make a render spawn the malicious binary, and the head build is exactly what stops it. The opt-in env var still restores local renderers, so the hardening is a deliberate default-deny, not a blanket break. 9/9 pass in this section.
Secondary claim — sizing wire-oracle
The renderer passes --size=WxH to chafa; the benign fake records its argv, so the harness reads back the actual dimensions the renderer chose (not a hand-computed expectation). Cells (cell = 8×16 px):
Table 3 — sizing (read back from chafa --size):
| image (px) | contentWidth / availHeight | got WxH |
property |
|---|---|---|---|
| 1600×1600 | 100 / 40 | 48×24 | row-limited, aspect kept |
| 64×64 (small) | 100 / 40 | 8×4 | not upscaled to content width |
| 1920×1080 | 100 / 40 | 72×20 | width-limited |
| 800×1600 (portrait) | 100 / 40 | 24×24 | 1:2 aspect via 8×16 cells |
| 1600×1600 | 30 / 40 | 30×15 | respects narrow terminal |
| 10000×10000 | 200 / 100 | 47×23 | within 72×24 cap, aspect ~2:1 |
6/6 pass. (The last row's exact value is not 72×24 because a square image is 2:1 in 8×16 cells and is row-limited; the asserted property is "within cap and aspect-preserving", which holds — 47/23 ≈ 2.04.)
Renderer selection (env logic) + metachar predicate
supportsKittyImageProtocol / getTerminalImageRenderSupport over synthetic env tuples: Kitty (KITTY_WINDOW_ID/TERM=kitty)/Ghostty → native; Warp, tmux, SSH_TTY, SSH_CLIENT, non-TTY → excluded; plain xterm → not kitty; no-kitty + chafa-on-PATH → available; no-kitty + no-chafa → unavailable with the explicit "chafa is not installed" reason. containsCmdShellMetacharacters flags all 11 cmd.exe metacharacters (& | < > ^ % " ! ( ) \n \r) and does not flag clean unix/windows/space paths; shouldRunThroughShell(.cmd) is false on Linux (the Windows shell-guard is dormant here, so the metachar gate is correctly inert on this platform). 28/28 pass across these two sections.
Findings
F1 (medium, non-blocking) — failing chafa's stderr leaks unbounded into the terminal scrollback
What the PR claims vs what it does. Commit 10337397 ("Bound a failing chafa's stderr … before rendering it into permanent scrollback") bounds only the returned reason string via firstLineBounded(err.stderr) (capped to 200 chars + …, the value Ink renders). It does not stop the raw stderr from reaching the terminal: renderWithChafa calls execFileSync(chafaPath, args, { encoding, env, shell, maxBuffer, timeout }) with no stdio option, and on Node 22 that default inherits the child's stderr to the parent's fd2 in addition to capturing it into err.stderr. (maxBuffer caps stdout only; it does not touch the inherited stderr.) So when chafa fails while rendering a user image, its full stderr — unbounded and un-sanitized — streams into the user's scrollback.
Why this is PR-introduced, not pre-existing. The sibling it was modelled on, renderPngWithChafa in mermaidImageRenderer.ts, uses spawnSync with the same default options — and spawnSync's default does not leak (captured only). I probed both directly on this image's Node (v22.23.2):
| call | child stderr on parent fd2 | captured into result/err |
|---|---|---|
spawnSync default (mermaid path) |
none | yes |
execFileSync default (display_image path) |
leaked | yes |
execFileSync + stdio:['pipe','pipe','pipe'] |
none | yes |
The display_image chafa path is entirely new in this PR, so the leak is introduced by its choice of execFileSync without stdio.
Measured A/B (harness harness-leak.mjs + harness-leak-table.mjs; a failing chafa writes 5000 E bytes to stderr; we measure what reaches the renderer's parent fd2). Capture: 03-stderr-leak-ab-head-vs-fixed.png.
Table 4 — stderr leak A/B:
| build | returned reason len | leaked bytes to terminal fd2 |
|---|---|---|
HEAD (execFileSync, no stdio) |
201 | 5000 ← unbounded, hits scrollback |
+ stdio: ['pipe','pipe','pipe'] |
201 | 0 ← bounded; reason unchanged |
The trailing block of E… lines visible at the bottom of 02-renderer-security-ab.png is this same leak from the renderer's own failing-chafa cell (the capture tool feeds the harness no TTY, so the inherited stderr lands after the captured grid).
Severity / blast radius. Not a security issue: the hardened findExecutable (proven in Table 2) means the chafa that emits the stderr is a trusted/system binary or one the user opted into — an attacker cannot place it. So the content is chafa's own diagnostics, not attacker text: no injection. The harm is robustness/UX: a misbehaving or version-mismatched chafa, or a PNG chafa rejects, dumps an unbounded, un-sanitized stderr line into permanent scrollback (and on a TTY, mid-render), exactly the "permanent scrollback" surface the commit message said it bounded. The interactive TUI does not redirect process.stderr away from the terminal, so the leak reaches the user's screen.
Reproduce:
D=tmp/pr8217-verify-20260801-112351
CLI=packages/cli/dist/src/ui/utils
node "$D/make-mutants.mjs" # regenerates the fixed build from the current dist
node "$D/harness-leak.mjs" "/__w/qwen-code/qwen-code/$CLI/terminal-image-renderer.js" 2>/tmp/leak-head.txt
node "$D/harness-leak.mjs" "/__w/qwen-code/qwen-code/$CLI/terminal-image-renderer-fixed.mjs" 2>/tmp/leak-fixed.txt
echo "head=$(wc -c </tmp/leak-head.txt) fixed=$(wc -c </tmp/leak-fixed.txt)" # head=5000 fixed=0Minimal suggested fix (measured, preserves intent)
In packages/cli/src/ui/utils/terminal-image-renderer.ts, renderWithChafa's execFileSync options, add a stdio that captures stderr instead of inheriting it:
{
encoding: 'utf8',
env: createRendererChildEnv(env),
shell: useShell,
stdio: ['pipe', 'pipe', 'pipe'], // <- capture stderr; stops the fd2 leak
maxBuffer: CHAFA_MAX_OUTPUT_BYTES,
timeout: CHAFA_TIMEOUT_MS,
}Measured in a scratch copy of the built module (terminal-image-renderer-fixed.mjs, derived from the current build by make-mutants.mjs, differing by this one option): leak 5000 → 0 bytes on fd2; returned reason unchanged at 201 chars (still bounded by firstLineBounded); render result kind unchanged (unavailable on the failing chafa). The 46-assertion renderer suite's behaviour on the success path is unaffected because a succeeding chafa writes nothing to stderr. (Not folded into the working tree — reported only.) The same stdio should be considered for the sibling spawnSync calls in mermaidImageRenderer.ts for consistency, though they do not currently leak.
Corrections
- Commit
10337397overstates its own effect. Its message says it bounds a failing chafa's stderr "before rendering it into permanent scrollback." Measured (Table 4), it bounds only the returned reason string; the raw stderr still streams unbounded to the terminal viaexecFileSync's default stderr inheritance. This is a correction to the commit's description, surfaced as finding F1 with a measured fix — not a request to change anything else in the PR.
Not covered
- Real-terminal pixel rendering — Kitty/Ghostty native virtual-image placement, Warp/iTerm2 direct placement, tmux/SSH passthrough, and the actual on-screen appearance of the chafa ANSI output were not exercised: this container has no graphical terminal and no
chafainstalled (confirmed:command -v chafa→ not found;process.stdout.isTTYtrue but no emulator). These are exactly the surfaces the PR lists as out-of-scope / manually-observed-on-macOS. I covered their branch selection (env logic, Table for selection) and the chafa fallback via a faithful fake-chafa wire-oracle (Tables 2–4), which proves the code paths and the data handed to chafa, not the rendered pixels. Distinguishing shape from cause: the harness reproduces the wire shape the renderer emits (the--size, the spawn argv, the env allowlist), not a real terminal's interpretation of it. - Per-commit attribution — the checkout is shallow (depth 2):
git rev-list HEAD^1..HEAD^2returns1locally while the metadata snapshot lists 22 commits, the shallow-boundary trap. Per-commit claims (e.g. which commit added the metachar guard vs the cache) were not individually exercised; only the aggregateHEAD^1..HEADdiff was verified. - Windows shell-injection path end-to-end —
shouldRunThroughShellis true only onwin32, so thecontainsCmdShellMetacharactersgate is dormant on this Linux image; I verified the predicate (all 11 metachars flagged, clean paths not) but could not exercise the actualshell:true+.cmd-shim spawn that the guard protects. The predicate coverage is the part that transfers cross-platform. - The PR's own test suite as vacuity evidence — reported separately in
logs/vacuity.txt(a source-level mutation, not rerunnable from the artifact dir without touching the working tree), so it is method evidence, not counted inassertions.json.
Targeted gates (affected workspaces only)
All green on the head build:
| suite | result |
|---|---|
packages/core display-image + fork-subagent |
26 passed |
packages/core agent + tools + config + coreToolScheduler + background-agent-resume |
1117 passed |
packages/cli terminal-image-renderer + TerminalImage + ToolMessage |
96 passed |
The config suite includes registers display_image only for the main interactive TUI (covers interactive / sdkMode / screenReader / forSubAgent gating) and forwards terminal image renderer support to the display tool — both passed, corroborating the registration gate independently of my harness. The Ink patch (patches/ink+7.0.3.patch) change is a cosmetic patch-package 8.x regeneration (hunk-header context suffixes + blob hashes; functional hunks unchanged): npx patch-package reports ink@7.0.3 ✔ and the patched selectionBreakAfter prop is present in node_modules/ink/build/components/Text.js.
Methodology
Environment: node:22-bookworm verify container, node v22.23.2, process.platform=linux, no chafa, no graphical terminal; working tree = refs/pull/8217/merge at depth 2 (HEAD merge, HEAD^1 base tip bd855991, HEAD^2 PR head 3e568af8); npm ci + npm run build pre-run at head, reused as-is. Each harness imports the compiled dist/ output (real code, no stubs of the unit under test) and drives it with real child processes (fake chafa binaries) over real execFileSync/spawnSync. A/B controls are same-directory copies of a dist module with exactly one guard disabled (make-mutants.mjs), so relative imports resolve identically and the control differs from head by nothing but the guard; the chafa-hardening A/B additionally asserts the realpath/oracle (the PWNED marker) so the head's clean result is not a false negative. Internal-workspace-link confound: harnesses import dist/ directly by absolute path (not via node_modules/@qwen-code/* symlinks), so the base/head symlink trap does not apply. Evidence captures via scripts/verify-capture.mjs (ANSI→PNG, no browser). Raw per-assertion logs in logs/; scratch mutant/fix dist copies were removed after capture (tree clean). Assertion counts in assertions.json (69/0/69) map one-to-one to scripted checks that executed; the source-level vacuity mutation and the targeted-gate runs are method evidence reported above, not in assertions.json. No network calls, no GitHub writes.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
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 — CI landed green after the review. ✅
|
Released in v0.21.3. |
4 participants
What this PR does
Adds a model-invocable
display_imagetool exclusively to the main interactive TUI. The tool validates an absolute workspace PNG path, regular-file status, the PNG signature, and an 8 MiB limit, then persists only a structured path and MIME type rather than image bytes or terminal escape sequences.Direct Kitty and Ghostty sessions use native virtual-image placement with Unicode placeholders. Warp, iTerm2, tmux/SSH sessions, and terminals without a compatible native path use
chafaANSI symbol output when it is installed. If no renderer is available, the tool reports failure to both the TUI and model instead of claiming that the image was displayed. Warp is deliberately excluded from direct Kitty placement because its image layer drifts away from Ink content during scrollback and terminal reflow.Previews preserve aspect ratio, stay within the terminal, default to at most 72 columns by 24 rows, and use a conservative natural-size estimate so small images are not deliberately enlarged. The tool is not registered in headless, SDK, screen-reader, bare, or subagent contexts.
Why it's needed
Repositories frequently contain image assets that users need to inspect while coding. Without a terminal preview tool, users must leave the TUI, and a model can incorrectly assume that an attempted image display succeeded even when the terminal could not render it. This change keeps image inspection inside the interactive workflow while making renderer success and failure explicit.
Reviewer Test Plan
How to verify
chafainstalled and display a colorful PNG. Confirm that ANSI symbol output retains color, occupies normal terminal rows, and remains aligned with the transcript after scrolling and resizing.chafaabsent fromPATH. Confirm that the tool reports an explicit “image was not displayed” failure and the model does not claim success.display_imageis not registered.Evidence (Before & After)
chafareceive an explicit execution failure.chafaANSI output when available, otherwise it fails explicitly.Tested on
Environment (optional)
macOS arm64 with Node.js 24.18.0. Manual development testing used direct Kitty, Ghostty, cmux, and Warp sessions. The focused renderer suite passes 50 tests; a clean dependency install applies the Ink patch successfully, and the full repository build and typecheck pass.
Risk & Scope
chafa.Linked Issues
Closes #8216
中文说明
本 PR 做了什么
仅在主交互式 TUI 中新增模型可调用的
display_image工具。该工具会校验工作区内的绝对 PNG 路径、普通文件状态、PNG 文件签名以及 8 MiB 大小限制；持久化内容仅包含结构化路径和 MIME 类型，不包含图片字节或终端转义序列。直接运行的 Kitty 和 Ghostty 使用带 Unicode placeholder 的原生虚拟图片放置。Warp、iTerm2、tmux/SSH 会话以及没有兼容原生路径的终端，在安装了
chafa时使用 ANSI 字符输出；没有任何渲染器时，工具会向 TUI 和模型明确报告失败，而不是声称图片已经展示。Warp 被明确排除在 Kitty 直接放置之外，因为它的图片层会在 scrollback 和终端 reflow 时与 Ink 内容发生偏移。预览会保持宽高比、受终端空间约束，默认最大为 72 列 × 24 行，并使用保守的自然尺寸估算，避免主动放大小图。该工具不会注册到 headless、SDK、屏幕阅读器、bare 或 subagent 场景。
为什么需要
代码仓库经常包含开发者需要在编码过程中检查的图片资源。没有终端预览工具时，用户必须离开 TUI；而当终端实际无法渲染时，模型还可能误以为展示成功。本改动让图片检查留在交互流程内，并明确区分渲染成功与失败。
Reviewer 测试计划
如何验证
chafa的 Warp 中展示彩色 PNG，确认 ANSI 字符输出保留颜色、占用普通终端行，并且滚动或调整窗口大小后仍与对话内容对齐。PATH中没有chafa的 Warp 中执行展示，确认工具明确报告“图片未展示”，且模型不会声称成功。display_image。证据（改动前后）
chafa的终端会收到明确失败。chafa时使用普通终端行的 ANSI 输出，否则明确失败。测试环境
macOS arm64，Node.js 24.18.0。开发期间在 Kitty、Ghostty、cmux 和 Warp 会话中进行了人工测试。聚焦渲染器测试共 50 项通过；干净依赖安装可成功应用 Ink 补丁，完整仓库 build 和 typecheck 均通过。
风险与范围
chafa。关联 Issue
Closes #8216