Resolve import conflict in bridge.test.ts by keeping both WORKTREE_MCP_DEFER_META_KEY (this PR) and LOAD_REPLAY_HIDE_INHERITED_META_KEY (main) in the bridgeTypes import block.
* feat(web-shell): add contextual task panels
* fix(web-shell): harden contextual task panels
* fix(web-shell): preserve side task titles
* fix(web-shell): address review feedback on context panels PR (#7929)
- Add POST /session/:id/side-task to telemetry route catalog (51 routes)
- Increase SDK browser bundle size limit to 184KB
- Fix duplicated data-testid="chat-pane" → "chat-pane-container" on container
- Gate sourceType behind session_source_metadata capability check
- Add removeSession cleanup after killSession in !res.writable path
- Add i18n key sideTask.renameFailed for error fallback
- Add unit tests for selectVisibleHistoryRecords invariant
* fix(cli): update telemetry-catalog route drift guard to 51 routes (#7929)
* fix(web-shell): address review feedback round 2 on context panels PR (#7929)
- Fix /fork sider discarding createSideTask() return value: show toast
when side tasks are unavailable
- Fix layout feedback loop: availableWidth no longer depends on
environmentPanelVisible since the CSS overlay does not change the
chat pane DOM width
- Remove dead environmentPanelSuppressed state (never set to true)
- Restore setArtifactPanelOpen(false) in closeArtifactPanelTab when
the last tab is closed
- Extract agentDisplayName(task) to a local variable to avoid triple
invocation per render
* fix(web-shell): dedupe completed background agents in environment panel (#7929)
getEnvironmentAgentTasks correlated a transcript tool card with the live
/tasks snapshot only on toolUseId, the notification taskId, and a
<subagentType>-<callId> derived id. A completed background agent can lose
that linkage (its live task carries no usable toolUseId and its daemon id
is general-purpose-<internalId>), so the trailing loop appended the live
task as a second entry. Add a conservative content fallback (prompt, or
description+subagentType) mirroring the daemon's legacy resolver.
* feat(web-shell): support side tasks during active turns
* fix(web-shell): deduplicate completed subagents and gate sourceType on capability (#7929)
* fix(web-shell): restore background agent reconciliation and fix agent dedupe (#7929)
Restore the one-shot subagent reconciliation for inline background Agent tool
cards. Persisted notification records do not always retain a toolUseId, so the
SSE discrete-notification path alone can leave a card stuck in Running; the
documented fallback resolves pending cards through the subagent endpoint after
catch-up, reconnect, and terminal notifications.
Also stop the loose description content fallback in getEnvironmentAgentTasks
from claiming a live task that another transcript tool call already links
precisely (by toolUseId, message taskId, or derived id). Two agents sharing a
description previously collapsed into one: the fallback stole the linked task,
its owner re-matched the same task, and the orphan was dropped.
* fix(web-shell): address critical review feedback on context panels (#7929)
* fix(web-shell): reconcile side-task state across sessions and listings (#7929)
* fix(web-shell): preserve contextual panel fallbacks
---------
Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
* feat(review): statement-level mutation probes in test-efficacy
The revert probe is all-or-nothing: it reverts whole production files, so a
suite that gates six of a diff's behaviours goes red and the probe says
"gated" — even when the seventh behaviour, a one-line safety statement, has no
test at all. Dogfooded on a live PR: deleting a single `reminders.clear()`
inside the not-continued branch left the full 471-test suite green, and that
line carried the PR's headline safety property (an abandoned task's todos must
not bleed into an unrelated new prompt). A human reviewer found it with a
hand-rolled mutation probe; the command could not.
Add the probe kind the human ran, deterministically. Candidates are added
lines from the committed head (never the dirty worktree) whose trimmed
statement calls a safety verb — .clear(), .delete(), .reset(), .abort(),
.removeListener(), .unref() — or reassigns state to empty ([] / new
Map()/Set()), and that are removable as a whole: single complete expression
statements, brace-balanced via a string/comment-aware scanner, previous
significant line ending ;, { or } (which rejects fluent tails, continuations,
and the brace-less-if silent-rebind trap), outside template literals and block
comments. Selection is conservative by design: a false negative costs
nothing, a false positive burns a suite run.
Each mutant (capped at 8, files with collocated new tests first) deletes its
one line in the existing probe worktree and re-runs the affected suites
through the existing vitest-json classifier: red = killed (the line is
guarded), green = SURVIVED — the invariant it enforces ships unprotected —
filed as a finding in the unreachable/inert register so the Agent-7 pipeline
picks it up without any skill change. Compile/load failure = inconclusive,
never a finding. Mutants run only after a cleanly green baseline, inside the
command's existing deadline budget (each run must leave room for the revert
probe); candidates that no longer fit are counted, not silently dropped.
* fix(cli): harden test-efficacy mutant selection text checks (#8020)
Mutant selection ran its end-anchored checks on raw trimmed lines, so a
trailing comment hid a statement's real end (dropping genuine candidates
like `reminders.clear(); // why`) and a safety verb inside a string faked
one (a wasted suite run plus a misleading survivor). Route the
`SAFETY_VERB_RE`, `endsWith(';')`, and predecessor `/[;{}]$/` checks
through a shared `codeOnly()` that strips comments and blanks literal
contents first.
Also guard the template-state escape skip against a backslash-continued
line, mirroring the single/double-quote branch: swallowing that newline
dropped a per-line literal flag and shifted every later line's verdict.
Adds unit coverage for all three selection fixes and an integration test
for the baseline-not-green skip branch.
* fix(cli): gitignore fake vitest in test-efficacy integration fixture (#8020)
The fake vitest bin was committed by `git add -A` and checked out into
the probe worktree as the stale passing copy, so installFailingVitest's
overwrite in the main worktree was never seen by npx in the probe tree.
The baseline read green and the baseline-not-green skip test failed.
* fix(review): make mutation-probe reporting precise (#8020)
Address review feedback on the test-efficacy mutation probe:
- Count candidates the MAX_MUTANTS cap drops in `skippedForCap` instead of
silently losing them, so a capped `survived: 0` cannot read as "every
safety statement is covered" (mirrors the existing `skippedForBudget`).
- Gate the mutant phase per probe file: run each mutant against the files
that are green in the unmutated baseline, so one unrelated quarantined
(all-skip) suite — `inconclusive`, not red — no longer disables the whole
probe.
- Scope the `mutant-survived` finding to the diff's own tests ("confirm an
existing test covers it, or add one") rather than asserting the invariant
ships unprotected, which an untouched pre-existing test may still cover.
* test(review): cover the budget-skip path in mutation-probe integration tests (#8020)
* fix(review): harden mutation-probe selection and diff parsing (#8020)
* fix(review): handle multi-line class headers in mutation-probe selection (#8020)
* fix(review): stop class-body walk at braces before matching class keyword (#8020)
* fix(review): whole-file literal scan for mutant selection; pin the untested guard paths
The per-line scanner pair (codeOnly + lineStartsInsideLiteral) shared a blind
spot: a backtick inside a `${…}` interpolation read as the outer template's
closing backtick. That flipped the literal state for every following line and,
in the single-line skip, exposed nested-template content as code — so a safety
verb inside a string could be selected as a mutant (a false-finding vector) and
a class field below a brace-bearing template could slip the class-body
rejection. Replace the pair with one whole-file pass that tracks interpolation
brace depth: per-line code text with comments stripped and literal contents
blanked, plus the same `${}`-aware skip for delimiter scanning. Differential
audit over every core+cli source file: zero selection differences on real code;
the pathological shapes are pinned by three new tests.
Also pin the remaining untested paths from review: the selection-failure catch
(discloses and still runs the revert probe), the runOneMutant line-mismatch
guard (now exported; inconclusive, file untouched), and the budget-skip stdout
disclosure.
* fix(review): clamp probe deadlines to the whole-command budget (#8020)
* fix(review): harden mutant selection guards and disclosure accounting (#8020)
* fix(review): close silent-zero paths in mutant disclosure and harden diff parsing (#8020)
* fix(review): drop interpolation quote-skip that mis-parsed regex literals (#8020)
* fix(review): track template nesting with a stack; disclose derailed files; harden and pin the remaining probe paths
The nested-template fix that landed as a counter cannot represent a nested
template INSIDE a nested interpolation: at two levels the deep template's text
`}` is charged against the wrong frame, the scan desyncs, and the file either
admits template text as a mutant or derail-drops its real candidates. Replace
the counter with a stack — one frame per open template, `}` closes only the
top interpolation, a backtick closes only the innermost template — and derive
the end state from the stack. The two-level trigger is pinned by a test
written red-first against the counter.
Derailed files are now disclosed, not silently dropped: selectMutants returns
them, and the note composer stacks the derail note with the red-baseline note
instead of clobbering. The hostile-git-config path gets its missing test (repo
diff.srcPrefix/dstPrefix, diff.external, core.quotePath with a non-ASCII
path — fails with the pinned flags removed). The budget test drops its
Date.now call-count coupling for an injected clock threaded through
runTestEfficacy/runProbeSuite. The mutation-phase catch gets an end-to-end
test (ENOBUFS mid-phase → all candidates inconclusive, revert probe still
runs, report still written).
---------
Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
* fix(serve): prevent repeated workspace skill rescans
Make workspace skill status reads use committed snapshots and move refresh work to explicit mutation paths. Add generation-safe daemon caching, conditional HTTP responses, SDK revalidation, and multi-session extension refresh safeguards.
Refs #8079
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): narrow the workspace-skills read model and close its regressions
Follow-up to the previous commit on this branch, from reviewing it.
Subtractions — these were separable from the fix and carried more surface
than value, so they move out of this change:
- Revert the ETag / If-None-Match layer (CORS allow+expose headers, the SDK
conditional JSON cache, the browser bundle budget bump). Express already
emits an ETag and answers 304 for these routes, so the only new behavior
was the SDK cache. It saves transfer bytes but no daemon work — the ETag is
a hash of the already-serialized body — and it shipped without a paired
`Cache-Control`, which is what actually keeps an intermediary from serving
a stale snapshot of an authenticated, mutable resource. The SDK cache was
also unbounded, with no eviction or clear entry point.
- Revert moving `extensions_final` ahead of skill initialization in
`Config.initialize`. In non-safe, non-bare mode `extensions_initial` is
already the same argument-less `refreshCache()`, and it runs
`applyStoreActivation`, so `getActiveExtensions()` is fully populated
before skills are enumerated either way. The move changed only startup
event order (and pushed permissionManager past the extension refresh) for
every surface including the interactive CLI.
Regression fixes — the read went pure, but two of its inputs lost their only
path back to disk:
- Extension sources have no watcher, unlike skills. With the per-read
`extensionManager.refreshCache()` gone, an extension installed, removed,
enabled, or disabled outside the daemon would never reach the snapshot
until the child restarted — and because extension-level skills are derived
from the extension set, a skill-watcher tick could not recover it either.
Adds `ExtensionManager.refreshCacheIfSourcesChanged()`: a stat-based
fingerprint over the extension directory entries, each manifest, the
enablement file, and the store state, which refreshes only when they moved.
A status read pays one readdir plus one stat per entry instead of a
directory scan and a full parse, and stays self-healing.
The baseline is the pre-load fingerprint, so a change landing during a
refresh stays visible to the next check instead of being masked by a
post-load stat. The directory and store halves are captured at different
points because a refresh writes the store itself but never the manifests.
- Revalidation is skipped in safe and bare mode, and the whole of it —
including that mode check — sits inside its error boundary. Those modes never
populate the extension cache by design, while the snapshot derives extension
skills from `getExtensions()`, so revalidating there would have loaded the
extensions the mode exists to exclude. Keeping the mode check outside the
boundary would also have let a config missing those accessors fail a read.
- `initialized: true` with an empty list when the config has no
`SkillManager` is now `initialized: false`. The daemon latches any
initialized answer into `lastWorkspaceSkillsStatus` and then prefers it
over its own local enumeration, so the old value could suppress the
fallback permanently.
Also:
- The retained-snapshot path bumped the freshness timestamp without checking
its generation, so a read that started before an invalidation could push
out the TTL of a snapshot a later read had committed — letting a
post-mutation snapshot go unrevalidated for longer than the window.
- `setWorkspaceSkillEnabled` folded `configsFailed` into `sessionsFailed`,
but it sends `reason: 'settings'`, which never refreshes a skill cache, so
the term was structurally zero. Report `configsFailed` from the `content`
path instead, where it can actually be non-zero.
- Documents the settings-freshness gap this read model accepts: enablement
now comes from the child's in-memory `LoadedSettings`, which `SettingsWatcher`
keeps current for the User and Workspace scopes but not for System /
SystemDefaults (locked-skill policy) or an untrusted workspace.
Tests: adds a real-filesystem guard that drives 50 consecutive cached reads
and asserts zero additional readdir/readFile calls — the mocked suites could
only prove `refreshCache` was not *called*, which is not the invariant that
broke. Adds coverage for the fingerprint gate (steady state, install,
removal, in-place manifest edit, concurrent callers, and the mid-refresh
race), for the null-manager, moved-sources, and safe/bare-mode read paths, and
for the generation guard. The generation-guard and safe/bare-mode tests were
each verified to fail with their fix reverted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): allow bounded reads of large text files
* fix(serve): bound large-text reads by scan cost, not by which knob was set
Follow-up to the bounded large-text read path. Three changes:
Gate on any explicit window argument, not on `limit`. Gating on `limit`
had the cost model backwards in both directions: `{ line: 900_000_000,
limit: 20 }` was admitted despite walking the whole file, while
`{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A
read with no window argument at all still fails, since a caller that
believes it holds the whole file may write it back truncated.
Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read
returns; nothing capped what it cost. Line offsets are resolved by
scanning from byte 0, so a query param could turn into an
uninterruptible multi-second scan of an arbitrarily large file — and on
Windows hold a read handle for that span, blocking renames and deletes.
Past the budget the read is refused with `file_too_large` pointing at
readBytes, which reaches any offset in O(1).
Tolerate appends on streamed windows. Requiring whole-file size/mtime
stability after reading a prefix rejected reads whose returned bytes
were still valid, and the case it rejected — tailing a live log — is the
one this path exists for. Streamed windows now assert inode identity
plus "did not shrink"; truncation and replacement are still rejected.
Also: non-UTF-8 large text now returns `binary_file` rather than
`file_too_large`, so a client retrying on 413 with a smaller window
can't loop forever; and `readFileWithLineAndLimit` throws instead of
silently ignoring a caller-supplied `fileHandle` on the by-path
fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(core): thread the descriptor instead of forking text-read helpers
PR #7947 pinned large-text reads to one inode by threading a caller-owned
FileHandle into readTextRange as an optional field, plus a second field,
forceStreaming, to suppress the buffering fast path. Two optional fields
produced four combinations: one meaningful, one used by a single test, one
unreachable, and — in readFileWithLineAndLimit — one that silently fell
through to a by-path read, defeating the reason the caller opened a handle.
Unify the two encoding detectors. detectFileEncoding now takes a path or a
borrowed handle, so detectFileHandleEncoding is deleted along with the
message discrepancy between them: an encoding iconv-lite cannot load now
raises LargeNonUtf8TextError naming that encoding rather than deferring to
the decoder's generic invalid-utf8 variant. Both still refuse the file, and
the Serve boundary maps both to binary_file.
Split the reader into readTextRange (path) and readTextRangeFromHandle
(always streams, both byte bounds required). The unreachable combination and
its untested readFileHandleBuffer are gone, and with no fileHandle parameter
left for readFileWithLineAndLimit to ignore, the RangeError guarding that
fallthrough is deleted too — the trap can no longer be expressed.
CoreReadTextFileHandleRequest drops its required stats field. Nothing
downstream read it, and because the ACP request type it extends permits
extra properties, TypeScript accepted the dead argument silently.
readFileHandleChunks becomes chunksFromHandle(fh, from) — the one seam
byte-cursor text paging needs.
No observable change at the Serve boundary: its 222 tests pass unmodified.
Two fileSystemService tests were deleted rather than repaired; they asserted
the arguments readFileWithLineAndLimit received, which is nothing once the
handle path stops calling it. Their coverage lives in read-text-range.test.ts
against real files and in workspace-file-system.test.ts at the real boundary.
258 production lines in core, net -71 overall.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(core): make CoreReadTextFileHandleRequest standalone
Self-audit follow-up to f55c867a. Two fields survived the reshape that the
handle path never reads:
- `stats` was documented as required ("must pass the Stats captured from that
handle") and nothing downstream read it. The handle path always streams, so
it never needs a size to choose a strategy, and the encoding probe does its
own fstat.
- `path` became dead once readTextRangeFromHandle replaced the path-plus-handle
call. Errors are labelled with the path by the Serve boundary that owns it.
Neither was caught by the compiler: the ACP ReadTextFileRequest the type
derived from permits extra properties, so the CLI kept passing both silently.
That is the argument for declaring the type standalone rather than Omit-ing
four of six inherited fields and quietly re-admitting the rest.
Also record the second behaviour delta of the detector merge in the design
doc: detectFileEncoding catches I/O errors and falls back to 'utf-8', where
detectFileHandleEncoding let them propagate. The failure is not lost — a handle
that fails the 8 KiB probe fails the streaming read immediately after — but a
different call now reports it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(serve): page large text files by byte cursor
Line offsets address a byte stream, so `readText` resolves them by scanning
from byte 0. Paging a large log that way is O(n^2) across pages, and past
MAX_TEXT_SCAN_BYTES (8 MiB) a deep page is refused outright — agents had no
O(1) path short of dropping to GET /file/bytes and splitting lines themselves,
losing encoding handling, multibyte safety, and the binary_file refusal.
A response that leaves content behind now returns `hasMore`, and where a file
byte offset is derivable, an opaque `nextCursor`. Passing it back as `cursor`
resumes in O(1). Page 1 is an ordinary `limit` read, so clients never compute
byte offsets themselves, and a paging loop does not break when a file happens
to be small.
The cursor is unsigned base64url JSON carrying {off, size, dev, ino}, matching
encodeOrganizedCursor rather than the HMAC-signed transcript codec: the path is
re-resolved through the workspace boundary on every request, so a forged cursor
can only move the offset within a file the caller may already read — what
GET /file/bytes?offset= allows today. What the payload is for is staleness:
a replaced or truncated file yields hash_mismatch instead of bytes from the
wrong place, while an append leaves an outstanding cursor valid — the case the
feature exists for.
Every minted cursor points at the start of a line. When a single line exceeds
maxOutputBytes the reader emits a truncated prefix and skips to the next line
rather than resuming mid-line, because a mid-line cursor makes the following
page snap forward and silently drop the rest of that line at the seam. Windows
cut mid-line by a byte cap therefore report hasMore with no cursor, as do
non-UTF-8 snapshot reads whose decoded text is a UTF-8 re-encoding with no
mapping back to file offsets. That is why hasMore is a field rather than a
restatement of nextCursor.
Cursor reads branch before the size check, not by widening the window gate:
a cursor read of a file under MAX_READ_BYTES would otherwise land on the
snapshot path, which knows only line/limit, and silently return line 0.
Adds the workspace_file_read_cursor capability, per the convention that new
behavior gets a new tag, and retargets the scan-budget hint at cursor paging.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(core): advance UTF-8 cursors after truncation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* docs(serve): clarify cursor bootstrap limits
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(sdk): raise daemon browser bundle budget
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(serve): cover ACP cursor dispatch and cursor binary_file mapping (#8002)
* fix(core): only set sawCrlf for emitted lines in cursor paging (#8002)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
getMcpServers() injected cwd into the effective config, which changed
the hash used for MCP approval binding (hashMcpServerConfig). This
caused approved project/workspace-scoped stdio servers to show as
pending approval, and re-approving silently gated them off on the next
daemon start.
Move the cwd stamping down to populateMcpServerCommand — the
transport-config seam every spawn path already goes through — so
getMcpServers() stays an identity-stable view of configuration and the
approval hash contract holds. Also make populateMcpServerCommand
non-mutating (spread instead of in-place assignment).
Additionally, clean up the abort listener registered during the
direct-shell cwd-queue race so it does not accumulate on the
long-lived per-session signal (ACP path).
* feat(core): tag UserPromptSubmit hook context and record display provenance
UserPromptSubmit additionalContext was appended to the request as a bare
text part and persisted verbatim, so hook-injected text was
indistinguishable from user-authored text in the transcript, polluted
resumed sessions, telemetry, and auto-memory recall queries.
- Wrap injected context in a reserved
<qwen:user-prompt-submit-context> tag (hook output already escapes
angle brackets, so the tag cannot be forged from inside).
- Record the pre-injection user prompt as systemPayload.displayText plus
the injected string as hookContext on the user record; the model-bound
message stays verbatim for faithful resume replay.
- Use the pre-injection prompt text for telemetry prompt attributes and
managed auto-memory recall.
- Resume projection prefers displayText, strips a trailing whole-part
tagged block when no payload exists, and leaves legacy bare-injected
records unchanged.
- Apply the same tag wrapping on the ACP session injection path, which
already records the pre-injection prompt.
Closes#7940
Co-authored-by: Cursor <cursoragent@cursor.com>
* docs: note UPS promptText TDZ ordering and sole-part resume guard
Document the conflict-resolution constraint that promptText must be
declared before the injection assignment, and the sole-part read-path
guard that keeps a user-authored whole-tag message intact.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test(cli): cover at_command resume with tagged UPS context
Confirm the at_command branch still prefers payload.userText when a
paired user record carries a trailing tagged hook-context part, and
falls back to the tag-stripping projection only when userText is absent.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(core): address PR 7956 review findings and Goal recording spy
Omit the optional UserPromptRecordPayload third arg when no hook
injected, so Goal admission spies expecting two args stay exact and
CI client-goal.test.ts passes.
Project plain UserPromptSubmit-augmented records through
transcript-replay with the same displayText / trailing-tag strip
fallback as the TUI, covering ACP/export surfaces. Strengthen the
displayText preference fixture so it disagrees with the tag-strip
path, and use the named UserPromptRecordPayload type in resume.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(acp-bridge): import UPS tag helper via Node-free package export
transcript-replay is inlined into the browser daemon/transcript SDK
bundle. Importing isUserPromptSubmitContextPartText from the core
package barrel pulled the whole Node-bound core graph into that
bundle and failed CI (esbuild Could not resolve "node:*") across
Test, web-shell E2E, and Real daemon E2E.
Export the pure helper as @qwen-code/qwen-code-core/userPromptSubmitContext
and import that path instead.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(test): alias userPromptSubmitContext for Vitest source resolution
CLI and acp-bridge Vitest configs already map goalWire/transcriptRecords
to TypeScript sources; without the same alias the new package export
fails import analysis and breaks dozens of CLI suites.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(acp-bridge): keep images when projecting displayText user records
Preferring UserPromptSubmit displayText previously returned early and
skipped projectMessageParts, dropping multimodal inlineData. Rebuild
parts so displayText replaces text while images keep their order.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(core): drop unused hookContext and cover image-only displayText
UserPromptRecordPayload.hookContext had no read sites; keep displayText
only and recover injected text from the tagged message part. Also cover
the image-only !replaced append path and simplify the recording guard.
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: cover remaining UserPromptSubmit provenance Suggestions
Share stripTrailingUserPromptSubmitContextPart between TUI resume and
ACP replay, assert ACP Session tags additionalContext, and lock
telemetry to the pre-injection prompt text.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Extract McpClientManager.getEffectiveMcpServers() as the single source of truth for the populateMcpServerCommand recipe, replacing six duplicated call sites that this PR otherwise had to edit in lockstep. Also cover relocateWorkingDirectory's combined memory + MCP refresh failure path so both errors stay surfaced.
* fix(cli): stamp QWEN_CODE_CLI at the workspace entry and publish the active model as QWEN_CODE_MODEL
Skill subprocesses shell out through `"${QWEN_CODE_CLI:-qwen}"`. The npm entry
(scripts/cli-entry.js) stamps QWEN_CODE_CLI, but the workspace entry
(packages/cli/dist/index.js) never did — so a dev run, or any direct
`node dist/index.js` launch, leaves the variable unset and every review
subcommand the /review skill issues silently lands in whatever global `qwen`
PATH resolves to. Measured on a live run: a freshly built CLI's review pipeline
executed entirely on a global v0.21.0 — `script-lint` did not exist there, so
the deterministic gate the skill expected was silently absent, and any
behavioral fix to the review CLI is inert in such runs.
Stamp the entry in runCliEntryPoint, first-writer-wins: an outer launcher
(cli-entry.js, the desktop shim) has already stamped in-process and must keep
winning; an empty value counts as unset, matching the consumer's `:-`
semantics. The entry is derived as `../index.js` from the compiled
dist/src/cli.js — the shebang-bearing bin — and skipped entirely for non-file
schemes (vitest) and unbuilt layouts (tsx dev runs keep today's fallback).
tsc emits dist/index.js as 0644 and the spawn-time filter blanks a
non-execable entry, so the stamp grants 0o755 best-effort; a failed chmod
degrades to today's `qwen` fallback.
Separately, subprocesses had no authoritative way to learn the ACTIVE model:
the /review skill's compose step wants a modelId, and the orchestrator resorts
to reading settings files — wrong under QWEN_HOME isolation and after /model
switches (measured: a report stamped with a model that never ran the review).
Publish QWEN_CODE_MODEL exactly the way QWEN_CODE_SESSION_ID is published: the
first Config claims the process-global slot, only the claiming instance
republishes (on refreshAuth and every model-change notification), and
getShellContextEnvVars passes it through, omitted when absent. The daemon
limitation is the session ID's own — later sessions read the first session's
model — and is documented at both the producer and the consumer.
* fix(core): clarify QWEN_CODE_MODEL daemon comment and cover refreshAuth republish (#7993)
* test(cli): cover stampCliEntryEnv wiring in runCliEntryPoint (#7993)
* fix(core): publish QWEN_CODE_MODEL per session and preserve entry mode on stamp (#7993)
Address review feedback:
- Key QWEN_CODE_MODEL on the session (registerSessionModel/getSessionModel),
mirroring the project dir, so daemon-mode subprocesses read their own
session's active model instead of the first session's. The process-global
slot remains as the single-session CLI fallback. This also neutralizes the
order-dependent claim: a throwaway Config's registration is keyed under a
session id no real spawn resolves.
- stampCliEntryEnv now adds exec bits to the existing mode (mode | 0o111)
rather than setting 0o755, so a private 0o600 checkout becomes execable
without becoming world-readable.
- Cross-reference scripts/dev.js and scripts/start.js in the stamp doc comment
and note the bundled `node dist/cli.js` launch is intentionally not stamped.
- Widen the AuthType test mock to include QWEN_OAUTH, pin the stamp-before-run
ordering in the wiring test, and cover the per-session model lookup.
* fix(cli): correct comment on Vite rewrite mechanism in protocol guard (#7993)
* fix(core): re-key per-session model registry on startNewSession (#7993)
startNewSession minted a new session id and re-stamped QWEN_CODE_SESSION_ID
but left the per-session model registry keyed on the outgoing id. After
/clear (or /reset, /new, /resume) a non-owner Config's subprocesses then
resolved the model by the new id, missed, and fell back to another
session's value. Unregister the old entry and republish under the new id.
Also correct the stampCliEntryEnv comments: npm start / npm run dev route
through scripts/start.js and scripts/dev.js, which stamp QWEN_CODE_CLI
themselves, so the only uncovered launcher is a direct node dist/index.js.
---------
Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@alibaba-inc.com>
* fix(core): preserve active Todo context across tool turns
* test(cli): update automatic turn prompt expectation
* fix(core): preserve Todo ownership across automatic turns
* fix(core): preserve Todo ownership at prompt boundaries
* test(todo): cover automatic reminder boundaries
* fix(core): throttle active Todo reminder re-injection to bound history growth
Every injected reminder copy lands permanently in chat history, so per-turn
injection grew the live context linearly with tool turns. Tool-turn injection
now re-issues the reminder only every third tool turn since the state was
last presented; turn-start injections always fire and reset the cadence. The
payload becomes a compact status/content line list capped at 800 characters.
History stays append-only, so provider prefix caching is unaffected.
Also: cover the new-ordinary-prompt-clears-stale-reminders invariant on the
real Config, add TUI coverage for the work-chain notification batch split,
cover todoWorkChainId continuation forwarding, and document the deliberate
enterWith binding in the daemon tool runner.
* fix(core): keep todo reminder before drained input
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
estimatePromptTokens's steady-state branch uses char/4 to estimate the
size of newly-added content (e.g. tool results just appended to history).
char/4 is a flat ratio that does not account for CJK text density, and two
independent real production failures (both 2026-07-28, on
KAT-Coder-V2.5-Dev and Nex-N2-mini) traced back to this: large CJK-dense
tool results (parallel read_file calls returning design docs) were
under-counted by 39-54%, so clampOutputTokensToWindow computed a
maxOutputTokens that was too large, and prompt + max_tokens exceeded the
window by ~1 token in production, triggering a 400 that propagates past
reactive compression (which correctly detects it has no room left, but
can't undo an already-oversized request).
Add an opt-in `conservative` parameter to estimatePromptTokens that
inflates only the new-content term by 1.5x (ceil'd) — calibrated against
the two observed under-count ratios with headroom, applied only at the
two call sites that feed clampOutputTokensToWindow (where under-counting
is dangerous), left off at the cheap-gate/compaction-trigger call sites
(where the existing code comments already document under-counting as
safe — it only makes compaction trigger earlier, never skip it).
Co-authored-by: Zamba Lee <zamba@thinktop.com>
The transport stream retry was gated on !streamYieldedChunk, which any
thinking/reasoning chunk trips within seconds for thinking models. A
gateway closing the long-lived SSE connection minutes into the thinking
phase (UND_ERR_SOCKET) then propagated as a fatal error even though
zero user-visible content had been produced — making large generations
in YOLO/headless mode effectively impossible (#7832).
Track a separate streamYieldedContentChunk that is only set when a
chunk carries a non-thought candidate part (text, functionCall, …) and
gate the replay on it instead. Thought parts are ephemeral — never
recorded as the assistant's response in history — so replaying after
thinking-only output cannot duplicate visible content. Content-phase
failures keep the existing behavior: once a non-thought part has
flowed, the retry stays blocked and the error propagates.
The not-taken log now reports skipped_after_content, and the retry log
gains yieldedNonContentChunks so thinking-phase replays are visible in
diagnostics.
Fixes#7832
Co-authored-by: ComplexSimply <rudy.arrowsong@gmail.com>
* fix(core): add certified session writer handoff
Seal managed writer ownership after a durable recorder drain and allow trusted replacements to take over only when the transcript proof still matches.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): retain writer claim after failed rollback
Keep the fixed transition claim unless the exact predecessor primary is restored, and avoid rollback after claim ownership changes or unlink completes with a reported error.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): close session handoff claim races
Wait for claim-aware primary candidates to back out of transition gaps, and release losing claims when no primary transition started.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): bound session handoff candidate waits
Fail closed when a claim-aware primary candidate is abandoned during a handoff transition, preserving the transition fence for authoritative recovery.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): reconcile session handoff proof edges
Reject dangling transcript paths and reconcile ambiguous lock-record link outcomes by exact bytes before continuing a certified handoff.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): preserve inspect error cause and document handoff fail-closed paths (#7976)
---------
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: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* feat(core): preload deferred tools within a context-window threshold
Adds tools.toolSearch.threshold (default 10, percent of the context
window). At session start, when the combined estimated schema footprint
of every deferred tool - bundled built-ins and MCP alike - fits within
the budget, all are revealed upfront so the declaration list stays
stable for the whole session and prefix KV caches survive; otherwise
everything stays deferred. Set 0 to always defer. Mirrors Claude Code's
ENABLE_TOOL_SEARCH=auto threshold mode, extended to bundled deferred
tools because here every reveal rewrites the declaration list and busts
the prompt-cache prefix.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat(core): log deferred-tool preload budget decision
Emit debugLogger diagnostics in preloadDeferredToolsWithinBudget covering
the computed budget, estimated token footprint, candidate count, and which
branch of the all-or-nothing gate was taken (no candidates, over budget, or
preloaded). Lets an operator diagnosing session-startup cost tell from debug
logs whether the deferred set fit the budget or was left behind ToolSearch,
without adding temporary instrumentation. No behavior change.
* fix(tools): bound toolSearch.threshold to 0-100%
The threshold setting is a percentage of the context window but had no
upper bound, so a value like 200 (a typo or misreading of the "(%)"
label) made the preload budget exceed the whole window and
unconditionally preloaded every deferred tool — the opposite of the
prefix-stability the threshold buys.
- Add minimum:0/maximum:100 to the setting schema (jsonSchemaOverride,
like autoCompactThreshold) and regenerate the VS Code settings schema.
- Add a symmetric runtime upper guard next to the existing
'thresholdPercent <= 0' lower guard in client.ts, clamping to 100% so a
hand-edited settings file cannot slip a larger budget past validation.
Adds a client test asserting a 200% threshold clamps to a full-context
budget.
* test(core): cover configured preload budget
* fix(tool-search): harden preload threshold
* test(tool-search): cover preload exclusions
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(core): rebind fork capabilities on resume
* refactor(core): share fork parent-tool extraction and cover resume guards
Extract the duplicated parent-tool-name derivation (flatten function
declarations, drop EXCLUDED_TOOLS_FOR_SUBAGENTS, dedupe) into a shared
extractParentToolNames() helper in agent-core, used by both fork launch
(the Agent tool) and fork resume (resolveCurrentForkRuntime), so the two
paths cannot silently diverge when the extraction or exclusion logic changes.
Add tests for the two previously-uncovered resolveCurrentForkRuntime
early-return branches: (1) every advertised parent tool is excluded from
subagents, and (2) no advertised tool is still registered in the live
ToolRegistry. Both keep the fork paused with the current-runtime-unavailable
blocked reason.
* test(core): cover fork-resume runtime guards and add resolve diagnostics
Address review feedback on the fork capability-rebind path:
- Add a direct unit test for extractParentToolNames (agent-core.test.ts)
covering cross-group flattening, dedup, EXCLUDED_TOOLS_FOR_SUBAGENTS
filtering, empty/undefined name filtering, and undefined/empty config.
- Add a fork-resume test asserting the MCP, skill, and deferred-tool
reminder branches of buildForkResumeCapabilityReminder are injected into
the resumed task prompt (previously only exercised with empty data).
- Emit debug diagnostics at each early return in resolveCurrentForkRuntime
(no_system_instruction / no_advertised_tools / no_registered_tools) so a
paused fork's blocking cause is distinguishable in logs.
* fix(core): address fork capability review
---------
Co-authored-by: Claude <noreply@anthropic.com>
Anthropic's Messages API rejects any tool `input_schema` carrying a
top-level `oneOf`/`allOf`/`anyOf` combinator with a hard 400:
input_schema does not support oneOf, allOf, or anyOf at the top level
send_message's schema used `oneOf: [{ required: ['to'] }, { required:
['task_id'] }]` to express "either a teammate recipient or a
background-task id is required." Since this schema is forwarded
verbatim as `tools[].input_schema` to Anthropic
(anthropicContentGenerator/converter.ts -> convertSchema), every call
to send_message on an Anthropic-backed model (native API, Vertex, or
an Anthropic-compatible proxy) failed outright -- not degraded, fully
unusable.
Verified live against the Anthropic Messages API (claude-sonnet-5):
the schema with `oneOf` 400s with the exact message above; the
identical request with `oneOf` removed succeeds (200, tool_use
returned).
The `oneOf` constraint was redundant defense-in-depth, not the only
enforcement: send_message's execute() already returns a clear runtime
error ("No active team and no task_id provided...") when neither `to`
nor `task_id` is supplied. Dropping the schema-level constraint leaves
that runtime validation intact and unchanged for OpenAI/Gemini-backed
models, while making the tool actually callable on Anthropic-backed
models.
Fixes#7984
Co-authored-by: Palanisamy, Dinesh <Dinesh.Palanisamy@netapp.com>
* feat(hooks): add security.allowPrivateNetworkHooks to bypass SSRF range checks for trusted scopes
HTTP hooks hard-block all private/link-local address ranges via ssrfGuard,
which makes them unusable in platform-managed environments where the hook
receiver is a first-party, VPC-internal endpoint (e.g. an internal API
gateway resolving to 172.16.0.0/12).
Add an opt-in setting, security.allowPrivateNetworkHooks (default false),
that skips the SSRF IP-range checks in urlValidator.isBlocked (literal IPs)
and validateResolvedHost (literal + post-DNS-resolution paths).
Security properties:
- Honored only from User/System/SystemDefaults scopes; the value is
stripped from Workspace settings during the merge (with a startup
warning), so a cloned repository can never self-grant the bypass.
- BLOCKED_HOSTS (169.254.169.254, metadata.google.internal, ...) remains
blocked even when the flag is on.
- Default false keeps every code path byte-for-byte compatible with
current behavior; bare/safe mode forces it off.
* fix(hooks): enforce metadata endpoint blocklist regardless of allowPrivateNetworkHooks
Address review findings on #7968: with the flag on, cloud metadata
endpoints were reachable through gaps in the relaxed checks.
- ssrfGuard: add METADATA_IPS (169.254.169.254, 100.100.100.200) and
isMetadataAddress(), which normalizes IPv4-mapped IPv6 forms
(::ffff:a9fe:a9fe, ::ffff:6464:64c8, ...) via the existing
extractMappedIPv4/expandIPv6Groups helpers.
- urlValidator.isBlocked: BLOCKED_HOSTS matching and the literal-IP
isMetadataAddress check now run unconditionally; only the general
range check (isBlockedAddress) is relaxed by the flag.
- httpHookRunner.validateResolvedHost: no longer returns early with the
flag on — DNS resolution still runs and resolved addresses are checked
against isMetadataAddress, so a hostname resolving to a metadata
endpoint is blocked. DNS failures still defer to fetch, as before.
- settings warning text now lists User/System/SystemDefaults, matching
the schema and docs.
- docs: precise wording — the flag relaxes only range checks; metadata
endpoints stay blocked in all serialized forms and after DNS resolution.
The flag now opens RFC1918/CGNAT/link-local ranges only; cloud metadata
endpoints (169.254.169.254, 100.100.100.200 in any form, plus the
BLOCKED_HOSTS hostnames) are unreachable in every configuration.
---------
Co-authored-by: 欢伯 <ri.xur@alibaba-inc.com>
* fix(serve): allow bounded reads of large text files
* fix(serve): bound large-text reads by scan cost, not by which knob was set
Follow-up to the bounded large-text read path. Three changes:
Gate on any explicit window argument, not on `limit`. Gating on `limit`
had the cost model backwards in both directions: `{ line: 900_000_000,
limit: 20 }` was admitted despite walking the whole file, while
`{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A
read with no window argument at all still fails, since a caller that
believes it holds the whole file may write it back truncated.
Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read
returns; nothing capped what it cost. Line offsets are resolved by
scanning from byte 0, so a query param could turn into an
uninterruptible multi-second scan of an arbitrarily large file — and on
Windows hold a read handle for that span, blocking renames and deletes.
Past the budget the read is refused with `file_too_large` pointing at
readBytes, which reaches any offset in O(1).
Tolerate appends on streamed windows. Requiring whole-file size/mtime
stability after reading a prefix rejected reads whose returned bytes
were still valid, and the case it rejected — tailing a live log — is the
one this path exists for. Streamed windows now assert inode identity
plus "did not shrink"; truncation and replacement are still rejected.
Also: non-UTF-8 large text now returns `binary_file` rather than
`file_too_large`, so a client retrying on 413 with a smaller window
can't loop forever; and `readFileWithLineAndLimit` throws instead of
silently ignoring a caller-supplied `fileHandle` on the by-path
fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): harden large text range snapshots
Treat caller-owned file handles as bounded streaming reads, cap them to the captured file size, and reuse the chunk buffer.
Restore strict Serve snapshot stability and align returned-slice metadata with the full-snapshot path.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): make large text ranges snapshot-safe
* fix(serve): harden large-text ctime tests and document buffer reuse (#7947)
Address review feedback on the large-text range read PR:
- Pause before restoring mtime in the two ctime-dependent mutation tests so the change-time advances past the pre-read snapshot even on coarse-resolution filesystems, removing a latent flake in the same-size-overwrite precondition. The assertions are unchanged.
- Document at the readFileHandleChunks yield site that the 512 KiB buffer is reused across iterations, so yielded views must be decoded or copied before advancing the generator.
* docs(serve): soften same-size rewrite guarantee to coarse-clock best-effort (#7947)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
* fix(core): mark folders the item budget never expanded
A folder is pushed into its parent's subFolders and queued for expansion.
When it is later dequeued and the maxItems budget is already spent, the
BFS skips it with a bare `continue`, leaving hasMoreFiles and
hasMoreSubfolders unset. It then formats as a bare leaf -- byte for byte
how a genuinely empty directory formats.
So the same tree was described differently depending only on the budget.
The existing suite shows it: `level1/level2/level3/file.txt` renders
level3 with its file at maxItems 10, and as an empty directory at
maxItems 3. The subfolder test is worse -- folder-0 through folder-3 each
contain child.txt and all four render as empty.
The parent's hasMoreSubfolders flag does not cover this. It means "I could
not add every subfolder", and these folders were added; it is only the
expansion that never happened.
Set hasMoreSubfolders on the skipped node so it renders with the
truncation indicator. Only that one flag is set: formatStructure emits an
indicator per flag, and this one alone produces the single trailing `...`.
Three existing expectations encoded the old rendering and are updated.
* docs(core): state both meanings of hasMoreSubfolders on the interface
This PR widened the field: it already meant "subfolders were truncated" and
now also means "the item budget ran out before this folder was read." The
interface comment only described the first, so a second consumer reading just
the type could gate rendering on `subFolders.length > 0` and silently drop the
indicator for never-expanded folders, reintroducing the same
looks-like-an-empty-directory bug in a new path.
* fix(core): improve ripgrep runtime reliability
Make ripgrep failures distinguishable from true no-match results so the
model avoids unsafe conclusions from partial or failed searches. Add a
narrow recovery path for confirmed worker-thread EAGAIN failures.
- Retry confirmed thread EAGAIN once with a single worker thread
- Treat exit code 1 as no-match only when both streams are empty
- Mark partial runtime results as incomplete, separate from display limits
- Add privacy-safe telemetry and focused coverage for recovery behavior
# Conflicts:
# packages/core/src/utils/ripgrepUtils.test.ts
* fix(core): restore ripgrep runtime recovery tests
Restore the runRipgrep coverage that was lost during the rebase and keep
EAGAIN detection aligned with the runtime reliability design.
- Re-add coverage for strict no-match handling and incomplete output
- Verify single-thread retry behavior for confirmed EAGAIN failures
- Keep spawn, cancellation, timeout, and max-buffer paths covered
- Treat os error 11 as the short EAGAIN marker documented by the plan
* docs(core): document ripgrep recovery boundaries
Clarify the non-obvious runtime reliability edges around ripgrep recovery so
future changes preserve the intended narrow behavior.
- Document why only confirmed worker EAGAIN failures are retryable
- Explain incomplete-output handling for interrupted ripgrep output
- Note the privacy boundary for runtime recovery telemetry
* fix(core): correct exit-1 no-match gate for --json summary output (#7888)
* test(core): remove duplicate mockReset and add telemetry coverage (#7888)
* fix(core): address review feedback on ripgrep recovery semantics (#7888)
- Fix stale `truncated` property in test mock to match RipgrepRunResult
- Narrow `incomplete` flag to genuinely interrupted executions only
- Add test for exit code 1 with both stdout and stderr
- Add test for EAGAIN retry producing partial stdout
- Alias RipgrepRuntimeRecoveryFailureKind to RipgrepFailureKind
* docs(core): align exit-1 no-match spec with stderr-only gate (#7888)
* fix(core): mark exit-failed ripgrep searches with stdout as incomplete (#7888)
---------
Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* feat(core): integrate Goal turn engine
* test(core): preserve Goal tool isolation in agent overrides
* fix(core): pause Goal at Stop hook cap
* fix(core): keep Goal recovery from blocking sessions
* fix(core): degrade failed Goal migration writes
* fix(core): reset loop detector in Goal stop hook continuation (#7895)
The goal-runtime stop hook continuation path was missing
loopDetector.reset(prompt_id) before recursing, causing tool calls
to accumulate across iterations and trip TURN_TOOL_CALL_CAP after
a handful of healthy iterations. The non-goal stop hook path already
had this reset.
Also simplifies the redundant conditional in Turn.run() — the two
near-identical sendMessageStream calls are collapsed into one since
sendMessageStream already handles an undefined goalContext internally.
* fix(core): align turn.test.ts assertion with unified sendMessageStream call (#7895)
* fix(core): preserve error cause in GoalPersistenceUnavailableError (#7895)
---------
Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com>
* feat(web-shell): add git branch picker, commit dialog, and create PR flow
Add an IntelliJ-style branch picker popover to the web shell git
workspace, accessible from the branch chip in both the composer toolbar
and sidebar. The picker provides search-filtered branch listing (local,
remote, tags, recent), branch checkout, new branch creation, pull, push,
and a commit view integrated into the existing GitDialog.
The commit view reuses the diff panel (expandable file diffs with syntax
highlighting, fullscreen support) and adds a commit message textarea with
Commit / Commit and Push buttons. When a session is available (or one is
auto-created), the commit message and PR title/body are generated via the
model using session side-queries (btwSession), giving the agent full
conversation context for accurate generation.
The Create PR flow provides an inline form with auto-detected base branch
and model-generated title/description, backed by a new daemon route that
shells out to gh pr create.
New daemon routes:
- GET /workspaces/:workspace/git/branches
- POST /workspaces/:workspace/git/checkout
- POST /workspaces/:workspace/git/branch
- POST /workspaces/:workspace/git/push
- POST /workspaces/:workspace/git/pull
- POST /workspaces/:workspace/git/commit
- POST /workspaces/:workspace/github/prs/create
- GET /workspaces/:workspace/github/default-branch
* fix(web-shell): resolve correct workspace session for AI generation
The commit message and PR title/body generation now resolves the
most recent session for the target workspace via listWorkspaceSessions,
rather than using the globally active connection.sessionId which may
belong to a different workspace or session. Falls back to creating a
new session only when no sessions exist for the workspace.
Also improves the PR body editor with an Edit/Preview toggle using
the existing Markdown component, and updates the generation prompt
to follow the project PR template structure from AGENTS.md.
* fix(web-shell): stabilize session resolver prop to prevent infinite re-generation
Pass resolveSessionForWorkspace as a stable useCallback reference
instead of an inline arrow function. The inline function created a
new reference on every App render, causing the GitDialog useEffect
to abort and restart generation in an infinite loop.
* fix(web-shell): group remote branches by remote name and add PR target branch dropdown
Remote branches in the branch picker are now grouped by remote
(origin, upstream, etc.) with sub-headers, making fork workflows
clear. The PR create form's base branch field is now a select
dropdown populated from the workspace's branch list, grouped by
remote with optgroup labels, instead of a free-text input.
* fix(web-shell): use ref for session resolver to prevent effect re-run abort
When resolveSessionForWorkspace creates a new session, it updates
connection.sessionId in the provider, which changes the useCallback
reference, which triggers the useEffect to re-run and abort the
in-flight btwSession generation. Store the callback in a ref so the
effect never depends on its identity.
* fix(web-shell): resolve session per workspace, not from global active session
The commit/PR generation effects used connection.sessionId directly
without checking if it belongs to the target workspace. When opening
the commit dialog from a sidebar workspace different from the active
session's workspace, the wrong session was used for generation,
producing incorrect content. Now always routes through
resolveSessionForWorkspace(workspaceCwd) which checks workspace
membership before reusing the active session.
Also replaces 'PR' with 'Pull Request' / '合并请求' in all UI strings
and adds error logging to the generation catch blocks.
* fix(web-shell): retry btwSession with fresh session when stale session detected
When listWorkspaceSessions returns a session that no longer exists in
the daemon's memory (e.g. after daemon restart), btwSession fails with
'No session with id ...'. The generation effects now catch this error,
force-create a new session via resolveSessionForWorkspace(cwd, true),
and retry the btwSession call once. Both btwWithRetry and
resolveSessionForWorkspace are stored in refs to avoid useEffect
dependency chain aborts.
* fix(web-shell): base PR generation on branch diff, not working tree
PR title/body generation now fetches the commit log between the
resolved base branch and HEAD (git log <base>..HEAD) plus any
uncommitted changes, instead of only the working tree diff. The
base branch is resolved inside the effect's promise chain (not
from state) to avoid stale values and dependency warnings. Also
adds range parameter support to fetchGitLog and workspaceGitLog.
* feat(web-shell): replace PR base branch select with searchable popover
The native <select> for the PR target branch is replaced with a
custom searchable popover (BranchSelect) that shows a search input
and a filtered branch list grouped by remote. The default selection
is the target repository's main branch (resolved via
getDefaultBranch). Supports filtering by typing in the search box.
* fix(web-shell): show full remote ref in branch select (origin/main)
Branch select now stores and displays full remote refs like
origin/main instead of stripped names. getDefaultBranch returns
the full ref (origin/main) instead of stripping the prefix.
When creating the PR, the remote prefix is stripped for the
gh pr create --base flag (origin/main → main).
* fix(web-shell): stop pointer propagation in branch picker list to prevent popover dismiss
Radix Popover's outside-click detection was incorrectly firing when
clicking section headers (Recent/Local/Remote/Tags) inside the popover
content, causing the popover to close immediately. Adding
onPointerDown stopPropagation on the list container prevents pointer
events from reaching Radix's document-level handlers.
* fix(web-shell): use onPointerDownOutside guard for branch picker popover
The previous stopPropagation approach failed because Radix Popover
uses capture-phase document listeners for outside-click detection,
which fire before bubble-phase stopPropagation. The correct fix is
onPointerDownOutside on PopoverContent: when Radix incorrectly fires
the outside handler for a click that is actually inside the content
(can happen with portal containers), we check contentRef.contains()
and preventDefault to keep the popover open.
* fix(web-shell): stop click propagation on branch picker popover content
Root cause: the ChatEditor composer container has onClick that calls
core.focus(), stealing focus from the popover. React synthetic events
bubble through the React tree (not DOM tree), so portaled popover
clicks reach the container handler. Radix then detects focus-outside
and dismisses the popover.
Fix: onClick stopPropagation on PopoverContent, matching the existing
pattern in GitModePopover and ToolbarPopover which already have this
fix with an explanatory comment.
* docs: add PR verification screenshots for branch picker feature
* fix(web-shell): mock useWorkspace in tests for BranchPickerPopover
BranchPickerPopover calls useWorkspace() which requires
DaemonWorkspaceProvider context. The existing WorkspaceSection and
ChatEditor tests didn't provide this context, causing 5 test failures.
Added vi.mock with importActual to preserve other exports while
providing a mock useWorkspace. Also updated the git chip click test
to reflect that clicking now opens the branch picker popover instead
of directly calling onOpenGitDiff.
* fix: harden git write paths against argument injection
Address review feedback on the web-shell git surface:
- Reject option/pathspec injection in git checkout ref and branch start
point (isValidCheckoutRef), and terminate `git checkout` argv with `--`.
- Drop `git log` range values that start with `-` and terminate the argv
with `--` so a range can never be reinterpreted as `--output=<file>`.
- Fix getDefaultBranch always falling back to origin/main: the promisified
exec lacked `encoding: 'utf8'`, so stdout was a Buffer and .trim() threw.
- Parameterize ghErrorMessage so `gh pr create` timeouts name the right
command and duration; sanitize workspace paths in PR-create errors.
- GitDialog: guard doCommit against double-submit (button + keyboard), and
strip only a known remote prefix from the PR base so local branches with
"/" are not mangled; use theme tokens for commit button/success colors.
- BranchPickerPopover: guard checkout/new-branch behind busyAction, reset
inline-input text on reopen, and hide the commit action when unavailable.
Adds regression tests for the checkout/branch validation and the git log
range guard.
* fix(web-shell): address review feedback on branch picker PR (#7731)
- Fix Commit+Push error masking: split try/catch so push failure
reports alongside the successful commit SHA
- Replace hardcoded screenshot path with captureScreenshot harness
- Replace silent if-isVisible skip with explicit assertion in visual test
- Add focus-visible style for search input accessibility
- Fix CSS specificity for active PR tab hover state
- Add viewChanges to actionsVisible search filter
- Wrap toggleSection in useCallback to avoid unnecessary re-renders
- Add windowsHide: true to getDefaultBranch subprocess
- Fix trailing slash handling in mockDaemon git action routing
- Add git methods to top-level client mock in tests
- Remove dead branchPicker.commitSuccess i18n key
- Remove dead .actionShortcut CSS class
- Show generation failure feedback in commit message placeholder
* fix(web-shell): address review feedback on branch picker PR (#7731)
- Add workspace trust checks to bound git branch routes
- Use workspace-scoped client in BranchPickerPopover (fixes wrong-workspace mutation)
- Add branch name validation and -- terminator to gitCreateBranch
- Filter refs/remotes/*/HEAD from branch listings
- Force LC_ALL=C for reflog parsing (non-English locale fix)
- Narrow 'could not resolve' error regex to avoid DNS false positives
- Add range validation to git log (reject path traversal)
- Fix commit+push error i18n (dedicated key instead of concatenation)
- Add i18n for BranchSelect component strings
- Fix commit tab ARIA attributes
- Add onBranchChanged callback to handlePush
- Add btw to mockDaemon isDaemonPath regex
- Add workspace_github_prs to visuals spec capabilities
- Add -- terminator regression test
- Remove docs/pr-assets/ from repo
* fix(web-shell): address review feedback on branch picker PR (#7731)
* fix(cli): reject dash-prefixed branch name with 400 in branch route (#7731)
* fix(web-shell): address review feedback on git branch picker (#7731)
Security:
- Clear GIT_DIR/GIT_WORK_TREE/GIT_COMMON_DIR/GIT_INDEX_FILE from git
subprocess env to prevent repository redirection
- Add strict mutation gate to all POST git branch routes
- Add generation guard to qualified write routes
- Fail closed on invalid ?cwd= in mutation routes (resolveContainedCwdOrFail)
- Reject wrong-typed startPoint, fetchOnly, rebase, and PR options with 400
Correctness:
- Filter remote symbolic refs (origin/HEAD) by %(symref) instead of /HEAD
name suffix, preserving valid branches like feature/HEAD
- Add git rev-parse --git-dir probe so non-git dirs get 404 instead of
empty available:true
- Push preserves existing upstream; only adds --set-upstream when unset,
resolving the remote from branch config or the sole configured remote
- git commit -a replaced with git add -A + git commit so untracked files
displayed in the UI are included
- Always pass --body to gh pr create to prevent interactive prompts
- getDefaultBranch returns null instead of fabricating origin/main
- Memoize workspaceByCwd client in BranchPickerPopover to fix infinite
render loop
- Move sessionId to a ref in GitDialog effects to prevent self-abort
- Bound commit-message prompt to fit /btw 4096-char limit
- Mark all platforms as unverified in PR template (no fabricated ✅)
- Guard PR auto-fill effect against wiping user edits on reconnect
Accessibility:
- Add tabIndex and onKeyDown to commit-mode tab span
- Add aria-label to BranchSelect trigger and search input
Cleanup:
- Remove dead CSS (.prInputSmall, .prSelect)
- Remove 9 unused i18n keys
- Add ^ to git log range validation regex
- Add busyAction guard to handlePush/handlePull
- Add unit tests for gitCommit, gitPull, and route input validation
* fix(web-shell): address review feedback on git branch picker PR (#7731)
- Change commit tab from <span> to <button> for keyboard accessibility
- Move setCommitMsg('') to success-only branches so the message is
preserved when push fails after a successful commit
- Add mutate middleware and generationGuard to PR creation route,
matching all other POST mutation routes
- Set genFailed when session resolution returns undefined so the user
sees the failure indicator instead of a silent empty textarea
- Make PR number nullable when URL regex does not match instead of
returning a misleading 0
- Add LC_ALL=C and LANG=C to gitEnv() so for-each-ref upstream track
parsing is locale-independent
- Validate setUpstream and force as booleans in handlePush, matching
the existing validation in handlePull
- Add missing workspaceCwd and available fields to test mocks
- Use stable data-web-shell-git-branch attribute in e2e selector
* fix(web-shell): address R5 review feedback on git branch picker PR (#7731)
- Classify git errors on stdout+stderr instead of err.message to fix
false-positive no_upstream on every push failure and dead
nothing_to_commit classifier
- Sanitize workspace paths and cap error message length in sendGitError
- Fix remote branch checkout to strip remote prefix so git DWIM creates
a local tracking branch instead of detaching HEAD
- Restore keyboard accessibility on composer branch chip (span → button)
- Trim startPoint in handleCreateBranch before forwarding to git
- Return bare branch name from getDefaultBranch (strip remote prefix)
- Fix i18n shortcut hint to show ⌘/Ctrl+Enter for cross-platform
- Update aria-label to reflect git management menu, not just changes
- Add available: true to mockDaemon gitDiff default payload
- Add regression tests: upstream preservation, sole remote resolution,
strengthened fetch-only with divergent remote commit
- Add aria-expanded assertion to sidebar picker test
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(web-shell): address R6 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R7 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R8 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R9 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R10 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R11 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R12 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R13 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R14 review feedback on git branch picker PR (#7731)
* fix(web-shell): address R15 review feedback on git branch picker PR (#7731)
- Validate localName derived from remote-tracking ref to prevent option
injection (e.g. origin/-f → git checkout -f)
- Add gitCwd prop to BranchPickerPopover and pass it to all git SDK calls
so worktree sessions target the correct directory
- Add symlink-escape and non-existent-path tests for resolveContainedCwdOrFail
- Pin initial branch name in makeRepo() with git init -b master
- Add gitPull merge and rebase integration tests
* fix(web-shell): address git branch picker review feedback (#7731)
* fix(web-shell): address R6 review feedback on git branch picker PR (#7731)
* fix(web-shell): address review feedback on git branch picker PR (#7731)
- Strip GIT_CONFIG_GLOBAL/SYSTEM/NOSYSTEM in gitEnv to prevent
inherited config redirection (consistent with extension/github.ts)
- Pass gitCwd to workspaceGitBranches in GitDialog loadPrBranches
so worktree sessions fetch branches from the correct repository
- Add aria-expanded to collapsible branch section headers
- Add happy-path tests for PR create (201) and default-branch (200)
routes, including the null fallback to origin/main
* fix(cli): add sendGenerationClosedError to POST routes and cover untested branches (#7731)
* fix(web-shell): address review feedback on branch picker and PR creation (#7731)
- Refresh branch list after push/pull to avoid stale ahead/behind counts
- Add pre-flight check for unpushed branches before PR creation
- Fix base branch prefix stripping when branch list is unavailable
- Cap PR body file list at MAX_SUMMARY_CHARS to bound model prompt size
- Add qualified route tests: trust guard, input validation, cwd containment
* fix(web-shell): hoist MAX_SUMMARY_CHARS to module scope for PR body generation (#7731)
* fix(web-shell): address review feedback for git branch picker (#7731)
- Hoist onOpenCommit to useCallback to fix App.test.tsx prop stability test
- Keep commit tab visible after navigating away (startedInCommit ref)
- Add onClick handler to commit tab for navigation back to commit view
- Fix branch-prefix strip mangling local branch names containing '/'
- Update sessionIdRef after force-creating a stale session replacement
- Pin core.hooksPath in test makeRepo for reliable rollback tests
- Add test asserting --force-with-lease is used for force pushes
* fix(web-shell): target the worktree for sidebar commits and harden git actions (#7731)
Scope the sidebar commit dialog to the active session's worktree checkout
(matching the composer path) so linked-worktree sessions commit to the right
checkout, guard PR creation against a double-click race, and surface an error
when an invalid branch name is submitted. Adds focused coverage for the branch
picker action wiring and the git branch route validation paths.
* fix(web-shell): address review feedback for git branch picker (#7731)
---------
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
`isApproveOutcome` is documented as the single source of truth for "the
user said yes", shared by the CLI scheduler and the ACP Session so the two
cannot drift. It listed every proceed_* outcome except
`proceed_always_server` and `proceed_always_tool`.
Those two are deprecated but live. web-shell's ToolApproval sends them,
and `background-tasks.ts`, `telemetry/tool-call-decision.ts`, the ACP
Session and `agent.ts` all treat them as approvals. This function was the
only place that did not.
Both consumers use it for the same thing: in AUTO mode, when a call fell
back to a manual prompt because denialTracking was armed, an approval
clears the counters so later calls return to classifier flow. A user who
approved with the "always allow for this server" or "for this tool" button
did not clear them, so the session kept prompting after the user had
granted the broadest permission of all.
The test now enumerates the proceed_* values from the enum instead of
hand-listing them -- the hand-written list is what drifted, while claiming
to cover "every proceed_* outcome".
Quote tracking was gated on `substitutionDepth === 0`, so quotes inside a
`$(...)` body were invisible to the parser. A `)` inside those quotes then
closed the substitution early, and the body's own closing quote -- now
seen at depth 0 -- flipped the parser into "in quote" state, which
swallowed every separator to the end of the line.
splitCommands(`echo $(echo ')') ; rm -rf /tmp/pwned`)
-> ["echo $(echo ')') ; rm -rf /tmp/pwned"] one segment
getCommandRoots(same)
-> ["echo"] rm is gone
The trailing command did not merely stay joined to the first, it vanished
from the roots. shell.ts uses these segments to decide which sub-commands
need confirmation, and to locate the git commit / gh pr create segment for
attribution, which silently no-ops on a mis-split.
Gating the closing paren on the surrounding quote state is not enough,
because `"$(...)"` puts a double quote around the whole substitution and
that quote belongs to the outer command. Save the enclosing quote state on
`$(` and restore it on the matching `)`, so each body is quoted
independently of its surroundings.
splitCommands had no direct unit tests; this adds them, including the
nesting and quoting shapes that must keep behaving as they do.
* fix(core): short-circuit the flush wait when the output stream cannot flush
Review follow-up on the settle flush wait:
- A destroyed stream (autoDestroy after an earlier EIO/ENOSPC write
error) or an already-finished one emits neither 'finish' nor 'error',
and .end() on it is a silent no-op — the settle stalled for the full
10s timeout with nothing left to flush, and an abortAll() landing in
that window (/clear, shutdown) would mark the entry cancelled.
Short-circuit before arming the timer. writableFinished, not
writableEnded: the latter is already true mid-flush.
- Hoist clearTimeout into runOnce so the synchronous-throw path no
longer leaves the timer armed to log a misleading flush-timeout
warning for a shell that settled long ago.
- Drop the vacuous duplicate-'finish' assertion (once semantics already
drained the handler; the guard is pinned by the 'error' emit and the
timeout test's late finish) and cover both short-circuit states with
deferred-stream tests asserting the immediate transition and that
.end() is never called.
* test(core): pin the mid-flush wait and the throw-path timer disarm
Closes the two coverage gaps from the verification review on #7905 —
both mutations previously survived the full suite:
- guard mutated to include writableEnded (short-circuit mid-flush, the
exact truncation the flush wait prevents) — killed by a deferred
stream with { writableEnded: true, writableFinished: false }
asserting the transition still waits for 'finish';
- hoisted clearTimeout dropped from runOnce — killed by a throwing
end() plus fake timers asserting no flush-timeout warning fires
after the timeout elapses.
---------
Co-authored-by: ComplexSimply <rudy.arrowsong@gmail.com>
* fix(safe-mode): preserve caller-supplied top-tier MCP servers
Safe mode is meant to distrust LOCAL/ambient state (settings.json,
extensions, project .mcp.json) so a user can isolate which local
customization is misbehaving. It was also unconditionally dropping
topTierMcpServers -- the caller-supplied servers from an ACP
session/new's mcpServers field or --mcp-config -- which are an
explicit, per-invocation argument, not ambient local state.
The real gate turned out to be in packages/core/src/config/config.ts's
Config.getMcpServers() (the accessor mcp-client-manager.ts actually
reads for discovery), not just the mcpServers assembly in
loadCliConfig -- fixed both so the raw field and the accessor agree.
Also guarded the hot-reload path (hot-reload.ts) with the same
bare/safe-mode check, so a live settings.json edit can't smuggle local
servers into an already-running bare/safe-mode session.
Fixes#7819
* docs: clarify safe-mode's MCP servers note distinguishes local vs caller-supplied
Per CONTRIBUTING.md guideline 5 (docs for user-facing changes).
* fix(safe-mode): apply allowedMcpServers to top-tier servers, skip pendingMcpServers I/O under safe mode
Address Copilot's review of PR #7827:
- getMcpServers() now runs the safe-mode top-tier map through the same
allowedMcpServers filter as the non-safe-mode path -- safe mode is
not an exemption from a session's own --allowed-mcp-server-names
upper bound (High severity finding, confirmed real).
- Re-added safeMode to pendingMcpServers' skip condition in
loadCliConfig. Functionally a no-op either way (top-tier servers are
never gated, #4615), but skips getMcpApprovals' local file read
entirely under safe mode instead of doing a no-op read -- safe mode
shouldn't touch local/ambient state at all, not even harmlessly
(Medium severity finding).
* fix(safe-mode): actually run MCP discovery for surviving top-tier servers
Config.getMcpServers() reporting a top-tier server as configured isn't
enough on its own -- something has to actually connect to it and
register its tools with the model. That's a separate gate in
initialize(): startMcpDiscoveryInBackground() was skipped outright
whenever isSafeMode() was true, written back when getMcpServers()
always returned {} under safe mode (so skipping discovery was a
harmless no-op). Left unpatched after the earlier fix, a caller-supplied
top-tier server survives getMcpServers() but is never actually
discovered/connected -- confirmed live against a real ACP session
before this commit: the agent reported the tool as "not configured"
even though Config.getMcpServers() already returned it.
Found by actually running the fix end-to-end against a live ACP
session (qwen --acp --safe-mode + a real stdio MCP fixture server)
instead of relying on unit tests of Config in isolation. After this
commit the same live session correctly discovers and calls the
caller-supplied tool.
Checks getMcpServers() (not topTierMcpServers directly) so the
allowedMcpServers filter still applies -- no discovery is kicked off
if the only top-tier server present is filtered out.
* fix(safe-mode): also run MCP discovery for surviving bare-mode top-tier servers
The discovery-kickoff gate in Config.initialize() special-cased safe
mode (skip only when there's nothing to discover) but left bare mode's
half of the same guard unconditional (!this.getBareMode()), even
though loadCliConfig feeds top-tier MCP servers into bare mode's
mcpServers assembly exactly the way it does safe mode's
topTierMcpServers field. A bare-mode session with a caller-supplied
server (qwen --bare --mcp-config, or ACP session/new under bare mode)
had that server reported as configured by getMcpServers() but never
actually connected/discovered — the same stranded-server regression
already fixed for safe mode in this PR, just the bare-mode twin of it.
Found by an automated review pass on PR #7827 after the safe-mode fix
had already landed. Added the same three-case regression coverage
(present / nothing supplied / filtered out by allowedMcpServers)
mirroring the existing safe-mode tests, using mcpServers (not
topTierMcpServers) since bare mode's "local sources dropped" guarantee
lives entirely in the CLI-layer assembly, not a core-level short-circuit.
* refactor(safe-mode): simplify the bare/safe discovery-gate condition
(!bare || has) && (!safe || has) reduces by distributivity to
!(bare || safe) || has, so factor the has-servers check into a single
hasMcpServers computed once, instead of calling getMcpServers() (which
allocates and filters) twice.
Suggested by an automated review pass on PR #7827, commit 25ddf8b.
No behavior change — same 23 discovery-gate tests pass unmodified.
* fix(safe-mode): stop reading settings.mcp.allowed/excluded under safe mode
allowedMcpServers/excludedMcpServers assembly in loadCliConfig() only
guarded the settings-sourced branch with `!bareMode`, missing `!safeMode`
— so a local settings.json mcp.allowed/excluded list (LOCAL/ambient state,
same category as settings.mcpServers itself, which safe mode already
drops) was still read under safe mode. Combined with getMcpServers()'s own
allowedMcpServers filter (added earlier in this PR for the
--allowed-mcp-server-names case), a settings.json mcp.allowed list
narrower than the caller's own top-tier servers would silently filter
them back out — defeating the guarantee this PR exists to provide, via
the filter's source rather than the mcpServers map directly.
The argv.allowedMcpServerNames branch is unaffected: that's an explicit
per-invocation argument, not local state, so it still applies under safe
mode same as topTierMcpServers itself.
Found by an automated review pass (doudouOUC, CHANGES_REQUESTED) on PR
#7827. Regression test confirmed red before the fix (session-supplied
server silently filtered out) and green after. Full targeted vitest run
(packages/cli config/: 1018 passed, same 3 pre-existing Windows-only
extension-file-watcher failures as before, unrelated), tsc --noEmit,
eslint, prettier --check all clean.
* fix(safe-mode): stop reading settings.mcp.allowed/excluded on hot-reload too
Same class of bug as the previous commit's loadCliConfig fix, found by an
automated review pass on the SAME PR: recomputeMcpGating (hot-reload.ts)
reads settings.merged.mcp.allowed/excluded unconditionally, with no
bare/safe guard of its own. registerMcpHotReload's existing bare/safe
guard only covered the servers map (`next`), not the admission lists
computed right after it — so a live settings.json edit narrowing
mcp.allowed during an already-running safe/bare session would flow
straight into setAllowedMcpServers and silently filter the caller's
top-tier server out of getMcpServers() mid-session. Same stranded-server
outcome as the boot-time bug, reached through the gating list's SOURCE
instead of the mcpServers map.
Fix: under bare/safe mode, skip recomputeMcpGating entirely and build the
gating directly from only the CLI --allowed-mcp-server-names bound
(explicit, per-invocation, not local state — same treatment as
topTierMcpServers itself); excluded/pending are irrelevant once nothing
but the never-gated top-tier servers can be present.
Regression tests (safe mode + bare mode) confirmed red before the fix
(setAllowedMcpServers called with the settings-sourced list) and green
after (called with the CLI bound, undefined here). Full targeted vitest
run (packages/cli config/: 1020 passed, same 3 pre-existing Windows-only
extension-file-watcher failures as before this PR touched anything,
unrelated), tsc --noEmit, eslint, prettier --check all clean.
* fix(safe-mode): stop reading settings.mcp.allowed/excluded on ACP reload too
Third instance of the same bug class found by an automated review pass on
this PR: reloadWorkspaceMcpDiscovery (packages/cli/src/acp-integration/
acpAgent.ts) — the ACP control-endpoint reload path (workspaceMcpReload),
distinct from registerMcpHotReload's settings-file-watcher path fixed in
the previous commit — called assembleMcpServers(settings.merged.mcpServers,
...) and recomputeMcpGating(settings, ...) unconditionally, per live Config
in liveConfigs, with no bare/safe guard. A workspaceMcpReload request
against an already-running safe/bare session would fold local
mcpServers/mcp.allowed/excluded back in, silently stranding or filtering
the caller's own top-tier server mid-session — same outcome as the two
prior fixes, reached through a third independent reload path.
Fix: per-config (liveConfigs holds a Set of potentially differently-moded
Configs — the base config, active session configs, and the discovery
config), skip assembleMcpServers/recomputeMcpGating under bare/safe mode
and build servers/gating directly from that config's own
getTopTierMcpServers()/getCliAllowedMcpServerNames() — same treatment as
the other two fixes.
Regression test (packages/cli/src/acp-integration/acpAgent.test.ts)
confirmed red before the fix (settings-sourced 'local' server leaked
into reinitializeMcpServers alongside the caller's 'probe') and green
after. Also added getBareMode/isSafeMode mocks (defaulting false) to the
two pre-existing Config-shaped mocks in this describe block that didn't
have them — reloadWorkspaceMcpDiscovery now calls these unconditionally
per config, which would otherwise throw "not a function" against any
mock missing them, even for a normal-mode test. Full targeted vitest run
(packages/cli/src/acp-integration/acpAgent.test.ts: 318 passed), tsc
--noEmit, eslint, prettier --check all clean.
* test(safe-mode): add bare-mode counterpart for the workspaceMcpReload guard
Suggested by an automated review pass on PR #7827: the previous commit's
regression test for reloadWorkspaceMcpDiscovery only exercised
isSafeMode: true, leaving the bare-mode half of
config.getBareMode() || config.isSafeMode() unverified at this layer —
unlike the hot-reload.ts tests, which already cover both modes for both
the servers map and the admission lists. A future change narrowing that
guard to isSafeMode() only would go undetected here.
Confirmed red before the fix (temporarily reverted acpAgent.ts to the
prior commit): settings-sourced 'local' leaked into reinitializeMcpServers
alongside the caller's 'probe', same as the safe-mode case. Green with
the fix restored. Full targeted vitest run (acp-integration/acpAgent.test.ts:
319 passed), tsc --noEmit, eslint, prettier --check all clean.
The Anthropic request previously used 3 of the 4 allowed cache_control
breakpoints: last tool, end of system, last user message. Because the
system prompt ends with volatile tails (git status, auto-memory), the
scope:'global' entry on the system end almost never hit across sessions
— only the tools breakpoint did.
Split the outgoing system prompt at the stable → context / volatile
layer boundary that GeminiClient already assembles: the client records
the gitStatus/autoMemory-free prefix on Config, and the converter emits
it as its own text block with an early breakpoint (scope:'global' when
enabled), demoting the system-end breakpoint to the per-session shape.
New sessions and in-session memory saves now re-bill only the small
volatile tail instead of the whole system prompt. The converter matches
the prefix via startsWith and fails open to the previous single-block
layout (subagent prompts, stale prefix), so the wire shape never
regresses. This fills all 4 breakpoints.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(core): retry on transient network errors in defaultShouldRetry and API-call predicate
The retry classification layer (classifyRetryError) already recognizes
transport errors (ECONNRESET, ETIMEDOUT, etc.) as retryable, but neither
defaultShouldRetry nor the inline shouldRetryOnError in geminiChat.ts
consulted the classification for transport codes. TCP-level errors carry
no HTTP status, so they fell through every predicate and propagated as
raw [API Error: terminated (cause: read ECONNRESET)].
Wire the existing classifyRetryError transport detection into both
retry decision points:
1. defaultShouldRetry in retry.ts — append a transport-kind check after
the existing rate-limit / 5xx predicates, preserving all current
behavior (including bounded retry for fail-fast quota 429s).
2. Inline shouldRetryOnError in geminiChat.ts makeApiCallAndProcessStream —
add the same transport-kind check so the API-call-level retryWithBackoff
wrapper covers network errors.
The stream-level transport retry (gated on !streamYieldedChunk) already
exists and is unchanged.
Closes#7831
* fix(core): classify SDK-wrapped transport errors nested in the cause chain (#7898)
---------
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
`Part.thought` is a boolean flag and the reasoning lives in `part.text`.
`partToString` declared it as `thought?: string` in a local cast and
interpolated the flag, so every thought part rendered verbose as the
literal `[Thought: true]` and the reasoning was dropped.
The local cast was the only thing making that compile -- the SDK types it
as `thought?: boolean`, `createOpenAIReasoningThoughtPart` builds
`{ text, thought: true }`, and every other consumer reads the text and
tests the flag for truthiness. Drop the cast and follow the same shape.
Testing `!== undefined` also caught a part carrying `thought: false`,
which is an ordinary part: it rendered as `[Thought: false]` rather than
as its own text. Use truthiness, matching the rest of the codebase.
Two existing tests asserted the old output and are corrected. Both were
built from shapes the SDK never emits: `{ thought: 'thinking' }`, which
only type-checks through `as unknown as Part`, and a bare `{ thought:
true }` expected to print its own flag.
* fix(core): apply maxDepth to flat-format memory imports
* fix(core): track import depth per file so a shallower route can re-expand
* docs(core): trim the historical narrative from the depth-guard comment
Per AGENTS.md comments default to none and exist to explain a non-obvious
why. The first paragraph does that -- it states which semantics the guard
matches. The account of what the old code did wrong belongs in the commit
message, where it already is, not inline where it can go stale.
* fix(core): name the file in the depth-limit warning, correct a stale comment
The summary comment still said the flat path deduplicates with a set; it
has used a depth-keyed map since the previous commit, and the whole point
of that map is that a shallower route may re-expand a file a set would
have dismissed.
memoryDiscovery processes memory files concurrently, so several chains can
hit the limit at once and every warning read identically. normalizedPath
is already in scope.
* fix(core): announce a re-expanded flat import only on its first visit
Letting a shallower route re-expand a file made onFileImported fire twice for
it: once from the truncated deep route and again from the shallow one, with a
different parentFilePath. The Set this replaced skipped the child outright, so
the callback ran once per file. Nothing downstream de-duplicates --
notifyInstructionsLoaded in memoryDiscovery forwards every call to the
consumer -- so one loaded file was reported as two.
The file itself is still emitted into the flat output once, so gate the
notification on the first visit to match. The test is the re-expansion fixture
with a callback attached: x.md was announced twice before this commit.
* fix(core): budget flat imports from the depth already spent
The flat path started its own counter at 0 and ignored importState.currentDepth,
so a caller entering with budget already spent got the whole limit over again
in flat mode while tree mode gave it only what was left. Now that flat enforces
the limit at all, the two counters have to share an origin.
Every caller today enters flat mode at depth 0 -- memoryDiscovery passes 0, and
the internal recursion at the tree branch is unreachable from flat -- so this is
a no-op in practice; it removes the discrepancy rather than any live symptom.
Test enters at currentDepth 2 of maxDepth 3 and asserts both formats stop at the
same file. Before this commit flat expanded [0,1,2] where tree gave [0].
Step 3 rewrites a numeric exclusiveMinimum/exclusiveMaximum into the
Draft 4 boolean form, but step 6 then skipped both keys unconditionally.
A schema that already used the boolean form lost the flag, silently
relaxing "> 10" into ">= 10", and the function stopped being idempotent
- feeding its own output back in dropped the flag it had just written.
Skip the key only when the value is a number, the one form step 3
consumes. MCP servers declaring draft-04 send the boolean form directly,
so this reached the wire through the OpenAI and Anthropic converters.
* fix(core): read the stash reflog from the common git dir
* fix(core): harden the commondir read the same way the HEAD read is
`resolveCommonGitDir` reads `<gitDir>/commondir` with a plain `readFile`,
while `gitDirect.ts` reads that same file through `readFirstLineNoFollow`,
which opens with `O_NOFOLLOW | O_NONBLOCK` and bounds the read. The two
reads had different protections against the same hostile input.
That matters because `getGitWorkingTreeStatus` polls unattended: a
`commondir` named pipe planted in a cloned repository would hang the
`readFile` forever and pin a libuv thread-pool slot, and a symlink would
be followed out of the repository. The adjacent `countStashEntries`
already guards against exactly this hazard for the reflog it reads.
Move `readFirstLineNoFollow` to `gitUtils.ts` and point both callers at
it. It cannot be exported from `gitDirect.ts` because `gitDirect.ts`
already imports `gitDiff.ts` — that would close an import cycle.
`gitUtils.ts` imports nothing but node builtins, so it can be shared by
both.
`previewChars` is meant to bound the preview that goes into the model's
context, but two fixed strings were emitted outside it.
The separator is 39 characters and was appended whatever the budget was,
while the head budget never subtracted it -- only the tail budget did. And
when fewer than three characters remained, the 3-character ellipsis marking
a cut line was still emitted. Together:
previewChars=0 -> 39 characters
previewChars=10 -> 42
previewChars=40 -> 47
Take the separator out of the budget before splitting head and tail, drop
it entirely when the budget cannot fit it alongside any content (the
wrapper message above already says the output was truncated), and skip the
ellipsis when there is no room for it either.
Reachable through truncateToolOutput's per-tool limits.previewChars.
Sweeping previewChars from 0 to 260 across all three keep directions now
reports nothing over budget; before, every value below ~47 was.
`getErrorMessage` truncates to MAX_STRINGIFIED_ERROR_MESSAGE_LENGTH on
every branch except one: an `instanceof Error` with no distinct cause
returned `error.message` untouched.
That made the cap depend on details unrelated to length. The same 5000
character string comes back at 1000 as `{ message }`, at 1000 once a
distinct `cause` is attached, at 1000 through the JSON.stringify path --
and at 5000 as a bare `Error`. The uncapped shape is exactly the one a
provider SDK throws when it packs a whole response body into the message,
and the result flows into `llmContent` at dozens of call sites, which is
what the cap exists to prevent.
Both external search strategies pass the user's pattern as a positional
argument, so a pattern that begins with a dash is parsed as an option.
`git grep --untracked -n -z -E --ignore-case '-n'` consumes the `-n` as a
flag and exits with `fatal: no pattern given`. That failure is caught and
falls through to the system grep, which has the same flaw with a much
quieter symptom: `grep -r -n -H -E --null '-n' .` treats `-n` as a flag
and promotes the `.` search path to be the pattern, so it matches every
line of every file and hands the whole tree back as if it were the result.
`validateToolParams` accepts these patterns -- `new RegExp('-n')` is a
valid regex -- so nothing upstream rejects them.
Introduce the pattern with `-e` in both strategies. `-e` is POSIX, and the
search path operand stays the literal `.`, so no `--` separator is needed.
The sibling ripgrep tool already does this via `--regexp`.
isSafeCombinedFlagArg documents that only forms with i last are safe,
because sed treats everything after -i in a combined flag as the backup
suffix. The check tested flags.startsWith('i'), which rejects i first
rather than i anywhere but last, so -Eir and -riE were accepted.
Real sed reads those as -E plus in-place with the backup suffix r / E and
writes an f.txtr or f.txtE alongside the edit. The parser reported them
safe, shell.ts intercepted the command, and the simulation wrote the file
without ever creating the backup the user asked for - the same hazard
-i.bak is already rejected for.
Test the documented rule instead. A declined command falls through to
real sed, so the cost is a lost fast path rather than a lost backup.
