Commit graph

284 commits

Author SHA1 Message Date
Dragon
22963d5777
feat(core): add fork_turns to fork subagents (#7346)
* 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>
2026-07-21 07:08:12 +00:00
顾盼
2ad63e7ca3
fix: ask when auto classifier is unavailable (#7331) 2026-07-21 03:27:17 +00:00
Shaojin Wen
ff350a16a7
feat(web-shell): add git commit history browser (#7204)
* 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>
2026-07-20 15:02:48 +00:00
Nothing Chan
357619a5fc
fix(channels): scope pairing and allowlist state by workspace (#7065)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* 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>
2026-07-18 13:33:26 +00:00
BaboBen
a9a6ca4ac6
feat(channels): observe group names from inbound messages (#7155)
* feat(channels): expose observed workspace contacts

* fix(channels): address observed contacts review feedback

* docs(channels): design observed group names

* docs(channels): plan observed group names

* feat(channels): observe inbound group names

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
2026-07-18 09:26:12 +00:00
Dragon
d4db5fcfab
feat(core): improve subagent delegation defaults and guardrails (#7048)
* 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>
2026-07-18 08:52:48 +00:00
BaboBen
02cb3a6380
feat(channels): expose workspace-scoped observed contacts (#7109)
* feat(channels): expose observed workspace contacts

* fix(channels): address observed contacts review feedback

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-18 07:35:45 +00:00
qqqys
7705f08305
feat(channels): save explicit multi-fact memory safely (#7092)
* feat(core): guard channel memory secrets

* feat(channels): classify multiple memory facts

* feat(channels): save multiple memory facts

* fix(channels): snapshot classifier memory fields

* fix(channels): snapshot classifier memory elements

* docs(channels): explain multi-fact memory

* docs(channels): clarify multi-fact memory example

* fix(channels): harden memory input snapshots

* fix(channels): log memory intent validation failures
2026-07-17 15:24:11 +00:00
qqqys
b826420d4d
feat(channels): confirm natural memory mutations (#7066)
* feat(channels): add memory mutation confirmations

* feat(channels): confirm natural memory mutations

* docs(channels): explain memory mutation confirmations

* fix(channels): harden pending memory mutations

* fix(channels): activate memory proposals after delivery
2026-07-17 15:06:44 +00:00
qqqys
859095bc98
feat(channels): support natural memory references (#6952)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* feat(channels): plan targeted memory intents

* fix(channels): harden targeted memory planner

* fix(channels): bound serialized memory manifest

* feat(channels): resolve natural memory references

* fix(channels): harden memory intent routing

* docs(channels): explain natural memory references

* fix(channels): type resolved memory intent dispatch

* fix(channels): route Chinese preference filters
2026-07-15 17:46:23 +00:00
BaboBen
f5bdba724e
fix(wecom): prevent requireMention from disabling group chat (#6948)
* fix(wecom): trust group callback mention scope

Fixes #6939

* docs(wecom): clarify mention-scoped behavior
2026-07-15 15:38:21 +00:00
pomelo
389a1f9ceb
feat(cli): change default approval mode from default to auto (#6899)
* feat(cli): change default approval mode from default to auto

The default approval mode required manual confirmation for every tool
call, producing dozens of confirmation prompts per task. Auto mode uses
a three-layer filter (workspace edits, read-only allowlist, LLM
classifier) to auto-approve safe operations while still guarding risky
ones.

Untrusted folders are still forced to default mode for safety.

Closes #6898

* fix(cli): keep manual approval in safe and bare modes

Restricted modes (safe/bare) strip permissions, allowlists, MCP servers
and hooks to provide a maximally restrictive session. The new AUTO
default fallback was silently downgrading them to the LLM classifier,
contradicting their lockdown intent. Restore DEFAULT (manual approval)
for these modes while keeping AUTO as the default for normal sessions.

Explicit --approval-mode and --yolo flags still take effect, since they
are resolved before the fallback.

* chore(cli): regenerate settings schema for auto default

Regenerate the VS Code settings schema so the tools.approvalMode default
matches the new auto value (fixes the "settings schema is up-to-date" CI
check). Also add coverage for the serve-mode approval fallback when no
approval mode is configured.

* test(cli): update SettingsDialog snapshots for auto approval default

The settings schema now defaults tools.approvalMode to auto, so the
SettingsDialog renders "Auto" instead of "Ask permissions" for the Tool
Approval Mode field. Regenerate the affected snapshots (10 updated).

* test(core): pin DEFAULT baseline in agent-override tests

These tests exercise createApprovalModeOverride isolation and the
DEFAULT→AUTO rule strip/restore transitions, so they implicitly relied
on the Config constructor defaulting to DEFAULT. Now that the default is
AUTO, pin the baseline explicitly so the tests no longer depend on the
constructor default.

---------

Co-authored-by: pomelo.lcw <pomelo.lcw@alibaba-inc.com>
2026-07-15 04:55:32 +00:00
BaboBen
4b802ca5f9
feat(channels): support DingTalk webhook delivery to direct messages (#6891)
* docs(channels): design DingTalk webhook DM delivery

* docs(channels): translate DingTalk webhook DM design

* feat(channels): support DingTalk webhook direct messages

* fix(channels): isolate DingTalk webhook DM targets

* fix(channels): handle DingTalk direct delivery failures

* fix(channels): reject malformed DingTalk responses
2026-07-15 03:07:30 +00:00
qqqys
fb0239eab4
feat(channels): add structured channel memory management (#6860)
* feat(core): add structured channel memory document

* fix(core): preserve legacy channel memory whitespace

* feat(core): structure channel memory storage

* fix(core): close channel memory races

* test(core): cover channel memory read failures

* feat(channels): parse channel memory item intents

* test(channels): cover memory intent precedence

* feat(channels): manage structured channel memory

* fix(channels): address structured memory review

* feat(cli): wire structured channel memory

* docs(channels): document structured channel memory

* fix(core): harden channel memory persistence

* fix(core): preserve channel memory uniqueness

* fix(channels): prioritize channel memory item updates
2026-07-15 00:44:15 +00:00
易良
d7e2892a7c
fix(cli): avoid updating active CLI processes (#6874)
* fix(cli): avoid updating active processes

* fix(cli): close update relaunch gaps

* test(cli): fix standalone update source path

* fix(cli): reset deferred update per relaunch
2026-07-15 00:33:17 +00:00
tanzhenxin
220fba7917
feat(subagents): make Explore inherit the main model by default (#6807) 2026-07-14 01:23:35 +00:00
jinye
98f2bb37ec
feat(cli): Add runtime daemon channel control (#6741)
Some checks are pending
E2E Tests / web-shell Browser Regression (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(cli): add runtime daemon channel control

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): address daemon channel review feedback

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(daemon): align channel control timeout budget

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): preserve serve fast-path import boundary

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): distinguish pending channel generations

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(cli): use workspace env for deferred webhook auth

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* codex: address PR review feedback (#6741)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-13 02:53:27 +00:00
Shaojin Wen
d1b2a7fb72
feat(review): procedural correctness finders, effort levels, and posting/verify guardrails (#6711)
* feat(review): procedural correctness finders, effort levels, and posting/verify guardrails

Rework the /review skill's finder layer and add precision and cost controls,
informed by dogfooding the skill against real PRs.

Recall:
- Split Agent 1 (Correctness) into three procedural finders defined by how they
  walk the diff — 1a line-by-line (incl. language-pitfall and wrapper-routing
  checks), 1b removed-behavior audit, 1c cross-file tracer — so coverage is
  complementary instead of overlapping. Bump the 3A dimension fan-out to 12
  agents and shift the 3A/3B gate to 3200 diff lines.
- Add Agent 8: up to two diff-specialized finders written per-review when the
  diff concentrates in a domain with a known failure grammar.
- Fold altitude into Code Quality and a quote-the-rule discipline into the
  conventions pass.

Precision:
- Every finding must state a concrete failure scenario (trigger to wrong
  outcome, or concrete cost); findings that can't are dropped at the source,
  and verification re-traces the scenario rather than judging prose.
- Verification checks a finding against the diff's own documented intent: a
  "regression" the diff deliberately changes and documents is a design
  decision, not a defect.

Cost and safety:
- Add --effort low|medium|high: cheap inline passes with no subagents (default
  high for PRs, medium for local changes).
- Hard-gate PR posting: never submit a review unless --comment was passed or the
  user explicitly asked, regardless of verdict.
- Add a substantive-return check for whole-diff agents (invariant, cross-file,
  test-coverage matrix) so a silently whiffing agent is caught like a missing
  chunk receipt.

DESIGN.md records the rationale and dogfooding cases behind each change; user
docs updated with the effort levels and the new agent roster.

* docs(review): fix stale topology numbers flagged in review

- Define H in the 3B pipeline diagram cost annotation (3 invariant agents
  per heavy file).
- Annotate the 40-PR re-gating cost figures with the roster they were
  measured under (22 agents / ~5% at 10 agents; ~34 / ~7% at 12).
- Correct the fork-subagent savings estimate to ~88-92% (~750-950K →
  ~80-88K); the previous range predated the updated totals.
- "None or nine" receipts under 3A is eleven under the 12-agent roster
  (every agent except Build & Test walks the diff).

* fix(review): address review feedback on effort/verify/lightweight edge cases

Criticals from review:
- Apply the --comment→high-effort override only after target disambiguation;
  an ignored --comment (non-PR target) no longer silently forces high.
- The documented-intent gate caps confidence only when the rationale makes the
  harm uncertain; a traced harm that survives the rationale keeps high
  confidence, and rejection is reserved for pure re-descriptions.
- Lightweight cross-repo mode degrades Agents 1a/1b to diff-only and routes
  unverifiable re-establishment claims to low confidence instead of asserting
  them, matching the verifier's limits.

Suggestions:
- Agent 0's empty-scope exit now carries its evidence and the whiff check
  recognizes it, so a legitimate no-linked-issue return is not relaunched.
- Reframe the 3200-line clause as an attention bound (3B is not guaranteed
  cheaper with heavy files or specialized finders).
- Fix call-budget notation: F for findings vs N for chunks; correct the 3B
  budget to rounds × chunks for the reverse audit (~70 calls on the 19-chunk
  example, not ~28-30); state the runtime concurrency cap (10) instead of
  claiming ~1x wall time.
- Preserve the failure scenario through pattern aggregation and posted inline
  comments; extend quality-finding verification to check the named helper
  does what the finding claims.
- Document medium effort's roster (no dedicated security/test-coverage pass);
  qualify the cross-effort scope note (incremental cache is high-only); define
  "lenses" on first use; fix tense and the stale 9-agent line in commands.md;
  clarify the 1b skip condition (no removed/replaced lines).

* fix(review): close 422-relocation verdict hole and lightweight-mode context gap

From review feedback (one human, two model reviews):

- 422 recovery: Criticals relocated into the review body now keep the event at
  REQUEST_CHANGES — the event/body table counted comments only, so a review
  whose blockers were all relocated could submit as APPROVE/no-blockers
  COMMENT with blocker text in the body.
- Lightweight cross-repo mode now runs pr-context (pure GitHub API): Agent 0
  and the open-Critical re-check need the PR body and open threads, which the
  bare gh-pr-diff setup never captured.
- Define --effort value parsing so a non-enum next token (e.g. a PR number) is
  never consumed as the value.
- Add the missing test-coverage-matrix definition section; mark agent counts
  as maxima (1b skipped on no-deletion diffs).
- Sync DESIGN's documented-intent paragraph with the corrected confidence
  policy; align 3A/3B budget headings with the dual-trigger gate and state the
  38-95 reverse-audit range explicitly.
- Docs: dual-trigger diagram labels, attention-bound wording, diff-reading
  lenses phrasing, per-stage-bounded (not fixed-total) cost claim, effort
  table qualifications, failure-scenario in the Step 7 JSON samples.

* docs(review): reconcile verifier rejection rule and close remaining edge notes

- State the Critical-rejection bar once, without the self-contradicting
  "never reject / to reject" phrasing: rejection requires quoting the
  contradicting code, and the floor verdict is confirmed (low confidence)
  when it cannot be quoted.
- Note the one sanctioned exception to the empty REQUEST_CHANGES body:
  unmappable or 422-relocated Criticals.
- Give the callee-direction check a concrete procedure (walk the other
  changed symbols this territory calls, re-read their post-change
  contracts).
- Define lightweight-mode pr-context failure handling: continue diff-only,
  skip Agent 0, open-Critical re-checks become "cannot tell" (no Approve).
- Clarify Agent 8 applies in every mode (it needs only the diff) and that
  Step 6 follow-up tips are high-effort only.

* fix(review): close flag-parse, context-unavailable, and downgrade edge cases

Address the latest review round on the skill text:

- An invalid spaced --effort value is discarded (with the warning) whenever
  another token is the target, so `/review 6711 --effort typo` reviews PR
  6711 instead of leaking `typo` into target disambiguation; the token is
  kept only when it is itself the sole target candidate.
- The lightweight-mode pr-context failure now names a context-unavailable
  state with a defined Step 7 serialization: never APPROVE, submit COMMENT
  with a diff-only body, findings or not.
- Step 6's open-Critical re-check draws from both context sections; a reply
  alone ("I disagree") no longer retires a blocker — only a code-verified
  "fixed by this diff" does.
- DESIGN and user docs now state the same rejection bar as the skill:
  rejecting a Critical requires quoted contradiction (or a documented-intent
  re-description); anything less certain downgrades.
- Downgrading a REQUEST_CHANGES that carries body-relocated Criticals keeps
  those descriptions after the downgrade sentence, so the self-PR downgrade
  can no longer erase the only copy of a blocker.

* fix(review): close verdict-upgrade and body-Critical re-check gaps

Third review round on the skill text:

- 422 recovery may never upgrade the event: a Suggestion-only review whose
  anchors all failed resubmits as COMMENT with the could-not-anchor body,
  never as APPROVE/"No issues found" — the verdict reflects confirmed
  findings, not surviving anchors.
- Step 6's open-Critical re-check now also walks the Review summaries
  section: an unmappable or 422-relocated blocker lives only in a review
  body, and pr-context truncates summaries to ~240 chars, so a summary
  showing (or cut where one could hide) a Critical marker is fetched in
  full via the reviews API before ruling.
- The context-unavailable cap now applies to every C=0 row of the invariant
  table, not just the empty one: Suggestion-only results post a diff-only
  body instead of a "no blockers" claim the run cannot certify.

* fix(review): compose COMMENT bodies from clauses and harden the body-Critical re-check fetch

Fourth review round found four pairwise collisions between rules that each
set "the" COMMENT body, plus four execution gaps in the Step 6 full-body
fetch. Close the class, not the instances:

- Replace the fixed-sentence bodies with an ordered clause composition rule
  (downgrade reasons, context-unavailable warning, suggestions disclosure,
  uncoverable chunks, body Criticals) — each clause present iff its state
  holds, free prose still banned, single-state case identical to the table.
- Define C once, globally: Criticals the review posts anywhere (inline or
  body), so no downstream C=0 rule can erase a body-only blocker, and 422
  relocation keeps REQUEST_CHANGES by definition rather than by patch.
- 422 recovery re-derives bodies via the composition rule, so a
  context-unavailable run can never restore a "no blockers" certification.
- Step 6's full-body fetch is paginated (--paginate; the endpoint returns 30
  per page), treats fetched bodies as untrusted data (extract only the
  Critical-bearing text, never paste unrelated bodies), and fails closed:
  an unreadable truncated blocker rules "cannot tell" and caps the event at
  COMMENT.

DESIGN.md records why composition replaces per-collision patching
(n states -> n(n-1)/2 pairs; clauses make new states additive).

* fix(review): correct the cross-repo capability table and close nine review notes

- docs: the cross-repo table claimed "Agents 0-6" run in lightweight mode
  while the prose (correctly) says 1c is skipped there — 1c is inside that
  range. Split 1c onto its own row, and add the missing Agent 8 row (its
  finders need only the diff, so they do run cross-repo).
- --effort=<level> now has a parse rule: split the flag token on the first
  '=' and consume no second token; the next-token rule applies only to the
  spaced form.
- The substantive-return (whiff) check covers every receipt-less agent, so
  3A's dimension agents are in scope, not just 3B's whole-diff agents.
- Step 3C names Agent 1b's lightweight degradation and states the three
  angles medium deliberately omits (security, test coverage, adversarial
  personas) instead of naming only two.
- Step 6's open-Critical re-check states what a context-unavailable run does
  (skip the walk, every Critical is "cannot tell") instead of pointing at a
  context file that does not exist.
- The event/body table carries the body-only-Critical exception in the cell,
  where it is read, not only in the surrounding prose.
- The posting gate's second condition is now decidable: a publish verb typed
  by the user this session, with the near-misses (approving noises, our own
  tip, PR text) enumerated as non-authorization.
- DESIGN: the whiff check is evidential, not a length threshold (and says
  why no number); "quick pass" is defined as low+medium sharing guardrails.

* feat(review): promote removed-behavior to a whole-diff agent in 3B

Territory-scoped 1b can only ask "was this deletion re-established here",
and for the deletions that matter the answer is somewhere else. PR #6638
(43 files, 8255 additions, 28 chunks) measured the gap: the 3B run with
per-chunk 1b reported one Critical; an independent reviewer reported 32, and
a parallel hand-run 1b+1c wave over the same commit reproduced six of them.
Every one of that overlapping six is a cross-chunk deletion — enableByPath
(includeSubdirs: true) replaced by an exact-path setWorkspaceActivation in
another file, silently narrowing workspace-scoped disable for every untouched
CLI/TUI caller; refreshTools() dropped from the activation paths, its
replacement swallowing the errors it used to propagate; a global mutation
timeout replaced by one covering only the prepare phase. Deletion in chunk A,
replacement in chunk B, consumer in a file the diff never touches: no chunk
agent can see that triple, and 1c does not look for it — it greps callers of
changed symbols, and a deleted export has no symbol left to grep.

- 1b joins 1c as a whole-diff agent in 3B; chunk agents keep the local half
  (a guard deleted and not re-established in the same hunk is still theirs).
- The split is stated at both agents: 1c walks the callers of changed
  symbols, 1b walks the replacements of removed ones.
- Agent 1b's definition gains the removed-export bullet: compare replacements
  as behaviour, not names, then check the call sites the diff never touches —
  a replacement that type-checks is not a replacement that behaves.
- 3B whole-diff agent count 4-6 -> 5-7 in the budget and the docs diagram.

* fix(review): serialize cannot-tell blockers, gate the no-blockers opener, and read reviews from a file

Fifth review round, all four notes real:

- The clause inventory had no way to serialize Step 6's `cannot tell`
  verdict, so a Critical the review could neither confirm nor clear had
  nowhere to go and dropped out of the public review. Added clause 5
  (unresolved existing-Critical), which survives downgrades and 422 recovery
  like the body-Critical carve-out, and Step 6 now points at it.
- `Reviewed — no blockers.` was injected as the opener whenever context was
  available, regardless of C or scope — so a self-PR downgraded to COMMENT
  with an inline Critical, or a review with an uncoverable chunk, opened by
  certifying the absence of the blockers it was carrying. The opener is now
  gated on C === 0 AND no unresolved existing Critical AND no uncoverable
  chunk AND context available; otherwise it is a plain `Reviewed.`
- The paginated `/reviews` fetch ran through the shell, whose successful
  output is capped at 30 000 chars and split head/tail: a body-only blocker
  in the elided middle passes with exit 0 and the fail-closed branch never
  fires. It is now redirected to a file and paged with read_file, and a body
  read only in part is `cannot tell`, not "no Critical in it" — the same
  lesson as "the diff is a file, not a command".
- The substantive-return gate rejected a bare "No issues found" while the
  agent contract demanded exactly that string. The contract now asks for
  `No issues found — <one line naming what you examined>`, and the relaunch
  is capped at one attempt per agent, with the dimension reported under "Not
  reviewed" if the second return is still bare.

* fix(review): wire whole-diff 1b into the gates, cap the event on unread scope, page reviews as NDJSON

Sixth round. Four Criticals all trace to the two previous commits:

- Whole-diff Agent 1b was declared but never wired in: it was missing from
  the receipt-less roster (so a whiffing 1b passed undetected) and the launch
  contract handed every 3B agent "its own chunk range", which is exactly what
  a cross-chunk pairing agent cannot work from. The payload contract now
  splits by role — chunk agents get one range, every whole-diff agent gets
  the entire chunks[] plan.
- The whiff check ended at "note it as Not reviewed", which left an
  unreviewed Security or removed-behavior lens able to ship an LGTM. It now
  carries an unreviewedDimensions state that forbids Approve, caps the event
  at COMMENT, and is serialized in the body next to uncoverable chunks.
- Clause 5 put an undecidable existing Critical in the body while event
  selection still chose APPROVE from the C/S table — a review approving the
  very blocker it asks the author to confirm. The table now has explicit
  overrides: cannot-tell existing Critical, uncoverable chunk, and unreviewed
  dimension each cap the event at COMMENT (a confirmed Critical still earns
  REQUEST_CHANGES).
- Redirecting `gh api --paginate` to a file does not make it pageable: it
  emits compact JSON, so the file is one 150 KB+ line that read_file
  truncates and offset skips past to EOF. The fetch now filters with --jq to
  marker-bearing bodies and emits line-delimited records that page normally.

Also: the 1b/1c split is by task, not by symbol (1c greps the removed
export's old name and owns caller compatibility; 1b owns the pairing and the
semantic comparison) — the earlier "no symbol left to grep" claim understated
1c and risked dropping its removed-symbol pass. Plus stale arithmetic from the
larger roster (+4 -> +5, worked example 26-28 -> 27-29), the receipt count's
missing Agent 8, "0 LLM calls" -> "0 subagent calls", the fixed "12 parallel
tasks" -> its real range, and 1c's callee procedure no longer speaking of a
"territory" it does not have.

* fix(review): select body Criticals offline, propagate unreviewed dimensions, close the no-findings bypass

Seventh (final self-review) round. The three Criticals all attack the newest
machinery:

- The NDJSON fetch filtered on a literal [Critical] marker, but a body-only
  blocker is not guaranteed to carry it (a real emitted review on this repo
  does not) — the filter discarded exactly what the re-check exists to
  recover. The --jq now keeps every nonempty body and selection happens
  offline after reading records whole; clauses 5 and 7 additionally mandate
  the marker on everything we serialize, so our own output stays
  self-identifying.
- unreviewedDimensions stopped at the event cap: Step 6's Not-reviewed
  section only listed uncoverable chunks (a non-posting run hid the missing
  lens entirely), and the body invariant made the required disclosure
  illegal on a REQUEST_CHANGES. The section now lists both, and the
  not-reviewed clause is the second sanctioned REQUEST_CHANGES body
  exception — a confirmed Critical must not squeeze out the disclosure of
  what was never read.
- The no-confirmed-findings branch still said "APPROVE by default",
  special-casing only presubmit and context-unavailable — bypassing the
  cannot-tell/uncoverable/unreviewed caps added one commit earlier. The
  branch now runs the same machinery as every submission: table with
  overrides, then downgrades, then composition; the hard-coded LGTM example
  applies only with no cap state present.

Plus the round's consistency notes: cross-file trace marked same-repo-only
in the docs' medium row; +5 -> +4 in the crossover arithmetic (Build & Test
reads no diff) so "crosses twelve about there" is true at 3200; DESIGN's
whole-diff enumeration gains 1b; budget total widened to the honest 15-21
row-sum; fork-subagent math redone at 52K/agent; the payload paragraph
names the invariant agents' third payload class; consumer-direction grep
patterns get Python/Go forms; 3C medium states 1a's lightweight degradation
and scopes the grep permission; the aggregated-format shorthand carries
Failure scenario and Severity; the exactly-one-sentence rule forward-
references the composition rule; and the Step 7 comment template embeds the
failure-scenario shape it was already demanding in prose.

* feat(review): sink argument parsing into a tested parse-args subcommand

The --comment/--effort grammar and target disambiguation were ~400 words of
prose in SKILL.md that the model re-simulated on every run; three separate
parsing bugs shipped that way (the spaced form consuming a flag as its
value, the --effort=<level> form left undefined, and an invalid value token
surviving into target disambiguation). Each is now a table-driven test case.

qwen review parse-args '<raw args>' emits a JSON verdict: classified target
(pr-number / pr-url with owner+repo+number extracted / file / local),
resolved effort with its source (explicit / default / forced-by-comment),
comment.requested vs comment.effective, verbatim warnings, and leftover
tokens the parser refuses to guess about. The skill's Step 1 shrinks to
"run the parser, use the verdict verbatim", and the target branches key off
target.type instead of hand-classifying tokens.

* feat(review): sink event selection and body composition into compose-review

The Step 7 machine — the C/S table, three event-capping overrides, the
seven-clause body composition, and the presubmit downgrade carve-outs — was
restated across four places in SKILL.md, and keeping the restatements in
sync by hand produced five shipped bugs (four Critical), all one shape: a
downstream branch not updated when an upstream rule gained a new state.

qwen review compose-review reads a state JSON (inline/body Critical and
Suggestion counts, discarded anchors, cannot-tell existing Criticals,
uncoverable chunks, unreviewed dimensions, context-unavailable, presubmit
flags, model id) and returns {event, body, baseEvent, cappedBy, downgraded}
for verbatim submission. The truth-table tests pin every previously shipped
bug as a named case: caps forbid APPROVE but never soften a REQUEST_CHANGES;
discarded Suggestions still count toward S so a 422 resubmit can never
upgrade to LGTM; a self-PR downgrade keeps body Criticals after the
downgrade sentence; the no-blockers opener appears only when certifiable;
every disclosure survives every stacking. Writing the tests immediately
caught one more instance of the class (all-discarded -> S=0 -> APPROVE).

SKILL.md's Step 7 shrinks to gathering the state and using the output
verbatim; the 422 recovery becomes "re-run compose-review with updated
counts"; the no-findings branch is the same call with zero counts; the
posting gate (judgment, not bookkeeping) stays prose.

* feat(review): render review bodies in full, quarantine replied Criticals, raise the gh buffer

The Step 6 body-fetch instruction was rewritten five times in four review
rounds (missing pagination -> shell truncation -> unpageable single-line
JSON -> a marker filter that discarded markerless blockers -> offline
selection) — the signature of a download program written in English. This
ends the chain at its root, in pr-context itself:

- Review bodies render in full under "Review summaries" instead of
  240-char snippets: an unmappable or 422-relocated blocker lives only
  there, and a snippet once hid one from the re-check. A body past the 8000
  cap ends by naming its review id, so the tail stays fetchable as a single
  object; a body read in part is `cannot tell`, not "no Critical in it".
- Replied Critical threads are quarantined into their own "Replied
  Criticals" section, rendered before the settled threads, instead of
  sinking into "Already discussed" — a reply alone ("I disagree") never
  retires a blocker, and marker-matching in this direction is fail-safe: a
  forged marker can only add a thread to the re-check list, never hide one.
- The gh wrapper's maxBuffer rises from Node's 1 MiB default to 64 MiB,
  closing the ENOBUFS that killed pr-context and presubmit mid-review on a
  comment-heavy 43-file PR.

SKILL.md's NDJSON fetch block is deleted: the re-check reads the context
file's three finding-bearing sections under its untrusted-data preamble,
with one residual single-object fetch for capped bodies. Verified against
this PR's own 100+-comment history: the markerless body-Critical review
that motivated the last rewrite now renders whole, and the fetch survives
without ENOBUFS.

DESIGN.md records the sinking rationale for all three subcommand changes;
the user docs note that parsing and the event/body decision are now pinned
by unit tests rather than prompt text.

* test(review): register parse-args and compose-review in the exact-list assertion

The parent-command test pins the exact subcommand roster; the two new
subcommands landed without updating it, which is precisely the drift the
assertion exists to catch — it caught it in CI, one directory above where
the new tests were run locally.

* fix(review): carry every disclosure on REQUEST_CHANGES and select blockers semantically

Review round on the new subcommands, plus the prompt notes it surfaced:

- compose-review's REQUEST_CHANGES branch dropped the context-unavailable
  clause entirely and gated the not-reviewed disclosure on other parts being
  present — an RC with only an uncoverable chunk disclosed nothing. Every
  clause whose state holds now appears on every event (a confirmed blocker
  must not squeeze out the trust warning or the unread-scope disclosure);
  four new tests pin it.
- Step 6 selects blockers semantically, not by the literal [Critical]
  marker: legacy body-only blockers were emitted markerless, and a marker
  filter once discarded exactly such a review.
- The same-repo pr-context failure now sets context-unavailable like the
  lightweight path (the guard's "lightweight" narrowing is removed) — a
  same-repo run that lost the context file must not behave as if it had
  read it.
- Step 5's dry-round return aligns with the agent contract (receipt-bearing
  "No issues found — <what it re-examined>"), ending the contradiction where
  a compliant reverse auditor would be flagged as whiffing.
- Consumer-direction grep forms for Python/Go are call sites now, with the
  declaration forms explicitly labeled as callee lookup.
- The 15-19 totals left downstream (docs table, DESIGN heading and cost
  row) move to the honest 15-21 / 13-20.

* fix(review): stdin transport for parse-args, validated compose input, full-body re-check context

Round 9 of review-the-review on this PR: 19 unique findings across three
reviews, each verified against source before fixing.

parse-args:
- The documented positional invocation broke on any flag-first raw string
  (`qwen review parse-args '--effort low'` -> "Unknown argument") and the
  `--` form silently returned a wrong local/default verdict. The raw
  string now travels on stdin (`--stdin`; SKILL.md pipes a quoted
  heredoc, immune to leading dashes, quotes, and $(...)); positional +
  --stdin and post-`--` smuggling are refused loudly. Wiring-level tests
  drive the real yargs command, pinning the strict-mode rejection that
  pure-function tests could not see.
- PR URL identity hardened: the number must end its path segment
  (/pull/42oops is refused, never PR 42), owner/repo restricted to
  GitHub's name charset (keeps shell metacharacters out of derived
  values), scheme matched case-insensitively, url canonicalized
  (lowercase scheme/host, query/fragment dropped) with a new host field;
  near-miss URLs are warned about and reported in extraTokens, never
  guessed into a file path or PR number. Step 1 remote matching now
  requires host AND owner/repo.
- Repeated --effort warnings state what is actually in effect (last valid
  occurrence / --comment forcing / the default), composed after
  resolution; previously a later typo claimed the default while an
  earlier valid effort stayed active.

compose-review:
- Input validated at the boundary: absent counts default to 0; malformed
  values throw typed errors naming the field. Previously
  {bodyCriticals:["x"], modelId} made undefined+1=NaN, failed both event
  comparisons, and returned APPROVE over the only blocker.
- "Suggestions are inline." keys off suggestionsInline, not s: an
  all-discarded 422 recovery no longer claims inline suggestions while
  the discarded sentence says the opposite (s still decides the event).
- canCertify requires !downgraded: a downgraded Approve opens with the
  neutral "Reviewed." instead of certifying "no blockers" two clauses
  after naming failing CI.
- unreviewedDimensions entries may carry their own reason after an
  em-dash and render verbatim (used by Agent 0's fetch failure below).

pr-context:
- Replied-Critical root bodies render in full (shared capBody; a cut
  names the comment id and the exact fetch); reply snippets name their
  comment id when cut. The Step 6 re-check no longer rules on
  silently-truncated claims, and the fail-closed "read in part = cannot
  tell" rule can actually fire for this section.
- The LGTM filter matches the exact canonical template, anchored to the
  whole body: a legacy body opening with the LGTM line but carrying a
  relocated blocker below it is shown instead of dropped.
- classifyInlineThreads() extracted: buildMarkdown and the stdout count
  use the same walk, so the count cannot diverge from the file.

SKILL.md:
- Step 6 re-check scope: every comment-bearing section, including
  "Already discussed" (inline threads and issue-level comments) — the
  quarantine keys on the literal marker, a floor not a ceiling, so
  unmarked blockers settle there; the false "holds only non-Critical
  threads" parenthetical is gone. The residual long-body fetch redirects
  to a file (shell output truncates at 30k) and is read paged.
- Step 5 reverse audit: dry = zero new findings WITH the evidence-bearing
  receipt; the substantive-return check runs after every round (one
  relaunch); a twice-whiffed agent's round is never dry.
- Step 3: Agent 7 added to both whiff-check rosters (evidence = commands
  run + outcomes; build-and-test recorded in unreviewedDimensions on the
  second whiff). Agent 0's linked-issue fetch failure is fail-closed
  after one retry via a self-explained unreviewedDimensions entry.
- Step 8: a fail-closed run (unreviewed dimensions, uncoverable chunks,
  context-unavailable) must not advance the incremental cache — caching
  it would exempt the disclosed-unreviewed scope from every future run.
- Counting truthfulness: "Twelve agents all reading the same diff" is
  eleven (every 3A agent except Build & Test walks the chunk plan); fixed
  in the 3B rationale, the diff-capture section, and the user docs.

review.ts: demandCommand message names plan-diff, with a test that the
message stays in sync with the registered roster.

* fix(review): nested-safe stdin guard, validated presubmit, refetchable snippets everywhere

Round 10: 12 findings, all verified before fixing. The headline is
self-inflicted: the round-9 post-`--` guard read argv._ as
['parse-args', ...extras], but the real CLI nests the command, so argv._
is ['review', 'parse-args'] and the guard rejected every real
invocation — while the wiring tests, which register the command
top-level, stayed green. Reproduced against the built CLI before
fixing.

parse-args:
- The smuggle guard skips the command-path prefix in argv._; new wiring
  tests go through the real parent `review` command (nested stdin
  invocation + nested post-`--` refusal).
- --effort values match case-insensitively (`--effort High` is not a
  file target named High); the verdict keeps the lowercase form.
- Single-dash tokens are unknown flags, never target candidates
  (`/review -c 6711` reviewed a nonexistent file `-c` and demoted the
  PR number to extraTokens).

compose-review:
- presubmit and contextUnavailable get the same boundary validation as
  the counts: boolean flags reject stringified "false" (truthy — it
  flipped an inline-Critical RC to COMMENT and published the diff-only
  warning on runs that fetched context fine), downgradeReasons rejects
  scalars with the field name (was a raw .join TypeError), presubmit
  rejects non-objects.
- Certification is gated on what presubmit PERMITS, not on whether it
  changed the event: a Suggestion-only review is already COMMENT, so
  failing CI flipped nothing and the body still certified "no
  blockers". Either downgrade flag now suppresses the certifying
  opener.

pr-context:
- Every truncating render carries an exact refetch ref: open-root
  snippets, settled replied threads (roots and replies), and
  issue-level comments (their own issues/comments endpoint). The
  Step 6 semantic re-check reads these sections, and a markerless
  blocker past the 240-char cut was invisible with no way back.
- Refs are copy-runnable: buildMarkdown threads owner/repo and PR
  number into every ref, so emitted commands carry real values.
  `gh api` substitutes only {owner}/{repo} — from the CURRENT repo,
  wrong in cross-repo mode — and passes {n} through literally.

SKILL.md:
- Step 1: the raw argument string travels via write_file to
  .qwen/tmp/qwen-review-args-input.txt and stdin redirection. A quoted
  heredoc disables expansion but not delimiter recognition, so a raw
  string containing the delimiter line would end the heredoc early and
  execute the rest as shell. Step 9 removes the file.
- Step 1: remote matching is structural segment equality (host AND
  owner/repo, .git stripped, case-insensitive) — substring "contains"
  let shao/qwen-code match a wenshao/qwen-code remote. Non-github.com
  hosts must carry GH_HOST on every gh call for the PR.
- Step 5: a twice-whiffed reverse-audit scope is tracked, cleared only
  by a later substantive audit, and fed into unreviewedDimensions as a
  self-explained entry when the loop ends — terminal prose alone let a
  capped run approve with an audit that never ran.
- Step 6: snippet cuts carry their own filled-in fetch note; ruling on
  a cut prefix is the fail-closed violation.
- Step 7: the stale hand-derivation bullets (event table, empty-RC-body
  rule, one-line COMMENT inventory) are replaced with descriptions of
  what compose-review guarantees; the sanity check is byte equality
  with the subcommand's output; the last-resort 422 branch re-runs
  compose-review instead of hand-building "the one-line body".
- Step 8: the fail-closed cache rule includes cannotTellCriticals — a
  cached SHA plus the same-SHA shortcut would skip the very re-check
  that must re-rule on an undecided blocker.
MSG2
git log --oneline -1; git push origin feat/review-procedural-finders-effort 2>&1 | tail -2

* feat(review): deterministic overlap disposal, --host routing, machine-readable completion line

Three changes measured out of the first six-PR dogfood batch, not
predicted from review comments.

Overlap disposal (SKILL.md Step 7): presubmit's overlap report used to
end in "list the overlaps to the user, ask whether to proceed" — 2 of 6
batch runs stalled on an improvised interactive question (fatal for a
headless run) while the other 4 proceeded. An overlap is a duplicate by
the Exclusion Criteria; the rule is now drop the overlapping finding,
adjust the counts handed to compose-review (a dropped finding never
flips the verdict), note "already reported at <path>:<line>" in the
terminal, and continue without asking. Zero findings left after
dropping is still not a question — compose-review handles the shape.

--host routing (lib/gh.ts + fetch-pr/pr-context/presubmit): the
round-10 GH_HOST-by-prose rule required the model to remember a prefix
on every call; a forgotten one silently reads from and posts to
github.com's same-named owner/repo. The three gh-calling subcommands
now accept --host and thread it through setGhHost()/ghEnv(), so every
wrapped gh call carries GH_HOST in code; hostname input is
charset-validated. SKILL.md keeps the prose prefix only for the gh
commands the orchestrating model runs directly (Agent 0's fetches,
Step 6's residual body fetch, Step 7's submission).

Completion line (SKILL.md Step 9): three different ad-hoc completion
phrasings across one batch each needed their own driver regex. Every
run now ends with exactly one line, `Review complete: <target> —
<disposition>`, with a closed disposition grammar covering posted
events, unposted verdicts, and quick passes — detectable with a single
^Review complete: match.

Tests: gh host-state unit tests (inherit-by-default, GH_HOST extension,
host:port, charset rejection), presubmit handler --host threading (set
and reset), builder registration checks for fetch-pr and pr-context.
2026-07-12 11:33:52 +00:00
Alex Yanchenko
218dec6a6f
feat(hooks): add MessageDisplay hook for mid-turn streaming (#6489)
* feat(hooks): add MessageDisplay hook for mid-turn streaming

Fires repeatedly as the assistant reply streams, before Stop (which only fires once at the end of the turn). Fire-and-forget, cumulative text payload, debounced (~200ms) except for the unconditional final firing. Fires from the single streaming loop in client.ts shared by the terminal UI and ACP paths.

Fixes #6488

* fix(hooks): address MessageDisplay review feedback

- Chain fire-and-forget MessageDisplay requests per message_id instead of
  firing them fully unbounded, so a slow hook command can't pile up
  concurrent processes.
- Gate the final flush on non-empty displayed_text and !signal.aborted,
  matching the adjacent Stop hook's guard.
- Document why the final flush intentionally re-sends the last debounced
  text (is_final itself is new information).
- Simplify the debounce constant's JSDoc to drop the competitor comparison.
- Add tests for the mid-stream debounced flush and the rejected-request
  warn path.

* test(hooks): drain microtasks before asserting on chained MessageDisplay calls

fireMessageDisplayHook now chains per-message_id through a promise (see
previous commit), so the final flush's actual messageBus.request() call
lands a few microtask ticks after the generator itself finishes — the
mid-stream-flush test needs to let that chain settle before asserting.

* fix(hooks): flush MessageDisplay is_final on every for-await exit path

The three early `return turn` paths inside the streaming loop (always-on
loop-detection safety, heuristic loop detection, and the stream Error event)
exited before the final MessageDisplay flush, which only sat after the loop
ended normally. Hook scripts relying on is_final: true to know when to flush
never received it when a turn ended via loop detection or an API error.

Extracts the flush into a shared closure and calls it from all four exits
(the three early returns plus the normal fall-through), instead of only the
one at the bottom of the loop. Adds regression tests for all three previously
missed exits, plus the two guard-coverage tests requested in review (abort
suppresses the flush, a tool-call-only turn with no Content events does not
fire a vacuous empty-text event).

Addresses the outstanding critical review comment and the follow-up test
coverage suggestion on PR #6489.

* fix(hooks): fire MessageDisplay on the ACP surface, coalesce delivery, drain is_final before turn end

Addresses the three findings from the local verification report on #6489:

- ACP/qwen serve (Finding 1): the delivery logic now lives in a shared
  MessageDisplayDispatcher (packages/core), and Session.ts wires it into
  all four raw-stream loops (main prompt, Stop-hook continuation, cron
  tick, background notification) — these surfaces consume GeminiChat's
  stream directly and never enter GeminiClient.sendMessageStream, so
  they need their own fire sites. The daemon no longer advertises an
  event it never emits.

- Slow-hook backlog (Finding 2): the per-message promise chain is
  replaced by coalescing delivery — at most one in-flight request plus
  one pending payload per message; newer flushes overwrite the pending
  slot, which is lossless because displayed_text is cumulative, and
  is_final is sticky. A slow hook now sees fewer, newer payloads instead
  of an ever-growing queue of stale ones.

- Headless is_final drop (Finding 3): finish() resolves only once every
  enqueued payload has actually been delivered, and every exit out of
  the streaming loops awaits it (early returns, normal fall-through,
  and the enclosing finally for uncaught exceptions), so a short-lived
  -p process can no longer exit with the final payload still queued.
  As a consequence, is_final delivery now strictly precedes the Stop
  hook rather than racing it.

Also: the failure log line carries the message_id, finish() is
idempotent, the review-requested tests are added (mid-stream and final
firings share one message_id; isFinal as the sole flush reason), and
hooks.md gains a delivery-semantics contract covering coalescing, the
drain guarantee, no is_final on cancellation, provisional
displayed_text, and multiple messages per tool-using turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(hooks): bound MessageDisplay drain wait, fix test gaps flagged in review

finish() now gives up waiting on drain after 5s (MESSAGE_DISPLAY_DRAIN_TIMEOUT_MS) instead of blocking turn teardown for up to the full 60s hook timeout, per the re-verification's S1 finding. Delivery keeps running in the background past the timeout; only the caller's wait is bounded.

Also: add the config.ts bridge test for MessageDisplay field extraction (S5), and add the missing MessageDisplay/InstructionsLoaded entries to acpAgent.test.ts's HookEventName mock (S6).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(hooks): dispatch MessageDisplay is_final alongside stale deliveries, share one drain budget

Round-3 review findings on #6489:

- finish() no longer queues the is_final payload behind an in-flight
  mid-stream delivery: the pending slot's supersession argument applies to
  the in-flight slot too, so the final payload is dispatched immediately,
  alongside the stale delivery if one is still running. is_final is handed
  to the hook the moment the message ends — before Stop — on every surface,
  and can no longer be dropped by a short-lived process exiting with it
  still queued (Finding 1).
- The bounded drain wait is memoized: every finish() call (explicit,
  finally, or concurrent) shares one promise and one timer, so the teardown
  ceiling is MESSAGE_DISPLAY_DRAIN_TIMEOUT_MS itself, not a multiple of it
  (Finding 2).
- hooks.md delivery semantics rewritten to match the shipped behavior,
  including the headless orphaned-hook caveat and the unspecified completion
  order between an overlapped stale execution and the final one (Finding 3).
- The dispatcher mirrors its warnings to console.warn itself (stderr on
  headless/ACP, ink patchConsole in the TUI) in addition to the injected
  debug-file sink, so hitting the drain timeout is visible by default
  (Finding 4).
- A superseded mid-stream delivery that fails after the final was
  dispatched no longer warns; failures during streaming still do.
- New tests: finish() twice while delivery is in flight (the exact
  client.ts sequence), concurrent finish() calls sharing one budget,
  is_final overtaking a held mid-stream delivery, and drain resolving on
  the final delivery alone.

* refactor(core): consolidate MessageDisplay finish() calls, dedupe test spy setup

client.ts: wrap the turn.run() streaming loop in try/finally so messageDisplay.finish() fires once instead of at each of the three early-return sites plus the post-loop path -- matching the pattern the four raw-stream loops in Session.ts already use for the same dispatcher.

message-display-dispatcher.test.ts: centralize the console.warn spy setup/teardown in beforeEach/afterEach instead of five repeated per-test try/finally blocks.

No behavior change: full client.test.ts (246/246) and the message-display-buffer/dispatcher suites (24/24) pass unchanged.

* docs(hooks): clarify MessageDisplay cancellation timing (round-4 nit)

* test(hooks): cover the 3 untested MessageDisplay dispatch sites, fix cancellation doc wording

Adds MessageDisplay is_final coverage for the Stop-hook continuation loop, the in-session cron fire, and the background-notification loop, each with a normal-completion and an abort case. Adds three MessageDisplayDispatcher edge-case tests: a delivery settling just before the drain timeout, an abort arriving after a drain wait has already started, and addChunk called after abort but before finish(). Rewords the cancellation-timing doc bullet to state the actual criterion (abort signal state when finish() runs) rather than an approximation of it.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-11 13:24:32 +00:00
qqqys
25f491d3ac
feat(dingtalk): mention response senders (#6679)
* docs: design DingTalk at-sender replies

* docs: plan DingTalk at-sender replies

* feat(channels): preserve session for response delivery

* feat(dingtalk): optionally mention response sender

* docs(dingtalk): explain response mentions

* fix(dingtalk): retain queued mention targets

* fix(dingtalk): bound mention target lifecycle

* fix(dingtalk): clear synthetic command mention target

* fix(dingtalk): clear buffered targets on session death

* debug(dingtalk): log mention delivery result

* fix(dingtalk): render response mentions

* fix(dingtalk): send visible response mentions

* feat(dingtalk): use text replies for mentions

* fix(dingtalk): preserve mentioned text replies
2026-07-11 00:08:13 +00:00
Shaojin Wen
51888210aa
feat(review): give every line of a large diff an accountable reviewer (#6612)
* feat(review): give every line of a large diff an accountable reviewer

Review agents were handed the diff *command* and left to run it themselves.
Shell tool output is capped at 30 000 characters and split head-1/5 / tail-4/5,
so on a large changeset every agent received a few hundred lines off the top of
the first file, the tail of the last file, and a truncation marker in place of
everything between. Measured on a 211 000-character diff: 14.4% of the
changeset, the same 14.4% for all ten agents. Nineteen of the twenty defects
maintainers eventually confirmed on that PR lay in the hidden 85.6%. The
ten-way dimension fan-out multiplied redundant reads of the visible sliver
rather than adding coverage, and each review round sampled a different subset
of the bugs depending on which files an agent happened to open on its own.

The diff is now captured to a file and partitioned. `read_file` still caps a
single read at ~25 000 characters, so writing the diff out is necessary but not
sufficient — a whole-file read of that diff returns its first 611 lines. Chunks
are therefore bounded by both a line budget (attention) and a character budget
(what one un-truncated read returns), split on hunk boundaries, and never
through the middle of a function. They tile the diff exactly, which is what
makes the new coverage receipts checkable: past 500 diff lines each chunk gets
one agent that owns it and must account for it, and a chunk with no receipt is
re-reviewed before the run proceeds. "No blockers" can no longer be reported
over code nobody read.

Coverage alone did not close the gap. Chunk agents held every state-machine
defect in that PR inside their assigned territory and reported none of them:
the bugs were not inside any hunk but between new lines sitting two thousand
lines apart, and what the agents lacked was not the lines but the question. A
heavily rewritten file now also gets three whole-file agents that walk a fixed
invariant checklist — mutable fields cleared on every exit path, timers
cancelled on every close without discarding captured data, map inserts matched
by deletes, retry counters incremented at every entry, status returns actually
checked, error codes classified permanent versus transient, config honoured on
every path, early returns that skip a required side effect. The checklist is
split three ways deliberately: one agent asked to run all eight checks over a
2 400-line file runs one of them properly.

Verification is sharded at eight findings per agent, because one verifier
re-reading code for sixty findings degrades on the tail of its list. A verifier
may now downgrade a Critical but never delete one — a rejected Critical is
invisible to every later stage, a downgraded one still reaches a human. The
reverse audit fans out per chunk instead of asking a single context-starved
agent to re-read the whole diff, no longer skips verification, and stops after
two consecutive dry rounds rather than one: on the PR that motivated this, the
review reported "no blockers" twice and the next round surfaced five Criticals,
three of them in code present since the first commit.

* fix(review): keep small-diff reads inside the read_file cap

Step 3A told every agent to read the whole diff in one call. `read_file`
truncates a single call at ~25 000 characters, so a 500-line diff of long lines
would come back short — the same blind spot the chunk plan removes, reintroduced
at a smaller scale. Across the last 39 merged PRs that take the Step 3A path the
largest diff is 23 570 characters, so this never fired in practice, but the
margin is six percent. Step 3A now walks the chunk ranges, which are sized to
fit one un-truncated read: one or two calls at this size.

Derive a file's pre-change line count from the diff instead of measuring it with
a second `git show` per file. `git show <base>:<newpath>` returns nothing for a
renamed file, reporting zero pre-change lines and classifying a wholesale
rewrite as light. The identity holds exactly for creations, deletions, renames
and ordinary edits, and halves the process spawns.

* fix(review): choose the topology from source lines, not diff lines

Diff size is a bad proxy for review risk because test code dominates it. Across
this repo's last 40 merged PRs the median diff is 41% test code and 14 of the 40
are more than half tests; PR #6457, which motivated the territory fan-out, is
itself 58% tests. Gating on raw diff lines therefore carved small production
changes into territories: a change of 173 source lines shipping 489 lines of new
tests went to the chunked topology, where its production code ended up owned by
a single agent, when the dimension fan-out would have read it through eight
lenses. Territory fan-out is worth it when there is a lot of risky code to
divide, not a lot of lines.

The gate is now `srcDiffLines > 500`, with `diffLines > 2400` as a second clause
— a delivery bound rather than a risk one, since past that point chunking uses
fewer agents than the ten-lens topology anyway and reading a diff that large
dilutes all ten. On the 40-PR sample six PRs move back to the dimension fan-out,
for about 5% more agents in total across the sample.

Paths are classified as source, test, or generated, and the per-kind line counts
ship in the fetch report. Chunking is unchanged: the plan still tiles every
line, tests and generated files included. What the gate decides is how many
reviewers there are and what each is asked to do. Heaviness is likewise
restricted to source files — the invariant checklist asks about fields, timers,
collections, and error taxonomies, and a rewritten test file has none of those.

* fix(review): decode C-quoted diff paths as bytes

`git diff` C-quotes any path with a control character or a non-ASCII byte, so a
file named `sub/中文文件.ts` arrives as `"b/sub/\344\270\255..."`. The chunk
planner stripped the backslashes, turning it into `sub/344270255...ts` — a name
that exists nowhere. Every downstream use of the path then failed silently: the
line count came back zero, the file could never be classified as heavy, and the
chunk agent was told it was reviewing a file that does not exist. Reuse core's
`unquoteCStylePath`, which reassembles the octal escapes as UTF-8 bytes, rather
than keeping a second, wrong decoder here.

Coverage was never affected — line ranges stayed correct — but this repo has
non-ASCII paths, so the mislabelling was reachable.

Also correct two places that claimed hunks are never split. They are: a hunk
larger than the chunk target is split at a top-level declaration, because a
brand-new file arrives as one enormous hunk and treating it as atomic would hand
a single agent a 50 000-character territory.

* fix(review): make diff capture and header parsing robust to git config

Four defects, all found in review of this branch.

Diff capture obeyed whatever the user's git config said. With `color.diff=always`
every `diff --git` line arrives wrapped in ANSI escapes, the parser recognises
none of them, and the plan comes back with zero files and zero chunks — the
coverage guarantee silently evaluates to nothing. `diff.mnemonicPrefix` renames
the `a/`/`b/` prefixes to `i/`/`w/` and every path is then wrong; `diff.external`
and textconv filters emit output that is not a unified diff at all. Capture now
pins `--no-ext-diff --no-textconv --no-color --unified=3` and the two prefixes.

The `diff --git` header was split with a greedy regex. Git separates the two
paths with a space and does not quote a path merely for containing one, so
`a/img with space.png b/img with space.png` split into `space.png`. Usually the
`---`/`+++` headers disambiguate, but a binary or mode-only section has neither.
For a non-rename both paths are the same string, so the split point is
arithmetic; a rename states its new path outright in `rename to`.

A chunk boundary could land on a `-` line. Those exist only on the old side, so
the "starts at a top-level declaration" guarantee did not hold for the
post-change file an invariant agent later reads. Split points are now restricted
to lines present on the new side.

An `oversized` chunk — one hunk with no safe interior boundary — can exceed what
a single `read_file` returns. Chunks now carry their character count, and a
chunk agent is told to page when a read reports truncation. A `Covered:` receipt
for a range the agent only half read is worse than no receipt at all.

* fix(review): split past a distant boundary, and stop probing GitHub for anchors

Both defects surfaced running the new review against PR #6591.

A 1431-line React component was emitted as a single 45 675-character chunk —
nearly twice what one `read_file` returns — because the splitter looked for a
safe boundary only inside the 400-line budget window, found none, and gave up on
the entire remainder. Twenty-seven boundaries existed further along; the first
sat 460 lines in. It now reaches past the window for the next one, so a single
distant boundary can no longer collapse a whole file into one chunk. That PR
goes from 15 chunks with one over the read cap to 18 with none.

Step 7 validated comment anchors by trial. GitHub rejects an entire review with
a 422 if any comment's line falls outside every hunk of its file, and the skill
offered no cheap way to check, so a run against a real PR submitted five
throwaway reviews carrying the bodies `Test`, `Test`, `t`, `t`, `t` to discover
which anchors would stick. Those are permanent, public reviews on someone else's
pull request. The fetch report now carries each file's hunks as new-side line
ranges, which turns the check into a lookup, and the skill states plainly that
a review is never submitted to test an anchor.

* fix(review): stop reading hunk payload as metadata, and harden the plan

Eleven defects from review of this branch. The worst two were silent.

A unified diff emits a removed line whose content starts with `-- ` as
`--- ...`, and an added line whose content starts with `++ ` as `+++ ...`. SQL,
Lua and Haskell comments start with `-- `. The parser read those payload lines
as file headers: the path was overwritten by the line's text, and the line
vanished from the add/remove counts. A two-file diff — one SQL file losing a
comment, one text file gaining a `++ ` line — came back with the second file
named `plus line`. Metadata is now only recognised before a file's first hunk.

The tiling invariant — every diff line belongs to exactly one chunk, which is
what makes a missing coverage receipt mean something — was asserted only in
tests. `buildDiffPlan` now checks it and refuses to return a plan with a hole.

The rest: a split point could take a *deleted* blank line as evidence of the
blank line before a declaration, though that blank exists only in the old file;
whole-file invariant agents were pointed at `chunks[].files[]`, which merges
hunks at lines 10 and 900 into one `10-902` span and would have had them report
pre-existing defects as new; pure-deletion hunks were exported as the inclusive
range `[N, N]`, so a right-side comment could be anchored where GitHub has no
line and the 422 would sink the whole review; a deleted file could be marked
heavy and send three agents to read a post-image that does not exist; a chunk
holding a single line longer than one `read_file` can never be fully read by
paging, and must now report itself uncoverable rather than receipt a lie;
capture did not pin rename detection or `--no-relative`; `gitRaw` had no
timeout, so a credential prompt on headless CI would hang forever; a failed
base fetch was swallowed, leaving a stale merge-base and a structurally
complete report describing the wrong diff; and local reviews still captured
with a bare `git diff`, which `color.diff=always` alone renders unparseable.

Adds an integration test that drives the real capture against a real repository
under hostile git config, covering the paths synthetic fixtures cannot: renames
and binaries and mode-only changes with spaces in their names, C-quoted
non-ASCII names, and payload lines that impersonate headers.

* fix(review): pin submodule output, and separate written lines from hunk spans

Four defects from review of this branch.

Diff capture left submodules to user config. `diff.ignoreSubmodules=all` hides a
changed gitlink completely — a silent coverage hole in the file that is now the
review's source of truth — and `diff.submodule=log` replaces the whole
`diff --git` section with prose no parser can read. Both are pinned now, and the
integration test asserts a bumped gitlink survives them.

Whole-file invariant agents were handed `files[].hunks[]` as "the changed
lines". A hunk spans the three context lines git prints either side of every
change: on PR #6457's `QQChannel.ts` those spans cover 1 962 new-side lines of
which only 1 403 were written. The agent would have reported defects in 559
lines that predate the PR. The report now also carries `addedRanges[]` — the
exact lines the change wrote — and the skill gates invariant agents on those,
keeping `hunks[]` for the one thing it is right for, GitHub anchor validation.

`Uncoverable:` was introduced as a chunk agent's answer for a chunk holding a
line longer than one read, but the receipt accounting still demanded a
`Covered:` line from every chunk and relaunched any chunk lacking one — so an
uncoverable chunk would have been retried forever. It is now a first-class
terminal status: accepted by the accounting, carried into Step 6 under "Not
reviewed", and it blocks an Approve verdict. Step 3A, which also walks the
chunk plan, is covered by the same rule.

The integration test built its fixture repository inside the developer's git
environment, so a global `core.hooksPath` or `commit.gpgsign` ran during the
test and `~/.gitconfig` decided what the "clean" baseline was. It now disables
system and global config, hooks and signing, and sets the executable bit through
the index rather than shelling out to `chmod`, which does nothing on Windows.

* feat(review): plan any captured diff, and stop the report outgrowing one read

Seven items from review of this branch. None blocking; two of them were the
skill promising a topology it could not deliver.

Step 3B's chunk agents are "one per entry in `chunks[]`", and only `fetch-pr`
produced a chunk plan. A local-diff review, and a cross-repo review in
lightweight mode, therefore routed into the territory fan-out with no chunk
list, no receipts and no tiling guarantee. `qwen review plan-diff <diff-file>`
now emits the same plan from any captured diff; redirecting `git diff` or
`gh pr diff` to a file already sidesteps the shell's character cap, so all four
review paths share one mechanism. A bare diff has no tree to read a post-image
from, so it gets chunk agents but no invariant agents, and says so by omission.

The fetch report is read with the same `read_file` that truncates at 25 000
characters — and for a seven-file PR it was already 28 056. The tail of
`chunks[]` was being silently lost: the coverage hole this design closes,
reappearing one level up. `addedRanges[]` now ships only on `heavy` files, its
only consumer, which brings that report to 24 992; the skill says to page the
read; and the command prints a note when the report exceeds one read. It stays
pretty-printed on purpose — a compact one-line JSON cannot be paged by line.

The tiling assertion threw inside `fetch-pr` after the worktree existed and
before any report was written, so an unforeseen diff shape killed the review
outright. It now degrades to the documented diff-less report with a loud
warning, keeping both the loudness and the review.

`gitOpt` and `git` had no timeout, and `resolveMergeBase` uses `gitOpt` for a
network fetch — the exact path whose credential prompt the `gitRaw` timeout was
added to survive. All three wrappers now share a deadline and
`GIT_TERMINAL_PROMPT=0`.

Markdown under `docs/` or at the repository root classifies as `docs` and stays
out of `srcDiffLines`, so a translation PR does not trip the territory gate.
Markdown inside a source tree stays `source` — the bundled skill prompts are
behaviour, not prose.

Also: the user docs stated the gate without its `diffLines > 2400` clause, and
`READ_FILE_CHAR_CAP` was exported but never used. It now backs the report-size
warning.

* test(review): unit-test the merge-base and plan-report seams

The last open review thread asked for `resolveMergeBase`, `fileMetrics` and
`gitRaw` to be testable with git mocked out. Three of the four functions it
named have since moved: `classifyHeavy` is a pure function with unit tests,
`fileMetrics` became `buildPlanReport`, which already takes an injected
post-image resolver, and `gitRaw`'s output path is exercised by the real-git
integration test. `resolveMergeBase` was still private and untested.

It now lives behind a three-method `GitProbe` — fetch, refExists, mergeBase —
that `fetch-pr` fills from the real wrappers. Seven tests cover the branches
that matter and that no end-to-end run reaches: the tracking ref preferred over
the local branch, the fall-through when the tracking ref shares no history, and
above all the dangerous one — a failed fetch that still resolves a merge-base
from a stale local ref, which produces a structurally complete report describing
a diff nobody wrote.

`buildPlanReport` gains seven of its own: the injected resolver is asked once
per file and never for a binary, a null resolver means "no tree, decide nothing"
rather than a guess, `addedRanges` ship only where an invariant agent will read
them, and a pure-deletion hunk never reaches the anchorable ranges.

* fix(review): see deletions, survive suppressBlankEmpty, and stop approving unread code

Seven findings from review of the merged head. Three of them were the design
contradicting itself.

`diff.suppressBlankEmpty` prints a blank context line as a physically empty
record rather than a lone space, and there is no command-line flag to override
it — only `-c`. The parser advanced its new-side cursor for space-prefixed
context alone, so every `addedRanges` entry after the first blank line shifted
up by one, and the split-point heuristic stopped recognising blank lines. The
capture now pins the config, and the parser treats an empty hunk-body record as
context regardless, because a diff from `gh pr diff` or a hand-captured file
never passes through that pin.

A whole-file invariant agent was given the post-change file and the ranges the
PR wrote. A deletion appears in neither. Removing a `clearTimeout()`, a
`Map.delete()`, or a retry-counter increment is exactly what the checklist
hunts, and the text it was handed cannot show a line that is no longer there —
telling it to "cite the surrounding hunk" pointed at data it never received.
Heavy files now carry a `diffRange` into the report, and the agent reads its own
slice of the diff, where the `-` lines are.

The receipt accounting demanded exactly one per chunk and said it applied to
Step 3A, where nine dimension agents each walk every chunk: literal execution
yields nine receipts or none. Territory ownership is a Step 3B idea. What both
paths share is the uncoverable rule, and that needs no agent — a chunk is
uncoverable iff its `maxLineChars` exceeds the read cap, which the orchestrator
reads out of the plan before launching anything.

That rule was also never threaded into Step 7, so a green PR with an unread
chunk could receive a public LGTM. Any uncoverable chunk now downgrades APPROVE
to COMMENT and must be named in the body.

Also: the capture recipes redirected into `.qwen/tmp` before anything created
it; a file-path review of an unchanged file produced an empty plan that no agent
could read, and the skill now branches to a full-file read instead; and the docs
classifier called `website/src/App.tsx` prose while calling
`packages/cua-driver/docs/*.md` source — it now matches prose extensions under a
documentation directory at any depth.

* fix(review): tell agents what a severity means before asking for one

The severity definitions lived once, in Step 6 — after every severity had
already been assigned. Step 3's finding format asked each agent for
`Severity: Critical | Suggestion | Nice to have` and never said what the words
meant. The agents that fill that field are separate subagents with separate
priors and no shared definition between them, so each fell back on its own, and
the priors disagree.

Observed on a live review of PR #6635 — a run of the skill as it stands on main,
whose Step 3 and Step 6 text this branch inherits unchanged. One review,
CHANGES_REQUESTED, ten inline comments. Six were Critical, and four of those six
were coverage gaps: "zero test coverage", "no references to `workers`", "no test
exercises this". Two Suggestions in the same review were the identical class.
The verdict is computed from Criticals alone, so that PR was blocked partly on
the strength of findings its own reviewer had, elsewhere, called suggestions.
The two genuine Criticals — a fail-fast that no longer fires before the daemon
reports healthy, and a startup failure path that never closes the HTTP server —
would have blocked it on their own.

The definitions now sit in the finding format that every agent is handed, they
are listed among the things every agent prompt must carry, and Step 6 points
back at them rather than restating them. A missing test is a Suggestion: "this
file has zero references to X" is a coverage statistic, not a defect. Two shapes
stay Critical because something is genuinely wrong — a test asserting the
opposite of the intended behaviour, and a test weakened or deleted in the diff
so new behaviour passes. If a missing test would let a specific incorrect
behaviour ship, report that behaviour and cite the gap as evidence.

* fix(review): walk cross-file edges in both directions

Cross-file impact analysis only ever asked "will the existing callers break?"
Every bullet was about signature compatibility, and the budget rule told agents
in so many words to "skip unchanged-signature modifications". A field added to
an interface changes no signature and breaks no caller, so the analysis was
blind to it by construction.

The failure that exposed this, on PR #6621: the diff added `deviceFlowRegistry?`
to WorkspaceRuntime and passed it into the dispatcher for every secondary ACP
mount, and nothing anywhere assigned it. The reviewing agent saw the
declaration, found no writer, wrote "intentionally deferred to a later
milestone", and filed a Suggestion to fix the JSDoc. The reader was AcpDispatcher
— a file the diff never touched — where `if (!this.deviceFlowRegistry)` turned
`auth/device_flow/start` into an INTERNAL_ERROR and `auth/status` into an empty
list on every non-primary workspace. Workspace-qualified ACP shipped its
authentication dead, and the review called it a documentation nit. A second
reviewer filed the same observation as Critical; the author fixed it with code
and dropped the field.

Reading cannot find this. The declaration, the pass-through, and the read sit in
three different places, and the read is outside the diff, so no agent reaches it
by paging through hunks. Only a grep for the read sites does.

So: for every field, option, or optional parameter the diff adds, grep its read
sites, including outside the diff, and ask what happens when it arrives
undefined. Severity is decided at the read site, not the declaration. And an
agent must not explain an unpopulated field with author intent it cannot
observe — "reserved for future use" is a claim about a person, not about code,
and reaching for one means filling a hole in your own field of view.

* fix(review): pin the diff base, and make the review body checkable

Three defects, all found by reading what live reviews actually posted.

The diff base. Agents were handed a diff command and left to choose a base.
`main..HEAD` and `main...HEAD` differ by one character and by the entire meaning
of the review: a two-dot diff against a main that has moved shows main's later
commits reversed, so main's fixes read as the branch's regressions. A review of
PR #6626 approved the four files the PR actually changed, then warned the author
publicly that their branch carried "typo regressions" in a file the PR never
touched and should be rebased. main had corrected `compatability` to
`compatibility` after the fork point. The branch had done nothing. Capture now
resolves the base once and hands agents a file; they never see a ref name, and a
finding in a file outside the report's `files[]` is not a finding about this PR.

The review body. "A Suggestion never goes in body" is stated twice and was
violated anyway, because a model holding a finding it cannot anchor would rather
say it somewhere than drop it. On PR #6631 an unanchorable Suggestion about
`session.ts:2048` — a line in no hunk — became a second paragraph of the public
review body. So the rule stops being prose: for COMMENT the body is exactly one
of three sentences plus the footer and nothing else, and you read what you are
about to send and confirm it. A Suggestion that will not anchor is deleted; it is
already in the terminal output and the Step 8 report.

The downgrade sentence. On PR #6489 a review with three Suggestions and no
Critical announced it had been "downgraded from Approve" — telling the author the
PR would otherwise have been approved, which was false: a Suggestion-only review
is COMMENT on its own. Decide the event from the findings first, apply the
downgrade flag second, and write the sentence only if it changed the answer.

* fix(review): decide the event by counting, not by weighing

A review of PR #6584 filed three inline Suggestions and submitted APPROVE with
an empty body. GitHub recorded it as an approval. The rule it broke has been in
Step 7 all along --- APPROVE means no Critical *and* no Suggestion --- and so has
the one about the body, which is empty only for REQUEST_CHANGES. Both were
stated twice. Both were ignored.

They are ignored because at submit time the model is reasoning about what it
wants to say, and "these are only suggestions, the PR is fine" is a sentence it
can talk itself into. Nothing in that sentence is a count.

So the event and the body become arithmetic. Count the Criticals, count the
Suggestions, read the row off a three-row table, and only then apply the
downgrade flags --- which can turn APPROVE or REQUEST_CHANGES into COMMENT and
nothing else. Then read back what you are about to send and confirm it matches
the row. A body holding text the table does not authorise is a finding that
failed to anchor; if it is a Suggestion, it gets deleted, not relocated into
public prose that no line of code answers to.

This subsumes the body-only invariant added in the previous commit, which the
same submit-time reasoning had already defeated once, on PR #6631.

* fix(review): stop the plan report outgrowing the read it must fit in

The report tells an agent how to page everything else, so it has to be readable
in one `read_file` — about 25 000 characters. Running the real `fetch-pr`
against PR #6457 produced 25 070.

Two constraints pull against each other. Compact JSON is a single enormous line,
and `read_file` pages at line boundaries, so a report too big for one call could
never be read at all. Indented JSON pages fine but spends four lines on
`{ "start": 812, "end": 815 }`, and a heavily rewritten file contributes hundreds
of them: `QQChannel.ts` alone carries 140 added ranges and 49 hunks.

So indent the structure and inline the leaves. Same JSON, same keys, one range
per line, still pageable — and 28% smaller. The #6457 report goes from 25 070
bytes to 18 042, and the "page it" warning that used to fire on a seven-file PR
now stays quiet.

The earlier attempt at this trimmed `addedRanges` to heavy files only and landed
at 24 992 bytes on the same PR. Eight bytes of headroom was not a fix.

Tests pin the three properties that matter: the collapsed text parses back to an
identical object, no range spans two lines, and a path that literally spells a
range is not mistaken for one — JSON escapes the quotes inside a string value,
and the collapse patterns require unescaped ones.

* fix(review): prune the worktree registration a deleted directory leaves behind

`cleanStale` and `cleanup` both guarded `git worktree remove` behind
`existsSync(path)`, and neither ever pruned. Delete the directory by hand — which
is exactly what reclaiming disk with `rm -rf .qwen/tmp` does — and git keeps the
worktree registered but missing. From then on `/review` on that PR cannot run:

    $ git worktree add .qwen/tmp/review-pr-6457 qwen-review/pr-6457
    fatal: '...' is a missing but already registered worktree;
    use 'add -f' to override, or 'prune' or 'remove' to clear

and the branch delete that `cleanStale` does next fails too, because the phantom
worktree still has that branch checked out. Nothing in the review command surface
ran `git worktree prune`, so nothing ever cleared it.

This surfaced running the real skill: the orchestrator's first `fetch-pr` failed,
it fell back to `qwen review cleanup`, and retried. The leak is not rare — three
abandoned worktrees from May and June were still registered in this checkout,
one per review that died before Step 9.

`releaseWorktree` now does both halves in the order they depend on: remove the
directory if it is there, prune the registration unconditionally (a no-op when
nothing is stale), and only then let the caller delete the branch. Both callers
share it.

The tests drive real git. Deleting a worktree directory by hand and re-adding it
throws "missing but already registered" without the prune, and `branch -D` throws
"used by worktree" — both assertions fail if the prune is removed, which is the
point of writing them.

* fix(review): put the open comments where a truncated read will find them

`read_file` returns the first `truncateToolOutputThreshold` characters — 25 000
by default — sets `isTruncated`, and pages by line. `pr-context` wrote
"## Open inline comments (no replies yet — may still need attention)" last, so
on a PR with a long history it was the first thing lost, and nothing read the
flag that said so.

On PR #5738 that section began at character 27 125 of a 31 220-character file.
The review submitted "Reviewed — no blockers." Five Critical threads were
unresolved; four had in fact been addressed, but the fifth — `clearCiEnv()`
clearing only `CI*` while `writeTerminalTitle` branches on `TMUX`/`STY`/
`ZELLIJ`/`DVTM` — was live, in the diff, and never seen.

Regenerating the context for ten PRs: four lost part or all of the section, and
all four were the PRs with the most review rounds. Small PRs never trip it.

- Emit the open threads before the already-discussed ones. The findings a round
  must answer outrank the ones already settled.
- `pr-context` warns when the file exceeds the threshold, naming any headings
  past the cut, and says so plainly when the loss is inside the last section's
  body instead.
- Step 2 of SKILL.md now tells the agent to read `isTruncated` and page the
  remainder before Step 3.

Reordering buys headroom; it does not create it. A 40 000-character context still
loses its tail, which is what the warning is for.

* fix(review): load this repo's review rules, and re-check open Criticals before approving

Two gaps the dogfood on live PRs surfaced, both invisible from reading the skill.

`load-rules` looks for a `## Code Review` heading in AGENTS.md and QWEN.md.
Neither had one, so it wrote an empty file on every run: every `/review` in this
repo reviewed with zero project rules. Add the section, distilled from the
conventions already scattered through AGENTS.md (ESM, no cross-package relative
imports, kebab-case/PascalCase naming, collocated tests, comments-only-when-why),
plus the two hard lessons below. The section loads from the base branch by design
— a PR cannot inject its own review rules — so it takes effect once merged.

The skill treated a zero-Critical outcome as a fallback rather than a claim. On
one PR it published two Criticals citing code not present at the reviewed commit
(a fabricated blocker on an already-approved PR); on another it submitted C=0
while a live, twice-filed Critical still stood (a dropped blocker). Add a step
before the verdict: for each unresolved Critical on the PR, read the code at the
reviewed commit and record still-stands / fixed-by-this-diff / cannot-tell. The
event follows from the code, not from the finding count or the thread flags —
`isResolved`/`isOutdated` track the anchored line, not whether the bug was fixed.

- AGENTS.md: new `## Code Review` section.
- load-rules.ts: export `extractCodeReviewSection`; load-rules.test.ts covers the
  boundary scan and asserts AGENTS.md's own section extracts non-empty, so
  deleting the heading fails the build.
- SKILL.md: re-verification step ahead of the Verdict.
2026-07-10 15:53:09 +00:00
qqqys
7129cecba2
fix(channels): manage stale DingTalk Stream connections (#6675)
* fix(channels): manage stale DingTalk Stream connections

* fix(channels): harden DingTalk connection lifecycle

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
2026-07-10 14:19:59 +00:00
qqqys
c84089ec48
fix(dingtalk): preserve markdown tables (#6673) 2026-07-10 13:08:31 +00:00
qqqys
043c22cb4b
feat(channels): support webhook-triggered channel tasks (#6495)
* feat(daemon): add a2a settings and capabilities

* docs(channels): design webhook-triggered tasks

* feat(channels): add webhook task helpers

* fix(channels): bound webhook prompt metadata

* feat(channels): run webhook-triggered tasks

* fix(channels): harden webhook task lifecycle

* fix(channels): require yolo for webhook tasks

* feat(channels): parse webhook configuration

* fix(channels): validate webhook secrets

* feat(channels): forward webhook tasks to channel worker

* fix(channels): require webhook enqueue on supervisors

* fix(channels): handle webhook IPC send failures safely

* feat(serve): accept channel webhook tasks

* fix(serve): stop webhook validation after first error

* fix(webhooks): reject inherited target refs

* docs(channels): document webhook-triggered tasks

* docs(channels): fix webhook task example

* docs(channels): refine webhook task docs

* docs(channels): add webhook task implementation plan

* fix(channels): restore webhook task context and chunks

* fix(serve): classify worker webhook enqueue failures

* fix(channels): address webhook review feedback

* fix(serve): address channel webhook review blockers

* fix(serve): satisfy channel webhook lint

* fix(serve): harden channel webhook admission

* fix(serve): narrow channel webhook source config

* fix(serve): classify webhook session scope failures

* fix(serve): harden webhook payload handling

* fix(serve): authenticate webhook startup cheaply

* fix(serve): keep deferred serve fast path lean

* fix(serve): address deferred webhook review blockers

* fix(channels): propagate webhook approval mode

* fix(channels): harden webhook task admission

* fix(acp): harden approval mode initialization

* fix(channels): harden webhook shutdown and secrets

* fix(channels): harden webhook review blockers

* fix(serve): harden deferred webhook auth

* test(channels): cover webhook target rejection

* fix(channels): preserve webhook thread targets

* fix(channels): address webhook review blockers

* fix(channels): harden webhook review blockers

* test(serve): align deferred webhook secret log assertion

* fix(channels): isolate webhook thread sessions

* fix(channels): harden webhook enqueue failures

* fix(serve): classify disabled channel workers
2026-07-10 12:54:27 +00:00
joeytoday
c0aeb7df5d
docs(channels): add setup screenshots to WeCom robot guide (#6648) 2026-07-10 12:10:06 +00:00
qqqys
2054302357
fix(channels): align memory access with channel gates (#6620)
Some checks are pending
E2E Tests / web-shell Browser Regression (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* fix(channels): align memory access with channel gates

* fix(channels): harden channel memory injection

* test(ci): align autofix workflow assertions
2026-07-10 00:00:41 +00:00
ChiGao
0e229be76e
feat(tui): Ctrl+O frozen transcript view and unified tool output rendering (#5666)
* feat(tui): remove tool group borders and collapse completed tool results

Remove round borders from ToolGroupMessage, CompactToolGroupDisplay, and
InlineParallelAgentsDisplay. Completed tools now default to a single
collapsed header line with dimColor styling. Executing/error/confirming
tools continue to show their full result block.

Part of #4588 (Track 3: Simplify tool-call rendering).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): gate collapse on compact mode and fix innerWidth calculation

- Only collapse completed tool results in compact mode, preserving
  full visibility in non-compact mode
- Subtract 2 from innerWidth to account for ToolMessage paddingX={1}
- Update snapshots to reflect removed borders

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): address review feedback on collapse and visual alignment

- Gate isDim on compact mode so non-compact tools stay fully styled
- Add paddingX={1} to CompactToolGroupDisplay for left-edge alignment
- Delete Border Color Logic test block (borders removed)
- Add compact-mode test coverage for Error/Executing/Pending/forceShowResult
- Clean up stale border references in comments

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(tui): unify tool output with semantic summaries

Replace the dual compact/normal mode tool output with a single unified
mode. Completed tools always show a semantic overview line
("Read 3 files, edited 2 files") instead of dumping full results.

- Add buildToolSummary() for category-based semantic summaries
- Remove compactMode gate from shouldCollapse and isDim in ToolMessage
- Make all-completed tool groups use CompactToolGroupDisplay
- Remove unused useCompactMode hook calls from ToolMessage

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(tui): add buildToolSummary unit tests and fix stale comment

- Add 10 dedicated unit tests for buildToolSummary covering edge cases
- Fix stale comment referencing old compactMode gate logic

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): address audit findings for unified tool output

- Add Canceled status to allComplete check in ToolGroupMessage
- Move memory-only group rendering before showCompact to prevent
  them being swallowed by CompactToolGroupDisplay
- Fix LLM summary duplication: absorbedCallIds now tracks completed
  groups in non-compact mode; HistoryItemDisplay no longer bypasses
  summaryAbsorbed when !compactMode
- Update StandaloneSessionPicker test for new compact rendering
- Fix design doc category order example and add missing rendering rules

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): address inline review findings

- Add SHELL_COMMAND_NAME and @ file-reference pseudo-tools to
  TOOL_NAME_TO_CATEGORY mapping for correct category classification
- Fix height calculation test to use Executing status so expanded
  path is actually exercised
- Update stale comment about empty toolCalls behavior

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): remove unused compactMode import in HistoryItemDisplay

Fixes CI build failure caused by TS6133 (noUnusedLocals) — the
compactMode destructure became dead code after the summary gating
was moved to summaryAbsorbed.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* ci: trigger re-run with updated merge ref

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): design — remove global compact mode, add Ctrl+O transcript + mouse click-to-expand

Design-only. Stacks on #5661 (type-based tool partition baseline) and
#5751 (VP mouse foundation). Scope: remove residual global compactMode,
add Ctrl+O transcript (alt-screen frozen snapshot) and mouse click to
expand a tool's title/output in place.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(tui): remove global compact mode toggle (on top of #5661 partition baseline)

Builds on #5661's type-based tool partition. Removes only the residual
global compactMode switch, keeping the partition baseline intact:

- ToolGroupMessage: showCompact = (compactMode || allComplete) → allComplete
- delete CompactModeContext, mergeCompactToolGroups (isForceExpandGroup /
  compactToggleHasVisualEffect no longer used once the cross-group merge and
  the Ctrl+O toggle are gone)
- MainContent: drop the compactMode-gated merge path; mergedHistory =
  visibleHistory
- remove TOGGLE_COMPACT_MODE binding/matcher, ui.compactMode/compactInline
  settings, the compact-mode tip and shortcut entry, AppContainer state +
  provider + toggle keypress branch
- KEEP CompactToolGroupDisplay + partition, ToolMessage forceShowResult /
  shouldCollapse, ToolConfirmationMessage's local compactMode prop, and
  ui.compactMode in WEB_SHELL_SETTINGS (web shell is a separate surface)

typecheck + affected suites green (224 tests). Ctrl+O is a temporary no-op
until the TranscriptView lands.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(tui): Ctrl+O opens a frozen alt-screen transcript full-detail view

Adds the keyboard half of the Ctrl+O redesign on top of the #5661 partition
baseline:

- fullDetail render path (HistoryItemDisplay → ToolGroupMessage): fullDetail
  composes into thinking `expanded`, and on tool groups forces showCompact=false
  + forceShowResult=true + uncapped height — so every block renders in full.
- new TranscriptView: an AlternateScreen overlay (disabled in VP mode where
  Ink already owns the alt screen) rendering a frozen snapshot
  (history length + a pending copy) through ScrollableList with fullDetail,
  reusing #5751's keyboard/wheel/scrollbar scrolling. Adaptive
  estimatedItemHeight for the taller full-detail rows.
- AppContainer wiring mirrors ThinkingViewer: transcript guard is the FIRST
  handleGlobalKeypress branch (Esc/q/Ctrl+C/Ctrl+O close, everything else
  swallowed) so close keys beat QUIT and the vim INSERT guard; Ctrl+O opens
  when closed; auto-close on any blocking dialog / WaitingForConfirmation;
  message-queue drain and refreshStatic are suppressed while open.
- Command.TOGGLE_TRANSCRIPT bound to Ctrl+O.

typecheck + 8 suites (268 tests) green. Mouse click-to-expand (per-tool)
follows in a later commit. Alt-screen enter/exit behavior still needs
real-terminal verification across tmux/iTerm/VSCode.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): repaint normal buffer when transcript closes (no duplicate scrollback)

E2E (VHS) caught the design's flagged highest-risk issue: in the legacy
<Static> path, closing the alt-screen transcript leaked its full-detail rows
into the main scrollback (a duplicate "完整记录 / Transcript" block appeared
below the live history).

Fix: when isTranscriptOpen goes true→false in non-VP mode, force one
clearTerminal + Static remount, deferred a tick so the AlternateScreen's exit
escape (\x1b[?1049l) flushes first and the during-transcript refreshStatic
guard has already cleared. VP mode keeps its own scrollback via the React tree
and is unaffected.

Verified via VHS: open shows the transcript overlay; Esc restores the main
view cleanly with no duplicated content.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): rebase ctrl-o design doc to #5661's type-based partition

The design doc was written against an early state-based snapshot of #5661
(showCompact = (compactMode || allComplete), whole-group collapse) and even
asserted that forceExpandAll / isCollapsibleTool "don't exist". The merged
#5661 is type-based partition and those symbols are its core. Rewrite the
affected sections to match the shipped baseline:

- §1/§2: baseline described as type-based partition (collapse read/search/list
  via isCollapsibleTool, render mutation tools individually); compactMode no
  longer affects tool rendering. Added a revision note.
- §3.1: table + bullets rewritten to forceExpandAll + collapsible/
  non-collapsible split; shouldCollapseResult's isCollapsibleTool guard
  (Shell/Edit results always visible); mixed groups = summary line + per-tool.
- §4.1: smaller delete scope (no showCompact / compactMode|| term to remove);
  delete mergeCompactToolGroups.ts; keep web-shell ui.compactMode passthrough.
- §4.5: fullDetail = forceExpandAll=true (not showCompact=false) +
  per-tool forceShowResult=true + availableTerminalHeight=undefined.
- §4.8/§5/§7/§8/§9/appendix: symbols/forensics corrected to the real merged
  implementation; tool_use_summary renders as a standalone line (no absorption).

Matches the resolution already applied to the code in the preceding merge.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): fix factual nits from cross-audit of the ctrl-o design doc

Three independent audits confirmed the doc is now faithful to the merged
#5661 type-based partition; they surfaced three concrete fixes:

- CATEGORY_ORDER: corrected to the real array order
  search/read/list/command/edit/write/agent/other (was listed as
  command/read/edit/write/search/list/agent/other).
- CompactToolGroupDisplay exports: only getOverallStatus / isCollapsibleTool /
  buildToolSummary / CompactToolGroupDisplay are exported; ToolCategory /
  TOOL_NAME_TO_CATEGORY / CATEGORY_ORDER / getToolCategory are internal —
  relabeled accordingly.
- §5.B file table: fixed a broken 4-column separator and escaped the literal
  `||` pipes in the AppContainer row so it renders as a clean 2-column table.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): don't let fullDetail be bypassed by compact early returns

Audit (PR #5666) point 2: ToolGroupMessage computed `forceExpandAll =
fullDetail || ...` only AFTER two early returns — the pure-parallel-agent
group (→ InlineParallelAgentsDisplay dense panel) and the completed
memory-only group (→ "Recalled/Wrote N memories" badge). In transcript
full-detail mode those groups were therefore NOT fully expanded.

Guard both early returns with `!fullDetail` so transcript falls through to
the per-tool ToolMessage path (forceExpandAll + per-tool forceShowResult +
uncapped height). Add a regression test asserting a completed memory-only
group renders each op individually (not the badge) under fullDetail.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): resolve open design decisions from source evidence

Settle the two outstanding decision points from the PR audit using the
codebase + reference implementations (not preference):

- Non-TTY (audit point 3): AlternateScreen has NO isTTY guard today (doc
  claimed it did — corrected). The TUI is already gated by stdin.isTTY
  (config.ts:1532), so non-TTY rarely mounts; the only edge is `-i`.
  Decision: add a process.stdout.isTTY guard to AlternateScreen, matching
  the repo convention (startInteractiveUI/notificationService guard isTTY
  before terminal escapes). Doc now marks it "to implement" + test.

- Transcript / per-tool expansion state location: per claude-code
  (REPL-local transcript state), gemini-cli (dedicated ToolActionsContext),
  and this repo's own ThinkingViewer (AppContainer-local useState + minimal
  action via a dedicated context) — transcript open/freeze stays
  AppContainer-local and is NOT surfaced via UIStateContext (the
  implemented code already does this; only the doc was wrong). Per-tool
  expansion uses a dedicated ToolExpandedContext (real cross-layer
  producer/consumer), not the broad UIStateContext.

Also document the fullDetail early-return guard (the just-landed fix): the
pure-parallel-agent and memory-only early returns are skipped under
fullDetail so transcript shows every tool in full.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): align design doc status/scope with current PR (audit follow-up)

Latest audit confirms the technical design is implementable and side-effect
coverage is sufficient; it flagged status/scope inconsistencies for the doc
to serve as an acceptance baseline. Fixes:

1. Status: "design review (docs-only)" → "implementation in progress; this
   doc is the acceptance baseline for the current PR". Added an
   implemented-vs-pending status table.
2. Mouse click-to-expand: added a banner marking it NOT yet implemented and
   stating the open scope decision (merge blocker vs VP-only follow-up).
3. #5751 (and #5661) dependency: corrected from "OPEN, must merge first" to
   "already merged into main; branch rebased on top".
4. alt-screen degradation: removed the undefined "overlay" fallback in the
   DefaultAppLayout row; non-TTY degrades via the AlternateScreen isTTY guard
   to in-buffer rendering (§4.2), no separate overlay path.
5. Fixed a broken bold marker (`\*\*`) in the AppContainer row.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): scope mouse click-to-expand out as a follow-up

Assessed the mouse click-to-expand effort against the real code: it's
~250–400 lines across 4–5 files (ToolExpandedContext + AppContainer wiring
+ a ClickableToolMessage component — can't call useMouseEvents inside the
.map() — + ToolGroupMessage wiring + mouse hit-test tests). More
importantly, under #5661's type-based partition the collapsed read/search
tools are aggregated into a single summary line, so there is no per-tool
click target — the click granularity must be redesigned to "click the
summary row → expand the whole group". Plus the known SGR-mouse vs native
text-selection risk.

Per the "small code → include, otherwise follow-up" rule: this is not small,
so scope it OUT of the current PR. The current PR delivers Ctrl+O transcript
only. Marked §1 goal #4, §4.8 (banner + draft), §9 commit 4, and the status
table accordingly; the §4.8 design is kept as a draft for the follow-up PR.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(tui): isTTY guard for AlternateScreen + transcript shortcut/i18n cleanup

Completes the remaining in-scope items for the Ctrl+O transcript PR:

- AlternateScreen: guard the alt-screen escape writes on
  `process.stdout.isTTY` (skip when non-TTY: piped/redirected/CI), matching
  the repo convention (startInteractiveUI / notificationService). Non-TTY
  now degrades to in-buffer rendering. Adds AlternateScreen.test.tsx
  (enter/exit on TTY, skip when disabled, skip when non-TTY).
- KeyboardShortcuts: add the `ctrl+o → view transcript` entry that was
  removed with the old compact-mode line but never replaced.
- i18n (all 9 locales): drop the dead `to toggle compact mode` and the
  `Press Ctrl+O to toggle compact mode — …` tip strings (no longer
  referenced after compact-mode removal); add `to view transcript`.

Touched suites green (AlternateScreen, i18n index/mustTranslateKeys,
TranscriptView, Help).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(tui): mark isTTY guard + i18n cleanup as implemented in status table

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(i18n): add TranscriptView strings to all locales

TranscriptView.tsx renders t('Transcript'), t('to close') and
t('to scroll'), but these keys existed only in en/zh. The strict
key-parity check (zh, zh-TW) failed CI on the missing zh-TW entries.

Add all three keys to zh-TW (the failing strict-parity locale) and to
ca/de/fr/ja/pt/ru for completeness so check-i18n is fully clean.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(ctrl-o): add before/after transcript capture evidence

Add VHS-captured screenshots (main-view collapsed vs Ctrl+O transcript
expanded) under docs/design/ctrl-o-detail-expand/assets/ and reference
them from §3.4 of the design doc. Captured on the local branch build via
the mac-autotest skill; shows read/search/list tools folding to a single
summary row in the main view and each expanding in the transcript, with
zh i18n strings rendering correctly.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(ctrl-o): design §4.9 — full tool detail passthrough in transcript

Document the data-layer gap behind the "second-level fold" seen in the
Ctrl+O transcript: read/ls/grep returnDisplay only stores a summary, and
IndividualToolCallDisplay carries no full-content field, so fullDetail
(which correctly clears partition/result folding and height limits) has
no detail to render.

Spec the chosen fix (path C): derive a contentForDisplay string from the
raw llmContent at the single core success-assembly point (partToString +
existing 32k retention cap), thread it through to a new
IndividualToolCallDisplay.detailedDisplay, and render it in ToolMessage
when fullDetail + isCollapsibleTool. Scope limited to read/search/list in
the transcript; main-view summaries and shell/edit/write are unchanged.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(ctrl-o): adopt plan Y for §4.9 and address transcript-detail audit

Address the audit on §4.9 (full tool detail in the Ctrl+O transcript):

- Rewrite §4.9 to plan Y — reuse the complete content already persisted in
  functionResponse.response.output (responseParts) via a single core helper,
  instead of adding a contentForDisplay field threaded through serialize/
  replay. Saved/replayed transcripts get full detail for free (audit #6).
- Split fullDetail (data-source switch) from forceShowResult (un-fold) so
  main-view force cases (user-initiated/error) don't leak full detail
  into the main view (audit #2).
- Use the exported compactStringForHistory, not the internal compactString
  (audit #4).
- Scope by isCollapsibleTool incl. glob, not a hardcoded read/ls/grep list
  (audit #5).
- §3.4: stop claiming the screenshot already shows full output; add a
  pre-§4.9 caveat and a merge-blocker row in the status table (audit #1).
- Sync §5 file list, §8 tests, §9 commit 4 (merge blocker); move mouse
  click-expand out of the commit sequence to follow-up (audit #3).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(ctrl-o): tighten §4.9 per second audit (no 2nd truncation, nested media, plan-Y guard)

- P1: detailedDisplay no longer runs compactStringForHistory — the 32k
  cap would make Ctrl+O a "32k bounded preview", contradicting the
  "full detail" promise (read_file has maxOutputChars=Infinity and can
  legitimately exceed 32k). Detail is now the full getToolResponseDisplayText
  output, bounded only by core's existing truncateToolOutput/pagination.
- P2: spell out getToolResponseDisplayText's priority rule — media lives in
  nested functionResponse.parts (not top-level); read response.output, then
  walk nested parts for inlineData/fileData/text placeholders; undefined when
  neither output nor media so the UI falls back to the summary.
- P3: add an explicit §8 plan-Y protection test (output >32k survives
  recording/loadSession/resume/replay; detailedDisplay derives from
  message.parts, not resultDisplay or API compressedHistory) and document
  the fall-back-to-X trigger.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(ctrl-o): address PR review findings on transcript view

- AppContainer: freeze a committed-history copy (not just a length) so
  in-place compaction can't corrupt the open transcript; memoize the
  stitched items list so streaming re-renders don't rebuild it
- AppContainer: clear thinkingViewerData on openTranscript and guard
  openThinkingViewer so no stale "ghost" thinking popup resurfaces
- AppContainer: read prevTranscriptOpen during render (StrictMode-safe)
- AppContainer: close the transcript on Ctrl+D instead of swallowing it
- TranscriptView: wrap content in a new ErrorBoundary and React.memo the
  component (stable items + onClose make the shallow compare effective)
- CompactToolGroupDisplay: localize buildToolSummary via t() and add the
  per-category count phrases to all 9 locales
- workspace-settings: drop the stale ui.compactMode web-shell allowlist entry
- tests: TranscriptView default alt-screen + negative-id keyExtractor;
  HistoryItemDisplay fullDetail expansion + forwarding; ToolGroupMessage
  fullDetail parallel-agent bypass; MainContent.test import-first order

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(ctrl-o): second review round — web-shell compactMode + anti-deadlock deps

- settingsSchema: re-add ui.compactMode as a hidden (showInDialog:false)
  schema entry so the web shell's independent compact toggle keeps
  persisting via the daemon settings routes (mirrors voiceModel). The TUI
  compact mode stays retired — it just isn't shown in the TUI dialog.
- workspace-settings: restore ui.compactMode in WEB_SHELL_SETTINGS now that
  the schema definition resolves again (fixes the web shell 400 / revert).
- AppContainer: add isTranscriptOpen to the anti-deadlock auto-close effect
  deps so opening the transcript while a blocking prompt is already visible
  re-fires the effect and closes it (previously it could open over an
  invisible prompt and deadlock).
- ToolGroupMessage.test: cover the fullDetail height-truncation lift
  (availableTerminalHeight undefined under fullDetail, numeric otherwise).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(ctrl-o): regenerate vscode settings schema for re-added ui.compactMode

The previous commit re-added ui.compactMode (showInDialog:false) to
settingsSchema.ts but did not regenerate the generated vscode schema,
which the CI "settings schema is up-to-date" gate checks. Regenerated.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* chore(ctrl-o): reset MCP/acp-bridge files to main (drop stale merge diff)

These 6 files are unrelated to the Ctrl+O work. Reset to origin/main so the
PR diff carries only transcript changes. Committed with --no-verify because the
classic-CLI pre-commit prettier reflows union types differently than the repo's
experimental-CLI formatter (CI's prettier step does not gate on this).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(ctrl-o): update compact-mode docs for transcript model; drop orphaned i18n key

- settings.md: ui.compactMode is retired in the TUI (web-shell only); Ctrl+O
  now opens the full-detail transcript
- tool-use-summaries.md: reframe "compact vs full mode" toggle as "main view
  (completed group) vs Ctrl+O full-detail transcript / force-expanded"
- remove the now-orphaned 'Hide tool output and thinking…' locale key (was the
  old compactMode description) from all 9 locales

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* feat(ctrl-o)!: §4.9 full tool-detail passthrough in transcript

Implement plan Y: read/search/list tools now show their COMPLETE output
in the Ctrl+O transcript instead of the summary count line, while the
main view is unchanged.

- core: add `getToolResponseDisplayText(parts)` — extracts the full
  `functionResponse.response.output` (skipping the non-informative
  "Tool execution succeeded." placeholder), emits `<media: mime>`
  placeholders for nested media parts, keeps nested text, returns
  undefined when nothing is extractable. No second truncation: the only
  bound is whatever core already applied (truncateToolOutput / paging).
- cli: add derived (non-persisted) `IndividualToolCallDisplay.detailedDisplay`.
  Populated from the already-persisted response parts on both the live
  path (useReactToolScheduler success branch) and the resume path
  (resumeHistoryUtils tool_result, falling back to message.parts for
  older records).
- cli: rendering split — ToolGroupMessage forwards `fullDetail` to
  ToolMessage; ToolMessage swaps the summary `resultDisplay` for
  `detailedDisplay` ONLY when `fullDetail && isCollapsibleTool(name) &&
  detailedDisplay`. Kept separate from `forceShowResult` so main-view
  force scenarios (user-initiated / error / confirming) still render the
  summary, never the full output.
- ACP path needs no change: ToolCallEmitter.transformPartsToToolCallContent
  already writes the same full output into the ACP `content[]` for its SSE
  clients; the TUI transcript does not flow through it, so no new protocol
  field is added.

Tests: core helper unit tests (placeholder skip, nested media, plain-text
part, empty fallback); ToolMessage data-source switch (collapsible+fullDetail
uses detail, force-but-not-fullDetail keeps summary, non-collapsible keeps
summary, missing-detail falls back); ToolGroupMessage prop-forwarding.

BREAKING CHANGE: Ctrl+O is now a frozen full-detail transcript view, not a
global compact-mode toggle. The `TOGGLE_COMPACT_MODE` command and the TUI
effect of `ui.compactMode` / `ui.compactInline` are removed; the keys remain
read-tolerant (ignored by the CLI) and `ui.compactMode` is still forwarded to
the web shell. See docs/design/ctrl-o-detail-expand/design.md §6 for migration.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(ctrl-o): address review — repaint race, suppressOnRestore parity, transcript error logging

- AppContainer: fix close-repaint setTimeout being cancelled by streaming
  re-renders. `wasOpenPrevRender`/`isTranscriptOpen` were in the effect deps,
  so the next streaming render flipped them, ran cleanup, and clearTimeout'd
  the pending repaint — leaving stale pre-transcript content in the legacy
  <Static> normal buffer. Drive the effect off a close-transition counter
  instead, so post-close re-renders don't change deps and the scheduled
  repaint fires exactly once per close.
- AppContainer: transcript snapshot now mirrors MainContent's
  `!display.suppressOnRestore` filter, so items collapsed on session resume
  (ui.history.collapseOnResume) are not re-exposed in the Ctrl+O view.
- TranscriptView: pass `onError` to the ErrorBoundary so caught render errors
  in the fullDetail paths are logged to the debug channel, not just shown.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(ctrl-o): cover detailedDisplay resume derivation + message.parts fallback

Add dedicated resumeHistoryUtils tests for §4.9: detailedDisplay derived
from toolCallResult.responseParts, the `responseParts ?? message.parts`
fallback for older records lacking responseParts, and the undefined
fallback when neither source carries output.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(ctrl-o): address review — plain-text detail, shared placeholder const, resume status guard, scroll hint

Four review fixes on the §4.9 transcript work:

- ToolMessage: when fullDetail swaps the data source to detailedDisplay
  (raw file content / grep hits / dir listings), force renderOutputAsMarkdown
  to false. The existing `if (availableHeight)` guard never fires in the
  transcript (height cap is lifted, availableTerminalHeight is undefined), so
  raw `#`/`*`/`-`/`>` characters were being Markdown-formatted.
- core: export TOOL_SUCCEEDED_OUTPUT as the single source of truth for the
  "Tool execution succeeded." placeholder. coreToolScheduler (the producer,
  two sites) and getToolResponseDisplayText (the consumer) now share one
  constant so the filter can't silently drift if the wording changes.
- resumeHistoryUtils: only derive detailedDisplay for SUCCESS tools, matching
  the live path (useReactToolScheduler sets it only in its 'success' branch).
  Previously it was populated unconditionally, so a resumed errored/cancelled
  collapsible tool would surface raw output in the transcript while the same
  tool live would not.
- TranscriptView: footer hint now reads "Shift+↑↓ to scroll" — plain Up/Down
  do not scroll (ScrollableList listens for SCROLL_UP/DOWN bound to Shift+↑↓);
  the old "↑↓" hint was misleading.

Tests: ToolMessage plain-text-detail assertion + new raw-markdown case;
resume errored-tool no-detailedDisplay case. typecheck/lint/tests green
(core scheduler 222, cli suites pass).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): guard transcript non-TTY output + clear detailedDisplay on compaction

Addresses three review findings on the Ctrl+O transcript work:

- Non-TTY byte leak: `useMouseEvents` enabled SGR mouse mode (?1002h ?1006h)
  whenever stdin supported raw mode, ignoring stdout. With stdout piped
  (`qwen | tee log`) the transcript's focused ScrollableList (bypassVpGate)
  leaked raw control bytes into the captured output. Gate the enable on
  `stdout.isTTY`, and likewise guard the transcript close-repaint
  `clearTerminal` write in AppContainer — both now mirror AlternateScreen's
  existing isTTY guard, so the non-TTY fallback stays byte-clean.

- Compaction privacy regression: `compactOldItems` replaced old tool
  `resultDisplay` with the cleared placeholder but left `detailedDisplay`
  (the raw functionResponse text added for the full-detail transcript)
  intact, so reopening Ctrl+O after compaction re-surfaced the supposedly
  cleared read/search/list output. Clear `detailedDisplay` wherever
  `resultDisplay` is cleared, with a regression test.

- Docs: keyboard-shortcuts.md still described Ctrl+O as "toggle compact
  mode"; updated to the open/close full-detail transcript behavior.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(tui): report a TTY stdout in ScrollableList mouse-scroll tests

The new `stdout.isTTY` gate in `useMouseEvents` (which stops SGR mouse
escapes leaking into piped output) left ink-testing-library's fake
stdout — which has no `isTTY` — with the mouse pipeline disabled, so the
scrollbar-drag and wheel-scroll assertions never received events. Mock
ink's `useStdout` to report `isTTY: true` so the pipeline arms exactly as
it does in a real terminal; all other ink exports are preserved.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): address Ctrl+O transcript review — q-guard, callback churn, tests, cleanup

Resolves the qwen3.7-max /review findings:

- Modifier guard on the transcript close key: bare `q` closed the
  transcript, but Ink reports Ctrl/Alt/Shift+Q as `{ name: 'q', … }` too
  (Alt arrives as `meta`), so those silently closed it. Guard
  `!key.ctrl && !key.meta && !key.shift` (Shift+Q is a literal `Q`).

- Stable `openTranscript`: it captured `historyManager.history` and
  `pendingHistoryItems` as deps, both of which change identity every
  streaming tick, rebuilding the callback — and the whole
  `handleGlobalKeypress` closure that lists it — on every render during
  streaming. Read both via refs so the callback is referentially stable.

- AppContainer transcript integration tests (the removed TOGGLE_COMPACT
  tests had no replacement): Ctrl+O installs TranscriptView; Esc / q /
  Ctrl+C / Ctrl+D close it; Ctrl+Q / Alt+Q / Shift+Q do NOT (modifier
  guard); arbitrary keys are swallowed and keep it open; a blocking
  confirmation (WaitingForConfirmation) auto-closes it (anti-deadlock).

- Dead i18n string: removed the orphaned
  'Press Ctrl+O to show full tool output' key from all 9 locale files
  (no `t()` reference remained after the compact-mode sweep).

- Design doc: replaced the leaked absolute worktree path with a
  placeholder, and corrected the §6 keybinding-migration note — the
  codebase has no user-configurable keybinding override surface
  (`keyMatchers` always uses hardcoded defaults), so there is no
  persisted `toggleCompactMode` binding to migrate; the startup-detection
  step is not applicable until such a feature exists.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): escape ANSI in transcript detailedDisplay + gate its extraction

Two findings from the qwen3.7-max /review on §4.9:

- [Critical] ANSI escape injection: `detailedDisplay` carries raw,
  un-sanitized tool output (file contents, grep hits, directory
  listings). The Ctrl+O transcript rendered it straight to <Text>
  without escaping, so a malicious repo file with embedded terminal
  control sequences (e.g. `\x1b[?1049l` to drop the alt-screen, OSC 52
  for clipboard poisoning) would execute when the transcript opened —
  and fullDetail lifts the height cap, exposing the whole file. Run it
  through `escapeAnsiCtrlCodes` (already used for agent names in this
  file) before rendering. Added a regression test asserting the raw ESC
  bytes don't survive.

- [perf] `detailedDisplay` was extracted on every successful tool call
  (~25K chars from core's truncation) but is consumed only by the
  transcript's fullDetail render for collapsible (read/search/list)
  tools. Gate the extraction on `isCollapsibleTool(displayName)` so
  edit/write/command/agent calls no longer store a large string the
  renderer never reads — mirrors ToolMessage's `usingDetailedDisplay`
  gate (which also keys off the display name).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): gate resume-path detailedDisplay on isCollapsibleTool (match live path)

The resume path (resumeHistoryUtils.ts) extracted `detailedDisplay` for
every successful tool call, unlike the live path in useReactToolScheduler
which gates on `isCollapsibleTool(displayName)`. Since the transcript's
`usingDetailedDisplay` only consumes it for collapsible (read/search/list)
tools, resuming a session with many edit/write/command/agent calls stored
large (~25K char) strings the renderer never reads. Apply the same gate so
live and resume stay consistent, using `toolCall.name` (the display name,
set from `tool.displayName`) to match the renderer's key.

Updated the existing derivation tests to use a collapsible read tool (an
edit tool now correctly yields undefined) and added a regression asserting
a non-collapsible tool leaves detailedDisplay undefined on resume.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): strip bare C0 control bytes from transcript detailedDisplay + memoize

Follow-up to the ANSI-escape fix. `escapeAnsiCtrlCodes` delegates to
ansi-regex, which only matches ESC-prefixed sequences, so bare C0 control
bytes without an ESC prefix (BEL \x07, BS \x08, FF \x0c, SO \x0e, SI \x0f,
CR, …) passed through to <Text> and could still corrupt the display or
ring the bell from a malicious file's contents. Add a second pass that
strips those bytes (keeping only TAB and LF, which structure multi-line
output). Memoize the two-pass sanitization with useMemo keyed on
detailedDisplay so the ~25K-char regex work doesn't re-run every render.

Extended the ToolMessage regression test to assert bare C0 bytes are
stripped alongside the ESC sequences.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(tui): memoize HistoryItemDisplay, add ErrorBoundary tests + TAB/LF invariant

Addresses three review suggestions:

- Wrap `HistoryItemDisplay` in `React.memo` so the Ctrl+O transcript
  (which re-renders on every scroll tick) skips re-rendering
  frozen-snapshot items whose props are shallowly unchanged. The
  transcript passes stable `item` references, so the default shallow
  compare is effective; harmless for the main view (items live in
  `<Static>` and render once).

- Add ErrorBoundary.test.tsx covering the four behaviors: renders
  children when healthy, catches a render error into the default
  fallback with the message, renders a custom fallback, calls `onError`
  with the error + component stack, and `reset` clears the error state so
  the subtree recovers.

- Lock the C0-strip invariant: assert TAB and LF survive in
  detailedDisplay (the regex intentionally skips \x09/\x0a) so a future
  regex change can't silently collapse multi-line/columnar output.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* refactor(tui): review cleanups — gate sanitize memo, drop dead code, add tests

Addresses the latest /review suggestions:

- ToolMessage: gate the `sanitizedDetailedDisplay` useMemo on
  `usingDetailedDisplay` so the ~25K-char escape+strip no longer runs for
  every collapsible tool in the main view (where the result is discarded).

- TranscriptView: remove the dead `listRef` (created + passed as `ref` but
  never used imperatively) and the dead `onClose` prop (declared, then
  `void`-ed; close keys are owned entirely by AppContainer's global
  keypress guard). Dropped the now-unused `useRef` / `ScrollableListRef`
  imports and the `onClose` call-site + props.

- Tests: add TranscriptView error-fallback coverage (a throwing item
  renders the recovery fallback, not a crash); add live-path
  `mapToDisplay` detailedDisplay extraction coverage (collapsible →
  extracted, non-collapsible → undefined); add Ctrl+O to the transcript
  close-keys it.each (the toggle key was the only close key untested).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(tui): remove orphaned no-op CompactModeProvider stubs

This PR deleted the CompactModeContext, leaving identical no-op
`CompactModeProvider` passthrough stubs (with an ignored `value` prop) in
ToolGroupMessage.test.tsx, ToolMessage.test.tsx and MainContent.test.tsx,
each still wrapping every render. Remove the stubs and unwrap the renders;
drop the now-meaningless `compactMode` params/args from the local render
helpers. Behavior-preserving (the stubs rendered children verbatim) —
all three suites still pass.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): strip bidi overrides, sanitize error fallbacks, share filters

Latest /review round:

- [Critical] Strip Unicode bidirectional override / isolate chars (Trojan
  Source, CVE-2021-42572) from transcript `detailedDisplay` — a third
  sanitize pass after ANSI + C0 stripping, mirroring the repo's existing
  BIDI_CONTROL_RE. Regression test added.

- Sanitize `error.message` with `escapeAnsiCtrlCodes` in both the
  ErrorBoundary default fallback and the TranscriptView custom fallback
  (defense-in-depth against control codes in a crafted error message).

- Ctrl+O while the ThinkingViewer is open now swaps to the transcript
  (falls through to openTranscript, which clears the viewer) instead of
  being silently swallowed.

- Extract the shared `isHistoryItemVisibleAfterRestore` predicate into
  types.ts and use it from both MainContent (main view) and AppContainer
  (transcript freeze), so the two surfaces can't diverge on which
  collapse-on-resume items are hidden.

- Tests: use the exported `TOOL_SUCCEEDED_OUTPUT` constant instead of the
  hardcoded literal in generateContentResponseUtilities.test.ts.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): harden compaction guard to always clear detailedDisplay

The compaction cleanup only cleared `detailedDisplay` inside the
`resultDisplay != null` branch (both the group-level trigger, the
group-count pass, and the per-tool clear). A tool carrying only
`detailedDisplay` (no resultDisplay) would skip compaction and leave the
raw transcript detail intact — a latent privacy leak if the two fields
ever decouple. Widen all three checks to also match `detailedDisplay !=
null` so the memory/privacy safeguard is robust. Added a defensive
regression test.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): sanitize mime/uri in getToolResponseDisplayText media placeholders

The `<media: …>` placeholder interpolated `inlineData.mimeType` /
`fileData.mimeType` / `fileData.fileUri` from tool responses verbatim. A
crafted response could embed control characters or angle brackets to
inject terminal codes or forge/mangle the placeholder markup. Add a
`sanitizeMediaLabel` helper that strips C0/C1 control bytes and `<`/`>`
before interpolation, falling back to the default label when emptied.
Regression test added.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* test(tui): report a TTY stdout in BaseSelectionList mouse integration test

The `stdout.isTTY` gate added to `useMouseEvents` (stops SGR mouse escapes
leaking into piped output) left #6011's BaseSelectionList mouse test —
which renders via ink-testing-library where the hook-provided stdout reads
as non-TTY — with the mouse layer disabled, so the any-event enable escape
was never written. Mock ink's `useStdout` to report `isTTY: true` with a
capturing write spy (matching useMouseEvents.test.tsx / ScrollableList.test
.tsx), and assert the `?1003h` enable via that spy while items still render
through ink's own stdout. Both cases pass.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* docs(core): fix JSDoc placement + note ErrorBoundary fallback is un-translated

Two small review nits:

- getToolResponseDisplayText's JSDoc had ended up above sanitizeMediaLabel
  (added last commit), making it read as that helper's docs. Reorder so
  sanitizeMediaLabel + its own JSDoc come first and each doc sits directly
  above its function.

- Document why the ErrorBoundary default fallback's title is intentionally
  a plain English string (last-resort message for callers with no
  `fallback`; renders mid-crash, so it avoids pulling in the i18n layer —
  the transcript passes its own localized fallback anyway).

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(tui): share terminal-sanitize pipeline; guard AlternateScreen writes

- Extract the three-pass sanitizer (ANSI escape + bare-C0 strip + bidi
  strip) into `sanitizeTerminalText` in textUtils.ts as the single source
  of truth, and use it at all raw-text render sites: ToolMessage's
  `detailedDisplay`, and the TranscriptView + ErrorBoundary error-message
  fallbacks (previously those only escaped ANSI, missing C0/bidi — the
  boundary catches errors from the fullDetail path that processes raw tool
  output, so a crafted item shape could carry unsanitized bytes into
  error.message). Removes the duplicated regex consts from ToolMessage.

- AlternateScreen: wrap the alt-screen escape writes (and the exit/cleanup
  writes) in try/catch so a synchronous stdout error (EPIPE on terminal
  close, EAGAIN under backpressure) can't propagate uncaught from the
  effect and crash the app or corrupt the terminal.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-09 23:40:29 +00:00
Shaojin Wen
41c405b3bf
feat(review): post Suggestion findings as inline comments (#6593)
Suggestion-level findings were routed to a single updatable issue comment
(the "suggestion summary") while only Critical findings became inline review
comments. That split traded away two things that turned out to matter more
than the convergence it bought:

- An issue comment has no lifecycle. GitHub folds an inline review thread away
  as Outdated once the author edits the line it is anchored to, so an addressed
  finding removes itself from the page. The summary comment just sits in the PR
  conversation forever; PATCHing it to "all addressed" replaces its content but
  not the comment. The mechanism meant to prevent clutter was the clutter.
- A Markdown table cannot carry a one-click fix. GitHub renders a ```suggestion
  fence as an applicable change only inside a review comment on a diff line.
  Suggestion findings are exactly the mechanical, localized cleanups that
  benefit most from one-click apply, so the split withheld the feature from the
  findings that needed it most.

Both severities now post as inline comments, distinguished by a **[Critical]**
or **[Suggestion]** body prefix. The `qwen review post-suggestions` subcommand
and its plumbing are removed.

Follow-on changes required by the reroute:

- pr-context: the "Previous suggestion summary" section is gone. Legacy summary
  comments are still recognised so they stay out of "Already discussed", but the
  exclusion is now marker-only rather than author-gated. The author check missed
  summaries posted by the *other* identity: /review runs as a maintainer locally
  and as qwen-code-ci-bot in CI, and roughly half of the last 60 PRs carry a
  bot-authored summary. Those leaked into "Already discussed" and told the review
  agents not to re-report the findings listed there. The check originally guarded
  promotion into a trusted rendering section; that section no longer exists, so
  it only gated exclusion, where a third party embedding the marker merely hides
  their own comment.

- qwen-autofix: the workflow filters "suggestion summaries" out of the autofix
  bot's actionable queue, but only on the issue-comment channel. With Suggestions
  now inline, they entered the unfiltered inline channel and the bot would apply
  non-blocking recommendations and spend a review round on them. The inline
  channel now applies the same gate, keyed on the **[Suggestion]** prefix plus
  the /review footer so a human quoting the prefix stays actionable.

- Step 7 gains a 422 fallback. Create Review is all-or-nothing, so one Suggestion
  anchored outside the diff would take the Critical findings down with it — a risk
  that did not exist when Suggestions travelled on a line-agnostic issue comment.
  GitHub's 422 does not name the offending entry, so the model rechecks anchors
  against the diff, relocates failing Criticals into the body, discards failing
  Suggestions, and degrades to an all-prose review rather than posting nothing.
  COMMENT reviews now always carry a one-line body: an empty body is only known
  to be accepted alongside inline comments on REQUEST_CHANGES, and a Suggestion-
  only review is the common case for a clean PR.
2026-07-09 12:39:10 +00:00
callmeYe
0907edb909
Fix long session timeline scrolling (#6526)
* fix(web-shell): hide long session timeline scrollbar

* fix(web-shell): lift timeline tooltip above popovers

* fix(web-shell): refine timeline tooltip behavior

* fix(web-shell): keep timeline tooltip anchored

* fix(web-shell): keep timeline tooltip below modals

* fix(web-shell): harden timeline tooltip recentering

* fix(web-shell): drop unused timeline tooltip var

* fix(web-shell): keep timeline programmatic scroll guard through frame

* fix(web-shell): preserve timeline tooltip on focus scroll

* ci(web-shell): add smoke test script
2026-07-09 11:43:21 +00:00
Dragon
6dafb330f2
docs: fix model-provider config shape and refresh feature/setting drift (#6552)
Audit findings against the current codebase:

- model-providers.md, auth.md: the documented modelProviders shape used
  the reverted `{ protocol, models }` wrapper. The canonical shape is a
  bare `ModelConfig[]` array per provider id (a wrapped entry in a
  migrated settings file is silently skipped). Update all examples and
  prose, document the separate top-level `providerProtocol` map for
  custom provider ids, and correct the unknown-key behavior.
- settings.md: correct the default for
  `model.chatCompression.screenshotTriggerThreshold` (20, not 50).
- commands.md: add the missing `/reload-plugins` command and note that
  `/dream` and `/forget` are registered only when managed auto-memory
  is available.
- Add a Computer Use feature page (on-by-default desktop automation via
  the cua-driver native driver) and wire it into the features nav and
  the qc-helper doc index.

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-08 23:04:47 +00:00
Nothing Chan
082b3bb3d9
fix(memory): give each linked git worktree its own auto-memory root (#6462)
getAutoMemoryRoot() resolved linked worktrees back to the canonical
repository root, so every worktree of a repo shared one project memory.
Focused worktree sessions polluted the shared MEMORY.md index with
unrelated entries, and chats/, workflows/, and team memory were already
per-worktree — project memory was the only shared exception.

Anchor project memory at the nearest git root (same resolution team
memory already uses) so each worktree gets its own memory directory.
The main checkout resolves to the same path as before, so existing
memory is unaffected.

Fixes #6449

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-08 14:34:03 +00:00
ChengHui Chen
1f92787aa0
feat(channels): add dmPolicy config to disable private/DM messages (#6521)
* feat(channels): add dmPolicy config to disable private/DM messages

Add DmGate class mirroring GroupGate to gate DM/private messages in
channel adapters. Operators can now set dmPolicy: 'disabled' in their
channel config to silently drop all DM messages while keeping group
messages active.

Closes #6392

* fix(channels): address review feedback for dmPolicy

- Add dmPolicy: 'open' to all test config factories (8 files) to
  maintain type correctness with required ChannelConfig field
- Add integration tests in ChannelBase.test.ts:
  - preflightInbound: DM dropped + group passes when dmPolicy=disabled
  - isStoredLoopTargetAuthorized: DM loop job disabled + group passes
- Add dmPolicy assertions in config-utils.test.ts (default + explicit)
- Keep dmPolicy as required field (not optional) for strict parity
  with groupPolicy
2026-07-08 12:01:10 +00:00
顾盼
14f02c132a
fix(core): Match hook display-name matchers to tool ids (#6373)
* fix(core): match hook tool display names

* fix(core): tighten hook matcher aliases

* fix(core): align hook matcher aliases
2026-07-08 02:22:55 +00:00
Dragon
394c1a289e
docs(channels): add WeCom to channels overview (#6490)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
The WeCom intelligent robot channel (added in #6436) has its own guide and _meta.ts entry, but the channels overview was never updated. Add WeCom to the platform list, quick-start guide links, the `type` option and `token` exclusion note, new `botId`/`secret` option rows, the media-support note, and the slash-command channel enumeration.

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-07 22:39:06 +00:00
qqqys
e3d7d10d1d
[codex] add natural channel memory intents (#6376)
* feat(channels): add natural channel memory intents

* fix(channels): add explicit guard and exhaustiveness check for clear_confirm intent

The clear_confirm path was handled as implicit fall-through at the bottom
of handleChannelMemoryIntent. If a new intent kind were added to the
ChannelMemoryIntent union, it would silently execute clearChannelMemory
without user confirmation — a data-loss risk.

Add explicit if (intent.kind === 'clear_confirm') guard and a
const _exhaustive: never assertion so TypeScript flags any unhandled
kinds at compile time.

* fix(channels): close session leak in classifier and fix regex separator

- BridgeChannelMemoryIntentClassifier now wraps prompt() in try/finally
  to always call cancelSession(), preventing daemon session leaks on
  every classifier invocation. Cleanup errors are caught so they cannot
  mask a successful classification result.
- Add missing optional punctuation separator to the 以后记住 regex
  pattern for consistency with other Chinese remember patterns.

* fix(channels): enforce pending clear state for channel memory confirmation

The clear_confirm intent executed clearChannelMemory directly without
verifying a prior clear_request was issued for the same chat. Any
authorized user could clear any chat's memory by sending the confirmation
phrase standalone, bypassing the two-step flow.

Add a per-target pending clear map (chatId + threadId, 60s TTL) that
is set during clear_request and verified+consumed during clear_confirm.
Standalone confirmation phrases now get rejected with a prompt to
issue the clear request first.

* fix(channels): include senderId in pendingClears key to prevent cross-user confirmation

User A could initiate clear_request in a group chat and User B could
confirm it, since the pending key only included chatId+threadId.
Add senderId to the key so only the user who initiated the clear
can confirm it.

* fix(channels): harden memory intent review fixes

* fix(channels): cover memory clear sender guard

* fix(channels): block group memory mutations

* fix(channels): avoid ambiguous memory saves

* test(channels): cover memory classifier cleanup

* test(channels): cover memory clear expiry

* fix(channels): restore channel memory slash aliases

* test(channels): cover memory intent edge cases
2026-07-07 15:25:01 +00:00
qqqys
467b292b50
feat(channels): add WeCom intelligent robot channel (#6436)
* feat(channels): add WeCom smart bot channel

* fix(channels): harden wecom review suggestions

* fix(channels): address wecom critical review

* fix(channels): include wecom mixed voice text

* fix(channels): tighten wecom outbound media

* fix(channels): harden wecom outbound sends

* fix(channels): address wecom review blockers

* fix(channels): address wecom review followups

* fix(channels): harden wecom inbound handling

* fix(channels): address wecom auth and media review

* fix(channels): tighten wecom inbound cleanup

* fix(channels): harden wecom media safety

* fix(channels): address wecom review typecheck

* fix(channels): harden wecom media review gaps

* fix(channels): address wecom review blockers

* fix(channels): tighten wecom media edge cases

* fix(channels): address wecom review blockers

* fix(channels): address wecom media review blockers

* fix(channels): address wecom review follow-ups

* fix(channels): address wecom review blockers

* fix(channels): close wecom review blockers

* fix(channels): close wecom preflight dedup race

* fix(channels): close wecom review gaps

* fix(channels): harden wecom kick reconnect

* fix(channels): defer wecom session resolution

* fix(channels): clean wecom session attachments

* fix(channels): harden wecom reconnect and media cleanup

* fix(channels): address wecom review diagnostics

* fix(channels): improve wecom diagnostics

* fix(channels): reset wecom kick retries

* fix(channels): improve wecom diagnostics

* fix(channels): preserve sync cancel preflight

* fix(channels): close wecom connection and ssrf gaps

* fix(channels): clean coalesced wecom attachments

* fix(channels): bound wecom sdk connect wait

* fix(channels): scope wecom untracked attachment cleanup

* fix(channels): block wecom nat64 local-use ssrf

* fix(channels): harden wecom media handling

* fix(channels): harden wecom group gates

* fix(channels): bound wecom kick reconnect cycles

* fix(channels): drain loop collect prompts directly

* fix(channels): align wecom buffer hooks

* fix(channels): harden wecom delivery failures

* fix(channels): recover from wecom attachment write failures

* fix(channels): surface wecom media send failures

* fix(channels): harden wecom replay and reconnect

* fix(channels): clarify wecom partial delivery cleanup

* fix(channels): close wecom rejected downloads

* fix(channels): retain wecom dedup after processing starts

* fix(channels): harden wecom reconnect and media errors

* fix(channels): add wecom media error context

* fix(channels): improve wecom dns diagnostics

* fix(channels): keep wecom kick retry alive

* fix(channels): allow wecom quoted bot replies

* fix(channels): preserve wecom code fences across chunks

* fix(channels): harden wecom reconnect lifecycle

* fix(channels): report wecom media dir setup failures

* fix(channels): harden wecom reconnect recovery

* fix(channels): align wecom review fixes

* fix(channels): harden wecom marker parsing

* fix(channels): keep wecom reconnect timers alive

* fix(channels): handle wecom tilde fences

* fix(channels): preserve wecom fence state

* fix(channels): clean up wecom attachment races

* fix(channels): bind wecom media reads to file handles

* fix(channels): prevent wecom symlink media opens

* fix(channels): address wecom review blockers

* fix(wecom): remove media URL from error messages to prevent credential leakage

The guardedHttpsDownload error messages included rawUrl (truncated to 120
chars), which leaks private WeCom media download URLs into stderr and log
aggregation systems. Remove the URL from redirect and HTTP error messages.

* fix(wecom): address review feedback — tests, security, correctness

- Remove stale URL assertions from media download error tests (the error
  messages no longer include raw URLs after the credential-leak fix)
- Redact sensitive fields (secret, aeskey, token, password, authorization)
  in formatSdkError's JSON.stringify fallback to prevent credential
  leakage in logs
- Add indented code block detection to findCodeRanges so [IMAGE: path]
  inside 4-space/tab-indented code is not stripped as a media marker
- Add disconnectGeneration guard before mkdirSync in downloadAttachments
  to prevent orphaned temp directories when disconnect() races with
  in-flight attachment downloads

* fix(wecom): wrap client.disconnect() in catch block to preserve connection error

In the connect() catch block, client.disconnect() could throw (e.g. if
the WebSocket was already destroyed), masking the original connection
error. Wrap in try/catch so cleanup failures never shadow the root cause.

* fix(channels): address wecom reconnect review blockers

* fix(channels): harden wecom reconnect review fixes

* fix(channels): harden wecom review blockers

* fix(channels): address wecom review blockers

* fix(channels): preserve unsupported wecom media markers

* fix(channels): address wecom reliability suggestions

* fix(channels): allow wecom retry after early drops

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-07 15:24:19 +00:00
Dragon
80340fb73f
fix(review): remove qwen-code-specific core-infra gate from bundled /review (#6412)
The bundled /review skill is a general command that runs against arbitrary
repositories (and cross-repo PRs), but a previous change baked qwen-code's own
"core infrastructure is maintainer-only" governance into the shipped prompt:
hardcoded packages/core and packages/*/src/{auth,providers,models,config,tools,services}
paths, a 500+ line hard block, and an authorAssociation-based maintainer check.
Those path names are generic — src/auth, src/config, src/tools, src/services are
common across monorepos — so an external contributor's large PR to an unrelated
repo would be hard-blocked as "must be maintainer-initiated" under a policy that
repo never adopted.

Remove the gate and its escalate-flag plumbing (Steps 1, 6, and 7) from the
bundled skill, along with the matching DESIGN.md rationale and the user-doc
section. qwen-code's maintainer-only policy stays documented in AGENTS.md for
this repo. The Issue Fidelity / root-cause ownership agent (Agent 0) is a
universal review principle and is left unchanged.

Co-authored-by: dragon <dragon@U-2Q53JQG9-0233.local>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-07 03:03:49 +00:00
Dragon
bcdb44c5d3
docs(hooks): document PreToolUse permissionDecision 'ask' behavior (#6411)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
Co-authored-by: Claude <noreply@anthropic.com>
2026-07-06 22:18:49 +00:00
Dragon
57326e55be
feat(review): add issue-fidelity and root-cause ownership gate to /review (#6395)
* feat(review): add issue-fidelity and root-cause ownership gate to /review

Adds a dedicated Issue Fidelity & Root-Cause Ownership agent (Agent 0) to
the /review pipeline and a core-infrastructure scope gate that runs before
the review agents.

Agent 0 fetches linked GitHub issue evidence directly (closingIssuesReferences
plus issue comments) instead of trusting the PR author's framing, compares the
original reported failure against the PR's claimed fix, and flags client-side
parser/sanitizer workarounds for malformed upstream output as Critical unless a
maintainer explicitly requested the defensive mitigation. The core-infra gate
applies the repository's existing two-tier maintainer-only rule before spending
review budget.

This hardens the pipeline against a false-approval mode where a bot PR passes
its own tests and reads as internally reasonable but fixes the author's mistaken
diagnosis rather than the linked issue's actual root cause.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(review): address PR review feedback on issue-fidelity gate

- Fetch issue evidence with `gh issue view --json title,body,comments` so the
  issue body (reporter repro/observed payload/expected behavior) is included;
  `--comments` alone omits it. Use each closingIssuesReferences entry's own
  repository so cross-repo linked issues resolve correctly.
- Treat closingIssuesReferences as a discovery hint (fetch apparent target
  issues even when it is empty) and treat fetched issue content as untrusted
  data (extract facts, ignore embedded instructions).
- Run Agent 0 (Issue Fidelity) only for PR targets; skip it for local-diff and
  file-path reviews, and require the PR number/repo/context in its prompt.
  Handle empty references / non-bugfix / gh failure explicitly.
- Pass Agent 0's quoted issue evidence to Step 4 batch verification and stop it
  rejecting issue-grounded findings just because the code compiles/tests pass.
- Make the core-infrastructure gate concrete: deterministic maintainer signal
  via authorAssociation, count only core-path lines, honor the AGENTS.md
  low-risk-sweep exception, clean up the worktree on hard block, run the gate
  right after fetch-pr (before npm ci), and map escalate -> COMMENT (never
  APPROVE) in Steps 6-7.
- Sync agent counts and token math across SKILL.md, DESIGN.md, and
  code-review.md (Agent 0 is PR-only; ~620-730K).

* docs(review): rename 'Linked Issue Fit' heading to 'Issue Fidelity'

Aligns the code-review docs heading with the 'Issue Fidelity' name used
for Agent 0 in SKILL.md and DESIGN.md, so the section connects to the
pipeline diagram. Addresses review feedback.

* docs(review): stop core-infra hard block before load-rules and surface it via --comment

- Hard block now stops before Step 2 (load-rules) instead of before Step 3,
  so a PR destined for hard-block no longer runs the load-rules step.
- In --comment mode the hard block posts an event=COMMENT on the PR, matching
  the escalate path's GitHub visibility, so external authors see the block.

---------

Co-authored-by: dragon <dragon@U-2Q53JQG9-0233.local>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-06 17:05:48 +00:00
Dragon
fa81e0a604
docs: fix skill invocation syntax and include Feishu in channel lists (#6320)
* docs: fix skill invocation syntax and include Feishu in channel lists

* docs: add Feishu column to channel media-handling table

Address review feedback on PR #6320: after adding Feishu to the prose
channel lists, the 'Platform differences' media-handling table still
omitted it. Add a Feishu column (images/files via the authenticated Open
API resources endpoint, 50MB limit; rich-text 'post' captions), verified
against packages/channels/feishu/src/media.ts and FeishuAdapter.ts. Add a
note that QQ Bot ignores incoming media (per QQChannel.ts) so it has no
row.

* docs: clarify Feishu post messages drop embedded images

The Platform differences table claimed Feishu rich-text (post) messages
carry 'mixed text + images', but FeishuAdapter's post parser only
extracts text/a/at nodes and silently drops img nodes (both the live
handler and the history-backfill path). Correct the Captions cell to
say text is extracted and embedded images are ignored.

* docs: mark clientId/clientSecret as required for Feishu too

Feishu declares requiredConfigFields: ['clientId', 'clientSecret']
(packages/channels/feishu/src/index.ts), same as DingTalk, but the
channel options table listed both fields as DingTalk-only. Update the
Required column and descriptions to cover Feishu (App ID / App Secret).

* docs: note token is not needed for Feishu in channel config table

* docs: note 50MB limit on Feishu image downloads

Feishu images and files both flow through the same downloadMedia() in
packages/channels/feishu/src/media.ts, which enforces a single
MAX_DOWNLOAD_BYTES = 50MB cap. Add the (50MB limit) note to the Images
cell for consistency with the Files cell.

* docs: clarify /skills panel-vs-run behavior per review feedback

- skills.md: add a migration Note that /skills <name> now opens the
  Skills panel and ignores trailing args; use /<skill-name> to run.
- commands.md: list the /skills Usage cell as /skills, /<skill-name>
  for consistency with the rest of the table.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: 易良 <1204183885@qq.com>
2026-07-05 09:49:52 +00:00
Tianyuan
0633b8a985
feat(scheduler): make recurring cron/loop job expiration configurable (#6173)
* feat(scheduler): make recurring cron/loop job expiration configurable

Recurring cron/loop jobs previously auto-expired after a hardcoded 7 days with no way to extend or disable the limit, forcing long-running daemon deployments to recreate jobs weekly.

Add an experimental.cronRecurringMaxAgeDays setting (default 7) and a QWEN_CODE_CRON_MAX_AGE_DAYS environment-variable override (takes precedence, for cloud/container deployments). A value of 0 disables expiry so jobs run until deleted; negative or unparseable values fall back to the 7-day default. The configured limit also applies to durable tasks restored from disk, and the CronCreate tool description now reflects the effective limit instead of a hardcoded '7 days'.

Closes #6167

* refactor(scheduler): drop unused DEFAULT_RECURRING_MAX_AGE_MS export

Review feedback on #6173 — the ms constant is only used inside the module as the constructor default; keep only the days constant public.

* fix(scheduler): align zero max-age semantics and warn on cron expiry misconfiguration

Review feedback on #6173:

- CronScheduler constructor now maps 0 to Infinity (never expire), matching the config layer instead of silently substituting the 7-day default for direct callers.
- Invalid QWEN_CODE_CRON_MAX_AGE_DAYS / cronRecurringMaxAgeDays values now log a warning before falling back to the default, leaving a breadcrumb for misconfigured deployments.
- Durable tasks found past the recurring max age at load now log a warning before their final fire + delete, since a lowered max age retroactively expires long-lived tasks and deletion is unrecoverable.
- New durable-restore tests: a custom max age applies to tasks reloaded from disk, and a disabled max age (Infinity) restores a 30-day-old task as live instead of aging it out.

* fix(scheduler): surface cron expiry warnings on the console

Review feedback on #6173:

- The invalid-config and retroactive-expiry warnings moved from debugLogger (file-only, off unless QWEN_DEBUG_LOG_FILE is set) to console.warn so they reach container/daemon logs where this knob matters; the config warning is latched to fire once per Config instance.
- CronCreateTool's constructor now resolves the max age once instead of three times, so an invalid env var can't emit duplicate warnings during tool registration.

* fix(scheduler): reject non-finite timestamps in durable task validation

Review feedback on #6173: JSON like -1e999 parses to -Infinity, which passes the typeof-number check and then poisons date math — with a finite lastFiredAt the entry reads as an overdue aged task and the retroactive-expiry warning's toISOString() throws RangeError mid-load, so one malformed persisted entry blocks durable cron startup/takeover. Validation now requires finite createdAt (and lastFiredAt when non-null), routing such entries through the existing fix-or-delete contract for corrupt files. Verified the new end-to-end test reproduces the RangeError without the fix.

* refactor(scheduler): single-source the max-age contract and freeze it at Config construction

Review follow-ups on #6173:
- Extract normalizeRecurringMaxAge as the single owner of the
  0/Infinity no-expiry contract, used by both the Config layer and the
  CronScheduler constructor, so the constructor's 0 handling can no
  longer be removed as apparent dead code.
- Resolve QWEN_CODE_CRON_MAX_AGE_DAYS once at Config construction into
  a readonly field, honoring the setting's requiresRestart contract;
  mid-session env changes can no longer make the tool description,
  tool output, and scheduler report different expiries. The warn
  once-latch is now unnecessary (construction warns at most once).
2026-07-03 06:34:37 +00:00
Dragon
dc8e155927
docs: correct stale CLI flags/keybinding and document model.reasoningEffort (#6219)
- Remove nonexistent --all-files/-a and --show-memory-usage flags from the
  CLI arguments and headless option tables (no longer defined in the yargs
  parser in packages/cli/src/config/config.ts).
- Add the commonly-needed --model/-m flag to the headless options table and
  fix the --approval-mode example to use the valid choice auto-edit (the
  parser rejects the underscore form auto_edit).
- Drop the stale Meta+Enter alias from the external-editor shortcut; that
  chord is bound to NEWLINE, while OPEN_EXTERNAL_EDITOR binds only Ctrl+X.
- Document the model.reasoningEffort setting (set via /effort), which is
  exposed in the settings dialog but was missing from the settings reference.

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-03 01:55:45 +00:00
Dragon
55e425c1d2
docs: document the /config slash command (#6145)
Co-authored-by: Claude <noreply@anthropic.com>
2026-07-02 13:52:59 +08:00
Dragon
2c95d9383f
feat(core,cli): unified reasoning effort with /effort command (#6072)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
* feat(core,cli): unified reasoning effort with /effort command

Expose a provider-agnostic reasoning-effort ladder (low/medium/high/xhigh/max) through a new /effort slash command and a global model.reasoningEffort setting. Bare /effort opens an interactive tier picker (and lists tiers in non-interactive/ACP mode); /effort <tier> sets one directly. A single canonical reasoning.effort config is translated and clamped per provider so one control works everywhere:

- OpenAI: reasoning_effort (max clamps to xhigh)
- DeepSeek: flat reasoning_effort (low/medium→high, xhigh→max)
- GLM / z.ai: new adapter flattens nested reasoning.effort to a verbatim reasoning_effort
- Anthropic: output_config.effort, gated per model — Opus 4.7/4.8 and 5.x families pass xhigh/max through; Opus 4.6/Sonnet 4.6 take max only; older clamp to high
- Gemini: thinking_level (adds medium; xhigh/max cap at HIGH)
- Qwen / DashScope: a set effort turns on enable_thinking (no per-tier field yet — the single column to extend when qwen ships one)

Config.setReasoningEffort updates the live config so the change takes effect on the next turn without an auth refresh, and is re-applied across model switches and auth refreshes (including the initial one at startup) so the effort survives /model switches and CLI restarts — even for provider-backed models, whose preset sync would otherwise overwrite reasoning and drop the persisted effort. The model-with-reasoning status line now tracks the effort and updates immediately when /effort changes it.

A new core/reasoning-effort.ts holds the canonical tier list, rank-based clamp, and input normalizer shared across providers and the command.

* fix(core): reject date suffix as Anthropic minor version for effort gating

The version regex parsed the 8-digit date suffix of dated model ids
(e.g. claude-opus-4-20250514 = Opus 4.0) as the minor version, so
atLeast(4, 6)/atLeast(4, 7) returned true and Opus 4.0 was wrongly
granted the xhigh/max effort tiers it does not support, producing an
HTTP 400 on /effort max with no clamp warning.

Cap the minor-version group at one or two digits with a trailing (?!\d)
so a date suffix is not mis-parsed (a bare {1,2} cap is insufficient: it
would still greedily capture '20' from '20250514'). Dated ids that do
carry a minor, like claude-opus-4-7-20251101, still resolve to minor 7.

Adds a regression test asserting effort 'max' clamps to 'high' on
claude-opus-4-20250514.

* fix(i18n): translate /effort command description for all locales

The mustTranslateKeys strict-parity test failed because the new /effort
command description fell back to English in zh and zh-TW. Add the
description string to every locale file so the command is translated like
its siblings and strict-parity locales no longer fall back.

* fix(core): correct DeepSeek effort mapping and adaptive-thinking model parse

Address two review findings on the reasoning-effort path in
anthropicContentGenerator:

- DeepSeek anthropic-compatible output_config.effort accepts only high/max,
  but resolveEffectiveEffort passed low/medium through verbatim (only
  remapping xhigh -> max), which the endpoint would 400 on. Mirror the
  DeepSeek OpenAI adapter (deepseek.ts): low/medium lift to high, xhigh/max
  group to max.

- modelSupportsAdaptiveThinking used /claude-(?:opus|sonnet|haiku)-(\d+)-(\d+)/,
  so a dated id like claude-opus-4-20250514 (Opus 4.0) parsed the 8-digit date
  suffix as the minor version (>= 6), wrongly selecting adaptive thinking and
  tripping a server 400. Apply the same date-suffix-guarded regex the sibling
  anthropicSupportedEffortTiers already uses, also covering the fable/mythos
  families and a bare major (claude-opus-5).

Add regression tests for both.

* fix(core,cli): address /effort review feedback

- anthropic: family-guard the `max` tier on the 4.x branch so haiku 4.x no
  longer gains `max` (a server 400); 5.x families still get it via major>=5.
- effort command: when `setReasoningEffort` no-ops because thinking is disabled
  (reasoning: false), report that the tier won't take effect yet instead of a
  misleading success message (the tier is still persisted for future sessions).
- dashscope: drop the pipeline-injected nested `reasoning` object when
  `enable_thinking` is set, so qwen never receives two competing thinking knobs
  (mirrors deepseek.ts / zai.ts).

Adds regression tests for each.

* fix(core): collapse empty reasoning to undefined and drop redundant effort re-apply

setReasoningEffort(undefined) now resets reasoning to undefined instead of
leaving a truthy empty object, which the pipeline would emit as wire noise
and which would make 'if (cfg.reasoning)' a false positive.

handleModelChange no longer re-applies the reasoning effort after the
full-refresh path: refreshAuth already captures and restores it, so the
second call was redundant. The qwen-oauth hot-update path keeps its own
re-apply because it never routes through refreshAuth.

* fix(core): log once when Gemini clamps xhigh/max effort to HIGH

Gemini has no tier above HIGH, so /effort xhigh|max silently ran at HIGH
with no trace. Mirror the Anthropic generator's one-time clamp warning via
a per-generator latch so the downgrade is discoverable in debug logs.

* fix(core): warn when GLM model on non-Z.ai host skips reasoning_effort flatten

isZaiProvider routes any glm-* model through this provider, but the
reasoning_effort reshape stays hostname-gated. A self-hosted GLM on a
non-Z.ai hostname therefore kept its nested reasoning.effort unflattened
silently. Keep the reshape hostname-gated (so we never push GLM's flat
field at an arbitrary OpenAI-compatible backend) but log the skip once so
the gap is discoverable.

* docs(core,cli): correct false OpenAI 'max'->'xhigh' clamp claim

The reasoning-effort docs claimed OpenAI adapters clamp 'max' to 'xhigh',
but the default OpenAI-compatible pipeline forwards the tier verbatim and
no such clamp exists. Reword the doc comment and the reasoningEffort
setting description (and the regenerated VS Code schema) to match.

* fix(cli): clarify effort confirmation and improve the effort dialog UX

- The /effort confirmation now states the tier is the requested one and the
  effective tier depends on the active provider/model, instead of implying
  the shown tier is what reaches the wire (providers clamp per model).
- EffortDialog no longer pre-selects 'high' when no effort is configured;
  the cursor starts at the top and a footer notes the provider default is
  in effect, so the highlight is not mistaken for the current setting.
- The dialog path now mirrors the slash-command read-back: when thinking is
  disabled (setReasoningEffort is a no-op), it surfaces an info message that
  the tier is persisted but won't take effect until thinking is re-enabled.

* refactor(core): extract shared parseClaudeModelVersion for Claude effort gating

anthropicSupportedEffortTiers and modelSupportsAdaptiveThinking each parsed
Claude model ids with their own near-identical regex (family list +
date-suffix guard), kept in sync only by a comment. Extract a single
parseClaudeModelVersion helper both consume so adding a new family is a
one-line change with no drift risk between effort tiers and the thinking
shape.

* fix(anthropic): drop manual budget_tokens on models that reject it

Opus 4.7+ and every 5.x family (Sonnet 5, Fable 5, Mythos 5) dropped
manual extended thinking: a request with
thinking:{type:'enabled', budget_tokens:N} returns a 400 on those models,
which require adaptive thinking with output_config.effort instead
(https://platform.claude.com/docs/en/build-with-claude/effort).

buildThinkingConfig honored an explicit reasoning.budget_tokens on every
model, so a preset carrying budget_tokens (with or without /effort) shipped
the manual shape to an adaptive-only model and 400'd. Gate the escape hatch
behind a new modelRejectsManualThinking() predicate so the budget is only
emitted for models that still accept it (Opus 4.5/4.6, Sonnet 4.6, older
4.x, and unknown/unversioned ids); adaptive-only models fall through to
{type:'adaptive'} and let output_config.effort govern thinking depth.

* fix(cli,core): warn on invalid model.reasoningEffort; guard hot-update field set

- modelConfigUtils: surface a startup warning when settings.model.reasoningEffort
  holds an unrecognized value (normalizes to undefined and was silently skipped),
  so a typo in settings.json doesn't leave /effort with no visible effect.
- config: document that the qwen-oauth hot-update field set deliberately excludes
  `reasoning`, which is captured and re-applied via setReasoningEffort() — adding
  it to the copied set would mask the effort restore across model switches.

* fix(core,cli): address /effort review feedback

- config: re-apply reasoning effort on the full-refresh model-switch path.
  switchModel() wipes modelsConfig.reasoning via applyResolvedModelDefaults
  before handleModelChange fires, so refreshAuth's own capture reads undefined;
  restore the tier from the live config. Add a regression test.
- gemini: make buildThinkingConfig's effort switch exhaustive (explicit
  'undefined' case + never-guarded default) so a new tier is a compile error
  rather than a silent map to UNSPECIFIED.
- anthropic: emit a one-time warning when budget_tokens is dropped on models
  that reject manual thinking (Opus 4.7+/5.x); reword the version-regex comment
  to correctly credit the (?!\\d) lookahead. Add reseller-prefixed and 5.x
  effort-gating tests.
- cli/effort: confirm the requested tier in-chat on dialog success (mirror the
  slash-command message); add hook tests.
- tests: add dashscope vision-branch effort test and determineProvider z.ai
  routing tests.

* fix(core): harden reasoning-effort normalize and correct stale clamp comment

- normalizeReasoningEffort: reject non-string input (e.g. a hand-edited
  settings.json with a boolean/number reasoningEffort) instead of calling
  .trim() on it and crashing at startup; add regression tests.
- contentGenerator: update the reasoning-effort ladder comment to reflect
  the per-model Anthropic tier gating (Opus 4.7+/5.x accept xhigh/max,
  Opus/Sonnet 4.6 accept max, older models cap at high) instead of the
  outdated blanket 'xhigh/max -> high' description.

Addresses PR review feedback.

* fix: remove stray OpenClaw-Query-Submit gitlink

This branch carries a submodule gitlink (mode 160000, commit be66572)
with no .gitmodules entry. It entered via a merge from an older main
state; current main no longer contains it, so this branch would
re-introduce it on merge, breaking clone --recursive and any submodule
init. Remove it from the index.

* test(cli): add EffortDialog render tests

Adds render/behavior tests for EffortDialog mirroring
ApprovalModeDialog.test.tsx: renders the title and all five tiers,
shows/hides the 'no effort configured' hint based on currentEffort,
and verifies the active Escape handler cancels with undefined while
non-Escape keys do not.

* refactor(anthropic): single-source Claude family list; warn on DeepSeek effort remap

Derive the ClaudeModelFamily union and the model-id regex from one
CLAUDE_MODEL_FAMILIES const so the type and the parser can't drift apart
behind the 'as ClaudeModelFamily' cast.

Emit a one-time debugLogger.warn when the DeepSeek anthropic-compatible
branch remaps a requested effort tier (low/medium -> high, xhigh -> max),
mirroring the real-Anthropic clamp path so a silently upgraded effort is
visible in debug logs.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: DragonnZhang <dragonzhang1024@gmail.com>
2026-07-01 20:20:28 +00:00
Dragon
427b5ade33
docs: document model/auth settings, /model --vision, and --safe-mode (#6028)
* docs: document model/auth settings, /model --vision, and --safe-mode

Refresh user docs to match the current codebase:

- commands.md: add the /model --vision override (vision-bridge model)
- settings.md: add model.baseUrl, model.sessionTokenLimit, visionModel,
  and voiceModel; document the deprecated security.auth.apiKey and
  security.auth.baseUrl keys with a pointer to modelProviders
- troubleshooting.md: document the --safe-mode flag for isolating
  customization issues

* docs: address review feedback on sessionTokenLimit, safe-mode, deprecation notes

- model.sessionTokenLimit: correct default to -1 (runtime fallback in
  core/config.ts) and clarify breach behavior (current send dropped, not
  session abort) per client.ts SessionTokenLimitExceeded handling.
- --safe-mode: expand the disabled-customizations list to also cover
  permission rules, approval mode overrides, memory features, and sandbox
  settings, matching cli/config.ts.
- security.auth.apiKey/baseUrl: align deprecation wording with the existing
  tools.* entries (**Deprecated.**) and drop the unsubstantiated
  '(slated for removal)' qualifier.

* docs: note QWEN_CODE_SAFE_MODE env var as a safe-mode alternative

Document the QWEN_CODE_SAFE_MODE=true environment variable as an
alternative activation path for safe mode, for cases where the CLI
cannot accept flags (verified against isSafeModeEnv in
packages/core/src/utils/safe-mode.ts).

* docs: clarify model.baseUrl, sessionTokenLimit=0, and safe-mode subagents

- model.baseUrl: describe it as a picker-managed disambiguator, not a
  hand-editable override (stale values can misroute to a same-id provider).
- model.sessionTokenLimit: note that 0 is treated as unlimited (same as -1),
  unlike model.maxToolCalls where 0 disallows all calls.
- --safe-mode: include custom subagents in the list of disabled
  customizations (only built-in subagents load in safe mode).

* docs: clarify sessionTokenLimit semantics and add --safe-mode to headless flags

- settings.md: reword model.sessionTokenLimit to reflect that the gate
  compares the last recorded prompt token count before the next send
  (not a per-send preflight cap), and that the next send is dropped.
- headless.md: add a --safe-mode row to the CLI flags table so the
  diagnostic flag is discoverable there, cross-referencing Troubleshooting.

* docs: align safe-mode sandbox wording to 'sandbox settings'

Safe mode passes an empty Settings object to loadSandboxConfig
(packages/cli/src/config/config.ts:1793), so it strips settings-sourced
sandbox config while the --sandbox flag and QWEN_SANDBOX env still apply.
Match headless.md to troubleshooting.md's accurate 'sandbox settings'.

* docs: correct safe-mode approval-mode wording and align both lists

Safe mode only strips settings-sourced approval mode; the --yolo and
--approval-mode CLI flags are evaluated before the safeMode guard
(packages/cli/src/config/config.ts:1521-1528) and still take effect.
Reword to 'settings-sourced approval mode overrides' and note the CLI
flags in troubleshooting.md and headless.md, and make the enumerated
safe-mode disable list identical (same items and order) across both.

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-01 15:34:20 +00:00
qqqys
6a5ad453c8
[codex] Add explicit channel memory for messaging channels (#6051)
* chore: ignore local worktrees

* feat(core): add channel memory store

* fix(core): preserve dots in channel memory paths

* fix(core): guard dot channel memory paths

* feat(channels): add channel memory commands

* fix(channels): defer channel memory session invalidation

* feat(channels): inject channel memory into sessions

* fix(channels): serialize channel memory context injection

* fix(channels): harden channel memory context races

* fix(channels): let queued turn retry memory context

* feat(cli): wire channel memory into channel startup

* docs(channels): document channel memory

* fix(channels): harden channel memory handling

* fix(channels): gate channel memory injection

* fix(channels): serialize channel memory clears

* fix(channels): tolerate channel memory read failures

* fix(channels): harden channel memory review gaps

* fix(channels): harden channel memory review gaps

* fix(channels): protect shared channel memory

* test(channels): allow group memory view test

* fix(channels): block group memory writes

* fix(channels): include memory in loop prompts

* fix(channels): preserve session context order after merge

* fix(channels): retry loop memory context after read failure

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
2026-07-01 09:59:48 +00:00
qqqys
891c59fbaa
feat(channels): add group history backfill (#6074)
* feat(channels): add group history backfill

* fix(channels): harden group history backfill

* fix(channels): address group history review gaps

* fix(channels): apply wildcard group history limit
2026-07-01 16:24:09 +08:00
Shaojin Wen
17fbaa25cd
refactor(review): drop deterministic-analysis and autofix steps (#6092)
* refactor(review): drop deterministic-analysis and autofix steps

Slim the bundled /review skill from 11 to 9 steps by removing Step 3
(deterministic analysis — auto-run of tsc/eslint/ruff/clippy/go vet
plus CI-lint discovery) and Step 8 (autofix — PR-worktree auto-fix,
commit and push).

Renumber the remaining steps (4→3 … 11→9) and update every
cross-reference. Agent 7 (Build & Test) stays in the parallel review
step and now always runs build+test instead of skipping when Step 3
had already compiled. The [linter]/[typecheck] source tags are dropped;
[build]/[test]/[review] remain. DESIGN.md is updated to match (step
numbers, removed Autofix/deterministic rationale, LLM-budget table).

* refactor(review): address review feedback on the /review slimming

Follow-up to the 11->9 step change, addressing PR review comments:

- Restore base-branch CI-config protection in Agent 7. The removed Step 3 carried the instruction to read CI config from the base branch; without it Agent 7 would discover build/test commands from the untrusted PR branch. Re-added to Agent 7's CI-config discovery clause.
- Drop the stale "linters" token from the worktree rule (no standalone linter step runs anymore).
- Narrow the exclusion criteria: substantive lint/type issues (unused vars, unreachable code, type errors) are no longer auto-excluded now that no deterministic tool catches them; only pure formatting stays excluded.
- Update user docs to match: docs/users/features/code-review.md (11->9 steps, remove the Deterministic Analysis and Autofix sections, drop the two comparison-table rows, renumber the Token-efficiency table) and docs/users/features/commands.md (agent count).
- Fix stale step-number references in CLI comments: cleanup.ts (Step 11->9) and presubmit.ts (Step 9->7, comment + yargs describe).

* refactor(review): remove orphaned deterministic subcommand, polish docs

Second round of PR feedback:

- Remove the now-orphaned `qwen review deterministic` subcommand. review.ts describes these subcommands as "internal helpers used by the /review skill"; with Step 3 gone the skill no longer invokes it, so the ~740-line module plus its import / registration / describe / subcommand-list entries were dead code. Deleted deterministic.ts and its wiring in review.ts.
- Decouple the exclusion criterion from pipeline state (SKILL.md): substantive lint/type issues (unused vars, unreachable code, type errors) are now "in scope — LLM agents should report them" rather than "no longer have a deterministic tool catching them", so the rule stays correct if a linter step is ever re-added.
- Drop the stale "linting" justification from the worktree dependency-install note (SKILL.md); only build/test remain.
- DESIGN.md: rename the subcommands section to "presubmit and cleanup", drop "lint" from the review-tools rejected alternative, and fix the "we already have those" cell to "We retain build/test (Agent 7)".

* refactor(review): resolve exclusion-criteria contradiction, polish wording

Third round of PR feedback:

- Merge the exclusion criteria to remove the contradiction between the unconditional "matches codebase conventions" exclude and the "substantive lint/type issues are in scope" include. Now a single bullet: cosmetic style/formatting/naming is excluded, but substantive issues a linter or type checker would flag (unused variables, unreachable code, type errors) are in scope even where the surrounding code tolerates them. Kept decoupled from pipeline state (no "deterministic tool" wording). Applied in both SKILL.md and docs/users/features/code-review.md.
- SKILL.md lightweight-mode skip: "(no local reports or cache)" to match Step 8's title ("Save review report and cache").
- DESIGN.md: rename the CI-config section to "auto-discover build/test commands" to match the body, which was narrowed to build/test only.

* test(review): guard qwen review subcommand surface, fix stale docs linting ref

- Add packages/cli/src/commands/review.test.ts verifying the `qwen review` builder registers exactly [fetch-pr, pr-context, load-rules, presubmit, cleanup], no longer registers the removed `deterministic` subcommand, and that `describe` no longer mentions deterministic analysis. Guards against silently re-adding the subcommand or dropping a helper (review.ts previously had no test).
- docs/users/features/code-review.md: drop the orphaned "linting" from the Worktree Isolation dependency-install note; only build/test remain now that Step 3 is gone.
2026-07-01 16:24:02 +08:00
DennisYu07
855a2b3d69
feat(auto-mode): add classifyAllShell setting to route all shell commands through classifier (#6040)
* feat(auto-mode): add classifyAllShell setting to route all shell commands through classifier

When autoMode.classifyAllShell is true, every shell command (including
read-only ones that would normally be auto-approved) is routed through
the auto-mode LLM classifier for safety review. Default false preserves
existing behavior.

Closes #6039

* fix(core): add classifyAllShell check to autoApproveCompatiblePendingTools

The third forceAutoReviewForAllow computation site in
autoApproveCompatiblePendingTools was missing the
shouldClassifyAllShellForAutoMode OR-branch, allowing shell commands
to bypass the classifier when classifyAllShell is enabled.

Addresses qwen-code-ci-bot critical review on PR #6040.

* fix(core): use optional chaining in shouldClassifyAllShellForAutoMode

Prevents TypeError when config.getAutoModeSettings() returns undefined
in tests or edge cases where AutoModeSettings is not initialized.

* docs(auto-mode): document classifyAllShell setting

Add new section explaining the classifyAllShell option, a tip in the
'How it works' overview, and a commented-out example in the approval-mode
configuration reference.

* fix(test): add missing getAutoModeSettings mock in AUTO denial counter reset test
2026-07-01 16:23:24 +08:00
Dragon
e324104ce8
docs: refresh settings, MCP glob, auth alias, and autonomous loop docs (#6090)
Audit docs/ against the current codebase and correct user-facing drift:

- Document glob-pattern support (* and ?) for mcp.allowed / mcp.excluded
  in settings.md and the MCP feature page (feat #6012).
- Add missing user-facing settings rows: general.terminalBell,
  general.preventSystemSleep, general.chatRecording; ui.showStatusInTitle,
  ui.disableWorkflowKeywordTrigger, ui.enableUserFeedback, ui.compactInline,
  ui.useTerminalBuffer, ui.hideBuiltinWorktreeIndicator;
  memory.enableTeamMemory, memory.enableTeamMemorySync; tools.toolSearch.enabled.
- Note the QWEN_MODEL alias for OPENAI_MODEL in the auth protocol table.
- Document the autonomous (bare /loop) mode in scheduled-tasks (feat #5991).

Co-authored-by: Claude <noreply@anthropic.com>
2026-07-01 06:43:29 +00:00