fix(live): restore Live Host after desktop removal - #9994
Merged
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Collaborator
|
Thanks for the fast follow-up to PR 9085!
Moving on to code review. 🔍 中文说明感谢对 PR 9085 的快速跟进！
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Collaborator
Code reviewBefore reading the diff, my independent proposal for "the installer contract lives on but its producer was deleted with the desktop package" was exactly what this PR does: lift the app out of git history into a top-level The restoration fidelity checks out (spot-diffs against the pre-9085 tree quoted in the gate comment), and the workflow code is the strongest part of the PR: secrets flow through Blocking — CI is red on this commit, twice, both PR-caused:
Neither is deep; both are one-line-class fixes. But a PR whose own new CI gate fails on its own commit isn't mergeable as-is. Not verified, and deliberately so: signing, notarization, GitHub release publication, and the OSS upload run only behind the dispatch-only path with Files changed (24 of 68 shown)
…plus build scripts, resources, and remaining restored sources. CI evidence (this PR's own checks on the reviewed commit)Quoted from the check-runs API for the commit below — not re-run locally, per the review's static-only rule. The two failures are deterministic and PR-attributed (module missing from the new standalone lockfile; new workflows missing from the size ratchet), not infra noise: both error messages name artifacts this PR introduces. Log excerpt from the Live Host job:
Real-scenario testing: N/A for this run — unattended CI, and the PR changes no TUI surface. 中文说明代码审查在看 diff 之前，我对"安装契约还在、生产端却随桌面包被删除"这个独立方案，与本 PR 的做法一致：把应用从 git 历史提升到顶层 恢复的保真度经抽查确认（见门禁评论中的逐文件对比），workflow 代码是本 PR 最强的部分：secret 走 阻塞——本 commit 的 CI 红了两次，均由本 PR 造成：
两者都不深，都是一行级别的修复。但一个让自己的新 CI 门禁在自己 commit 上失败的 PR，现状不可合并。 未验证且刻意如此：签名、公证、GitHub release 发布和 OSS 上传只在带 CI 证据引自该 commit 的 check-runs API（按静态审查规则不本地重跑）。两个失败是确定性的、可归因于本 PR（新独立 lockfile 缺模块；新 workflow 未登记进大小棘轮），不是基础设施噪音：两条错误信息都点名了本 PR 引入的产物。 真实场景测试：本次不适用——无人值守 CI，且本 PR 不改 TUI 界面。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 2/5 — the substance is the right minimal restoration, verified file-by-file against history, but the PR's own CI is red on its own commit, so it is not mergeable as-is. Being honest: this is the PR I would have written for this problem — it matches my independent proposal almost exactly, the restoration is faithful (byte-identical where it matters, mechanical path/format deltas elsewhere), the root wiring is a two-line exclusion following the desktop-shell precedent, and the three workflows are more carefully guarded than most of what is in It is not green. Two deterministic failures, both caused by this PR: the new After a fix: expect the Stage 0 escalation to stand — ~6.8k production lines and brand-new release/OSS infrastructure mean a maintainer, not the bot, should make the final call on merge. @yiliang114 details in the review comment above. 🙏 中文说明置信度：2/5 —— 实质内容是正确的最小化恢复，且已逐文件对照历史验证，但本 PR 自己的 CI 在自己的 commit 上是红的，现状不可合并。 说实话：这就是我针对这个问题会写的 PR——它与我的独立方案几乎完全一致，恢复是忠实的（关键处逐字节一致，其余是机械性的路径/格式差异），根目录接线是沿用 desktop-shell 先例的两行排除，三个 workflow 的防护比今天 但 CI 不是绿的。两个确定性失败，都由本 PR 造成：新的 修复之后：Stage 0 的升级处理仍然有效——约 6.8k 生产行加上全新的发布/OSS 基础设施，最终合并决定应由维护者而非机器人做出。 @yiliang114 详见上方审查评论。🙏 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
previously requested changes
Aug 25, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Direction, restoration fidelity, and the workflow hardening all look right — but CI is red on this commit for two deterministic, PR-attributed reasons (full evidence in the stage-2 comment above):
Live Host (macos-latest)fails withCannot find module '.../packages/live-host/node_modules/@electron/asar/bin/asar.mjs'—@electron/asaris used by the packaging assertions but not declared inpackages/live-host/package.json(it resolved via hoisting inside the old desktop workspace). Add it to devDependencies and regenerate the standalone lockfile. Until then the security-assertion battery never completes.Test (ubuntu-latest, Node 22.x)fails at the workflow-size ratchet before the suite runs: add16647 live-host-release.yml,5998 live-host.yml, and10018 sync-live-host-to-oss.ymlto.github/workflows/.size-baseline(byte counts as printed by the CI error).
Both are one-line-class fixes. Once they land, a re-run will confirm CI green; the final merge call on the new release infrastructure still belongs to a maintainer. 🙏
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Contributor
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. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Author
|
CI attribution for the failing This PR does not touch
Leaving the main-side regression to maintainers; happy to rebase once it's green on main. |
yiliang114
enabled auto-merge
August 25, 2026 11:59
chiga0
approved these changes
Aug 25, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Checked and approved — no blockers found.
CI
Test (ubuntu-latest, Node 22.x): ✅ pass (25 min) — including workflow-size ratchetLive Host (macos-latest): ✅ pass (51 s) — typecheck + 67 Node tests + unsigned package build + security assertion batteryBuild Qwen Live Host: ✅ pass (2 min)Desktop Shell ubuntu/windows: ✅ pass (workspace isolation check)Dependency CVE audit+Secret scan (TruffleHog): ✅ passTest (macos-latest, Node 22.x),Integration Tests: ⏭ SKIPPED (path filter / merge_group gate)
What I verified
Workflow security — live-host-release.yml, live-host.yml, sync-live-host-to-oss.yml:
- All three use
pull_requesttrigger (notpull_request_target); no fork-token escalation. - Base permissions are
contents: read. The publish job escalates tocontents: writeonly whenworkflow_dispatch && dry_run == false && github.repository == 'QwenLM/qwen-code'. - User-supplied inputs (
version,source,dry_run, etc.) are passed viaenv:blocks and validated with regex/allowlist before use in shell — no direct${{ inputs.* }}interpolation inrun:scripts. - ossutil binary is downloaded and SHA256-verified before installation.
- OSS credentials are consumed from
secrets:viaenv:and the OSS job is protected byenvironment: production-release. - All third-party actions pinned to SHAs.
Workspace isolation:
!packages/live-hostadded to root workspace globs inpackage.jsonandpackage-lock.json.check-desktop-isolation.jsaddspackages/live-hostto its forbidden-prefix scan.- Desktop Shell ubuntu/windows CI (which runs the isolation check) is green.
Protocol contract:
LIVE_PROTOCOL_VERSION = 6(live-hostshared/protocol.ts) ==LIVE_HOST_PROTOCOL_VERSION = 6(CLItypes.ts).LIVE_HOST_BUNDLE_IDaligned.protocol.test.tsimports CLI types by name and enforces structural parity at compile time.live-host.ymltriggers onpackages/cli/src/serve/live/**, so a CLI type change that breaks parity is caught in CI.
desktop-pet removal: complete and symmetric — SKILL.md, gen_spritesheet.py, localCommands.ts key, and i18n.tsx EN + ZH keys all removed.
Prior CHANGES_REQUESTED (qwen-code-ci-bot, 2026-08-25T08:06:54Z) — both issues fixed at this head:
@electron/asar: 3.4.1is now inpackages/live-host/package.jsondevDependencies →Live Host (macos-latest)is green..github/workflows/.size-baselineupdated with all three new workflow file sizes →Test (ubuntu-latest)size-ratchet passes.
Reviewed with AI assistance.
ytahdn
approved these changes
Aug 25, 2026
wenshao
added a commit
that referenced
this pull request
Aug 25, 2026
Carried in from main by the last merge: #9998 added the vi.fn variant of getCurrentCustomTitle to the getChatRecordingService fake while #9994's Live Host restore re-added the plain arrow variant in the same literal — each PR built green alone, the clean merge stacked both keys, and `tsc --build` fails the cli package with TS1117 inside `npm ci`'s prepare step, so this PR's Install dependencies step dies on fresh runners. Keep the vi.fn variant: a JS literal keeps the LAST key, so it is the one every run actually used — behaviour unchanged, suite 10/10. Same one-line fix as #10023 proposes for main; landing it here unblocks this PR's CI without waiting, and the two are byte-identical so neither merge order conflicts.
Collaborator
|
Released in v0.22.2. |
pull Bot
pushed a commit
to bhardwajRahul/qwen-code
that referenced
this pull request
Aug 27, 2026
…w-fix loop (QwenLM#9659) * feat(review): content-anchored incremental rounds for the local review-fix loop A local or file-path review at high effort can now skip what it already reviewed, anchored on CONTENT rather than on a commit. The reviewed state is a dirty working tree: it has no commit to anchor on, and `local-diff.ts`'s standing constraint — nothing on the capture path writes to the index, the worktree, or any ref — rules out snapshot commits and stashes. So the anchor is the hashed per-file state of exactly what the plan covered, plus the HEAD the diff was measured against. `git hash-object` without `-w` computes the blob id git WOULD store and writes nothing. The identity is `<mode>:<blob>`, not the blob alone: an exec-bit flip or a file/symlink typechange is its own diff lines, so identical bytes under a different mode are not an identical change. Symlinks hash their link text at 120000, exactly what `git diff` renders. Whatever cannot be captured faithfully is UNHASHABLE — which never equals itself, and is therefore re-reviewed every round rather than silently certified. Rendering attributes ride in the identity too, from `git check-attr` rather than a hand-derivation: `.gitattributes` at any level, `.git/info/attributes`, the commondir's copy in a linked worktree, `core.attributesFile`, and the config-side `diff.<driver>.binary` that flips a section between readable hunks and "Binary files differ" with the bytes standing still. The scope is the same slicing the PR flow uses — the round's own files plus one import hop — and every refusal falls back to the full capture with its reason on stderr. Hardening from review, each mutation-checked: - The same-model gate is ruled over the provider-qualified identity the runtime publishes, recorded by the capture itself. `{{model}}` interpolates the bare model id, so two provider configurations exposing one model name passed each other's gate. - The cache is keyed by the SOURCE path, not the flattened token. `safeTarget` is not injective — `src/foo.ts` and `src_foo.ts` collide — and two different files were accepting each other's anchor and erasing each other's findings. - The target stem is capped with a digest suffix. Unbounded, a legal deep path made every write for that target throw ENAMETOOLONG. - Either side of a rename keeps its section, so a staged `git mv` no longer publishes an empty slice and spins the loop until HEAD moves. - The review's own plumbing is excluded by path segment at any depth, and on the TRACKED half too, so a round cannot review its own cache and never converge. - `check-attr`'s NUL protocol is read raw; the trimming wrapper stole the first record from any path beginning with whitespace, and failed open on exactly the driver lookup the identity exists for. - The mid-capture tree guard samples three interleaved states, so a phase-aligned write can no longer certify bytes no round reviewed. - The decided stops carry a machine-readable `nothingToReview`, so `qwen review run` stops reporting "Review did not complete" over a round that decided something. Replaces #9190, which cannot be merged or repaired: its base is the head of the closed #9188, GitHub counts it as a stack, and stack validation fails on the closed root while base changes are refused for stack members. The review is there — 20 reviews, 166 inline comments. * fix(review): close nine Criticals, most of them my own last round's Round 1 on this PR found nine, and the majority are regressions the previous round's fixes introduced. Taken in that order: **The PR cache lost its only writer.** The last round removed `lastModelId: "{{model}}"` from Step 8's template because the LOCAL capture had started recording the identity itself — but the PR flow has no such writer in this PR (that producer is the follow-up's), so every PR cache written after it carried no certifier, Step 1 omitted `--since-model`, and `fetch-pr` refused every anchor as `cross-model-anchor` for ever. The template line is back; the "do not hand-carry a model" instruction stays where it belongs, on the local bullet whose command does derive one. **Two stops contradicted each other.** The `clean-tree` decided stop was written without consulting `treeHeldStill`, so a capture whose own guard had just proved the tree moved mid-capture still ended the round as "nothing to review": stderr printed both lines back to back and the just-written change went unreviewed while the run recorded clean. It is gated now, like the skipped-content case beside it. **A third decided shape had no stop at all.** A cached path that vanished is a change by design, so the unchanged-since stop cannot fire, and the clean-tree stop is gated on `!incremental` — leaving `chunks: []` with an `incremental` block and no field: `agent-prompt --roster` threw and the parent reported "Review did not complete". That shape is `scope-emptied`, and only when no more specific stop already fired — the first cut of this overwrote `unchanged-since-last-round`, which the existing test caught. **The attribute probe was buffer-bound.** `gitWithInputRaw` inherited `execFileSync`'s 1 MB default while `check-attr --stdin -z` emits ~3 records per path — ~1.16 MB at this repo's file count. Past it the call threw, the blanket catch answered an empty map, every identity became UNHASHABLE, and since UNHASHABLE never equals itself the whole target was silently re-reviewed every round with a stable stateId and no refusal. Same 512 MiB ceiling `gitRaw` takes. **The parent polled a name the skill lets the model choose.** The stop was published inside the plan, whose `--out` is the orchestrator's to pick — necessarily, since the target token does not exist at Step 1 — so every file review's decided stop was invisible to `qwen review run`. It is a sidecar now, named from the same target the parent derives. **A decided stop passed `--fail-on request-changes`.** Both stop branches open by rendering the cache's still-open findings, and the common shape is a user who committed without fixing a Critical. Reported with no verdict, the gate returned 0 over a blocker the round itself called standing — passing the moment the author stopped touching the tree. The sidecar carries the ledger's open-blocker count and the parent maps a non-zero one to `REQUEST_CHANGES`. **The ledger reads named a token that does not exist yet.** Step 1's reads and Step 8's write all spelled `<target>.json` before the command that derives `target` had run, and `safeTarget` is not hand-reproducible past 64 characters or through a symlink. The capture publishes the resolved `cachePath` and every reader takes it from there. **Step 8's local paragraph pointed at a rule that cannot apply.** It extended "the same fail-closed rule as the PR cache" to local rounds, but that rule keys on a `sha` inside a posted review's marker, and a local round posts nothing — read literally it skipped the cache write on every round. The local conditions are stated in full instead. **The cache key did not discriminate the subject.** The anchor gate's `source` check is the second layer, not the first: it can only refuse a cache the round already opened, leaving the ledger — read and written by the orchestrator — sharing one file. `src/foo.ts` and `src_foo.ts` erased each other's findings, a root file named `local` produced the whole-tree key byte for byte, and one named `pr-<n>` produced PR <n>'s. File reviews get their own namespace and a digest of the source path. Safe to respell because nothing predicts the name any more. Every fix is mutation-checked, and the ordering bug in the third one is the reason: the fix that closes a finding is exactly as capable of opening one. * fix(review): close round 2 — the cache rename's fallout, and the stop's gaps Nine Criticals, and the shape is the same as last round: the fixes were what opened them. **The cache rename was split-brain (R2-1).** `cachePathFor` namespaces a file review's cache by source path, and the directory resolver still probed the old `<dir>/<target>.json` — so a file review reported "the cache is missing or unreadable" over a cache sitting right there. The resolver goes through the same speller now. (The follow-up PR already carried this fix; it belonged here, with the rename. Second time a fix and its dependency landed in different halves of this stack.) Step 6's prose named the old spelling too. **The stderr clean-tree warning was not gated (R2-2).** The field-level stop got `treeHeldStill` last round; the sentence beside it did not, so the round still printed "the working tree changed while the capture was being hashed" and "the working tree is clean" back to back — and the orchestrator branches on prose. It now says the tree is NOT clean and to re-run. **Three stops, two branches (R2-3, R2-12).** `scope-emptied` appeared nowhere in SKILL.md, so the reason existed with nothing to act on it; and the skipped-only shape — no chunks, non-empty `skippedFiles`, field deliberately withheld — had no executable continuation at all. Both have branches now, the second saying explicitly that it is NOT a stop. **The stop sidecar was collidable (R2-13).** It decides `completed` and can carry a REQUEST_CHANGES event, while its name is the flattened target token, which is not injective — and the epoch fence separates earlier runs, not concurrent ones. `run` stamps its child with a nonce and accepts only a sidecar stamped back. **A file review of an unmodified file read as a clean stop (R2-15).** An empty diff is not decided for a file target — SKILL.md's no-diff branch owes it a whole-file review — and marking it decided turned "Review did not complete" into a passing gate over a file nobody read. **One fixed plan name for every file review (R2-8).** File reviews take no lease and the plan is re-read all round, so two concurrent ones overwrote each other's central artifact mid-run: the second reviewed the first's file and merged its findings into the wrong ledger. The name must be unique to the run; it does not have to match anything. **The file-path ledger was write-only (R2-20).** Every ledger, incremental and stop bullet sat under the `local` branch; the `file` branch had none, so round 2 of a file review presented zero blockers over a Critical round 1 had recorded open. Mutation-checked: reverting the file-review exclusion, the sidecar stamp, or the stderr gate each turns exactly one test red. * fix(review): close round 3 — the cache records the gate's identity, and the naming prose follows the writer Two Criticals and six Suggestions. The PR cache wrote `lastModelId` from `{{model}}`, which interpolates the BARE model id — but the same-model gate inside `fetch-pr` compares whole-string against the provider-qualified identity it samples from the runtime. A bare-id cache is refused as `cross-model-anchor` on every later round and never heals (each clean round rewrites it bare again), so cache-path incremental scoping and the `upToDate` stop were dead for every PR, and a run that does not post lost the anchor entirely. Step 8 now copies the fetch report's `reviewModelId` — the CLI-published qualified identity, the gate's kind of string — verbatim, omitting the field when the runtime published none (the gate then fails closed to a full review, the designed state for an unrecorded identity). Pinned in SKILL.test.ts. The round-2 commit renamed the file-review cache to `file-<target>-<digest>.json` but left three texts spelling the old `<target>.json`: the Step-6 ledger-source parenthetical (the finding's anchor), the Step-1 incremental bullet, and `capture-local`'s own docblock/help. All four now point at the plan's published `cachePath`, and DESIGN.md's identity-channel section names what Step 8 actually types. The six Suggestions: the garbled comment restored to English; the model-refusal message's cached-side fallback aligned to `||` so a legitimately-empty recorded identity prints "an unrecorded model" instead of a blank name (regression test added); the source-path gate's stderr sentence pinned plus a hostile-source escaping variant; the malformed-cache refusal added as the third leg its test title always promised; the driver-binary fold matched segment-exactly so a driver whose name is a prefix of another (`md` / `mdbook`) no longer folds its config into the other's paths (two-driver fixture added); and the shared `IncrementalScope.fullDiffPath` declared optional, matching the PR producer that never emits it. Every guard mutation-probed: each one removed makes its test fail, each one restored makes the suite green (packages/cli review suites 4363 passed, packages/core SKILL.test.ts 24 passed; build, typecheck, lint clean). * fix(review): close round 4 — the named-path flow's six dead ends Six Criticals, all in the capture's named-path handling, all probed against the round-3 commit before fixing. **The driver fold re-parsed its own serialization (R3-1).** A driver NAME may contain a comma — `*.bin diff=a,b` is a legal gitattributes line — and the fold matched by splitting the comma-joined attribute string, which can never equal such a value. Its `binary` flag silently left the identity, so flipping the config changed how `git diff` rendered the same bytes while the identity stood still: the next round certified the previous verdict against a different rendering. The fold now matches a structured path→driver map recorded during parsing. **Directory-shaped plumbing targets reviewed nothing (R1-21, escalated).** `.qwen/reviews/` is the shape the named-path exemption was designed for, but the exemption matched `p === pathspec` — a child path never equals its parent directory — so every child was filtered with no skipped record. The exemption now keys on one named-plumbing predicate that covers the path and its children. **The same flow was dead under ignore rules (R3-2).** `.qwen/*` in `.gitignore` is the common configuration, and `--exclude-standard` applies to explicitly named pathspecs too, so the named file never reached the filter at all. A deliberately named plumbing path now wins over ignore rules; the new integration tests carry `.qwen/*` in `.gitignore`. **A directory target kept its tracked plumbing (R3-4).** The tracked drop skipped for ANY pathspec, but only a FILE-shaped pathspec is "what the user asked for": a `sub/` target kept every tracked plumbing section beneath it while the untracked half dropped plumbing descendants. The drop now runs unless the pathspec itself is plumbing, so both halves of the capture agree. **The cap gate ran after the parse it exists to skip (R1-20, escalated).** An oversized tracked diff was decoded and section-parsed before the 10 MB cap rejected it — and near `gitRaw`'s 512 MiB ceiling an all-ASCII diff decodes past Node's maximum string length, so the decode threw instead of producing the graceful skip record. The gate now runs before any decode; an over-cap diff is rejected whole. **A file review of an unchanged file was told the tree is clean (R3-3).** The field-level stop excludes `--file` captures; the stderr sentence beside it did not, so the orchestrator — which reads prose too — stopped on "the working tree is clean" over a pathspec-scoped capture that says nothing about the tree. The prose now carries the same exclusion and points at the no-diff branch's whole-file review. Every guard mutation-probed: removing it turns its witness red, restoring it turns the suite green (packages/cli review suites 4365 passed, 4 skipped; build, typecheck, lint clean). * fix(review): close round 5 — including a gate of mine that could never clear Five Criticals. Three are the same shape as the last three rounds: a rule applied to one branch and not its sibling. **The blocker gate could not be cleared (worst of the five, and mine).** Round 1's fix mapped the cache's open-Critical count to REQUEST_CHANGES so a stop round would not silently pass `--fail-on`. But the ledger is rewritten only by a round that WRITES the cache, and a stop round does not — so once the user fixes the blocker and commits, the ordinary workflow, every later round reads the same stale `open` entry and fails the gate over code that no longer contains the defect, with nothing the user can do to clear it. The CLI cannot tell that case from "committed without fixing": both leave a clean tree and a moved HEAD. A false failure no action clears is worse than a false pass sitting beside a rendered blocker list, so the synthesised event is gone. The count stays in the sidecar as reporting. The gate question's real answer is a composed verdict on the stop path — a verdict the model produces after re-ruling the ledger, not one this process invents from a file it cannot date against the code. **`scope-emptied` lacked the file-review exclusion** both sibling stops carry, so a file review whose anchored change was discarded completed as decided — while the identical tree WITHOUT a cache routed to the whole-file review SKILL.md owes a file target. Same tree, two answers. **The fail-closed list said "in full" and was not.** It omitted the two anchor-withholding classes the PR paragraph beside it names — a finding still `— [unverified]`, and an undecided blocker whose verifier never returned. Neither enters `findings[]` and neither reads as "unreviewed scope", so a local round promoted the anchor over a Critical nobody ruled on. Two that predate this PR's changes: **A decoded path is not a name.** Every invalid byte folds to U+FFFD, and beside a file literally named with one, two plan paths fold to a single key: `lstat` succeeds on the real file, the sibling inherits its identity, is never hashed, and compares unchanged for ever. The `lstat` guard cannot see it because the stat succeeds. Such paths are UNHASHABLE now — over-review, the affordable direction. **The repository root was classified as an escape.** `classifyRunTarget` accepts a directory target, so the root is reachable, and calling it an escape split the parent's pin from the child's derivation: the poll never matched and a review that had run reported no verdict, while `--file <root>` threw "resolves to <root>, which is outside the repository at <root>". Mutation-checked: each of the three code fixes turns exactly one test red when reverted. * fix(review): close round 6 — the five Criticals the re-read of HEAD still found * fix(review): close round 7 — and stop enumerating the fail-closed list Five Criticals, all judged against real git before changing anything. **The empty diff-driver name.** `*.dat diff=` is a legal attributes line, `check-attr --stdin -z` answers it with an empty value, and `git config diff..binary true` flips that section between readable hunks and "Binary files differ" with the mode and the blob standing still — verified against git 2.47.3 rather than taken on the report's word. The `value !== ''` clause dropped exactly that spelling, which was the last entrance of the family whose `set`/`unset`/`unspecified` siblings the previous round closed, under a comment claiming every answer was covered. **A driver name that did not survive the decode.** The record stream is utf8-decoded, so an invalid byte in a driver NAME folds to U+FFFD and the config probe asks for a key git never matches — nothing folds, and the flip moves the rendering with every identity component still. Such paths are UNHASHABLE, the same discipline this module already applies to a decoded PATH. That fix took two more passes before it was right, both caught here rather than in review: writing UNHASHABLE mid-loop let the path's later `binary`/`text` records append onto it, and leaving the path in the driver map let the config fold append onto it again. It is recorded in a set and applied once, after the stream. **A round that sees LESS cannot certify one that saw more.** With `--no-untracked` the untracked block never runs and records no `skipped` entries, so the skipped-content gate sees zero while a cached untracked path reads as vanished rather than out of scope: the slice keeps nothing and the round stops decided over bytes it never captured — and the stop does not advance the cache, so every later narrow round repeats it. The candidate records the capture's scope and the anchor is refused when this round's is narrower. **The plan's `--out` was still unbounded.** "Keep it short" is not a bound: a basename may be 255 bytes and the decoration adds 34. The guidance names a number now, and the guard pins the number. **The fail-closed list is gone, replaced by the rule.** This is the third round that "completed" it — cannot-tell was the shape that walked through the last completion. Write the cache only when every Critical carries a confirmed disposition; everything else withholds. The reason is one reason, which is why it is stated as one: the anchor's whole claim is "the next round may skip this", and a claim nobody ruled on is exactly what must not be skipped. * fix(review): close round 8 — the four shapes the last fixes left open Four Criticals, each reproduced with a failing test before the fix and mutation-probed after: each witness fails when its guard is removed. **An ignore rule is visibility, not deletion.** The round-6 fix closed the `--no-untracked` flag entrance, but an ignore rule added between rounds narrows the capture identically while no flag moves: the cached untracked path stops being enumerated, reads as vanished, and the scope-emptied stop fires over bytes no round captured — every round, because a stop never advances the cache. The anchor gate now probes the cached paths this capture lost: one still on disk that the base HEAD does not certify refuses the anchor, reason said out loud. A tracked file reverted to HEAD is certified by HEAD itself and keeps the designed discarded-change stop; a genuine deletion still reads as a change. **An undecodable driver name unhashes the WHOLE identity.** The round-6 fix set the attribute slot to UNHASHABLE, but the composition appended it — `100644:<blob>:unhashable` equals itself across rounds, so a rendering flip still moved nothing. What cannot be named faithfully cannot be certified: the whole identity takes it now. Pinned against real git with a raw 0xFF driver name. **The stop sidecar is snapshotted in-run.** It is the shared per-target name written with plain writeFileSync, read once after the child exits: a concurrent same-target run overwrites it (foreign stamp turns a decided round into "did not complete") and a same-stem cleanup sweep can unlink it, over a window spanning the whole child session. The capture poll snapshots it exactly like the composed verdict; the post-close read stays as the fallback for a child that exits inside one tick. **A trailing backslash is a filename character on POSIX.** The parent's pin stripped it while the child's derivation never does, so a file literally named `notes\` polled `notes` while every child artifact carried `notes_` — the review ran and posted while the parent reported no verdict, every run. The pin strips forward slashes only; the two-sides agreement is pinned against real git. * fix(review): close round 9 — date a stop's blockers; key the local cache write to the marker * fix(review): close round 10 — date each stop blocker against its own file, not the whole cached state * fix(review): close round 11 — date blockers on bytes; certify vanished paths by bytes Six Criticals from the round-10 re-read, each reproduced with an executed probe before the fix, each fix pinned by a test that fails on the pre-round branch and witnessed by a mutation probe. Blocker dating now compares mode-plus-blob, never the rendering-qualified identity: an .gitattributes normalisation commit, an info/attributes edit, or a repo-local diff.<driver>.binary flip changes no bytes and appears in no git diff, yet it unseated a standing blocker and passed --fail-on while the stop still rendered the Critical as open. And a blocker whose recorded identity is UNHASHABLE is undatable — no user action ever changes an UNHASHABLE<->UNHASHABLE comparison — so it no longer stands for ever over a fix nothing clears; it leans the documented false-pass way beside the rendered blocker list. The vanished-on-disk guard certifies by bytes, not by name: under assume-unchanged git hides the edited tracked file from git diff HEAD while ls-tree HEAD still names it, so the name check certified a divergence no round ever read and the scope-emptied stop fired DECIDED over it. The added-file veto keys on the population the cached round could actually enumerate: a scoped capture (--file, --no-untracked) skips it, and the listing excludes the review's own plumbing, so pre-existing untracked files and the cache file itself no longer permanently disarm every blocker. hashWorktreeFiles dedups at the module boundary — check-attr answers per input occurrence, so a path listed twice forged a double-suffix identity that never matched — and revisionIdentities pins LITERAL_PATHSPECS like every sibling, so one :(magic ledger path no longer fatals the whole batch and clears the gate. Also closes the two carried confirmations: the ledger key space is digest-namespaced (verified end to end for the local/pr-<n> reserved names and the non-injective token pair), and Step 9 now names the file review's duty to remove its own chosen plan name — the one artifact cleanup cannot glob without racing a concurrent run. * fix(review): close round 12 — attribute landed fixes, date by regime, stop on the published ledger (#9659) * fix(review): close round 13 — pay the isolation test's import cost at collect, not in its timeout (#9659) * refactor(review): drop the stop-round blocker-dating chain Remove blockerStateStillMatchesTree, filesAddedSince, byteIdentity and openCriticalsInCache from capture-local, the openBlockers/blockersStand fields they fed through the stop sidecar, and the standingBlockers half of exitCodeFor. A decided stop now completes with event null and exits 0 under --fail-on, unconditionally. The chain existed to let a stop round block CI while an open Critical still stood, without the failure-no-action-clears trap a raw ledger count carries. Three rounds of review showed the dating itself is the trap: every comparison regime it grew (dates, then bytes, then per-file regime bits) spawned a successor defect in the next round -- R10-2 begat R11-4/R11-6 begat R12-1/R12-2 -- because "did the user fix it" is a judgement about a mechanism, not a property of file bytes. Seven of the last sixteen review Criticals sat on these functions. What remains is what was already sound: the stop sidecar still decides completedness (reason + runId nonce), the skill still renders the cache ledger's open findings on every stop branch, and vanishedStillOnDisk stays -- it feeds the anchor gate, not this chain. The gate that can block a stop round correctly is a composed verdict on the stop path, where the model re-rules the ledger; that is a separate change. * fix(review): close round 14 — certify what git diff cannot see, split the emptied ledger (#9659) - renderingAttributes: fold a `diff` answer of `set`/`unset` to UNHASHABLE instead of the identity — check-attr answers the attribute STATE and a driver NAME that spells a state byte-identically while git renders them differently (`-diff` binary vs `diff=unset` readable, probed on git 2.39.5), so the pair cannot be certified on this stream - capture-local: hash a FILE review's subject into the anchor even with no diff section; refuse a cache whose files map is empty; condition all three decided stops (and the clean-tree prose) on `git ls-files -v` finding no assume-unchanged/skip-worktree bit, since `git diff` is blind to any edit on a marked path - SKILL.md: split the scope-emptied ledger by cited path (SUPERSEDED only for gone bytes — the gate cannot tell removed from byte-identical); name the visibility-bit shape among the withheld-stop shapes; move the file-review plan family to `file-review-…` so no cleanup sweep prefix (`qwen-review-<target>-*`) can ever match it — a target named `file` or `file-<X>` deleted concurrent file reviews' live plans mid-round - persistence.md: end the file-review report stem at `<target>` when it already ends in `.md` — the report pin never doubles the extension * fix(review): close round 15 — withhold the candidate under visibility bits, name the record dir's removal (#9659) - capture-local: condition the cache-candidate write on the same visibility-bit oracle the three decided stops already consume — `hash-object` reads the worktree bytes THROUGH a set assume-unchanged/skip-worktree bit while `git diff` cannot see them, so a candidate written under a bit recorded the identity of bytes the round's diff never showed; promoted, clearing the bit between rounds keeping the bytes left every comparison unchanged and every gate clean, and the unchanged-since stop certified them. Withheld with the reason said out loud (hoist the memoized oracle above the write; the enumeration-failed arm withholds too) - SKILL.md: Step 9's manual removal now covers the plan-derived `-prompts` record directory beside the plan JSON — the `file-review-` rename moved it out of every cleanup sweep prefix and retention scan, and nothing else removes it; drop the reverse-audit transcripts from the token-bearing inventory, because they ride the plan's free stem via that record directory - persistence.md: name the visibility-bit shapes among the capture-owned candidate withholdings Step 8 can observe as absence * fix(review): close round 15 review — bracket the visibility oracle, gate clean-tree on untracked scope R15-1: the visibility-bit oracle was sampled exactly once, after the three-pass capture loop, while the candidate write and all three decided stops conditioned on that single reading. A bit set through every diff pass and cleared before the one query read clean while the captured diffs were blind to the edit it hid — the candidate then certified the identity of bytes no pass ever showed, and the next round's comparison sliced them out of scope for ever. The oracle now rides the same both-endpoints discipline as the diffs and hashes: one sample before the first capture, one inside each re-capture pass, one after the loop. A bit visible at ANY sample withholds (the union is reported), and a single failed enumeration withholds everything. R15-2: the clean-tree stop claims "nothing staged, nothing unstaged, nothing untracked", and under --no-untracked the third clause was checked by nobody — the untracked enumeration never runs and records no skipped entries, so a tracked-clean tree with pending untracked work stopped decided and `review run` exited 0 over files no round enumerated. SKILL.md's own recovery from an oversized-untracked skip re-runs with exactly this flag. The stop and its prose twin now carry the exclusion the anchor gate has had since the candidate recorded `untracked`, said out loud; the field-less shapes bullet gains the fourth shape, with do-not-re-run guidance (the flag is the cause). Deliberately NOT a synthetic skipped entry: Step 8 withholds the cache write on non-empty skippedFiles, and a --no-untracked round anchors legitimately through the candidate's recorded `untracked` scope — a synthetic entry would have killed incremental review for that population wholesale. Both fixes mutation-checked: post-only sampling and a dropped untracked conjunct each turn their new tests red. * fix(review): close round 16 — withhold the incremental stops under --no-untracked (#9659) R16-1: the unchanged-since-last-round and scope-emptied stops lacked the args.untracked !== false exclusion their sibling clean-tree stop carries. The anchor gate's untracked clause only refuses a narrower round than the cache, so two --no-untracked rounds pass it and either stop decided "nothing to review" over untracked content neither round enumerated — a brand-new file stopped the loop while never seen. Both stops now follow the sibling: withheld, out loud, with the SKILL.md field-less bullet routing the shape to the same do-not-re-run branch. * fix(review): gate decided stops on a composed re-rule verdict (#9941) * fix(review): gate decided stops on a composed re-rule verdict (#9908) A decided stop round whose cache ledger still holds open findings used to complete with event: null, so `qwen review run --fail-on request-changes` exited 0 while its own prose said Request changes — a user who committed without fixing a Critical left a permanently clean tree and CI passed. The stop branches now re-rule the open ledger against the current tree and compose a real verdict: `compose-review` gains a `stopReRule` state, honoured only when the plan carries the capture's own `nothingToReview` decision and — under a `review run` parent — the runId-fenced stop sidecar agrees, so it exempts the round from agent-transcript floors that are unsatisfiable when no agents ran. `review run` gates the composed verdict exactly like a full round: a still-standing Critical exits 3, a re-rule that judged every blocker fixed or superseded exits 0. * fix(review): address round-1 review of the stop-round gate (#9908) - Strip stopReRule at the submit boundary: a posted verdict is a PR round's, and no PR round is ever a decided stop, so the floor exemption is illegitimate on every path that posts — closing the laundering path the round-1 review opened (R1-1). - Exempt the findingsPath floor as well: the reverse-audit delivery check is unsatisfiable on a round that ran no reverse audit, and a faithful stop state can only carry a previous round's file (R1-2). - Require the capture's stop sidecar for the exemption even when no run id is published (interactive rounds write one too), and disclose on stderr which read refused a claimed stopReRule (R1-1, R1-5). Document the fence's honest residual instead of overclaiming it. - Share the sidecar parse between run.ts and compose-review.ts in lib/stop-sidecar.ts so the stamp contract has one reader-side spelling (R1-3). - Report the decided-stop reason as stopReason in the run result so a gate can separate a clean stop from a stop that owed a verdict (R1-6). - Tests: hermetic cwd + QWEN_REVIEW_RUN_ID isolation for the suite (R1-4, R1-7); coverage for the new fence arms and stopReason. * fix(review): pin the stop-round gate against its round-2 mutations (#9908) - Handler pass-through: add a CLI-glue test asserting the stopReRule exemption survives the compose-review handler's {...parsed} spread — a plausible `delete parsed.stopReRule` (the submit idiom) left every direct composeReview() test green while the exemption died end-to-end (R2-1). - findingsPath exemption: regression-test both arms — a previous round's file with surviving unverified tags, and a since-cleaned-up path — dropping `!stopReRule &&` now turns red (R2-3). - Shared sidecar contract: STOP_SIDECAR_SUFFIX / STOP_SIDECAR_NAME / stopSidecarNameFor / isValidStopReason in lib/stop-sidecar.ts are now the single spelling the writer (capture-local) and both readers (run.ts, compose-review) derive the name family and reason check from (R2-2). - stopReason docstring: state the true contract — it separates a decided stop from a verdict round, NOT a clean stop from a lost re-rule (both are stopReason-set + composedPath-null; that shape needs the cache ledger, which the result deliberately does not carry) (R2-4). - submit strip seam test: a stop-shaped state + sidecar must still post a floored COMMENT (capped by criticals-unverified); deleting the strip now turns red (R2-5). All three code-arm mutations verified red under the new tests and green at this commit; 787 tests across the six affected suites pass. * Revert "fix(review): gate decided stops on a composed re-rule verdict (#9941)" This reverts commit af521c5d58a26acb843440a730458143733daccb. #9941 was merged into this PR branch by accident: it is the #9908 successor (composed re-rule verdicts on the stop path), deliberately kept out of this PR by the recorded R13-3 ruling — sixteen review rounds have converged on the branch as scoped, and the merge landed ~750 new lines of unreviewed mechanism (plus files outside the review surface: agent-core.ts, geminiChat.ts) at the finish line. Reverted to restore the reviewed scope; the change returns as its own PR against main once this one lands, exactly as the ruling recorded. * fix(cli): drop duplicate getCurrentCustomTitle key that breaks npm ci Carried in from main by the last merge: #9998 added the vi.fn variant of getCurrentCustomTitle to the getChatRecordingService fake while #9994's Live Host restore re-added the plain arrow variant in the same literal — each PR built green alone, the clean merge stacked both keys, and `tsc --build` fails the cli package with TS1117 inside `npm ci`'s prepare step, so this PR's Install dependencies step dies on fresh runners. Keep the vi.fn variant: a JS literal keeps the LAST key, so it is the one every run actually used — behaviour unchanged, suite 10/10. Same one-line fix as #10023 proposes for main; landing it here unblocks this PR's CI without waiting, and the two are byte-identical so neither merge order conflicts. * fix(cli): drop the second duplicate key too (getSessionDisplayName) The merge stacked two duplicate keys in this file and the previous commit removed only getCurrentCustomTitle; the sessionService fake also carries getSessionDisplayName twice. Identical expressions, and a JS literal keeps the last one — behaviour unchanged. Matches #10022's fix of main line for line, so neither merge order conflicts. * test(review): pin the visibility oracle's sample count The sandboxed verifier's mutation matrix caught the one unpinned guard (M6): the visibility test's scripted mock consumes by CALL ORDER, so deleting sample 0 in production merely shifts the script — the dirty sample still lands on a later query and the test stays green. Pin the count: four samples exactly, 0 before the first capture, 1-2 in the loop, 3 after. Replanted M6 locally — the pin turns exactly this test red ("expected 3 to be 4"), matching the verifier's proof fixture. * fix(review): close round 17 — restore the merged-away fake, publish the split key, harden the oracle's edges R17-1: the duplicate-key cleanups each removed a DIFFERENT copy of getSessionDisplayName (#10022 on main, its twin here) and the clean merge resolved to zero — the /branch hook TypeError'd on the absent method and three telemetry tests went red. One copy restored, with a comment pinning why exactly one. R17-2: the scope-emptied split keyed on file PRESENCE, which cannot see a discarded change (file present, cited bytes gone), and no channel named the affected paths at all. The capture now publishes incremental.scope.supersededPaths — cached paths whose recorded change is gone from this capture — and the bullet routes the SUPERSEDED split through membership, both directions. R17-3: check-attr answers diff=unspecified byte-identically for the no-rule state and an explicit diff=unspecified value, so with diff.unspecified.binary configured a rendering flip crossed the anchor unseen. The config's presence now takes the whole answered dimension to UNHASHABLE; without it — every ordinary repo — nothing changes. R17-4: the candidate path is stable per target with no lease, so a concurrent same-target run overwrites it mid-round undetectably. The plan now publishes cacheCandidateStateId beside the path and Step 8 compares before promoting; a mismatch is a withheld candidate. R17-5: sparse-checkout S-tags every out-of-cone path by design, so the oracle read non-empty on every sample in every sparse repo — permanent non-convergence on a clean materialized tree. Under core.sparseCheckout an S path counts only while it exists on disk (nothing to hide an edit in); the assume-unchanged family is never exempted, and outside sparse an absent S path stays a hidden deletion. R17-6 entrance 2 (created by this diff): the 64-char cap's `-` joiner made every exactly-55-char uncapped stem a strict sweep-prefix of every capped stem sharing its head, so cleanup deleted the capped twin's live artifacts. The joiner is now `~`, outside the token alphabet, making capped stems prefix-free against ALL stems. Entrance 1 (organic dash-prefix tokens) predates this PR and is tracked in #10057. Every fix mutation-checked: dropped sample-0 exemption, reverted sparse filter, removed unspecified special-case, dropped supersededPaths, and a `-` joiner each turn exactly their new tests red. * fix(review): close round 18 — ask git for the cone, make the cap idempotent, keep the stop evidence R18-1: tmpFile/tmpPrefix apply safeTarget a second time to tokens other callers already derived with it, and the second flatten rewrote the capped `~` joiner to `_` while run's pins interpolated the single-flatten spelling raw — every deep-path round wrote one name and polled another, resurrecting the never-matching-poll regression this PR exists to kill. safeTarget is idempotent over its own outputs now (the capped shape is recognized before the flatten); the pass-through that permits is no wider than the flatten's own collisions. R18-2/3/4: the sparse exemption re-derived git state by hand and got all three edges wrong — a raw --get missed legacy truthy spellings and read an inherited global; "absent + S" is not "out of cone" on gits that retain a manual bit inside the cone (2.43 does, 2.47 re-clears); and the combined-bit lowercase `s` spelling missed the S-only match. The oracle now asks git at every step: the flag reads --worktree --type=bool (sparse-checkout writes config.worktree via extensions.worktreeConfig), cone membership comes from `git sparse-checkout check-rules`, and the rule is uniform over every tag — exempt exactly the paths that are absent AND outside the rules; a failed check-rules exempts nothing. The version-dependent arms real git 2.47 cannot stage (it re-clears the manual bit and canonicalizes the bool even on read) are pinned by a scripted-git unit suite beside the real-git integration arms. R18-5: Step 9's file-review `-prompts` removal was unconditional, and it is that family's only remover — every unconverged file review destroyed its own budget-stop evidence on the way out, voiding the #9206 retention cleanup implements for swept families. The removal now keeps the plan and the record directory when the budget-stop marker is present, with cleanup's Kept-line wording. Mutation-checked where constructible: the exempt-all-absent mutant is killed by the scripted in-rules arm (proven live-red before restore), the joiner and idempotence mutants by paths.test, the flag-read mutant by the 2.43-class CI git via the raw-spelling arm. * fix(review): close round 19 — only ENOENT is absence, undecodable is never exempt Three foldings of failure semantics onto the safe side, all instances of the discipline hashWorktreeFiles and revisionIdentities already carry: R19-1: the sparse exemption ran both its halves on a name the decode had already mangled — lstat missed the PRESENT file through the U+FFFD spelling and check-rules was fed bytes git never knew — so a bit-carrying present file was silently exempted. An undecodable name is never exempt. R19-2: every lstat failure folded into "absent", so a PRESENT flagged path under an unmeasurable ancestor (EACCES/ENOTDIR/ELOOP) was exempted while git diff stayed blind to its bytes. Only ENOENT proves absence; anything else stays flagged. R19-3: vanishedStillOnDisk folded every lstat failure into "genuinely gone", ending the round at a decided scope-emptied stop over bytes no round captured. Only ENOENT proves deletion; an unmeasurable cached path stays in the on-disk set, whose downstream is an anchor refusal at the cost of a full round. Deterministically staged as root via ENOTDIR (a regular file where the ancestor directory was) — EACCES needs a non-root runner. All three foldings mutation-checked red. * fix(review): close round 20 — bool-typed fileMode, no directory in the anchor, certify gitlinks R20-1: `core.fileMode` was read with a raw `--get`, which echoes the stored spelling, so `off`/`no`/`0`/`FALSE` all failed the `!== 'false'` test and silently disabled the exec fold — the same misreading R18-2 fixed for `core.sparseCheckout`, at the call site that fix did not touch. Read `--type=bool`, and fold only on an EXPLICIT false: the knob defaults to true and an unset one must not erase a real divergence. R20-2: a FILE review's subject entered the hashed population unconditionally, so `qwen review <dir>` recorded the directory itself as UNHASHABLE in every candidate. UNHASHABLE never equals itself, so `changedSince` reported the directory every round and the unchanged-since stop was unreachable for that target for ever — the very non-convergence `movedSince` was added to close. A confirmed directory is skipped now; its files carry the bytes, and an unmeasurable subject keeps the pre-existing coverage. R20-3: a submodule gitlink is UNHASHABLE on both sides by design, so the both-unhashable refusal wedged the loop for ever once a round had touched one. Git measures submodules itself and the pinned flags keep them in the capture, so a gitlink's absence from the diff is git's own answer that the pointer did not move. Which paths are gitlinks is asked of `ls-tree`, not inferred from the placeholder they share with undecodable names — those still refuse. R20-5, disclosed rather than folded: `core.symlinks=false` erases the file↔symlink type the same way, and the comment here claimed the type survives every fileMode — true of fileMode, false of this knob. A mode fold does NOT close it: the two spellings also differ in whether they carry rendering attributes at all, and equalizing those would drop the rendering dimension for that path. The bounded over-review stands, now pinned by a test so it cannot become a silent certification. R20-4 is pre-existing and NOT introduced here — before this PR the file token was the basename, so a `local` at ANY depth collided; the repo-relative derivation narrowed that to the repo root. The general class is tracked in #10057. Step 9 gains the cheap guard meanwhile: a file review whose token derives to a reserved name must not run `cleanup` at all, and removes only what it wrote. Every code fix mutation-checked red. * fix(review): close round 21 — measure the pointer, ride the skipped lists, one path spelling R20-3 follow-up: the gitlink exemption took the diff's ABSENCE as git's answer that the pointer did not move — but with the submodule's gitdir gone that absence is git's silence. The pointer is now read directly (`rev-parse HEAD` inside the submodule) and compared to HEAD's recorded oid; a submodule whose HEAD cannot be read is unmeasurable, and unmeasurable is uncertifiable. R20-4 follow-up: the reserved list was under-enumerated — a repo-root file literally named `pr` derives the bare token `pr`, whose sweep prefix engulfs EVERY PR family while the lease guard lives inside the `pr-<n>` branch a bare `pr` never enters. `cleanup` refuses the token outright now (no target legitimately owns that prefix), and the Step 9 reserved list names it. R21-1: `repoRelativeOf` returned node's platform-separated `relative()` output verbatim, which flows into git pathspecs, the candidate's recorded `source`, and `cachePathFor`'s digest — on win32 one file got two cache filenames across platforms and the Windows lane failed every posix-spelled assertion. `rel` is normalized to forward slashes (git's own spelling on every platform), after the escape check computes against the platform separator. R21-2: the sampling loop kept only `.diff` from re-captures 1 and 2 — a file entering the window that lands in a skip class is in no capture's diff BYTES, so the byte comparison read "held still" while two of the three captures explicitly skipped content, and every stop gate reads only capture 0's list. Skip-set movement is tree movement: the skipped path-sets ride the treeHeldStill comparison now. All but the win32 normalization mutation-checked red locally (that branch is a no-op where sep is '/'; the Windows lane is its enforcement). * fix(review): close round 22 — a gitlink's identity is real, on both sides R22-1: a moved or in-diff submodule pointer entered the hashed population and could only record UNHASHABLE — which never equals itself — so changedSince reported it every round and the unchanged-since stop was unreachable for the lifetime of any change set holding a dirty pointer, with a misdescribing diagnosis. Yet the module already measures exactly this identity (the R20-3 fix did it inside a special case). Make the identity real instead: hashWorktreeFiles answers 160000:<oid> for a readable, content-CLEAN submodule (the pointer oid says nothing about internal edits — git renders those as `-dirty` — so cleanliness is part of measurability and a dirty submodule stays UNHASHABLE), and revisionIdentities answers the same shape from ls-tree's recorded oid. With both sides real, vanishedStillOnDisk's R20-3 special case became redundant and is deleted along with its helper: a restored pointer certifies through the ordinary equality, an unreadable or dirty one refuses through the ordinary UNHASHABLE clause — net-negative plumbing. The submodule fixture now pins all three arms: the dirty pointer CONVERGES (unchanged-since reachable), internal dirt never certifies (no decided stop over `-dirty` bytes), and the odb-removed pointer still refuses. Both mutants — UNHASHABLE-again and dirt-invisible — turn it red. * fix(review): close round 23 — withhold over the dropped-out set, exclude file targets, keep every byte Six findings, all confirmed against the tree: The candidate write now gates on the dropped-out-while-on-disk set: a refused-anchor round used to write a candidate that silently OMITTED the dropped path, Step 8 promoted the omission, and two rounds later a scope-emptied stop certified bytes no round read. The cache is read before the write now, the withholding is voiced with its own sentence, and the scoping branch reuses the early read. The unchanged-since-last-round stop gains the file-review exclusion BOTH sibling stops carry — a cached round-2 file review of an unmodified subject stopped decided while the identical tree without a cache routed to the whole-file review. The excluded shape gets its own honest stderr line instead of falling into the unhashable-paths diagnosis, and the directory-subject test now pins convergence as the ABSENCE of the wedge. gitlinkIdentity asks the submodule's OWN visibility bits: status --porcelain honours an assume-unchanged bit set inside the submodule, so cleanliness judged by status alone held the identity still over interior bytes no round can see — the fix-induced half of R22-1, closed with the same oracle one level down. canonicalise strips only the platform's separators on the ancestor walk: `\` is a legal POSIX filename byte this PR's own fixtures insist on, and the two-class strip corrupted a dangling `\link` into `link`. SKILL.md: the Step 1 file bullet's --out template carries the 24-char truncation (the full-basename spelling died with ENAMETOOLONG past ~226-byte basenames, measured), and both PR stops (up-to-date, empty diff) now write the stop sidecar with the run's nonce before cleanup — the reader predicted the name but nothing in the PR flow ever wrote it, so every decided PR stop exited 1 "Review did not complete". All four code fixes mutation-checked red; guards pin both prose fixes. * fix(review): keep this run's stop sidecar across its own cleanup Human review on #9659: the round-23 PR stop protocol writes the sidecar and runs `cleanup pr-<n>` in the same breath, but cleanup swept the same prefix — and the parent's first in-run poll is up to 250 ms away, so a write+cleanup finishing before the first tick left NOTHING for either the snapshot or the post-close fallback to read: an already-decided up-to-date/empty-diff round still exited 1 "Review did not complete". The local flow never hit this only because its stop (Step 1) and its cleanup (Step 9) are minutes apart. cleanup now spares `<prefix>stop.json` exactly when its runId matches the environment the parent stamped — a foreign or unstamped sidecar is residue and sweeps as before, and the NEXT run's cleanup (different nonce) collects this one. SKILL.md says so beside the write instruction, so the orchestrator does not "tidy" the file by hand. Pinned from both sides of the window: cleanup.test spares the matching sidecar and sweeps a foreign one (mutation-checked red); run.test gains the zero-timer-advance arm — child writes the sidecar and closes on a microtask, before ANY poll tick, and the post-close fallback still reads the decision (the existing race arms all advanced 1000 ms first, which is exactly how the window went unpinned). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This restores Qwen Live Host as a standalone macOS helper after the legacy desktop workspace was removed in #9085. The CLI and Web Shell Live Voice flow still install and communicate with this helper, so the PR restores its source, native Appshot module, tests, packaging, GitHub release workflow, and OSS mirror/recovery workflow without restoring the old Electron desktop application.
The helper now has an independent npm toolchain and lockfile and is explicitly excluded from the root workspace, keeping Electron and electron-builder out of the root CLI installation. The release path also serializes stable publications, rejects prerelease flag/version mismatches, and refuses to downgrade the fixed
live-host-latestfeed.The PR also removes the remaining desktop-pet skill exposure because its floating-window runtime and settings integration were removed with the legacy Electron application.
Why it's needed
After #9085, the Live Voice consumer path remained in the CLI and Web Shell, including the installer contract for
Qwen-Live-Host-manifest.jsonand architecture-specific ZIP assets. The source and workflows that produce and recover those assets were removed withpackages/desktop, leaving a live installer backed only by a frozen feed with no production or disaster-recovery path.This restores that missing producer side at the narrowest boundary. It does not restore the legacy desktop UI, OpenWork workspace, WhatsApp, viewer, Office integrations, session MCP tools, or other desktop-only functionality.
Reviewer Test Plan
How to verify
packages/live-hostand that a clean root installation does not install Electron-only dependencies.packages/live-host, run the typecheck, 67 Node tests, and production build. Confirm the daemon protocol test remains synchronized with the CLI Live Host protocol.live-host-latestmanifest with an older version.Commands run locally:
Observed results: Live Host 67/67 tests passed, release/OSS workflow 23/23 tests passed, Web Shell command mapping 10/10 tests passed, and the Live Host production build and root dependency-isolation check passed.
Evidence (Before & After)
Before: the CLI and Web Shell retained the Live Host installer and runtime protocol, but the repository no longer contained the helper source or any workflow capable of producing or recovering the installer feed.
After: the helper and its release/feed lifecycle are maintained as an isolated package, while the removed legacy Electron desktop runtime remains out of the repository.
Tested on
Environment (optional)
macOS arm64, Node.js 22, standalone Live Host npm installation. Signing, notarization, GitHub release mutation, and Aliyun OSS upload require the hosted release environment and were not executed locally.
Risk & Scope
packages/desktop.Linked Issues
Follow-up to #9085.
中文说明
本 PR 做了什么
本 PR 在 #9085 删除旧桌面 workspace 后，将 Qwen Live Host 恢复为独立的 macOS helper。CLI 和 Web Shell 的 Live Voice 流程仍然会安装并连接该 helper，因此本 PR 恢复其源码、原生 Appshot 模块、测试、打包、GitHub release workflow，以及 OSS 镜像和恢复 workflow，但不会恢复旧 Electron 桌面应用。
Live Host 现在拥有独立的 npm 工具链和 lockfile，并明确排除在根 workspace 之外，避免 Electron 和 electron-builder 进入 CLI 根安装。发布链路还会串行化稳定版本发布，拒绝 prerelease 标志与版本不一致的输入，并阻止旧版本降级固定的
live-host-latestfeed。本 PR 同时删除剩余的 desktop-pet skill 暴露，因为它依赖的悬浮窗口 runtime 和设置集成已经随旧 Electron 应用被删除。
为什么需要
#9085 合并后，CLI 和 Web Shell 中的 Live Voice 消费链路仍然存在，包括
Qwen-Live-Host-manifest.json和不同架构 ZIP 资产的安装契约。但生成和恢复这些资产的源码及 workflow 随packages/desktop一起被删除，导致仍在使用的 installer 只能依赖一个冻结的 feed，仓库内不再具备生产或灾备恢复能力。本 PR 在最小边界内恢复缺失的生产端。它不会恢复旧桌面 UI、OpenWork workspace、WhatsApp、viewer、Office 集成、session MCP tools 或其他仅桌面端功能。
Reviewer Test Plan
如何验证
packages/live-host，且根目录 clean install 不会安装 Electron 专用依赖。packages/live-host中运行 typecheck、67 个 Node 测试和生产构建，确认 daemon protocol 测试与 CLI Live Host protocol 保持同步。live-host-latest。本地执行的命令：
结果：Live Host 67/67 测试通过，release/OSS workflow 23/23 测试通过，Web Shell command mapping 10/10 测试通过，Live Host 生产构建和根依赖隔离检查通过。
Evidence (Before & After)
Before：CLI 和 Web Shell 仍保留 Live Host installer 和运行时协议，但仓库已不再包含 helper 源码，也没有能够生产或恢复 installer feed 的 workflow。
After：helper 及其 release/feed 生命周期作为独立 package 继续维护，同时已删除的旧 Electron 桌面 runtime 仍不会被恢复。
Tested on
Environment (optional)
macOS arm64、Node.js 22、Live Host 独立 npm 安装。签名、公证、GitHub release 修改和阿里云 OSS 上传需要托管发布环境，本地未执行。
Risk & Scope
packages/desktop删除的其他功能。Linked Issues
#9085 的后续补救。