* 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.
* fix(test): restore first-output benchmark measurement validity
Anchor the post-session dwell to SSE readiness so a slow connect cannot
silently reduce a dwell scenario to an immediate-prompt run, isolate the
runner in its own serial vitest config, decide the Phase 1 prototype gate
on the paired bootstrap CI instead of a bare difference of two P50s, and
normalize every invalid timing rather than only the first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(test): correct first-output benchmark artifact schema and simplify (#7825)
Drop the bundle git commit, which resolved HEAD of whatever repository
happened to contain the bundle directory rather than the revision it was
built from; the harness commit and bundle hash already record provenance
correctly. Rename the prompt-shape config field, which held a description
of the prompt rather than the prompt itself, and bump the artifact schema
for both field changes.
Also remove an unreachable AB/BA balance check, fold a duplicated success
predicate into one, parse the comparison-only dwell after the mode check
so single mode reports the accurate error, and document the two median
definitions, the compile-cache path lifetime, and the actual buffer
overflow and cold/warm attribution semantics.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(test): correct copyright years to 2026 (#7820)
* fix(test): reuse metricForOrdinal in coldWarmProviderDeltas (#7820)
* fix(test): strengthen benchmark test fixtures and align config with Vitest defaults (#7820)
* test(integration): cover prototype-gate input validation guards (#7820)
* fix(integration): summarize sseReadyToPromptMs metric and clarify gate error (#7820)
* test(integration): pin prototype-gate artifact shape for empty deltas (#7820)
* fix(integration): fail loudly on missing SSE timestamp; document sseReadyAt (#7820)
Replace the non-null assertion on the dwell anchor with an explicit guard so a
future path that resolves SSE readiness without recording a timestamp fails as
harness_error instead of silently degrading into an immediate-prompt run that
still reports its configured dwell. Also define sseReadyAt in the timestamp
table and note that each metric's bootstrap seed is positional, so inserting or
reordering a metric shifts later seeds and makes artifacts incomparable.
* test(integration): cover findInvalidTimings in the fast CI suite (#7820)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: qwen-code-autofix[bot] <qwen-code-autofix[bot]@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com>
* feat(triage): add revert-pattern high-risk path detection
Replace the behavior-neutral PR filter (PR #7414 v1, ~2% hit rate) with a
data-backed triage gate based on revert-history analysis of 111 revert
commits and 46 unique reverted PRs in this repo.
Stage 1e checks three signals identified by the analysis:
- touches_high_risk (66.7% precision, 32.3% recall)
- contested-merge pattern (50.0% precision, 19.4% recall)
- non-maintainer + high-risk (58.3% precision, 22.6% recall)
The gate escalates review depth and recommends maintainer sign-off; it
never blocks or closes PRs. Design doc and analysis scripts included.
* fix(triage): avoid stale-exempt hold label
* fix(triage): address review risk detection feedback
* fix(triage): tighten high-risk path patterns
* fix(triage): address review feedback on Stage 1e revert-pattern gate (#7414)
* fix(triage): address round-2 review feedback on Stage 1e gate (#7414)
- Fix APPROVE → APPROVED state name (GitHub API enum)
- Use gh api --paginate for file listing (fixes 100-file truncation)
- Anchor shell/relaunch/sandbox patterns with (^|/) to avoid false positives
- Append || true to grep (exit 1 on no match is the 92% case)
- Scope E2E recommendation to write-access authors per Stage 2c
- Add bot author filter to contested-merge query
- Define core paths explicitly in contested-merge condition
- Wire Stage 1e do-not-auto-approve into Stage 3 guardrail
- Replace precision percentages with p-values/raw counts in skill text
- Add sampling caveat and statistical significance notes to design doc
- Fix design doc errors: 71%→61.5%, 10→8 PRs, Rule 3 attribution,
~20% baseline→10% prevalence, Area field, no_e2e inconsistency
- Make test assertions specific to Stage 1e (not vacuous)
- Add Risk: template field assertion
- Revert drive-by prettier reflow
- Note need-discussion label removal by maintainer
* fix(triage): address round-3 review feedback on Stage 1e gate (#7414)
- Separate gh api call from grep so API failures are visible instead of
being masked by || true (rc:3660753982)
- Include author identity in contested-merge jq output and require
different reviewers for the disagreement check, avoiding false
positives from same-reviewer iteration (rc:3660753990)
- Add Stage 1e to the approval summary checklist so it is not omitted
from the pre-approval conditions (rc:3660753994)
* fix(triage): address round-4 review feedback on Stage 1e gate (#7414)
* fix(triage): use portable ERE grep for test-file exclusion (#7414)
* fix(triage): guard deferred approval on discussion label
* fix(triage): keep only supported revert signal
---------
Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* 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(web-shell): render background notifications as system messages
* fix(web-shell): use basic table rendering by default
---------
Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>
Add an opt-in cold-process benchmark, deterministic paired statistics, and artifact reporting to gate any future Provider preload work without changing production behavior.
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(serve): hot-reload workspace trust changes
Rebuild workspace runtime generations when trust policy changes, fail closed across daemon routes, and expose reconciliation status to SDK and Web Shell clients.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* codex: address PR review feedback (#7268)
Document the trust hot-reload capability and reuse the daemon environment fallback so the serve process environment guard remains satisfied.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): cache workspace trust status snapshots
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix: address trust reload race regressions
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): avoid repeated runtime containment
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): harden workspace generation boundaries
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): restore stale session owner fallback
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): preserve workspace metadata across trust reloads
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): align hot-reload trust semantics
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): stop git-state watcher on dispose only, fix git chip test (#7268)
beginDrain stopped the git-state watcher but cancelDrain had no way to
restart it, leaving the watcher disposed until the next lazy poll.
disposeRuntime already stops git-state when the drain is committed, so
the beginDrain stop was redundant — remove it.
Also fix the WorkspaceSection git chip test that broke when the trigger
changed from <button> to <span role="button"> inside DropdownMenuTrigger:
use closest('[role="button"]') and interact with the dropdown menu item.
* fix(serve): address review feedback on trust polling and setValue assertion (#7268)
* fix(cli): correct daemon trust policy settings precedence and drain continuation (#7268)
* fix(serve): address review feedback on fork cleanup, persist simplification, sync guard, and a11y (#7268)
* fix(serve): assert before mutate in setValue, add pre-mutation guard, trust-before-generation ordering (#7268)
* fix(serve): honor system defaults in trust policy
Apply the documented settings precedence to daemon folder trust evaluation and keep workspaces outside configured trust rules fail-closed.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): preserve managed scratch trust during reloads
Keep daemon-created scratch workspaces trusted across policy reloads while retaining controlled-root validation, and reject trust mutations that cannot apply to these fixed-trust runtimes.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): guard auth provider persistence by generation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(serve): remove Web Shell trust UI
Keep this PR focused on daemon and SDK trust reconciliation; the Web Shell integration can follow separately.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(sdk): restore workspace trust bundle budget
Preserve the merge-only browser bundle allowance required by the additive workspace trust v2 SDK surface after rebasing.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): handle trusted folder write failures
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): keep capabilities available during trust reload
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): address review feedback for workspace trust hot reload (#7268)
Drop the closed generation guard before retrying dynamic workspace
runtime creation so the retried runtime starts with a fresh, open guard
instead of inheriting the one closed during the abandoned attempt. Make
the /workspace/reload trust reconcile fire-and-forget with a swallowed
rejection (failures are reported separately), reuse sendGenerationClosedError
for the memory write error path, and assert the subagent deletion commit
boundary once before unlinking so a closed generation fails atomically.
Add coverage for the blocked-entry deep health probe and the /session/:id/cd
generation-close-during-flight path.
* fix(serve): close trust reload cleanup gaps
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): use fire-and-forget for trust reconcile in workspace-qualified reload (#7268)
* fix(serve): address review feedback on generation guard and trust reconciler (#7268)
* fix(serve): use shared helpers for untrusted/generation-closed responses (#7268)
* fix(serve): continue cleanup after drain commit errors
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): retry transient trust policy disappearance
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): align status provider trust default with route-level check (#7268)
* fix(serve): clean up worktree on generation guard abort (#7268)
* fix(cli): guard tool and skill settings commits
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(cli): add discriminating persistent-ENOENT test for trust policy read (#7268)
* fix(serve): close runtime generation gaps
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): preserve scheduled task cap errors
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(serve): address review feedback on trust reconciler, settings guard, and route simplification (#7268)
* fix(serve): preserve containment retry semantics
Restore the last verified trust-reconciliation and generation-guard behavior after the automated review fix marked an unconfirmed disposal as contained and removed per-scope commit checks. Defer the remaining late-round suggestions to avoid expanding the PR.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Autofix <qwen-autofix@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@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>
* perf(web-shell): paint the composer git chip before git status completes
New sessions gated the chip on a full `git status --porcelain` subprocess
behind GET /workspaces/:ws/git, so the branch chip appeared hundreds of
milliseconds (worst case seconds) after the composer was ready.
The daemon now keeps a per-workspace last-known summary with in-flight
dedup and a 2s background-refresh throttle: the default GET returns the
cached status (branch-only on a cold start) immediately and recomputes in
the background, publishing git_status_changed over SSE only on a delta,
while ?wait=1 keeps the previous blocking semantics. The composer fetches
both paths concurrently — the fresh GET also covers the no-session state,
which has no per-session SSE stream — so the branch paints in ~3ms and
the counters land when the computation finishes. The sidebar keeps
wait:true since it has no SSE fill-in path.
* fix(web-shell): add exhaustiveness guard and worktree git-status test (#7680)
* fix(web-shell): add exhaustiveness guard and worktree git-status test (#7680)
* fix(web-shell): add exhaustiveness guard and worktree git-status test (#7680)
* fix(cli): use writeStderrLineSafe in git-status refresh error path (#7680)
* fix(web-shell): add debug trail to fresh-path catch and test branch-watcher dispose guard (#7680)
* fix(cli): assert writeStderrLineSafe in git-status refresh failure test (#7680)
---------
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
* fix(core): exit_plan_mode returns guidance error from execute() (#7671)
When the model calls exit_plan_mode outside plan mode (e.g. after the
user manually switched modes via Shift+Tab), it previously got a
generic permission deny error that gave zero guidance.
Changed getDefaultPermission() to return 'allow' instead of 'deny'
when not in plan mode — this is a state issue, not a security issue,
so the permission layer should not block it. The execute() method now
checks approvalMode !== PLAN (with no approval snapshot) and returns
a context-aware errorResult telling the model what happened and what
to do instead.
This also fixes the YOLO mode issue where a permission deny error
was semantically wrong.
* fix(core): cover getConfirmationDetails() outside-Plan guidance path (#7671)
The guidance error for outside-Plan exit_plan_mode calls was only
reachable through execute(). A PM ask rule or a Plan-to-non-Plan
mode switch between permission evaluation and confirmation
construction routes through getConfirmationDetails() instead, which
still threw a generic error.
Extract outsidePlanGuidanceMessage() and reuse it in both
getConfirmationDetails() (throw) and execute() (errorResult) so
the model receives actionable guidance on every reachable path.
Add scheduler-level regression tests for both bypass paths and
update the design doc failure-behavior section.
* fix(core): use StructuredToolError and ToolErrorType for exit_plan_mode guidance (#7671)
---------
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
* docs: design spec for @ session reference + tabbed completion UI
* docs: implementation plan for @ session reference + tabbed completion UI
* feat(cli): add @session: mention ref parser
* feat(core): add SessionReferenceService for slimmed session injection
* feat(cli): inject slimmed prior-session context on @session: mention
* feat(cli): surface prior sessions as @ completion suggestions
* feat(cli): tabbed category layout for @ completion dropdown
* feat(cli): ←/→ tab switching for @ completion categories
* docs: mark @ session reference design as implemented
* fix(cli): preserve assistant text on tool-call turns, keep newest turn under budget, guard stale session results
* docs(core): clarify SessionReferenceService slimming rules
* perf(cli): remove @ completion input latency from session listing
Dispatch file/MCP suggestions immediately and append prior-session
suggestions in a second render once the disk listing resolves, instead
of blocking the first render on session I/O. Cache the per-cwd session
listing for a short TTL so rapid keystrokes don't re-walk the chats dir.
* Revert "perf(cli): remove @ completion input latency from session listing"
This reverts commit 08bbbc21386ac0cd6db7a6f480f0fd59f2e6b8d4.
* fix(cli): harden session ref resolution and i18n tab labels (#7302)
* fix(cli): guard session ref I/O errors to never abort the turn (#7302)
* fix(cli): strip session: prefix in completion filter and align tab guard (#7302)
* fix(cli): address review feedback on session refs and tab tests (#7302)
* fix(cli): use SESSION_MENTION_PREFIX constant in completion filter (#7302)
* fix(cli): address review feedback on session refs and tab tests (#7302)
* fix(cli): address review feedback on session refs and tab tests (#7302)
- Constrain ctrl/command on completion tab-switch bindings so
ctrl+left/right (word-jump) is not consumed for tab switching
- Derive a friendly title from the first user message in
SessionReferenceService.resolve() when no explicit title is given,
so UUID-based refs show meaningful context headers
- Fix dead conditional in plan doc code snippet
* fix(cli): surface real session lookup errors and test left tab switch (#7302)
* fix(cli): reset suggestion indices when active category tab disappears (#7302)
* fix(cli): address review feedback on session refs docs and tests (#7302)
* fix(cli): address review feedback on session refs and completion ordering (#7302)
* docs(cli): clarify title matching semantics in session reference design (#7302)
* test(cli): assert error card in ambiguous session title test (#7302)
* fix(core,cli): address review feedback on session refs (#7302)
- Replace O(N²) budget trimming with single-pass backward accumulation
(maintainer-measured 6.8s → ~80ms for 8k-record sessions)
- Include header/marker overhead in approxTokens
- Prefer custom_title system record in deriveTitle over first user message
- Move session-ref detection before MCP resource ref to prevent collision
with an MCP server literally named "session"
- Handle bare @session (no colon) as empty filter in completion
- Clamp activeSuggestionIndex when suggestion list shrinks
- Strengthen not-found test assertion to check ToolCallStatus.Error
- Fix unreachable mock state in InputPrompt tab-guard test
- Add tests: custom_title derivation, overhead in approxTokens,
bare session filter, index clamping, activeCategory reset
* fix(core,cli): address review feedback on session refs (#7302)
- Fix approxTokens to exclude omission marker cost when not truncated
- Clamp visibleStartIndex when suggestion list shrinks to prevent empty dropdown
- Document merge order invariant for session suggestions in @ completion
* fix(core,cli): address review feedback on session refs (#7302)
* fix(core): address review feedback on session refs (#7302)
* fix(core,cli): address review feedback on session refs (#7302)
* fix(core): address review feedback on session refs (#7302)
* fix(cli): address review feedback on session refs (#7302)
* fix(core,cli): address review feedback on session refs (#7302)
- Wrap MCP category label in t() for i18n consistency
- Fix comment to match > 2 tab guard semantics
- Add test for Ctrl+arrow with exactly 2 categories (guard boundary)
- Fix budget loop to not reserve marker tokens when session fits
---------
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
* feat(cli): support native video input in learn
* fix(cli): address review feedback for /learn video input (#7497)
- Fix .m4v MIME filter: fall back to parser MIME type when mime/lite
does not recognise the extension (e.g. .m4v → video/x-m4v)
- Surface text diagnostics from readPathFromWorkspace when the read
succeeds but returns no video part (e.g. file size limit errors)
- Move YouTube rejection after the capability gate so text-only model
users get the correct error first
- Extract shared buildSkillContext helper to deduplicate skill
directory enumeration between buildLearnSkillPrompt and
buildLearnVideoSkillRequest
* fix(core): classify .m4v as video so /learn can attach it (#7497)
* fix(cli): address review feedback for /learn video input (#7497)
- Fix .m4v mimeType in processSingleFileContent by falling back to
MIME_LITE_MISSING_VIDEO_TYPES when mime/lite returns null
- Move YouTube rejection before the capability gate so users always get
actionable download guidance regardless of model capability
- Use optional chaining for getContentGeneratorConfig()?.authType
- Add [focus] to the empty-input usage string
* fix(cli): address review feedback for /learn video input (#7497)
---------
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 mode selector for new session creation
Support three git workflows when creating a new session:
1. Current branch (default, unchanged behavior)
2. New branch — daemon runs git checkout -b before spawning
3. Worktree isolation (existing, now unified into the same UI)
The mode selector lives in the composer's git chip as a popover,
replacing the previous worktree-only toggle in the welcome header.
API: POST /session accepts branch: { name } (mutually exclusive
with worktree). Server validates branch name, checks dirty tree,
creates branch, and rolls back on spawn failure.
Design doc: docs/design/2026-07-22-webshell-session-git-mode.md
* fix(web-shell): prevent Radix popover dismissal in git mode selector
The portal container used by Web Shell's Popover primitive is not
recognized by Radix's DismissableLayer, causing the popover to close
on any interaction. Add onInteractOutside prevention so the popover
only closes via explicit selection.
Also replace prototype screenshots with real Playwright captures and
add e2e test + screenshot capture script.
* refactor(web-shell): remove redundant worktree toggle from welcome header
The composer git chip popover now fully covers worktree selection,
making the welcome header toggle/badge redundant. Remove the toggle
UI, its state (worktreeToggleEligible, refs, handlers, focus effect),
and all associated tests (unit + e2e + visuals).
* chore: re-capture PR screenshots after worktree toggle removal
* fix(web-shell): audit fixes for git mode selector
- Add missing setSessionBranch(undefined) in loadSidebarSession,
createNewSession, and session switch effect (!sid path)
- Add setSessionBranch(summary.branch) in session status restore
- Add branch rollback in client disconnect (!res.writable) path
- Fix onInteractOutside: use containment check instead of
unconditional prevention so genuine outside clicks close popover
- Hoist promisify(execFile) to module level
- Narrow reserved branch name check to only HEAD (FETCH_HEAD etc.
are valid branch names)
* fix(web-shell): address review feedback for git mode selector (#7471)
* test(web-shell): capture the git-mode selector in the visuals suite
This PR adds the new-session git-mode selector (current branch / new branch /
worktree) but no visuals scenario renders it, so the before/after preview showed
no image for an entirely new UI — the empty result was a coverage gap, not a
clean bill of health. The PR also removed the `worktree empty state` scenario
(its `worktree-welcome-toggle` no longer exists, replaced by this popover),
leaving the suite with no view of the new-session empty state at all.
Add a `git mode selector` scenario that seeds a trusted git-repo workspace and
lands on the empty state (the only place App.tsx wires the intent props), then
captures the composer chip and the opened three-mode popover in both themes.
Both are byte-stable across runs (0% pixel diff), and asserting an option is
visible makes a regression that fails to open the popover fail here rather than
only in the screenshot.
The branch-name sub-state is deliberately not captured: its input autoFocuses
and the popover then dismisses on the idle frame the capture waits for, so it
can't be shot stably through this pipeline — the functional
web-shell.git-mode.spec.ts already drives that path. Restores the empty-state
coverage this PR dropped and gives the new selector a head-only (NEW) preview.
* fix: address review feedback for git mode selector (#7471)
- Forward the branch override in createDetachedSession so the cold-start
(no active session) path no longer silently drops a user-selected new
branch.
- Reserve the workspace before 'git checkout -b' to close the TOCTOU in
the activeBranchSessions guard; two concurrent branch creations could
both pass the guard and race on HEAD. The reservation is released on
every exit path.
- Return (and close the browser) when the branch input never appears in
the screenshot script instead of falling through to a guaranteed throw.
- Add an e2e test asserting the default current-branch submit sends
neither branch nor worktree.
* fix(web-shell): address review feedback for git mode selector (#7471)
* fix(web-shell): address review feedback for git mode selector (#7471)
* fix(web-shell): address review feedback for git mode selector (#7471)
* fix(cli): sync ink patch with semantic selection types (#7471)
* fix(web-shell): remove orphaned worktree CSS and dead i18n keys (#7471)
* fix: address review feedback for git mode selector (#7471)
* fix: address review feedback for git mode selector (#7471)
Release the route-local in-flight branch reservation in the
disconnect-after-spawn cleanup path so a throwing killSession/
removeSession no longer permanently blocks the workspace from new
branch sessions. Also reject branch names ending in .git on both
the server and the composer validator, associate the branch-name
label with its input, abort the screenshot capture script cleanly
when the chip or popover is missing, and add focused coverage for
the git-mode gating, branch forwarding, and branch pass-through.
* test(cli): cover branch session route validation and mutual exclusion (#7471)
* fix(cli,web-shell): accept Unicode branch names in validation (#7471)
The branch name validation regex rejected all non-ASCII characters,
preventing users from creating branches with Unicode names that git
accepts (e.g. 功能/fix-login). Replace the ASCII-only character class
with Unicode property escapes (\p{L}\p{N}) and the u flag, applied
consistently to both the server-side route and the client-side
GitModePopover validation.
* fix(web-shell): address review feedback on git mode selector (#7471)
- Revert unrelated ink patch change (transformers: [] → newTransformers)
- Extract duplicated branch rollback logic into rollbackBranchCreation helper
- Pessimistically track activeBranchSessions when killSession throws in
disconnect-reap path, preventing concurrent branch session on surviving
session
- Use ref pattern for gitModeIntent in ensureSessionForPrompt to avoid
callback cascade on every git-mode toggle
- Add aria-label to git mode clear button for screen reader accessibility
* fix(cli): harden git branch session creation and clarify UX (#7471)
Address review feedback on the git mode selector:
- Bound every branch git operation with a 30s timeout (mirroring
GitWorktreeService) so a stuck repository lock or slow hook can no
longer hang the request and leave the workspace permanently reserved
in inFlightBranchWorkspaces.
- Run branch shape/name validation before the active-session conflict
check so a malformed body gets 400 instead of 409.
- Compare the reserved HEAD name case-insensitively (ref storage is
case-folding on macOS/Windows), in both the route and the popover.
- Surface the design-doc "switches the working directory to a new
branch" hint in the popover so users know HEAD will move.
- Correct the design doc: branch metadata is in-memory only and does
not survive a daemon restart.
* fix(web-shell,cli): fix light theme, stale intent, and branch init guard (#7471)
- Replace undefined --web-shell-* CSS variables with shadcn design tokens
(--foreground, --muted-foreground, --border, --popover-foreground, etc.)
and add --git-mode-* accent variables to both .themeDark and .themeLight
so the git mode popover is readable in light theme.
- Add useEffect to clear gitModeIntent when gitModeEligible flips to
false, preventing stale branch intent from leaking to another workspace.
- Move gitModeIntentRef assignment from render body into useEffect to
avoid ref mutation during render (concurrent React safety).
- Wrap GitWorktreeService constructor in try/catch on the branch path,
matching the worktree path's guard, so a constructor throw returns 500
instead of hanging the request.
- Show branchConflictWarning hint only when a valid branch name is
entered, not as a static default hint.
- Reword GIT_RESERVED_BRANCH comment and add cross-reference comments
between the duplicated client/server validation predicates.
* fix(cli,web-shell): address review feedback on git-mode PR (#7471)
- Add clearBranchSessionEntry cleanup hook on session close/delete to
prevent stale activeBranchSessions entries from causing spurious
409 branch_session_conflict on the next branch creation request.
- Extract git branch mutations (rev-parse, status, checkout -b,
rollback) into a mockable git-branch-ops module, closing the test
gap on the git-mutation paths that previously had no CI coverage.
- Add 6 new server tests: branch_already_exists, branch_dirty_tree,
branch_checkout_failed, happy-path 200 with branch metadata,
rollback on spawn failure, and branch_session_conflict.
- Export validateBranchName and add shared test vectors matching the
server-side validation to catch future client/server drift.
* fix(cli): close concurrency guard gap in branch session creation (#7471)
The synchronous reserve point only re-checked inFlightBranchWorkspaces,
not activeBranchSessions. A request that passed the early guard before a
concurrent request registered could slip through after the first request
completed and cleared inFlightBranchWorkspaces. Re-check both structures
at the reserve point (no await between check and add) to close the window.
Also clarifies the dirty-tree gate comment to explain the real intent
(surprise-prevention, not data protection).
* test(cli,web-shell): cover worktree intent forwarding and branch session delete lifecycle (#7471)
* fix(cli): address review feedback on git-mode branch sessions (#7471)
* test(cli): cover git-branch-ops git command semantics (#7471)
* fix(cli,web-shell): roll back failed checkouts and guard shared-checkout branch creation (#7471)
* fix(cli,web-shell): address review feedback on git-mode branch sessions (#7471)
---------
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
