feat(cli): support native video input in /learn - #7497
Conversation
E2E test reportFull paper analysis, implementation notes, test procedure, and evidence boundary: Qwen Know-How — Resource2Skill paper analysis and Qwen Code Environment and fixture
Transport and learning results
The final learned Skill used the loadable slug Fresh-session applicationA new session in the same project explicitly recorded: It then created a single Browser behavior checksPlaywright measured the real DOM and pseudo-element computed styles:
The final tutorial-fidelity verdict is nevertheless FAIL:
Code verification
ConclusionNative video routing, the two-file provenance contract, loadable Skill naming, fresh-session invocation, and generation of a working pure-CSS artifact are verified. Faithful reproduction of the official tutorial is not verified and failed this case. The next layer should be deterministic schema/provenance validation plus domain-specific structural and render acceptance gates, rather than treating a successful model response as a verified Skill. |
|
Thanks for the PR! Template looks good ✓ Problem: this is a feature addition, not a bug fix — Direction: aligned. Extending Size: core paths touched ( Approach: the scope feels right. The PR adds video parsing, capability gating (model modality + provider path), a video-specific distillation prompt with provenance contract, YouTube page rejection, and a Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个功能新增，不是 bug 修复—— 方向：对齐。扩展 规模：触及核心路径（ 方案：范围合理。PR 添加了视频解析、能力门控（模型模态 + provider 路径）、带 provenance contract 的视频专用蒸馏 prompt、YouTube 页面拒绝，以及 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
gwinthis
left a comment
gwinthis
left a comment
There was a problem hiding this comment.
本地验证报告 — PR #7497
论点：此 PR 应合并。 /learn 的原生视频输入设计严谨——三态视频源分类（local/remote/youtube）、双门控能力检查（model modality + provider transport）、YouTube 显式拒绝带引导。47 个单元测试全部通过，CLI 在此分支上正常运行。
验证环境
- macOS darwin, Node v22.22.1
- 分支：
lazzy/learn-video-input（通过pull/7497/head拉取）
1. 单元测试（47/47 通过）
learn-skill-agent.test.ts（36 tests）：
✓ src/memory/learn-skill-agent.test.ts (36 tests) 20ms
Test Files 1 passed (1)
Tests 36 passed (36)
learn-command.test.ts（11 tests）：
✓ src/ui/commands/learn-command.test.ts (11 tests) 8ms
Test Files 1 passed (1)
Tests 11 passed (11)
2. tmux CLI 启动验证
$ npx tsx packages/cli/src/cli.ts --version
0.20.1
3. 代码审查
架构决策：三态视频源分类
parseLearnVideoInput 将输入分为三种 kind：
local：本地路径（.mp4/.webm/.mov/.m4v）→ 通过readPathFromWorkspace读取 inline dataremote：HTTP(S) URL 且 pathname 以视频扩展名结尾 → 直接传fileData给 provideryoutube：YouTube 页面 URL（watch/embed/shorts/live/youtu.be）→ 显式拒绝，引导用户下载本地文件
双门控能力检查（learn-command.ts L68-80）：
config.getEffectiveInputModalities().video === true— 模型必须声明视频能力authType === USE_OPENAI || authType === QWEN_OAUTH— provider 必须走 OpenAI 兼容路径
两个条件任一不满足 → 返回错误消息，不提交 model turn，不写 skill。
论据链：
- YouTube 页面 URL ≠ 视频文件。RESOURCE2SKILL 论文使用 resource connector 做视频采样，直接传页面 URL 给
video_url不会返回 provider 结果 - 本地视频复用已有的 workspace 边界、ignore 规则、MIME 检测、10MB 限制——零新基础设施
- 蒸馏合约严格：不执行命令、不安装依赖、不添加权限、不声称 execution-verified、untrusted source 标记
references/source.md要求时间戳证据映射——可溯源- 47 个测试覆盖：4 种远程 MIME、5 种本地路径、5 种 YouTube 路由、10 种非视频输入、能力门控、附件失败
- 非视频输入完全不受影响——
parseLearnVideoInput返回 null 时走原有路径
结论：合并安全，无回归风险。
Code ReviewIndependent proposal: I would add a video-source parser in Findings: no critical blockers, no AGENTS.md violations. The implementation is clean and well-structured. One non-blocking observation: the defence-in-depth MIME relabel in Real-Scenario Testingtmux is not available in this CI environment. Used Python PTY ( YouTube URL rejection✅ Correctly rejected before any provider submission. Video capability gating (non-video model)✅ Correctly rejected — the test environment's model does not advertise video input. Text input regression✅ Existing text-learning behavior preserved — the model received the prompt and started creating a skill. Unit testsDirect function verification (built dist/)中文说明代码审查独立方案： 我会在 发现： 无关键阻塞项，无 AGENTS.md 违规。 实现干净、结构良好。 一个非阻塞观察： 真实场景测试此 CI 环境中 tmux 不可用。使用 Python PTY（ YouTube URL 拒绝✅ 在任何 provider 提交前正确拒绝。 视频能力门控（非视频模型）✅ 正确拒绝——测试环境的模型未声明视频输入。 文本输入回归✅ 现有文本学习行为保持不变——模型收到 prompt 并开始创建技能。 单元测试— Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean implementation that ships the feature correctly; only non-blocking nit is the potentially unreachable defence-in-depth MIME relabel. This PR does one thing well: it makes Going back to my independent proposal — the PR matches it closely. I didn't find a simpler path it missed. The scope is tight: no download, no transcription, no frame extraction, no automatic model switching — just explicit video transport with proper gating. Every change in the diff serves the stated goal. After seeing it run: YouTube rejection fires before any provider call, the capability gate blocks non-video models cleanly, and the text-input regression confirms the existing path is untouched. 214 unit tests pass, typecheck passes. If I had to maintain this in six months, I'd thank the author — the code is readable, the tests are thorough, and the design doc explains the why. The one thing I'd double-check is whether the defence-in-depth MIME relabel in Approving. ✅ 中文说明置信度：4/5 —— 干净的实现，正确交付了功能；唯一的非阻塞小问题是可能不可达的 defence-in-depth MIME 重标记。 这个 PR 做好了一件事：让 回到我的独立方案——PR 与之高度一致。我没有找到它遗漏的更简路径。范围紧凑：没有下载、没有转录、没有抽帧、没有自动切换模型——只有带正确门控的显式视频传输。diff 中的每个改动都服务于既定目标。 运行验证后：YouTube 拒绝在任何 provider 调用前触发，能力门控干净地阻止非视频模型，文本输入回归确认现有路径未受影响。214 个单元测试通过，类型检查通过。 如果六个月后我要维护这段代码，我会感谢作者——代码可读，测试全面，设计文档解释了 why。我唯一会再确认的是 批准。✅ — Qwen Code · qwen3.8-max-preview Reviewed at |
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. ✅
|
@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 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Could not address the latest feedback automatically (round 1/100). A human should take over this PR. What I found before stopping: Address Review Summary — PR #7497Feedback points1. [Critical]
|
|
@qwen-code /retry |
|
🔄 AutoFix re-armed. The next scan re-reads this PR's feedback from the start and the round counter resets. Nothing was deleted — this marker supersedes the evaluation markers above it. 中文说明🔄 已重新武装 AutoFix。下一次扫描会从头重新读取本 PR 的反馈，轮次计数也已重置。未删除任何内容 —— 本标记使其上方的评估标记失效。 |
|
🤖 Could not address the latest feedback automatically (round 1/100). A human should take over this PR. Review feedback addressed[Critical] rc:3629803884 —
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
[Critical] Existing blocker still stands: .m4v files always fail the MIME filter (root cause in fileUtils.ts, not modified by this PR) — previously reported at learn-command.ts:95
— qwen3.7-max via Qwen Code /review
|
🤖 Could not address the latest feedback automatically (round 2/100). A human should take over this PR. Review Feedback Summary — PR #7497Feedback Points1. [Critical] .m4v files always fail the MIME filter (CHANGES_REQUESTED)Decision: Fixed. Root cause: The Changes:
2. [Suggestion] Duplicated async setup (rc:3632931631)Decision: Implemented. Changes:
Why it was not pushed: tests failed in packages/core Run log: https://github.com/QwenLM/qwen-code/actions/runs/29949130868 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /retry |
|
🔄 AutoFix re-armed. The next scan re-reads this PR's feedback from the start and the round counter resets. Nothing was deleted — this marker supersedes the evaluation markers above it. 中文说明🔄 已重新武装 AutoFix。下一次扫描会从头重新读取本 PR 的反馈，轮次计数也已重置。未删除任何内容 —— 本标记使其上方的评估标记失效。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
[Critical] packages/cli/src/ui/commands/learn-command.ts:91-95 — .m4v files always fail the MIME filter (existing blocker re-confirmed, not re-posted inline due to overlap with rc:3629803884). Root cause in fileUtils.ts: mime/lite returns null for .m4v, so detectFileType classifies it as binary and processSingleFileContent returns a string instead of inlineData. The startsWith('video/') filter then finds nothing.
— qwen3.7-max via Qwen Code /review
- Fix .m4v MIME filter: fall back to parser MIME type when mime/lite does not recognise the extension (e.g. .m4v → video/x-m4v) - Surface text diagnostics from readPathFromWorkspace when the read succeeds but returns no video part (e.g. file size limit errors) - Move YouTube rejection after the capability gate so text-only model users get the correct error first - Extract shared buildSkillContext helper to deduplicate skill directory enumeration between buildLearnSkillPrompt and buildLearnVideoSkillRequest
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[Critical]
|
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.7-max via Qwen Code /review
Code Review —
|
- Fix .m4v mimeType in processSingleFileContent by falling back to MIME_LITE_MISSING_VIDEO_TYPES when mime/lite returns null - Move YouTube rejection before the capability gate so users always get actionable download guidance regardless of model capability - Use optional chaining for getContentGeneratorConfig()?.authType - Add [focus] to the empty-input usage string
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressedInline comments[rc:3640200698] The video case in Fix: Changed the mimeType assignment to fall back through mimeType:
mime.getType(filePath) ??
MIME_LITE_MISSING_VIDEO_TYPES.get(path.extname(filePath).toLowerCase()) ??
'application/octet-stream',Also added a [rc:3640200721] YouTube check ordering — Implemented. The capability gate fired before the YouTube rejection, so a user on a text-only model running Fix: Moved the Issue-level suggestions (@wenshao)Optional chaining for Changed to YouTube ordering — Same as rc:3640200721 above, implemented. Usage string missing Updated the empty-input usage string from Ignored local video → opaque error — Deferred. Valid improvement but out of scope for this review round: it requires a new error path distinguishing the empty-array (ignored file) case in Practical reach of local path (scope note) — Acknowledged, no action needed. The 9.9 MB inline cap is surfaced clearly to users and remote URLs bypass it via provider fetch. Not a bug. i18n keys in other locales — No action needed. Confirmed the three new keys are not in Verification
中文说明已处理的评审反馈行内评论[rc:3640200698]
修复： 将 mimeType 赋值改为先回退到 mimeType:
mime.getType(filePath) ??
MIME_LITE_MISSING_VIDEO_TYPES.get(path.extname(filePath).toLowerCase()) ??
'application/octet-stream',同时在 [rc:3640200721] YouTube 检查顺序 — 已实现。 能力门在 YouTube 拒绝之前触发，导致文本模型用户执行 修复： 将 Issue 级别建议（@wenshao）
改为 YouTube 顺序 — 同 rc:3640200721，已实现。 用法字符串缺少 将空输入用法字符串从 被忽略的本地视频报错不透明 — 延后处理。 是合理的改进，但超出本轮评审范围：需要在 本地路径实际可用范围（范围说明） — 已确认，无需操作。 9.9 MB 内联上限已清晰提示用户，远程 URL 通过 provider 拉取不受此限。非 bug。 其他语言的 i18n key — 无需操作。 已确认三个新 key 不在 验证
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.
— qwen3.7-max via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No changes were made in response to this review round. The only feedback was a No code changes are warranted. 中文说明本轮审查未做任何代码变更。 唯一的反馈来自自动审查机器人（ 无需进行代码变更。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Code Review —
|
| Check | Result |
|---|---|
packages/core — learn-skill-agent.test.ts + fileUtils.test.ts |
✅ 197/197 |
packages/cli — learn-command.test.ts |
✅ 14/14 |
npm run check-i18n (CI gate) |
✅ passed |
mime/lite coverage probe |
.mp4/.webm/.mov resolve; only .m4v returns null → the fix is correctly scoped and minimal |
| Converter reachability | converter.ts:924 (inlineData) and :977 (fileData) both map video/* → video_url; AuthType has exactly 5 members, so the USE_OPENAI || QWEN_OAUTH allowlist is complete for the OpenAI path |
Prompt-injection hygiene is good — JSON.stringify on both source and focus, plus an explicit untrusted-source-data preamble and a no-allowedTools/no-hooks clause. Extracting buildSkillContext is a clean dedup.
Findings
1. [Medium] The local-video path rejects the exact workflow the YouTube error recommends
The YouTube rejection says "Download the video and pass a local video file to /learn" — but readPathFromWorkspace does not expand ~, and rejects absolute paths outside the workspace. Driving learnCommand.action with a real (unmocked) reader:
'~/Downloads/tutorial.mp4'
→ "The local video could not be attached for /learn. Path not found in workspace: ~/Downloads/tutorial.mp4"
'/Users/me/Downloads/tutorial.mp4'
→ "The local video could not be attached for /learn. Absolute path is outside of the allowed workspace: ..."
So the recommended remedy is a dead end unless the user first copies the video into the repo. /learn's own usage string also advertises ~/projects/acme-sdk, so ~ is an expected input shape here.
Suggestion: run expandHomeDir (packages/core/src/utils/paths.ts:123) on video.source before readPathFromWorkspace, and reword the YouTube guidance to say the file must live inside the workspace.
2. [Medium] Prose whose first token ends in a video extension is hijacked with no fallback
parseLearnVideoInput commits to the video branch on extension alone, before any existence check, so ordinary text loses the old path:
'demo.mov is how we record release walkthroughs; always trim the intro'
→ ✕ "…Path not found in workspace: demo.mov" # was: learned as text
'Recording demo videos is how we document release walkthroughs'
→ submit_prompt (generic buildLearnSkillPrompt) # control: still works
On a text-only model the same input instead produces "does not support native video input", which is doubly confusing for what is plainly prose.
Suggestion: for kind: 'local', fall through to buildLearnSkillPrompt(rawInput, projectRoot) when the path does not resolve, rather than hard-erroring. That also softens finding 1.
3. [Medium] Gemini / Vertex are blocked even though they accept video parts natively
The gate is an auth-type allowlist in a UI command. But geminiChat forwards inlineData/fileData untouched to @google/genai — there is no modality stripping on that path (unsupportedModalityPlaceholder exists only in openaiContentGenerator/converter.ts). A video-capable Gemini model with modalities.video=true would work today and is rejected anyway.
Suggestion: move the decision into core as e.g. supportsNativeVideoTransport(config) next to the converter, so provider knowledge lives in one place and Gemini can be enabled without touching the CLI.
4. [Medium] Effective local-video ceiling is ~7.4 MB, not 10 MB
processSingleFileContent rejects both raw > 9.9 MB (fileUtils.ts:1211) and base64 > 9.9 MB (:1405). Base64 expands 4/3, so the binding constraint is ~7.4 MB of raw video. The PR fixture (6,533,189 B ≈ 6.2 MiB → ≈8.3 MiB encoded) only just fits; a typical screen-recorded tutorial will not. The design doc's "10 MB encoded-data limit" understates this.
Suggestion: state the real ceiling in the design doc, and consider surfacing "use a direct HTTPS URL instead" in the size-limit error — the remote path has no such cap.
5. [Medium] YouTube hard-reject removes the previous best-effort path
Before this PR, /learn https://youtu.be/x fell into the generic prompt and the model would web_fetch the page (title, description, transcript panel). Now it terminates at a hard error. The E2E evidence that YouTube fails is about passing the page as video_url — a different mechanism from the one being removed.
Suggestion: emit a warning and fall through to buildLearnSkillPrompt, so the user still gets the page-level skill they used to get.
6. [Low] Inlined video persists in session history for the whole session
compactionInputSlimming.transformPart (:327) keeps inlineData verbatim when the model supports the modality — which the gate guarantees — and the image-payload-references.ts offload path only matches image/*. So ~10 M characters of base64 stay in history and are re-sent on every subsequent request in that session. Given /learn is a one-shot distillation turn, consider extending payload-reference offloading to video/*, or dropping the part after the turn completes.
7. [Low] Non-file:// URI schemes are misclassified as local paths
Only file:// is special-cased; everything else that isn't http(s) falls into the local branch:
'ftp://host/clip.mp4' → { kind: 'local' } → "Path not found in workspace: ftp://host/clip.mp4"
's3://bucket/clip.mp4' → { kind: 'local' } → "Path not found in workspace: s3://bucket/clip.mp4"
Rejecting any ^[a-z][a-z0-9+.\-]*:// that isn't http(s) is a one-line fix and gives a clearer message.
8. [Low] The learn-command.ts MIME fallback is now unreachable, and can mislabel
With the fileUtils.ts root-cause fix in this same PR, processSingleFileContent already returns video/x-m4v, so the startsWith('video/') find always succeeds and the fallback block never runs in production. Worse, when it can run it relabels any inlineData part with the extension-derived video MIME: a directory named clips.mp4 expands to multiple parts, and the first inline part (possibly a PNG) would be sent to the provider tagged video/mp4.
Note the guarding test mocks readPathFromWorkspace returning application/octet-stream for .m4v — a response the core fix now makes impossible, so it no longer reflects production.
Suggestion: delete the fallback (the core fix supersedes it), or scope it to a single-part result.
9. [Nit] ca.js locale missed
en / zh / zh-TW / ca all sit at 1538 keys (the other five locales are partial at ~1205). This PR updated three of the four full locales. npm run check-i18n passes, so not a blocker — just parity drift.
10. [Nit] Quoted or space-containing paths silently degrade to text
/learn "./my videos/tutorial.mp4" parses to null and is learned as prose rather than erroring. Stripping surrounding quotes before extension matching is cheap.
Verdict
Solid, well-scoped, well-tested feature; the .m4v fix in particular is the right root-cause change. I'd fix 1 and 2 before merge — together they make the local path work for the workflow the command itself recommends, and stop the video branch from swallowing ordinary text input. 3–5 are worth a follow-up decision; 6–10 are cleanups.
中文说明
概述
为 /learn 增加视频分支：core 中的 parseLearnVideoInput 把首个 token 分类为本地视频 / 远程直链 / YouTube 页面，learn-command.ts 按模型视频模态 + OpenAI 兼容 auth type 做门控，buildLearnVideoSkillRequest 输出 [videoPart, {text: prompt}] 并带两文件 provenance contract。fileUtils.ts 中补上 mime/lite 缺失的 .m4v → video/x-m4v 是正确的根因修复。
已验证
core 197/197、CLI 14/14、check-i18n 通过；探针确认 mime/lite 只缺 .m4v（.mp4/.webm/.mov 都有），converter 的 inlineData 与 fileData 两条路径都能映射到 video_url，AuthType 共 5 个成员，allowlist 对 OpenAI 路径是完整的。Prompt 注入防护良好。
主要问题
- [中] 本地视频路径拒绝了错误提示自己推荐的做法：
readPathFromWorkspace不展开~，且拒绝 workspace 外的绝对路径。实测~/Downloads/tutorial.mp4→ "Path not found in workspace"，/Users/me/Downloads/tutorial.mp4→ "outside of the allowed workspace"。而 YouTube 报错正是让用户"下载后传本地文件"。建议先用expandHomeDir（paths.ts:123），并在提示中说明文件需在 workspace 内。 - [中] 首 token 像视频扩展名的普通文本被劫持且无回退：实测
demo.mov is how we record release walkthroughs...直接报错，而去掉该 token 的同句仍走文本路径。建议kind: 'local'路径解析失败时回退到buildLearnSkillPrompt。 - [中] Gemini / Vertex 被无谓拦截：
geminiChat原样透传inlineData/fileData，没有模态剥离（unsupportedModalityPlaceholder只存在于 OpenAI converter）。建议把判断下沉为 core 的supportsNativeVideoTransport(config)。 - [中] 本地视频实际上限约 7.4 MB 而非 10 MB：
fileUtils.ts:1211限原始 >9.9MB，:1405限 base64 >9.9MB，4/3 膨胀后原始上限约 7.4MB。PR 的 6.2MiB fixture 刚好卡住。建议在设计文档中写明，并在超限报错里提示改用直链。 - [中] YouTube 硬拒绝移除了原有的兜底能力：此前会走通用路径由模型
web_fetch页面。E2E 证明失败的是"把页面当video_url"，与被移除的机制不同。建议改为告警 + 回退。 - [低] 内联视频会在整个会话中常驻历史：
compactionInputSlimming.transformPart:327在模型支持该模态时原样保留inlineData，而image-payload-references.ts只处理image/*，约 1000 万字符的 base64 会在后续每个请求中重发。建议把 offload 扩展到video/*，或在该 turn 后丢弃。 - [低] 非
file://的 URI scheme 被误判为本地路径：ftp://、s3://→kind: 'local'，报错信息令人困惑。 - [低]
learn-command.ts的 MIME 兜底已不可达且可能误标：core 修复后第一个find必定命中；而该兜底会把任意inlineDatapart 打上视频 MIME（如目录clips.mp4中的 PNG）。对应测试 mock 的响应在修复后已不可能出现。 - [Nit] 漏了
ca.js：en/zh/zh-TW/ca 同为 1538 key，本 PR 只更新了其中三个。check-i18n通过，非阻塞。 - [Nit] 带引号或含空格的路径静默降级为文本。
结论
功能扎实、范围克制、测试充分，.m4v 根因修复尤其正确。建议合并前先修 1 与 2；3–5 值得单独决策；6–10 属清理项。
Independent local verification — real build, real videos, real wireRe-verified as maintainer at head Fixtures are real videos — a 2,174,096-byte ISO-BMFF
1 · Wire oracle — the video really leaves the CLI, byte-for-byte
The base64 recorded on the wire decodes to exactly the fixture bytes —
The remote arm used 2 · Every pre-submission gate, on the real CLI
3 · The
|
| check | result |
|---|---|
vitest packages/core — fileUtils.test.ts + learn-skill-agent.test.ts |
197 passed |
vitest packages/cli — learn-command.test.ts |
14 passed |
| A/B: base sources overlaid, PR tests kept | 41 discriminating failures (31 core + 10 CLI); the other 4 CLI tests are regression guards that pass both ways |
npm run typecheck (all workspaces) |
pass |
eslint --max-warnings 0 on all 9 changed code/locale files (10th is the Markdown design doc) |
pass |
npm ci → prepare build, PR strings present in cli/dist + core/dist |
pass |
| CI on the PR | 11 SUCCESS / 19 SKIPPED / 0 failing (ubuntu test suite + web-shell E2E smoke green; the macOS/Windows jobs are matrix-skipped) |
Both .m4v tests are in the discriminating set, so the fileUtils.ts change is pinned by tests that genuinely fail without it.
Findings — all non-blocking
1 · A video path containing a space silently falls back to the generic path (new, low/medium — UX)
parseLearnVideoInput takes the first whitespace-delimited token as the source, so /learn ./screen recording.mp4 focus on hover parses ./screen — not a video — and the whole input drops to the generic /learn path with no error and no hint (scenario S11: 0 video parts on the wire). This is the default filename shape for macOS screen recordings (Screen Recording 2026-07-24 at 10.00.00.mov) and for most downloaded tutorials, so it is likely to be hit. Quoting does not help — "./screen is still the first token.
Cheap fix: before falling through, also try the whole trimmed input as the source when it ends in a supported video extension and resolves to an existing file; or accept a quoted first token. Either keeps the "first token" rule as the default and only adds a fallback.
2 · Git-ignored local video gives a reason-less error (already deferred by the author in round 3 — confirmed live)
readPathFromWorkspace returns [] for an ignored file, so errorDetail is empty and the user sees a bare The local video could not be attached for /learn. (panel 5 above). Everything else on this path surfaces a concrete reason — the oversize case, for instance, reports 11.06MB encoded. Worth a follow-up, not a blocker.
3 · The MIME-override fallback in learn-command.ts is now unreachable (new, cleanup)
After the root-cause fix in fileUtils.ts, processSingleFileContent already stamps video/x-m4v, so the first parts.find(… startsWith('video/')) always matches and the if (!localVideoPart) MIME-override block can never run for any extension parseLearnVideoInput accepts.
Proven, not inferred: I inserted a hard-fail probe at the top of that block in the built CLI and re-ran the .m4v scenario — it still succeeded with video/x-m4v on the wire, i.e. the branch was never entered. Control run with the same probe moved onto the reachable path fired immediately and blocked submission, so the probe mechanism works.
Harmless as defence-in-depth, but the unit test "falls back to the parser MIME type when mime/lite does not recognise the extension" now pins a state the real chain can no longer produce (it mocks readPathFromWorkspace into returning application/octet-stream). Worth either a comment saying so, or dropping the branch.
4 · Context (not caused by this PR): the video is re-sent on the follow-up turn
The managed-memory classifier side-turn replays conversation history, so one /learn <video> produced two ~2.9 MB request bodies (2,987,750 and 2,950,404 bytes) versus 87,371 / 50,025 on main. Pre-existing history-replay behaviour, but video payloads amplify it a lot — worth knowing when reasoning about cost.
Verified non-issues
- ACP mode.
/learndeclaressupportedModes: ['interactive', 'acp']and now returnsPartListUnioninstead of a string.Session.#processSlashCommandResultroutessubmit_promptthroughnormalizePartList(result.content), so array content is handled — no breakage. (code-read; not executed under ACP.) - Prompt-expansion hooks don't leak base64.
serializeUserPromptExpansionPrompt→partToString(…, { verbose: true })renders an inline part as<video/mp4>, andappendUserPromptExpansionAdditionalContextappends a text part rather than replacing the array — the video part survives and no 2.9 MB base64 reaches hook stdin. - Non-video
/learnis untouched. Plain text still emits the originalweb_fetch-style prompt with zero video parts (S7).
Out of scope for this verification
I have no video-capable model credentials, so this run verifies transport, gating and regression — not distillation quality. Whether the model writes a faithful SKILL.md + references/source.md is covered by the author's E2E comment, which already reports the honest result (files and provenance correct; tutorial fidelity FAIL on that case). That split is the right one: this PR ships the transport, and it does that correctly.
Recommendation: merge. Finding 1 is the only one I'd suggest picking up soon, and it is a small, self-contained follow-up.
中文说明
独立本地验证 —— 真实构建、真实视频、真实请求
以维护者身份在 head 3e178fdc（merge-base e7097d0e）重新验证。使用独立 worktree + 独立 npm ci，确保被测的 bundle CLI 确实是本 PR 的代码。传输路径没有任何 mock/stub：通过 PTY 驱动 bundle CLI，运行在隔离的 HOME/project 中，指向一个本地 mock OpenAI 兼容服务器，完整记录实际发出的 JSON 请求体。"before" 一侧用同一套 harness 跑在单独的 main worktree 上，因此每组 A/B 只有构建不同。
fixture 是真实视频 —— macOS 自带的 2,174,096 字节 ISO-BMFF ftyp mp42 MP4 和 80,315 字节 QuickTime .mov。
结论：approve，可以合并。 原生视频传输、四条提交前门控、以及未改动的非视频路径都已端到端确认。下面记录两个非阻塞的粗糙点（一个新发现，一个作者已在第 3 轮延后处理）。
1 · Wire oracle —— 视频确实逐字节离开了 CLI
线上记录的 base64 解码后精确等于 fixture 字节：独立对磁盘文件计算的 sha256 与解码 payload 的哈希一致。在 main 上，同样的命令发出零视频字节，路径只是 <user_data> 里的一个字符串。
| 传输方式 | 记录到的 video_url |
payload |
|---|---|---|
本地 .mp4 |
data:video/mp4;base64,… |
2,174,096 B · sha256 与 fixture 一致 |
本地 .m4v |
data:video/x-m4v;base64,… |
2,174,096 B |
本地 .mov |
data:video/quicktime;base64,… |
80,315 B |
| 远程 URL | 原样 https://cdn.example.com/tutorial.mp4 |
—（Qwen Code 不下载） |
远程用例使用的 cdn.example.com 没有 A 记录（curl 直接失败），命令却依然成功 —— 这本身就证明 Qwen Code 从不拉取该 URL，而是直接交给 provider。
2 · 全部提交前门控，在真实 CLI 上验证
provider requests: 0 是 mock 服务器实测，不是单测断言。第 2 个面板现场确认了第 3 轮的顺序修复：文本模型上 YouTube URL 现在给出可操作的"下载视频"提示，而不是"换视频模型"的死路提示。
3 · .m4v 修复是必需的，而且不只影响 /learn
对真实、未 mock 的 mime/lite 确认：mime.getType('x.m4v') 返回 null。直接调用各自 worktree 构建出的 core/dist，对同样三个文件测试。在 main 上，真实 .m4v 视频会变成字符串 Cannot display content of binary file —— 也就是说 read_file、@file 上下文和 read_many_files 全都受影响，不只是 /learn。修在 fileUtils.ts 而不是命令层，是正确的选择。
代码级检查（3e178fdc）
| 检查 | 结果 |
|---|---|
vitest packages/core — fileUtils.test.ts + learn-skill-agent.test.ts |
197 通过 |
vitest packages/cli — learn-command.test.ts |
14 通过 |
| A/B：覆盖回 base 源码、保留 PR 测试 | 41 个判别性失败（31 core + 10 CLI）；其余 4 个 CLI 测试是两边都通过的回归护栏 |
npm run typecheck（全部 workspace） |
通过 |
eslint --max-warnings 0（全部 9 个代码/语言文件，第 10 个是 Markdown 设计文档） |
通过 |
npm ci → prepare 构建，cli/dist 与 core/dist 含 PR 字符串 |
通过 |
| PR 上的 CI | 11 SUCCESS / 19 SKIPPED / 0 失败（ubuntu 测试套件与 web-shell E2E smoke 通过；macOS/Windows job 被 matrix 跳过） |
两个 .m4v 测试都在判别集合中，说明 fileUtils.ts 的改动确实被会失败的测试钉住。
发现 —— 均非阻塞
1 · 文件名含空格的视频路径会静默回退到通用路径（新发现，低/中，UX）
parseLearnVideoInput 取第一个空白分隔 token 作为源，因此 /learn ./screen recording.mp4 focus on hover 解析出 ./screen（不是视频），整条输入落回通用 /learn 路径，没有报错也没有任何提示（场景 S11：线上 0 个视频 part）。这正是 macOS 录屏（Screen Recording 2026-07-24 at 10.00.00.mov）和多数下载教程的默认命名，很容易命中。加引号也没用 —— 第一个 token 变成 "./screen。
低成本修法：在回退之前，若整条 trim 后的输入以受支持的视频扩展名结尾且能解析到存在的文件，则将其作为源；或支持带引号的第一个 token。默认仍保持"第一个 token"规则，只是多一层回退。
2 · 被 git-ignore 的本地视频报错没有原因（作者第 3 轮已延后 —— 现场确认）
readPathFromWorkspace 对被忽略文件返回 []，于是 errorDetail 为空，用户只看到裸的 The local video could not be attached for /learn.。这条路径上其他分支都会给出具体原因（例如超限会报 11.06MB encoded）。值得后续跟进，不阻塞。
3 · learn-command.ts 里的 MIME 回退分支现在不可达（新发现，清理项）
在 fileUtils.ts 做了根因修复之后，processSingleFileContent 已经会打上 video/x-m4v，因此第一个 parts.find(… startsWith('video/')) 总能命中，if (!localVideoPart) 的 MIME 覆盖块对 parseLearnVideoInput 接受的任何扩展名都不会执行。
这是实测而非推断：我在构建产物中该分支开头插入了一个硬失败探针，重跑 .m4v 场景 —— 仍然成功且线上 mime 为 video/x-m4v，即分支从未进入。对照组把同样的探针移到可达路径上，立即触发并阻止了提交，说明探针机制有效。
作为纵深防御无害，但单测 "falls back to the parser MIME type when mime/lite does not recognise the extension" 现在钉住的是真实链路已无法产生的状态（它把 readPathFromWorkspace mock 成返回 application/octet-stream）。建议要么加注释说明，要么删掉该分支。
4 · 背景信息（非本 PR 引入）：后续 turn 会重发视频
managed-memory 分类器的副作用 turn 会重放会话历史，因此一次 /learn <video> 产生了两个 ~2.9 MB 的请求体（2,987,750 与 2,950,404 字节），而 main 上是 87,371 / 50,025。这是既有的历史重放行为，但视频负载会显著放大它，评估成本时值得知道。
已确认不是问题
- ACP 模式。
/learn声明supportedModes: ['interactive', 'acp']，现在返回PartListUnion而非字符串。Session.#processSlashCommandResult通过normalizePartList(result.content)处理submit_prompt，数组内容可正常处理，不会破坏。（阅读代码确认，未在 ACP 下实跑。） - prompt-expansion hook 不会泄漏 base64。
serializeUserPromptExpansionPrompt→partToString(…, { verbose: true })将 inline part 渲染为<video/mp4>；appendUserPromptExpansionAdditionalContext是追加 text part 而非替换数组 —— 视频 part 得以保留，且 2.9 MB base64 不会进入 hook stdin。 - 非视频
/learn未受影响。 纯文本仍然发出原有的web_fetch风格 prompt，且 0 个视频 part（S7）。
本次验证的范围外
我没有支持视频的模型凭据，因此本轮验证的是传输、门控与回归，不是蒸馏质量。模型能否写出忠实的 SKILL.md + references/source.md，由作者的 E2E 评论覆盖，其中也如实报告了结果（文件与 provenance 正确；该用例的教程忠实度 FAIL）。这个分工是合理的：本 PR 交付的是传输，而传输做对了。
建议：合并。 只有发现 1 值得尽快跟进，且它是一个小而独立的后续改动。
Harness: @lydell/node-pty + @xterm/headless PTY driver · node http wire recorder · Playwright-rendered evidence · isolated worktrees at 3e178fdc and e7097d0e · macOS 24.6.0 · node v22.23.1
|
@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. ✅
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed on PR #7497Addressed the two findings the reviewer marked as merge blockers (1 and 2), plus the unreachable/MIME-mislabel cleanup (8) that both review passes flagged. The remaining findings are design decisions or follow-ups and are deferred with reasons below. No conflicts ( ImplementedFinding 1 — [Medium] local-video path rejected the workflow the YouTube error recommends.
Finding 2 — [Medium] prose whose first token ends in a video extension was hijacked with no fallback.
Finding 8 — [Low] the
Deferred (with reasons)
Conflict notes
Verification
中文说明PR #7497 已处理的评审反馈处理了评审人标记为合并阻塞项的两个发现（1 与 2），以及两轮评审都指出的、不可达且可能误标 MIME 的清理项（8）。其余发现属于设计决策或后续跟进项，下文给出延后理由。无冲突（ 已实现发现 1 — [中] 本地视频路径拒绝了错误提示自己推荐的做法。
发现 2 — [中] 首 token 以视频扩展名结尾的普通文本被劫持且无回退。
发现 8 — [低]
延后处理（附理由）
冲突说明
验证
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/模型 |
1590480
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
Code Review (round 5) —
|
| Check | Result |
|---|---|
learn-command.test.ts |
✅ 17/17 |
learn-skill-agent.test.ts + fileUtils.test.ts |
✅ 197/197 |
| CI (all non-skipped checks) | ✅ 11 pass, 0 fail |
| i18n: old key fully retired, no orphan in any locale | ✅ |
Behaviour probe with the real readPathFromWorkspace |
9 scenarios, 3 new defects |
The shipped learn-command.test.ts mocks readPathFromWorkspace, so none of the paths below are exercised by CI. I drove learnCommand.action with the real reader over a temp workspace, using a genuine 2,174,096-byte MP4 (SiriUI.framework/…/Speak to Siri.mp4) as the fixture:
A ./tutorial.mp4 (in workspace) → submit_prompt PARTS [inline:video/mp4(2898796 b64), text(2549)] ← control
C2 ~/<ws-under-home>/tutorial.mp4 → submit_prompt PARTS [inline:video/mp4(2898796 b64), text(2534)]
B /outside-ws/tutorial.mp4 → submit_prompt TEXT (1106 chars) ← no error shown
C ~/Downloads/tutorial.mp4 → submit_prompt TEXT (1052 chars) ← no error shown
D "demo.mov is how we record …" → submit_prompt TEXT (1059 chars) ← intended
E ./ignored-tutorial.mp4 → error, no reason ← known / deferred
F ./clips.mp4/ (dir, ONE png) → submit_prompt PARTS [inline:video/mp4(24 b64), text(2544)]
G ./notes.mp4/ (dir, 2 txt) → error, 1611 chars, contains file bodies
H "./Screen Recording 2026.mov" → submit_prompt TEXT (1046 chars) ← known / deferred
Confirmed fixed
- Round-4 finding 1 —
expandHomeDir. Genuinely works: probe C2 shows~/<workspace-under-$HOME>/tutorial.mp4now resolving and attaching as a real 2.9 MB base64video/mp4part, where round 3 hard-failed withPath not found in workspace: ~/…. Correctly placed at the call site rather than insidereadPathFromWorkspace, so no blast radius on other readers. - Round-4 finding 2 — prose fall-through. Probe D confirms
demo.mov is how we record release walkthroughslearns as text again. fileUtils.tsMIME fix is applied at both sites (detectFileType:813and the video branch at:1434), so the root cause is genuinely closed for.m4v.
Findings
1. [Medium] Every local-read failure is now silent — including the workflow this PR's own error message prescribes
learn-command.ts:96 catches with a bare catch { and discards the reason. readPathFromWorkspace throws in three distinct situations — Absolute path is outside of the allowed workspace, Path not found in workspace, and any fs.stat/glob I/O error — and all three now produce a submit_prompt with no user-visible signal at all.
The fall-through itself is right (I asked for it, and case D is exactly why). The problem is that it is applied uniformly and silently. Probes B and C show the cost:
- The YouTube rejection tells the user "Download the video into your workspace and pass the local video file path to /learn." Browsers download to
~/Downloads, which is outside the workspace for essentially every project. That user now gets no error — round 3 at least told themAbsolute path is outside of the allowed workspace. - What the model receives instead is the generic prompt built from the raw path string:
<user_data> /Users/me/Downloads/tutorial.mp4 focus on the hover animation </user_data> … - If the source is a file/directory path, use read_file / list_directory to read it.read_filethen fails on the same workspace boundary, one layer deeper, and the failure is no longer attributable to the video path. So this is not a graceful degradation for the path-shaped case — it is the same dead end, minus the diagnosis.
Suggested fix — keep the fall-through, but say so. context.ui.addItem is already on CommandContext (types.ts:61) and is used this way in bugCommand.ts:62:
} catch (error) {
const reason = error instanceof Error ? error.message : String(error);
context.ui.addItem(
{ type: MessageType.INFO, text: `${t('Could not attach as video; learning the input as text instead.')} ${reason}` },
Date.now(),
);
return { type: 'submit_prompt', content: await buildLearnSkillPrompt(rawInput, projectRoot) };
}2. [Medium] The round-4 relabel guard does not close the hole it claims to close
The round-4 note states a directory "can never have its first inline part — possibly a PNG — relabelled as video/mp4". That holds only for directories with two or more media files. Probe F — a directory literally named clips.mp4/ containing exactly one PNG — still mutates the PNG to video/mp4 and ships those 24 base64 characters as the video part, discarding every directory text marker.
The shipped guard test (learn-command.test.ts:186) uses two PNGs, so it passes while the single-file case mislabels. Reducing it to one PNG flips it, which I confirmed directly:
F2 result type : submit_prompt
F2 png mimeType after call: video/mp4
The guard keys on part count rather than on the condition it was designed to repair (an unrecognised MIME). Since the fileUtils.ts fix, every extension in DIRECT_VIDEO_MIME_TYPES already resolves to video/*, so this fallback is unreachable for its stated purpose and reachable only through this unintended case.
Suggested fix: delete the fallback and its unit test — the root-cause fix supersedes it, and the note in the code already says as much. If you prefer to keep it, gate on the actual repair condition rather than the count:
if (inlineParts.length === 1 &&
(!inlineParts[0].inlineData!.mimeType ||
inlineParts[0].inlineData!.mimeType === 'application/octet-stream')) { … }3. [Low] errorDetail is an unbounded join and can dump file contents into the error message
learn-command.ts:129 joins every string part into the error text. For a directory read, processSingleFileContent returns whole text-file bodies as strings, so probe G (notes.mp4/ holding two text files) produced a 1,611-character error message containing the files' contents verbatim. The intent was to surface the short one-line oversize diagnostic; nothing bounds it.
Suggested fix: truncate the joined detail (e.g. .slice(0, 200)), or only surface it when the read target was a single file.
Findings 2 and 3 share one root cause: the
localbranch never checks that the resolved path is a file.readPathFromWorkspacehappily expands a directory, and a.mp4-suffixed directory is the only way to reach either bug. A singlestats.isFile()guard (or rejecting multi-part reads outright) closes both.
Still open from earlier rounds (unchanged, previously deferred)
- Probe E — a git-ignored video returns
[], producing a reason-less "could not be attached" error. - Probe H —
./Screen Recording 2026.mov(the macOS default filename) silently takes the text path.
Both remain reasonable follow-ups; neither is affected by this round.
Verdict
The round-4 fixes for findings 1 and 2 are real and verified. But the two mechanisms introduced this round each have a defect: the fall-through lost the diagnosis for the workflow the PR itself recommends (finding 1), and the relabel guard still mislabels the case it was added to prevent (finding 2). Both are small, local changes.
Request changes — one short round. Findings 1 and 2 are worth fixing before merge; finding 3 is a one-line clamp. Everything else in this PR — transport, gates, prompt contract, i18n, the .m4v root-cause fix — is in good shape.
中文说明
代码评审（第 5 轮）— feat(cli): support native video input in /learn
在独立 worktree 中于 159048027 复审。本轮仅针对第 4 轮的增量（3e178fdcf..159048027，5 个文件 / +86 −29）——传输链路、能力门控与 prompt 契约已在上一轮端到端验证过，本轮未改动。
第 4 轮的三处改动为：读取工作区前先 expandHomeDir、读取失败时由硬报错改为回退文本路径、以及把 parser-MIME 重标限定为单个 inline part。
验证内容
| 检查项 | 结果 |
|---|---|
learn-command.test.ts |
✅ 17/17 |
learn-skill-agent.test.ts + fileUtils.test.ts |
✅ 197/197 |
| CI（所有非 skip 检查） | ✅ 11 通过，0 失败 |
| i18n：旧 key 已完全退役，各语言无孤儿条目 | ✅ |
使用真实 readPathFromWorkspace 的行为探针 |
9 个场景，发现 3 个缺陷 |
仓库内的 learn-command.test.ts mock 掉了 readPathFromWorkspace，因此下列路径 CI 完全没有覆盖。我用真实 reader 在临时 workspace 上驱动 learnCommand.action，fixture 使用真实的 2,174,096 字节 MP4（SiriUI.framework/…/Speak to Siri.mp4）：
A ./tutorial.mp4（workspace 内） → submit_prompt PARTS [inline:video/mp4(2898796 b64), text(2549)] ← 对照组
C2 ~/<ws-under-home>/tutorial.mp4 → submit_prompt PARTS [inline:video/mp4(2898796 b64), text(2534)]
B /outside-ws/tutorial.mp4 → submit_prompt TEXT (1106 字符) ← 无任何报错
C ~/Downloads/tutorial.mp4 → submit_prompt TEXT (1052 字符) ← 无任何报错
D "demo.mov is how we record …" → submit_prompt TEXT (1059 字符) ← 符合预期
E ./ignored-tutorial.mp4 → 报错，无原因 ← 已知 / 已延后
F ./clips.mp4/（目录，1 个 png） → submit_prompt PARTS [inline:video/mp4(24 b64), text(2544)]
G ./notes.mp4/（目录，2 个 txt） → 报错，1611 字符，含文件正文
H "./Screen Recording 2026.mov" → submit_prompt TEXT (1046 字符) ← 已知 / 已延后
已确认修复
- 第 4 轮发现 1 —
expandHomeDir。 确实生效：探针 C2 显示~/<$HOME 下的 workspace>/tutorial.mp4现在能解析并作为真实的 2.9 MB base64video/mp4part 附加，而第 3 轮会硬失败于Path not found in workspace: ~/…。改动放在调用点而非readPathFromWorkspace内部，对其他 reader 无影响，位置正确。 - 第 4 轮发现 2 — 普通文本回退。 探针 D 确认
demo.mov is how we record release walkthroughs重新作为文本被学习。 fileUtils.ts的 MIME 修复在两处都已应用（detectFileType:813与:1434的 video 分支），.m4v的根因确实已关闭。
发现
1. [中] 本地读取的所有失败现在都是静默的——包括本 PR 自己的错误提示所推荐的那条路径
learn-command.ts:96 使用裸 catch { 并丢弃了原因。readPathFromWorkspace 会在三种不同情形下抛错——Absolute path is outside of the allowed workspace、Path not found in workspace，以及任意 fs.stat/glob 的 I/O 错误——而这三种现在都会产出一个 submit_prompt，且对用户没有任何可见提示。
回退本身是对的（是我上轮提的，场景 D 正是理由）。问题在于它被无差别地静默应用。探针 B 和 C 显示了代价：
- YouTube 拒绝提示告诉用户*"Download the video into your workspace and pass the local video file path to /learn."*。浏览器默认下载到
~/Downloads，而它对几乎所有项目而言都在 workspace 之外。这类用户现在收不到任何报错——第 3 轮至少还会告诉他们Absolute path is outside of the allowed workspace。 - 模型收到的是用原始路径字符串拼出的通用 prompt：
随后
<user_data> /Users/me/Downloads/tutorial.mp4 focus on the hover animation </user_data> … - If the source is a file/directory path, use read_file / list_directory to read it.read_file会在同一个 workspace 边界上失败，只是深了一层，而且失败已无法归因到视频路径。所以对"路径形态"的输入而言，这并不是优雅降级——而是同一个死路，只是少了诊断信息。
建议修法 —— 保留回退，但要告知用户。context.ui.addItem 已在 CommandContext 上（types.ts:61），bugCommand.ts:62 就是这样用的：
} catch (error) {
const reason = error instanceof Error ? error.message : String(error);
context.ui.addItem(
{ type: MessageType.INFO, text: `${t('Could not attach as video; learning the input as text instead.')} ${reason}` },
Date.now(),
);
return { type: 'submit_prompt', content: await buildLearnSkillPrompt(rawInput, projectRoot) };
}2. [中] 第 4 轮新增的重标护栏并没有堵住它声称堵住的洞
第 4 轮说明中称目录*"永远不会把它的第一个 inline part（可能是 PNG）误标为 video/mp4"*。这只在目录含两个及以上媒体文件时成立。探针 F——一个名字就叫 clips.mp4/、内含恰好一个 PNG 的目录——依然会把该 PNG 改标为 video/mp4，并把这 24 个 base64 字符作为视频 part 发出，同时丢弃全部目录文本标记。
仓库内的护栏测试（learn-command.test.ts:186）用的是两个 PNG，所以它通过，而单文件场景仍然误标。把它减为一个 PNG 即可翻转，我已直接确认：
F2 result type : submit_prompt
F2 png mimeType after call: video/mp4
该护栏基于 part 数量，而非它本要修复的条件（无法识别的 MIME）。自 fileUtils.ts 修复之后，DIRECT_VIDEO_MIME_TYPES 中的每个扩展名都已能解析为 video/*，因此这段回退对其既定用途而言不可达，只能通过这个非预期场景到达。
建议修法： 删除该回退及其单测——根因修复已经取代了它，代码里的注释其实也这么写了。若希望保留，请按真正的修复条件而非数量来判定：
if (inlineParts.length === 1 &&
(!inlineParts[0].inlineData!.mimeType ||
inlineParts[0].inlineData!.mimeType === 'application/octet-stream')) { … }3. [低] errorDetail 是无界拼接，可能把文件正文倒进错误消息
learn-command.ts:129 把每一个字符串 part 都拼进错误文本。对目录读取而言，processSingleFileContent 会把整个文本文件正文作为字符串返回，因此探针 G（notes.mp4/ 内含两个文本文件）产生了一条 1,611 字符、逐字包含文件内容的错误消息。原意只是想透出那条一行的超限诊断，但没有任何长度约束。
建议修法： 对拼接结果做截断（例如 .slice(0, 200)），或仅在读取目标是单个文件时才透出。
发现 2 与发现 3 同源：
local分支从未检查解析出的路径是否为文件。readPathFromWorkspace会照常展开目录，而以.mp4结尾的目录是触达这两个 bug 的唯一途径。加一个stats.isFile()判断（或直接拒绝多 part 读取结果）即可同时关闭两者。
前几轮遗留（本轮未变，此前已延后）
- 探针 E —— 被 git-ignore 的视频返回
[]，产生没有原因的 "could not be attached" 错误。 - 探针 H ——
./Screen Recording 2026.mov（macOS 默认文件名）静默走文本路径。
两者作为后续跟进都合理，本轮均未受影响。
结论
第 4 轮针对发现 1 和 2 的修复是真实且已验证的。但本轮引入的两个机制各自带有缺陷：回退丢失了本 PR 自己所推荐工作流的诊断信息（发现 1），重标护栏对它本要防范的场景仍会误标（发现 2）。两者都是很小的局部改动。
建议修改后合入——一个短轮次即可。 发现 1 与 2 值得在合入前修掉；发现 3 只是一行截断。本 PR 的其余部分——传输、门控、prompt 契约、i18n、.m4v 根因修复——状态良好。
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. ✅
Re-verification at round 4 — real build, real videos, real wire ·
|
| # | Prior finding | Sev | Status at round 4 |
|---|---|---|---|
| 1 | home-relative ~/… video rejected with a dead-end error |
Medium | ✅ Fixed — expandHomeDir(video.source) before readPathFromWorkspace; now attaches (proven on the wire) |
| 2 | prose whose 1st token ends .mov hijacked, no fallback |
Medium | ✅ Fixed — read failure now falls through to the generic text path (submit_prompt) |
| 8 / v3 | learn-command.ts MIME fallback unreachable & could mislabel |
Low | ✅ Addressed — scoped to a single inline part; multi-part directory never relabelled; kept as a guarded regression test |
| 3 | Gemini/Vertex blocked though they accept video natively | Medium | ⏸ Deferred (provider-architecture decision) — still true |
| 4 | effective local ceiling is ~7.4 MB, not 10 MB | Medium | ⏸ Deferred (shared fileUtils error) — still true |
| 5 | YouTube hard-reject dropped the old best-effort path | Medium | ⏸ Deferred (product call); message reworded |
| 6 | inlined video persists in session history | Low | ⏸ Deferred (shared offload infra) — still true |
| 7 | non-file:// URI schemes misclassified as local |
Low | ✅ Softened — now degrades to the text path instead of a confusing hard error |
| v1 | space-containing paths silently degrade | — | ⏸ Deferred (parser is pure-string) — still true |
| v2 | git-ignored video → reason-less error | — | ⏸ Unchanged — still true (reproduced live, see row 9 below) |
| v4 | video re-sent on the managed-memory classifier turn | context | ⏸ Pre-existing — confirmed (each /learn puts the video on the wire twice) |
How this was verified
The built CLI (packages/cli/dist/index.js) was driven through a real pty (node-pty + @xterm/headless) against a mock OpenAI-compatible endpoint that records every /v1/chat/completions body, with an isolated HOME and OPENAI_MODEL=qwen3-vl-plus (video-capable) — so each scenario is judged by what actually left the process, not by screen text. Real macOS system videos were used as fixtures (.mov 80 KB, .mp4 2.17 MB); .m4v is a renamed MP4 container.
The three ● rows are exactly the fixes; every other row is unchanged and correct.
Findings 1 & 2 — before/after in the real TUI (round 3 dead-ended with 0 wire requests; round 4 succeeds with 2):
Test A/B + wire oracle + scoped-catch proof:
- Full-PR A/B (PR test files over merge-base
e7097d0esource): 42 discriminating failures (31 core + 11 cli); all214pass on the real head. - Round-4 delta A/B (round-4 tests over round-3 source): exactly 5 failures = Finding 1 + Finding 2 + Finding 8, nothing else — the diff does only what it claims.
- Wire oracle: the attached video decoded from the recorded request body is byte-identical to the file on disk (
sha256 d9855e7b…, 80,315 B). The.m4vcase is stampedvideo/x-m4vby thefileUtilsMIME fix — i.e. through the real chain, not thelearn-commandfallback (confirming Finding 8's fallback is now genuinely defence-in-depth). - Scoped-catch proof (the one thing worth double-checking about the round-4 change): the new
catchthat falls through to text is scoped to path-resolution failures only. Genuine diagnostics still surface — an oversize video still errorsFile size exceeds the 10MB limit, and an unreadable (mode-000) file still errorsEACCES: permission denied. The fall-through does not over-swallow.
One observation on the round-4 trade-off (non-blocking)
The fall-through triggers on any read failure, so a single-token path that's clearly meant as a file but mistyped — e.g. /learn ./cilp.mov (typo of clip.mov) or an absolute path outside the workspace — now silently becomes text for the model to "learn", where round 3 told the user "Path not found in workspace: ./cilp.mov". This is a direct and intended consequence of the Finding-2 fix (the reviewer asked for fall-through over hard-erroring), and it is the correct default for genuine prose. If you want to keep the actionable diagnostic for obvious typos, a later refinement could fall through only when the input looks like prose (contains whitespace / more than one token) and keep the hard error for a bare single-token path. Not a merge blocker — flagging so the trade-off is a conscious one.
Checks
packages/cli—learn-command.test.ts17, i18nindex.test.ts+mustTranslateKeys.test.ts31 → 48 passpackages/core—learn-skill-agent.test.ts+fileUtils.test.ts197 pass- Bundles built cleanly on both heads; each verified to contain its own head's source; CI
Test (ubuntu-latest)+web-shell E2E Smokegreen on the PR.
中文版本
第 4 轮复核 —— 真实构建、真实视频、真实请求线（15904802）
承接我在 3e178fdc 的第 3 轮评审与独立验证，我以维护者身份复核了 autofix bot 的第 4 轮提交 15904802。使用两个隔离 worktree（第 4 轮 vs 第 3 轮），各自独立 npm ci + npm run build；驱动前均确认各自 bundle 携带本 head 的源码字符串。macOS，Node 22.23.1。
结论：两个被标为合并阻塞的发现（1、2）与 MIME 清理项（8）确已修复，未引入回归，214/214 聚焦测试通过。可以合并。
上一轮发现 → 在 15904802 的状态
| # | 上一轮发现 | 级别 | 第 4 轮状态 |
|---|---|---|---|
| 1 | home 相对路径 ~/… 视频被死路式报错拒绝 |
中 | ✅ 已修复 —— 在 readPathFromWorkspace 前经 expandHomeDir(video.source)；现能附加（请求线已证明） |
| 2 | 首 token 以 .mov 结尾的普通文本被劫持且无回退 |
中 | ✅ 已修复 —— 读取失败现回退到通用文本路径（submit_prompt） |
| 8 / v3 | learn-command.ts 的 MIME 回退不可达且可能误标 |
低 | ✅ 已处理 —— 限定为单个 inline part；多 part 目录绝不被误标；保留为带护栏的回归测试 |
| 3 | Gemini/Vertex 原生支持视频却被拦截 | 中 | ⏸ 延后（provider 架构决策）—— 仍然存在 |
| 4 | 本地实际上限约 7.4 MB 而非 10 MB | 中 | ⏸ 延后（共享 fileUtils 报错）—— 仍然存在 |
| 5 | YouTube 硬拒绝移除了原有兜底能力 | 中 | ⏸ 延后（产品决策）；提示语已改写 |
| 6 | 内联视频常驻会话历史 | 低 | ⏸ 延后（共享 offload 基础设施）—— 仍然存在 |
| 7 | 非 file:// scheme 被误判为本地路径 |
低 | ✅ 已软化 —— 现降级到文本路径而非令人困惑的硬错误 |
| v1 | 含空格路径静默降级 | — | ⏸ 延后（纯字符串解析）—— 仍然存在 |
| v2 | 被 git-ignore 的视频报错无原因 | — | ⏸ 未改动 —— 仍然存在（第 9 行现场复现） |
| v4 | managed-memory 分类器 turn 会重发视频 | 背景 | ⏸ 既有行为 —— 已确认（每次 /learn 视频上线两次） |
验证方式
构建后的 CLI（packages/cli/dist/index.js）通过真实 pty（node-pty + @xterm/headless）驱动，对接一个记录每一次 /v1/chat/completions 请求体的 mock OpenAI 兼容端点，隔离 HOME，OPENAI_MODEL=qwen3-vl-plus（支持视频）—— 因此每个场景以进程实际发出的内容判定，而非屏幕文本。使用真实 macOS 系统视频作为素材（.mov 80 KB、.mp4 2.17 MB）；.m4v 为改名的 MP4 容器。
上方三张图分别为：请求线矩阵（3 个 ● 行即修复项，其余行均未变且正确）、发现 1 & 2 的 TUI 前后对比（第 3 轮死路、0 次请求；第 4 轮成功、2 次）、以及测试 A/B + 请求线 oracle + catch 作用域证明。
- 全 PR A/B（PR 测试文件覆盖 merge-base
e7097d0e源码）：42 个判别性失败（31 core + 11 cli）；真实 head 上214全通过。 - 第 4 轮增量 A/B（第 4 轮测试覆盖第 3 轮源码）：恰好 5 个失败 = 发现 1 + 发现 2 + 发现 8，别无其他 —— diff 只做了它声称的事。
- 请求线 oracle：从记录的请求体解码出的视频与磁盘文件逐字节一致（
sha256 d9855e7b…，80,315 B）。.m4v由fileUtils的 MIME 修复直接标为video/x-m4v（走真实链路，而非learn-command回退）—— 印证发现 8 的回退如今确为纵深防御。 - catch 作用域证明（第 4 轮改动中最值得复查的一点）：新的回退
catch仅作用于路径解析失败。真实诊断仍会呈现 —— 超限视频仍报File size exceeds the 10MB limit，不可读（mode-000）文件仍报EACCES: permission denied。回退不会过度吞掉错误。
关于第 4 轮取舍的一点观察（非阻塞）
回退对任何读取失败都触发，因此一个明显想指文件但拼错的单 token 路径 —— 例如 /learn ./cilp.mov（clip.mov 的拼写错误）或 workspace 外的绝对路径 —— 现在会静默变为交给模型"学习"的文本，而第 3 轮会提示 "Path not found in workspace: ./cilp.mov"。这是发现 2 修复的直接且有意的后果（评审要求以回退取代硬报错），对真正的普通文本而言是正确默认。若希望为明显的拼写错误保留可操作的诊断，后续可仅在输入看起来像普通文本（含空格 / 多于一个 token）时回退，而对裸单 token 路径保留硬错误。这不是合并阻塞项，仅提示让该取舍成为有意识的选择。
检查项
packages/cli——learn-command.test.ts17、i18nindex.test.ts+mustTranslateKeys.test.ts31 → 共 48 通过packages/core——learn-skill-agent.test.ts+fileUtils.test.ts197 通过- 两个 head 均构建干净；各自确认携带本 head 源码；PR 上 CI
Test (ubuntu-latest)与web-shell E2E Smoke均绿。
Verified locally by the maintainer at 15904802780bfe73db88dfa2de521d5f978c1b2c (merge-base e7097d0e). Screenshots rendered from the actual captured terminal + recorded wire.
6 participants
What this PR does
Adds a native-video path to
/learnfor local MP4, WebM, MOV, and M4V files and direct HTTP(S) video-file URLs. The command gates submission on the active model's video modality and a provider path that preserves video parts, then asks the main agent to create exactly one learned skill with a separate timestamped provenance record. YouTube watch pages are detected and rejected with local-download guidance instead of being passed to the provider as if they were media files. Existing text, document, directory, and webpage learning behavior remains unchanged.Why it's needed
The generic
/learnpath treats URLs as webpages and does not provide tutorial video bytes to a video-capable model. Local video could reach the provider through the generic file path, but it lacked a video-specific output and provenance contract. This change makes native video transport explicit, prevents silent submission through unsupported model/provider combinations, and keeps source-grounded knowledge distinct from execution-verified behavior.Reviewer Test Plan
How to verify
Configure an OpenAI-compatible video-capable model, run
/learn <local-video.mp4> <optional focus>, and confirm that the request contains the video plus the distillation prompt. The turn should create onelearned-skill-*directory containing onlySKILL.mdandreferences/source.md; the latter should use the exact statussource-grounded, not execution-verifiedand include timestamped evidence. Start a fresh session and confirm the generated skill is discoverable and can be invoked. Also confirm that a YouTube page URL is rejected before provider submission, a model without video input is rejected before submission, and ordinary text or document input still follows the existing path.Evidence (Before & After)
Before: a local tutorial video followed the generic file-learning path and produced only
SKILL.md; a YouTube watch page sent asvideo_urlproduced no provider result within five minutes. After: the same complete local MP4 produced exactlySKILL.mdplusreferences/source.md, and a fresh session explicitly invoked the learned skill. The generated browser demo rendered and reproduced the split-text transforms, but failed the final tutorial-fidelity check because its sweeping line used the menu as its containing block. The detailed E2E evidence is posted as a separate PR comment.Tested on
Environment (optional)
Local bundled CLI with an isolated project and QWEN_HOME, using
qwen3.5-omni-pluswith a 262144 context window and text/image/audio/video modalities enabled. The full fixture was a 6,533,189-byte, 895.2-second MP4 from the official Resource2Skill skill library.Risk & Scope
/learninputs retain the existing behavior.Linked Issues
N/A
中文说明
本 PR 做了什么
为
/learn增加原生视频路径，支持本地 MP4、WebM、MOV、M4V 文件和 HTTP(S) 视频文件直链。命令会在提交前检查当前模型是否声明视频输入能力，以及 provider 路径是否会保留视频 part；通过后，要求主 Agent 只创建一个 learned Skill，并额外写入带时间戳的独立来源记录。YouTube watch 页面会被识别并直接拒绝，提示用户先下载为本地视频，而不是把网页 URL 冒充媒体文件交给 provider。现有文本、文档、目录和普通网页学习行为保持不变。为什么需要
通用
/learn路径会把 URL 当网页处理，无法把教程视频 bytes 提供给支持视频的模型。本地视频虽然可以通过通用文件路径到达 provider，但缺少视频专用输出和 provenance contract。本改动显式定义原生视频传输，阻止不支持的模型/provider 组合静默提交，并把 source-grounded 知识与 execution-verified 行为区分开。Reviewer 测试计划
如何验证
配置一个 OpenAI-compatible 且支持视频输入的模型，运行
/learn <local-video.mp4> <optional focus>，确认请求包含视频和蒸馏 prompt。该 turn 应创建一个learned-skill-*目录，且只包含SKILL.md和references/source.md；后者的状态必须精确为source-grounded, not execution-verified，并包含带时间戳的证据。启动一个全新会话，确认生成的 Skill 可以被发现和调用。还应确认 YouTube 页面 URL 在 provider 请求前被拒绝、不支持视频的模型在提交前被拒绝、普通文本或文档输入仍走原有路径。证据（Before & After）
Before：本地教程视频走通用文件学习路径，只生成
SKILL.md；把 YouTube watch 页面作为video_url发送后，五分钟内没有 provider 结果。After：同一个完整本地 MP4 精确生成SKILL.md和references/source.md，全新会话显式调用了 learned Skill。生成的浏览器 demo 可以渲染，并复现切片文字位移，但最终教程忠实度验收失败，因为扫线把整个菜单作为 containing block。详细 E2E 证据会作为独立 PR 评论发布。测试平台
环境（可选）
本地 bundle CLI，使用隔离的 project 和 QWEN_HOME；模型为
qwen3.5-omni-plus，上下文 262144，启用 text/image/audio/video modalities。完整 fixture 来自 Resource2Skill 官方 Skill library，是一个 6,533,189 bytes、895.2 秒的 MP4。风险与范围
/learn输入保持现有行为。关联 Issue
N/A