Files
qwen-code/packages/cli/src/commands/review/lib/workspace-scope.ts
T
易良andQwen-Coder 73aa65a4b4 chore(desktop): rename packages/desktop-shell to packages/desktop (#12653)
* chore(desktop): rename packages/desktop-shell to packages/desktop

The Electron-era packages/desktop was removed in #9085 and the OpenWork
downstream has forked, so the short name is free and the shell suffix is
the only thing left of the transition. Rename the package directory and
every load-bearing reference: workspace negations (npm + pnpm), the ci.yml
crate filter, desktop-release.yml, the isolation guard, lint/ignore configs,
scripts tests, CLI workspace-fixture tests, and living docs.

User-facing identity is untouched: the crate stays qwen-code-desktop,
productName stays "Qwen Code Desktop", the bundle identifier, updater
feed, release tag scheme, and the legacy electron-bridge asset names are
all unchanged. check-desktop-isolation now guards both path names so the
old one cannot re-enter a workspace set. Dated design docs under
docs/design/ keep their historical paths deliberately.

Refs #8596

* docs(design): follow the rename in the undated notification design notes

The web-shell-browser-notification-details pair is a living doc without a
date prefix, so it falls under the PR's update rule rather than the
historical-record exception.

* docs(desktop): stop citing paths the rename invalidated

Three prose sites and one CI comment still described the pre-rename world,
or contradicted themselves once `packages/desktop` named both the removed
Electron app and the living Tauri shell.

- ci.yml `desktop_shell`: the comment this branch added credited the
  Cargo.toml guard with keeping "branches from either side of the rename"
  green. It does not. A pre-rename head's files sit under
  `packages/desktop-shell`, which the changed-files filter no longer
  matches, so the job skips before the guard ever runs. Say that, and say
  what the guard does cover (a head the filter matched because ci.yml
  itself changed, with no crate at the new path).
- docs/design/web-shell-user-message-links.md: "desktop-shell routing" /
  "desktop-shell clicks" -> "desktop-host", the wording this branch already
  chose in LinkifiedText.tsx for the same rule.
- docs/design/telemetry-runtime-client-attribution-design.md: repoint the
  `runtime.rs` citation. It backs the live `QWEN_CODE_DESKTOP=1` claim, and
  the other document of record for that invariant -- the comment in
  acp-channel-fallback.ts -- was already repointed by this branch, so the
  two disagreed. Only the path token changes; the doc's dated findings are
  untouched.
- brand-create.mjs / SKILL.md / src-tauri/src/main.rs: name the historical
  referent as historical (the Electron `packages/desktop` removed in PR
  9085) instead of letting it read as a self-reference. In main.rs the
  citation `packages/desktop/packages/shared/src/config/storage.ts` now
  looked like a subpath of the file's own package. Rewritten, not deleted:
  the `QWEN_DEFAULT_WORKSPACE_DIR` clause next to it is the only in-code
  documentation of that override and still resolves.

Deliberately not swept: `scripts/check-desktop-isolation.js`'s
`packages/desktop-shell` tripwire, ci.yml's #8132 history comment,
`docs/design/9152-architecture-invariant-classification.md` (names both
spellings and is accurate), and the date-prefixed
`docs/design/2026-07-31-desktop-web-shell-release.md`.

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmug1iw422v

* test(scripts): pin the desktop crate path and the isolation guard's coverage

Both invariants fail green today, and this rename is the second time the
crate directory has moved.

`scripts/check-desktop-isolation.js` gained `'packages/desktop'` in
`nativePrefixes` -- the only production-logic line in this branch -- and
nothing read it. Deleting that line leaves the guard's output and exit code
byte-identical, because `isNativeLocation('packages/desktop')` then matches
no remaining prefix; a later PR that drops `!packages/desktop` from the root
workspace list would re-admit the package with CI green.

The `desktop_shell` job names the crate directory in five places (the
changed-files filter, the Cargo.toml guard and its `::notice::` text, the
rust-cache `workspaces:`, and two `working-directory:` values). Nothing
asserted they agree with each other or with a directory that exists, so the
next drift reports a skipped job as a passed one.

The new suite parses both instead of re-running them, and is superset-shaped
on purpose: `nativePrefixes` carries a fourth entry (the
`packages/desktop-shell` tripwire) that nothing negates, and an exact-match
assertion would invite deleting it. The npm/pnpm mirror of the negation list
is already pinned by package-scripts.test.js, so this reads package.json only.

Verified by mutation, each applied alone and then restored: reverting the
filter alternative or either `working-directory` to `packages/desktop-shell`,
dropping `'packages/desktop'` from `nativePrefixes`, and dropping
`!packages/desktop` from package.json all turn this suite red. Today no other
test does.

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmug1iw422v

* test(scripts): pin what the desktop-isolation tests actually claim

Four review findings on scripts/tests/desktop-isolation.test.js, each
verified by mutating the thing it claims to pin, before and after.

- The comment above the local `isNativeLocation` copy claimed a change to
  the script's rule "has to be made here too". Nothing reads the rule back
  from the producer -- `nativePrefixesFromSource` parses the array literal
  only -- and narrowing the script's rule to prefix equality leaves the
  suite green. Describe the copy instead of claiming coupling that does not
  exist.
- `still carries the Cargo.toml existence guard` matched only the guard's
  condition text. Flip the body's `changed=false` to `changed=true`, or move
  it after `fi`, and the suite stayed green at 5/5 while the job runs every
  gated step against a crate that is not there. Assert the assignment sits
  inside the guard's own `if ... fi` body.
- Both on-disk checks keyed on the bare directory, so git-ignored residue
  under `packages/desktop-shell` (`node_modules`, which survives a branch
  switch with an empty `git status`) reddened two tests on a tree with no
  defect and blamed the root workspace set. Key on `package.json`, which is
  what `packages/*` requires to make a directory a workspace member; a real
  re-entry still reddens both.
- The `startsWith(filterAlternative)` assertion compared a path built from
  `crateDir` -- `filterAlternative` with one trailing slash stripped -- back
  against `filterAlternative`, so it could not fail for any input. Dropping
  the trailing slash from the changed-files filter in ci.yml left it green.
  Replace it with the property that makes the alternative a path prefix
  rather than a bare string prefix.

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmugedurp3j

---------

Co-authored-by: Qwen-Coder <[email protected]>
2026-09-25 05:41:25 +00:00

287 lines
14 KiB
TypeScript

/**
* @license
* Copyright 2026 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/
// Which test suites a diff obliges a review to run — and when the answer must
// carry a caveat.
//
// `build-test` scopes its BUILD to the diff (see `workspaces.ts`), and for a
// while it tested only the workspaces the diff changed. That under-tests in
// exactly the way a compile cannot catch: a behaviour change in `core` leaves
// every dependent compiling and still fails their suites — the break surfaces
// in a consumer's tests or nowhere. So the test scope is the reverse-dependency
// closure of the diff: the changed workspaces plus everything that depends on
// them, computed from the same declared graph the build set uses.
//
// Every input that makes the scoped set possibly incomplete is DISCLOSED as a
// `caveat` on the scope — a fallback the report does not disclose is a claim
// ("these tests were run") the review cannot honestly make. Every caveat that
// applies is reported, strongest first: "nothing is silent" means composing
// the disclosures, not letting the first one hide the rest:
//
// - A workspace whose `package.json` does not parse (or has no usable `name`)
// is invisible to the dependency graph, so its reverse edges are missing
// and the closure may be silently too small — the confident false green
// this pipeline exists to prevent. The same holds for a member a glob
// ORDERING hides from the graph (a literal entry listed before a `*` that
// claims its parent segment): npm includes it, the walker cannot.
// - A changed file OUTSIDE every workspace can affect any package: the test
// scripts themselves live in the root `package.json`, and `scripts/` is
// imported by whatever chooses to. No per-workspace subset covers that.
// The one carve-out is the license family — a LICENSE edit cannot fail a
// suite. Docs-classified files are NOT carved out: this repo's own root
// AGENTS.md is read and asserted on by packages/cli's load-rules.test.ts,
// which is exactly the load-bearing prose the carve-out would certify as
// unable to fail anything. The root `docs/` tree IS carved out — no suite
// here reads it, and a caveat that fires on most PRs teaches the reader to
// ignore caveats. When the cost of erring is a sentence of
// disclosure, not a full-suite run, err toward disclosing.
// - A closure past HALF the testable workspaces is not a meaningful
// narrowing, and the report should say so.
// - A root suite that fans out over every workspace (`npm test
// --workspaces`) cannot run as one scoped command — it would repeat the
// whole suite inside a single deadline — so it does not run, and that is
// disclosed the same way.
//
// None of these fall back to the repo's root test command. That command — one
// `npm test` over every workspace — is exactly what cannot finish inside a
// command deadline on a large monorepo: this repo's suite took 31 minutes in
// CI against a 300-second deadline, and measured over recent PRs a third of
// diffs would have hit the fallback. A fallback that reliably times out is not
// coverage — it is zero signal framed as a failure. The scoped set is the run
// that can finish; the caveat says what it might miss.
import {
affectedWorkspaces,
isNegationExcluded,
reverseDependencyClosure,
workspaceDirFor,
type WorkspacePackage,
} from './workspaces.js';
/**
* What the test phase covers, for the report. The scoped set is what runs —
* there is no full-suite mode (see the module comment for why).
*/
export interface TestScope {
/**
* The dirs whose suites the run executes — exactly those with a test
* script, in scope (alphabetical) order, NOT run order. The run itself
* goes affected-first; the report's `test[]` array records that order.
*/
workspaces: string[];
/**
* Suites the whole-call budget stopped before they ran, when that happened.
* Structural, not just prose: `workspaces` names what ran, `notRun` names
* what the budget trimmed — a report must never list a suite as run that
* was not.
*/
notRun?: string[];
/**
* Present when the scoped set may be incomplete — rendered verbatim into the
* report so the review can state what it does not cover. Absent means the
* run covers everything the diff can break, as far as the graph can see.
*/
caveat?: string;
/**
* The scope's OWN caveat, before any budget-stop or resume clause was
* appended to `caveat` — the machine-readable half of the split that lets a
* continuation retire the appended clauses without re-parsing rendered
* prose. The prose parse was the defect: caveat segments interpolate
* workspace dirs from the reviewed diff, dirs may contain the segment
* separator, and the skipped/unmapped/excluded producers put their honest
* tail in the same segment AFTER the interpolated list — so a dir NAMED
* like a machine clause fabricated a segment boundary and retired the live
* limitation with it. A continuation now carries this string through
* untouched (empty when the scope had no caveat of its own) and rebuilds
* `caveat` from it plus its own current clause; nothing content-matches.
*/
liveCaveat?: string;
}
/**
* License-family files, which carry no extension the docs classifier could key
* on (`LICENSE`, `COPYING`, `NOTICE`, and suffixed variants like `LICENSE-MIT`
* or a generated `NOTICES.txt`). The optional text extension is deliberate:
* `LICENSE.js` must NOT match — a name is only inert when nothing executes it.
*/
const LICENSE_LIKE_RE =
/(^|\/)(LICEN[CS]ES?|COPYING|NOTICES?)(-[^/.]+)?(\.(md|txt|rst))?$/;
/**
* Is this changed file inert — unable to fail any test suite?
*
* Consulted only for files OUTSIDE every workspace, to decide whether they
* deserve the incomplete-scope caveat: a LICENSE edit cannot fail a suite, so
* it neither widens the run nor earns a disclosure. Everything else outside
* the workspaces is caveat-worthy — including docs-classified prose, which is
* load-bearing in this very repo (root AGENTS.md is asserted on by
* packages/cli's load-rules.test.ts).
*/
export function isInertLicense(path: string): boolean {
return LICENSE_LIKE_RE.test(path);
}
/**
* Outside files no workspace suite reads, however central they are to the
* repo's OTHER machinery: CI definitions (`.github/` — the workflow tests
* live outside the npm workspaces and are not run here either), the
* changelog, and editor/VCS dotfiles. Measured over 120 recent commits,
* naming these in the caveat would fire on ~2 of 5 diffs — a caveat that
* fires on most PRs teaches the reader to skim past caveats.
*/
const INERT_OUTSIDE_RE =
/^(\.github\/|CHANGELOG\.md$|\.gitignore$|\.gitattributes$|\.editorconfig$|\.idea\/|\.vscode\/)/;
/**
* Decide the test scope for a workspace monorepo. Pure given its inputs; the
* caveats are DISCLOSED in trust order — every one that applies, strongest
* first, because "nothing is silent" means composing the disclosures, not
* letting the first one hide the rest.
*/
export function resolveTestScope(input: {
changed: string[];
globs: string[];
packages: WorkspacePackage[];
/** From `readWorkspacePackages` — dirs the graph cannot see. */
skipped: string[];
/**
* The root package as a graph node, whenever the root manifest is a package
* with a build or test script. Its declared dependencies are reverse edges
* the closure cannot do without — a root that depends on a changed
* workspace is a dependent like any other, and a dependent reached THROUGH
* the root's name is dropped when the root is absent. Whether the root's
* own scripts RUN is decided separately (a build-only root joins the graph
* but no test list; a fan-out root test joins neither — see below).
*/
rootPackage?: WorkspacePackage | null;
/**
* True when the root's `test` script fans out over every workspace
* (`npm test --workspaces …`). Such a suite cannot run as one scoped
* command — it would repeat the ENTIRE suite inside a single command
* deadline, the fallback this module exists to refuse — so the root is
* dropped from the executed set and the non-run is disclosed as a caveat.
*/
rootTestFansOut?: boolean;
}): TestScope {
const { changed, globs, packages, skipped } = input;
const affected = affectedWorkspaces(changed, globs);
// The root goes FIRST: on a name collision a member must win — this very
// repo's root and packages/cli share the name `@qwen-code/qwen-code`, and
// last-write-wins would resolve a dependent of the CLI package to the root,
// silently dropping the member's dependents from the closure.
const graph = input.rootPackage ? [input.rootPackage, ...packages] : packages;
const scriptsOf = new Map(graph.map((p) => [p.dir, p.scripts]));
// Every caveat that applies is disclosed, strongest first. The graph
// caveat leads: a graph that cannot be computed makes the later,
// graph-derived answers the least of the report's worries.
const caveats: string[] = [];
if (skipped.length > 0) {
caveats.push(
`the workspace graph could not be fully computed: ${skipped.join(', ')} ` +
`${skipped.length === 1 ? 'has' : 'have'} a package.json that does not ` +
'parse, has no usable `name`, or is shadowed by a later workspace glob, ' +
'so a reverse dependency may be missing from the scoped set',
);
}
// A dir a positive glob claims but no manifest populates hosts no suite
// the scoped set can run, and its reverse edges are invisible to the
// closure — an empty scoped set there must not read as a complete answer.
// (build-test's unmapped guard reaches the same conclusion; the scope
// discloses it itself so the invariant does not rest on one call site.)
const unmapped = affected.filter(
(d) => d !== '.' && !scriptsOf.has(d) && !skipped.includes(d),
);
// Files a negation excludes (!packages/desktop — a separate toolchain with
// its own lockfile) cannot affect any included workspace's tests, so they
// earn no incomplete-scope caveat — but their own suites were not run
// either, and "nothing is silent" covers that too: disclose it as the
// softest line, not as an incompleteness. Nor does the root `docs/` tree
// (no suite here reads it) or the CI/changelog/dotfile family. Root-LEVEL
// prose (AGENTS.md) stays influential — this repo's load-rules.test.ts
// asserts on it.
const outside = changed.filter((f) => workspaceDirFor(f, globs) === null);
const excluded = outside.filter((f) => isNegationExcluded(f, globs));
const influential = outside.filter(
(f) =>
!isInertLicense(f) &&
!f.startsWith('docs/') &&
!INERT_OUTSIDE_RE.test(f) &&
!isNegationExcluded(f, globs),
);
if (unmapped.length > 0) {
caveats.push(
`the workspace globs claim ${unmapped.join(', ')}, but no readable ` +
'package.json there makes a graph member — no suite covers a change ' +
'inside, and a dependent of the diff may be missing from the scoped set',
);
}
if (influential.length > 0) {
caveats.push(
`${influential.length} changed file(s) sit outside every workspace ` +
`and are not inert (e.g. ${influential.slice(0, 3).join(', ')}); a ` +
"root script or config can affect any package's tests, and the " +
'scoped set cannot cover them',
);
}
if (excluded.length > 0) {
caveats.push(
`${excluded.length} changed file(s) sit in negated workspaces (e.g. ` +
`${excluded.slice(0, 3).join(', ')}) — excluded from the npm ` +
'workspace graph (a separate toolchain and lockfile); their own ' +
"toolchain's suites were not run",
);
}
const closure = reverseDependencyClosure(affected, graph);
// Exactly the suites the run executes: the closure, minus members that
// define no test script — naming those would claim coverage nothing can run.
let workspaces = closure.filter((d) => scriptsOf.get(d)?.includes('test'));
// A root suite that fans out over every workspace (`npm test --workspaces`)
// is one command that repeats the ENTIRE suite — the run that cannot finish
// inside a command deadline, which this module exists to refuse. The root
// stays in the graph (its edges matter) but leaves the executed set, and
// the non-run is disclosed rather than dressed up as coverage.
let fanOutCaveat: string | undefined;
if (input.rootTestFansOut && workspaces.includes('.')) {
workspaces = workspaces.filter((d) => d !== '.');
fanOutCaveat =
"the root's `test` script fans out over every workspace " +
'(`--workspaces`), so the root suite did not run — it cannot finish ' +
'inside one command deadline; the scoped member suites are the coverage';
}
// Testable-to-testable: a closure past half the suites that CAN run is not a
// meaningful narrowing, and counting script-less members would overstate it.
// The root suite counts on BOTH sides when it participates — with a running
// root the executed set is larger than workspace-only arithmetic models. A
// build-only root has no suite to count; a fan-out root cannot run.
{
const rootRuns =
input.rootPackage?.scripts.includes('test') && !input.rootTestFansOut
? 1
: 0;
const testable =
packages.filter((p) => p.scripts.includes('test')).length + rootRuns;
const scoped = workspaces.length;
if (scoped * 2 > testable) {
caveats.push(
`the diff's reverse-dependency closure covers ${scoped} of ` +
`${testable} testable ${
rootRuns ? 'suites (including the root)' : 'workspaces'
} — more than half, so the scoped set is not a meaningful narrowing`,
);
}
}
if (fanOutCaveat) caveats.push(fanOutCaveat);
return caveats.length > 0
? { workspaces, caveat: caveats.join('; ') }
: { workspaces };
}