chore(desktop): rename packages/desktop-shell to packages/desktop - #12653

Review at 65afaeb (author is myself — this COMMENT is the record; a non-author maintainer vote is needed to clear REVIEW_REQUIRED).

No blocking findings on a rename this size (121 files):

1. Completeness verified at the tree level: zero desktop-shell paths remain in the head's git tree (10,293 entries walked), and every desktop-shell hit in the diff is either a removed line or a deliberate historical note in comments. New references consistently use packages/desktop, including the root package.json workspace exclusion.
2. CI wiring updated coherently: the path filter, the Cargo.toml presence guard, rust-cache workspaces, and both working-directory entries all moved together; the Desktop Shell lanes (ubuntu-22.04 + windows-2022) are green on this head, which is the only lane that actually compiles the crate.
3. The one real trade-off is disclosed in the workflow comment itself: a pre-rename branch (files still under packages/desktop-shell) no longer matches the filter and would report green having compiled nothing — the comment states such branches need a rebase to be gated again. That's the honest answer for a rename and acceptable: those branches must rebase before merge anyway.
4. The review-lib path updates (workspaces.ts, workspace-scope, build-test, test-efficacy) are mechanical renames with their tests updated to match; the desktop-brand-builder skill's relative paths follow the move.

CI green except review-pr (queued, bot infrastructure) and web-shell E2E Smoke (queued). All 8 threads resolved.

No blocking findings.
Approval blockers: none.

Scope: 121 files, 402 additions / 204 deletions. ~96 pure renames (packages/desktop-shell/* → packages/desktop/*) plus targeted path-string substitutions in CI workflows, root workspace manifests, tool configs, docs, test fixtures, and two CLI workspaces source files (comment-only updates). One production-logic change: check-desktop-isolation.js gains 'packages/desktop' in nativePrefixes. New test: scripts/tests/desktop-isolation.test.js.

Checked:

• Workspace isolation coherence (class 1 — writer/reader contract): package.json workspace negation (!packages/desktop), pnpm-workspace.yaml exclusion, check-desktop-isolation.js nativePrefixes, and pnpm-worktree-smoke.yml path filters all updated consistently. packages/desktop-shell kept in nativePrefixes as a re-entry tripwire — deliberate and documented.

• CI wiring (class 1): The desktop_shell job's five crate-path references (changed-files filter alternative, Cargo.toml existence guard, guard ::notice:: text, rust-cache workspaces:, both working-directory: values) all move together to packages/desktop. desktop-isolation.test.js pins all five sites to agree, existence-checks Cargo.toml, and verifies the filter alternative carries a trailing / (preventing prefix collision with any future sibling named packages/desktop-extra or similar).

• Pre-rename branch skip risk: Explicitly documented in both the PR description and the ci.yml job comment. A branch still at packages/desktop-shell/ no longer matches the filter and reports green having compiled nothing. Not a defect introduced here — it is the acknowledged trade-off of a directory rename. The Cargo.toml existence guard continues to cover the complementary case (filter matched because ci.yml itself changed, crate absent at the new path).

• npm package name (@qwen-code/desktop-shell → @qwen-code/desktop): private: true throughout — no published consumer can break.

• Callers of renamed path strings (class 2): acp-channel-fallback.ts, LinkifiedText.tsx, web-shell/client/index.html, .qwen/skills references, docs all updated. LinkifiedText.tsx:9 deliberately uses desktop-host routing (not desktop routing); per the author's thread reply, this is the terminology adopted in the living docs.

• Deliberate desktop-shell survivors (class 10 — stated intent): the isolation-guard tripwire in check-desktop-isolation.js, two ci.yml comment lines naming both sides of the rename, docs/design/9152-architecture-invariant-classification.md (names both spellings accurately), and the date-prefixed docs/design/2026-07-31-desktop-web-shell-release.md (historical record). All are correct per the PR description.

• desktop-isolation.test.js test validity (class 5): The guard-body assertion now pins changed=false inside the if..then..fi block (not just the condition text), fixing R2-2. The pre-rename tripwire checks packages/desktop-shell/package.json existence rather than the bare directory, fixing R2-3 (build residue under the old path would no longer produce a false positive). The prior tautological startsWith assertion (R2-4) is absent from the current head.

• Binary renames (icons, images): All shown as similarity-100 renames in the diff — no content changes, only path moves.

Not covered: local execution of the vitest suite (no node_modules available); Windows path behaviour; CI matrix runs (delegated to the Desktop Shell CI job on this PR).

Reviewed with AI assistance.

Critical-only review at 65afaeb9, base 78faaec8.

Verdict: APPROVE — no Critical, and no blocking issue was ever filed here. For a 121-file rename the thing to verify is not the moved bytes but every reference that has to keep resolving, so that is what I checked: the load-bearing ones all point at the new path, and the two places that still name the old one are deliberate.

The rename is coherent across everything that resolves a path

Both workspace negations moved together — "!packages/desktop" in the root package.json workspaces array and '!packages/desktop' in pnpm-workspace.yaml. That pair matters more than the rest: the release set is derived from the workspace globs, so a negation left behind at the old path would have pulled packages/desktop into RELEASE_WORKSPACES and into a publish step that has no business publishing it. It is also private: true, so the exclusion is doubled rather than resting on one mechanism.

In ci.yml the changed-files filter, the rust-cache workspaces: key and both working-directory: values all read packages/desktop. desktop-release.yml, eslint.config.js and .prettierignore contain zero occurrences of the old name at this head. Of the 121 files, the overwhelming majority are pure renames with no content change; the substantive edits are the reference updates above plus comments.

Both remaining references to the old path are intentional, and I read them rather than counting them

scripts/check-desktop-isolation.js keeps packages/desktop-shell in nativePrefixes alongside the new packages/desktop, with the reason stated: nothing should be created under the pre-rename location again, and it must never enter a workspace set. Dropping it would have removed the tripwire exactly when a stale branch is most likely to recreate the directory.

The two occurrences in ci.yml are inside the comment that documents the rename's own hazard, and the job carries two guards plus one disclosed gap:

• The changed-files filter includes .previous_filename as well as .filename, with the comment naming why — renaming a file out of the crate changes it, and only the old path says so. Without that, moving a source file out of packages/desktop/ would not trigger the crate compile.
• The Cargo.toml existence guard sets changed=false with a ::notice:: when the filter matched but packages/desktop/src-tauri/Cargo.toml is absent, so a pre-rename head that matched only because ci.yml itself changed does not fail with a missing working directory blamed on the PR.
• The gap is disclosed rather than papered over: a pre-rename head's files sit under the old path, the filter no longer matches, the job skips and reports green having compiled nothing, and because a 100%-similarity directory rename merges cleanly with an edit to the old path, such a branch reports no conflict either. Those branches need a rebase. The description also records that widening the filter to packages/desktop(-shell)?/ was considered and rejected, with the reasoning — on its own it is a no-op, since the guard would set changed=false anyway, and making it real means resolving a crate directory and plumbing it into four sites, which is a transition-window redesign rather than a rename.

scripts/tests/desktop-isolation.test.js is what keeps that coherent: it pins all five ci.yml crate-path sites to one directory, asserts that directory's src-tauri/Cargo.toml exists, and pins nativePrefixes superset-shaped against the root workspace negations so the old-path tripwire stays legal. The description reports it was verified by mutation — reverting the filter alternative or either working-directory, dropping packages/desktop from nativePrefixes, or dropping !packages/desktop from package.json each turns it red. That is the right shape of test for a rename, where the failure mode is a reference nobody greps for.

The non-mechanical edits are corrections the rename forced

packages/cli/src/commands/review/lib/workspaces.ts changes nine lines and every one is inside a comment: the negation-ownership examples that used packages/desktop-shell as their illustration now use packages/desktop, with the surrounding reasoning about fallback ownership preserved verbatim. No logic moves.

packages/desktop/src-tauri/src/main.rs changes two comment lines, and it is the most interesting edit in the diff. The old comment cited packages/desktop/packages/shared/src/config/storage.ts — a path belonging to the Electron package #9085 removed, which was already dead. Left alone, this rename would have made that dead citation look live again, since packages/desktop now exists and means something different. It is rewritten to name the removed Electron package and the PR that removed it. That is the class of error a rename produces and a grep for the old name cannot find.

One item in Risk & Scope is now stale, in the harmless direction

The description says #12649 is open and edits packages/desktop-shell/scripts/prepare-runtime.js and test-release.js, and should be rebased onto this rename before merging. #12649 merged to main since this branch last took main, so that ordering advice no longer applies. The outcome is still the one the description relies on elsewhere: a 100%-similarity rename merges cleanly with edits made at the old path, so #12649's comment changes follow the files to their new location rather than being lost, and GitHub reports this PR MERGEABLE against a main that contains them. Worth a line in the description so the next reader does not act on advice that has already been overtaken, and worth knowing that the two files #12649 touched are exactly the ones the skipped node scripts/test-release.js step exercises — which is the concrete form of the rebase gap documented above.

CI

28 checks pass and 26 are skipped at this head, with web-shell E2E Smoke and review-pr pending and nothing failing. The lanes that would catch a missed reference are green: both Desktop Shell lanes, which compile the crate at its new path, all three Install lanes, which would fail on an incoherent workspace set, Test (ubuntu-latest, Node 22.x), Lint & Static and Integration Tests (no-AK, No Sandbox). I did not wait on the two pending lanes. No review thread is unresolved.

The one thing no lane can verify here is the actual desktop release from the renamed path — the workflow edits are path substitutions and the next desktop-v* tag is what exercises them, which the description states plainly rather than claiming coverage it does not have.