feat(web-shell): add unified session sources - #11262

Local implementation and E2E acceptance

Implementation commit: a9be7a0a6115704885afb2cdadaafee4d212b255.

The product daemon, ACP child, tool scheduler, recording files, session APIs, and installed Chrome ran locally. Model replies came from a deterministic localhost fixture with a test-only credential; this does not claim external-provider acceptance. Only isolated test configuration and runtime directories were used.

The real API/lifecycle runs also covered 44 registration/admission checks, 22 restart/fork/rewind/compaction/archive checks, and 19 malformed-snapshot/write-recovery/privacy checks. Later UI-only changes were followed by targeted component/browser checks and final full build/typecheck. These counts describe separate runs with overlapping coverage; they are not a single aggregate suite count.

Confirmed behavior includes historical attachment access without registration, metadata-title precedence and deduplication, registration removal without automatic metadata resurrection, inert HTML after tab restoration, visibly rendered PDF body text, and binary downloads whose bytes match the original upload. Failed source/attachment-list reads remain independently visible. The actual metadata Retry issued no additional prompt or attachment upload; a definite rejected prompt cleaned the uploaded bytes.

Declared fault injection: targeted metadata/list HTTP 503 responses, definite prompt HTTP 400 rejection, and a capability response without source support; all unaffected requests used the real daemon. Recording tests used only dedicated backed-up fixtures for an unsupported snapshot version and a truncated final JSON line, plus a temporary read-only recording to verify the existing degraded-writer recovery boundary.

Local evidence remains under .qwen/e2e-tests/ in the implementation worktree. The PR includes cropped current UI screenshots for review. macOS and Chrome were exercised; Windows/Linux runtime behavior and external models were not tested locally.

Follow-up in a2b12d0, with current main incorporated by af2c846. The source lifecycle, cancellation, preview state, capability catalogs, translations and telemetry findings are addressed. In particular, standalone activation now binds sources, refreshes the model tool snapshot, and rebuilds the existing startup context after replay; a real provider request confirmed the default deferred record_source reminder without a forced reveal. The unified single-line Sources panel still starts at three entries and supports View all/Collapse, with historical attachments retained and metadata removal independent of file bytes.

Validation:

• Full build, typecheck and bundle passed, along with ESLint, Prettier and required locale checks. The source-fix commit passed 2,728 focused unit tests. After merging main, the four complete affected Web Shell test files passed again: 1,098 tests, including 802 App tests.
• Actual Chrome 152 against the real local daemon passed the final combined candidate's 20 unified-source checks, five desktop/narrow disclosure and single-line checks, and two standalone checks. This includes real uploaded text/image/PDF/binary/HTML previews, metadata deduplication/removal semantics, old-capability fallback, retry behavior, keyboard/focus/ARIA, Chinese labels, and the actual standalone model request. The model endpoint was an isolated local fixture; these results do not claim live external-model behavior.
• Earlier unchanged API/lifecycle paths passed 44 API checks, the capacity matrix, 38 real-daemon capabilities integration tests, and real cwd/fork checks covering stable source IDs/revision, copied attachment bytes, regenerated child IDs, and independent metadata removal. The final combined bundle SHA256 is 73281ca4cd94b363af34c530189a87a383fa228f489cb4b54bd2780c6cf5446f; served HTML/JS/CSS matched the local artifacts before and after the final browser run. No page or CSP errors were observed.
• The earlier browser CI failure was investigated separately: 22 retry traces contained 1,928 ERR_NETWORK_CHANGED resource-load failures, and 67 captured error pages showed the boot fallback. See the original CI artifact. This is separate from the reproduced and repaired code-contract failures; the newly pushed head is running fresh CI.

The review threads record explicit follow-ups for acknowledged copy cancellation/draining, unused restore-projection cleanup, host retry-notice aggregation, the portable path namespace, and a separate writer-error transport taxonomy. A plain timeout was deliberately not added around a mutation that may still hold the target writer. Required reviews and CI remain in force; this update is validation evidence, not a replacement for those gates.

CI follow-up in 59bc323: Electron Builder followed the newly introduced ancestor workspace configuration and selected pnpm while Live Host uses its own npm lockfile and install. Declaring an empty workspace boundary for the standalone app keeps the dependency collector at the app root. A regression test calls the actual locked builder detector and was verified red before the fix.

Validation passed with the locked Electron Builder 26.15.3: clean npm installation, typecheck, all 69 host tests, build, and the exact unsigned macOS packaging step from CI, including ASAR/native-resource checks, Info.plist checks, strict ad-hoc signature verification and entitlement checks. Dependency versions and the lockfile did not change. Fresh CI is running on this head.

Fixed the reproducible history viewport failure from Smoke runs 34191710804 and 34197997866.

The 200-record paging test was consistently losing the reading position by 286px. When scrolling outran virtual row rendering, the initial DOM anchor capture found no row. The response later prepended a page with no anchor to restore, leaving the virtualizer's estimated scroll offset in place. The viewport now refreshes the reading anchor immediately before admitting a current boundary response, including retries; canceled requests are checked again before admission.

Reproduction and verification:

• Reproduced the original 286px drift locally with Chromium CPU throttling at 6x. A new regression also failed before the fix when rows became visible after paging started without another scroll event.

• After synchronizing the latest remote branch, full build, typecheck and bundle passed, along with 93 focused viewport/store tests. Added cases cover canceled responses and cancellation from the pre-admission callback.

• All three original history browser scenarios passed against built static assets at CPU6, with zero retries. The 200-record scenario completed all eight wheel steps; the formerly failing second older-page step retained the message at offset 12px with scrollTop 18914.

• The synchronized source-based Smoke suite passed all 53 tests with 9 workers and zero retries (1.9m), including the 600px attachment/layout scenario. No browser page errors were recorded.

The existing browser assertions and timeouts are unchanged. The separate pre-fix composer startup failure was at the initial application-root mount assertion, before attachment layout checks; it is not attributed to the anchor fix.

@qwen-code /resolve

Qwen Code resolved the merge conflicts and pushed the branch update.

Root cause

#11250 "Improve split-view session navigation" (70cf363395) rewrote the same load() in TranscriptViewport.tsx this PR edits. Both fix one bug from opposite ends: a boundary load can start before the virtualized rows mount, so capture() returns undefined and no reading position survives. #11250 defers the load (retry capture over 8 frames, give up rather than load unanchored); this PR loads immediately and passes refreshAnchor as a beforeAdmit callback the store fires just before admitBoundary — the last moment old rows are mounted.

Semantic, not textual

Both sides edited one statement, void viewport.load(...). Kept #11250's loop, threaded this PR's callback through; only the last line differs from main:

if (!saved) {
  if (--remaining > 0)
    loadFrame.current = requestAnimationFrame(loadWhenVisible);
  return;              // never load without an anchor
}
anchor.current = saved;
void viewport.load(direction, refreshAnchor);

Load-bearing

• load() must keep writing loadFrame.current: handleScrollIntent() and the viewKey cleanup (both feat(web-shell): Improve split-view session navigation #11250, merged outside the conflict) cancelAnimationFrame it — this PR's original three-line load() would break deferred-load cancellation.
• The intent check must stay before capture(): it lets a pointerdown mid-retry abandon the load.
• refreshAnchor is not redundant — the loop anchors at request start, the callback re-anchors at admit. It must read via captureRef, not capture, or the long-lived callback freezes a stale toolSources/pin.

Could not verify: a NON-conflicted test contradicts this

TranscriptViewport.scroll.test.tsx auto-merged cleanly (both sides added an identical hideRows mock flag) but now holds mutually exclusive getTranscriptPage counts while rows are hidden: L286/L314 (main) expect 1, L377 (this PR) expects 2 immediately. No load() satisfies both. I kept main's behaviour, so this PR's own L377 test will fail at toHaveBeenCalledTimes(2) — it observes 1, the load being deferred rather than fired. Rework it to main's timing (start the request with rows visible, hide only while in flight) or drop it as covered by L286. Not edited: it did not conflict.

No build/lint/tests were run, per instructions.

Closed the current source security/bootstrap review findings and the CI regressions in cfa059c5fe.

Source metadata now rejects Unicode control and format characters while retaining significant filename whitespace. All three source operations use the established standalone activation guard; ordinary persistence failures keep their existing semantics. The history regression respects main's deferred-row loading and now proves response-time anchor recapture by mutation. The split-session tests use the renamed activity hook, and the HTML source/attachment preview test waits for its scheduled list load and asserts the button exists before clicking.

Validation: full locked install, build, typecheck and bundle passed; 871 focused unit/contract tests passed, including all 819 tests in the two previously failing WebShell files. The current original browser Smoke suite passed 58/58, and all three built-asset history cases passed at CPU6. All final browser/test runs used zero retries. The original failures, invalid-input red tests and the anchor callback mutation failure were retained as evidence; no assertion tolerance or timeout was relaxed.

The three Suggestion threads now explicitly record their remaining follow-up scope (general retry classification, session-scoped branch warnings, and an additional live-boundary mutation test). They are not claimed implemented. Valid Critical findings are fixed; fresh CI and required review remain authoritative for merging.

Resolved the conflict with main 1919ff97f5 in the standalone entry point. The merge preserves main's browser notifications, authentication bootstrap and Plan action while retaining this PR's Sources entry. The existing entry configuration test now explicitly asserts that Sources is present.

Locked installation, full build, typecheck and bundle passed. Source/history tests passed 103/103 and entry/authentication/notification tests passed 48/48. An independent browser check of the actual standalone entry confirmed Sources and its add action after startup. The built-asset 200-record history scenario also passed at CPU6, retaining the reading row at 12px. Final browser assertions and timeouts are unchanged.

The current original Smoke suite passed all 58 cases with zero retries (2.0m), including main's updated transcript/composer edge alignment. No browser page errors were recorded.

Fixed the two reproduced round-5 UI defects: hiding the environment panel now closes its Add source portal through the existing dialog state callback, and standalone URL source details stay open without requiring workspace file authority. File previews still require their trusted workspace target and matching stored cwd; unavailable standalone file references show an explanation and perform no workspace reads. Unsafe URLs remain non-clickable.

Both defects were reproduced in an isolated Chromium browser before editing. The same two browser cases now pass with zero retries, including modal/body-pointer cleanup and the standalone Open original link. New React regressions cover floating/docked hidden panels, restored outside dismissal, standalone URL and unavailable workspace-file behavior, and unsafe URLs.

The collection Suggestions were checked against actual runners: Live Host job 102382259435 logs show the packaging dependency-tree regression ran and passed. The serve route E2E is explicitly collected by the dedicated E2E runner. Packaging dependency ownership is recorded as a separate maintenance follow-up; this UI fix does not restructure that package.

Full build/typecheck/bundle and lint passed. Final complete App/environment tests passed 868/868, and the original full Smoke suite passed 58/58 (9 workers, zero retries, 1.9m). The first Smoke run had two transient failures (catalog count and details popup bounds); both passed unchanged in a focused run and then in the final full run. The first failure logs and traces are retained; no product, assertion or timeout change was made for them.

Acknowledged the convergence concern in review 5151984218. The current head is c817df5, with both R5 findings reproduced in Chromium and fixed, all 51 threads addressed, and its CI passing. Those results cover the fixes; they do not establish that the whole change has converged.

I will wait for the full review already running on this head (34327474704) and a round with no new Critical findings before pursuing merge. The old blocking reviews remain in place. If that full round still finds new Criticals across layers, I will bring back a concrete split proposal rather than treating another isolated fix as proof of convergence. No review gate is being dismissed or bypassed.

Following up on the convergence concern with concrete work on fe5ebb17a189905315a61dd8e959a344468f3371.

I reviewed the complete added/changed production logic across core persistence/tool activation, ACP ownership and copying, HTTP routes/error mapping, SDK events, and the WebShell state/preview paths. An independent test engineer reran the real daemon and browser workflows against isolated local fixtures. This was broader than the last changed lines, and it found one further real defect: an uploaded standalone attachment previewed normally until it was registered as a source, when the source preview incorrectly required workspace authority. That finding is fixed in this commit. Session-owned bytes now use the existing Blob/previewOnly path; workspace file references still require their trusted target and stored cwd, and HTML stays text-only. The new text/HTML regressions failed before the change and pass afterward, with revoked-owner access denied and no workspace reads.

Evidence and limits:

The PDF observation establishes frame visibility and sizing, not exhaustive browser-PDF rendering. Held activation and every possible race are not claimed newly proven by real E2E; existing focused tests cover those gates. The post-fix delta was checked against the complete boundary map with no additional confirmed Critical. This is author-side review plus independent execution evidence, not a self-approval or a substitute for your formal judgment.

There is also a concrete split option, with per-file ownership mapped locally: runtime/API/SDK (56 files, 3,486 additions), WebShell UI (20 files, 2,527 additions), independent history-anchor follow-up (5 files, 98 additions), and the packaging regression separately (1 file, 22 additions); docs account for another 698 lines. Runtime comes first, UI depends on its contracts, and public history/test coverage would be preserved. Splitting is ready to plan from those boundaries rather than deleting tests to reduce the number. No replacement PR has been published without agreement.

Please re-review the repaired head against the full convergence concern. All 51 inline threads are addressed; the two old formal bot requests refer to commits 1673176 and b265680 and their findings have been fixed or explicitly dispositioned. The maintainer's broader convergence discussion remains visible and the required review/CI checks remain in force.

Thanks for rechecking the previous findings. I checked R2-1/R2-2 against fe5ebb1 and am recording both as follow-up work, consistent with the repository's late-review rule to land only Critical fixes after roughly five rounds. This is not a claim that either improvement has been implemented.

• R2-1: preserving internal error causes and a more precise private-ACP error taxonomy is useful follow-up. One scope clarification: the invalid-copy example is a trusted private-parent operation; the bridge consumes its sourceError once and produces a copy warning rather than retrying that invalid target. Public invalid source input already receives invalid_source/400. The tested persistence failure contract remains a failed write with no success acknowledgement, followed by authoritative reload. A diagnostics follow-up should preserve useful causes without leaking private child paths through the HTTP response.
• R2-2: confirmed that first source access still materializes the transcript once. The complete audit explicitly listed large-history performance as not newly benchmarked. A bounded/selective implementation should be benchmarked separately and preserve the integrity rule: a malformed or unsupported newest snapshot must fail closed rather than fall back to an older pre-removal snapshot. The existing corruption/restore tests and real recovery probes are the acceptance baseline for that optimization.

The copy-to-an-already-populated-fork and cancellation points are also retained as follow-up considerations, not silently marked fixed. Current user-facing Critical defects have been addressed with regression and real-browser evidence; fresh CI and the requested full/maintainer review remain the merge gates.

Local real-environment verification (maintainer pass) — head fe5ebb17a1

I built this PR and drove it as a product: a real qwen serve daemon, a real ACP child, real transcript files on disk, real Chrome against the built Web Shell bundle. No mocked daemon, no mocked bridge. Model replies came from a local deterministic OpenAI-compatible fixture; everything else (scheduling, storage, REST/SSE, browser) was the real thing.

Bottom line: the feature does what the description says, and its stated failure boundaries hold — nothing certified a wrong result and nothing silently lost a file in any probe I ran. Two of the recorded deferred Criticals reproduce live, and one of them blocks a legitimate everyday input (a workspace file whose name contains a Persian/Indic/emoji joiner). Neither is a data-loss or wrong-answer defect, so this is mergeable in my view, with the invisible-character gate worth narrowing before or right after merge.

Rig

Fixtures in the workspace: a Markdown file, an HTML file containing <script>window.__RIG_HTML_EXECUTED = true;</script>, a 64×64 PNG, a one-page PDF with visible text, an application/octet-stream blob, plus two files whose names contain U+200C / U+200D.

Reviewer test plan — reproduced

Screenshots for each of these are in the gallery below.

The two deferred Criticals both reproduce

1. session-sources.ts:85 — the \p{Cf} gate rejects real files, not just decorative characters.

The finding as recorded says "blocking ZWNJ/ZWJ input", which reads like a cosmetic title restriction. Live it is stronger: text() also validates workspacePath, so a file that exists in the workspace cannot be registered at all if its name contains U+200C or U+200D — which is ordinary for Persian and Indic filenames and for any emoji ZWJ sequence. In the UI the failure is also mis-attributed: the Add-source dialog derives the title from the filename, so pasting the path first fails with "Invalid title", and only after you type an ASCII title do you get the real reason, "Invalid workspacePath".

Add source, path only	Add source, with an ASCII title

It is genuinely fail-closed (nothing is stored, nothing wrong is certified), so it is not a merge blocker — but the cheap fix is to keep \p{Cc} plus the specific format characters worth banning (the bidi overrides U+202A–U+202E and U+2066–U+2069, and U+FEFF) instead of all of \p{Cf}.

2. session-sources.ts:343 — one write failure latches the read-only listing, and repairing the disk does not clear it.

The PR's stated failure boundary is "a degraded recording writer rejects metadata writes until session recovery/restart". Live, the read side goes down with it, and it stays down after the cause is gone: commit() sets loaded = false on a persist failure, so the next list() re-enters load(), which does await recording.flush() — and ChatRecordingService.writeFailure is a permanent per-process latch that pre-dates this PR. Neither repairing the file nor POST /session/:id/resume clears it; only a daemon restart does.

The user-visible shape of this is the one flow the test plan already asks about — and it is where the two findings meet:

After the injected failure	After the disk is repaired, pressing Try again

Retry can never succeed, and pressing it adds a second, permanent error to the panel. Nothing is lost — the uploaded file stays listed and the restart-time list is correct (no false success) — but "Try again" is offering an action that cannot work. A read-only list() could serve the in-memory snapshot, or load() could skip flush() when it is only reading.

On the standing round-6 Importants

• R2-1 (error classification). Through every REST route I could reach, the mapping is correct: invalid_source→400, source_limit_reached→409, source_attachment_not_found→404, source_persistence_unavailable→503. The 4xx-flattened-to-503 case is confined to qwen/session/sources/copy, which has no REST route and is only reachable from the bridge's fork path — so the user-facing blast radius today is smaller than the finding implies. The second half of R2-1 is real and I hit it repeatedly: the daemon log shows only the generic message, never the underlying EACCES.
• R2-2 (full transcript parse). Measured rather than argued:

One cold read per session at ≈1.7 ms/MB (219 ms at 129 MB), warm reads flat at ~2 ms, and transient ACP-child RSS that tracks file size and does not settle back (+123 MB at 129 MB). The "synchronous spike stalls other sessions on the same child" part does not reproduce: the read is a streaming readline loop with an await per line, and a second session on the same child polled 60× during the 61 MB cold read saw p90 1.1 ms / max 2.5 ms. So: worth a tail-bounded read as follow-up, not a blocker.

Mutation matrix

8 of 10 killed. Both survivors are defence-in-depth rather than the primary path: M4 is the parse-side 200 cap, only reachable via a hand-edited or corrupted snapshot because the write path caps first; M8 drops workspaceCwd from sessionSourceId, which changes the hash but which I could not turn into a live divergence, since a session's project root is fixed for its lifetime. Worth one test each, not worth blocking on.

Local suite results at this head

One earlier acp-bridge run reported 2 failures; that run overlapped the 6912-test web-shell run on the same machine and it is green on a clean re-run — load flake, not a real failure.

Smaller things I noticed

• Error strings render route templates to end users: "GET /session/:id/sources: Stored sources are unavailable" and "POST /session/:id/sources: Invalid title" in the panel and the Add-source dialog.
• Opening a workspace-file binary shows a stale "Loading file…" line above GET /file: binary file: /abs/host/path/..., with no download affordance — the uploaded-attachment path for the same bytes offers a clean "Preview is not available for this file type" card with a download button. The absolute host path also comes through verbatim.
• At the 200-source cap, sending an attachment silently does not register it (source_limit_reached is deliberately treated as permanent and dropped from the retry queue). Nothing is lost — the file still shows as an uploaded file — but there is no signal at all. Fine as designed; flagging in case the silence was not intended.

Not verified

Owner-unavailable / owner-replaced preview redirection; "a definite prompt rejection cleans the uploaded bytes"; cross-client source copying through the standalone CLI; Windows and Linux (macOS only); external model providers. Compaction was exercised via /compress on a small transcript, so it proved "registered metadata survives compaction" but not the large-context compaction path.

On the convergence concern in review 5151984218

For what it is worth as a data point: this pass was independent of the review rounds — real daemon, real browser, mutation matrix, failure injection at the filesystem — and it surfaced no new Critical. It reproduced the two already-recorded deferred ones and sharpened their reachability, and everything else it found is a nit or a follow-up. That does not settle the diff-size argument, but it is one full pass over the feature's behaviour that did not open a new fails-closed surface.

Screenshot gallery (Web Shell, real daemon)

Collapsed — default 3 rows, total 13	Expanded — all 13, dedup + explicit title

Workspace PDF — body actually renders	Workspace HTML — stays literal text, script never runs

Binary attachment — no preview, download offered	Source list down, attachments healthy — independent errors

Older daemon (capability removed) — no Add-source affordance, uploads still listed:

_{Evidence images: wenshao/qwen-code@assets-pr11262 · rig details reproducible from the tables above.}