mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-24 16:16:13 +00:00
14 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Shaojin Wen
|
7edc16ba11
|
feat(review): say so when the bundle is older than the review it runs (#8390)
* feat(review): say so when the bundle is older than the review it runs Every `qwen review …` step runs the BUILT bundle, not the working tree. So editing a review command, or switching to a branch that contains one, changes nothing about the run until someone rebuilds -- and the failure is silent and total: the run behaves like the last build, and every conclusion drawn from it is a conclusion about that build. Measured on 2026-08-02, dogfooding /review against #8368 from a checkout whose bundle was fourteen hours old. Three things were invalidated at once and none announced itself: `drive` and `mock-provider` had merged that morning and were absent from the binary, so "the agent never reached for them" measured nothing; and #8345's guard against scoring a mutant `survived` when its own collocated test was red had merged too, so the run reproduced the bug it fixed and filed three findings the current code holds as `inconclusive`. The round was discarded and re-run after a rebuild. `parse-args` is the first command of every review, which makes it the only place a notice reaches a reader before they act on a result. It names the file that is ahead, by how much, what actually runs from the bundle, and the command to rebuild -- "rebuild" without evidence is advice nobody can check. mtime, not git: the question is whether this bundle was built from this source, and a git comparison answers a different one. A margin absorbs a checkout, which writes everything at once in no guaranteed order. An installed package has no sources beside it, finds nothing to compare, and stays silent -- a check that cannot see the files must not accuse the build. Also documents `findings --test-delta` for users: it can lower a severity, and therefore change what the verdict is computed from, so it belongs beside `--outcomes` rather than only in the skill. * fix(review): watch the file every subcommand is registered in `packages/cli/src/commands/review.ts` is where all 30-odd subcommands are imported and registered, and it sits beside the directory rather than in it -- so a new command, or a changed dispatch, was exactly the change this check could not see. A root may now be a single file, which is what that one is. Confirmed end to end: with `review.ts` three hours ahead of a fresh bundle, the warning names it. Also two comments that did not match the code: symlinks of every kind are skipped, not only directories (`isFile()` is false for a symlinked file too), and the module now says what `QWEN_CODE_CLI` already covers -- talking to a different program -- so it is clear this guards the other half, the right program built before the change. * fix(review): compare content, because a timestamp check cried wolf The first version compared the bundle's mtime against the newest review source, and it was wrong in the direction that matters most. `git checkout` rewrites every file that differs between two commits, so returning to the branch a bundle was built from re-stamps exactly those files and the check calls a byte-for-byte correct bundle stale. Measured: with the sources untouched and the bundle two minutes older, it warned. A line that fires when nothing is wrong teaches its reader to skip the line, which would have made this worse than absent. The build now stamps a digest of the review sources it bundled into `dist/review-sources.sha256`, and the check re-derives that digest from the tree and compares. No margin to tune, no clock to trust, and no answer but the true one. Verified end to end across all five cases: a clean tree is silent, a source touched but unchanged is silent, and a real change under any of the three roots -- the command directory, the `review.ts` that registers them, the bundled skill -- warns. The digest is now one rule stated twice, since the build script cannot import the package it runs before building. `scripts/tests/review-source-digest.test.ts` holds the two equal, on this repo and on a synthetic tree that exercises the file-shaped root; a package test may not reach into `scripts/`, so it lives on the side of the boundary that may. Paths are folded relative to the repo root with separators normalised, and the file list is sorted -- `readdir` order is a property of the filesystem, so without it a bundle built in CI and a tree cloned locally would hash the same source differently and every run would warn. * fix(review): a diagnostic must not kill the run, and tests are not the bundle Two Criticals and five suggestions from review, all verified before changing anything. `writeStderrLine` throws on EPIPE, so stderr piped to `head` would have killed the review before it parsed a single argument -- a warning that destroys the run it was warning about, and the opposite of this change's own invariant. `writeStderrLineSafe` is the convention for diagnostics in this subsystem and is what it calls now. `reviewSourceRoots` builds paths with the platform `join`, and the test asserted forward-slash literals, so all three elements would have failed on the merge queue's Windows leg -- which the pull_request event never runs, so the green CI here proved nothing about it. Test files left the digest. esbuild follows imports from the CLI entry and no test is reachable that way, so folding them in fired the warning for an edit that cannot change a byte of the bundle -- the false positive this module already rejected once. 112 files became 61, and a test-only edit is now silent while a production one still warns. The handler wiring is tested at last, against a real temp tree rather than a mock of the reads under test: the derivation from `process.argv[1]`, the stamp read, and the warning. All three mutations the review named -- dropping the call, reading the stamp from the wrong directory, collapsing repoRoot to distDir -- now redden it. Also: the stamp's filename is pinned across the boundary it crosses (the build wrote a literal while the check read `DIGEST_FILE`, so a one-sided rename would have silenced the feature with every test green); the digest is computed only when there is a stamp to compare it against, instead of hashing a hundred files for a value the first guard discards; the `rebuildCommand` parameter no caller ever set is gone; and the build script's comment no longer claims a code-sharing relationship that does not exist. * fix(review): fixtures are not in the bundle either The same false positive, a third time and one directory over. Excluding tests from the digest was right and incomplete: `review/__fixtures__` holds four files — three responder modules and a captured comment — that a test loads at runtime, from no import the bundler follows. Measured against `dist`: none of the four appears in it, so editing one changed the digest while the bundle stayed byte-identical and the warning claimed a review command had changed. Both walks skip the directory now, and the parity test's synthetic tree grows a fixture and a `.spec.tsx` so the two implementations are held equal on the whole exclusion, not just the part the first case exercised. Reverting one side reddens the local case AND both parity cases, which is what that guard is for. Verified the other direction too, since an exclusion can overshoot: every review source that reaches `dist` is still covered. `DESIGN.md` and `SKILL.md` both ship and both remain in the digest — checked, not assumed, after two rounds of this exact mistake. Six cases end to end after a rebuild: a clean tree, a test edit and a fixture edit are silent; a production edit, a `review.ts` edit and a `DESIGN.md` edit each warn. * fix(review): allowlist the stamp, and stop guessing what the bundle holds The Critical first: `create-standalone-package.js` fails on any top-level dist entry outside its allowlist, and `review-sources.sha256` was on neither list. The next release would have aborted the standalone archive on all five targets, and no PR-time job runs the packager, which is why this suite is green. Allowlisted -- shipping it is harmless, since a standalone install has no `packages/` to compare against and the check stays silent there. `lib/test-utils.ts` was in the digest: test support with a production-looking name, imported by two test files and nothing else. That is the fourth patch to one rule -- `.test.ts`, then `__fixtures__/`, then this, plus `.DS_Store` -- and each was found by a reviewer after it shipped. So the rule stops being a list somebody remembers to extend: a new test asserts the property the list approximates, that every file the digest folds in is reachable from production code and nothing reachable is left out. Dropping `test-utils.ts` from the exclusion reddens it, which is the fifth instance failing in CI instead of in a review. Three branches that no test reached, each with a mutant the review measured surviving the whole suite: the walk's symlink skip (a directory cycle would send the first command of every review into unbounded recursion), the read-failure path (hashing the survivors of a concurrent checkout would accuse a tree that is merely mid-change), and the build's stamp call site (removing it left the scripts suite green while `npm run bundle` silently stopped writing the stamp). All three now redden. And `unmeasured` had no reader, so the one edge this check cannot measure but can see -- sources present, stamp absent -- passed in silence. That is the state of every existing checkout the moment this ships, and it is exactly the silent failure the change was written to end. It now says so, while an installed package, which has no sources either, still says nothing. * fix(review): the guard was shallower than the property it claimed The guard added last round asserts that every file in the digest is reachable from production code. It did not: a file imported by nothing passed, because the filter also required some test to import it; only `.ts` was inspected, so a test-only `.tsx` or `.mts` helper walked through; and it read static imports only, while this directory has nine `await import('./…')` edges. It asserts the property now — every extension, orphans included, dynamic edges seen — and the tree has no violators, so the strictness cost nothing today and is there for the next file. `__snapshots__` joins the exclusions. `vitest --update` regenerating a snapshot would have moved the digest with the bundle byte-identical; none exists under the review roots today only by chance, and 120 `toMatchSnapshot()` calls live elsewhere in this package. Three couplings that no test held: - the allowlist entry that fixed the release-breaking R2-1 -- reverting those five lines left the whole scripts suite green, and the next failure would have been a release aborting on all five targets. `isAllowedDistEntry` is exported and the stamp's own name is asserted against it, so a one-sided rename fails here instead; - the `.DS_Store` member of `NOT_BUNDLED_FILE`, absent from the repo and so from the parity tree -- one-sided removal stayed green while a macOS checkout would digest differently on the two sides forever; - each `unmeasured` reason. Swapping the two arguments at the single call site kept all 76 tests green while telling a pre-stamp checkout its sources were missing. And two comments that said the opposite of the code beneath them: the digest is computed unconditionally on purpose (the pre-stamp notice needs it), and `NOT_BUNDLED_FILE` helpers are deliberately not importers, since nothing reaches the bundle through a file the bundle does not contain. The two stderr diagnostics are documented for users, beside the sibling paragraph this PR already added. * fix(review): measure only the layout that can carry a stamp `npm start` launches `node <root>/packages/cli`, and node sets `argv[1]` to that directory -- so the derivation found sources under `<root>` with no stamp beside them and printed "could not check" on every review, forever, with advice that could never make it stop. That is the fires-when-nothing-is-wrong failure this change argues against, on the path `start.js` sets `QWEN_CODE_CLI` to precisely so reviews reach that build. Only a `<root>/dist/cli.js` layout is measured now; anything else has no stamp to find and no way to grow one. The build-side digest could kill `npm run bundle` where the check side degrades gracefully: a file vanishing mid-walk threw out of the hash loop, and the stamp is the copier's last step, so the build would fail with every asset already in place. Caught and skipped -- a missing stamp is `unmeasured`, which the runtime already treats as an acceptable answer. The skill now says what to do with the warning, which is the half that makes it reach a human: `parse-args` runs inside an agent's shell tool, the user reads the agent's summary rather than raw stderr, and a line nobody repeats is a line nobody sees -- which is how the 2026-08-02 round went wrong in the first place. It also records that the instruction cannot help the run that needs it, since the skill comes from the same bundle. And the scope is stated where silence could be over-read: the digest covers the review commands, the file that registers them, and the bundled skill -- not the shared helpers those import. A quiet run means the review code matches the bundle, not that the tree does. * fix(review): refuse to certify a bundle the copier may not describe The stamp described the tree as the COPIER saw it, and the copier runs after esbuild -- so a source edited in between, or `copy_bundle_assets.js` run on its own (it self-executes), wrote a digest certifying a `cli.js` built from something else. Silence then means "verified fresh" when it is not, and that is the only direction here where a quiet run is affirmatively wrong rather than merely uninformative: every other gap degrades to `unmeasured`. Timestamps are the wrong tool for judging staleness and the right one for judging whether this stamp can be honest at all, so the build refuses when any source is newer than the bundle it would attest to, and says why. Driven for real: touching a review source and running the copier alone now prints "skipped the source digest rather than certify a bundle it may not describe". `it('counts the same files')` compared nothing -- it asserted `> 50` on the build side while the check side exposes no count, so the title claimed a parity the body never checked, and the margin over the real 56 made it a future false alarm in `scripts/` for an unrelated change. Removed; the digest parity already holds the file set. "Root is a file" was inferred from `readdirSync` raising ENOTDIR, an assumption about every platform's libuv on the one root that is a file -- `review.ts`, where "a new subcommand was registered" lives. `statSync(root).isFile()` says it instead. And the check itself moves out of the handler into `bundleStalenessNotices`, which is where the rest of it already lived. `parse-args` is about parsing arguments again, the wording is testable without the yargs harness, and a second caller -- an agent resuming a review never runs step 1 -- is one line. * fix(review): align the twin walk, and stop a test from passing on nothing The build side still inferred "this root is a file" from `readdirSync` raising ENOTDIR, one commit after the check side stopped doing exactly that and said why. A platform that maps the case differently would drop `commands/review.ts` from one digest and not the other, and a byte-for-byte correct bundle would warn on every review forever, on that platform alone, with rebuilding reproducing the same one-sided walk. Both sides ask `statSync(...).isFile()` now. Fixing one half of a pair and not the other is the mistake this file keeps making. The filename parity test had been passing on nothing since the previous commit: it matched `writeFileSync(join(distDir, '…'))` against the script's source, the literal moved into a `stampPath` variable, and the regex returned `undefined` so the assertion compared against nothing. It runs the build against a fixture now and reads the name off `dist/`, so it measures what the build does instead of what its source looks like. Renaming the stamp on one side reddens it. Also from review: the duplicated comment block in `parse-args`; an unreadable source now says the check could not run rather than passing in the same silence as an installed package, which is what the docstring already promised; the "could not check" line no longer asserts that the checkout predates the feature, since the build has three refusal paths and one of them means the opposite; every refusal removes an existing stamp, because leaving an older attestation beside a newer bundle is a weaker form of the certifying it refuses; and `drive` calls the check, which the module comment argued for and the diff had not done -- a resumed review never runs step 1, and that is where the long work starts. * fix(review): pin the regex group the parity tree missed, and say source, not command * fix(review): allowlist what the bundle holds, and cover the drive notice (#8390) * fix(review): treat unreadable review sources as unmeasured (#8390) * test(review): pin the stamp guard mutations that survived the suite (#8390) * fix(review): close staleness-check gaps and pin the round-4 survivors (#8390) * fix(review): close round-5 staleness gaps for parity, refusals, and partial checkouts (#8390) * fix(review): close round-6 gaps in the clause classifier, symlink layout, and pin honesty (#8390) * fix(review): close round-7 gaps in the closure oracle, parity pin, and refusal pins (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-8 gaps from the maintainer review (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-9 nits from the maintainer review (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): pin the lease root in the synthetic digest parity case (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Autofix <autofix@qwen-code.dev> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
易良
|
d7e2892a7c
|
fix(cli): avoid updating active CLI processes (#6874)
* fix(cli): avoid updating active processes * fix(cli): close update relaunch gaps * test(cli): fix standalone update source path * fix(cli): reset deferred update per relaunch |
||
Nothing Chan
|
b19ebd8fc6
|
fix(packaging): bundle clipboard addon in standalone builds (#6708) | ||
|
易良
|
fbdaa52c52
|
Gate browser automation MCP on external adapter (#6472)
* feat(cli): gate browser automation adapter * fix(cli): close browser automation review gaps * test(cli): cover browser automation gates * fix(cli): close browser automation review gaps * fix(cli): close browser automation review gaps |
||
jinye
|
aa8f9bb993
|
fix(standalone): Route serve shim through cli-entry (#5977)
* fix(standalone): route serve shim through cli-entry Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#5977) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
易良
|
8935398b12
|
fix(release): skip dist/node_modules when building standalone archives (#5878) | ||
qqqys
|
8809c16b57
|
fix(voice): bundle native audio addon into standalone archives (#5628)
Standalone archives shipped only curated dist/ entries, so the esbuild-external @qwen-code/audio-capture addon couldn't be resolved at runtime — streaming voice was unavailable in standalone installs (batch only, and only with SoX on PATH). create-standalone-package.js now bundles the addon into lib/node_modules (where the bundled lib/cli.js resolves bare specifiers): the trimmed package.json (install hook removed; type/exports kept for ESM resolution) + dist + only this target's prebuild (win-x64 -> win32-x64) + its zero-dep runtime dependency node-gyp-build. Targets without a matching prebuild (e.g. local builds) ship without it and degrade to SoX/arecord as before (warns, doesn't fail). The release pipeline already downloads prebuilds before packaging. Refs: #5502, #5590. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
Shaojin Wen
|
715ef938f5
|
feat(cli): serve the Web Shell UI from qwen serve (#5392)
* feat(cli): serve the Web Shell UI from `qwen serve` `qwen serve` now serves the built Web Shell SPA at its root on the same origin as the API, so a released binary exposes the browser terminal without the dev-only Vite server (the `npm run dev:daemon` two-process setup is unchanged for development). - New `webShellStatic.ts` mounts `/`, `/assets/*` and an SPA deep-link fallback. The fallback uses the same document-navigation discriminator as the Vite dev proxy so it never shadows API JSON 404s. - The static shell is registered BEFORE bearerAuth (a browser can't attach a token to a `<script>` subresource or an address-bar navigation; the shell carries no secrets and every API route stays token-gated). HTML responses set CSP + X-Frame-Options + Referrer-Policy + no-cache. - `--open` launches the browser at the daemon URL (with `?token=` when set) once the listener is up, guarded by `shouldLaunchBrowser()`. - `--no-web` opts out for an API-only daemon. - Bundle / npm publish / standalone packaging now ship `dist/web-shell/`. Missing assets degrade to API-only with a breadcrumb, never a hard fail. Tests: +6 cases in server.test.ts (root shell, assets, SPA fallback, non-navigation 404 passthrough, security headers, --no-web off). * fix(cli): address review on Web Shell serving Review fixes for #5392 (qwen-code-ci-bot): - [Critical] SPA fallback no longer shadows /health or /demo on non-loopback binds — those paths fall through to their own routes / bearerAuth instead of receiving index.html. - [Critical] --open trims the bearer token before putting it in the browser URL, matching runQwenServe's own trimming, so a trailing newline from `$(cat token.txt)` no longer makes every API call 401. - --open is wrapped in its own try/catch so a failed browser launch can't take down the already-listening daemon; it normalizes wildcard binds (0.0.0.0 / ::) to loopback, and only fires when the UI is actually mounted (new RunHandle.webShellMounted). - resolveWebShellDir() now requires BOTH index.html and assets/, so a partial build degrades to API-only instead of serving a shell whose chunks 404. - runQwenServe logs a positive "Web Shell UI served from <dir>" breadcrumb, and warns that on a non-loopback bind without --allow-origin the shell is read-only (same-origin POSTs are blocked by the CORS wall). - Document the --open token-in-process-list exposure in help text + a stderr note when a token is forwarded. - Tests: POST method guard, sec-fetch navigation signal, /health not shadowed, sendFile 500 path, plus isDocumentNavigation and resolveWebShellDir units. * fix(cli): harden Web Shell asset resolution and send-error logging Second-round review (claude /qreview on the initial commit): - resolveWebShellDir() now walks up from this module to find a sibling packages/web-shell/dist, covering the transpiled layouts the previous fixed `..` depth missed — per-package `tsc` output and the integration daemon harness (packages/cli/dist/index.js), which would otherwise resolve to nonexistent paths and silently run API-only. - sendFile failures are no longer silent: log the error (matching the /demo handler — previously the only 5xx path that emitted nothing) and res.end() a half-streamed response instead of leaving the client on a 200 with a partial body. The remaining comment (open-browser inside the boot try) was already fixed in |
||
|
易良
|
511a22864b
|
fix(release): allow cli-entry.js in standalone dist allowlist (#5153)
The OOM-prevention work in #4914 added a dist/cli-entry.js bin wrapper (re-spawns node --expose-gc cli.js) via prepare-package.js, but did not register it in the standalone packager's strict dist allowlist. The release job then fails with: Error: Unexpected dist asset: .../dist/cli-entry.js Add cli-entry.js to DIST_ALLOWED_ENTRIES, same fix as #5049 did for fzfWorker.js. |
||
yao
|
f9080e44fb
|
fix(cli,core): harden OOM prevention — idempotent compaction tests, explicit GC, debug log defaults (#4914)
* test(cli): add compactOldItems idempotency regression tests
Cover the scenario fixed in commit
|
||
|
易良
|
c2962eef73
|
fix(release): allow fzfWorker.js in standalone dist allowlist (#5049)
esbuild emits dist/fzfWorker.js as a standalone entry next to cli.js, but create-standalone-package.js's DIST_ALLOWED_ENTRIES did not list it, so 'Build Standalone Archives' failed with 'Unexpected dist asset'. prepare-package.js already whitelists it for the npm tarball; this syncs the standalone packer. |
||
|
易良
|
aef3e704b4
|
feat(installer): verify release assets + switch public docs to standalone entrypoint (#3855)
* fix(installer): tighten verifier base-url + clarify test helper
Three small refinements from the second review pass:
- normalizeHttpsBaseUrl rejects everything except https, since real release
URLs are always HTTPS. Accepting http previously would let an operator
silently target a stale or attacker-controlled mirror.
- Drop EXPECTED_RELEASE_ASSET_NAMES from the public exports; it was only
used internally for the verification log line.
- Rename the test helper standaloneChecksumContent to
placeholderChecksumContent and document that the hashes in its output are
placeholders — the remote verifier does not download archives or compare
hashes, it only validates that SHA256SUMS lists the expected names and
that each archive URL is reachable.
The non-https rejection test now also covers `http://` in addition to the
existing `file://` case.
* style(installer): align installer completion output
* revert(installer): keep hosted installer output unchanged
* fix(installer): address release validation review feedback
* docs: switch public install commands to standalone hosted entrypoint
Update README, quickstart, and overview to point at the new
install-qwen-standalone.sh / install-qwen-standalone.ps1 hosted URLs.
Add standalone uninstall instructions to Uninstall.md. Remove the
staged-rollout note from INSTALLATION_GUIDE.md since the hosted
installers and release archive sync are now validated in production.
* docs: clarify pull request size guidance
* fix(installation): harden standalone release validation
* fix(installation): redact release verifier credentials
* feat(installer): add visual branding to Linux/macOS install script
Add brand-colored ASCII art logo, custom download progress bar with
Unicode block characters, and step indicators [1/3] [2/3] [3/3] to
match the quality of competing CLI installers.
* fix(test): update stale assertion after guide text was removed
The text "Public installation documentation" was removed in
|
||
ChiGao
|
9d20536343
|
perf(cli): code-split lowlight to cut startup V8 parse cost (#4070)
* perf(cli): code-split lowlight to cut startup V8 parse cost
Move the syntax-highlight engine out of the synchronously-parsed cli.js
entry into a separately-emitted chunk and load it via dynamic import on
the first code-block render. Until the chunk arrives, code blocks render
as plain text; the next React commit of the surrounding subtree picks up
the highlighted version, so users never see incorrect highlighting –
just an imperceptibly later transition for the very first code block.
Mechanics:
- esbuild config: switch entry to outdir + splitting:true so that
`await import('lowlight')` produces an actual on-disk chunk that's
only parsed by V8 when first needed.
- esbuild-shims: rename injected __dirname/__filename to qwen-prefixed
symbols + use `define` to redirect free references. Previous inject
collided with vendored libraries (yargs) that ship their own
`var __dirname` ESM-compat polyfill once splitting flattens chunks.
- prepare-package: include the new chunks/ directory in the published
package's files list.
- CodeColorizer: keep the public colorize{Code,Line} signatures and HAST
rendering identical; on first call when the chunk hasn't loaded it
returns the plain line and fires the dynamic import via a tiny
standalone loader module.
- lowlightLoader (new): isolates the lazy-load surface to a module with
zero transitive imports (no themeManager, settings, or core). This
lets test-setup prime the cache without dragging the whole UI module
graph into every test file, which was observed to perturb theme and
settings test outcomes when CodeColorizer was imported directly.
- test-setup: await loadLowlight() once via the standalone loader so
synchronous snapshot tests see the highlighted output deterministically.
Measurements (real $HOME, n=15 interleaved A/B vs main HEAD, macOS):
| Metric | Before (mean±sd ms) | After (mean±sd ms) | Δ | t | p |
| ------------------ | ------------------- | ------------------ | -------- | ------ | -------- |
| firstByte (wall) | 1633.5 ± 88.7 | 1475.8 ± 73.3 | -157.7 | 5.31 | 1.33e-5 |
| idle (wall) | 2048.7 ± 93.6 | 1902.3 ± 80.2 | -146.3 | 4.60 | 8.71e-5 |
| cli.js size | 25 MB | 6.9 MB | -18.1 MB | — | — |
Both metrics clear the +50ms-or-10% Welch's t-test bar by an order of
magnitude. cli.js drops 72%; total payload (cli.js + chunks/) is
similar but only cli.js is parsed at module-eval time, which is the
phase that dominates the user-visible startup gap.
How to validate:
npm run bundle
ls dist/ # cli.js + chunks/lowlight-*.js
node dist/cli.js -y # interactive UI still renders
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): resolve chunk-relative sibling paths under esbuild splitting
With `splitting: true`, esbuild hoists modules with shared dependencies
into `dist/chunks/`. Three modules derived runtime paths from
`import.meta.url` assuming they were co-located with `cli.js`; once
hoisted, `path.dirname(fileURLToPath(import.meta.url))` resolved to
`dist/chunks/` and sibling-asset lookups silently missed:
- `skill-manager.ts`: bundledSkillsDir → `dist/chunks/bundled` (actual
`dist/bundled/`). The `existsSync` guard swallowed the miss, dropping
all four bundled skills (`/review`, `/qc-helper`, `/batch`, `/loop`)
with no user-visible signal.
- `ripgrepUtils.ts`: `getBuiltinRipgrep()` → `dist/chunks/vendor/...`.
Falls back to system rg if installed, otherwise null on minimal
hosts — degrading grep to the slow internal scanner.
- `i18n/index.ts`: `getBuiltinLocalesDir()` → `dist/chunks/locales`.
User-visible behavior survives via the static glob import in
`tryImportBundledTranslations`, but the loose-on-disk override path
is dead.
Each module now strips a trailing `chunks` segment when present, so
the lookup resolves under `dist/`. In source / transpiled modes the
basename is never `chunks`, so the fallback is a no-op.
Also:
- Add `chunks` to `DIST_REQUIRED_PATHS` in `create-standalone-package.js`
so a regressed bundle that produces only `cli.js` fails the
pre-packaging check instead of shipping a broken archive.
- Expand `esbuild-shims.js` header so future contributors understand
that `__qwen_filename` / `__qwen_dirname` always resolve to the
shim's chunk file (dist/chunks/) and that sibling-asset lookups
must strip the `chunks` segment.
Reported by claude-opus-4-7 via Qwen Code /qreview on #4070.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* perf(cli): prefetch lowlight from AppContainer + harden loader
Three follow-ups to the lowlight code-split:
- AppContainer fires `loadLowlight()` from a mount effect so the dynamic
import is already in flight before any code block needs colorizing.
Without this, code blocks committed to ink's append-only `<Static>`
region before the import resolves stay plain text for the rest of
the session — Static can only be re-rendered via `refreshStatic`,
which is not wired to lowlight load completion. Common reachable
paths: short `--prompt -p` runs that finalize quickly, Ctrl+C-
cancelled first turns, and the first-paint history replay on
`--resume`. The startup parse-cost win is preserved (V8 still
parses off the critical path).
- `lowlightLoader.ts` latches the first import failure so subsequent
calls short-circuit to a rejected promise instead of re-attempting
`import('lowlight')` on every keystroke. The colorizer already falls
back to plain text on miss; recovery requires a fresh process anyway.
- `test-setup.ts` wraps the top-level `await loadLowlight()` in
try/catch. A transient import failure no longer crashes the entire
vitest run — tests that hit a code block render the plain-text
fallback and surface a warning.
- `CodeColorizer.tsx` header comment updated to point at the
AppContainer prefetch instead of claiming first-paint always sees
a loaded instance.
Reported by DeepSeek/deepseek-v4-pro and claude-opus-4-7 via Qwen Code
/review and /qreview on #4070.
Generated with AI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(bundle): extract resolveBundleDir helper, apply to extensions/new
Centralises the `chunks/` strip pattern that three sites
(`i18n/index.ts`, `skills/skill-manager.ts`, `utils/ripgrepUtils.ts`)
each duplicated after the round-3 fix in
|
||
|
易良
|
cb7059f54d
|
feat(installer): add standalone archive installation (#3776)
* feat(installer): add standalone archive installation * fix(installer): harden standalone archive installs * fix(installer): address standalone review findings * chore(installer): clarify review followups * fix(installer): stabilize standalone script checks * chore(installer): remove internal planning docs * chore(installer): simplify standalone release review fixes * test(installer): add Windows batch install smoke * test(installer): fix Windows batch smoke quoting * test(installer): preserve Windows cmd quotes * fix(installer): use robust Windows checksum hashing * ci: narrow installer debug matrix * fix(installer): address standalone review hardening * fix(installer): avoid Windows validation parse errors * fix(installer): simplify Windows option validation * fix(installer): harden standalone review fixes |