ci: host PR evidence outside Git - #9985
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Re-run at the author's request — gate re-checked at the current head rather than assumed from the first pass.
Moving on to code review. 🔍 中文说明应作者要求重跑——门禁在当前 head 上重新核验,而不是沿用首轮结论。
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-reviewed the full diff at the current head. No blocking issues. Every finding raised across the earlier rounds — mine, the
Verified against the base repo, not just the diff: all sparse-checkout entries exist ( Non-blocking, recorded:
Files changed (15 of 15 shown)
Testing evidence (PR's own CI, fetched via API — no PR code was executed)The decisive job is Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 One honest gap, named: both publishing paths live in workflow YAML that runs from the default branch ( Not verified: real OSS credentials/upload (post-merge only; no live OSS call was made in this review). Author-reported local results are their claim, not evidence here — the CI signal above is what this review relies on. 中文说明代码审查在当前 head 上重新审查了完整 diff。无阻塞问题。此前各轮提出的所有发现——我的、
对照基础仓库核实(而非只看 diff):所有稀疏检出条目均存在( 非阻塞,记录在案:
测试证据(来自 PR 自身 CI,通过 API 获取——未执行任何 PR 代码)决定性任务是 一个如实说明的缺口:两条发布路径都在默认分支运行的工作流 YAML 中( 未验证:真实 OSS 凭据/上传(仅合并后可能;本次审查未发起任何真实 OSS 调用)。作者自报的本地结果是其声明而非本审查的证据——本审查依赖的是上方的 CI 信号。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — every prior blocker is fixed and test-pinned at this head; one point withheld only because the decisive unit suite is still running and the live OSS wiring is exercisable only post-merge. Stepping back: this is what a good infrastructure PR looks like after seven review rounds and an independent human cross-check. The problem is real and I re-measured it — 755 What convinced me on this re-run is not the prose but the pinning: the round-7 Critical (dropped run-attempt segment) now has an executed regression test that rebuilds the exact collision this PR exists to prevent; the credential-residue blocker is closed by a runner pin a test will fail on regression; the self-targeting guard is exercised as real bash against disguised inputs rather than asserted as text. When the author says a re-run of the same workflow run keeps its run id and only increments the attempt, a test encodes that fact. Approval is deferred until CI lands green on 中文说明置信度:4/5 —— 当前 head 上此前所有阻塞项均已修复并被测试钉住;扣掉的一分仅因为决定性单测套件仍在运行,且 OSS 实际接线只能合并后演练。 退一步看:这是经过七轮审查与一次人工独立交叉核验后,一个优秀基础设施 PR 应有的样子。问题真实且我重新度量过——远端现有 755 个 这次重跑真正说服我的不是文字说明,而是钉扎:第 7 轮的 Critical(丢失的 run-attempt 段)现在有一个实际执行的回归测试,重建的正是本 PR 要防止的碰撞;凭据残留阻塞项由 runner 固定关闭,且有测试会在回退时变红;自指防护作为真实 bash 对各种伪装输入执行,而不是文本断言。作者说同一工作流运行重跑时 run id 不变、只有 attempt 递增,测试就把这个事实编码了下来。 批准推迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
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. ✅
Upload verification resultVerdict: PARTIAL PASS — the upload wiring is verified, but a live OSS upload has not been exercised yet.
The remaining gap is a real credential-backed upload. Both changed publishers are default-branch workflows ( So the precise answer to “is upload OK?” is: the code path, URL construction, validation, and degradation behavior are green; actual OSS credentials/network/public-read delivery still needs the first post-merge Web Shell visual publication or |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 10.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:反向审计——在 10 轮的反审轮数上限内未收敛。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
The publish-verify evidence upload resolved `node` through a PATH prefixed with $RUNNER_TEMP — the one directory PR code can write to via the verify container's bind mount — so a planted fake `node` there won interpreter resolution inside the step that holds CI_BOT_PAT and the OSS config (PATH hijack). Resolve node under the inherited clean PATH and run ossutil from a fresh job-private copy of the sha256-verified binary installed above instead of resolving either through $RUNNER_TEMP. Apply the same shape to web-shell-visuals-publish.yml so the pattern cannot regress if that job ever moves to a persistent pool. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Mirror qwen-triage.yml in web-shell-visuals-publish.yml: mark the ossutil install and credential-config steps continue-on-error so a setup failure cannot abort the no-image marker-comment path (the upload itself stays loud when images are present). In both publishers, derive the default ALIYUN_OSS_PUBLIC_BASE_URL from ALIYUN_OSS_BUCKET so overriding only one of the two vars cannot post comment links that 404 against (or show stale objects from) the other bucket. Re-record both workflow sizes in the baseline. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The single "Failed to upload evidence images" warning could not distinguish its causes: the ossutil install and credential-config steps run with continue-on-error and render green when they fail, so a missing binary or config looks identical to a transient upload blip or rotated keys. Probe both preconditions into the warning (ossutil=ok/MISSING, config=ok/MISSING) so the investigator sees the gap without trawling three steps of logs. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The evidence-hosting stub shifted --config away unvalidated, and every harness run sets VERIFY_ASSETS_UPLOADER, so the production `node "$uploader"` arm and its --config flag were never exercised: dropping --config from the workflow kept every test green while the real uploader exits 1 on it, silently degrading every /verify report to text-only. Make the stub reject a missing/empty --bucket, --config or --prefix, and pin the production invocation shape (inherited-PATH node resolution plus all three ossutil flags). Mutation-probed: with --config removed from the workflow both tests now fail. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Nothing pinned the credential lifecycle this PR adds — the sha256-checked ossutil install, the credential-config step, and the if:always() cleanup — so a future edit dropping any of them (or the always() condition) would leave every test green while the OSS key pair persists in $RUNNER_TEMP on the persistent ecs-qwen pool. Add shape assertions for all three steps in qwen-triage.yml and web-shell-visuals-publish.yml, mirroring the release/desktop wiring pins in install-script.test.js and desktop-oss-workflow.test.js. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The PR-review workflow now blanks a QWEN_REVIEW_ASSETS_REPO that equals the repository under review, but the user doc, the bundled review skill, and the parseAssetsRepo unset-error still recommended pointing it at the repo under review — so a maintainer following the published guidance landed in the "not set" refusal path, and the refusal message re-recommended the exact value the guard had just rejected. Update all three surfaces to the new contract: a dedicated external image-host repository (or a fork/scratch repo), with unset or self-targeting values deliberately degrading to prose and local artifact paths. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
The workflow-level assertions covered the OSS upload path with shape regexes only, so a --prefix that drifts away from RAW_BASE (every preview URL 404s) or a dropped --config line (the uploader hard-fails before any comment posts) both left the suite green. Extract the hosting block and run it against a stub uploader that mirrors the real flag contract and records the upload destination, in the same pattern as the verify path in scripts/tests/qwen-triage-workflow.test.js: the images arm must land the staged files under $STUB_ROOT/$ALIYUN_OSS_BUCKET/pr-assets/web-shell-visuals/<pr>/<sha> with the exact --bucket/--config/--prefix wiring, and RAW_BASE must equal that layout; the no-change arm must never invoke the uploader. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Prevent the persistent publish runner from executing a stale binary under RUNNER_TEMP when the best-effort install step fails. Pin both the install outcome gate and the hardened PATH invocation in focused workflow tests. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
scripts/tests/qwen-triage-workflow.test.js:3982 — [probe] the sha256-check pin cannot distinguish an enforced checksum from a || true-bypassed one (mutant survives both suites).github/scripts/web-shell-visuals-publish.test.mjs:98 — [probe] runHostingBlock leaks its mkdtemp fixture dir on every run (probe-measured +2 per run)
Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 11 (11 new). Findings keep coming back to the same files: .github/workflows/qwen-triage.yml (findings in round 2; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 11 条(其中 11 条首次提出)。发现反复回到同一批文件:.github/workflows/qwen-triage.yml(第 2 轮已出过发现,本轮又有 2 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- triage-side trailing-slash-strip test gap — already reported as R2-11 (comment 3855148331)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Convergence: round 4 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: .github/workflows/qwen-triage.yml (findings in round 3; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛情况:第 4 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:.github/workflows/qwen-triage.yml(第 3 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply 中文说明👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 |
The release syncs download ossutil with no retry, which is fine there: a red run is rerun by hand. Here the consequence differs. A CDN blip in the web-shell publisher leaves no ossutil for the upload, and since the upload aborts the step under set -e, the PR loses its preview comment entirely rather than degrading; in the verify publisher it silently costs the report its evidence images. Three retries with --retry-all-errors covers the transient class without changing any success path. Also reword the unset-QWEN_REVIEW_ASSETS_REPO refusal. It ended with a flat claim about the PR-review workflow blanking a self-targeting designation, which reads as a non sequitur to the far more common reader: someone running the CLI locally who simply never set the variable. Keep the diagnostic — a maintainer whose repository variable IS set needs it — but scope it to CI and say why self-targeting is discouraged in the first place.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- publisher sparse-checkout lists pinned by no test — already recorded in rounds 5 and 6 (reviews 5030238908 and 5031975486)
- bucket→public-URL default derivation pinned by no test — already recorded in rounds 5 and 6 (reviews 5030238908 and 5031975486)
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-triage.yml:5001 (+2 locations) — [probe] Install curl retry budget (~20min) exceeds 10min job capscripts/tests/qwen-triage-workflow.test.js:4132 — [review] New curl retry flags pinned by no test.github/workflows/qwen-triage.yml:5465 — [probe] Evidence upload has no timeout; stall kills whole report.github/workflows/qwen-triage.yml:4976 — [review] Sparse checkout pins no ref; trust rides a distant gate
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
收敛姿态下延后(第 7 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
The workflow_run id is stable across re-run attempts, so a maintainer re-run of the visuals publish overwrote the exact object keys an already-posted comment references (camo keeps rendering attempt-1 shots). Append run_attempt to the prefix, mirroring the verify lane, and cover same-runId attempt-1-vs-attempt-2 in the hosting harness. Co-authored-by: Qwen-Coder <qwen-coder @alibabacloud.com>
|
@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: 658 passed · 1 failed · 659 total Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:658 通过 · 1 失败 · 659 总计 抖动门:✅ 3 changed test file(s) x 5 identical rounds, no divergence Verification reportreportCentral claim proven: see A/B table. Flakiness gate logEvidence images20 additional image(s) did not pass the hosting checks (PNG magic, unique sanitized name, ≤2 MB, max 8) and remain in the run artifacts. 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. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
- The web-shell visuals prefix gains the run attempt: a re-run keeps the run id and only increments the attempt, so the old prefix let a same-head re-run write back over the object keys an already-posted comment references (camo keeps serving the stale screenshots). Mirrors the verify lane <run-id>-<attempt>; the re-run test now models same-run attempt 1 vs 2, not just distinct run ids. - The QWEN_REVIEW_ASSETS_REPO self-targeting guard now trims and compares case-insensitively before the CLI reads the value, so padded or case-shifted self-references degrade like an unset one. - Both publisher checkouts pin ref: github.sha (the default-branch head on their events) instead of relying on default resolution. - The ossutil download retry budget fits the 10-minute job cap in both publishers (~6.1 min worst case; was ~20 min). - Upload attempts are bounded via OSS_UPLOAD_ATTEMPT_TIMEOUT_MS so a stalled ossutil degrades to the text-only report / re-triggerable publish instead of burning the job cap; release syncs stay unbounded (the knob is opt-in). - Fix the serving-model comment (the extension allowlist, not the magic bytes, is what pins the served Content-Type) and pin what earlier suites left unpinned: sparse-checkout lists, bucket-to-URL derivation, curl retry flags, persist-credentials, and the ossutil isolation lines. Both uploader stubs now require the --config file to exist, so config drift turns the harnesses red. - runHostingBlock tears down its mkdtemp fixture instead of leaking it on every call. # Conflicts: # .github/scripts/web-shell-visuals-publish.test.mjs # .github/workflows/web-shell-visuals-publish.yml # docs/design/2026-08-25-pr-evidence-oss-hosting.md
chiga0
left a comment
There was a problem hiding this comment.
Round-2 re-review (prior head cf6994e2e8e8, round 1)
Prior findings — status at current head
| id | prior severity | file | summary | status |
|---|---|---|---|---|
| R1-1 | Blocker | qwen-triage.yml |
OSS credential file written to $RUNNER_TEMP on persistent ecs-qwen pool; bound-mount shared with verify job executing PR code |
Fixed — publish-verify now runs on ubuntu-latest (ephemeral, not shared); ecs-qwen pool removed from runs-on |
All prior Suggestion-level findings from other reviewers (PATH-hijack hardening, credential lifecycle, stub-contract tests) confirmed fixed at this head.
CI
| check | conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | pending |
| Test (macos-latest, Node 22.x) | skipped |
| Test (windows-latest, Node 22.x) | skipped |
| Integration Tests | skipped |
| Desktop Shell (ubuntu-22.04) | success |
| Desktop Shell (windows-2022) | success |
| Secret scan (TruffleHog) | success |
| Dependency CVE audit | success |
Test (ubuntu-latest) is still pending at review time; no confirmed pass. macOS and Windows test jobs are skipped for this PR.
New finding (minor)
Inline comment filed on web-shell-visuals-publish.yml line 96 — see below.
Scope
Reviewed: web-shell-visuals-publish.yml (full OSS hosting path, credential lifecycle, cleanup) · qwen-triage.yml publish-verify (collect_and_host_evidence function, install/configure/cleanup steps) · qwen-code-pr-review.yml (self-targeting guard, shell normalization) · scripts/upload-aliyun-oss-assets.js (timeout handling, ETIMEDOUT check).
Checked: ETIMEDOUT handling in uploadWithRetry (spawnSync timeout → result.error.code === "ETIMEDOUT", skips re-throw, falls through to retry, exhausts attempts then throws) · OSS prefix uniqueness (<head-sha>/<run-id>/<run-attempt> in web-shell; pr<pr>-<run-id>-<attempt> in verify) · collect_and_host_evidence else-branch has return 0 (empty upload_cmd never reaches the upload call) · self-targeting guard two-layer coverage (YAML expression + shell trim+case-fold, tested) · contents: read permission present on publish-verify · credential cleanup if: always() in both workflows.
Not reviewed: execution rungs 1-3 (no toolchain in this environment). Test (ubuntu-latest) result not yet in.
Reviewed with AI assistance.
The Install ossutil step keeps continue-on-error: true so a failed install must not block the no-image marker-comment path, but the unguarded Configure Aliyun OSS credentials step still ran and failed confusingly on the missing binary. Give the install step the install-ossutil id and gate Configure on its success outcome, matching the qwen-triage.yml twins. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@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: 758 passed · 1 failed · 759 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:758 通过 · 1 失败 · 759 总计 抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9985 deep verification — round 2 (follow-up)Verdict: 中文摘要判定:
Previous-finding status (follow-up round)The round-1 snapshot (
Carried-forward measurements were all re-run, not diffed: all five suites, the Central claim and A/B proofCentral claim: the two automated PR-evidence publishers stop writing image Replay method: the verify publisher (qwen-triage.yml) — witness
|
| cell | arm | environment | oracle | result |
|---|---|---|---|---|
| B1 calibration | base | stub git; run id 33055091624, attempt 1, 28 fixtures (8 valid) | section == previous-report.md section; push to pr-assets/9985-verify |
✅ byte-identical (1271 B content); branch pushed |
| B2 push fails | base | stub git: push=1, pull=1 | exit 0; empty section; "Failed to push evidence images; posting a text-only report." | ✅ |
| H1 happy | head | real node + real uploader; stub ossutil at $RUNNER_TEMP/ossutil; .ossutilconfig present |
exit 0; 8 objects in fake OSS under qwen-code-assets/pr-assets/verify/pr9985-33055091624-1/, bytes == fixtures; zero git invocations (poison); argv cp <stage> oss://<bucket>/<prefix>/<name> -c <config> -f --acl public-read; ossutil PATH excludes $RUNNER_TEMP; stderr clean |
✅ all |
| H2 install failed | head | OSSUTIL_INSTALL_OUTCOME=failure, no binary, no config |
exit 0; empty section; warning OSS publishing unavailable (ossutil=MISSING, config=MISSING); uploader never invoked |
✅ |
| H3 config missing | head | install success + binary, no config | exit 0; empty section; (ossutil=ok, config=MISSING) |
✅ |
| H4 upload fails | head | stub ossutil cp exits 1 | exit 0; empty section; Failed to upload evidence images (ossutil=ok, config=ok); uploader retried first image exactly 3× then stopped (6.1 s wall = 2 s + 4 s backoffs) |
✅ |
| H5 re-run attempt 2 | head | GITHUB_RUN_ATTEMPT=2 |
prefix …pr9985-33055091624-2 in section and object keys; attempt-1 keys untouched |
✅ |
| H6 no images | head | no verify-results | exit 0; empty section; uploader never invoked | ✅ |
web-shell visuals publisher — witness 02-ab-visuals.png
| cell | arm | oracle | result |
|---|---|---|---|
| V1 happy | head | prefix pr-assets/web-shell-visuals/9985/<sha>/4400/1; 2 objects in fake OSS; production invocation (no seam); ossutil PATH excludes $RUNNER_TEMP; zero git calls |
✅ |
| V2 attempt 2 | head | same run id → distinct prefix (…/4400/2), attempt-1 keys untouched (camo-cache immutability) |
✅ |
| V3 no-change | head | HAS_IMAGES=0 → uploader never invoked, RAW_BASE empty |
✅ |
| V4 install failed, images present | head | loud abort (nonzero exit, no URL announced) — the accepted tradeoff, tracked upstream as #10240 | ✅ expected failure observed |
| V5 happy | base | force-push HEAD:pr-assets/web-shell-visuals-9985 to the token URL; RAW_BASE = raw.githubusercontent pinned to the snapshot SHA; images staged into the orphan workdir |
✅ |
| V6 push fails ×3 | base | exit 1 with ::error::Failed to push web-shell visuals |
✅ expected failure observed |
A/B verdict: the mechanism flips completely from Git refs (base: branch push in
both publishers) to OSS prefixes (head: zero git invocations on either path),
and the degradation contract holds on every failure cell.
Supporting harnesses
- Uploader wire (
03-uploader-wire.png, 20/20): exact ossutil argv and
object keys; retry ladder (transient fail → success, 2 s backoff; permanent
fail → exit 1 after exactly 3 attempts, ~6 s); the new
OSS_UPLOAD_ATTEMPT_TIMEOUT_MScap SIGKILLs a stalled attempt at 800 ms,
surfaces(timed out)twice, exhausts in 8.5 s instead of hanging; invalid
values (abc,-5,1.5) rejected with a clear message; unset env =
unbounded (release-sync compatibility), 3000 ms cap above a 1.5 s natural
duration passes (control). - Bucket/base-URL ladder (
05-expr-ladder-review-guard.png, 20/20): the
exact${{ }}expressions extracted from both publishers are byte-identical
across the two workflows and, evaluated across all 16 var combos, resolve
ALIYUN_OSS_PR_ASSETS_BUCKET → ALIYUN_OSS_BUCKET → qwen-code-assetswith the
base URL always derived from the winning bucket; with neither PR knob set the
resolution equals the shared-bucket wiring (today's behaviour); setting only
the bucket knob moves the derived base URL with it. - Review self-target guard (same witness): env-level expression blanks the
exact self value and passes external repos; the extracted bash block
(trim + case-fold) blanks padded and case-shifted self-references
(" QwenLM/qwen-code ",QWENLM/QWEN-CODE) and keeps/trim-keeps external
designations — 7/7 driven cases. - Sparse checkout + ESM (
04-sparse-esm.png, 14/14): real scratch repo +
the publishers' exact no-cone patterns land the root/package.json,
uploader and helper while excludingpackages/*/package.jsondecoys;
node scripts/upload-aliyun-oss-assets.js --helpexits 0 inside the sparse
checkout. Without the/package.jsonpattern the uploader fails with
Cannot use import statement outside a moduleon a non-detecting Node
(--no-experimental-detect-module), while this container's node (v22.23.2,
detection on by default) infers ESM — confirming the version-dependent
failure mode the pattern removes. - Live step execution: the verbatim
Install ossutilstep ran against the
real CDN in this container: sha256dcc512e4…verifiedOK, 11 304 833-byte
binary installed 0755 at$RUNNER_TEMP/ossutiland executed. The real binary
accepted the Configure step's exact argv (config -e … -i … -k … -L EN -c …,
dummy creds → config file written). Install steps are identical across the two
publishers modulo comments; Configure steps byte-identical. - Structural checks: both publisher jobs run on
ubuntu-latest(off the
persistent ECS pool), 10-minute caps,permissions: contents: read + pull-requests: write(verify), sparse checkout pinned togithub.shawith
persist-credentials: false, and analways()cleanup removing
$RUNNER_TEMP/.ossutilconfigin both jobs.
Targeted gates (all at head 20ea50eb56)
| gate | result |
|---|---|
node --test .github/scripts/web-shell-visuals-publish.test.mjs |
34/34 pass |
vitest run scripts/tests/qwen-triage-workflow.test.js |
155/155 pass (31 s) |
vitest run scripts/tests/upload-aliyun-oss-assets.test.js + workflow-size.test.js |
211/211 pass (validates the updated .size-baseline) |
vitest run scripts/tests/qwen-pr-review-workflow.test.js |
177/177 pass — the 2 failures the PR body reports on the author's sandbox are the root-user chmod fixture artifact: this container runs uid 1000 and they pass |
vitest run packages/cli/src/commands/review/lib/assets.test.ts |
62/62 pass (the only TS change is an error-message rewording; the suite pins the /QWEN_REVIEW_ASSETS_REPO/ shape, not the prose — appropriate for a cosmetic change) |
| prettier --check on all 13 changed text files | clean |
| actionlint (all workflows) | clean (exit 0) |
| shellcheck (repo gate) | clean — all remaining style warnings are in scripts/test-rewind-e2e.sh, untouched by this PR |
bash -n on all four extracted step scripts |
clean |
| flakiness | suites re-executed 2–5× across baseline + mutation rounds with zero divergence |
Mutation matrix (vacuity of the PR's own tests) — witness 06-mutation-matrix.png
Each mutant reverts one guard the PR introduces, runs the suite that should
catch it, then restores the file (restore verified by git status + content
hash every time). Unmutated controls were green for all four suites.
| mutant | guard reverted | suite | result |
|---|---|---|---|
| M1 | drop --config from the publish-verify uploader invocation |
triage | KILLED (2 tests: "pins the production uploader invocation and its ossutil flags", "hosts only valid…degrades to text") |
| M2 | drop the Configure-step install-outcome gate (visuals) | visuals | SURVIVED → adjudicated, see F1 |
| M3 | drop /${RUN_ATTEMPT} from the visuals prefix |
visuals | KILLED (same-file positive control for M2's file/suite) |
| M4 | case-fold self-target comparison disabled (tr→cat) |
review | KILLED ("normalizes whitespace and case variants of the assets-repo designation") |
| M5 | revert the ETIMEDOUT retry tolerance in the uploader |
uploader | KILLED ("kills a stalled attempt and retries it when attemptTimeoutMs is set") |
| M6 | drop -${GITHUB_RUN_ATTEMPT:-1} from the verify prefix |
triage | KILLED (2 tests) |
5/6 killed. The M2 survivor was escalated before being reported: a patched
visuals test adding the gate assertion is green on head (34/34) and red
under M2 (not ok 5 - workflow pins the ossutil credential lifecycle), so
the axis is pinnable and the survival is a real gap, not a harness artifact.
Findings
F1 — Suggestion: the visuals Configure install-outcome gate is unpinned (coverage gap)
What: the head commit 20ea50eb56 added
if: "${{ steps.install-ossutil.outcome == 'success' }}" to the Configure Aliyun OSS credentials step of web-shell-visuals-publish.yml. Mutant M2
(delete that line) leaves web-shell-visuals-publish.test.mjs fully green —
the suite pins the step's continue-on-error and -c "$RUNNER_TEMP/.ossutilconfig"
but not the gate. The twin gate in qwen-triage.yml is pinned
(scripts/tests/qwen-triage-workflow.test.js:4160), and the commit that
introduced the lifecycle pins says it pins "the install outcome gate … in
focused workflow tests" — true for triage, not for visuals.
Severity: low. Behavior at head is correct (gate present; removal is also
masked downstream — the visuals upload block hard-fails on the missing binary,
and the verify publisher re-checks both preconditions, A/B cells H2/H3), so
nothing is exploitable or broken today. The risk is regression-shaped: a future
edit deleting the gate would leave every test green while re-introducing the
"configure/run against a stale or missing ossutil" pattern the PR explicitly
hardens against.
Reproduce:
# delete the gate, run the suite, observe green:
sed -i '/if: "\${{ steps.install-ossutil.outcome == .success. }}"/d' \
.github/workflows/web-shell-visuals-publish.yml # (line 98 at head)
node --test .github/scripts/web-shell-visuals-publish.test.mjsSuggested fix (measured)
In .github/scripts/web-shell-visuals-publish.test.mjs, inside workflow pins the ossutil credential lifecycle…, after the existing configure assertions:
assert.match(
configure,
/if: "\$\{\{ steps\.install-ossutil\.outcome == 'success' \}\}"/,
);Measured in a scratch copy: with the patch, head runs 34/34 green; with the
patch + M2 mutant the suite goes red on exactly not ok 5 - workflow pins the ossutil credential lifecycle. Both files restored afterwards. The rest of the
suite's counts are unchanged with the patch, so it pins only this axis.
Observations (not findings)
- V4 loud abort is an accepted tradeoff, confirmed: with images staged but
no ossutil binary, the visuals publish step aborts instead of posting —
the PR states this ("the upload itself stays loud when images are present")
and tracks the comment-loss consequence as follow-up follow-up(ci): a failed visuals upload drops the whole web-shell preview comment (from #9985) #10240. My V4 cell
reproduces the shape (nonzero exit, no URL) end-to-end. - The PR body's suite counts (151+1 skipped, 206, 170+2+2) predate the last
commits; the head counts above supersede them. Not a discrepancy worth
action.
Not covered
- Per-commit attribution: checkout is depth 2 (only merge commit + both
parents;git rev-parse --is-shallow-repository= true,rev-list HEAD^1..HEAD^2returns the grafted1vs 21 commits in the metadata).
Verified the aggregateHEAD^1..HEADdiff only. - Head-arm calibration against a real published OSS comment: none exists
yet (this PR introduces the path). Would be calibrated by the first real
publish run post-merge. Base arm is calibrated (B1). - Live OSS upload (real bucket): no OSS credentials exist in this sandbox
by design; uploads were proven against a fake ossutil that mirrors the
binary's argv contract, plus the real binary's argv acceptance forconfig. - yamllint: not installable in this container (
pip3absent); actionlint
covers the workflow YAML shapes instead. - Repo-wide ESLint / full
npm run typecheck: out of budget/scope; the
only TS change is covered by its suite, and the cli package build (run at
head before this round) compiled it. - Flakiness gate as a formal 5-round run: owned by the workflow; my
repeated suite executions (2–5× per suite across baseline and mutation
rounds) showed zero divergence. - OSS bucket-side accept path (bucket policy, camo proxy caching behavior):
external to this sandbox. - The working tree arrived with an uncommitted revert of this PR's
.qwen/skills/verify-pr/SKILL.mdwording hunk (lane setup artifact); left
untouched; it does not affect any verified behavior.
Methodology
Environment: the CI verify container itself (node:22-bookworm equivalent;
node v22.23.2, uid 1000, RUNNER_TEMP=/__w/_temp in-job) — the same runtime
the lanes use, so image facts (no zstd, no preinstalled linters) were measured
here. Harnesses (harness/*.mjs) parse the workflow YAML with the repo's
yaml package, extract run blocks verbatim, and drive them under the step's
own shell contract (bash --noprofile --norc, set -euo pipefail) with
stubbed edge binaries (git/ossutil) — the unit under test (workflow bash, the
real uploader script, the real ossutil binary where noted) is never mocked.
Raw per-cell logs live in logs/ (suite outputs, mutation matrix JSON,
install-step log), per-cell fixtures/outputs under cells*/. All mutated
files were restored and verified after each mutant. Verification head:
20ea50eb56740b70fe5faab9c120c88d3e5772ef.
Flakiness gate log
rounds=5 files=4 skipped=0
file .github/scripts/web-shell-visuals-publish.test.mjs: (cd .) node --test ./.github/scripts/web-shell-visuals-publish.test.mjs
file scripts/tests/qwen-pr-review-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-pr-review-workflow.test.js
file scripts/tests/qwen-triage-workflow.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/qwen-triage-workflow.test.js
file scripts/tests/upload-aliyun-oss-assets.test.js: (cd .) npx --no-install vitest run --config ./scripts/tests/vitest.config.ts ./scripts/tests/upload-aliyun-oss-assets.test.js
per-file results (P=pass F=fail I=infra-exit, one letter per run):
.github/scripts/web-shell-visuals-publish.test.mjs: PPPPP
scripts/tests/qwen-pr-review-workflow.test.js: PPPPP
scripts/tests/qwen-triage-workflow.test.js: PPPPP
scripts/tests/upload-aliyun-oss-assets.test.js: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · .github/scripts/web-shell-visuals-publish.test.mjs: P (exit 0)
round 1 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 1 · scripts/tests/qwen-triage-workflow.test.js: P (exit 0)
round 1 · scripts/tests/upload-aliyun-oss-assets.test.js: P (exit 0)
round 2 · .github/scripts/web-shell-visuals-publish.test.mjs: P (exit 0)
round 2 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 2 · scripts/tests/qwen-triage-workflow.test.js: P (exit 0)
round 2 · scripts/tests/upload-aliyun-oss-assets.test.js: P (exit 0)
round 3 · .github/scripts/web-shell-visuals-publish.test.mjs: P (exit 0)
round 3 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 3 · scripts/tests/qwen-triage-workflow.test.js: P (exit 0)
round 3 · scripts/tests/upload-aliyun-oss-assets.test.js: P (exit 0)
round 4 · .github/scripts/web-shell-visuals-publish.test.mjs: P (exit 0)
round 4 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 4 · scripts/tests/qwen-triage-workflow.test.js: P (exit 0)
round 4 · scripts/tests/upload-aliyun-oss-assets.test.js: P (exit 0)
round 5 · .github/scripts/web-shell-visuals-publish.test.mjs: P (exit 0)
round 5 · scripts/tests/qwen-pr-review-workflow.test.js: P (exit 0)
round 5 · scripts/tests/qwen-triage-workflow.test.js: P (exit 0)
round 5 · scripts/tests/upload-aliyun-oss-assets.test.js: P (exit 0)
Evidence images
194 additional image(s) did not pass the hosting checks (PNG magic, unique sanitized name, ≤2 MB, max 8) and remain in the run artifacts.
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
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed exact head 20ea50eb56740b70fe5faab9c120c88d3e5772ef. I did not find a new blocking defect in the OSS publishing, credential-isolation, untrusted-artifact validation, immutable-prefix, upload-timeout, or automated-review self-targeting paths. The focused workflow/uploader tests pass, and the current Ubuntu CI test is green.
The branch is nevertheless not approvable yet because GitHub currently reports it as CONFLICTING with main. A merge-tree check localizes the content conflict to .github/workflows/.size-baseline; the workflow files themselves auto-merge, but the ratchet entries need to be regenerated from the actual post-merge files while retaining the newer baseline entries already on main. Please update the branch, resolve that baseline against the resulting tree, and rerun the workflow-size/CI checks. I can verify and approve the new exact head after that.
Local verification: node --test .github/scripts/web-shell-visuals-publish.test.mjs (34/34), the OSS evidence test passes when isolated, uploader/workflow-size/review-workflow suites pass, Prettier and git diff --check pass. The combined triage suite hit four local 5-second timeout thresholds; all four pass with a 15-second timeout and the repository’s Ubuntu CI test is green.
…ts-to-oss Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Not explored to full depth (tool budget reached): "agent 1b": none — no check was cut short..
Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted.
Test Plan (not a blocker): 34 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed; 151 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed; 206 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed; 170 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
.github/workflows/qwen-triage.yml:5075 — [review] cross-region guidance comment names the wrong knob when ALIYUN_OSS_PR_ASSETS_BUCKET is setscripts/upload-aliyun-oss-assets.js:32 — [probe] env→main()→uploadAssets timeout glue exercised by no test (dropping the option ships green).github/workflows/qwen-triage.yml:5495 — [probe] verify-lane trailing-slash strip unpinned (mutant ships green).github/workflows/.size-baseline:44 — [review] size-baseline entries drift from committed sizes (350381≠350056; 20047≠20286).github/workflows/qwen-triage.yml:5083 — [probe] install (364s) + upload (366s/image, no aggregate bound) time budgets sum past the 10-minute job capscripts/tests/qwen-pr-review-workflow.test.js:1417 — [probe] normalization-fragment test pins no ordering — block relocated below the CLI launch ships green
中文说明
无阻断问题。LGTM!✅
未探索到全部深度(达到工具调用预算):"agent 1b":none — no check was cut short.。
未检查(工具限制,非阻断):the executable-script lint — .github/workflows/qwen-code-pr-review.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/qwen-triage.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-cleanup.yml: actionlint embedded-shell source mapping is not yet supported — not linted; the executable-script lint — .github/workflows/web-shell-visuals-publish.yml: actionlint embedded-shell source mapping is not yet supported — not linted。
Test Plan(非阻断):34 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed; 151 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed; 206 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed; 170 passed — this review observed 25163, 22033, 1770, 1667, 605, 4331, 639 passed。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改:共 6 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
Released in v0.23.0. |
















What this PR does
This PR stops automated PR evidence from adding image objects to the qwen-code Git repository. Web Shell visual previews and sandboxed
/verifyevidence now upload validated images to Aliyun OSS under immutable PR/run prefixes and embed the resulting public URLs. Automated reviews may still useqwen review publish-assetswith a dedicated external assets repository, but a destination that resolves toQwenLM/qwen-codeis rejected and degrades to prose and run artifacts.The legacy PR-close cleanup remains in place so historical
pr-assets/*refs continue to drain as PRs close.Why it's needed
The repository currently advertises thousands of branches, including hundreds of
pr-assets/*refs. The measured image refs account for roughly 374 MiB of reachable objects, almost entirely PNGs. Ordinary clones fetch objects reachable from remote branches, so evidence images increase clone traffic and repository storage even though they are unrelated to the source tree.Reviewer Test Plan
How to verify
pr-assets/web-shell-visuals/<pr>/<head-sha>/<run-id>/<run-attempt>prefix and contains no orphan checkout or Git push path. The run id and run attempt are load-bearing, not decoration: GitHub serves comment images through a caching proxy keyed by URL, so a re-run for the same head must not write back over the object keys an already-posted comment references — and a re-run keeps the same run id (only the attempt increments), so both segments are required. The Git-backed design got this free from the per-run commit SHA in the raw URL./verifypublisher preserves the image count, size, filename, duplicate-name, and PNG-magic checks; successful uploads usepr-assets/verify/pr<pr>-<run-id>-<attempt>. Three separate degradations must each still post a text-only report rather than block or abort it: a failedossutilinstall, a missing$RUNNER_TEMP/.ossutilconfig(the configure step iscontinue-on-error, so a rotated secret renders green there), and an upload that fails outright.ALIYUN_OSS_PR_ASSETS_BUCKET→ALIYUN_OSS_BUCKET→qwen-code-assets, with the public base URL derived from whichever bucket wins. With neither PR-assets variable set this is byte-for-byte the current behavior; setting one moves untrusted PR-derived evidence off the bucket that also serves release, desktop, and live-host downloads./package.json. The uploader is ESM in a.jsfile and neither publisher pins a Node version, so without the"type": "module"marker it only parses on the Node versions that infer module syntax — whileenginesdeclares>=22.0. The leading slash matters: it anchors the pattern to the root and keepspackages/*/package.jsonout of the checkout.QWEN_REVIEW_ASSETS_REPOand refuses a value equal to the current repository.Commands run locally:
Results on Linux / Node 22:
hosting block gives a re-run of the same head a fresh, non-colliding prefixandhosting block gives re-run attempts of the SAME run distinct prefixes), which execute the extracted hosting block against a stub uploader and assert no two runs share an object prefix while all stay under the same per-head path..size-baselineis updated for both workflows.fallback comment resilience (PR #8894 incident class)and are not caused by this change — they reproduce identically on the base commitf630c7f3d. They build an unwritable-directory fixture withchmod, which does not constrain the root user this sandbox runs as. They are expected to pass in CI.scripts/tests/qwen-triage-workflow.test.js(untouched lines) were formatted in passing./package.jsonsparse-checkout pattern was verified against a scratch repository containing a nestedpackages/cli/package.json.Evidence (Before & After)
N/A — CI workflow behavior only; no product UI changes.
Tested on
Environment (optional)
Node.js 22 with the repository's existing dependencies. Workflow YAML was parsed locally; no live OSS credentials were used.
Risk & Scope
/verifytreats uploader setup and upload as best-effort and preserves the text report on failure.releases/qwen-code/latest/VERSION. The jobs are hardened (hosted runner, no PR-code checkout, credential file removed in analways()step), so this is not directly exploitable, but the key is now reachable from many more workflow runs than before.ALIYUN_OSS_PR_ASSETS_BUCKETseparates the content; scoping a dedicated RAM user to thepr-assets/prefix separates the credential and is ops work outside this PR. Making the destination a variable rather than a constant is what leaves room for it.qwen review publish-assetsremains available for dedicated external assets repositories.ALIYUN_OSS_PR_ASSETS_BUCKET/ALIYUN_OSS_PR_ASSETS_PUBLIC_BASE_URLvariables fall back to the existing ones when unset. Operators should keepALIYUN_OSS_ACCESS_KEY_ID,ALIYUN_OSS_ACCESS_KEY_SECRET, and the existing OSS variables configured. If automated review images are desired,QWEN_REVIEW_ASSETS_REPOmust point to a dedicated external image-host repository rather thanQwenLM/qwen-code.Linked Issues
Follow-ups opened from this PR (both deliberately out of scope here, see Risk & Scope):
publish-assetshas no self-targeting guard for local runs. The automated lane is blanked by the workflow; the CLI path relies on the optional--reviewed-repoflag, so a real guard needs repository resolution that belongs in its own change.RAW_BASErenders as the false "no screenshot changes" message.中文说明
这个 PR 做了什么
这个 PR 会阻止自动化 PR 证据继续把图片对象写入 qwen-code Git 仓库。Web Shell 视觉预览和沙箱
/verify证据现在会把通过校验的图片上传到阿里云 OSS,使用不可变的 PR/运行前缀,并在评论中嵌入对应的公开 URL。自动 review 仍可通过qwen review publish-assets使用专用的外部图片仓库,但如果目标解析为QwenLM/qwen-code,工作流会拒绝该配置,并降级为文字说明和运行产物。旧的 PR 关闭清理仍然保留,因此历史
pr-assets/*ref 会在对应 PR 关闭时继续被删除。为什么需要
当前仓库公开了数千个分支,其中包括数百个
pr-assets/*ref。测量显示这些图片 ref 约占 374 MiB 的可达对象,几乎全是 PNG。普通 clone 会下载远端分支可达的对象,所以这些与源码无关的证据图片会增加 clone 流量和仓库存储。Reviewer 测试计划
如何验证
pr-assets/web-shell-visuals/<pr>/<head-sha>/<run-id>/<run-attempt>,并且不再包含 orphan checkout 或 Git push 路径。其中 run id 和 run attempt 都不是装饰:GitHub 通过按 URL 缓存的图片代理渲染评论中的图片,因此同一个 head 重跑时绝不能覆写已发布评论所引用的 object key——而重跑时 run id 保持不变、只有 attempt 递增,所以两段都必须存在。旧的 Git 方案是靠 raw URL 里的 per-run commit SHA 天然获得这个性质的。/verify发布器保留图片数量、大小、文件名、重名和 PNG magic 校验;上传成功时使用pr-assets/verify/pr<pr>-<run-id>-<attempt>。以下三种降级都必须仍然发布纯文本报告,而不是阻塞或中断报告:ossutil安装失败、缺少$RUNNER_TEMP/.ossutilconfig(configure 步骤是continue-on-error,所以密钥轮换后它仍然显示为绿色)、以及上传本身失败。ALIYUN_OSS_PR_ASSETS_BUCKET→ALIYUN_OSS_BUCKET→qwen-code-assets解析目标桶,public base URL 从最终胜出的桶推导。两个 PR-assets 变量都不设时,行为与当前完全一致;设置其一即可把未受信的 PR 产出证据从同时服务 release、desktop、live-host 下载的那个桶里分离出去。/package.json。上传器是.js文件中的 ESM,而两个发布器都没有 pin Node 版本,因此缺少"type": "module"标记时它只能在会推断模块语法的 Node 版本上解析——而engines声明的是>=22.0。前导斜杠是必需的:它把匹配锚定到根目录,避免把packages/*/package.json一并 checkout 进来。QWEN_REVIEW_ASSETS_REPO,并拒绝与当前仓库相同的值。本地执行的命令:
Linux / Node 22 上的结果:
hosting block gives a re-run of the same head a fresh, non-colliding prefix与hosting block gives re-run attempts of the SAME run distinct prefixes)——把抽取出来的 hosting 代码块对着 stub 上传器执行,断言任意两次运行都不会共用同一个 object prefix,同时全部仍挂在同一个 per-head 路径下。.size-baseline已同步更新。fallback comment resilience (PR #8894 incident class),与本次改动无关——在 base commitf630c7f3d上同样复现。它们用chmod构造“不可写目录”的 fixture,而这个沙箱以 root 运行,chmod 对 root 无效。预期在 CI 中通过。scripts/tests/qwen-triage-workflow.test.js中 3 处原有的格式违规(并非本次改动的行)顺带一并格式化了。/package.json这个 sparse-checkout 匹配模式也在一个含有嵌套packages/cli/package.json的临时仓库上验证过。证据(Before & After)
不适用——仅变更 CI 工作流行为,没有产品 UI 变化。
测试平台
环境(可选)
Node.js 22 和仓库现有依赖。本地解析了工作流 YAML;没有使用真实 OSS 凭据。
风险与范围
/verify会把上传器安装和上传视为 best-effort,失败时保留文本报告。releases/qwen-code/latest/VERSION的 key。这两个 job 本身是加固过的(hosted runner、不 checkout PR 代码、凭据文件在always()步骤中删除),所以并不能被直接利用,但这把 key 现在能被触达的工作流运行次数远多于以前。ALIYUN_OSS_PR_ASSETS_BUCKET分离的是内容;给pr-assets/前缀单独开一个 RAM 子账号才能分离凭据,那属于本 PR 之外的运维工作。把目标做成变量而不是常量,正是为了给它留出空间。qwen review publish-assets仍可用于专用外部图片仓库。ALIYUN_OSS_PR_ASSETS_BUCKET/ALIYUN_OSS_PR_ASSETS_PUBLIC_BASE_URL在未设置时会回退到现有变量。运维需要继续配置ALIYUN_OSS_ACCESS_KEY_ID、ALIYUN_OSS_ACCESS_KEY_SECRET和现有 OSS 变量。如果需要自动 review 图片,QWEN_REVIEW_ASSETS_REPO必须指向专用的外部图片托管仓库,而不是QwenLM/qwen-code。关联 Issue
从本 PR 拆出的后续事项(两者都刻意不在本 PR 范围内,见风险与范围):
publish-assets对本地运行的自指没有任何拦截。自动化那条路径已由工作流置空;CLI 路径只能依赖可选的--reviewed-repo参数,真正的守卫需要补仓库解析,应当独立成一个改动。RAW_BASE会渲染成"没有截图变化"这个与事实相反的说法。