feat(omni): S1 minimal path — local video via DashScope upload delivery - #8422
Conversation
Local @video files are recognized (magic-byte sniff + streaming SHA-256 + ffprobe metadata), promoted into a content-addressed object store under .qwen/omni/objects/, uploaded through the DashScope temporary upload channel (getPolicy + OSS multipart form POST), and delivered to the model as oss:// fileData parts resolved server-side via the X-DashScope-OssResourceResolve header. - omni pipeline gated on omni.enabled (or QWEN_CODE_ENABLE_OMNI=1) AND a DashScope-compatible endpoint AND an available API key; any other configuration falls back to the existing inline base64 behavior - ffmpeg/ffprobe are hard runtime prerequisites when omni is enabled: Config.initialize fails fast with an actionable FatalConfigError - upload failures fail closed with an explanatory error instead of silently downgrading to inline delivery - per-file ceiling omni.upload.maxFileBytes (default 1 GiB) replaces the 10MB inline cap for omni-delivered video - object store writes are atomic (.tmp + rename) and content-deduped; .qwen/omni/ is self-gitignored Part of the omni-experiment S1 slice (#8183).
Review round 1 findings addressed: - gate: exclude Qwen OAuth (its ContentGeneratorConfig carries the QWEN_OAUTH_DYNAMIC_TOKEN placeholder, unusable for the uploads endpoint), require an explicit baseUrl so the configured credential is never sent to a default origin, and require a trusted workspace before writing .qwen/omni/ or uploading workspace bytes - aborts: user cancellation propagates as the original abort error at every pipeline stage instead of being wrapped into a delivery failure - size ceiling: enforced from a cheap stat before any hashing/probing, and invalid (<=0) configured limits fall back to the default - object store: bytes are re-hashed while copying and verified against the object key (TOCTOU fail-closed); dedup hits re-hash the existing object and heal mismatched content; store directories refuse symlinks - error hygiene: messages that can reach model-visible content carry basenames only and a structured upstream error summary (status + code/message) instead of raw response bodies - getPolicy: validate the OSS ACL fields so a partial policy fails with a clear error instead of an opaque OSS 403 Part of the omni-experiment S1 slice (#8183).
- reuse the shared combineAbortSignals helper (with listener cleanup) instead of a local AbortSignal combinator - deduplicate the stream-SHA-256 helper: storage now imports hashFileSha256 from recognition - collapse probeBinary's callback plumbing into a Map-based availability cache - drop the unreachable video/x-matroska extension branch (the sniffer never emits that MIME type) - move the omni read-result shaping out of fileUtils into readVideoViaOmniDelivery so the file-reading hot path only carries the gate check and a single delegation call Part of the omni-experiment S1 slice (#8183).
Regression review of the previous two commits found: - upload.ts: cleanup() of the combined abort signal ran at headers-arrival time, leaving the subsequent response body read (getPolicy JSON / failure summary) unabortable — a stalled body would hang with neither the timeout nor ESC able to interrupt it. Body handling now happens inside the signal's lifetime. - storage.ts: the rename-race fallback could convert a user abort into a dedup success after an unabortable full-file re-hash; aborts now rethrow before the fallback, and the verify hash carries the signal. - storage.ts: healing a planted entry now removes directories too, not just files. Part of the omni-experiment S1 slice (#8183).
The @-resolution phase of a user query (file reads, and with omni enabled potentially minutes of media hashing + upload) ran while streamingState still reported Idle: no loading indicator appeared and cancelOngoingRequest early-returned, so Esc was silently swallowed for the whole preprocessing window. Track an isPreparingQuery state around prepareQueryForGemini for interactive user queries and feed it into streamingState. Preprocessing now renders the spinner with the esc-to-cancel hint, and Esc aborts the turn controller — the signal already flowed through handleAtCommand into the omni upload pipeline, which cancels cleanly (verified: no orphan temp files, no lingering sockets, session stays usable). Scoped to SendMessageType.UserQuery: notification/cron/teammate drains key off streamingState transitions and keep their existing behavior; /btw side-questions leave the main stream state untouched. Part of the omni-experiment S1 slice (#8183).
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
Thanks for the PR! Template looks good ✓ — all required sections present, bilingual body included. Problem: this is a scoped roadmap feature, not a speculative change. Tracking issue #8183 defines concrete acceptance criteria (startup validation, end-to-end Direction: aligned. This is the first vertical slice of the omni experiment, targeting the dedicated Size: core paths are touched ( Approach: scope matches the tracking issue 1:1 — recognition → content-addressed store → upload → Risk: one Stage-1e high-risk-path match — Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 所有必需章节齐全,含中文对照。 问题:这是路线图内的切片功能,不是臆测性改动。跟踪 issue #8183 定义了明确的验收标准(启动校验、 方向:对齐。这是 omni 实验的第一个纵向切片,目标是专用的 规模:触及核心路径( 方案:范围与跟踪 issue 一一对应——识别 → 内容寻址存储 → 上传 → oss:// 投递,其余明确延期(上传缓存、Qwen OAuth 支持、token 预检归 S2/S3)。新模块之外唯一值得关注的改动是 风险:命中一条 Stage-1e 高风险路径—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI wrote my independent proposal for this problem first (in the @-file path, detect video, stream it through DashScope's temporary upload channel, deliver the returned URL as a media part, gate the whole thing behind the experimental flag, and make sure cancel still works during a long upload) and then compared the diff against it. The PR matches that shape and goes further on the hardening side than I would have on a first pass. No blockers found. What I verified by reading the code against the
Non-blocking observations:
The runtime flow this PR introduces: sequenceDiagram
participant P1 as User
participant P2 as fileUtils
participant P3 as Omni gate
participant P4 as Recognize and store
participant P5 as Uploader
participant P6 as getPolicy API
participant P7 as OSS host
participant P8 as Model
P1->>P2: @video.mp4 in prompt
P2->>P3: gate check (enabled, trusted, static key, baseUrl, DashScope)
P3-->>P2: active
P2->>P4: sniff magic bytes, sha256, ffprobe metadata
P4->>P4: content-addressed putFile (atomic rename, hash verify)
P2->>P5: uploadFile (objectPath, model)
P5->>P6: getPolicy with Bearer key
P6-->>P5: policy plus upload_dir
P5->>P7: multipart form POST (streamed)
P7-->>P5: 200
P5-->>P2: oss:// URL
P2->>P8: fileData part with oss:// plus OssResourceResolve header
P8-->>P1: describes the video
Files changed (19 of 19 shown)
Testing evidenceThis is an unattended CI run — I did not build or execute any PR code; the evidence below is the PR's own CI on the reviewed commit, read through the API. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Context for the skips: on the What CI cannot settle here: the real-API behavior (an actual DashScope upload, the model describing the video) needs live credentials, ffmpeg, and a real video file. The PR body reports 13/13 real-API E2E scenarios and real-content validation on Sandboxed verification would settle what remains: 中文说明代码审查我先独立写下自己的方案(在 @ 文件路径识别视频、经 DashScope 临时上传通道流式上传、把返回的 URL 作为媒体 part 投递、整体置于实验开关门控之下、并保证长上传期间取消仍可用),再对照 diff。PR 与我的方案一致,且在加固方面做得比初稿更深。未发现阻塞项。 对照
非阻塞观察:
测试证据本次为无人值守 CI 运行——未构建、未执行任何 PR 代码;以上证据是所审提交上 PR 自身 CI 经 API 读取的结果。 跳过的说明: CI 无法覆盖的:真实 API 行为(真实 DashScope 上传、模型描述视频)需要线上凭证、ffmpeg 与真实视频。PR 正文报告 13/13 真实 API E2E 场景及 沙箱验证可以补齐剩余部分: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, heavily gated experimental slice with unusually thorough tests; the one reservation is that real-API behavior rests on the author's E2E evidence, and the Stage 2 comment names the lanes that would settle it. Stepping back: this PR does exactly what a first roadmap slice should do. My independent proposal for lifting the 10MB video ceiling was essentially the same pipeline — detect in the @ path, stream through the DashScope upload channel, deliver the URL, gate everything, keep cancel alive — and the implementation matches it while adding hardening I'd have deferred (content-verified storage, error sanitization, abort-through-body-reads). Nothing in the diff is decorative: every wiring edit maps to an acceptance criterion in the tracking issue, and the deferred work (upload cache, OAuth, token preflight) is explicitly S2/S3 rather than silently half-done. The honest reservations, both non-blocking: real-API behavior — the actual upload and the model comprehending the video — is the author's reported evidence, since CI here cannot hold live DashScope credentials; and the TUI state change touches a sensitive hook, though it's tightly scoped and the unit suite covering it is green. Neither outweighs the risk containment: this targets the experiment branch, defaults off, requires four further conditions, fails closed, and leaves the fallback path untouched. If this were aimed at Approving — the review found nothing to fix, and the gate philosophy's skepticism was exercised and ran out of objections on merit, not on fatigue. 中文说明置信度:4/5 —— 干净、门控严密的实验切片,测试覆盖超出常规;唯一保留是真实 API 行为依赖作者的 E2E 证据,Stage 2 评论已点名可补齐该证据的沙箱通道。 退一步看:这个 PR 正是路线图首个切片该有的样子。我独立构想的"解除 10MB 视频上限"方案与它基本同构——在 @ 路径识别、经 DashScope 上传通道流式上传、投递 URL、全链路门控、保持取消可用——而实现与我的方案一致,还补上了我本会推迟的加固(内容校验存储、错误净化、贯穿响应体读取的 abort)。diff 中没有装饰性改动:每处接线都对应跟踪 issue 的验收标准,延期项(上传缓存、OAuth、token 预检)明确归入 S2/S3,而非悄悄做一半。 两点诚实的保留,均不阻塞:真实 API 行为——真实上传与模型理解视频——是作者自述的证据,因为本 CI 无法持有 DashScope 线上凭证;TUI 状态变更触及一个敏感 hook,但其作用域收得很紧,覆盖它的单测全绿。两者都不足以压过风险收敛:目标是实验分支、默认关闭、还需满足四个条件、失败即关闭、回退路径原样保留。若这是 fork 直投 批准——审查未发现需要修改之处;门禁的怀疑主义已充分行使,反对意见是靠事实穷尽的,不是靠倦怠。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
What this PR does
Implements the S1 slice (tracer bullet) of the omni multimodal experiment: local video files referenced in a prompt are recognized (magic-byte sniff + streaming SHA-256 + ffprobe metadata), stored in a content-addressed object store under
.qwen/omni/objects/, uploaded through the DashScope temporary upload channel (getPolicy + OSS multipart form POST), and delivered to the model asoss://fileData parts resolved server-side via theX-DashScope-OssResourceResolveheader. This replaces the 10MB inline base64 ceiling with a 1 GiB upload-channel ceiling for video.Scope and design decisions are documented in the four design docs merged via #8110; the S1-specific implementation mapping lives in the tracking issue.
packages/core/src/omni/module: recognition, content-addressed storage (atomic.tmp+rename writes, hash-verified dedup, symlink refusal), DashScope uploader (streamed viaopenAsBlob, abort/timeout coverage through body reads), and the fileUtils-facing delivery wrapper.omni.enabled(orQWEN_CODE_ENABLE_OMNI=1) + trusted workspace + static API key (Qwen OAuth excluded — its config carries a placeholder token unusable for the uploads endpoint) + explicit DashScope-compatible baseUrl. Any failed condition falls back byte-for-byte to the existing inline behavior.Responding, so the spinner + esc-to-cancel work during it (previously Esc was silently swallowed and the UI looked idle).Why it's needed
First vertical slice of the omni roadmap (#8197): proves the recognition → storage → upload → delivery → answer pipeline end-to-end on the target model before the remaining slices (S2 input expansion, S3 upload caching/reliability, S4 policy orchestration, S5 memory) build on it. Real-model testing during this slice already produced design inputs for S2 (the 196608-token input ceiling binds far earlier than the 1 GiB byte ceiling for video).
Reviewer Test Plan
How to verify:
npm run build && npm run bundleexport QWEN_CODE_ENABLE_OMNI=1and point the CLI at a DashScope endpoint with a static API key (OPENAI_BASE_URL=https://dashscope.aliyuncs.com/compatible-mode/v1,OPENAI_API_KEY=<DashScope key>), model with video modality (e.g.-m qwen3.5-omni-plus).node dist/cli.js "@video.mp4 describe this video" --approval-mode yolo --output-format json --openai-logging --openai-logging-dir ./logs— the answer should describe the content;logs/*.jsonshowsvideo_url.urlstarting withoss://(notdata:);.qwen/omni/objects/contains one content-addressed object (re-running does not add another).QWEN_CODE_ENABLE_OMNIand re-run — behavior returns to inline base64 (>10MB is rejected as before).cd packages/core && npx vitest run src/omni/(46 tests).Evidence:
qwen3.5-omni-plus: PTCG battle video (36MB) — correct deck identification vs ground-truth transcripts; audio comprehension matches Whisper transcript nearly verbatim; duration boundary measured at 6–7 min (480p30) against the model's 196608-token input ceiling.Tested on:
Risk & Scope
omni.enableddefault false) and additionally gated on DashScope + static key + trusted workspace; with the gate closed every touched path is behavior-identical to main (covered by baseline-vs-post E2E comparison and the existing test suites: 513 affected unit tests pass).SendMessageType.UserQuerypreprocessing only; notification/cron/teammate drains and /btw keep their exact prior state transitions (regression-tested: 168/168 useGeminiStream + 141/141 AppContainer).Linked Issues
Closes #8183. Roadmap: #8197. Design docs: #8110.
中文说明
本 PR 做了什么
实现 omni 多模态实验的 S1 切片(最小通路):提示词中引用的本地视频文件经识别(magic-byte sniff + 流式 SHA-256 + ffprobe 元数据)后,存入
.qwen/omni/objects/内容寻址对象库,通过 DashScope 官方临时上传通道(getPolicy + OSS multipart 表单 POST)上传,并以oss://fileData part 投递给模型(请求携带X-DashScope-OssResourceResolveheader 由服务端解析)。视频的 10MB inline base64 上限由此替换为 1 GiB 上传通道上限。packages/core/src/omni/模块:识别、内容寻址存储(原子写入、哈希校验去重、拒绝 symlink)、DashScope 上传器(openAsBlob 流式、abort/超时覆盖到响应体读取)、面向 fileUtils 的投递封装。omni.enabled(或QWEN_CODE_ENABLE_OMNI=1)+ 受信任工作区 + 静态 API key(排除 Qwen OAuth——其配置携带的是占位 token,不适用上传端点)+ 显式的 DashScope 兼容 baseUrl。任一条件不满足即逐字节回落到现有 inline 行为。Responding状态,spinner 与 esc 取消在该阶段生效(此前 Esc 被静默吞掉、界面看似空闲)。为什么需要
Omni roadmap(#8197)的第一条垂直切片:在其余切片(S2 输入面扩充、S3 上传缓存与可靠性、S4 policy 编排、S5 memory)叠加之前,先在目标模型上端到端打通识别 → 存储 → 上传 → 投递 → 应答。本切片期间的真实模型测试已经为 S2 提供了设计输入(196608 token 输入上限远早于 1 GiB 字节上限成为约束)。
验证方式
见英文部分 Reviewer Test Plan:构建后以 DashScope 端点 + 静态 key + 视频模态模型运行
@video.mp4提问,核对请求日志中oss://URL、objects/ 去重、门控回落与启动校验;单测npx vitest run src/omni/(46 例)。证据:13/13 真实 API E2E 场景通过;目标模型
qwen3.5-omni-plus真实内容验证(PTCG 对战视频卡组识别与 Whisper 转写逐字级吻合的音频理解;实测 480p30 时长边界 6–7 分钟);三轮对抗性审查修复已应用。风险与范围
SendMessageType.UserQuery预处理;通知/cron/teammate 与 /btw 路径状态转换保持原样(回归测试通过)。关联
Closes #8183;Roadmap #8197;设计文档 #8110。