mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-09-12 14:46:02 +00:00
* chore(web-shell): follow up #9812 review findings and deferred suggestions Address the remaining review suggestions from the @qwen-code/webui retirement (#9812) and the deferred items tracked in #11076: - Fix the 01-architecture.md adapter diagram path (missing session/). - Correct the followup-suggestions doc: generation fires on end_turn, and headless/SDK daemon clients should opt out to avoid the per-turn LLM cost. - Pin playwright to 1.61.1 to restore version parity with @playwright/test. - Restore file: scheme rejection coverage in Markdown.test.ts. - Pin the historical sessionStorage key prefix with a literal assertion. - Cover the legacy exported JSONL rejection and ChatRecord happy path. - Restore the shell-substring classifier trap with a live path. - Point useDaemonFollowupSuggestion docs at ChatEditor, not the deleted InputForm. - Replace the theme-toggle timing-race assertion and exercise the unhandledrejection fail-closed path in the document browser gate. Closes #11076 * fix(test): poll the theme class instead of a matcher vitest lacks The browser gate imports expect from vitest and drives playwright's Locator directly, so toHaveClass does not exist on the assertion and typecheck:integration failed with TS2339 on this line. Poll the class attribute through the same expect.poll idiom the neighbouring assertions use, which keeps the retry the assertion was switched to and needs no @playwright/test dependency. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtooas8sln * fix(test): assert the exported theme class exactly `document-main.tsx` toggles `dark` and `light` on <html> mutually exclusively and nothing else writes `documentElement` classes, so the attribute is always exactly one of them. Matching `/light/` as an unanchored substring would also accept `dark light`, which is the theme-toggle breakage this poll exists to catch. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtow5pgslz * fix(test): realpath the exporter main check and pin its JSONL guard Node realpath-resolves `import.meta.url` for the ESM main module but leaves `process.argv[1]` as invoked, so comparing the raw spellings made a symlinked invocation skip `main()` and exit 0 having written nothing — a silent no-op the harness reads as success. Compare realpaths instead and export the predicate so both directions are pinned. Also extract the input gate into `assertRenderableJsonl` so the legacy exported-JSONL rejection can be asserted directly: the previous tests only pinned the two predicates feeding it, so deleting the guard and losing the remediation hint kept every test green. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtow5pgslz * docs(followup): name the real ACP stop reasons, drop "unconditional" The parenthetical this PR added named `interrupted` as a daemon stop reason. The ACP `StopReason` union is `end_turn | max_tokens | max_turn_requests | refusal | cancelled`, so there is no `interrupted`; an interrupted turn reports `cancelled`. Name the four suppressing values the union actually defines so an integrator writing `stopReason === '...'` gets a branch that can fire. "unconditional" was wider than the code and contradicted the condition list on the same page: `Session.ts#maybeEmitFollowupSuggestion` returns early on the stop reason, the todo stop guard, `ui.enableFollowupSuggestions`, PLAN approval mode, a missing chat, and a non-model last history entry. Point at those conditions instead; the cost advice still holds because the `enableFollowupSuggestions === false` return sits before `generatePromptSuggestion` is reached. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtp6vibomd * test(web-shell): drop the duplicate historical-key assertion `persists under the literal historical key prefix`, added on this branch, became a byte-equivalent twin of `writes under the historical WebUI key`, which reached main with #11107 and arrived here through the merge at this PR's head: same `persistStableClientId('client-a', 'session-a')`, same literal `qwen-code-webui-client-id:session:session-a`, same expectation. One fact was reported under two names, so a prefix rename reddened two tests and an auditor could not tell which copy was load-bearing. The surviving test's comment already carries the rename rationale, so dropping the copy loses no coverage. Verified with `cd packages/web-shell && npx vitest run client/daemon/session/clientLifecycle.test.ts`: 19 passed before, 18 passed after. Mutating SESSION_CLIENT_ID_STORAGE_PREFIX to `qwen-code-webshell-client-id:session:` after the dedupe still reddens 3 tests (15 passed), including the surviving `writes under the historical WebUI key`; reverting the mutation returns to 18 passed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtp6vibomd * test(integration): drop the unreachable unhandledrejection arm The block this PR added after the CDN-failure loop could not reach the `window.addEventListener('unhandledrejection', showLoadError)` handler its comment claimed to exercise. `page.route('**/*', route => route.abort('blockedbyclient'))` fails the `<script id="transcript-renderer">` load, the template's capture-phase `error` listener (document-index.html:45-56, matching `event.target.id === 'transcript-renderer'`) calls `showLoadError()` first, and `setContent(..., { waitUntil: 'load' })` only resolves after that subresource has failed - so `data-render-complete='error'` and the alert text were already in place before `page.evaluate` dispatched anything, and showLoadError's one-shot guard (`if (document.body.dataset.renderComplete) return;`, document-index.html:30) made the synthetic rejection a no-op. The block was a strict duplicate of the loop's own `rendererBody === null` iteration: identical all-abort route, identical two assertions. Exercising the handler for real needs a renderer body that passes the document's pinned SRI integrity and then rejects during boot; no body this gate can serve passes SRI, so the arm cannot be made honest here. Deleting it returns the file to origin/main's state for this test - the handler and its missing coverage are both pre-existing on main (packages/web-templates is not in this PR's diff), so no coverage this PR was responsible for is lost. Verified: `npm run typecheck:integration` reports 0 errors in this file (6 remain, all in packages/** against generated/dist paths absent from an unbuilt worktree: channels/feishu, cli/src/generated/git-commit.js, web-templates/src/generated/*); `npx prettier --check` and `npx eslint` clean on the file. The browser gate itself was not executed locally - it needs the full build chain plus Chromium. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtpfg5ewmp * ci(review-context): route concurrent-runner changes to the helper-tests lane This PR added `integration-tests/concurrent-runner/export-html-from-chatrecord-jsonl.test.mjs`, the first `node:test` file under `integration-tests/`, and wired it into CI through the `HELPER_TESTS` env list in `.github/workflows/ci.yml`. The tracked review-context manifest still mapped the whole `integration-tests/**` path to `recommendedTests: ['integration-tests']`, whose vitest lane collects `**/*.test.ts` only (`integration-tests/vitest.config.ts:21`), so a future PR touching only the exporter and its `.mjs` test is told to run a lane that cannot collect the file - the same silent drop `HELPER_TESTS` exists to prevent. Add a narrower rule for `integration-tests/concurrent-runner/**` recommending `helper-tests`. Matching rules merge rather than first-match, so the lane list for that directory becomes the union and every other `integration-tests/` path is untouched. The pinned `expectedManifest` in `manifest-repository-context.committed.test.ts` moves with it, which is what that pin is for. Verified through the real provider (tsx importing `packages/cli/src/commands/review/lib/manifest-repository-context.ts`) because the packages/cli vitest globalSetup gate refuses to run on this unbuilt worktree: concurrent-runner .mjs -> ["helper-tests","integration-tests"] concurrent-runner .js -> ["helper-tests","integration-tests"] chat-transcript-document.test.ts -> ["integration-tests"] (unchanged) all 10 rules co-matching -> non-null, 80 relatedPaths, and every `paths` probe matched some rule Also checked the committed manifest deep-equals the test's pinned literal (10 rules on both sides), and that dropping the new rule breaks that equality, so the pin is not tautological. `npx prettier --check` and `npx eslint` clean on both files. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtpfg5ewmp * fix(deps): sync pnpm lockfile for Playwright pin --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|---|---|---|
| .. | ||
| assets | ||
| design | ||
| developers | ||
| e2e-tests | ||
| images | ||
| plans | ||
| superpowers | ||
| users | ||
| verification | ||
| _meta.ts | ||
| index.md | ||