* feat(channels): add GitLab polling channel adapter
Poll GitLab todos via @gitbeaker/rest, dispatch notes through the
existing PollingChannelBase pipeline. Key design points:
- action_prompt_template config drives event filtering and metadata
rendering (unconfigured actions are skipped)
- Per-repo cursor (repo[chatId].last_read) as notes window lower bound,
global lastProcessedAt for todo-level dedup
- mark_done after successful processing; failure skips mark_done for
retry on next poll
- Mention gating delegated to base GroupGate (adapter only sets
isMentioned flag)
- First-contact body fallback for todos with no notes (e.g. mention in
issue description)
* fix(channels/gitlab): persist cursor after each successful todo
Call saveCursor() immediately after advancing lastProcessedAt so that
progress is durable even if the process crashes mid-poll. Also removes
the local watermark variable in favor of direct assignment.
* fix(channels/gitlab): persist cursor on every advancement including skips
* fix(channels/gitlab): address review critical issues
- Remove non-functional proxyAgent (gitbeaker doesn't support it)
- Construct repo_url from host + path (API doesn't return web_url)
- Handle directly_addressed action (falls back to mentioned template)
- First-contact fetches target description instead of using todo.body
- Move todo.project dereference inside try block
- Filter confidential notes
- Update channel-registry.test.ts for gitlab entry
* fix(channels/gitlab): address review suggestions
- Warn on connect if action_prompt_template is not configured
- Guard todo.target.iid before use
- Skip paths now mark_done (best-effort) to clean GitLab UI
- Remove postErrorComment (avoids duplicate comments on retry)
- Fetch only first page of notes (desc, maxPages:1, perPage:100)
instead of paginating entire note history
- Extract fetchRecentNotes for single-page windowed enumeration
* refactor(channels/gitlab): simplify to todo.body dispatch, add description mention support
- Remove notes API fetching; dispatch todo.body directly
- Detect description mentions via target_url anchor (#note_ absence)
- Always fetch target description for %description% metadata
- Remove per-repo cursor; dedup via cursor + mark_done only
- Cursor advances regardless of success/failure (no retry)
- Use zod for cursor validation
- Rename template vars to GitLab terminology:
%project% %project_url% %target_type% %iid% %title% %description% %todo_id%
- Support %% escape for literal percent
* docs(channels): add GitLab adapter documentation
- New user guide: docs/users/features/channels/gitlab.md
- Update _meta.ts navigation
- Update developer adapter matrix and SDK list
* fix(channels/gitlab): use correct Issues.show(issueIid, { projectId }) signature
* chore: regenerate NOTICES.txt for new gitlab channel dependencies
* fix(channels/gitlab): address review suggestions
- Add todo.project null guard (item 2)
- Single-pass regex for %% escape + %var% substitution (item 4)
- sendThreadMessage throws directly on undefined threadId (item 5)
- Dedup fetchDescription with per-poll cache (item 6)
- Remove per-todo saveCursor; base class saves after pollOnce (item 7)
- Add undefined threadId test (item 8)
- Expand confidential notes limitation in docs (item 3)
* test(channels/gitlab): add mention tests, directly_addressed coverage, skip assertions, temp cleanup
- New mention.test.ts: 14 cases for testBotMention/stripBotMention/escapeRegex
- Add directly_addressed fallback test
- Skip tests now assert TodoLists.done + cursor advancement
- afterEach cleans up mkdtempSync temp dirs
* fix(channels/gitlab): address review round 4
- Non-mention actions (assigned, etc.) set forceMentioned=true to bypass GroupGate
- Merge dead note-filter tests into single 'skips todo authored by bot'
- Log fetchDescription errors to stderr instead of silent swallow
- Post error comment on issue/MR when handleInbound fails (best-effort)
* fix(channels/gitlab): always force isMentioned=true, remove regex re-derivation
The action_prompt_template config is already the event filter, and
GitLab has already decided the mention when creating the todo.
Re-deriving isMentioned via regex on todo.body causes permanent
message loss when the regex misses (description mention + fetch
failure, group mentions). Always set forceMentioned=true so
GroupGate never drops a todo that passed the template filter.
* fix(channels/gitlab): propagate fetchDescription errors for description mentions
For note mentions, description is metadata-only — fetch failure is
logged and swallowed. For description mentions, description IS the
message — fetch failure now propagates to the outer catch, which
posts the ⚠️ error comment so the user knows to re-mention.
* perf(channels/gitlab): clean up stale todos, skip unnecessary fetchDescription
- Mark stale todos (updated_at <= cursor) as done on each poll to
prevent perpetual re-fetching of pre-existing pending todos
- Skip fetchDescription for note mentions when template does not
contain %description%, saving one API call per todo
- Update docs: stale todo cleanup, error comment on failure
* docs(channels/gitlab): clarify requireMention is bypassed, template is the real filter
* Apply suggestions from code review
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* fix(channels/gitlab): use todo ID cursor instead of timestamp to eliminate equal-timestamp loss
Timestamp-based cursors (second granularity) could silently destroy
todos sharing the same updated_at as the cursor boundary. Switch to
monotonically increasing todo IDs which are unique and collision-free.
Add initialized flag to preserve first-start drain semantics: pre-existing
pending todos are marked done without dispatch on the first poll cycle.
* fix(channels/gitlab): harden first-poll drain, add ordering tests, fix lockfile
- Replace Math.max(...spread) with reduce to avoid RangeError on large
backlogs (~100k+ todos). Move initialized=true after the drain work so
any throw retries the drain instead of falling through to dispatch.
- Add unit tests: identical-timestamp delivery and id-order-when-updated_at-disagrees
(kills M2 sort mutant).
- Align lockfile: file:../base → ^0.21.0 for channel-base dep.
* fix(channels/gitlab): include dot in mention lookahead for GitLab usernames
GitLab usernames may contain dots (e.g. bot.name). The lookahead
character class inherited from GitHub omitted '.', causing @bot.name
to match as @bot. Add '.' to the negated class.
* docs(channels/gitlab): align docs with ID cursor and drain semantics
- Add first-poll drain as step 2 in How It Works
- Clarify GroupGate always passes (isMentioned forced true)
- Document initialized flag in Known Limitations
* Apply suggestions from code review
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* fix(channels/gitlab): align package version and channel-base dependency to 0.21.1
Bump version from 0.21.0 to 0.21.1 to match other channel packages after
upstream merge. Pin @qwen-code/channel-base to exact 0.21.1 instead of
^0.21.0, matching the convention used by other published channels.
* fix(channels/gitlab): regenerate lockfile to match package.json versions
Manually add only gitlab-related lockfile entries (workspace, @gitbeaker
packages, transitive deps, channel-gitlab link) without unrelated npm
normalization churn.
* test(channels/gitlab): add regression tests for first-poll drain hardening
Two tests that kill the M1 (Math.max spread RangeError) and M2 (flag
ordering) mutants which survived the original 46-test suite:
- 150k todo drain verifies reduce() handles large backlogs without
RangeError and without dispatching
- Drain throw verifies initialized stays false so the next poll retries
the drain instead of falling through to dispatch
Test file duration: ~40ms → ~170ms.
* docs(channels/gitlab): clarify groupPolicy must be "open" and add runtime warning
The default groupPolicy "disabled" silently drops all mentions — todos are
marked done and cursor advances, but no dispatch occurs. Fix misleading docs
that said "GroupGate always passes" (only true at groupPolicy: "open") and
add a connect()-time warning when groupPolicy is not "open".
* fix(channels/gitlab): correct xcase integrity hash in lockfile
The manually added xcase entry had a typo in the sha512 hash (ys → ks),
causing npm ci EINTEGRITY failures in CI.
* fix(channels/gitlab): correct requester-utils integrity hash in lockfile
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(channels/gitlab): allow groupPolicy "allowlist" in warning and docs
The groupPolicy warning and docs incorrectly stated that groupPolicy
must be "open". In reality "allowlist" with the project listed also
works because isMentioned is forced true and GroupGate only requires
the group to be listed. Also fix the inaccurate "no error is logged"
claim — ChannelBase logs preflight rejected reason=group_disabled.
Fixes R5-🟡3 from PR #7862 review.
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* feat(hooks): add security.allowPrivateNetworkHooks to bypass SSRF range checks for trusted scopes
HTTP hooks hard-block all private/link-local address ranges via ssrfGuard,
which makes them unusable in platform-managed environments where the hook
receiver is a first-party, VPC-internal endpoint (e.g. an internal API
gateway resolving to 172.16.0.0/12).
Add an opt-in setting, security.allowPrivateNetworkHooks (default false),
that skips the SSRF IP-range checks in urlValidator.isBlocked (literal IPs)
and validateResolvedHost (literal + post-DNS-resolution paths).
Security properties:
- Honored only from User/System/SystemDefaults scopes; the value is
stripped from Workspace settings during the merge (with a startup
warning), so a cloned repository can never self-grant the bypass.
- BLOCKED_HOSTS (169.254.169.254, metadata.google.internal, ...) remains
blocked even when the flag is on.
- Default false keeps every code path byte-for-byte compatible with
current behavior; bare/safe mode forces it off.
* fix(hooks): enforce metadata endpoint blocklist regardless of allowPrivateNetworkHooks
Address review findings on #7968: with the flag on, cloud metadata
endpoints were reachable through gaps in the relaxed checks.
- ssrfGuard: add METADATA_IPS (169.254.169.254, 100.100.100.200) and
isMetadataAddress(), which normalizes IPv4-mapped IPv6 forms
(::ffff:a9fe:a9fe, ::ffff:6464:64c8, ...) via the existing
extractMappedIPv4/expandIPv6Groups helpers.
- urlValidator.isBlocked: BLOCKED_HOSTS matching and the literal-IP
isMetadataAddress check now run unconditionally; only the general
range check (isBlockedAddress) is relaxed by the flag.
- httpHookRunner.validateResolvedHost: no longer returns early with the
flag on — DNS resolution still runs and resolved addresses are checked
against isMetadataAddress, so a hostname resolving to a metadata
endpoint is blocked. DNS failures still defer to fetch, as before.
- settings warning text now lists User/System/SystemDefaults, matching
the schema and docs.
- docs: precise wording — the flag relaxes only range checks; metadata
endpoints stay blocked in all serialized forms and after DNS resolution.
The flag now opens RFC1918/CGNAT/link-local ranges only; cloud metadata
endpoints (169.254.169.254, 100.100.100.200 in any form, plus the
BLOCKED_HOSTS hostnames) are unreachable in every configuration.
---------
Co-authored-by: 欢伯 <ri.xur@alibaba-inc.com>
* fix(serve): allow bounded reads of large text files
* fix(serve): bound large-text reads by scan cost, not by which knob was set
Follow-up to the bounded large-text read path. Three changes:
Gate on any explicit window argument, not on `limit`. Gating on `limit`
had the cost model backwards in both directions: `{ line: 900_000_000,
limit: 20 }` was admitted despite walking the whole file, while
`{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A
read with no window argument at all still fails, since a caller that
believes it holds the whole file may write it back truncated.
Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read
returns; nothing capped what it cost. Line offsets are resolved by
scanning from byte 0, so a query param could turn into an
uninterruptible multi-second scan of an arbitrarily large file — and on
Windows hold a read handle for that span, blocking renames and deletes.
Past the budget the read is refused with `file_too_large` pointing at
readBytes, which reaches any offset in O(1).
Tolerate appends on streamed windows. Requiring whole-file size/mtime
stability after reading a prefix rejected reads whose returned bytes
were still valid, and the case it rejected — tailing a live log — is the
one this path exists for. Streamed windows now assert inode identity
plus "did not shrink"; truncation and replacement are still rejected.
Also: non-UTF-8 large text now returns `binary_file` rather than
`file_too_large`, so a client retrying on 413 with a smaller window
can't loop forever; and `readFileWithLineAndLimit` throws instead of
silently ignoring a caller-supplied `fileHandle` on the by-path
fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(serve): harden large text range snapshots
Treat caller-owned file handles as bounded streaming reads, cap them to the captured file size, and reuse the chunk buffer.
Restore strict Serve snapshot stability and align returned-slice metadata with the full-snapshot path.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): make large text ranges snapshot-safe
* fix(serve): harden large-text ctime tests and document buffer reuse (#7947)
Address review feedback on the large-text range read PR:
- Pause before restoring mtime in the two ctime-dependent mutation tests so the change-time advances past the pre-read snapshot even on coarse-resolution filesystems, removing a latent flake in the same-size-overwrite precondition. The assertions are unchanged.
- Document at the readFileHandleChunks yield site that the 512 KiB buffer is reused across iterations, so yielded views must be decoded or copied before advancing the generator.
* docs(serve): soften same-size rewrite guarantee to coarse-clock best-effort (#7947)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
* 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(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.
* 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>
* docs: add design placeholder for subagent model grade selection (#7685)
* feat(core): add subagent model grade selection
* test(subagent): cover resolveModelGrade deep guards and resume else branch
- subagent-manager: add tests for non-string grade values, blank values,
array-shaped modelGrades, and missing modelGrades (all return undefined)
- background-agent-resume: assert configured subagent model is preserved
(not forced to 'inherit') when launch flags (model + authType) are absent
Addresses test-coverage review findings.
* refactor(subagent): extract normalizeModelGradeSettings and merge model validate
- Extract normalizeModelGradeSettings helper shared by resolveModelGrade
and the Agent tool schema build, so the advertised grades and runtime
resolution cannot drift (addresses duplicated shape invariant).
- Merge the three model-parameter validate branches under a single
`params.model !== undefined` guard.
- Update agent.test.ts mock to preserve the real helper while still
mocking SubagentManager.
* refactor(core): simplify model grade resolution
* fix(core): reject unknown model grades
* docs(core): clarify model grade precedence
* docs: explain subagent model grades
* test(core): update subagent manager mock
* fix(core): list available model grades
* fix(core): trim model grade keys and cover schema removal
Grade keys were checked for emptiness via grade.trim() but stored in the
map and advertised in the tool schema enum untrimmed, while values were
trimmed. A padded key like ' small ' published a padded enum name the
model had to reproduce verbatim, and the allowlist check silently
excluded it. Normalize the key before storing, allowlist matching, and
schema publication.
Also adds a test for the delete schema.properties.model branch that fires
when grades transition from available to empty, so a regression that
breaks the delete leaves no stale model enum in the tool schema.
* fix(core): trim allowed model grade filters
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* fix(channels): use username as senderId in GitHub adapter to fix allowlist gate
isAuthorizedForSharedSessionTarget compares config.allowedUsers (logins)
against envelope.senderId — but senderId was a numeric ID resolved via
getByUsername, so every allowlisted user was rejected from /who, /clear,
/status, /loop, and channel memory commands.
Fix by using user.login as senderId throughout, assuming GitHub users
don't change usernames. This also removes the getByUsername resolution
step that made connect() non-idempotent on daemon reconnect.
- Remove botUserId field; bot self-filter uses botUsername
- Remove allowedUsers login-to-ID resolution in connect()
- Pass config.allowedUsers logins directly to gate
- senderId in envelopes uses user.login
* fix(channels): normalize allowlist/senderId to lowercase for case-insensitive matching
GitHub logins are case-insensitive but Set.has/Array.includes are not.
Without normalization, allowedUsers: ['Alice'] silently rejects a
commenter whose canonical login is 'alice' — a regression from the old
getByUsername round-trip which normalized casing implicitly.
- Normalize config.allowedUsers and gate to lowercase in connect()
- Lowercase senderId at both envelope assignment sites
- Remove dead != null guard in bot self-comment filter
- Add case-insensitive gate test and connect idempotency test
- Add senderId/allowedUsers comparability guard to dispatch test
- Document username-based allowlist rename risk in security section
* feat(skills): add overridable default-disabled state
* fix(skills): address review feedback on default-disabled PR (#7357)
- Fix disabledChanged comparison in SkillsManagerDialog to use
previousDisabled (locked names filtered) instead of workspaceDisabled,
preventing spurious settings writes when a skill is disabled at both
workspace and higher scope
- Import SettingScope as a value instead of string-casting literals in
skill-settings.ts for compile-time safety
- Add dual-key change test: enabling a workspace-hard-disabled
default-disabled skill produces both skills.disabled and
skills.enabled changes in one operation
- Add legacy inactive-extension branch tests: reject when
disabledReason is undefined and skill is not in settings
disablements; allow when it is disabled by settings
* fix(cli): address skills picker review feedback (#7357)
Extract the skills picker's workspace persistence computation into a tested pure function so orphaned workspace disables (skills not currently loaded) are explicitly preserved and pinned by a regression test. Also add an integration test asserting a workspace-scope hard disable surfaces disabledReason 'hard' through the full loadSettings -> resolveSkillSettings -> mapSkillConfigToStatus pipeline.
* fix(cli): resolve skill disablements in safe mode for status API (#7357)
* fix(cli): dynamically import skill-settings in serve to keep fast-path closure clean (#7357)
---------
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
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 Autofix <qwen-code-autofix[bot]@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
The live journal (DAEMON-009) caps were too conservative for real-world
agent turns: 2000 events / 2 MiB caused 79% event loss on a typical
long turn (9647 events). Raise defaults to 10 000 events / 8 MiB and
expose them as --max-journal-events / --max-journal-bytes CLI flags,
following the same config path as --compacted-replay-max-bytes.
Also fix stale docs that described the liveJournal as uncapped.
* fix(core): fire StopFailure hook on loop detection early returns (#7588)
When loop detection (always-on safety or heuristic) terminates a turn
early via `return turn`, the Stop hook code after the streaming loop
was never reached. StopFailure hooks were only fired from the CLI
layer for API errors, not from client.ts for loop detection.
Added `loop_detected` to StopFailureErrorType and fire the
StopFailure hook via MessageBus before each loop detection early
return, so cleanup/notification hooks run regardless of how the
turn ends.
All 284 client tests and 682 hook tests pass.
* fix(core): use direct hookSystem call for loop-detection StopFailure (#7588)
The MessageBus bridge has no StopFailure case, so the hook never
executed. Switch to config.getHookSystem()?.fireStopFailureEvent()
(matching the CLI's API-error path), make it fire-and-forget per the
StopFailure contract, drop the stale last_assistant_message that
carried the previous turn's text, deduplicate via a private helper,
update docs with loop_detected, regenerate the settings schema, and
add regression tests for both loop-detection paths.
* test(core): add negative-path test for StopFailure hook disable guard (#7592)
* test(core): add negative-path tests for StopFailure hook guards (#7592)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.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: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* feat(channels): add GitHub polling adapter with notification-as-wakeup architecture
Introduce a GitHub channel adapter that monitors notifications and
responds to @mentions on issues/PRs by posting comments. Uses
last_read_at as a per-thread watermark for comment enumeration,
replacing the unreliable latest_comment_url approach.
Foundation changes to ChannelBase:
- sendThreadMessage for thread-targeted delivery (IM adapters unchanged)
- Envelope.metadata appended to prompt after command parsing
- chat_thread session scope (channel:chatId:threadId) prevents
cross-repo session collision
- polling-helpers: testBotMention/stripBotMention (separate detection
from stripping, no whitespace collapsing), cursor persistence,
abortableSleep
GitHub adapter design:
- Notifications as wake-up signals only (unread filtering)
- listComments enumeration with last_read_at watermark
- Bot self-comment filtering, case-insensitive mention regex
- In-memory recentlyProcessed set for mark-read failure dedup
- First-contact: new issue body @bot triggers processing
- Error comment + cursor advance on handleInbound failure
- pollInterval minimum 60s, exponential backoff 2s-30s
* refactor(channels): extract PollingChannelBase from polling-helpers
Replace the loose polling-helpers module with a PollingChannelBase<Cursor>
abstract class that encapsulates the poll loop, cursor persistence (JSON,
atomic write), exponential backoff, and start/stop lifecycle. Subclasses
implement only pollOnce() and createInitialCursor().
- Delete polling-helpers.ts (cursor fns + abortableSleep moved into base)
- Move mention utilities (testBotMention/stripBotMention) to github pkg
- GithubAdapter now extends PollingChannelBase<{ lastProcessedAt }>
* fix(channels): remove Gitea/GitLab mention from sendThreadMessage JSDoc
* fix(channels): match /pulls/N in notification subject URL
GitHub PR notifications use /repos/{owner}/{repo}/pulls/{N} in
subject.url, not /issues/{N}. The regex only matched /issues/,
causing PR notifications to be skipped and marked read.
Also sets threadId to 'pr:N' for PRs (was always 'issue:N').
* test(channels): add PR body first-contact unit test
Verify that PR notifications with @mention in the body (not a comment)
correctly trigger the first-contact path: extractFromSubjectUrl matches
/pulls/N, listComments returns empty, tryFirstContactBody fetches the
PR body and dispatches to handleInbound with threadId 'pr:N'.
* feat(channels): read pollInterval from channel config in PollingChannelBase
Move pollInterval config reading from GithubAdapter to the base class.
The user's configured pollInterval in settings.json is now respected
directly without a minimum enforcement. Defaults to 60000ms when not
configured.
* fix(channels): prepend metadata before prompt text
Agent sees issue/PR context (type, title, URL) before the user's
request, improving comprehension. Metadata is still appended after
slash-command parsing so commands are not affected.
* refactor(channels): route all ChannelBase delivery through sendThreadMessage
Replace all internal sendMessage calls with sendThreadMessage, passing
envelope.threadId (or target.threadId / undefined) so polling adapters
can deliver to the correct thread. IM adapters are unaffected — the
default sendThreadMessage falls through to sendMessage.
* docs(channels): document sendThreadMessage delivery architecture
* fix(channels): address review findings
- Cap recentlyProcessed Set at 10k entries to prevent unbounded growth
- Validate cursor JSON shape (non-null object) in loadCursorFromDisk
- sendThreadMessage falls through to sendMessage when threadId is
undefined instead of silently dropping
- Remove duplicate pollInterval from GithubConfig (now in ChannelConfig)
- Fix chat_thread routing key trailing colon when threadId is undefined
* docs(channels): fix metadata JSDoc — prepended, not appended
* fix(channels): use recentlyProcessed dedup for first-contact body
Replace the fragile createdAt-vs-cursor check in tryFirstContactBody
with the recentlyProcessed set. The cursor advances globally based on
notification updated_at — when a different notification with a later
updated_at is processed first, the cursor can advance past the issue's
created_at, causing the first-contact check to incorrectly skip the
issue body (forget reply bug, found in E2E TC-2b).
* refactor(channels): two-layer dedup for GitHub adapter
Layer 1: global cursor filters notifications by updated_at (sorted
ascending, old first). Layer 2: server-side last_read_at filters
comments by created_at (sorted ascending).
- Delete recentlyProcessed Set (no longer needed)
- Sort notifications by updated_at ascending before processing
- Sort comments by created_at ascending before processing
- Pass latest comment created_at to markThreadAsRead as last_read_at
* fix(channels): address review findings on GitHub adapter
Blockers:
- sessionScope: add defaultSessionScope to ChannelPlugin, apply in
parseChannelConfig so router and adapter agree on 'chat_thread'
- channel-registry.test.ts: add 'github' to expected type list
Should-fix:
- Replace per-thread markThreadAsRead (PATCH) with bulk
markNotificationsAsRead (PUT /notifications + last_read_at).
API errors stop the batch without marking failed notifications
read; handleInbound errors still advance (error comment posted).
- connect() throws on bot identity failure instead of failing open
- metadata appended after promptText (inside sender attribution)
- isSharedSessionTarget includes 'chat_thread' scope
Nits:
- startPollLoop re-entrancy guard
- clean-package-build-artifacts.js includes github
- index.ts re-exports GithubChannel
* fix(channels): use max updated_at of all fetched notifications as last_read_at
Prevents re-fetching the same notifications in the next poll cycle.
The bulk PUT /notifications marks all fetched notifications as read
up to the max updated_at, regardless of per-notification success.
* fix(channels): address review round 2 findings
- #12: loadCursorFromDisk rejects arrays
- #13: pollInterval validates positive finite number
- #19: first-contact gate uses dispatchedMention flag (not newComments.length)
- #25: stripBotMention no longer trims (preserves indentation)
- #27: remove adapter-level requireMention, unify on GroupGate
- #31: add chat_thread SessionRouter routing key tests
- #33: clear metadata on collect-mode synthetic envelope
- #35: fix PollingChannelBase.test import path
- #36: add @octokit/rest to 15-channel-adapters.md dependencies
* docs(channels): document known limitations for GitHub adapter
- First start skips existing unread notifications (cursor = now)
- Requires classic PAT (fine-grained PATs lack notifications API)
- PR review comments not enumerated (issue comments only)
* fix(channels): address review round 3 findings
- #9: buildMetadata derives web URL from baseUrl (GHE support)
- #12: sendThreadMessage throws on invalid threadId format
- #19: mention lookbehind matches cc:@bot and "@bot" patterns
- #23: cursor file name uses sha256 hash to prevent collision
- #26: test verifies cursor persistence to disk
- #31: postErrorComment double-failure logs to stderr
- #45: tests use mkdtempSync isolation instead of real QWEN_HOME
* fix(channels): pass threadId through pairing flow + sendResponseMessage test
- #13+16: onPairingRequired receives envelope.threadId and passes it
to sendThreadMessage, so pairing codes are delivered on threaded
channels (GitHub) instead of throwing
- #6: add test verifying sendResponseMessage resolves threadId from
router.getTarget and passes it to sendThreadMessage
* fix(channels): pass proxy to Octokit for daemon-worker environments
- #44: read this.proxy from ChannelBaseOptions and pass
HttpsProxyAgent to Octokit request.agent, matching the
Telegram adapter pattern
* fix(channels): address review findings — immutable senderId, comment time window, validateCursor, retry wrapper
- senderId uses immutable user.id; allowedUsers resolved to IDs at connect
- Comment filter upper bound: updated_at <= maxUpdatedAt (batch window)
- Per-notification errors use continue (best-effort), not break
- validateCursor() virtual hook for subclass cursor shape validation
- sendThreadMessage/postErrorComment wrapped in githubApi() retry
- webOrigin handles default api.github.com → github.com
- Docs: classic PAT only, markNotificationsAsRead, dedup claims removed
- Tests: threadId priority, metadata consumption, defaultSessionScope,
QWEN_HOME isolation, persistent mock rejection
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(channels): mark notifications read before processing to prevent duplicate replies
Bot's own replies bump notification updated_at past the pre-captured
maxUpdatedAt, so markNotificationsAsRead(maxUpdatedAt) failed to mark
them read — the next poll re-fetched the same comments and replied
again.
Move markNotificationsAsRead + cursor advance before the processing
loop (best-effort delivery). This is safe because bot's own comments
do not flip notifications back to unread. Update docs to reflect the
new poll cycle order and best-effort semantics.
* fix(channels): update sender gate after allowedUser ID resolution and harden tests
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(channels): cursor-based comment window to prevent duplicate replies
PUT /notifications is async (202) with a last_read_at cutoff — the
bot's reply bumps updated_at past the cutoff before the server
processes the mark, so the notification is never marked read and gets
re-fetched on the next poll, causing duplicate replies.
Use the cursor value before advancement as an exclusive lower bound
for the comment enumeration window: (windowSince, maxUpdatedAt].
Comments already eligible in a previous poll are excluded regardless
of whether the mark succeeded. Zero new persistent state.
* fix(channels): cursor-based comment window to prevent duplicate replies
PUT /notifications is async (202) with a last_read_at cutoff — the
bot's reply bumps updated_at past the cutoff before the server
processes the mark, so the notification is never marked read and gets
re-fetched on the next poll, causing duplicate replies.
Use the cursor value before advancement as an exclusive lower bound
for the comment enumeration window, with per-notification last_read_at
as the preferred lower bound when available (server-side per-thread
watermark). Comments already eligible in a previous poll are excluded
regardless of whether the mark succeeded. Zero new persistent state.
* fix(channels): address review findings — null guard, cursor validation, metadata dedup, abortable sleep, docs
- Guard against null notification.subject.url in pollOnce
- Validate lastProcessedAt is a parseable date in validateCursor
- Add metadata: undefined to second collect-mode drain path
- Refactor abortableSleep as protected method on PollingChannelBase
- Fix docs: requireMention is nested under groups.*
- Add tests: chat_thread shared session, dispatchedBodies eviction,
cursor enumeration window, last_read_at in mention tests
* docs(channels): sync docs with implementation — cursor shape, error handling, GitHub adapter tables, first-contact
- Design doc: update Cursor to { lastProcessedAt, dispatchedBodies? }, add
validateCursor date check, abortableSleep protected method, break-on-error
semantics, subject.url null guard
- Developer docs: add GitHub to adapter table and adapter matrix
- User guide: add first-contact step to How It Works, clarify mark-before-process
* fix(channels): address review round 2 — error dedup, abortable retry, backoff reset, window test
- Record dispatchedBody on first-contact handleInbound failure to prevent
duplicate error comments when mark-read async hasn't taken effect
- Use abortableSleep instead of raw setTimeout in githubApi retry so
disconnect() can interrupt rate-limit cooldowns
- Reset consecutiveErrors in startPollLoop so stop/restart cycles don't
inherit stale elevated backoff
- Add test for cursor window client-side lower-bound exclusion filter
* fix(channels): address review round 3 — cursor validation, error dedup, sender gate, bot-self body
- validateCursor: normalize falsy non-array dispatchedBodies (false/0/""/null)
to [] instead of passing them through to .includes() which throws TypeError
- Set dispatchedMention after postErrorComment to prevent first-contact from
posting a duplicate error comment on the same thread
- Only set dispatchedMention when the sender passes the sender gate, so a
disallowed commenter's mention no longer suppresses a valid first-contact
body from an allowed issue author
- Skip bot-authored issue bodies in tryFirstContactBody to prevent
self-response loops under open sender policy
* fix(channels): address review suggestions — test coverage, cursor filename, assertion precision
- Pairing flow: add threadId pass-through regression test
- pollInterval: add table-driven edge cases (0, -1, NaN, Infinity, string)
- Add null-URL notification followed by valid notification batch test
- Fix comment window test to assert paginate call 3 (listComments) not call 2
- Truncate cursor filename encoded prefix to 200 chars (filesystem 255 limit)
- Assert mark-read uses batch maxUpdatedAt, not just { read: true }
- Assert real GitHub plugin declares defaultSessionScope chat_thread
- Add invocationCallOrder assertion for mark-before-process ordering
* fix(channels): address review round 4 — allowedUsers throw on resolve failure, crash table fix, mark-read failure test
* fix(channels): address review round 5 — created_at filter, retry-after NaN guard, retry/sendThreadMessage tests, docs fixes
* fix(channels): address ci-bot review 4778587403 — reconnect idempotency, github type enumerations, retry/webOrigin tests
* chore(channels): align channel-github version to 0.21.0 after upstream merge
* chore(channels): update package-lock.json for channel-github 0.21.0
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: OrbitZore <orbitzore@users.noreply.github.com>
* feat(review): add comment-status helper for existing-thread triage
One deterministic pass over a PR's existing inline comments, replacing
the per-comment `gh api` fetches the orchestrating model used to make
during /review: anchor validity at the live head (outdated detection,
with a file-level exemption), whether the anchored file changed in the
reviewed worktree since each comment's commit and which commits touched
it (the re-check's candidate "fixed by" list), reply participation and
PR-author response, the blocker signal (same carriesBlockerSignal as
pr-context, so the two surfaces agree by construction), and
worktree-vs-live head drift.
Measured on a heavily discussed PR (72+ inline comments), a single
review run burned 20+ model turns re-deriving exactly these fields one
comment id at a time. SKILL.md now runs the subcommand in Step 1 and
routes the Step 6 re-check's status questions at the report; comment
bodies stay in the pr-context file under its untrusted-data preamble,
and a comment-status failure only warns — it is an index, not the
evidence, so it never sets the context-unavailable state.
* test(review): add comment-status to the subcommand registry expectations
* fix(review): comment-status review follow-ups — size warning, --host wiring, scope clauses
Addresses the review at d098e4feb:
- High: warn when the report exceeds read_file's truncation threshold,
mirroring pr-context — measured 53k chars on the benchmark PR, where a
single read lost 36 of 71 threads (24 blocker-flagged) and the cut JSON
did not parse. The warning points at jq first: the file is
machine-shaped in a way the Markdown context file is not.
- Medium: thread --host through — the SKILL.md command block now says to
pass it (each subcommand is its own process, so a host set elsewhere
cannot carry over), and comment-status joins the host-required lists in
SKILL.md and the code-review docs.
- Minor: Step 6's routing sentence now states both scope limits — the
report exists only when Step 1 wrote it (worktree mode), and it indexes
inline threads only; issue-/review-level blockers keep the context-file
walk.
- Minor: touchedByTotal exposes the real commit count behind the capped
touchedBy list, so a cut list is visible instead of reading as "the fix
is not among them".
- Nits: drop the never-read `side` field; guard authorReplied against a
deleted-author/deleted-replier '' === '' match; correct the force-push
doc comment (a shared object database usually retains the old commit,
so the range widens — fail-safe — rather than going unknown).
* fix(review): comment-status second-round follow-ups — CWD-safe pathspec, shared walk, stale flag
Addresses the LGTM-with-suggestions round:
- The git probe's pathspec is anchored with `:(top)`: run from a
subdirectory of the worktree, the old CWD-relative form returned empty
output with exit 0 and every thread read as "untouched since the
comment" — silent, and pointing the one direction this index must not
fail in. A real-git integration test now drives the probe from the
repo root AND a subdirectory (plus the cap, the memo, and the
missing-commit gate — none of which the injected-probe unit tests
could see).
- findRootId is imported from pr-context (made generic and exported)
instead of duplicated — the thread walk now agrees by construction,
like the blocker signal already did.
- Head drift is denormalized onto every thread as code.staleWorktree, so
a jq consumer of threads[] cannot skip the top-level flag by
construction.
- Commit existence is memoized per SHA (it never depended on the path),
halving git spawns on thread-heavy PRs; summarizeThreads gets a named
return interface like every other exported shape here.
- DESIGN.md gains the missing section: why comment-status is a separate
subcommand and why the second fetch of pulls/{n}/comments is
deliberate (process boundary — pr-context must stay pure-API for
lightweight mode; this one exists to join API facts with worktree git).
* fix(review): harden comment-status against untrusted-PR inputs
Addresses the security review round:
- Symlink --out: the command now runs from the trusted main checkout (so
a relative --out cannot be redirected through a symlink an untrusted PR
planted in its own worktree) and scopes its git queries to the worktree
with `git -C <worktreePath>`, which it locates itself. SKILL.md no
longer cd's into the worktree for it. The report lands in the main
checkout's .qwen/tmp alongside every sibling report.
- Non-ancestor comment commit: after a force-push the anchor commit can
survive in the shared object store without being on HEAD's history, so
`sinceSha..HEAD` is empty and a changed file reads as changed:false —
the one direction this index must not fail in. A single
`merge-base --is-ancestor` gate now returns 'unknown' for a
non-ancestor OR a missing commit, replacing the cat-file existence
check (one git process instead of two).
- Literal pathspec: the GitHub-supplied path is passed as
`:(top,literal)<path>` so a value like `:(exclude)a.ts` cannot be read
as pathspec magic and inspect unrelated files.
- Fetch-race drift: the live head is sampled before AND after the
comments fetch; a push landing mid-fetch (which would pair newer
anchor mappings with a stale comparison) is now detected, recorded as
both samples, and warned on distinctly from ordinary worktree lag.
- pr-context renders the root comment id in the Open and Already-discussed
sections, giving Step 6 a stable join key back to comment-status's
per-thread rootId (the blocker renderer already did this).
- Handler-level tests (mocked gh/git/fs) for the three drift outcomes,
plus real-git integration cases for the non-ancestor gate and the
literal pathspec. Multi-page pagination is a non-issue: gh api
--paginate merges top-level arrays into one (verified on the live
93-comment PR).
* fix(review): comment-status round 3 — precise staleWorktree, worktree-missing warning, discriminating pathspec test
Addresses three Suggestions:
- staleWorktree is now keyed on worktreeStale alone, not the headDrift
union. A head that merely moved between the two samples while the
worktree already matches the final head is NOT a superseded checkout,
so its threads no longer carry staleWorktree:true against the field's
documented meaning. headMovedDuringFetch stays a separate top-level
flag + warning.
- A missing worktree (comment-status run before fetch-pr or after
cleanup) now sets worktreeMissing on the report and prints a warning —
previously every thread degraded to code:'unknown' silently, readable
as "nothing changed".
- The literal-pathspec test now uses a discriminating pathspec
(`:(glob)pkg/**`): magic would match the changed file (true), literal
is a nonexistent filename (false), so asserting false actually fails if
the `:(top,literal)` prefix is dropped — the old `:(exclude)…` read
false under both interpretations. A plain-path control proves the probe
is live.
* test(review): make the comment-status negation test actually exercise negation
The body 'No blockers here' matched no BLOCKER_PATTERN (the bare plural
never triggers /\bblocking\b/ etc.), so isBlocker returned false before
the negation branch ran — false coverage. It now uses 'No blocking
issues', which matches the signal and must be suppressed by the leading
'No', plus an un-negated control that asserts true.
* test(review): assert per-thread staleWorktree and --host wiring; document ghApiAll merge contract
Two test gaps + one recurring-review clarification:
- The worktree-lag drift test now includes a thread and asserts
staleWorktree:true is denormalized onto its code object — the old
positive case had zero threads, so the denormalization loop iterated
over nothing and would pass even if the block were deleted.
- A --host test now asserts setGhHost is called with the argv host,
matching the presubmit analog; without it a dropped setGhHost would
silently target github.com for a GHE review.
- ghApiAll's doc now explains why one JSON.parse is correct on multi-page
output: gh --paginate MERGES top-level arrays into one (it does not emit
one array per page); the per-page-concat failure only affects
key-nested arrays, which is exactly why ghApiAllNested exists. Verified
on a 4-page 97-comment response.
* fix(review): comment-status degrades gracefully on failure per its SKILL.md contract
SKILL.md promises this command is an index, not evidence — "if it fails
(auth, network), warn and continue" — but runCommentStatus had no
try/catch, so an ensureAuthenticated() or gh throw propagated as an
unhandled rejection and killed the whole review. The runtime body is now
wrapped: any throw writes a minimal empty report ({prNumber, ownerRepo,
error, threads: []}) so downstream jq still parses, prints a
"comment-status failed" warning, and exits 0. Handler test pins the
auth-failure path (no throw, empty report, warning). Reported by
@yiliang114.
* test(review): pin comment-status owner_repo guard and truncation-size warning
Two untested paths flagged in review: the owner_repo-without-slash guard
(a caller error that must still throw, distinct from the runtime
graceful-degradation path) and the report-size warning that fires when
the JSON crosses read_file's truncation threshold.
* fix(review): comment-status degraded report carries the full shape, not a stripped one
The graceful-degradation path wrote { prNumber, ownerRepo, error,
threads: [] }, omitting headDrift/summary/headMovedDuringFetch/etc. A
consumer reading report.headDrift then got undefined (falsy = "no
drift"), silently mistaking a total index failure for a clean "nothing
moved" — and the review orchestrator keys code-fact warnings on exactly
that field. The degraded report now emits the same shape as the success
report with safe defaults plus `error`, so a consumer that checks `error`
sees the failure and one that reads a fact gets a neutral value, never a
misleading one. Reported by @doudouOUC.
* fix(review): degraded report's worktreeMissing must not contradict worktreeHeadSha: null
The catch-block report hardcoded worktreeMissing: false beside
worktreeHeadSha: null — a positive "worktree present" assertion the
success path (worktreeMissing = worktreeHeadSha === null) would never
make for a null head. A consumer reading worktreeMissing without gating
on error would conclude the worktree exists on a run where nothing is
known. Now true, matching the null head and the fail-safe reading (code
facts unavailable). Reported by qwen-code-ci-bot.
---------
Co-authored-by: verify <verify@local>
* feat(core): add fork_turns to subagents
* fix(core): preserve nested agent context inheritance
* fix(core): isolate inherited subagent history
* refactor(core): scope fork_turns to fork agents
* test(core): cover zero-real-turns branch in selectForkHistory
Add a regression guard asserting selectForkHistory returns [] when a
numeric fork window finds no real user turns after the synthetic prefix
(e.g. only startup context present). This pins the
realUserTurnIndexes.length === 0 branch so a future refactor cannot
silently return the full history instead of an empty selection.
* docs(core): address fork_turns review feedback
- Explain the curated vs uncurated history split between the fork_turns
'all' and numeric paths in createForkSubagent.
- Document why includeCompressed is load-bearing in selectForkHistory.
- Gate the 'forks inherit ...' prose in the Writing-the-prompt section
behind isForkSubagentEnabled so non-interactive sessions no longer
advertise fork behavior, and lock it with description assertions.
* test(core): cover fork_turns 'all' and getHistoryForForkWindow fallback
Add two integration tests for prepareForkConfig fork-history selection:
- 'all' path: verify getHistoryShallow(true) sources the curated history
and selectForkHistory(history, 'all') seeds the fork with the full
history verbatim.
- numeric path: verify the getHistoryForForkWindow?.() ?? getHistory(true)
fallback still produces a correct bounded window (startup + latest real
turn) when getHistoryForForkWindow is unavailable.
* fix(core): use uncurated history for fork bounded-window fallback
The numeric fork_turns path falls back to geminiClient.getHistory(true)
when getHistoryForForkWindow is unavailable. Curated history coalesces
the leading startup reminder into the first real user turn, so
getStartupContextLength can no longer detect it as a pure prefix.
selectForkHistory then leaves the startup text embedded in the first
selected turn while the startupContext prefix is prepended separately,
duplicating startup context in the fork's initial messages.
Fall back to uncurated getHistory() instead, which keeps the startup
reminder as its own pure entry that selectForkHistory strips cleanly.
Update the fallback-path test to assert the uncurated call and document
why curated history is unsafe here.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(channels): exclude discrete messages from replies
* feat(serve): make ACP initialize handshake timeout configurable
Add --initialize-timeout-ms CLI flag to qwen serve, wiring it through
to BridgeOptions.initializeTimeoutMs. The ACP initialize handshake
defaults to 10 s (DEFAULT_INIT_TIMEOUT_MS); containerized deployments
where the child process needs longer can now raise the ceiling without
patching the source.
Fixes#7244
* fix(serve): wire initializeTimeoutMs to fast-path parser and embed bridge
Add the missing NUMBER_OPTIONS entry in fast-path.ts and forward
initializeTimeoutMs in the server.ts inline createAcpSessionBridge
call so the direct-embed / test path also respects the flag.
* fix(serve): address review — fast-path test, timer upper bound, revert #7223, docs (#7246)
* test(cli): add happy-path propagation test for initializeTimeoutMs (#7246)
* refactor(cli): reuse isPositiveIntegerMs for initializeTimeoutMs validation (#7246)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
* feat(web-shell): add git commit history browser
Add a read-only Git Log dialog to the Web Shell, accessible via /log
command or the History tab in the Changes dialog.
Full-stack implementation across core, daemon, SDK, and web-shell:
- core: fetchGitLog (paginated commit list) and fetchGitCommitDetail
(message body + per-file numstat) with 12 integration tests
- daemon: GET /workspace/git/log and /workspace/git/log/commit routes
with bound + qualified dual registration
- SDK: DaemonGitLog/DaemonGitCommitDetail types and client methods
- web-shell: GitLogDialog with commit list, expandable details,
SHA copy icon, Load more pagination, and Changes/History tab
switching in both dialogs
* fix(web-shell): address review feedback on git log browser
- Critical: use first-parent diff for merge commits (diff-tree without
-c or explicit parent outputs nothing for merges)
- Remove dead embedded prop from GitDiffDialog and GitLogDialog
- Replace span role=button with aria-hidden for copy icon (a11y)
- Add loadMore error feedback instead of silent catch
- Refresh relative timestamps every 60s (useState + interval)
- Remove dead branch param from subtitle i18n call
* fix(web-shell): address R2 review feedback on git log browser
- Use bounded split in parseLogFields (first 7 separators) to prevent
subject containing literal \x1f from shifting the parents field
- Add SHA hex format validation at daemon route layer (400 for invalid)
- Add rendering branch for detail.available === false (error message
instead of empty content)
* fix(web-shell): count renamed files in commit detail + test git-log route & dialog
Follows the R2 review-feedback commit (which fixed the bounded parse, the
non-hex SHA 400, and the unavailable-detail render). Remaining items:
- Commit detail counts renamed files. diff-tree is plumbing and does not
honour diff.renames, so a `git mv` split into a delete + add pair (or an
empty-path entry) instead of one file — understating filesCount /
linesAdded / linesRemoved. Run diff-tree with -M and give the inline
numstat parser the same pending-rename state machine as parseGitNumstat,
so a rename is one file keyed by its new path. Covered by a real-repo
rename test, plus a merge-commit test that locks the first-parent diff.
- Tests for the two previously-untested modules: the workspace-git-log
route (list shape, pagination clamping, sha-required + non-hex 400, trust
gating) and GitLogDialog (all five list state paths, load-more offset +
error, detail expand + both failure branches incl. available:false, and
the relative-time render).
* fix(web-shell): address R3 review feedback on git log browser
- Fix timeAgo '0y ago' for commits ~360-364 days old (Math.max(1, ...))
- Add .catch() to clipboard writeText to prevent unhandled rejection
- Reset loadMoreError on initial re-fetch (daemon reconnect)
- Preserve prev.available in loadMore merge instead of overwriting
- Add ARIA tab semantics (role=tablist/tab, aria-selected) to both
Changes and History tab bars
* fix(web-shell): address R4 review feedback on git log browser
- Fix vacuous limit-clamp test: seed 3 commits, verify limit=2 returns
2 + hasMore, limit=0 clamps to 1
- Add CSS var fallbacks for --subtle-bg and --success-bg (dialog
portals outside App.module.css scope)
- Extract GIT_DIALOG_SWITCH_DELAY_MS constant with doc comment
- Make copy-SHA control keyboard accessible (tabIndex, onKeyDown,
aria-label instead of aria-hidden)
* fix(web-shell): escape apostrophe in worktree welcome string
* fix(web-shell): address git history review feedback
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(web-shell): mock git diff content in app tests
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(core): harden git log metadata parsing
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(channels): scope pairing and allowlist state by workspace
PairingStore keyed its on-disk files by channel name alone, under the
global ~/.qwen/channels/ directory. Two workspace-scoped channel
configurations using the same channel name therefore shared pairing
requests and allowlist entries: a sender approved for workspace A was
implicitly approved for workspace B — an authorization-boundary
violation in multi-workspace daemon deployments.
PairingStore now takes the channel's workspace cwd and stores state
under channels/<basename>-<sha256[:12]>/, ChannelBase passes
config.cwd, and the pairing CLI commands gain a --cwd option
(defaulting to the current directory) so list/approve address the same
workspace-scoped store the channel worker uses.
Migration is a conservative one-time grandfather: on first scoped use,
existing legacy global files are COPIED into the scope (so
already-approved senders stay approved and other workspaces can
grandfather the same baseline later), after which the stores diverge —
no ongoing cross-workspace sharing, and legacy content can never
overwrite scoped state.
Fixes#7017
* fix(channels): canonicalize scope identity and gate migration per directory
Address the review findings on #7065:
1. Scope identity now follows the repo's workspace-canonicalization
contract: getWorkspaceScopeDirName realpaths the resolved path (with
the same ENOENT fallback as acp-bridge's canonicalizeWorkspace, which
channel-base mirrors locally to stay dependency-free). Symlinked and
platform-case-variant spellings of one directory — macOS /tmp/ws vs
/private/tmp/ws — now address the same store from a daemon worker
and from the CLI's --cwd.
2. Legacy grandfathering is gated at the scope-directory level instead
of per file: once the scoped directory exists, legacy files are
never consulted again. A per-file gate let a legacy allowlist
silently re-approve senders an operator had revoked by deleting the
scoped allowlist file, and let an in-use scope absorb a legacy file
that appeared later. The README now spells out that revocation means
removing entries, not deleting files.
3. The empty `pairing list` output names the workspace scope and points
at --cwd, mirroring the approve error, since a scope mismatch
surfaces there first.
Four new regression tests (symlink collapse, ENOENT fallback, no
resurrection after revoke, no late-legacy absorption) fail on the
previous commit and pass here.
Refs #7017
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(channels): state the broad realpath fallback is intentional; make the ENOENT scope assertion meaningful
Two round-2 review notes on #7065:
- canonicalizeWorkspacePath's docblock claimed to match acp-bridge's
ENOENT-only fallback while the catch swallows every realpath error.
Keep the broad catch — pairing storage is best-effort and a transient
FS error must not stop the channel from starting — and document that
divergence explicitly instead.
- The ENOENT-fallback test's second assertion compared a scope name to
itself. It now compares against the scope computed from the resolved
spelling, pinning that the realpath step degrades to a no-op for
nonexistent paths.
Refs #7017
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(channels): always close the migration gate, normalize nonexistent-path scopes, copy atomically
Address the automated round-2 inline findings on #7065:
- The migration gate is now closed on the very first construction even
when no legacy files existed: the scope directory itself is the
"migration decided" marker. Previously a workspace that first ran on
new code before any legacy state existed left the gate open, and a
legacy allowlist written later by an older version still running
concurrently would have been absorbed.
- resolvePath now runs every input through path.resolve, so
trailing-separator and dot-dot spellings of a path that does not
exist on disk (where the realpath step cannot help) canonicalize to
the same scope instead of three different ones.
- Legacy files are copied via temp file + atomic rename, so a crash
mid-copy cannot leave a truncated scoped file behind the now-closed
gate, and a concurrent first construction cannot observe a
half-written allowlist.
Adds three regression tests (late-legacy not absorbed after empty
first startup, nonexistent-path spelling collapse, unreadable legacy
file keeps the constructor best-effort); the first two fail on the
previous commit.
Refs #7017
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(channels): encode channel names in scoped paths, gate migration per channel, wire tests into CI
Three independently reproduced problems in the workspace-scoping change,
found in external review of d8c155ab3:
- Channel names come from unrestricted config keys and were joined into
the scoped path verbatim, so a name like `../support` climbed out of
the scope directory and landed every workspace on one shared file at
the channels root — silently undoing the isolation this PR exists to
establish. File names now URI-encode the channel name (mirroring
GroupHistoryStore), common names encode to themselves, and the legacy
source path is containment-checked as defense in depth.
- The directory-level migration gate let only the FIRST channel of a
workspace migrate: one process starts several channels in turn, and
once the first construction created the scope directory, every other
channel's legacy state was skipped forever. The gate is now a
per-channel `<channel>.migrated` sentinel inside the scope directory,
written even when there was nothing to copy.
- A single unreadable legacy file aborted the whole migration loop and
the gate still closed, so the other (valid) file was never migrated
and never retried. Files are now copied independently, best-effort,
via uniquely-named temp files + atomic rename, and scoped files are
never overwritten.
Also adds the missing test/test:ci scripts to channels/base (matching
its sibling packages), so the package's 784 tests actually run in CI's
`npm run test:ci --workspaces --if-present` sweep.
Four new regression tests (traversal-name isolation, multi-channel
migration, late-channel migration, unreadable-file independence) all
fail on d8c155ab3 and pass here.
Refs #7017
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(channels): read legacy files under the raw name, retry partial migrations, log failures
Round-3 review findings on #7065:
- Legacy sources are read under the RAW channel name again: pre-scoping
code wrote them unencoded, so looking them up under the encoded name
made any channel whose name changes under encoding (e.g. "my
channel") skip its legacy state and permanently lose approved
senders behind the sentinel. Encoded names remain in use for the
scoped destinations; the containment check keeps traversal-style raw
names from reading outside the channels root.
- The sentinel is only written when every present legacy file was
copied (or already existed). A partial failure (ENOSPC, transient
I/O) previously closed the gate with incomplete state; now the next
construction retries the failed file, and per-file stderr warnings
are emitted so operators can see why senders are missing instead of
instrumenting the constructor.
- The symlink test cleans up with unlinkSync — rmSync throws EISDIR
for a symlink to a directory on macOS.
- The pairing CLI gains tests covering --cwd scoping end to end (list
isolation, empty-scope hint, approve scoping, cross-workspace code
rejection), plus an explicit return after the mocked-in-tests
process.exit(1).
The raw-name and partial-retry regression tests fail on 954e76af4.
Refs #7017
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* docs(design): define default background subagents
* feat(core): improve subagent delegation defaults
* docs(core): cross-reference the three background-classification sites
Add pointer comments linking the core dispatch source of truth
(AgentTool.execute) and its two UI mirrors (web-shell
isBackgroundSubAgentToolCall, desktop detectBackgroundEvents) so the
replicated top-level-agent background heuristic is not changed in
isolation. Addresses PR review feedback.
* fix(core): align background classification for fork and named-teammate launches
Address review feedback on the background-classification rule so core dispatch
and the two UI classifiers stay consistent:
- core: exclude a name-without-active-team launch from the default-background
path so it stays foreground, matching both UI classifiers (which exclude
name). Previously such a launch was backgrounded by core but tracked as
foreground by the UIs.
- web-shell and desktop classifiers: exclude subagent_type "fork" from the
default-background heuristic, mirroring core's !isForkRequested guard. A
top-level fork request with an omitted flag runs foreground in core but was
classified as background by the UIs.
- add a core dispatch test asserting a working_dir launch with an omitted
run_in_background flag stays in the foreground.
* test: cover fork/background classification and precedence per review feedback
Address unresolved review threads on PR #7048:
- Add web-shell and desktop UI classifier tests asserting an omitted-flag
`subagent_type: "fork"` launch stays in the foreground, verifying the
documented `!isForkRequested` parity with core dispatch.
- Add a core AgentTool test asserting an explicit `run_in_background: false`
overrides a subagent config with `background: true`, locking in the
`run_in_background ?? config` precedence against a `||` regression.
- Harden the Explore read-only prompt: pipelines must not send data to a
network endpoint (no curl/wget/nc), closing the `cat file | curl`
exfiltration gap.
* fix(core): restore general no-unnecessary-files guard in general-purpose prompt
Address review feedback: the rewritten general-purpose prompt dropped the
broad guard against creating unrequested files, keeping only the
documentation-specific line. Restore a general 'do not create files unless
necessary' guard so speculative utility/config files are not created.
* test(desktop): cover named-teammate foreground guard in detectBackgroundEvents
Add a desktop tool-matching test asserting a top-level Agent with a
`name` set (named teammate) stays foreground and emits no
task_backgrounded event, mirroring the web-shell classifier's
named-teammate coverage and the existing fork-exclusion test.
* test(core): cover named-teammate foreground dispatch when flag omitted
Add a core-dispatch test asserting a top-level Agent launch with `name`
set and `run_in_background` omitted stays foreground when no team is
active, guarding the `this.params.name === undefined` exclusion in
backgroundRequested directly (previously only covered by the UI
classifiers).
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(core): make the per-turn tool-call cap adaptive
The per-turn tool-call cap (model.maxToolCallsPerTurn, default 100) was a
blunt circuit breaker: it halted any turn on the 101st tool call regardless
of whether the model was stuck or doing productive work. Large multi-package
implementation turns legitimately exceed 100 calls, so the cap killed
productive turns — a false positive.
Make the cap adaptive. The configured value is now a soft cap: once a turn
exceeds it, the cap halts only when a stuck-repetition signal is present (the
same (tool, args) call repeated 6+ times); a productive turn (diverse calls,
no repetition) continues up to a hard backstop of 3x the soft cap, which
always halts to bound an argument-varying runaway.
Validated against a real session whose 100-call turn was halted mid-build
with no repetition (max key repeat 2): that turn now continues, while genuine
stuck loops still halt at the soft cap. The always-on cap keeps its own
per-(tool,args) repeat tracker so it stays independent of skipLoopDetection.
The ACP/daemon path has a separate blunt cap that is not aligned here; noted
as a follow-up in the design doc.
* chore: regenerate settings schema for adaptive cap
* fix(core): clarify adaptive cap is interactive-only; strengthen cap tests
Address review feedback:
- The setting description now notes the daemon/ACP path still halts at the
configured value regardless of repetition, so it no longer overclaims the
adaptive behavior for non-interactive paths.
- Rename the misleading "fires at the built-in default soft cap value" test
(it only asserts diverse calls are allowed past the cap).
- Add a retry test that builds a stuck-repetition signal before the retry and
verifies it is cleared, so removing the capMaxKeyRepeat reset would fail.
* test(core): cover productive-then-stuck cap; clarify cap halt hint
Address review round 2:
- Add a test for a turn that crosses the soft cap with diverse calls and then
becomes stuck mid-range, so the stuck check is verified across the whole
(softCap, hardCap] range, not just at the boundary.
- Make the headless cap-halt message accurate for both triggers: it no longer
only suggests raising maxToolCallsPerTurn (correct for the hard backstop but
misleading for a stuck repeat) and now also points at the repetition.
* refactor(core): hash tool-call key once in always-on cap path; tighten cap tests
Address review round 3:
- Compute the (tool,args) key once in checkAlwaysOnSafeties and share it
between the consecutive-identical guard and the cap stuck tracker (was
hashed twice per call; args can be large). Skip hashing entirely when loop
detection is disabled for the session (no consumer).
- Add a test that the stuck signal accumulates across Finished round-trip
boundaries within a turn.
- Tighten the Stop-hook continuation budget test (cap 4 -> 1) so it still
guards the reset under the adaptive cap, where diverse calls no longer trip
the soft cap.
- Document the monotone stuck-signal non-goal and the telemetry-differentiation
follow-up in the design doc.
* fix(core): canonicalize tool-call key fields; clarify adaptive cap scope
Address Codex review (2 Critical):
- getToolCallKey now canonicalizes object keys recursively (preserving array
order) before hashing, so a stuck model cannot evade the repeat guards by
reordering argument fields. Adds a reordered-arguments regression test.
- Correct the maxToolCallsPerTurn description: the adaptive behavior applies to
both the interactive TUI and non-interactive (-p / JSON / stream-JSON)
core-client runs; only the daemon/ACP path is strict. Updated in
settingsSchema.ts, settings.md, and the regenerated settings.schema.json.
* test(core): cover nested key reordering and the consecutive guard's canonicalization
Address review:
- Extend the reordered-args stuck-signal test to include nested objects, so a
regression that breaks canonicalizeForHash's recursion is caught.
- Add a reordered-args test for the consecutive-identical guard, pinning the
canonicalization contract for that always-on detector (not just the cap).
* fix(core): treat explicit maxToolCallsPerTurn as a hard cap; keep default adaptive
Address yiliang114's Critical: v0.19.10 shipped maxToolCallsPerTurn as a hard
cap, but the adaptive change multiplied every configured value by 3, turning an
explicit N into a 3N budget — a breaking change for users who set it to bound
unattended cost.
Behavior now depends on whether the value was explicitly configured
(Config.isMaxToolCallsPerTurnExplicit):
- Explicit N -> hard cap (halt at N+1), preserving the released contract.
- Default (unset) -> adaptive: soft cap 100, halt only on a stuck-repetition
signal, with the hard backstop raised to 1000 (10x) so modern models making
hundreds of legitimate calls per task are not false-positived.
Adds the explicit-hard-cap regression (cap of 2 halts call 3) and a contrast
test proving the explicit flag drives the behavior. Updates the setting
description, headless hint/label, and design doc accordingly.
* docs(cli): design for VP mode mouse text selection and copy
First step of the VP mouse-selection feature: the design doc, submitted
ahead of implementation so the approach can be reviewed before code lands.
Proposes an application-level selection and copy system for VP mode, where
SGR mouse tracking currently suppresses native terminal selection. Evaluates
two routes and commits to exposing the Ink renderer cell grid via the
existing Ink patch, which makes coordinate-to-text mapping uniform across all
content types. Implementation follows as staged milestones on this branch.
* docs(cli): revise VP selection design per feasibility audit
Rework the design after the implementation-feasibility audit against the
Ink 7 renderer source. Key corrections and scope changes:
- Two-PR plan. PR 1 (this branch) delivers the Ink frame-buffer foundation
and a visible-region visual-selection MVP (visual-cell copy, clear on
scroll). PR 2 adds cross-screen selection and semantic copy fidelity
(soft-wrap rejoin, gutter exclusion), which need renderer semantic metadata.
- Highlight goes through a bidirectional frame controller with a
pre-serialization transform and throttled repaint; the earlier read-only
accessor plus post-commit callback could not highlight the current frame.
- Immutable cell/style allocation to avoid leaking the highlight onto other
on-screen occurrences of the same text via shared cached styled chars.
- Corrected coordinate formula (frame is not unconditionally top-anchored),
OSC 52 behavior (over-cap skips and returns false; surface a copy failure),
and the note that release already has non-selection consumers.
- Merge gate and milestones aligned to the two-PR split.
* feat(cli): M0 Ink frame-buffer controller for VP text selection
First implementation milestone of VP-mode mouse text selection: the Ink
frame-buffer foundation and its read-side wrapper, with the design's M0
go/no-go criteria proven by tests.
Extend the Ink patch into a small bidirectional frame controller. The
renderer now retains the composited cell grid, applies a selection-range
background highlight before serialization (allocating new cells so the
shared cached styled chars are never mutated), and publishes an immutable
frame each render. setSelection deduplicates and schedules exactly one
repaint through Ink's own throttle; getFrameController(stdout) exposes the
bridge to the application.
Add ScreenBuffer, the read-side wrapper the selection state machine (M1)
will build on: getCellAt / lineText / dimensions plus setSelection and
subscribe. Tests cover addressable cells, wide-character spacer handling,
pre-serialization highlight and clear, no highlight leakage onto identical
on-screen text, and single-frame-per-change dedup (no render loop).
The package.json export additions from the existing patch are preserved.
* feat(cli): M1 mouse text selection with live highlight in VP mode
Turn mouse press/drag/release in the VP history viewport into a text
selection, highlighted live and copied on release.
- SelectionState: anchor/focus model in composited-frame coordinates,
normalized to reading order, with collapsed/empty queries.
- selection-text: extract the visual text of a range, emitting a wide glyph
once (skipping its spacer) and trimming per-line trailing padding. B1
fidelity — visual cells as shown; soft-wrap rejoin and gutter exclusion
are PR 2.
- selection-coords: map a terminal cell to frame grid coordinates via the
frame anchor (bottom-pinned on overflow), plus viewport bounds/clamp.
- TextSelectionController: headless controller that subscribes button-level
mouse events, drives the state machine, highlights the range through the
frame controller (setSelection), and copies on release. Ignores the
scrollbar column and presses outside the viewport; clears on any scroll
(visible-region only in B1).
- VirtualizedList/ScrollableList expose getViewportRect(); MainContent mounts
the controller in the VP branch.
Copy-failure feedback, key preemption (Esc/Ctrl+Shift+C), streaming/resize
invalidation, and word/line selection follow in M3/M4.
* feat(cli): gate VP text selection behind ui.textSelection settings
Add ui.textSelection.enabled (default true) and ui.textSelection.copyOnSelect
(default true) and wire them into the selection controller: selection is only
active when enabled, and release copies only when copyOnSelect is on. Disabling
enabled leaves the mouse to the terminal for those who prefer it.
* feat(cli): M4 word/line select, selection invalidation, and docs
- Double-click selects a word, triple-click selects a line, driven by a
multi-click detector in the controller; word/line spans come from the
composited frame (non-whitespace run / first-to-last-content cell) and are
copied on select.
- Invalidate the selection when the content scrolls, streams (scrollHeight
changes), or the terminal resizes (frame height changes / resize event),
detected via a frame subscription with a baseline captured at selection
start. Our own highlight renders leave those unchanged, so there is no
render loop.
- Document mouse selection in the keyboard-shortcuts reference and the
ui.textSelection settings in the settings reference; refresh the
useTerminalBuffer description and regenerate the IDE settings schema.
A single click already clears a selection, so Esc-to-clear, the manual
copy keybinding, the footer "copied" toast, and a drag discoverability hint
are deferred follow-ups within this PR.
* fix(cli): drop redundant resize listener in selection controller
Real-terminal testing surfaced a MaxListenersExceededWarning (11 resize
listeners) on startup: the selection controller added a stdout 'resize'
listener on top of the existing ones. A resize reflows content, which changes
the frame/scroll height already watched by the frame subscription, so the
explicit listener was redundant. Remove it to stay under the max-listeners
cap; invalidation on resize still works via the subscription.
* fix(cli): close VP selection review gaps
* refactor(cli): make VP text selection unconditional
* fix(cli): make VP mouse selection usable
* fix(cli): preserve selection in thought blocks
---------
Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Expose persisted active/archived/total (plus live) via a dedicated aggregate
endpoint so clients do not need to page the full session list. Counts reuse the
existing chats-dir disk scan pattern from session title search; responses mark
expensive/disk_scan so callers know not to poll.
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
