Re-read devicePixelRatio inside each resize so the specular composer
effect and the new-session dot field keep a correctly sized backing
store when the page zoom or display scale factor changes. Add
component-level coverage for the prefers-reduced-motion guards and for
the typewriter replay after the empty editor loses focus.
* 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>
* feat(channels): dispatch GitHub notifications by reason
Route each GitHub notification by notification.reason into one of five
lanes, instead of dispatching every new comment regardless of trigger:
- mention: only dispatch comments that actually @ the bot (noise reduction)
- review_requested (PR): fetch PR meta via pulls.get and dispatch a
review-specific prompt, even with no new comments
- assign: fetch issue meta and dispatch a triage-specific prompt
- author/comment: aggregate the window's new comments into one check-and-
respond prompt
- other reasons: generic fallback (current behavior)
Add cursor dedup via dispatchedComments (by comment node_id) and
dispatchedNotifications (by notification id), surviving a
markNotificationsAsRead failure that leaves the cursor un-advanced.
Closes#7807
* fix(channels): mark review_requested/assign envelopes as mentioned
GroupGate defaults to requireMention: true, which silently drops
isMentioned:false envelopes as 'mention_required'. The review_requested
and assign lanes are explicit directed triggers — the bot was asked to
review or assigned — equivalent to a mention, so set isMentioned: true
so they pass the gate instead of being inert on the documented default
config.
Addresses review Critical on #7826.
* fix(channels): resolve github routing review comments
* fix(channels): dedupe github meta lane comments
* fix(channels): conditional assign framing for PR threads
The assign route already detected PR threads to use pulls.get, but the
trigger framing text always read 'assigned to this issue' even for PRs.
Make it conditional so PR assignments read 'assigned to this pull request'.
* fix(channels): dedup meta lane dispatch inputs
* fix(channels): simplify GitHub reason dispatch
* fix(channels): respect mention gate for github aggregate lane
* fix(channels): truncate aggregate comment bodies by code points
Match the code-point-aware truncation already used for meta-lane bodies
so a supplementary-plane emoji at the MAX_COMMENT_CHARS boundary is not
split into a lone surrogate.
* fix(channels): harden GitHub dispatch failures, event window, and framing (#7826)
- Classify deleted/transferred subjects (404/410) as terminal so a single
dead notification is logged and skipped instead of wedging the batch's
mark-read and cursor advance every poll.
- Widen the review_requested/assign event search to the newest ~100 events
by merging the preceding page when the last page is partial, instead of
inspecting only the last page (which can hold a single event).
- Move the aggregate lane's untrusted-data warning to the head of the prompt
text so it precedes the comment text it describes (metadata is appended
after text by ChannelBase).
- Add regression tests: permanent-failure two-poll advance, terminal 404
no-retry, multi-page event search, prompt caps, and the no-actor guard.
* fix(channels): drop lastReadAt filter in findMetaTrigger, add review coverage (#7826)
* fix(github): keep aggregate and meta windows bounded
* fix(channels): apply windowSince lower bound in findMetaTrigger (#7826)
* fix(channels): bound retry wedge, compute aggregate isMentioned, fix pairing pre-filter (#7826)
* fix(github): record dispatch before handler
* fix(github): persist skipped notifications
* fix(github): close dispatch retry loss cases
* test(github): cover cursor trim and meta floor validation
* fix(github): simplify notification reason dispatch
* fix(github): preserve batched dispatch comments
* fix(github): restore direct event dedup
* fix(github): preserve directed mention context
* fix(github): keep review fixes scoped
* fix(github): preserve delayed direct triggers
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code <qwen-code@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>
* fix(review): recover the resolved effort when --effort is not re-threaded
The capture commands (capture-local, plan-diff, fetch-pr) record the review
effort as plan.effort, which every downstream consumer — roster, check-coverage,
compose-review — reads. They learn it from --effort <level>, and the skill asks
the orchestrator to pass the level parse-args resolved. But that is a value the
model must copy from one file into a flag, and it does not reliably happen: a
`/review --effort medium` local run had the orchestrator omit the flag, so
plan.effort was absent and the roster safe-expanded to the FULL set — the user
asked for the reduced medium roster and silently got every agent (6a/6b/6c
included).
Close the gap deterministically. A new resolveEffort() prefers an explicit
--effort, and otherwise reads the level parse-args already wrote to its
conventional report (.qwen/tmp/qwen-review-parse-args.json). When neither is
available it returns undefined and the roster fail-safes to the full set exactly
as before, so a missing report never reduces coverage, and a malformed level is
ignored rather than trusted.
* refactor(review): dedupe effort resolution per review feedback
- Share one EFFORT_LEVELS set: export it from parse-args and import it in
effort.ts, so a new level cannot be added to one set but not the other.
- Collapse the three identical effort-spread IIFEs into planEffortField().
- Isolate CWD in plan-diff.test.ts (matching capture-local.test.ts) so
resolveEffort's CWD-relative report read cannot pick up a stale file and
fail the "omits effort" case.
* test(review): pin PARSE_ARGS_REPORT value and plan-diff effort fallback (#7855)
* test(review): dedupe seed helper, cover fetch-pr effort, trace resolution (#7855)
* fix(review): spread planEffortField last for consistent effort precedence (#7855)
---------
Co-authored-by: verify <verify@local>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
* fix(web-shell): report intended workspace to host when starting a new chat
Clearing a session leaves connection.workspaceCwd pointing at the previous session's workspace. The onSessionIdChange notification read that stale value, so starting a new chat in workspace A routed the host back to the old workspace (e.g. one with a running task) and the composer showed the wrong workspace. With no active session, report the workspace picked for the next session instead.
* fix(web-shell): reuse activeWorkspaceCwd for the no-session host report (#7910)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.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.
* fix(cli): remove redundant "Read file" prefix from @mention tool card
The @mention file-read tool card set its description to
"Read file <name>", which duplicated the display name ("Read")
when rendered as "{displayName} {description}", producing
"Read Read file README.md".
Change the description to "@<name>" — matching the @mention
syntax the user typed, and consistent with how normal read_file
tool calls show just the filename in their description.
* test(cli): assert @mention tool card description format (#7902)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>
* fix(cli): add polling fallback for git branch name display (#7828)
fs.watch on .git/logs/HEAD is unreliable on NFS, FUSE, Docker overlay,
and some Linux filesystems — events can be silently dropped with no
recovery path, leaving the footer branch name stale indefinitely.
Added a 5-second polling fallback in useGitBranchName that calls
resolveBranchName and updates the state only when the value changes.
The timer is unref'd so it doesn't keep the process alive. The
existing fs.watch mechanism is preserved for immediate updates when
it works correctly.
* test(cli): cover the git branch polling fallback (#7830)
* refactor(cli): use idiomatic timer.unref?.() in branch poller (#7830)
* fix(cli): order concurrent branch refreshes by generation (#7830)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
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(cli): hide sticky todo panel when a new turn starts
PR #7062 hid the sticky panel when streamingState is Idle, but the panel
resurfaces with stale todos from a previous turn as soon as the user sends
a new message (state → Responding). This is confusing — the user sees
in-progress indicators for work that already finished in an earlier turn.
Add a turn-boundary check in getStickyTodos: if a user message exists
after the todo snapshot in history, the snapshot belongs to a previous
turn and the sticky panel returns null.
Fixes#7061
* test(cli): pin sticky-todo turn boundary for local slash commands (#7061)
---------
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.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.
* feat(web-shell): add native workspace folder picker
* fix(serve): harden native directory picker and add coverage (#7849)
Add a 5-minute timeout to each native picker subprocess and to the
webui action so a dismissed dialog cannot leave an orphaned GUI process,
and distinguish a headless Linux "cannot open display" failure from a
deliberate zenity cancellation. Cover pickNativeDirectory's platform
branches, the route's 501/500 error paths, and the dialog's picker
failure path with focused tests.
* fix(serve): log directory picker failures to daemon stderr (#7849)
* fix(serve): abort directory picker on client disconnect and stagger timeouts (#7849)
* test(webui): add unit tests for pickWorkspaceDirectory action (#7849)
* fix(serve): set UTF-8 console encoding for PowerShell picker and treat timeout kills as cancels (#7849)
* fix(serve): abort directory picker on response close, not request close (#7849)
---------
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 CI <qwen-code-ci@users.noreply.github.com>
selectCodeBlock seeded `lang` from the first token before the classification
loop ran. For a bare number that is wrong: `/copy 1 2` strips the leading 1
as the message index and hands "2" over as the whole selector, so lang became
"2" while the loop set requestedIndex to 2. The filter then looked for blocks
written in a language called "2", found none, and reported that no code block
matched. The argument hint advertises `[N] [<lang>|code|latex|mermaid]
[<index>]` with every group optional, so this is a documented form.
The seeding is redundant as well as wrong: the loop already assigns lang for
any token that is neither a number nor `code`.
Same defect and same fix as #7789 in packages/web-shell; @wenshao verified the
CLI copy carried it and suggested the follow-up. Filed separately rather than
folded into that PR.
countHeadContentLines counted the newlines in the committed head, and when
the head did not end in a newline it returned the raw count -- treating the
unfinished last line as complete. The tail's start-line directive is
computed from that count, so every line number after a mid-line split was
one too high.
splitFencedMarkdown('```js\naaa\nbbb\nccc\n```', 12)
head: '```js\naaa\nbb\n```\n'
tail: '```js qwen-code:start-line=3\nb\nccc\n```'
The tail's first row is `b`, the remainder of source line 2, but the
directive labelled it 3.
A completed line and a partial one leave the same number of newlines
behind, so neither needs a special case: the count is always one less than
the newlines in the head, floored at zero. Splits landing on a line
boundary are unchanged.
An existing test asserted `start-line=2` for a block holding one 100-char
line split at 40. The tail there is the remainder of line 1, so it is
updated to 1.
`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.
wrapToVisualLines and sliceTextByVisualHeight both measure visual rows at a
given width, but only sliceTextByVisualHeight clamps the per-character
width with Math.max(..., 1). `string-width` returns 0 for TAB, ZWJ and
combining marks, so a run of them was charged nothing and collapsed into a
single row:
wrapToVisualLines('\t'.repeat(50), 10).length -> 1
sliceTextByVisualHeight('\t'.repeat(50), 3, 10) -> 5 rows
Same input, same width, two answers. Callers that mix the two -- scroll
offsets, pending-render height -- then disagree with themselves.
Apply the same clamp. Erring high is the safe direction for a terminal:
reserving one row too many costs a blank line, while counting one too few
overflows the region and pushes content off screen.
This aligns on the convention sliceTextByVisualHeight already follows,
including its treatment of combining marks, which both now charge a column.
Making that more precise would mean changing both functions and is left
alone here.
`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.
selectCodeBlock seeded lang from the first token before the loop that
classifies tokens. For '/copy 3' the one token was used twice: it set
lang to "3" and requestedIndex to 3, so the filter looked for blocks
written in a language called "3", found none, and returned early with
'No matching code block found' before the index was consulted.
The command's own argument hint advertises
'[code|<lang>|latex|inline-latex] [index]' with both groups optional, so
a bare index is a documented form - and it was the only one that failed.
Drop the pre-loop assignment. It was redundant: when the first token is
not 'code' it is already in selectorTokens, and the loop assigns lang for
every non-numeric token.
Two classes wrote '-' mid-class, where it is a range operator, and both
described something other than what was intended.
[*-+] was the range U+002A-U+002B, exactly {*, +}, so '- item' was not
seen as a list item and never reset tracking - a normal bulleted list
accumulated until it tripped the repetition check and halted a healthy
response.
[+-_=*] was the range U+002B-U+005F, every digit and every uppercase
letter, so 'SELECT', '12345' and '>>>' read as horizontal rules. A
divider also returns early, so that content left the history entirely
and a model chanting such a token could never be detected.
Place '-' first in both classes. The box-drawing span stays a range.
The SOCKS guard tested /^socks[45]?:\/\//, which misses socks5h:// and
socks4a:// - the hostname-resolving variants that curl, proxychains and
most tunnels emit. Those fell through to the http:// prefix and produced
'http://socks5h://127.0.0.1:1080', which parses as the host 'socks5h' on
port 80, so requests failed with ENOTFOUND for a nonexistent host and
nothing mentioned SOCKS.
Match the whole socks family instead. A scheme starting with 'socks' is
always a SOCKS proxy, and the pattern still requires '://' so a hostname
like socks.example.com is unaffected.
calculateCost ended with 'total > 0 ? total : null', and callers read
null as "no cost to report" - the stats table prints N/A and hides the
Cost section entirely when no model yields a number.
Testing the total conflated a zero cost with an unknown one. A model the
user deliberately priced at 0 (a free OpenRouter variant, a local Ollama
or LM Studio model) was reported as N/A, and the Cost section vanished
when it was the only model in the session.
Decide on the pricing and the usage before the arithmetic, then return
the total unconditionally. The no-usage case still returns null, which
statsCommand.test.ts documents as deliberate.
* feat: gate session writer lease behind opt-in
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp): freeze session writer lease per process
Snapshot the effective restart-required lease gate from the bootstrap Config and reuse it for every session Config in the ACP process.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): align recorder default lease gate
Use the effective session writer lease gate when ChatRecordingService is constructed without an explicit writer mode.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(webui): fall back to history_truncated marker recordId for transcript pagination anchor
A long in-flight turn can push the live journal past its cap with only
streaming session_updates (no turn-boundary recordId). The retained
replay window then has no `qwen.session.recordId` to anchor transcript
pagination, so `historyHasMore` collapsed to false and the
'History truncated' banner rendered with no loadMore recovery path.
The compaction engine now tracks the last-seen recordId and stamps it
on both history_truncated markers (compacted replay and live journal).
The webui's getPersistedReplayRecordId falls back to the marker's
recordId when no session_update in the retained window carries one,
unlocking transcript pagination again.
Backward compatible:
- Old daemon (no recordId field): marker is field-less, frontend
behavior matches pre-fix (banner with no loadMore).
- New daemon + old web-shell: extra recordId field ignored by SDK
validator/normalizer/hasFullTranscriptBeforeReplay.
Tests:
- compactionEngine: marker carries recordId on journal overflow,
post-seed ingest rebuilds activeRecordId, seedReplayEvents captures
evicted recordId, marker from evicted head when retained lacks one.
- DaemonSessionProvider: marker recordId used as pagination anchor
when session_updates lack one (regression for the retained 10000 /
dropped 7602 scenario).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): freeze replay marker anchor at eviction boundary (#7829)
The replay-path history_truncated marker stamped its recordId from
activeRecordId, which ingest() advances on every post-seed turn
boundary — pushing the anchor past records the client already
displays and causing duplicate transcript blocks on pagination.
Track replayAnchorRecordId separately, captured at the first
replay-window eviction and frozen thereafter. The live-journal
marker continues to use activeRecordId (correct for in-flight
turns). On the client, prefer session_update recordIds over the
marker's stamped anchor so the earliest retained recordId wins
the pagination scan.
Also: reset activeRecordId before the seedReplayEvents pre-scan
(stale-value guard), and add the matching pre-scan to seed() so
eviction cannot lose the only recordId anchor.
* refactor(acp-bridge): extract shared lastRecordIdIn helper (#7829)
* fix(acp-bridge): backfill transcript pagination anchor for live sessions
The marker-recordId fallback only covers sessions whose retained window
holds at least one recordId-bearing event. Live sessions never do:
`qwen.session.recordId` is stamped solely during replay of the persisted
transcript (HistoryReplayer), never on the live event stream. A long
in-flight turn that caps the live journal before any turn boundary fires
leaves the retained window — and thus the truncation marker — with no
recordId at all, so `historyHasMore` still collapsed to false and the
'History truncated' banner rendered with no loadMore recovery path
(observed: retained 10000, dropped 1259, window 8388608 bytes, 20
concurrent Web Shell sessions).
The daemon now backfills a `historyAnchorRecordId` on the load response:
when the replay snapshot carries a truncation marker with no recordId
anywhere, it reads the latest recordId from the persisted transcript and
returns it as a top-level field. The webui uses it as the last-resort
`beforeRecordId` anchor (after session_update and marker recordIds), so
transcript pagination works even for first-turn / mid-turn live sessions.
Best-effort by design: any transcript read failure omits the field and
the client degrades to the pre-fix banner behavior.
Tests:
- bridge: backfills historyAnchorRecordId from the transcript when the
marker carries no recordId (seeded replay without recordIds + attach
via in-memory snapshot).
- DaemonSessionProvider: uses daemon historyAnchorRecordId when neither
marker nor session_updates carry a recordId.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): anchor replay marker at eviction boundary, dedup prepended transcript
Review follow-up addressing two Critical findings on the pagination
anchor and one Suggestion.
Critical — replay-path marker anchor (compactionEngine):
`replayAnchorRecordId ??= activeRecordId` froze the pre-scanned
`activeRecordId` — the LAST recordId across ALL seed events. When a
retained segment carried that recordId, the anchor sat inside the
retained window, so the client's `beforeRecordId` re-fetched records it
already displays and `prependTranscriptHistory` (no dedup) rendered them
twice. Now the anchor prefers the FIRST retained recordId (the eviction
boundary — `beforeRecordId` fetches exactly the dropped records with no
overlap), falling back to the last DROPPED recordId only when the
retained window carries no recordId at all.
Critical — prepend overlap safety net (webui):
Even a well-placed anchor can overlap the retained window in edge cases
(the daemon's transcript backfill for a live-journal overflow returns
the latest recordId by design). `prependTranscriptHistory` now drops
fetched events whose `sourceRecordIds` are already displayed, so any
anchor source yields duplicate-free history.
Suggestion — reuse `getString` for the marker recordId instead of an
inline `typeof` check.
Tests:
- compactionEngine: anchor is the first retained recordId, not the last
overall (3-segment eviction where retained holds rec-B and rec-C).
- DaemonSessionProvider: fetched events whose records are already
displayed are dropped, not duplicated.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(acp-bridge): correct anchor docs, skip backfill when marker has recordId, close attach race (#7829)
---------
Co-authored-by: wenshao <wenshao@example.com>
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: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
