* fix(anthropic): don't strip a trailing tool_use with no subsequent message
Fixes#8159
cleanOrphanedToolCalls treated any assistant tool_use with no matching
tool_result as an orphan and stripped it -- including a tool_use in the
very last message, where there is no subsequent message to have found a
result in yet. "No result yet" is not the same as "no result ever": the
tool may simply not have finished executing, or the conversion may be
happening for a reason other than sending the completed turn to
Anthropic (token counting, a resumed/replayed session snapshot, a retry
issued before tool execution completes, ...).
Silently deleting a currently-active tool_use corrupts the assistant's
most recent turn, and the damage compounds when that turn also carries a
signed extended-thinking block: the block's signature is computed over
the full sibling content of the turn, so removing the tool_use next to
it invalidates the signature and replaying the mutated turn produces
Anthropic 400 "thinking blocks in the latest assistant message cannot be
modified" -- a genuinely confusing error for something the client did to
its own outgoing request.
Fix: when an assistant tool_use is in the last message of the array (no
message follows it at all), treat its tool_use blocks as valid/unresolved
rather than scanning for a match that can't exist yet. A tool_use is only
condemned as orphaned when a subsequent message was actually scanned and
found to lack a matching tool_result.
Also fixed one existing test that unintentionally pinned the buggy
behavior as expected output ("cleans orphaned tool_use blocks without
matching tool_result" used a fixture with no subsequent message at all,
which is the trailing case, not a genuine orphan) -- added a real next
message with unrelated content so it now exercises an actual orphan.
Added a new regression test for the trailing case.
Verification:
- New/updated unit tests in converter.test.ts (73 tests, was 72).
- Full anthropicContentGenerator/ suite: 193 tests pass.
- tsc --noEmit -p packages/core/tsconfig.json and eslint clean for
touched files.
* fix(anthropic): sanitize tool_use.id / tool_result.tool_use_id
Fixes#8160
The Anthropic converter passed Part.functionCall.id / FunctionResponse.id
straight through as tool_use.id / tool_result.tool_use_id with only an
empty-string-style fallback, never validating or sanitizing against
Anthropic's accepted character set.
Anthropic validates both fields against ^[a-zA-Z0-9_-]+$ server-side and
rejects anything else -- including the empty string, since `+` requires
at least one character. functionCall.id is the Gemini lingua-franca's own
id field with no such constraint, so it can legitimately carry another
provider's id scheme, a composite/namespaced id, or be entirely absent.
Live-verified against the real Anthropic Messages API (via our corporate
LiteLLM proxy in front of Vertex, model claude-sonnet-4-6):
- A tool_use.id containing characters outside [a-zA-Z0-9_-] (e.g.
`call:abc.def/ghi?jkl`) 400s: "String should match pattern
'^[a-zA-Z0-9_-]+$'".
- An empty-string tool_result.tool_use_id (the exact fallback the old
code emitted) 400s the same way.
- The sanitized replacement (`call_abc_def_ghi_jkl`) round-trips
successfully (HTTP 200) with the pair still linked.
Fix: resolveToolUseId() sanitizes non-conforming characters to `_`,
generates a real fallback id (never an empty string) when the source is
missing, and memoizes the source id -> sanitized id mapping per request
(toolIdMap) so a tool_use/tool_result pair sharing a source id still
resolves to the same wire id after sanitization. Uniqueness is enforced
across the whole request (usedToolIds) rather than the previous
per-Content-object counter, which fixes a latent collision: two
different Content objects each missing an id previously both started
their local counter at 0, producing duplicate fallback ids like `tool_0`
in two different assistant turns.
State is scoped to a single convertGeminiRequestToAnthropic call (reset
via resetToolIdState) since the converter instance is long-lived across
requests (constructed once per generator, not once per call).
Also merges in the trailing-tool_use fix from #8163 (fixes#8159): one
of the new tests here needs the model's missing-id tool_use to survive
as the final message in order to observe the generated fallback id,
which depends on that fix. This PR's net diff will shrink to just the
sanitization change once #8163 merges.
Verification:
- New tests in converter.test.ts covering: bad-char sanitization with
linkage preserved, non-empty fallback generation (not empty string),
no id collisions across two missing-id calls in one request, and
same-source-id consistency across tool_use/tool_result in different
messages.
- Full anthropicContentGenerator/ suite: 197 tests pass.
- Live proxy verification as described above.
- tsc --noEmit -p packages/core/tsconfig.json and eslint clean for
touched files.
---------
Co-authored-by: Palanisamy, Dinesh <Dinesh.Palanisamy@netapp.com>
Fixes#8161
When a Gemini Content for a user turn contains both a functionResponse
and other parts (e.g. accompanying text), the converter emitted the
resulting content blocks in whatever order the source parts happened to
arrive in -- it never guaranteed tool_result blocks come first. If a
text part preceded the functionResponse part within the same Content, the
resulting Anthropic user message put text before tool_result.
Live-verified against the real Anthropic Messages API: sending a user
message shaped [{type:'text', ...}, {type:'tool_result', ...}] right
after an assistant tool_use produces HTTP 400: "`tool_use` ids were found
without `tool_result` blocks immediately after: <id>. Each `tool_use`
block must have a corresponding `tool_result` block in the next
message." Anthropic doesn't scan past a leading non-tool_result block to
find the result later in the same message -- it must be first.
Worse: cleanOrphanedToolCalls's own "seenNonToolResult" gate (a separate,
pre-existing defensive check against exactly this ordering rule) reacts
to the misordering by treating the tool_result as if it were never found
at all, silently discarding BOTH the tool_result and its paired tool_use
rather than fixing the order. So the previous behavior wasn't even a
consistent 400 -- it was silent data loss of the tool call and its
result.
Fix: when a user-role message contains any tool_result blocks, a stable
sort moves them ahead of any other content in that message, preserving
the relative order of multiple tool_result blocks against each other.
This runs before cleanOrphanedToolCalls, so its ordering gate now
recognizes the pairing correctly instead of discarding it.
Verification:
- Rewrote the existing test that pinned the silent-discard behavior
("drops tool results that do not lead user content") to assert the
correct reorder-and-preserve behavior instead.
- Added a new test for relative-order preservation across multiple
tool_result blocks being reordered together.
- Full anthropicContentGenerator/ suite: 193 tests pass (was 192; net +1
test).
- Live proxy verification: the previously-400ing wire shape now returns
HTTP 200 with the reordered body.
- tsc --noEmit -p packages/core/tsconfig.json and eslint clean for
touched files.
Co-authored-by: Palanisamy, Dinesh <Dinesh.Palanisamy@netapp.com>
* fix(core): strip assistant-turn prefill and fix thinking.display on 4.6+ models
Two related bugs affecting every Claude Opus/Sonnet 4.6+ model and
every 5.x family (Fable 5, Mythos 5, Sonnet 5, etc.) on the Anthropic
wire, both verified live against the Anthropic Messages API.
**Assistant-turn prefill 400s with no mitigation.** When Gemini-format
history ends on a model turn with no follow-up (context trimming drops
the next user turn, a subagent transcript is replayed mid-turn), the
converter forwards a request whose last message has role: 'assistant'.
Anthropic rejects this on every 4.6+ model:
"This model does not support assistant message prefill. The
conversation must end with a user message."
Adds `stripTrailingAssistantPrefill` as a converter option, gated in
the generator on the existing `modelSupportsAdaptiveThinking()` check
(the same 4.6+ cutoff already used for the adaptive-thinking shape —
reused rather than duplicated). Two cases:
- Trailing assistant message is empty/whitespace-only (a leftover
prefill artifact) — drop it.
- Trailing assistant message carries real content (text, tool_use,
thinking) — keep it in history and append a synthetic user turn
("Continue.") instead of discarding anything the model already
said.
Live-verified: claude-opus-4.6/4.7/4.8, claude-sonnet-4.6, and
claude-sonnet-5 all 400 identically on a trailing-assistant request;
the identical request against claude-haiku-4.5 (a genuinely
pre-4.6-generation model, confirmed not a proxy alias) succeeds
normally, confirming the gate correctly discriminates on model
generation rather than being a universal restriction.
**thinking.display silently defaults to 'omitted' on newer models.**
Sonnet 4.6 defaults adaptive thinking's `display` field to
'summarized'; Opus 4.7+ and every 5.x family (including Sonnet 5)
silently changed the default to 'omitted' -- thinking blocks stream
back with empty text, no error, just a long pause before output where
the same code previously showed reasoning.
Sets `display: 'summarized'` explicitly on the `{ type: 'adaptive' }`
thinking shape only (the pre-4.6 budget_tokens shape and the explicit
reasoning.budget_tokens escape hatch are untouched -- Anthropic's docs
scope the default-changed problem to adaptive thinking specifically).
No-op on models whose default is already 'summarized'; required on
models whose default silently changed.
Live-verified: a task requiring genuine reasoning against
claude-sonnet-5 returns empty `thinking` text without `display` set,
and full non-empty reasoning text with `display: 'summarized'` set,
confirmed by direct comparison of both request variants against the
live API through the wire-capture layer.
Fixes#8039
Updated 10 existing test assertions in anthropicContentGenerator.test.ts
that pinned the old `{ type: 'adaptive' }` shape (now
`{ type: 'adaptive', display: 'summarized' }`), added 5 new
converter.test.ts cases for the prefill-stripping behavior (drop-empty,
append-synthetic-turn, leave-user-message-untouched, option-unset
no-op, thinking-only trailing message), and 1 new
anthropicContentGenerator.test.ts case pinning that `display` is never
set on the budget_tokens shape.
* test(anthropic): fix vacuous drop-empty fixture, add generator-level prefill wiring coverage
Addresses maintainer review feedback on this PR:
- converter.test.ts: "drops a trailing empty assistant message" used
`{ text: '' }`, but processContent only emits a text block when
part.text is truthy — an empty string never produces an assistant
message at all, so the fixture reached nothing and the assertion
passed identically with stripTrailingAssistantPrefill off. Changed
to whitespace-only (`' '`), which does produce a message and
actually exercises isEmptyAssistantMessage's `.trim()` check.
Verified this kills the two mutants the maintainer identified
(M1, M6).
- anthropicContentGenerator.ts: `stripTrailingAssistantPrefill` (the
generator-level constructor of the trailing-turn behavior) had no
test pinning that the generator actually reads
modelSupportsAdaptiveThinking() to turn it on/off. Added two tests:
the trailing turn is stripped + replaced with the synthetic
"Continue." user turn on claude-opus-4-6, and left untouched on
claude-opus-4-5. Verified these kill the two mutants the maintainer
identified (hardcoding the gate to `false` or `true`).
Both fixes verified against the mutation matrix by hand: reverted each
mutant, confirmed the new/fixed test fails, restored the source,
confirmed the suite passes again.
---------
Co-authored-by: Palanisamy, Dinesh <Dinesh.Palanisamy@netapp.com>
* fix(release): keep notes anchored and cap the release body
The v0.21.2 publish failed at "Create GitHub Release and Tag" with
HTTP 422 "body is too long (maximum is 125000 characters)", after every
npm package had already been published.
Stable releases are tagged on their own release/* branch and merged back
to main only afterwards, so the previous stable tag is never an ancestor
of the branch being released. The ancestor guard therefore dropped
--notes-start-tag on every stable release, and without an anchor GitHub
generates notes across the entire branch history (8000+ commits), which
overruns the body limit.
Always pass the previous tag instead: GitHub diffs it through the merge
base, which is how v0.21.1 produced a 27KB body from a tag that was
equally divergent. Generate the body through the generate-notes API
first so an oversized changelog is truncated on a UTF-8 boundary, and
degrade to an unanchored body and then a minimal one, rather than
aborting a release whose packages are already on npm.
* test(release): pin the anchored release-notes contract
The workflow test asserted the ancestor guard that dropped
--notes-start-tag on every stable release. Assert the replacement
instead: the previous tag is always passed to generate-notes, the body
is capped, and ancestry no longer decides whether notes are anchored.
* refactor(release): extract release-notes capping into a tested helper
The degradation chain lived inline in the workflow bash, so nothing
pinned that a capped body plus its footer stays under GitHub's 125000
character limit, that truncation never splits a multi-byte character, or
that the chain always yields a non-empty body. Move it to
.github/scripts/cap-release-notes.mjs with a collocated node:test suite,
matching the other workflow helpers.
Capping on code points rather than bytes drops the head/iconv dance and
makes the surrogate-pair case testable. The helper also absorbs the
empty-body fallback, which caught a real defect: gh writes the API error
payload to stdout when generate-notes fails, so a doubly failed call
would have published `{"message":"Not Found",...}` as the release body.
Discard a failed attempt's output instead.
* test(release): exercise the surrogate-pair cut and footer-overflow branch (#8199)
---------
Co-authored-by: Qwen Code Bot <qwen-code-bot@alibabacloud.com>
SubAgent type identifiers (general-purpose, Explore, etc.) were rendered
verbatim in the UI. Add agentType.* i18n keys and a localizeAgentTypeName()
helper so Chinese users see 通用/探索/状态栏设置 instead of raw English
identifiers. User-defined agent names still display as-is.
`packages/cli/src/cli.ts` is the esbuild entry point and bootstraps only under a
main-module guard:
if (process.argv[1] !== undefined &&
import.meta.url === pathToFileURL(process.argv[1]).href) {
void runCliEntryPoint();
}
The bundle is built with `splitting: true`. If any module the entry loads lazily
(e.g. `gemini.tsx`, reached through `await import('./gemini.js')`) adds a static
`import ... from './cli.js'`, esbuild moves the entry module's body into a shared
chunk and leaves `dist/cli.js` as a re-export stub. Inside a chunk
`import.meta.url` is the chunk's own URL, so the guard never matches and the
bundled CLI exits 0 without running anything.
Nothing catches that today: tsc, eslint and every src-based unit test stay green,
because the breakage only exists in the bundle. The single CI step that executes
`dist/cli.js` is the no-AK integration smoke test, which reports it as
`daemon exited with 0 before listening` from three unrelated serve suites — a
symptom that points nowhere near the import that caused it.
Assert instead that the entry output still compiles the entry module. When the
entry is hoisted, `dist/cli.js` keeps no inputs of its own, so the metafile the
existing closure checks already read is a precise signal, and the diagnostic can
name both the cause and the fix.
Co-authored-by: verify <verify@local>
* fix(core): keep compactString within its limit when the marker does not fit
The compaction marker embeds the original length, so it runs 60-80
characters on its own. `contentBudget` was clamped to 0 when the limit was
smaller than that, but the marker was then appended regardless:
compactStringForRecording('x'.repeat(70), 60) -> 79 chars
compactStringForHistory('x'.repeat(64), 63) -> 64 chars
compactStringForHistory('x'.repeat(100), 50) -> 65 chars
The first returns more characters than the input it was asked to compact,
which is the opposite of what a compaction function is for. Both entry
points are exported and take a caller-supplied limit.
When the marker cannot fit alongside any content there is nothing useful to
announce, so hard-truncate the head instead of explaining the truncation at
greater length than the truncated text. The marker is unchanged wherever the
limit leaves room for it.
* test(core): fix the purpose ternary and cover surrogates on the guard path
Two review findings on the compaction tests.
The `it.each` over both purposes called `compactStringForRecording` in both
branches, so the two `history` rows silently re-ran the recording case. The
history marker is 65 characters against recording's 79, so the two purposes
take different content budgets and the history budget was never exercised.
The `marker.length >= limit` guard also had no surrogate coverage: it slices
without a marker, so it has a boundary of its own, and both existing
surrogate-aware tests run at the default limit and take the head+marker+tail
path instead. Added four rows over both purposes at limits 9 and 8. Replacing
`safeHeadEnd(value, limit)` with a bare `limit` fails the two odd-limit rows
and leaves the even-limit controls green, so the coverage is load-bearing.
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* fix(cli): prevent silent VP-mode crash by adding uncaughtException handler and error visibility
VP (alternate-screen) mode swallows all error output: uncaught exceptions
write their stack trace to stderr which lands on the alternate screen
buffer, then gets discarded when teardown switches back to the primary
buffer. The user sees a silent exit with no error message and nothing in
the debug log.
Root cause: no `uncaughtException` handler existed anywhere in the CLI.
PR #7406's ErrorBoundary and unhandledRejection handler only cover React
render errors and promise rejections — synchronous exceptions bypass both.
Changes:
- Add `setupUncaughtExceptionHandler()` with sync debug-log write,
alternate-screen exit before stderr output, and clean process.exit(1)
- Add `consumeLastRenderError()` to ErrorBoundary for VP main-screen echo
after unmount leaves the alternate screen
- Remove SIGTERM/SIGINT handlers from kittyProtocolDetector.ts that
raced with the main signal handlers (#7779)
- Add SIGHUP handler alongside SIGTERM/SIGINT (#7781)
- Guard ThinkBody's MarkdownDisplay with per-item ErrorBoundary so
partial markdown during thought streaming degrades to plain text
Related: #7971#7972#7779#7781
* fix(cli): consolidate uncaughtException handler into a single listener (#8088)
The VP-crash handler added a second uncaughtException listener that conflicted with the pre-existing one in runCliEntryPoint: the first listener's process.exit(1) ran before the second, leaving the visibility feature inert for real errors, and the new listener lacked the PTY-race guard, crashing the session on benign teardown errors. Replace the startup handler with one session-aware listener (PTY guard, isTTY-guarded alternate-screen leave, writeStderrLineSafe), sanitize the inline render-error fallback, gate the exit-time render-error echo on onError, and cover SIGHUP exit code.
* fix(cli): harden uncaughtException handler and scope render-error echo (#8088)
* fix(cli): address review feedback on VP crash handler (#8088)
- Close over Config instead of sessionId string so the uncaughtException
handler reads the current session ID at crash time
- Create debug log directory before appending so the write succeeds on
fresh installs where ~/.qwen/debug/ does not yet exist
- Sanitize error stack with sanitizeTerminalText before writing to stderr
to prevent ANSI injection on the persistent main buffer
- Add onError debug logging to the ThinkBody ErrorBoundary
- Gate the '(logged to debug file)' hint on isDebugLogFileEnabled() so
the message is accurate when debug file logging is disabled
* fix(cli): move uncaught-exception helpers to a leaf module (#8088)
gemini.tsx statically imported handleUncaughtException and
isExpectedPtyRaceError from cli.ts, the esbuild entry point. gemini.tsx
is only ever loaded lazily, so that import formed an entry<->lazy cycle;
with splitting enabled esbuild hoisted the entry body into a shared chunk
and left dist/cli.js as a re-export stub. The bootstrap guard at the
bottom of cli.ts then never ran and the bundled CLI exited 0 silently.
Move the helpers (and the private getErrnoCode) into a new leaf module,
utils/uncaught-exception-handler.ts, imported by both cli.ts and
gemini.tsx. cli.ts re-exports them so existing importers (cli.test.ts)
are unaffected, and the handler reuses the shared writeStderrLine from
stdioHelpers.ts. The uncaughtException behavior itself is unchanged.
---------
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* fix(cli): add Ctrl+Tab alternative for @ completion tab switching (#8069)
Ctrl+←/→ is intercepted by most terminals (iTerm2, Windows Terminal,
macOS Terminal.app) for word-jump, making @ completion tab switching
unusable. Added Ctrl+Tab (next) and Ctrl+Shift+Tab (previous) as
alternative keybindings that are less commonly intercepted.
Also fixed the hint text: "(Ctrl+←/→ to switch)" was displayed even
when availableCategories <= 2 (where switching is disabled). Now the
hint only shows when there are >2 categories and mentions Ctrl+Tab
as an alternative.
* fix(cli): keep Ctrl+Shift+Tab from cycling approval mode (#8069)
Ctrl+Shift+Tab was added as a completion-category navigation binding, but the approval-mode cycling handlers matched any Shift+Tab without excluding ctrl. In Kitty-protocol terminals (which preserve the ctrl modifier on Shift+Tab) the navigation keystroke also silently cycled the approval mode. Exclude ctrl from the Shift+Tab match and mention both Ctrl+Tab directions in the switching hint.
* test(cli): cover Ctrl+Shift+Tab guard in AgentComposer (#8074)
* refactor(cli): drop redundant category-count guard in SuggestionsDisplay (#8074)
* test(cli): cover Ctrl+Tab completion tab-switch bindings (#8074)
---------
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>
* feat(anthropic): support extended 1h cache_control TTL
Adds `generationConfig.cacheRetention` / `cacheRetentionByBlock` settings
that let the Anthropic-compatible content generator request the 1-hour
extended prompt-cache tier (`cache_control: { type: 'ephemeral',
ttl: '1h' }`) instead of always relying on the spec's 5-minute default.
- `cacheRetention` sets the default retention for every cache anchor the
converter writes (system text, last tool definition, trailing user
message); `cacheRetentionByBlock` allows overriding per anchor.
- No model allowlist: live verification against the Anthropic Messages
API confirmed every currently-active model (Haiku 4.5 through Opus 4.8
and Sonnet 5) accepts the 1h tier, so a model that ever doesn't will
simply 400 directly rather than being silently masked by a stale
allowlist.
- The `extended-cache-ttl-2025-04-11` beta header is sent defensively
whenever the assembled request body actually carries `ttl: '1h'`
(mirrors the existing `hasGlobalCacheScopeOnWire` body-scan pattern),
even though live testing found it has no observable effect on at least
one Anthropic-compatible backend -- some backends may still enforce it.
Verification:
- New unit tests: 5 in converter.test.ts (ephemeral default omits ttl,
'1h' sets ttl on system/tool/trailing-user, composes with
scope:'global', per-anchor override wins, staticSystemPrefix suffix
carries ttl) and 2 in anthropicContentGenerator.test.ts (beta header
sent/omitted based on wire content).
- Live end-to-end verification: drove the real AnthropicContentGenerator
through a logging proxy and confirmed the outbound wire body carries
`cache_control: { type: 'ephemeral', ttl: '1h' }` and the real
Anthropic API response reports `ephemeral_1h_input_tokens` (not
`ephemeral_5m_input_tokens`).
Fixes#8047
* fix(anthropic): normalize cacheRetentionByBlock to keep wire order legal
Addresses the blocking review comment on this PR: cacheRetentionByBlock
could emit request bodies that violate Anthropic's documented ordering
rule ("cache entries with longer TTL must appear before shorter TTLs").
Render order is tools -> system -> messages, so the converter's three
cache anchors (tool, system, user.last) sit on the wire in exactly that
order; a per-anchor override that raises a later anchor to '1h' while an
earlier one stays at the 5-minute default inverts the required ordering.
4 of the 8 reachable retention combinations were invalid, including the
PR's own new test case (cacheRetentionByBlock: { system: '1h' }) and the
most natural usage ("cache my system prompt for an hour").
Applies resolution option 1 from the review: normalize instead of
validate. resolveCacheRetention now resolves each anchor by scanning
itself and every anchor later on the wire — if any of them is '1h', the
anchor resolves to '1h' too. This makes every cacheRetentionByBlock
configuration legal (retention is monotonically non-increasing in wire
order) without adding a new error surface or rejecting any input.
Also folds in the review's medium/minor findings while touching this
code:
- settingsSchema.ts: cacheRetention is now `type: 'enum'` with an
ephemeral/1h option list (previously unconstrained `type: 'string'`),
and cacheRetentionByBlock gets a jsonSchemaOverride enumerating the
three anchor keys with additionalProperties: false — a typo'd value
(e.g. '1hr', 'user_last') or key no longer silently degrades to 5m
caching with nothing on the wire to contradict it. Regenerated
settings.schema.json via `npm run generate:settings-schema`.
- cacheRetention's settings description now states the 1h tier's 2x
write-cost premium (vs 1.25x for the 5m default; reads stay 0.1x for
both) so the tradeoff the setting exists to expose is visible where
it's configured.
- Moved resolveCacheRetention above buildSystemWithCacheControl's doc
comment — it had been inserted between that comment and the method it
documents.
- Corrected the stale AnthropicCacheControl comment claiming ttl is
"gated behind" the extended-cache-ttl-2025-04-11 beta; Anthropic's
current docs describe the 1h tier as GA, and the beta is sent
defensively for older Anthropic-compatible backends that may still
gate on it.
Verification:
- Added 2 unit tests covering the two review-flagged case shapes: { tool:
'1h' } alone (no promotion needed, was already legal) and { 'user.last':
'1h' } alone (now promotes both tool and system). Updated the existing
"per-anchor override" test to assert the tool anchor's new promoted
value instead of the old (invalid) unpromoted one.
- Re-ran the reviewer's exact reproduction case live: drove the real
AnthropicContentConverter with cacheRetentionByBlock: { system: '1h' }
and a tool attached, confirmed the assembled body now resolves to
tool=1h/system=1h/user.last=ephemeral (monotonic, legal), and sent it
through the corp llm-proxy to Anthropic — HTTP 200 with tool_use
content and a cache_creation usage block (previously this exact shape
was the reviewer's case C ordering violation).
- `tsc --noEmit -p packages/core/tsconfig.json` and eslint clean for all
touched files.
- `vitest run` on converter.test.ts (79 tests), anthropicContentGenerator.test.ts
(112 tests), and settingsSchema.test.ts (36 tests) — all pass.
---------
Co-authored-by: Palanisamy, Dinesh <Dinesh.Palanisamy@netapp.com>
* fix(core): tolerate transcript timestamp drift
Treat transcript timestamps as advisory and verify content before fencing a live writer. Preserve exact shutdown release semantics while aborting slow acquisition scans.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): Address transcript reconciliation review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): address transcript snapshot review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): isolate FileHandle prototype spies to prevent test flakiness (#7886)
Capture the real native FileHandle.prototype.read/stat once in beforeAll
and use them as delegation targets in all reconciliation tests, instead
of each test capturing whatever is currently on the prototype (which
could be a previous test's spy if cleanup ordering varied). Add
vi.restoreAllMocks() and explicit prototype restoration to afterEach as
a safety net against spy chaining across the shared prototype.
* test(core): add append-path symlink-race regression test (#7886)
* fix(core): replace unreliable chmod with utimes in reconciliation tests (#7886)
Seven content-verification tests used chmod(path, sameMode) to create a
timestamp drift that forces the reconciliation path. On filesystems that
coalesce same-mode chmod ctime updates (e.g. ext4 with lazytime), the
drift is not observed and the fast path is taken, so the read/stat spy
never fires and the test fails non-deterministically.
Replace with utimes(path, atime, mtime + offset), which explicitly sets
a different mtime and is deterministic across all filesystems. Tests
whose spies also inject utimes offsets use +500 for the setup to avoid
colliding with the spy's +N*1000 offsets.
* fix(core): replace unreliable chmod with utimes in reconciliation tests (#7886)
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
* feat(verify-pr): ship a one-command capture helper
Fourth attempt at the same failure, and the first one aimed at the real
cause. The score so far:
#8016 captures were "Optionally … when text cannot carry the
oracle" -> 0 images / 14 runs
#8104 captures became budget item 4 in Scope
selection -> 0 images / 1 run
That last run is the one that settles it. Same PR (#7975), browser
installed and working ("Install evidence browser: success"), and the
verification got DEEPER — 64 assertions against 40, 53 tables against
31 — while still producing zero images. An agent reading the instruction,
doing more work than before, and still not capturing is not an agent that
missed the instruction.
The cause is one I should have checked when I wrote #8016: the skill sent
the agent to build node-pty -> xterm.js -> Playwright itself, and
`node-pty` is not a dependency of this repo. It needs a native build. The
`playwright` package is not a declared dependency either. So the
documented route did not exist, and the incentive was entirely against
trying it: authoring that pipeline risks failing and eats budget, while
skipping costs nothing and is invisible.
`scripts/verify-capture.mjs` makes a capture one command using deps that
are already installed:
node scripts/verify-capture.mjs --out evidence/01-ab.png \
--title 'A/B: the gate flips' -- node my-harness.mjs
Command (or stdin) -> @xterm/headless parses the ANSI into a cell grid
with colour and bold -> SVG -> sharp rasterises. No browser, no
pseudo-terminal. A non-zero exit from the captured command still produces
an image, because capturing a failing base arm is the normal case.
The skill's dead route is removed and replaced with that command, and the
budget line drops from ~5 minutes to ~2 because there is no pipeline to
author. QWEN_VERIFY_CHROMIUM and its install stay for a future web-UI
capture, but the terminal route no longer depends on them — gating on a
browser the route does not use is how an absent browser turns into a
skipped capture.
Mutation-verified 7/7 against the real helper and the real PNGs: bare LF
(staircase render), no event-loop turn (blank capture), dropped bold,
dropped colour, no blank-row trimming, tolerating an empty capture, and
dropped geometry validation.
Two of those initially SURVIVED. The colour test compared "coloured and
bold" against plain and asserted the bytes differ — which passes while
EITHER attribute survives. That is the wrong-reason trap this skill warns
about, met in the skill's own test file. Each attribute is now isolated
against the same plain baseline (green-no-bold, bold-no-colour), and both
mutations kill.
119/119 across both suites; prettier and eslint clean.
* fix(verify-pr): harden capture helper per review (#8114)
- Strip U+FE0F before rasterising: the emoji variation selector made Pango
abort() in native code (SIGTRAP, no PNG, no diagnostic) when no colour-emoji
font exists; the base codepoint renders. Add a non-ASCII regression test.
- Await xterm's write callback instead of a fixed 120ms sleep, so a large
capture is not read mid-parse and silently come out blank.
- Name a signal-killed child ("killed by SIGKILL") rather than "exited null".
- Warn on stderr when input is taller than --rows and the top is dropped.
- Correct the falsified "route did not exist" rationale: the browser pipeline's
deps do resolve from this repo; the real fragility is that
integration-tests/terminal-capture is not a root workspace. Keep a skill
pointer to terminal-capture for TUI/web-UI captures this helper cannot do.
- Qualify the colour claim (16 base ANSI colours; 256/truecolor fall back).
- Tests: feed both --cols and --rows to the geometry guard, exercise escapeXml,
flatten the SKILL.md assertion against reflow, and replace the platform-fragile
PNG byte-length check (flaked at 846B on Linux vs >1000B on macOS) with the
deterministic canvas geometry.
* fix(verify-pr): exercise colour fallback and fix wrap-aware truncation warning (#8114)
* fix(verify-pr): separate stdout/stderr join and pin next() guard (#8114)
* fix(scripts): correct verify-capture truncation guard and SGR 30 colour (#8114)
The truncation guard compared wrapped rows against --rows, but
newline-terminated output needs one row beyond its last line (the final
CRLF scrolls it off the scrollback-less viewport), so input of exactly
--rows lines — including the default 40 — lost its top line with no
warning, and taller input under-reported the drop by one. Compare against
a capacity of rows - 1 for newline-terminated input.
Also lift SGR 30 foreground to the default grey: it mapped to #1e1e1e,
identical to the canvas background, so black-foreground labels (e.g.
vitest's project badge) vanished as black-on-black.
* fix(scripts): address review feedback on verify-capture helper (#8114)
- Declare sharp as a root devDependency so the script does not rely on
workspace hoisting from packages/core
- Guard against TTY stdin hanging silently: check process.stdin.isTTY
before readFileSync(0) and print usage immediately
- Fix phantom blank row when stdout already ends with a newline: only
insert a separator between stdout and stderr when stdout lacks a
trailing newline
- Strengthen 256-colour/truecolor test to decode pixels and assert the
#d4d4d4 fallback grey is present
- Add test for non-newline-terminated input that fits exactly --rows
- Add test for the phantom blank row fix (console.log + stderr)
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(test): route setPermissionMode E2E timeouts through TEST_TIMEOUT (#8133)
The 'yolo to plan' and 'auto-edit' setPermissionMode tests hardcoded 10s/15s response timeouts while their passing sibling 'default to yolo' uses the CI-aware TEST_TIMEOUT (60s on CI). A single model round-trip routinely exceeds 30s on shared CI runners, so the tight values made these two tests time out waiting for the first/second response. Route all four hardcoded values through TEST_TIMEOUT to match the established CI-stability pattern.
* fix(test): widen closed-query test timeout to reduce flake risk (#8133)
---------
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
A cache whose producer and consumer never agree on identity is invisible
to every lens the review currently has. `actions/cache` matches an entry
on `(key | restore-key)` AND a `version` that hashes the literal `path`
strings plus the compression method, not the key alone. So two jobs can
share a key, share the `path:` line exactly as written, and never hit
once: `${{ runner.temp }}` expands to a different string on a hosted
runner than in a container job or on a self-hosted runner, and an image
without the `zstd` binary picks gzip where a hosted runner picks zstd.
The workflow path-rule already covered a cache a fork can poison; it said
nothing about one that can never fire. Add that class as a blocker, so
every dimension agent whose territory holds a workflow is asked whether
the side that writes agrees with the side that reads — a question no
assertion about the YAML's shape can answer, because the two sides'
strings match in exactly the case that fails.
Give verify-pr the method to settle it: read the matching key from the
implementation rather than the documentation, compare the two sides'
environment tuples rather than their YAML strings, and treat a hit that
leaves no observable signal as the finding rather than a nit.
Also record that the verify container is a live sample of the lane's own
runtime. When the diff changes what those lanes execute, `command -v
zstd` or `echo "$RUNNER_TEMP"` settles in one shell command what no
amount of YAML reading settles, and needs no GitHub token — which that
environment does not have.
* fix(web-shell): isolate worktree session execution
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(mcp): expect session cwd in effective config
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(acp-bridge): cover shell execution after a failed cwd change (#8068)
* fix(acp-bridge): abort direct shell waiting on a hung cwd change (#8068)
* refactor(core): centralize effective MCP server resolution (#8068)
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(core): align MCP cwd stamping with transport classifier (#8068)
* fix(core): move MCP cwd stamping to populateMcpServerCommand (#8068)
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).
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
The side-task creation route still constructed a session service directly on
two rollback paths after the surrounding module moved to the workspace runtime
helper, leaving the identifier unbound and breaking the CLI build on main.
Route both rollbacks through the workspace runtime helper so they honour the
per-workspace runtime base dir like every other session-service use in this
module.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
- positionInsideFence now skips lines inside invoke block ranges so
fence-like content in parameter values (e.g. an edit old_string
containing triple backticks) no longer causes later invoke blocks
to be silently skipped
- Decode the five predefined XML entities (< > & "
') in parameter values so recovered args match the literal
text the model intended
- Support single-quoted attribute values in invoke/parameter tags
- Strip empty <function_calls> wrapper tags from remainingText
- Include recovered tool names and contentLength in the success log
* 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>
* 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>
* fix(autofix): salvage race-lost pushes by merging the moved head and retrying
The review-address push is one-shot: when anything pushes to the PR head
during the agent's ~50-minute window, the final push dies 'fetch first'
and the entire verified agent run is discarded. The per-PR head-write
concurrency group cannot prevent this — it serialises this repo's
workflows, not the PR author or the fork side. Observed twice in one day
(#7983 after a 56-minute run, #7985 after 43 minutes).
On rejection, fetch the moved head, merge it into the local line, and
retry (bounded at 3 attempts). Merge rather than rebase: the agent's own
conflict-resolution rounds create merge commits, and a rebase would
flatten them and can silently re-introduce the conflicts they resolved.
The merge result descends from the remote head, so the retried push is a
fast-forward and rewrites nothing. A genuine content conflict aborts the
merge and falls through to the existing failure path unchanged.
When a salvage merge happened, the round report discloses that the
round's verification predates the merge so mid-run commits get
re-checked by a human.
* fix(autofix): address salvage-loop review findings
- Gate the PUSH_RACE_MERGED disclosure on HEAD actually advancing: a
transient push failure (upload timeout, 503) on an unmoved branch
no-ops the merge ('Already up to date') and must not tell the
reviewer to re-check mid-run commits that never existed.
- Annotate the salvage fetch failure with ::error:: like the two
adjacent failure paths, so a deleted fork branch or network error
does not kill the step with an unannotated exit 128 under bash -e.
- Re-pin the same-repo push URL construction in tests: it lost its old
'origin "${BRANCH}"' pin in this rework, leaving a ${REPO}→${HEAD_REPO}
mutation (malformed remote in the same-repo case) unkillable.
* test(autofix): restore dropped mutation-killing pins and add structural assertions (#8042)
* test(autofix): pin exit 1 in the give-up guard regex to kill the deletion mutation (#8042)
* test(autofix): pin exit 1 in the fetch-failure and merge-conflict salvage paths (#8042)
* test(autofix): strengthen salvage-test pins to kill init-value and capture-order mutations (#8042)
---------
Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
* fix(autofix): never defer maintainer feedback in Critical-only mode
Critical-only mode (after 5 change-producing rounds) classifies
feedback lexically: only a literal **[Critical]** tag or a
CHANGES_REQUESTED review survives; everything else is deferred before
the agent reads a word of it. That rule was built to stop the review
bot's suggestion ping-pong, but it catches maintainers too. Observed
four times in two days (#8037, #7944, #7885, #7799): a maintainer's
review with explicit merge-blocking findings — #8037's said 'I'd fix
before merge' on a correctness bug and a security-adjacent one — was
wholesale-deferred as one 'non-Critical item', and the bot then
reported 'No Critical feedback. The Issue-level comments sections are
empty', which was lexically true and substantively false. The bot's
own advertised definition ('correctness bugs, security issues, or
formally requested changes') is exactly what the deferred comments
contained; the agent that could have applied that definition never saw
them.
The lexical test now applies exclusively to the review bot's output:
- All three actionable filters (reviews, inline, issue-level) pass
anything not authored by the review bot straight through in
Critical-only mode — the agent judges maintainer feedback on
content, as everywhere else.
- All three deferred-list builders keep only review-bot items, so a
maintainer comment can never appear as an 'audit record'.
- The deferral note says what is actually deferred (the automated
reviewer's non-Critical suggestions), states that maintainer
feedback is never deferred, and names the exit (@qwen-code /retry
opens a fresh counting window).
- SKILL.md's Critical-only policy now marks everything rendered in
the actionable sections as in scope, so the agent does not re-refuse
what the filter passed through.
Behavioral test updated both ways: maintainer comments/reviews stay
actionable in Critical-only mode across all three sources, bot
suggestions still defer, and structural pins hold the bypass and the
bot-only select in all six filters.
* feat(autofix): per-author feedback budget in Critical-only mode
Follow-up to the author-based split, prompted by the obvious
counterexample: a human account can host an automated reviewer loop
with the exact regeneration property the review bot has — feedback
re-generated after every push at zero marginal cost — so 'not the
bot' cannot mean 'never throttled'. An account is an accountability
unit, not a throttle; the brake has to key on measured regeneration.
Unified model: once Critical-only engages, every source has a bounded
budget of untagged feedback batches per counting window. The review
bot's budget is zero (all deferred, as before). A human's is
CRITICAL_ONLY_HUMAN_BATCHES (2) CONSUMED batches: feedback items are
bucketed into the (prev marker ts, marker ts] span that evaluated
them, only spans from Critical-only rounds count, and an author needs
K distinct consumed spans before their new untagged feedback defers.
Fresh unevaluated feedback never counts against its own author, and
the census is window-scoped, so /retry resets the budget with the
window.
The observed cases (#8037/#7944/#7885/#7799 — one or two late
verification reports each) stay fully served under K=2; a looped
reviewer is throttled after 5+K driven rounds instead of grinding to
the 100-round cap. Past the budget, continuing requires one conscious
act — **[Critical]**, a Request changes review, or /retry — which is
precisely what separates intent from automation. Over-budget authors
are named in the deferral note with those exact escapes.
Tests: the six filter replays gain over-budget cases both ways (the
tagged/CR escapes survive even over budget), and the budget census
itself is replayed over fixture files — two consumed critical-tail
batches list the author; one batch, pre-Critical batches, unconsumed
feedback, untrusted authors, and command comments never count.
* fix(autofix): fix deferred-feedback bash quoting and drop a dead jq binding (#8071)
* test(autofix): exercise census window-isolation guard with a stale-window fixture (#8071)
* test(autofix): make census command-exclusion observable; surface census stderr (#8071)
* fix(autofix): exclude never-deferrable feedback from the budget census (#8071)
The Critical-only per-author budget census counted every trusted review,
inline comment, and issue comment, including feedback the deferred renderer
would never defer: **[Critical]**-tagged comments, Request changes / APPROVED
reviews, inline replies rooted at a Critical comment, and inline comments
attached to a Request changes review. A maintainer who followed the documented
escape hatches (tag Critical, request changes) thereby spent their own budget
and had later untagged feedback silently deferred — the exact bug this PR
fixes, re-created one level down.
Mirror the three deferred-builder predicates in the census item filter so a
batch is counted only when it is actually deferrable. Extend the census replay
test with protected authors (Critical-only, Request changes, APPROVED,
Critical-rooted replies, Request-changes-review inlines, the review bot as a
trusted MEMBER, and a sentinel-ts marker probe) that each carry two
consumed-span batches yet must stay absent, so dropping any one exclusion now
fails the suite. Also fold bash's stderr into the bash -n guard assertion so a
future quoting regression reports the syntax error, not just a non-zero exit.
---------
Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
* fix(autofix): answer cap-gate refusals on the PR instead of only in logs
Observed on #7836: the fleet shepherd detected a merge conflict, posted
'dispatched the autofix loop to resolve it', and the dispatch died at
the scan's round-cap gate with only a log line — the PR page showed a
promise, the run showed green, and the conflict sat unhandled for
hours. Three silences stacked: the standard-management cap itself is
silent (the pause notice was takeover-only, so #7836 hit 10/10 with
zero PR-visible notice), the forced-dispatch refusal is silent, and the
shepherd dedups per head SHA — a capped PR gets no pushes, so its head
never changes and conflict handling froze permanently.
Two scan-side changes (the shepherd stays untouched — the windowed
round computation lives in the scan and duplicating it would drift):
- A FORCED dispatch (shepherd conflict lever or a human) refused at the
cap gate now answers on the PR: cap value, what stays unhandled, and
the two recovery commands (/retry for a fresh window, /takeover for
the raised cap). No dedup — the shepherd sends at most one dispatch
per head, and a human asking twice deserves two answers.
- The cap pause notice covers ALL managed PRs: the takeover variant
keeps its wording, standard bot PRs get their own (/retry or
/takeover). Same marker, same once-per-window dedup, same consent
and PAT-identity checks — skip wins everywhere, and only the
takeover variant requires the label to still be present.
After a re-arm the next scheduled scan picks the PR up normally
(conflict targets are label-independent), so the frozen-head loop
resolves without any shepherd change.
* test(autofix): replay the cap-notice consent gate across label/takeover permutations (#8067)
* fix(autofix): gate the loud cap-refusal on workflow_dispatch (#8067)
FORCED_PR is populated for every trusted pull_request_review (route emits
pr_number for those), not just workflow_dispatch, so on a capped PR each
review submission landed in the un-deduped refusal branch — 7 "Dispatch
refused" comments on #7836 where 2 carried the information. Answer only
workflow_dispatch (the shepherd lever or a human); review submissions stay
covered by the once-per-window pause notice. Adds a verbatim behavioral
replay of the guard so a dropped EVENT_NAME condition fails the test.
---------
Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
