feat(serve): add read-only status routes#4241
Merged
Merged
Conversation
Contributor
📋 Review SummaryThis PR adds read-only daemon runtime status endpoints for workspace MCP servers, skills, model providers, session context, and session supported commands. The implementation is well-structured with comprehensive test coverage, proper type safety across CLI and SDK packages, and thoughtful security considerations around data redaction. The changes are additive and maintain backward compatibility with the existing v1 capabilities envelope. 🔍 General Feedback
🎯 Specific Feedback🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
a1e0b36 to
b590c87
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new set of read-only daemon runtime “status” endpoints to qwen serve, advertises them via serve capabilities, and mirrors them in the TypeScript SDK with typed helpers—enabling client UIs to inspect daemon/workspace/session state without mutating sessions or spawning ACP while idle.
Changes:
- Added new HTTP GET routes for workspace and session status snapshots (
/workspace/*,/session/:id/*) wired through the bridge with “idle workspace = initialized:false” behavior. - Implemented ACP ext-method handlers to produce redacted status payloads for MCP, skills, providers, session context, and supported commands.
- Extended the TS SDK with new status wire types and client methods + unit tests; updated serve protocol docs to describe the new routes/capabilities.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/serve/status.ts | Defines serve-side status wire types + ext-method names + idle snapshot helpers. |
| packages/cli/src/serve/server.ts | Adds the new read-only GET routes and wires them to the bridge. |
| packages/cli/src/serve/httpAcpBridge.ts | Implements workspace/session status requests without spawning ACP when idle. |
| packages/cli/src/acp-integration/acpAgent.ts | Implements ACP ext methods that build redacted status payloads. |
| packages/cli/src/acp-integration/session/Session.ts | Refactors available-command snapshot building for reuse by status endpoints. |
| packages/sdk-typescript/src/daemon/types.ts | Adds SDK-side typings for the new status payloads. |
| packages/sdk-typescript/src/daemon/DaemonClient.ts | Adds SDK helper methods that GET the new status endpoints. |
| packages/sdk-typescript/src/daemon/DaemonSessionClient.ts | Adds session-scoped wrappers for context + supported-commands status. |
| packages/cli/src/serve/capabilities.ts | Advertises new capability tags for the status endpoints. |
| packages/cli/src/serve/server.test.ts | Adds route-level tests for new status endpoints + 404 mapping. |
| packages/cli/src/serve/httpAcpBridge.test.ts | Adds tests for idle behavior (no spawn) + ext-method routing. |
| packages/sdk-typescript/test/unit/DaemonClient.test.ts | Adds tests for new DaemonClient status helpers (incl. encoded IDs). |
| packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts | Adds tests for new DaemonSessionClient status helpers. |
| docs/users/qwen-serve.md | Documents the new read-only status routes for users. |
| docs/developers/qwen-serve-protocol.md | Documents capability tags + wire shapes for the new status routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
b590c87 to
679b7cf
Compare
wenshao
requested changes
May 17, 2026
Add read-only daemon status endpoints for workspace MCP, skills, providers, session context, and session supported commands. Expose matching typed SDK helpers and document the new additive v1 status surface. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
679b7cf to
ca58c8d
Compare
wenshao
reviewed
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — gpt-5.5 via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
reviewed
May 17, 2026
wenshao
reviewed
May 17, 2026
wenshao
reviewed
May 17, 2026
wenshao
reviewed
May 17, 2026
wenshao
approved these changes
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Incremental review of changes since b590c87 (previous review).
The new changes — heartbeat route, maxQueued SSE query param, EventBus slow_client_warning, BoundedQueueEntry refactor, and SDK event types — are clean:
- Heartbeat route validates client ID before bumping timestamps (prevents attacker watermark manipulation)
- maxQueued param has thorough input validation with fail-closed 400 before SSE handshake
- EventBus slow_client_warning has correct hysteresis logic (warn at 75%, re-arm at 37.5%) with per-entry forced tracking
- All builds and focused tests pass (270 CLI tests, 103 SDK tests, 134 core tests)
- ESLint clean on changed files
No new issues found in the incremental diff. The 10 previously-reported findings in acpAgent.ts, Session.ts, httpAcpBridge.ts, and server.ts remain unaddressed.
— glm-5.1 via Qwen Code /review
wenshao
reviewed
May 17, 2026
wenshao
previously approved these changes
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Second opinion review (mimo-v2-5-pro)
Thorough multi-dimensional analysis (correctness, security, code quality, performance, test coverage, build verification, attacker simulation, 3AM oncall, and maintainer perspectives). 6/9 agents completed successfully; all 491 tests pass across 9 test files.
Deterministic checks:
- tsc: No new errors in PR source files (pre-existing errors in integration tests only)
- eslint: Clean
Agent findings summary:
- No new Critical or Important issues found beyond what the first review (deepseek-v4-pro) already identified
- Two code-quality observations on pre-existing code (not postable as inline comments since they target unchanged lines):
events.ts:345–368:permission_resolvedandpermission_already_resolvedswitch cases are identical — merge via fallthroughhttpAcpBridge.ts:2855–2935:respondToPermissionandrespondToSessionPermissionshare ~10 lines of option-validation logic — extract shared helper
Filtered out (pre-existing / out-of-scope):
restoreSessionHandlermissingres.on('close')detachClient — same pattern asPOST /sessionhappy path; consistent with existing architecture- Cross-session permission voting via non-session-scoped endpoint — pre-existing; the new session-scoped endpoint was designed to address this
- TOCTOU in
exportCommand.ts— outside PR scope - Session ID path traversal — consistent with existing routes; agent-level validation exists
Verdict: APPROVE — Well-structured PR with clean status route additions, proper capability advertisement, and comprehensive test coverage. The 11 inline comments from the first review are valid and should be addressed, but none are blockers for the overall design.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
previously approved these changes
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Re-verified on dd9a0d4 (force-push after conflict resolution + hardening commit 6244c89).
Local validation
| Command | Result |
|---|---|
npm run build (core) |
exit 0 |
npm run typecheck (cli) |
exit 0 |
npm run typecheck (sdk-typescript) |
exit 0 |
npx vitest run cli (server / httpAcpBridge / acpAgent / Session) |
396/396 passed (was 368 pre-hardening) |
npx vitest run sdk (DaemonClient / DaemonSessionClient) |
91/91 passed (was 84 pre-hardening) |
git diff --check |
exit 0 |
| GitHub CI | Lint / CodeQL ×2 / Test (ubuntu) / Classify all green; Test (mac+win) still running |
Diff since last approval (ca58c8d → dd9a0d4)
6244c894fix(serve): harden read-only status snapshots— 208 net lines, primarily inacpAgent.ts(mcp/skill/provider snapshot edge cases), with +185 lines of test coverage; passes locally and adds the +28 cli / +7 sdk tests the totals reflect.8df48219+dd9a0d42— merge resolution against current main; capability registry, server routes, doc table, Session.ts helper refactor all merge cleanly. I re-rannpm run buildon core before typecheck so the cross-package@qwen-code/qwen-code-coreexports added on main (collectMemoryDiagnostics,mid_turn_user_message,plansDirectory) resolve correctly.
Remaining open nits (unchanged from prior round, intentionally non-blocking)
Session.ts:198allocates anAbortControllerthat is never wired to a cancellation path. Suggestion only; no functional impact on the read-only status routes this PR ships.DaemonSessionClient.context()/supportedCommands()(sdk DaemonSessionClient.ts) don't forwardthis.clientIdlike the other session-scoped methods do. For read-only routes this is benign — there's no audit trail to attribute to a specific client — but the symmetry is worth a small follow-up.
Neither is a behavioral defect; both can fold into a Wave 3 follow-up.
LGTM.
wenshao
approved these changes
May 17, 2026
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Re-approving on 11623de after the author addressed the two remaining nits — Session.ts unused AbortController removed; DaemonClient.sessionContext / DaemonSessionClient.context / supportedCommands now forward clientId for audit symmetry with the other session-scoped methods.
Local re-validation on 11623de: cli vitest 396/396, sdk vitest 91/91, cli + sdk typecheck both exit 0.
LGTM.
5 tasks
doudouOUC
added a commit
that referenced
this pull request
May 17, 2026
…ventBus changes (#4245) - qwen-serve-routes.test.ts: expand expected features list to 24, adding slow_client_warning (#4237) and workspace_mcp/workspace_skills/ workspace_providers/session_context/session_supported_commands (#4241). Matches EXPECTED_STAGE1_FEATURES in server.test.ts:76-101. - qwen-serve-baseline.test.ts: update SSE backpressure assertion from 3 to 4 frames (tick, tick, slow_client_warning, client_evicted). PR #4237 changed EventBus to force-push a slow_client_warning synthetic frame when the per-subscriber queue reaches the 75% warn threshold, before the client_evicted terminal frame fires on overflow. Mirrors the unit test at eventBus.test.ts:103-122. Both integration mirrors drifted because integration tests only run on schedule / workflow_dispatch (release.yml:4-9), not PR CI. Fixes the release run 25992130532 failure in both Docker and No-Sandbox jobs. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
This was referenced May 17, 2026
doudouOUC
added a commit
that referenced
this pull request
May 18, 2026
…4250) * refactor(serve): add FileSystemService boundary (#4175 PR 18) Introduce a per-request workspace filesystem boundary inside the `qwen serve` daemon. The boundary centralizes path canonicalization, symlink-aware boundary checks, ignore/trust policy, size/binary limits, and audit hooks behind a single typed surface — preparing PR 19 (read-only file routes) and PR 20 (write/edit routes) to share a guarded chokepoint instead of re-implementing path safety per route. Wave 4 PR 18 of #4175 — pure refactor, no new HTTP routes; depends on PR 12 (#4241) and PR 15 (#4236), both merged. New module under `packages/cli/src/serve/fs/`: - `paths.ts` extracts `canonicalizeWorkspace` from `httpAcpBridge.ts` (re-exported there for backward compatibility) and adds: - `ResolvedPath` brand and `Intent` union (read/write/edit/list/ glob/stat) with exhaustiveness checks at the trust gate - `hasSuspiciousPathPattern` — detects NTFS ADS, 8.3 short names, long-path prefixes, UNC paths, trailing dots, DOS device names, and three-or-more-dot path components (claude-code-style) - `findExistingAncestor` with explicit ENOTDIR rejection so a regular file in a path component throws `parse_error` rather than passing boundary inspection and 500-ing later - `resolveWithinWorkspace` running a chain-aware realpath check with ENOENT-tolerant ancestor walk for write/stat intents - `errors.ts` defines `FsError` / `FsErrorKind` plus `wrapAsFsError`, which categorizes raw `fs.promises` errnos (EACCES → permission_ denied, ELOOP → symlink_escape, ENOTDIR → parse_error, etc.) so body-level failures emit audit events instead of escaping uncategorized - `policy.ts` carries `MAX_READ_BYTES` (256 KiB), `MAX_WRITE_BYTES` (5 MiB), `BINARY_PROBE_BYTES` (4 KiB), `shouldIgnore` (file/ directory aware), and `assertTrustedForIntent` with an exhaustive switch over `Intent` - `audit.ts` emits typed `fs.access` / `fs.denied` `BridgeEvent` frames with SHA-256-hashed paths, optional raw-path passthrough via `QWEN_AUDIT_RAW_PATHS=1`, and discriminator `kind` fields so SDK consumers can exhaustively narrow `event.data` - `workspaceFileSystem.ts` — `WorkspaceFileSystem` interface + `createWorkspaceFileSystemFactory` with eight methods (resolve, stat, readText, readBytes, list, glob, writeText, edit). Every body method funnels failures through `recordAndWrap`, which wraps raw fs errors and always emits an `fs.denied` audit event before rethrowing. `readText` enforces `MAX_READ_BYTES` *before* delegating to the slurping core service so unbounded requests against multi-gigabyte files can no longer OOM the daemon. `glob` realpath-checks each hit against the canonical workspace and reports filtered escapes via a single aggregated `fs.denied` event with the dropped count - `index.ts` is the barrel re-export PR 19/20 will import from Modified files: - `packages/cli/src/serve/httpAcpBridge.ts` — extracted `canonicalizeWorkspace` to `fs/paths.ts`; the bridge re-exports it so existing callers in `server.ts` and `runQwenServe.ts` keep working - `packages/cli/src/serve/server.ts` — added `fsFactory?: WorkspaceFileSystemFactory` to `ServeAppDeps`; `createServeApp` builds a strict default (`trusted: false`, warn-once no-op `emit`) when none is injected so a future refactor that forgets `fsFactory` injection cannot silently allow writes against an untrusted workspace; factory parked on `app.locals` for PR 19/20 route handlers - `packages/core/src/index.ts` — re-exports `Ignore`, `loadIgnoreRules`, and `LoadIgnoreRulesOptions` from `utils/filesearch/ignore.js` for cli consumption 411 serve tests pass; typecheck clean. Engineering principles checklist: - [x] Independently mergeable (no new routes, no new capability tag) - [x] Backward compatible (no removed routes / event fields / CLI behavior) - [x] Default off (no public surface change; PR 19/20 will activate routes) - [x] qwen serve Stage 1 routes preserved - [x] Gradual migration (PR 19/20 will adopt the boundary) - [x] Reversible (single PR rollback) - [x] Tests-first (101 unit tests across the new module + contract test) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): address PR review feedback (#4250) Codex + Copilot review found 8 substantive issues against 7b0db4c; this commit fixes all of them. The first two are P1 build-breakers introduced by the pre-commit `eslint --fix` auto-promoting value imports to `import type` — 31 fs tests were failing post-commit until this fix. Issue list (links to PR comments at #4250): 1. Import type erased runtime values — workspaceFileSystem.ts:10-15. eslint's consistent-type-imports rule rewrote `import { Ignore, StandardFileSystemService, loadIgnoreRules, type WriteTextFileOptions }` -> `import type { ... }` because it saw type-only usage of `Ignore`. That erased `loadIgnoreRules` (called at runtime) and `StandardFileSystemService` (constructed at runtime), causing TS1361/TS2206 and runtime ReferenceErrors. Restored as a value import with inline `type` modifiers per-symbol and an eslint-disable line + comment so future autofixes don't repeat the regression. 2. Same import-type erasure in contract.test.ts:14 — `isFsError` was lumped under `import type` even though it's called at runtime. Same fix shape. 3. edit() OOM hole — workspaceFileSystem.ts. The earlier review pass added a pre-stat MAX_READ_BYTES gate to readText but missed edit, which fsp.readFile's the whole file before any size check. Multi-GB targets inside the workspace could OOM the daemon. Now stat-first; refuses above the cap with file_too_large; also rejects binary files (string indexOf over arbitrary bytes is meaningless). 4. glob accepted absolute / device patterns — workspaceFileSystem.ts. The ..-segment check stopped lexical traversal but /etc/** / C:\\Users\\foo\\** / \\\\server\\share\\** / //server/share/** still reached globAsync, walking outside the workspace before per-hit filtering dropped the results. Now rejects these patterns up-front with parse_error so no I/O happens outside. 5. glob ignore filter probed every hit as `file` — workspaceFileSystem.ts. The underlying `ignore` library needs a trailing-slash probe for dist/ / .git/-style directory patterns; probing as `file` silently leaked directory matches. Now lstats each hit and routes 'directory' vs 'file' to shouldIgnore so dir-only ignore rules actually match. 6. ReadTextOptions.line off-by-one — workspaceFileSystem.ts. The public option was documented as 1-based but forwarded as-is to readFileWithLineAndLimit, which is 0-based. A request with line: 1 returned content starting at the second line. Now converts 1-based -> 0-based at the boundary; doc clarified; truncation check uses the converted index. 7. ServeAppDeps.fsFactory JSDoc said trusted=true — server.ts:96. Stale from before the strict-default refactor in the same review pass. Rewrote to match the actual trusted: false + warn-once emit behavior. 8. MAX_READ_BYTES JSDoc said reads above cap return truncated — policy.ts:18. Stale from before the hard-cap refactor; now correctly states the cap throws file_too_large and that soft truncation only applies under the cap via enforceReadSize. 7 new tests cover the new behaviors: - POSIX-absolute pattern rejection - Win32 / UNC pattern rejection (4 variants) - directory-pattern ignore (dist/) - edit file-too-large - edit binary refusal - readText line: 1 returns from first line - readText line: 2 starts from second line 418/418 serve tests pass; typecheck + eslint clean. Deferred follow-ups (per PR review reply): - glob maxResults is applied after globAsync materializes every match. A streaming iterator (glob.iterate) would bound the walk too. Non-trivial; tracked as a separate hardening follow-up since current behavior is correctness-safe (just not optimal under huge trees). - Per-path glob escape hash in audit hint (currently aggregated count) — can revisit once PR 19 wires the routes and we see real audit volume. - EVENT_SCHEMA_VERSION migration mechanism — orthogonal; the whole BridgeEvent schema lacks one and that's a Wave 5+ concern. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 3 review-round-2 findings (#4250) The wenshao + DeepSeek reviewer pass on a81ada4 surfaced 3 more issues; this commit fixes them. 1. Dangling-symlink write escape (paths.ts) — Critical security bug. A request like `write /ws/escape` where `<ws>/escape` is a symlink whose target doesn't exist YET would pass the ENOENT-tolerant ancestor walk: realpath fails ENOENT, the walk-up returns `<ws>` as ancestor, the canonical becomes `<ws>/escape`, containment passes — but the eventual write follows the symlink and creates the file at the symlink target outside the workspace. lstat-then-readlink before the ancestor walk catches this; the symlink target is itself resolved via the deepest existing ancestor so macOS /private/var canonicalization stays consistent with boundCanonical (an absolute target inside the workspace tmpdir would otherwise have been false-flagged on macOS). 2. glob realpath catch over-reported symlink_escape (workspaceFileSystem.ts) — every realpath failure inside the per-hit boundary loop was counted as `symlink_escape`. EIO, EACCES, ENAMETOOLONG, EBUSY are environmental failures, not security events; mislabeling them poisoned the audit signal for operators trying to investigate genuine escape attempts. Now distinguished: ENOENT/ELOOP count as escapes; other errnos count as transient errors and emit a separate aggregated `fs.denied` with errorKind: 'permission_denied'. 3. policy.ts:enforceReadSize JSDoc said the boundary "intentionally does NOT throw" — stale after a81ada4's hard-cap refactor. Rewrote to clarify the helper is the soft truncation gate that only fires under the hard cap; readText itself enforces the hard cap with file_too_large via its pre-stat check. The readme/contract is now consistent with workspaceFileSystem.ts. 2 new tests: - dangling symlink targeting outside-workspace path → symlink_escape - dangling symlink targeting future-inside-workspace path → succeeds (ahead-of-mkdir flow for atomic-write-via-rename) 420/420 serve tests pass; typecheck clean. Remaining tracked follow-ups (per PR review reply): - list/glob brand cast (P2 deferred per PR description) - glob audit pathHash hashes pattern not paths - edit() TOCTOU read-modify-write race (atomic-via-temp + rename) - wrapAsFsError ENOSPC/EIO mapping to a distinct kind - runQwenServe → fsFactory injection integration test - glob maxResults streaming (glob.iterate) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): cross-platform ENOTDIR detection (#4250 CI fix) Windows CI test failure on a81ada4 surfaced a real cross-platform bug in `findExistingAncestor`. POSIX returns `ENOTDIR` when `fs.stat` traverses through a non-directory in a path component (e.g. `${ws}/file.txt/leaf` where `file.txt` is a regular file). **Windows returns `ENOENT` for the same case.** The errno-based guard added in a81ada4 only branched on `ENOTDIR`, so the Windows path silently fell through to the ancestor walk and the boundary returned a "canonical" the eventual write could not honor — `WorkspaceFileSystem - audit always emits on body errors > rejects ENOTDIR ancestor walk with parse_error rather than passing boundary` failed with `expected false to be true` on the windows-latest runner. Fix: switch from errno-based detection (platform-divergent) to dirent-kind detection. After `fs.stat` succeeds during the walk-up, if the existing ancestor is NOT a directory AND there are unresolved tail components, throw `parse_error`. Both `ENOENT` and `ENOTDIR` from `fs.stat` are now treated as "the *current* path doesn't resolve, keep walking" — the post-walk kind check fires regardless of which errno surfaced. Cross-platform-safe. The local 110/110 fs tests still pass on macOS/Linux; the Windows case will exercise the kind-check branch on next CI run. macOS CI failures on the same workflow run (`InputPrompt.test.tsx` placeholder reuse, `SettingsDialog.test.tsx` 5s timeout) are pre- existing flaky UI tests, NOT touched by this PR. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 4 review-round-3 findings (#4250) DeepSeek's third review pass (after b38e821) flagged four more issues; this commit fixes all of them. 1. Multi-hop dangling symlink bypass (paths.ts) — Critical security fix. The earlier single-readlink fix at efd7a46 was bypassed by chained dangling symlinks: <ws>/leak -> <ws>/middle -> /scratch/evil where every layer is a symlink and the final target doesn't exist. The fix only readlink'd the first hop (<ws>/middle), saw it was inside the workspace via findExistingAncestor, and let the chain through. The OS write at <ws>/leak would then follow both hops and create /scratch/evil. Now loops lstat + readlink up to MAX_ANCESTOR_HOPS, tracks visited inodes for cycle detection, and only validates containment on the fully-dereferenced leaf. Cycle detection rejects with symlink_escape; chains that exceed the hop bound also surface as symlink_escape with a "too long or contains a cycle" hint. 2. opts.line validation (workspaceFileSystem.ts) — the docstring committed to "1-based positive integer" but Infinity / floats / negative values flowed through to readFileWithLineAndLimit and degraded silently. Now enforces Number.isSafeInteger + line >= 1 at the boundary; everything else throws parse_error. Test covers Infinity, -Infinity, 0, -1, 1.5, NaN. 3. io_error FsErrorKind (errors.ts) — wrapAsFsError previously conflated EIO/EBUSY/ENAMETOOLONG/EMFILE/ENFILE/ENOSPC/ETXTBSY under permission_denied. Monitoring pipelines that key on errorKind for security alerting would page oncall on a full disk. New io_error kind (HTTP 503) maps the environmental- failure errnos with distinct hints. EACCES/EPERM stay on permission_denied (literal access denial); EIO (failing disk), EBUSY (busy file), ENAMETOOLONG (PATH_MAX), EMFILE/ENFILE (fd exhaustion), ENOSPC (df -h reporting 100%), ETXTBSY (text-busy) all route to io_error. 4. glob audit kind taxonomy (workspaceFileSystem.ts) — three-way classification mirrors wrapAsFsError so the per-hit realpath catch surfaces ENOENT/ELOOP -> symlink_escape, EACCES/EPERM -> permission_denied, everything else -> io_error. Each class emits its own aggregated fs.denied event. 5. edit() matchedIgnore (workspaceFileSystem.ts) — readText and writeText both stamp matchedIgnore in their access audit; edit didn't, so operators monitoring fs.access events couldn't distinguish edits to .gitignore'd files (build artifacts, logs) from edits to tracked source. Added the same shouldIgnore + matchedIgnore plumbing that readText uses. 8 new tests: - multi-hop dangling symlink (security) - symlink cycle (security) - ENOSPC/EIO/EBUSY/ETXTBSY/ENAMETOOLONG -> io_error mapping - io_error -> HTTP 503 - EMFILE/ENFILE updated to io_error (was permission_denied) - opts.line rejects Infinity/-Infinity/0/-1/1.5/NaN - edit() audit records matchedIgnore on .log file 426/426 serve tests pass; typecheck clean. Remaining tracked follow-ups (per PR review reply): - list/glob brand cast (P2 deferred per PR description) - glob audit pathHash hashes pattern not paths - edit() TOCTOU read-modify-write race (atomic-via-temp + rename) — pinned to PR 20 - runQwenServe → fsFactory injection integration test — pinned to PR 19 - glob maxResults streaming (glob.iterate) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 3 review-round-4 critical findings (#4250) DeepSeek's fourth review pass surfaced three more critical bugs; all are fixed in-PR. 1. TOCTOU symlink substitution in readText/readBytes/edit (workspaceFileSystem.ts) — Critical. fsp.stat(p) and the subsequent lowFs.readTextFile(p) (or fsp.readFile / fsp.readFile for edit) are two separate syscalls. An attacker who can write into the workspace can swap p from a regular file to a symlink pointing outside between the two calls; pre-stat sees the original, the read follows the swap. Fix: assertInodeStableAfterRead(p, preIno) — after each read, re-lstat p; reject with symlink_escape if the path is now a symlink (post.isSymbolicLink()) or its inode changed (preIno !== post.ino, with a 0-ino fallback for procfs / virtual mounts that don't report meaningful inodes). Catches the swap-and-leave attack and the swap-and-keep-swapped attack. Residual race (attacker swaps back AFTER our read but BEFORE our lstat) is much smaller than the original window and outside PR 18's threat model; fd-based reading via fsp.open + fileHandle.read would close it entirely but requires a new variant of lowFs that takes a FileHandle — tracked as a follow-up. 2. UTF-8 truncation corruption in readText (workspaceFileSystem.ts) — Critical. buf.subarray(0, sizeOutcome.bytesToRead).toString ('utf-8') silently emits U+FFFD when bytesToRead falls mid-codepoint (CJK, emoji). A downstream consumer parsing JSON or source code over the truncated content would see broken trailing bytes; meta.truncated would be true but the prefix is corrupt. A subsequent edit() with the corrupted string as oldText would also fail to match the on-disk content. Fix: safeUtf8Truncate(buf, maxBytes) walks back from maxBytes through any continuation bytes (0b10xxxxxx), then verifies the leading byte's full sequence fits within the cap; drops the leading byte if it doesn't. The result is always a clean prefix at a valid codepoint boundary. Test pins '中文测试' (12 bytes / 3 bytes per char) truncated at 7 bytes -> '中文' (no U+FFFD). 3. glob opts.cwd bypasses workspace boundary (workspaceFileSystem.ts) — Critical. opts.cwd was used directly as the glob root with no validation against boundWorkspace. ResolvedPath is a brand cast and a stale or forged value lets a glob('**/*', { cwd: '/etc' }) enumerate files outside the workspace. The pattern-side absolute / UNC checks added in a81ada4 only constrain the *pattern*; cwd is the actual hazard. Fix: at the entry point of glob(), path.resolve cwd and isWithinRoot-check against boundWorkspace. Throws path_outside_workspace if cwd is outside, even when the pattern itself is harmlessly relative. Test pins the case with cwd: scratch (outside workspace). 3 new tests: - readText with mid-operation symlink swap -> symlink_escape - safeUtf8Truncate keeps CJK codepoints intact at 7-byte cap - glob with opts.cwd outside workspace -> path_outside_workspace 429/429 serve tests pass; typecheck + eslint clean. Remaining tracked follow-ups (Post-PR-18 hardening, in #4175): - list/glob brand-cast contract (PR 19) - runQwenServe → fsFactory injection contract test (PR 19) - edit() write-side TOCTOU + atomic-via-temp + expectedHash (PR 20) - glob audit pathHash (independent audit.ts commit) - glob maxResults streaming (independent hardening) - glob pattern preflight refactor to reuse hasSuspiciousPathPattern (cosmetic) 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 7 review-round-5 findings (#4250) DeepSeek round-5 surfaced 9 comments; 7 are real findings (the other 2 — UTF-8 truncation + glob opts.cwd — already fixed in 5468342 from round-4 and replied-to inline). Stack on round-4. 1. edit() empty-oldText silent-prepend (workspaceFileSystem.ts) — Critical silent data corruption. JS `''.indexOf('')` returns 0, so without an empty-string guard `current.slice(0,0) + newText + current.slice(0)` = `newText + current` — silently prepends `newText` to the whole file with a success audit event. PR 20 routes that thread user-supplied `oldText` verbatim must not be able to trigger this. Now throws `parse_error` BEFORE the read with a hint explaining why empty matches are rejected. 2. DOS device name regex misses bare names + first-extension forms (paths.ts) — Windows attack surface. The earlier /\.(CON|PRN|AUX|NUL|COM[1-9]|LPT[1-9])$/i only caught the last-extension form (file.CON). NTFS reserves these names regardless of extension: CON, NUL, CON.txt, NUL.dat, CON.foo.bar are ALL reserved device handles. New regex /(^|\.)(CON|...|LPT[1-9])(\.|$)/i covers all four positions (bare / first-ext / last-ext / middle-ext) while still admitting legitimate substrings (BACON, concat.txt, precon.go). 3. Buffer.byteLength replaces Buffer.from for size-only checks (workspaceFileSystem.ts) — 5 MB heap allocation per write eliminated. `writeText` and `edit()` previously materialized the entire UTF-8 payload (up to MAX_WRITE_BYTES) just to read `.length`; `Buffer.byteLength(content, 'utf-8')` returns the count without allocating. 4. edit() error message includes oldText snippet (workspaceFileSystem.ts) — Production debuggability. The earlier hint was just "edit() expects oldText to appear verbatim in the file" — at 3 AM an operator can't tell whether the mismatch is whitespace, a stale file, or a wrong target. Now includes a JSON-quoted truncated snippet (max 80 chars + ellipsis) of the searched-for text. 5. recordAndWrap forwards FsError message into fs.denied audit (workspaceFileSystem.ts + audit.ts) — Audit observability gap. Audit consumers debugging an incident saw `errorKind` + `hint` but lost the underlying OS error detail (path, errno text, byte count). FsDeniedAuditPayload now carries an optional `message` field; recordAndWrap forwards `fs.message` automatically. 6. EISDIR / ENOTDIR distinct hints (errors.ts) — UX. Both shared the same hint "a path component is not a directory where one was expected" — for EISDIR (path IS a directory but a file was expected) the wording was reversed. Now distinct hints with the errno name explicitly cited. 7. kindFromStats / kindFromDirent merged into kindFromStatLike (workspaceFileSystem.ts) — duplicate function bodies removed. Both fs.Stats and fs.Dirent expose the same isFile / isDirectory / isSymbolicLink interface, and both targets (FsStat['kind'], FsEntry['kind']) are the same 4-value union. Single helper avoids drift if the union grows. 4 new tests: - bare/multi-ext DOS device names (CON, NUL, CON.txt, CON.foo.bar) + legitimate substrings (BACON, concat, precon, contemplating) - edit() empty oldText -> parse_error + file unchanged - edit() not-found error includes searched snippet in hint - fs.denied audit payload carries FsError message Plus the 2 already-fixed items (UTF-8 boundary, glob opts.cwd) have new test coverage from round-4. 433/433 serve tests pass; typecheck + eslint clean. Stack: 7b0db4c → a81ada4 → efd7a46 → b38e821 → 911cb8e → 5468342 → THIS 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 7 review-round-6 findings (#4250) Round-6 review (wenshao + gpt-5.5) flagged 7 items including two Critical and one privacy regression I introduced in round-5. 1. writeText / edit pre-write symlink guard (workspaceFileSystem.ts) — Critical, two reviewers (wenshao #CsBcq + gpt-5.5 #CsB3M) independently flagged. `atomicWriteFile` (`packages/core/src/utils/atomicFileWrite.ts`) resolves symlinks at write time, so a swap between the boundary's `resolve()` and `lowFs.writeTextFile()` would let the write follow the symlink to outside the workspace. `writeText` had no read phase, so its window was wider than `edit()`'s. New helper `assertNotSymlinkBeforeWrite(p)` lstats the path immediately before each `lowFs.writeTextFile` call; ENOENT is fine (ahead-of-create flow), but an actual symlink throws `symlink_escape`. Used in `writeText` AND `edit()`. Residual race after this guard but before the write completes is the deferred PR 20 atomic-via-temp follow-up. 2. recordAndWrap message field bypassed privacy mode (audit.ts) — Critical privacy regression I introduced at ebd9e78 (round-5). The new `FsDeniedAuditPayload.message` field forwarded `FsError.message` unconditionally — and many throw-sites embed `${p}` (absolute paths) or user-supplied `oldText` snippets into the message. Privacy-mode operators (without `QWEN_AUDIT_RAW_PATHS=1`) saw paths leak through the message field even though they explicitly disabled raw-path logging. Fixed: `message` is now gated behind `includeRawPaths` alongside `relPath`. Privacy mode = no path-bearing fields, period. Operators wanting forensic context opt in via `QWEN_AUDIT_RAW_PATHS=1` and accept both fields together. 3. glob opts.cwd via symlink (workspaceFileSystem.ts) — gpt-5.5 #CsB3P. Textual `path.resolve(cwd) + isWithinRoot` admits `<ws>/link` even when `<ws>/link → /etc` is a symlink to outside; `globAsync` walks `/etc` before the per-hit filter drops results. Switched to `fsp.realpath(path.resolve(cwd))` so the containment check sees the actual walk root. ENOENT on cwd surfaces as `path_not_found`. 4. readText OOM via concurrent file growth (workspaceFileSystem.ts) — wenshao #CsBeE. The pre-stat `MAX_READ_BYTES` gate only sees the size at stat time; a concurrent writer can grow the file before the actual `readFileWithLineAndLimit` slurp. Added post-read `Buffer.byteLength(result.content) > MAX_READ_BYTES` check. The proper fix (fd-based read tying size + read to the same inode) is a hardening follow-up; this byte-length check is the defense-in-depth layer. 5. readBytes maxBytes can widen past MAX_READ_BYTES (policy.ts) — wenshao #CsBj5. `enforceReadBytesSize(st.size, opts.maxBytes)` used the caller-supplied `maxBytes` as the ceiling, replacing rather than clamping `MAX_READ_BYTES`. A future PR 19/20 route forwarding `req.query.maxBytes` could blindly bypass the daemon's 256 KiB safety cap. Now clamps via `Math.min(maxBytes, MAX_READ_BYTES)`. 6. ENOENT_TOLERATING_INTENTS docstring + test (paths.ts) — wenshao #CsBk3. The Intent docstring only mentioned `'write'` tolerating ENOENT; `'stat'` was in the set undocumented. A future maintainer removing `'stat'` thinking it was a copy-paste error would silently change behavior (stat on a concurrently-deleted path would throw `path_not_found` from the resolver instead of letting `fsp.lstat` throw `ENOENT` naturally). Amended docstring to call out `'stat'`'s rationale explicitly + added contract corpus case. 6 new tests: - glob cwd via symlink to outside → path_outside_workspace - writeText with mid-operation symlink swap → symlink_escape + outside file unchanged - edit with mid-operation symlink swap → symlink_escape + outside file unchanged - readBytes opts.maxBytes attempting widening → file_too_large - fs.denied message field absent in privacy mode (default) - fs.denied message field present in raw-paths mode (forensic) - contract corpus: resolve('newdir/leaf', 'stat') succeeds for ENOENT path 439/439 serve tests pass; typecheck + eslint clean. Stack: 7b0db4c → a81ada4 → efd7a46 → b38e821 → 911cb8e → 5468342 → ebd9e78 → THIS 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 5 review-round-7 findings (#4250) glm-5.1 round-7 review surfaced 6 items; 5 are real fixes, 1 was already addressed in 7f4f30d (writeText pre-write symlink guard — reviewer looked at ebd9e78 snapshot ~8h before the round-6 fix). Stack on round-6. 1. edit() bypassed lowFs.readTextFile (workspaceFileSystem.ts) — Critical encoding round-trip corruption. The earlier `fsp.readFile(p, 'utf-8')` included the UTF-8 BOM verbatim in `current`, breaking `oldText` matching even when the user passed the exact source text from a previous read; lost iconv-supported codepage handling (GBK / Big5 / Shift_JIS), so non-UTF-8 files would mojibake into `current` and round-trip-corrupt on write-back; and the subsequent `lowFs.writeTextFile` passed no `_meta`, stripping the BOM and forcing UTF-8 on write-back even when the original was BOM'd or non-UTF-8. Fix: read via `lowFs.readTextFile` AND forward `readResult._meta` into the write-back. Test pins UTF-8 BOM round-trip end-to-end. 2. hasSuspiciousPathPattern multi-digit and POSIX false positive (paths.ts) — `/~\\d/` only matched single-digit; NTFS allocates `~10+` on >9 collisions. POSIX false-positives: editor swap files, backup tools used `~N` legitimately. Fixed to `/~\\d+/` and gated behind `process.platform === 'win32'`, matching the ADS-colon check. 3. canonicalizeWorkspace redundant per-request syscall (paths.ts) — Performance. Factory canonicalizes once at build; every `resolveWithinWorkspace` also ran realpathSync on the same path, blocking the event loop. Added CANONICAL_BOUND_CACHE Map keyed on the input string; steady-state size = 1 per `1 daemon = 1 workspace`. 4. readBytes opts.maxBytes API contract (workspaceFileSystem.ts) — Semantic mismatch. Parameter name promised window-read; impl only used it as a hard reject gate. Now truncates the buffer post-read so `readBytes(p, { maxBytes: 1024 })` on a 200 KB file returns 1 KB. Hard `MAX_READ_BYTES` cap still throws for files above it. 5. glob walks node_modules and .git unnecessarily (workspaceFileSystem.ts) — Performance. Without an `ignore` option, `globAsync` traversed every file under those dirs before our per-hit `shouldIgnore` filter. Now passes `ignore: ['**/node_modules/**', '**/.git/**']` to short-circuit traversal. Post-filter via `shouldIgnore` remains authoritative. 5 new tests: - 8.3 short-name regex Windows / POSIX split - readBytes truncates 2048-byte file to 1024 with maxBytes - readBytes throws file_too_large only above hard cap - edit() preserves UTF-8 BOM round-trip - glob prunes node_modules and .git 442/442 serve tests pass; typecheck + eslint clean. Stack: 7b0db4c → a81ada4 → efd7a46 → b38e821 → 911cb8e → 5468342 → ebd9e78 → 7f4f30d → THIS 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 10 review-round-8/9 findings (#4250) Two more review passes (gpt-5.5 + DeepSeek + wenshao) flagged 13 items; 10 are real fixes, 3 are reviewer-stale-snapshot or already-tracked. Stack on round-7. Critical (5): 1. paths.ts symlink-escape hint embedded the symlink target (gpt-5.5) — Privacy regression sibling to round-6 audit `message` gate. `recordDenied` always forwards `hint` into `fs.denied` even with `QWEN_AUDIT_RAW_PATHS` off; the hint `'symlink points to /Users/alice/secret'` leaks the attacker's intended exfiltration path through audit. Hint is now path-free; operators wanting the resolved target enable `QWEN_AUDIT_RAW_PATHS=1` and read it from `relPath` / `message`. 2. paths.ts dangling-symlink chain discarded its verified canonical (DeepSeek) — After the multi-hop walk validated `cursor → canonicalTarget` was inside the workspace, the code fell through to `findExistingAncestor(absolute)`, re-walking from the original input and discarding the verified result. An attacker swapping an intermediate symlink between the verification and the re-walk could produce a different canonical than the one validated. The verified `canonicalTarget` is now captured in `symlinkResolvedCanonical` and used directly; the `findExistingAncestor(absolute)` fallthrough only runs when no symlink was traversed. 3. workspaceFileSystem.ts readBytes missing post-read size check (DeepSeek) — Same TOCTOU shape as `readText`'s round-6 fix. The pre-stat `enforceReadBytesSize` sees the size at stat time; a concurrent appender keeps the same inode but grows the file past the cap before `fsp.readFile` returns. `assertInodeStableAfterRead` catches inode changes but not same-inode growth. Added a post-read `buf.length > MAX_READ_BYTES` check matching `readText`'s defense-in-depth pattern. 4. errors.ts wrapAsFsError default = permission_denied (DeepSeek) — Misclassified non-errno errors (`TypeError`, programmer-error throws, native module exceptions) as security denials, paging security oncall for what should be a developer ticket. New `internal_error` kind (HTTP 500) is the new default; `permission_denied` reserved for actual `EACCES`/`EPERM`. 5. audit.ts AuditContext.sessionId not forwarded to BridgeEvent (DeepSeek) — Multi-session daemons couldn't trace audit events back to the session that triggered them. `originatorClientId` identifies the client, not the session. Added optional `sessionId` field to both `FsAccessAuditPayload` and `FsDeniedAuditPayload`, forwarded from `ctx.sessionId` when present. Improvements (4): 6. workspaceFileSystem.ts glob cwd realpath redundant when cwd === boundWorkspace (wenshao) — `boundWorkspace` is already canonicalized by the factory (`realpathSync.native` at build time), so calling `fsp.realpath` per-request when no `opts.cwd` was supplied is a redundant async syscall. Added a short-circuit. 7. workspaceFileSystem.ts kindFromStatLike JSDoc orphaned (wenshao) — Inserting `assertNotSymlinkBeforeWrite` between the JSDoc and `kindFromStatLike` left the doc floating above the wrong function. IDE hovers showed the wrong description. Moved the doc back to its function. 8. workspaceFileSystem.ts shared mutable Ignore object (DeepSeek) — `createWorkspaceFileSystemFactory` builds one `Ignore` instance and shares it across every `WorkspaceFileSystemImpl` returned by `forRequest()`. `Ignore.add(): this` is a public mutator. A future "per-session ignore rules" feature calling `.add()` from a request handler would silently corrupt all concurrent sessions. `Object.freeze` turns the cross-request mutation into a `TypeError` rather than a silent leak. 9. server.ts createDefaultFsAuditEmit one-shot warned (DeepSeek) — Permanent silent no-op after the first event; only logged the event `type` with no pathHash / errorKind / intent. If PR 19 forgets the real factory injection, every write 403s and audit is silent past the first warning — exactly the regression the warning exists to surface. Periodic warning (every 100th drop) + first-event context (errorKind, intent, pathHash) makes the regression actionable in production logs. Cleanup (1): 10. workspaceFileSystem.ts safeUtf8Truncate dead code (DeepSeek noted as "off-by-one") — The lead-byte seqLen-check block was dead code: `subarray(0, end)` already excludes the leading byte at `end`, so no further adjustment is ever needed. Removed the block; function is now 4 lines and still produces a valid codepoint prefix. Reviewer's suggested fix (`buf[end-1] → buf[end]`) was technically correct but redundant with the subarray cut. Already-fixed (3 reviewer-stale-snapshot, reply + resolve): - writeText pre-write symlink guard — fixed in 7f4f30d - edit() read-modify-write race — already deferred to PR 20 atomic-via-temp follow-up - glob maxResults walk-bound — already follow-up #5 3 new tests + 2 updated: - wrapAsFsError unknown errno → internal_error (default change) - internal_error has HTTP 500 - non-Error throwables → internal_error (not permission_denied) - readBytes post-stat growth → file_too_large - existing wrapAsFsError test updated for new default 445/445 serve tests pass; typecheck + eslint clean. Stack: 7b0db4c → a81ada4 → efd7a46 → b38e821 → 911cb8e → 5468342 → ebd9e78 → 7f4f30d → 1dc9d22 → THIS 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code) * fix(serve/fs): close 3 review-round-10 findings (#4250) Round-10 review (wenshao) flagged 4 items, all marked non-blocking by the reviewer. 3 are landed in-PR; 1 (glob withFileTypes optimization) is moved to issue #4175 follow-ups since the reviewer themselves recommended "test it on a 100K-file workspace after PR 19 lands." 1. canonicalizeWorkspace docstring (paths.ts) — Documentation. The earlier FIXME warned about sync syscall blocking the event loop without mentioning the `CANONICAL_BOUND_CACHE` added in 1dc9d22 brings steady-state cost to zero. Future reviewers reading the FIXME again would re-flag the already-mitigated concern. Added a paragraph noting cache hit rate = 100% under the `1 daemon = 1 workspace` model so per-request blocking only happens at boot or on fresh workspace values (e.g. tests). 2. enforceReadBytesSize dead `maxBytes` parameter (policy.ts) — Round-7 changed `readBytes` to use post-read truncation for the soft window cap, leaving the function's `maxBytes` parameter unused at the only callsite. The `Math.min(maxBytes, MAX_READ_BYTES)` clamp branch became dead code. Reviewer's option 1: tighten the signature to `enforceReadBytesSize(fileBytes: number): void`. Done — the function is now purely the hard-cap enforcer its name implies; soft-window truncation lives in the orchestrator's `buf.subarray(0, opts.maxBytes)` post-read step where it's visible alongside the cap check it complements. 3. relForAudit cross-drive sentinel (audit.ts) — Windows-only privacy edge case. `path.relative('C:\\ws', 'D:\\evil')` returns `'D:\\evil'` (an absolute path) because Win32 can't express cross-drive relatives. Even when raw-paths mode is ENABLED, the audit `relPath` field would carry the off-drive absolute path, exposing the attacker's drive letter + directory. Added a `path.isAbsolute(rel)` post-check that substitutes a `<cross-drive>` sentinel — audit consumers see the cross-drive case distinctly without leaking the offending path. This was previously P2 deferred in PR 18's description ("Windows cross-drive path.relative"); reviewer's "few lines beats deferring" assessment was right. Tracked as follow-up (not in this commit): 4. glob withFileTypes optimization — Replace per-hit `lstat` (line ~664) with `glob` v10's `withFileTypes: true` so each hit comes back as a `Path` object with `isDirectory()` / `isFile()` / `isSymbolicLink()` already available. Saves N syscalls in large workspaces. Non-trivial restructure (return type changes from `string[]` to `Path[]`). Reviewer themselves marked "[performance / non-blocking]" and said "test it on a 100K-file workspace after PR 19 lands so we know whether it's worth it." Added to issue #4175 body's Post-PR-18 hardening follow-ups. 446/446 serve tests pass; typecheck + eslint clean. Stack: 7b0db4c → a81ada4 → efd7a46 → b38e821 → 911cb8e → 5468342 → ebd9e78 → 7f4f30d → 1dc9d22 → a33d459 → THIS 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
46 tasks
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
Co-authored-by: matt korwel <matt.korwel@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
initialized: falsewithout spawning ACP), unknown-session 404 mapping, and the status payload redaction boundary for MCP/provider/skill details.Validation
git diff --checkreported no whitespace issues./capabilitiesfor the five new feature tags and poll/workspace/mcpbefore creating a session to confirminitialized: false.tsc --noEmitruns completing successfully.Scope / Risk
Testing Matrix
Testing matrix notes:
Linked Issues / Bugs
Related to #4175.