3741 commits

Author	SHA1	Message	Date
Shaojin Wen	dade3ab334	feat(web-shell): add git branch picker, commit dialog, and create PR flow (#7731) ... * feat(web-shell): add git branch picker, commit dialog, and create PR flow Add an IntelliJ-style branch picker popover to the web shell git workspace, accessible from the branch chip in both the composer toolbar and sidebar. The picker provides search-filtered branch listing (local, remote, tags, recent), branch checkout, new branch creation, pull, push, and a commit view integrated into the existing GitDialog. The commit view reuses the diff panel (expandable file diffs with syntax highlighting, fullscreen support) and adds a commit message textarea with Commit / Commit and Push buttons. When a session is available (or one is auto-created), the commit message and PR title/body are generated via the model using session side-queries (btwSession), giving the agent full conversation context for accurate generation. The Create PR flow provides an inline form with auto-detected base branch and model-generated title/description, backed by a new daemon route that shells out to gh pr create. New daemon routes: - GET /workspaces/:workspace/git/branches - POST /workspaces/:workspace/git/checkout - POST /workspaces/:workspace/git/branch - POST /workspaces/:workspace/git/push - POST /workspaces/:workspace/git/pull - POST /workspaces/:workspace/git/commit - POST /workspaces/:workspace/github/prs/create - GET /workspaces/:workspace/github/default-branch * fix(web-shell): resolve correct workspace session for AI generation The commit message and PR title/body generation now resolves the most recent session for the target workspace via listWorkspaceSessions, rather than using the globally active connection.sessionId which may belong to a different workspace or session. Falls back to creating a new session only when no sessions exist for the workspace. Also improves the PR body editor with an Edit/Preview toggle using the existing Markdown component, and updates the generation prompt to follow the project PR template structure from AGENTS.md. * fix(web-shell): stabilize session resolver prop to prevent infinite re-generation Pass resolveSessionForWorkspace as a stable useCallback reference instead of an inline arrow function. The inline function created a new reference on every App render, causing the GitDialog useEffect to abort and restart generation in an infinite loop. * fix(web-shell): group remote branches by remote name and add PR target branch dropdown Remote branches in the branch picker are now grouped by remote (origin, upstream, etc.) with sub-headers, making fork workflows clear. The PR create form's base branch field is now a select dropdown populated from the workspace's branch list, grouped by remote with optgroup labels, instead of a free-text input. * fix(web-shell): use ref for session resolver to prevent effect re-run abort When resolveSessionForWorkspace creates a new session, it updates connection.sessionId in the provider, which changes the useCallback reference, which triggers the useEffect to re-run and abort the in-flight btwSession generation. Store the callback in a ref so the effect never depends on its identity. * fix(web-shell): resolve session per workspace, not from global active session The commit/PR generation effects used connection.sessionId directly without checking if it belongs to the target workspace. When opening the commit dialog from a sidebar workspace different from the active session's workspace, the wrong session was used for generation, producing incorrect content. Now always routes through resolveSessionForWorkspace(workspaceCwd) which checks workspace membership before reusing the active session. Also replaces 'PR' with 'Pull Request' / '合并请求' in all UI strings and adds error logging to the generation catch blocks. * fix(web-shell): retry btwSession with fresh session when stale session detected When listWorkspaceSessions returns a session that no longer exists in the daemon's memory (e.g. after daemon restart), btwSession fails with 'No session with id ...'. The generation effects now catch this error, force-create a new session via resolveSessionForWorkspace(cwd, true), and retry the btwSession call once. Both btwWithRetry and resolveSessionForWorkspace are stored in refs to avoid useEffect dependency chain aborts. * fix(web-shell): base PR generation on branch diff, not working tree PR title/body generation now fetches the commit log between the resolved base branch and HEAD (git log <base>..HEAD) plus any uncommitted changes, instead of only the working tree diff. The base branch is resolved inside the effect's promise chain (not from state) to avoid stale values and dependency warnings. Also adds range parameter support to fetchGitLog and workspaceGitLog. * feat(web-shell): replace PR base branch select with searchable popover The native <select> for the PR target branch is replaced with a custom searchable popover (BranchSelect) that shows a search input and a filtered branch list grouped by remote. The default selection is the target repository's main branch (resolved via getDefaultBranch). Supports filtering by typing in the search box. * fix(web-shell): show full remote ref in branch select (origin/main) Branch select now stores and displays full remote refs like origin/main instead of stripped names. getDefaultBranch returns the full ref (origin/main) instead of stripping the prefix. When creating the PR, the remote prefix is stripped for the gh pr create --base flag (origin/main → main). * fix(web-shell): stop pointer propagation in branch picker list to prevent popover dismiss Radix Popover's outside-click detection was incorrectly firing when clicking section headers (Recent/Local/Remote/Tags) inside the popover content, causing the popover to close immediately. Adding onPointerDown stopPropagation on the list container prevents pointer events from reaching Radix's document-level handlers. * fix(web-shell): use onPointerDownOutside guard for branch picker popover The previous stopPropagation approach failed because Radix Popover uses capture-phase document listeners for outside-click detection, which fire before bubble-phase stopPropagation. The correct fix is onPointerDownOutside on PopoverContent: when Radix incorrectly fires the outside handler for a click that is actually inside the content (can happen with portal containers), we check contentRef.contains() and preventDefault to keep the popover open. * fix(web-shell): stop click propagation on branch picker popover content Root cause: the ChatEditor composer container has onClick that calls core.focus(), stealing focus from the popover. React synthetic events bubble through the React tree (not DOM tree), so portaled popover clicks reach the container handler. Radix then detects focus-outside and dismisses the popover. Fix: onClick stopPropagation on PopoverContent, matching the existing pattern in GitModePopover and ToolbarPopover which already have this fix with an explanatory comment. * docs: add PR verification screenshots for branch picker feature * fix(web-shell): mock useWorkspace in tests for BranchPickerPopover BranchPickerPopover calls useWorkspace() which requires DaemonWorkspaceProvider context. The existing WorkspaceSection and ChatEditor tests didn't provide this context, causing 5 test failures. Added vi.mock with importActual to preserve other exports while providing a mock useWorkspace. Also updated the git chip click test to reflect that clicking now opens the branch picker popover instead of directly calling onOpenGitDiff. * fix: harden git write paths against argument injection Address review feedback on the web-shell git surface: - Reject option/pathspec injection in git checkout ref and branch start point (isValidCheckoutRef), and terminate `git checkout` argv with `--`. - Drop `git log` range values that start with `-` and terminate the argv with `--` so a range can never be reinterpreted as `--output=<file>`. - Fix getDefaultBranch always falling back to origin/main: the promisified exec lacked `encoding: 'utf8'`, so stdout was a Buffer and .trim() threw. - Parameterize ghErrorMessage so `gh pr create` timeouts name the right command and duration; sanitize workspace paths in PR-create errors. - GitDialog: guard doCommit against double-submit (button + keyboard), and strip only a known remote prefix from the PR base so local branches with "/" are not mangled; use theme tokens for commit button/success colors. - BranchPickerPopover: guard checkout/new-branch behind busyAction, reset inline-input text on reopen, and hide the commit action when unavailable. Adds regression tests for the checkout/branch validation and the git log range guard. * fix(web-shell): address review feedback on branch picker PR (#7731) - Fix Commit+Push error masking: split try/catch so push failure reports alongside the successful commit SHA - Replace hardcoded screenshot path with captureScreenshot harness - Replace silent if-isVisible skip with explicit assertion in visual test - Add focus-visible style for search input accessibility - Fix CSS specificity for active PR tab hover state - Add viewChanges to actionsVisible search filter - Wrap toggleSection in useCallback to avoid unnecessary re-renders - Add windowsHide: true to getDefaultBranch subprocess - Fix trailing slash handling in mockDaemon git action routing - Add git methods to top-level client mock in tests - Remove dead branchPicker.commitSuccess i18n key - Remove dead .actionShortcut CSS class - Show generation failure feedback in commit message placeholder * fix(web-shell): address review feedback on branch picker PR (#7731) - Add workspace trust checks to bound git branch routes - Use workspace-scoped client in BranchPickerPopover (fixes wrong-workspace mutation) - Add branch name validation and -- terminator to gitCreateBranch - Filter refs/remotes/*/HEAD from branch listings - Force LC_ALL=C for reflog parsing (non-English locale fix) - Narrow 'could not resolve' error regex to avoid DNS false positives - Add range validation to git log (reject path traversal) - Fix commit+push error i18n (dedicated key instead of concatenation) - Add i18n for BranchSelect component strings - Fix commit tab ARIA attributes - Add onBranchChanged callback to handlePush - Add btw to mockDaemon isDaemonPath regex - Add workspace_github_prs to visuals spec capabilities - Add -- terminator regression test - Remove docs/pr-assets/ from repo * fix(web-shell): address review feedback on branch picker PR (#7731) * fix(cli): reject dash-prefixed branch name with 400 in branch route (#7731) * fix(web-shell): address review feedback on git branch picker (#7731) Security: - Clear GIT_DIR/GIT_WORK_TREE/GIT_COMMON_DIR/GIT_INDEX_FILE from git subprocess env to prevent repository redirection - Add strict mutation gate to all POST git branch routes - Add generation guard to qualified write routes - Fail closed on invalid ?cwd= in mutation routes (resolveContainedCwdOrFail) - Reject wrong-typed startPoint, fetchOnly, rebase, and PR options with 400 Correctness: - Filter remote symbolic refs (origin/HEAD) by %(symref) instead of /HEAD name suffix, preserving valid branches like feature/HEAD - Add git rev-parse --git-dir probe so non-git dirs get 404 instead of empty available:true - Push preserves existing upstream; only adds --set-upstream when unset, resolving the remote from branch config or the sole configured remote - git commit -a replaced with git add -A + git commit so untracked files displayed in the UI are included - Always pass --body to gh pr create to prevent interactive prompts - getDefaultBranch returns null instead of fabricating origin/main - Memoize workspaceByCwd client in BranchPickerPopover to fix infinite render loop - Move sessionId to a ref in GitDialog effects to prevent self-abort - Bound commit-message prompt to fit /btw 4096-char limit - Mark all platforms as unverified in PR template (no fabricated ✅) - Guard PR auto-fill effect against wiping user edits on reconnect Accessibility: - Add tabIndex and onKeyDown to commit-mode tab span - Add aria-label to BranchSelect trigger and search input Cleanup: - Remove dead CSS (.prInputSmall, .prSelect) - Remove 9 unused i18n keys - Add ^ to git log range validation regex - Add busyAction guard to handlePush/handlePull - Add unit tests for gitCommit, gitPull, and route input validation * fix(web-shell): address review feedback on git branch picker PR (#7731) - Change commit tab from <span> to <button> for keyboard accessibility - Move setCommitMsg('') to success-only branches so the message is preserved when push fails after a successful commit - Add mutate middleware and generationGuard to PR creation route, matching all other POST mutation routes - Set genFailed when session resolution returns undefined so the user sees the failure indicator instead of a silent empty textarea - Make PR number nullable when URL regex does not match instead of returning a misleading 0 - Add LC_ALL=C and LANG=C to gitEnv() so for-each-ref upstream track parsing is locale-independent - Validate setUpstream and force as booleans in handlePush, matching the existing validation in handlePull - Add missing workspaceCwd and available fields to test mocks - Use stable data-web-shell-git-branch attribute in e2e selector * fix(web-shell): address R5 review feedback on git branch picker PR (#7731) - Classify git errors on stdout+stderr instead of err.message to fix false-positive no_upstream on every push failure and dead nothing_to_commit classifier - Sanitize workspace paths and cap error message length in sendGitError - Fix remote branch checkout to strip remote prefix so git DWIM creates a local tracking branch instead of detaching HEAD - Restore keyboard accessibility on composer branch chip (span → button) - Trim startPoint in handleCreateBranch before forwarding to git - Return bare branch name from getDefaultBranch (strip remote prefix) - Fix i18n shortcut hint to show ⌘/Ctrl+Enter for cross-platform - Update aria-label to reflect git management menu, not just changes - Add available: true to mockDaemon gitDiff default payload - Add regression tests: upstream preservation, sole remote resolution, strengthened fetch-only with divergent remote commit - Add aria-expanded assertion to sidebar picker test Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(web-shell): address R6 review feedback on git branch picker PR (#7731) * fix(web-shell): address R7 review feedback on git branch picker PR (#7731) * fix(web-shell): address R8 review feedback on git branch picker PR (#7731) * fix(web-shell): address R9 review feedback on git branch picker PR (#7731) * fix(web-shell): address R10 review feedback on git branch picker PR (#7731) * fix(web-shell): address R11 review feedback on git branch picker PR (#7731) * fix(web-shell): address R12 review feedback on git branch picker PR (#7731) * fix(web-shell): address R13 review feedback on git branch picker PR (#7731) * fix(web-shell): address R14 review feedback on git branch picker PR (#7731) * fix(web-shell): address R15 review feedback on git branch picker PR (#7731) - Validate localName derived from remote-tracking ref to prevent option injection (e.g. origin/-f → git checkout -f) - Add gitCwd prop to BranchPickerPopover and pass it to all git SDK calls so worktree sessions target the correct directory - Add symlink-escape and non-existent-path tests for resolveContainedCwdOrFail - Pin initial branch name in makeRepo() with git init -b master - Add gitPull merge and rebase integration tests * fix(web-shell): address git branch picker review feedback (#7731) * fix(web-shell): address R6 review feedback on git branch picker PR (#7731) * fix(web-shell): address review feedback on git branch picker PR (#7731) - Strip GIT_CONFIG_GLOBAL/SYSTEM/NOSYSTEM in gitEnv to prevent inherited config redirection (consistent with extension/github.ts) - Pass gitCwd to workspaceGitBranches in GitDialog loadPrBranches so worktree sessions fetch branches from the correct repository - Add aria-expanded to collapsible branch section headers - Add happy-path tests for PR create (201) and default-branch (200) routes, including the null fallback to origin/main * fix(cli): add sendGenerationClosedError to POST routes and cover untested branches (#7731) * fix(web-shell): address review feedback on branch picker and PR creation (#7731) - Refresh branch list after push/pull to avoid stale ahead/behind counts - Add pre-flight check for unpushed branches before PR creation - Fix base branch prefix stripping when branch list is unavailable - Cap PR body file list at MAX_SUMMARY_CHARS to bound model prompt size - Add qualified route tests: trust guard, input validation, cwd containment * fix(web-shell): hoist MAX_SUMMARY_CHARS to module scope for PR body generation (#7731) * fix(web-shell): address review feedback for git branch picker (#7731) - Hoist onOpenCommit to useCallback to fix App.test.tsx prop stability test - Keep commit tab visible after navigating away (startedInCommit ref) - Add onClick handler to commit tab for navigation back to commit view - Fix branch-prefix strip mangling local branch names containing '/' - Update sessionIdRef after force-creating a stale session replacement - Pin core.hooksPath in test makeRepo for reliable rollback tests - Add test asserting --force-with-lease is used for force pushes * fix(web-shell): target the worktree for sidebar commits and harden git actions (#7731) Scope the sidebar commit dialog to the active session's worktree checkout (matching the composer path) so linked-worktree sessions commit to the right checkout, guard PR creation against a double-click race, and surface an error when an invalid branch name is submitted. Adds focused coverage for the branch picker action wiring and the git branch route validation paths. * fix(web-shell): address review feedback for git branch picker (#7731) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-07-28 14:25:11 +00:00
Shaojin Wen	6fb63b73e1	fix(review): recover the resolved effort when --effort is not re-threaded (#7855) ... * fix(review): recover the resolved effort when --effort is not re-threaded The capture commands (capture-local, plan-diff, fetch-pr) record the review effort as plan.effort, which every downstream consumer — roster, check-coverage, compose-review — reads. They learn it from --effort <level>, and the skill asks the orchestrator to pass the level parse-args resolved. But that is a value the model must copy from one file into a flag, and it does not reliably happen: a `/review --effort medium` local run had the orchestrator omit the flag, so plan.effort was absent and the roster safe-expanded to the FULL set — the user asked for the reduced medium roster and silently got every agent (6a/6b/6c included). Close the gap deterministically. A new resolveEffort() prefers an explicit --effort, and otherwise reads the level parse-args already wrote to its conventional report (.qwen/tmp/qwen-review-parse-args.json). When neither is available it returns undefined and the roster fail-safes to the full set exactly as before, so a missing report never reduces coverage, and a malformed level is ignored rather than trusted. * refactor(review): dedupe effort resolution per review feedback - Share one EFFORT_LEVELS set: export it from parse-args and import it in effort.ts, so a new level cannot be added to one set but not the other. - Collapse the three identical effort-spread IIFEs into planEffortField(). - Isolate CWD in plan-diff.test.ts (matching capture-local.test.ts) so resolveEffort's CWD-relative report read cannot pick up a stale file and fail the "omits effort" case. * test(review): pin PARSE_ARGS_REPORT value and plan-diff effort fallback (#7855) * test(review): dedupe seed helper, cover fetch-pr effort, trace resolution (#7855) * fix(review): spread planEffortField last for consistent effort precedence (#7855) --------- Co-authored-by: verify <verify@local> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>	2026-07-28 13:15:36 +00:00
qqqys	20f9f08ce5	feat(channels): expose loop tools in daemon sessions (#7891) ... * feat(channels): expose loop tools in daemon sessions * fix(serve): preserve fast-path bundle boundary --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com>	2026-07-28 13:07:36 +00:00
ZevGit	ad1b5f3de1	fix(cli): default to virtualized terminal history (#5738) ... * fix(cli): default to virtualized terminal history * fix(cli): remove redundant alternate screen exit handler * fix(cli): keep non-interactive output off VP mode * fix(cli): stabilize VP tests in CI environments * test(cli): resolve SDK daemon source in vitest * fix(cli): normalize CI env checks for VP mode * fix(cli): keep default VP mouse interactions enabled * fix(cli): align VP mouse behavior with runtime state * fix(cli): stabilize virtual viewport runtime state * test(cli): cover virtual viewport fallbacks * docs(cli): clarify virtual viewport requirements --------- 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-28 12:51:55 +00:00
VitaliBabkin	0afb48b1ea	fix(safe-mode): preserve caller-supplied top-tier MCP servers (#7827) ... * fix(safe-mode): preserve caller-supplied top-tier MCP servers Safe mode is meant to distrust LOCAL/ambient state (settings.json, extensions, project .mcp.json) so a user can isolate which local customization is misbehaving. It was also unconditionally dropping topTierMcpServers -- the caller-supplied servers from an ACP session/new's mcpServers field or --mcp-config -- which are an explicit, per-invocation argument, not ambient local state. The real gate turned out to be in packages/core/src/config/config.ts's Config.getMcpServers() (the accessor mcp-client-manager.ts actually reads for discovery), not just the mcpServers assembly in loadCliConfig -- fixed both so the raw field and the accessor agree. Also guarded the hot-reload path (hot-reload.ts) with the same bare/safe-mode check, so a live settings.json edit can't smuggle local servers into an already-running bare/safe-mode session. Fixes #7819 * docs: clarify safe-mode's MCP servers note distinguishes local vs caller-supplied Per CONTRIBUTING.md guideline 5 (docs for user-facing changes). * fix(safe-mode): apply allowedMcpServers to top-tier servers, skip pendingMcpServers I/O under safe mode Address Copilot's review of PR #7827: - getMcpServers() now runs the safe-mode top-tier map through the same allowedMcpServers filter as the non-safe-mode path -- safe mode is not an exemption from a session's own --allowed-mcp-server-names upper bound (High severity finding, confirmed real). - Re-added safeMode to pendingMcpServers' skip condition in loadCliConfig. Functionally a no-op either way (top-tier servers are never gated, #4615), but skips getMcpApprovals' local file read entirely under safe mode instead of doing a no-op read -- safe mode shouldn't touch local/ambient state at all, not even harmlessly (Medium severity finding). * fix(safe-mode): actually run MCP discovery for surviving top-tier servers Config.getMcpServers() reporting a top-tier server as configured isn't enough on its own -- something has to actually connect to it and register its tools with the model. That's a separate gate in initialize(): startMcpDiscoveryInBackground() was skipped outright whenever isSafeMode() was true, written back when getMcpServers() always returned {} under safe mode (so skipping discovery was a harmless no-op). Left unpatched after the earlier fix, a caller-supplied top-tier server survives getMcpServers() but is never actually discovered/connected -- confirmed live against a real ACP session before this commit: the agent reported the tool as "not configured" even though Config.getMcpServers() already returned it. Found by actually running the fix end-to-end against a live ACP session (qwen --acp --safe-mode + a real stdio MCP fixture server) instead of relying on unit tests of Config in isolation. After this commit the same live session correctly discovers and calls the caller-supplied tool. Checks getMcpServers() (not topTierMcpServers directly) so the allowedMcpServers filter still applies -- no discovery is kicked off if the only top-tier server present is filtered out. * fix(safe-mode): also run MCP discovery for surviving bare-mode top-tier servers The discovery-kickoff gate in Config.initialize() special-cased safe mode (skip only when there's nothing to discover) but left bare mode's half of the same guard unconditional (!this.getBareMode()), even though loadCliConfig feeds top-tier MCP servers into bare mode's mcpServers assembly exactly the way it does safe mode's topTierMcpServers field. A bare-mode session with a caller-supplied server (qwen --bare --mcp-config, or ACP session/new under bare mode) had that server reported as configured by getMcpServers() but never actually connected/discovered — the same stranded-server regression already fixed for safe mode in this PR, just the bare-mode twin of it. Found by an automated review pass on PR #7827 after the safe-mode fix had already landed. Added the same three-case regression coverage (present / nothing supplied / filtered out by allowedMcpServers) mirroring the existing safe-mode tests, using mcpServers (not topTierMcpServers) since bare mode's "local sources dropped" guarantee lives entirely in the CLI-layer assembly, not a core-level short-circuit. * refactor(safe-mode): simplify the bare/safe discovery-gate condition (!bare || has) && (!safe || has) reduces by distributivity to !(bare || safe) || has, so factor the has-servers check into a single hasMcpServers computed once, instead of calling getMcpServers() (which allocates and filters) twice. Suggested by an automated review pass on PR #7827, commit 25ddf8b. No behavior change — same 23 discovery-gate tests pass unmodified. * fix(safe-mode): stop reading settings.mcp.allowed/excluded under safe mode allowedMcpServers/excludedMcpServers assembly in loadCliConfig() only guarded the settings-sourced branch with `!bareMode`, missing `!safeMode` — so a local settings.json mcp.allowed/excluded list (LOCAL/ambient state, same category as settings.mcpServers itself, which safe mode already drops) was still read under safe mode. Combined with getMcpServers()'s own allowedMcpServers filter (added earlier in this PR for the --allowed-mcp-server-names case), a settings.json mcp.allowed list narrower than the caller's own top-tier servers would silently filter them back out — defeating the guarantee this PR exists to provide, via the filter's source rather than the mcpServers map directly. The argv.allowedMcpServerNames branch is unaffected: that's an explicit per-invocation argument, not local state, so it still applies under safe mode same as topTierMcpServers itself. Found by an automated review pass (doudouOUC, CHANGES_REQUESTED) on PR #7827. Regression test confirmed red before the fix (session-supplied server silently filtered out) and green after. Full targeted vitest run (packages/cli config/: 1018 passed, same 3 pre-existing Windows-only extension-file-watcher failures as before, unrelated), tsc --noEmit, eslint, prettier --check all clean. * fix(safe-mode): stop reading settings.mcp.allowed/excluded on hot-reload too Same class of bug as the previous commit's loadCliConfig fix, found by an automated review pass on the SAME PR: recomputeMcpGating (hot-reload.ts) reads settings.merged.mcp.allowed/excluded unconditionally, with no bare/safe guard of its own. registerMcpHotReload's existing bare/safe guard only covered the servers map (`next`), not the admission lists computed right after it — so a live settings.json edit narrowing mcp.allowed during an already-running safe/bare session would flow straight into setAllowedMcpServers and silently filter the caller's top-tier server out of getMcpServers() mid-session. Same stranded-server outcome as the boot-time bug, reached through the gating list's SOURCE instead of the mcpServers map. Fix: under bare/safe mode, skip recomputeMcpGating entirely and build the gating directly from only the CLI --allowed-mcp-server-names bound (explicit, per-invocation, not local state — same treatment as topTierMcpServers itself); excluded/pending are irrelevant once nothing but the never-gated top-tier servers can be present. Regression tests (safe mode + bare mode) confirmed red before the fix (setAllowedMcpServers called with the settings-sourced list) and green after (called with the CLI bound, undefined here). Full targeted vitest run (packages/cli config/: 1020 passed, same 3 pre-existing Windows-only extension-file-watcher failures as before this PR touched anything, unrelated), tsc --noEmit, eslint, prettier --check all clean. * fix(safe-mode): stop reading settings.mcp.allowed/excluded on ACP reload too Third instance of the same bug class found by an automated review pass on this PR: reloadWorkspaceMcpDiscovery (packages/cli/src/acp-integration/ acpAgent.ts) — the ACP control-endpoint reload path (workspaceMcpReload), distinct from registerMcpHotReload's settings-file-watcher path fixed in the previous commit — called assembleMcpServers(settings.merged.mcpServers, ...) and recomputeMcpGating(settings, ...) unconditionally, per live Config in liveConfigs, with no bare/safe guard. A workspaceMcpReload request against an already-running safe/bare session would fold local mcpServers/mcp.allowed/excluded back in, silently stranding or filtering the caller's own top-tier server mid-session — same outcome as the two prior fixes, reached through a third independent reload path. Fix: per-config (liveConfigs holds a Set of potentially differently-moded Configs — the base config, active session configs, and the discovery config), skip assembleMcpServers/recomputeMcpGating under bare/safe mode and build servers/gating directly from that config's own getTopTierMcpServers()/getCliAllowedMcpServerNames() — same treatment as the other two fixes. Regression test (packages/cli/src/acp-integration/acpAgent.test.ts) confirmed red before the fix (settings-sourced 'local' server leaked into reinitializeMcpServers alongside the caller's 'probe') and green after. Also added getBareMode/isSafeMode mocks (defaulting false) to the two pre-existing Config-shaped mocks in this describe block that didn't have them — reloadWorkspaceMcpDiscovery now calls these unconditionally per config, which would otherwise throw "not a function" against any mock missing them, even for a normal-mode test. Full targeted vitest run (packages/cli/src/acp-integration/acpAgent.test.ts: 318 passed), tsc --noEmit, eslint, prettier --check all clean. * test(safe-mode): add bare-mode counterpart for the workspaceMcpReload guard Suggested by an automated review pass on PR #7827: the previous commit's regression test for reloadWorkspaceMcpDiscovery only exercised isSafeMode: true, leaving the bare-mode half of config.getBareMode() || config.isSafeMode() unverified at this layer — unlike the hot-reload.ts tests, which already cover both modes for both the servers map and the admission lists. A future change narrowing that guard to isSafeMode() only would go undetected here. Confirmed red before the fix (temporarily reverted acpAgent.ts to the prior commit): settings-sourced 'local' leaked into reinitializeMcpServers alongside the caller's 'probe', same as the safe-mode case. Green with the fix restored. Full targeted vitest run (acp-integration/acpAgent.test.ts: 319 passed), tsc --noEmit, eslint, prettier --check all clean.	2026-07-28 11:29:53 +00:00
jinye	788e5cd3a8	feat(core): add ARMS session user ID (#7921) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-28 11:10:38 +00:00
qwen-code-dev-bot	f7fd831003	fix(cli): remove redundant 'Read file' prefix from @mention tool card (#7902) ... * fix(cli): remove redundant "Read file" prefix from @mention tool card The @mention file-read tool card set its description to "Read file <name>", which duplicated the display name ("Read") when rendered as "{displayName} {description}", producing "Read Read file README.md". Change the description to "@<name>" — matching the @mention syntax the user typed, and consistent with how normal read_file tool calls show just the filename in their description. * test(cli): assert @mention tool card description format (#7902) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com>	2026-07-28 11:00:12 +00:00
qwen-code-dev-bot	0be42b782d	fix(cli): add polling fallback for git branch name display (#7830) ... * fix(cli): add polling fallback for git branch name display (#7828) fs.watch on .git/logs/HEAD is unreliable on NFS, FUSE, Docker overlay, and some Linux filesystems — events can be silently dropped with no recovery path, leaving the footer branch name stale indefinitely. Added a 5-second polling fallback in useGitBranchName that calls resolveBranchName and updates the state only when the value changes. The timer is unref'd so it doesn't keep the process alive. The existing fs.watch mechanism is preserved for immediate updates when it works correctly. * test(cli): cover the git branch polling fallback (#7830) * refactor(cli): use idiomatic timer.unref?.() in branch poller (#7830) * fix(cli): order concurrent branch refreshes by generation (#7830) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-28 10:58:31 +00:00
jinye	4703cc5432	fix(serve): Release managed session writer locks on shutdown (#7812) ... * fix(serve): release managed writer locks on shutdown Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): address shutdown review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): release writer locks after flush failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(serve): harden managed shutdown recovery 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-28 10:18:35 +00:00
ChiGao	6f739c7a46	fix(cli): hide stale sticky todos from previous turns (#7900) ... * fix(cli): hide sticky todo panel when a new turn starts PR #7062 hid the sticky panel when streamingState is Idle, but the panel resurfaces with stale todos from a previous turn as soon as the user sends a new message (state → Responding). This is confusing — the user sees in-progress indicators for work that already finished in an earlier turn. Add a turn-boundary check in getStickyTodos: if a user message exists after the todo snapshot in history, the snapshot belongs to a previous turn and the sticky panel returns null. Fixes #7061 * test(cli): pin sticky-todo turn boundary for local slash commands (#7061) --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>	2026-07-28 10:12:14 +00:00
顾盼	9461aa860d	fix(core): bridge tool-result images for text-only models (#7484) ... * fix(core): bridge tool-result images for text-only models * test(vision-bridge): pin tool-result full-turn guards and surface bridge errors (#7484) * test(cli): cover drain-item model override conflict rejection (#7484) * test(cli): cover stop-hook full-turn model persistence * fix(core): disclose tool image routing * fix(cli): type restored vision notices * test(cli): fix stop-hook vision fixture --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-07-28 08:47:40 +00:00
jinye	b3873571aa	fix(daemon): harden Todo Stop Guard continuations (#7821) ... * fix(daemon): harden Todo Stop Guard continuations Linearize Guard continuation ownership across bridge consumers, preserve queued input across failure paths, and tighten session lifecycle and lineage handling. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(daemon): yield to event loop in waitForActiveTurnsToSettle (#7821) * fix(channels): discard retired ACP sessions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): preserve superseded continuation state Preserve unsent continuation results across prompt supersession and cancellation, and keep closing sessions from restoring stale FIFO priority. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-07-28 06:51:40 +00:00
qqqys	090b213ad8	feat(web-shell): add native workspace folder picker (#7849) ... * feat(web-shell): add native workspace folder picker * fix(serve): harden native directory picker and add coverage (#7849) Add a 5-minute timeout to each native picker subprocess and to the webui action so a dismissed dialog cannot leave an orphaned GUI process, and distinguish a headless Linux "cannot open display" failure from a deliberate zenity cancellation. Cover pickNativeDirectory's platform branches, the route's 501/500 error paths, and the dialog's picker failure path with focused tests. * fix(serve): log directory picker failures to daemon stderr (#7849) * fix(serve): abort directory picker on client disconnect and stagger timeouts (#7849) * test(webui): add unit tests for pickWorkspaceDirectory action (#7849) * fix(serve): set UTF-8 console encoding for PowerShell picker and treat timeout kills as cancels (#7849) * fix(serve): abort directory picker on response close, not request close (#7849) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code CI <qwen-code-ci@users.noreply.github.com>	2026-07-28 05:58:13 +00:00
chinesepowered	a8ad0bcbd9	fix(cli): make /copy <message> <index> select the code block (#7883) ... selectCodeBlock seeded `lang` from the first token before the classification loop ran. For a bare number that is wrong: `/copy 1 2` strips the leading 1 as the message index and hands "2" over as the whole selector, so lang became "2" while the loop set requestedIndex to 2. The filter then looked for blocks written in a language called "2", found none, and reported that no code block matched. The argument hint advertises `[N] [<lang>|code|latex|mermaid] [<index>]` with every group optional, so this is a documented form. The seeding is redundant as well as wrong: the loop already assigns lang for any token that is neither a number nor `code`. Same defect and same fix as #7789 in packages/web-shell; @wenshao verified the CLI copy carried it and suggested the follow-up. Filed separately rather than folded into that PR.	2026-07-28 05:42:25 +00:00
chinesepowered	6add598826	fix(cli): do not count a partial trailing line when re-opening a split fence (#7875) ... countHeadContentLines counted the newlines in the committed head, and when the head did not end in a newline it returned the raw count -- treating the unfinished last line as complete. The tail's start-line directive is computed from that count, so every line number after a mid-line split was one too high. splitFencedMarkdown('```js\naaa\nbbb\nccc\n```', 12) head: '```js\naaa\nbb\n```\n' tail: '```js qwen-code:start-line=3\nb\nccc\n```' The tail's first row is `b`, the remainder of source line 2, but the directive labelled it 3. A completed line and a partial one leave the same number of newlines behind, so neither needs a special case: the count is always one less than the newlines in the head, floored at zero. Splits landing on a line boundary are unchanged. An existing test asserted `start-line=2` for a block holding one 100-char line split at 40. The tail there is the remainder of line 1, so it is updated to 1.	2026-07-28 05:42:10 +00:00
chinesepowered	963142ab02	fix(cli): make wrapToVisualLines count zero-width characters like its sibling (#7873) ... wrapToVisualLines and sliceTextByVisualHeight both measure visual rows at a given width, but only sliceTextByVisualHeight clamps the per-character width with Math.max(..., 1). `string-width` returns 0 for TAB, ZWJ and combining marks, so a run of them was charged nothing and collapsed into a single row: wrapToVisualLines('\t'.repeat(50), 10).length -> 1 sliceTextByVisualHeight('\t'.repeat(50), 3, 10) -> 5 rows Same input, same width, two answers. Callers that mix the two -- scroll offsets, pending-render height -- then disagree with themselves. Apply the same clamp. Erring high is the safe direction for a terminal: reserving one row too many costs a blank line, while counting one too few overflows the region and pushes content off screen. This aligns on the convention sliceTextByVisualHeight already follows, including its treatment of combining marks, which both now charge a column. Making that more precise would mean changing both functions and is left alone here.	2026-07-28 05:41:20 +00:00
chinesepowered	3836e7e38c	fix(cli): report a genuine $0.00 cost instead of N/A (#7784) ... calculateCost ended with 'total > 0 ? total : null', and callers read null as "no cost to report" - the stats table prints N/A and hides the Cost section entirely when no model yields a number. Testing the total conflated a zero cost with an unknown one. A model the user deliberately priced at 0 (a free OpenRouter variant, a local Ollama or LM Studio model) was reported as N/A, and the Cost section vanished when it was the only model in the session. Decide on the pricing and the usage before the arithmetic, then return the total unconditionally. The no-usage case still returns null, which statsCommand.test.ts documents as deliberate.	2026-07-28 05:34:36 +00:00
jinye	b475d1a263	feat: Gate session writer lease behind opt-in (#7894) ... * feat: gate session writer lease behind opt-in Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(acp): freeze session writer lease per process Snapshot the effective restart-required lease gate from the bootstrap Config and reuse it for every session Config in the ACP process. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): align recorder default lease gate Use the effective session writer lease gate when ChatRecordingService is constructed without an explicit writer mode. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-28 04:37:02 +00:00
qqqys	bd2c0b47f1	feat(core): add full-resolution image zoom tool (#7809) ... * feat(core): add full-resolution image zoom tool * chore(vscode): refresh third-party notices * fix(ui): satisfy zoom image display drift checks * fix(web-shell): translate zoom image tool * fix(core): wire sharp into packaging and load it lazily (#7809) Externalize sharp in esbuild and declare it (plus the @img platform binaries) in the published package's optionalDependencies so an npm-installed CLI resolves the native binding. Import sharp dynamically inside execute() so a missing binding returns a bounded tool error instead of crashing startup during strict tool warmup, and so the module is only loaded when zoom_image actually runs. Also pin the EXIF auto-orientation test with a discriminating fixture and cover the .qwenignore and y1>=y2 validation paths. * fix(core): gate zoom_image at execute time and cap tiny-crop upscale (#7809) Register zoom_image unconditionally and move the image-modality check to execute time so first-run sessions and hot /model switches resolve the tool without re-running initialize(). Cap magnification at 8x so a tiny crop no longer inflates to the full image-token budget. Drop the redundant @img/* platform pins; sharp's own optionalDependencies install the matching binary for each OS/arch. * fix(core): emit zoom_image file telemetry and cover guard branches (#7809) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-27 23:40:26 +00:00
qqqys	6930e72970	feat(core): persist and replay Goal v3 state (#7815) ... * feat(core): persist and replay Goal v3 state * test(goal): pin replay seeding, validation, and rewind boundaries (#7815) * fix(cli): remove dead goalState param, test malformed-state guards (#7815) * test(core): cover recordToolResult provenance override path (#7815) --------- Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-07-27 23:39:46 +00:00
qqqys	436271067c	feat(acp): add session-scoped runtime MCP (#7847) ... * feat(acp): add session-scoped runtime MCP * fix(acp): allow slow session MCP mutations * test(acp): await async MCP assertions * fix(acp): allow MCP discovery during session spawn	2026-07-27 23:37:56 +00:00
jinye	9faa8068c3	perf(acp): preload providers after session creation (#7767) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-27 16:47:48 +00:00
jinye	17408f1028	feat(hooks): Add submitted prompt provenance (#7762) ... * feat(hooks): add submitted prompt provenance Add an optional pre-expansion prompt sidecar for interactive UserQuery hooks while preserving legacy prompt behavior and fail-closed provenance handling across queues, retries, and continuations. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(hooks): harden submitted prompt provenance Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(hooks): tighten submitted prompt provenance Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-27 15:53:15 +00:00
ytahdn	8785216be5	feat(web-shell): add monitor task details (#7817) ... * feat(web-shell): add monitor task details * fix(web-shell): align monitor tab title with merged snapshot and reset expansion (#7817) --------- Co-authored-by: ytahdn <ytahdn@gmail.com>	2026-07-27 12:30:17 +00:00
ytahdn	8a44b1b9f7	fix(web-shell): render task notifications as system messages (#7822) ... * fix(web-shell): render background notifications as system messages * fix(web-shell): use basic table rendering by default --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com>	2026-07-27 09:15:18 +00:00
Shaojin Wen	f5248acf21	feat(review): script-lint as a deterministic gate — compose-review reads the report, no agent (#7751) ... * feat(review): add script-lint — deterministic linters over a diff's executable scripts A diff's shell — a `.sh` file, a Makefile recipe, a Dockerfile `RUN`, a GitHub Actions `run:` block — is code, and its bugs (an unquoted `$x` that word-splits, a `${PIPESTATUS[1]}` read after the array was reset) are the class a reviewer misses by reading a long YAML and catches by running the checker. Measured: a model told in prose to "run the workflow scripts" reads instead (0/4 executed). So the execution is a command, not a request. `qwen review script-lint` reads the plan, dispatches shellcheck / actionlint / hadolint by file type over the changed executable files, filters every finding to whether its line is one the diff changed (`inDiff`), and reports JSON. A linter that is not installed is disclosed as skipped, never a clean bill. It is not GitHub-specific — shellcheck applies to shell wherever it appears; actionlint/hadolint are front-ends for two embeds. This is the command only; the agent, roster requirement and coverage gate that make it a non-skippable step follow. * feat(review): make script-lint a required, coverage-gated review step The script-lint command exists; nothing ran it. This wires it into the review the way build-test is wired, so a diff that changes an executable script cannot be certified without its linters having run. - A `script-lint` agent role (agent-briefs): reads no diff, runs the command, reports from its JSON. `inDiff` findings are the PR's; `skipped` (a linter not installed) is disclosed as unreviewed, never clean. Rules are not injected into it, same as Build & Test — it reports a tool's verdict, not a read. - agent-prompt welds the exact `qwen review script-lint --plan/--worktree/--out` into that agent's brief with absolute paths, guarding an absent PR number out of the --out name — the same treatment, and the same traps avoided, as the build-test block it sits beside. - The roster requires the agent whenever the diff carries a file a linter owns by path (a `.sh`/`.bash`, a `.github/workflows/*`, a Dockerfile) and the review has a worktree to lint in. Detected by the command's own `pathTool`, so the roster and the command cannot disagree about what counts. A pure-TS diff does not require it; a diff-only review (no tree) cannot run it, so does not. - Coverage needs no new code: it derives missing roles from the roster generically (`BRIEFS[role].label`), so a required script-lint agent that did not run exit-3s check-coverage like any other. The role carries its three labels for that. End-to-end on a crafted diff (a workflow plus a `deploy.sh` with `rm -rf $TARGET`): the roster requires script-lint, and the command blocks on the SC2086 on the changed line while disclosing the workflow as skipped where actionlint is absent. SKILL.md documents the step; tests cover the roster requirement, the brief weld, and the subcommand registration. * fix(review): harden script-lint after review — context lines, fail-closed, symlinks, quoting Addresses the review findings on this PR: - inDiff was keyed off the plan's hunk ranges, which include git's three context lines, so a pre-existing diagnostic near a real change was marked this PR's and could block it. Classify off the diff's added-line ranges (context excluded), parsed from the diff; fall back to the plan hunks only when the diff is absent. - The command wrote the report to --out and printed only "Wrote ...", while the agent's brief (and the roster's generated command, which passes --out) says to read the JSON it prints. Match build-test: write the file AND always print JSON. - runTool failed open — every non-ENOENT failure (EACCES, a signal, maxBuffer, an unexpected status) fell through as empty stdout and became ok:true. Fail closed: such a run is `errored`, which forces ok:false; ENOENT alone stays "not installed". - The shebang read slurped the whole file and followed symlinks — a changed `hang.sh` -> /dev/zero would hang the reviewer. Read only regular files (lstat, no follow) and only the first block. - The welded command interpolated plan/worktree/out as bare words; a worktree path with a space would split. Quote them with shellQuotePath. - Harden the checker environment: shellcheck --norc + drop SHELLCHECK_OPTS, so a PR-controlled .shellcheckrc or inherited opts cannot suppress SC2086. - The roster required the agent for a pure-deletion .sh (a mandatory no-op); gate on added lines. Drop the Makefile-recipe claim (no detector backs it). Fix the `ok` JSDoc (info blocks too, not just error/warning). - Tests: inject the tool runner (no binary needed) to cover actionlint/hadolint normalisation, the three fail-closed paths, and the context-line classification. * feat(review): make script-lint a deterministic gate — orchestrator runs it, compose-review is the authority The review of #7749 landed three architectural findings (#10/#11/#12): the executable-script lint was run by an AGENT, so its execution rested on the model's honor system, the model decided each finding's severity, and an uninstalled checker was only disclosed in prose. All three are the exact "a rule a model is asked to remember will eventually not be remembered" trap the feature exists to close — and a measurement on PR #7724 confirmed it: the strongest model's Step-3 agents, given the diff and the worktree, missed all three execution-confirmed bugs, one attacker persona walking into a double-execute and declaring it correct. So take the model out of the gate entirely: - The orchestrator runs `qwen review script-lint` as a deterministic step (like presubmit) and writes the report next to the plan. No agent. - `compose-review` derives the report path from the plan and reads it as the SOLE authority: a finding on a changed line above `style` is a pre-confirmed `[lint]` Critical (needs no verifier — the tool already ran); an uninstalled or crashed checker is unreviewed scope that caps a would-be Approve; and — proof it ran — a diff that carries an executable script but produced no readable report is itself unreviewed (fail closed). A diff-only review, which has no worktree to run it, is exempt. Nothing here comes from the input JSON a model wrote. Removed the `script-lint` agent role (agent-briefs, agent-prompt weld, roster requirement) and its SKILL.md agent entry; the roster's `hasExecutableScript` is now exported as the shared predicate the gate reads. The `script-lint` command itself is unchanged — it was always the deterministic engine; only its trigger and consumer moved. * fix(review): address the second review pass on script-lint - parseFindings failed OPEN on non-empty unparseable output (a version skew or a deprecation line before the JSON) → `[]` → recorded as a clean `checked` file. Return null on that path and push it to `errored`, fail-closed like a bad exit. - hadolint had no config isolation while shellcheck gets `--norc`; a PR-controlled `.hadolint.yaml` could `ignored:` its findings away. Add `--no-config`. - pathTool recognised only `.sh`/`.bash`, but the shebang regex matches four shells — a `.ksh`/`.dash` file never required the lint. Add those extensions so the gate's owed-predicate and the command cannot disagree. - Pin `r.ok` in the not-installed test, and add a mock test for the unparseable path. * fix(review): address the deterministic-gate re-review — real correctness bugs on the write path The second review pass on the gate found real bugs, including one this PR introduced. Fixed: - hadolint has no `--no-config` flag — a prior round added it, so hadolint exited 2 (usage error) on EVERY Dockerfile and reported it `errored`. Reverted to the plain working invocation; hadolint config isolation needs a verified mechanism and is tracked separately. - actionlint's JSON anchors each diagnostic at the `run:` key line (not the changed shell line) and flattens ShellCheck severity, so a style nit read as a blocking `error` and a real finding read as pre-existing. Until that source mapping is parsed and verified, a workflow is deferred to `skipped` (unreviewed) and actionlint is never run — shellcheck still covers standalone shell. - A recognised path that `firstLineOf` refused (a `hook.sh` symlink, a fifo) was silently dropped from the report, so an empty report read as clean. It is now recorded as `skipped`. `firstLineOf` distinguishes a true deletion (skip) from an irregular file (record). - The gate's owed-predicate excluded any file with zero added lines, so a deletion-only edit that breaks a surviving `.sh` was treated as not-owed. Keyed on the post-image (`fileLines`) now: a surviving script is owed, only a true deletion is exempt. - compose-review trusted any body-Critical string containing `[lint]` as deterministic — a model-written or injected claim could launder itself past verification. Provenance decides now: only `scriptLintGate`'s own findings are deterministic (tracked by count); `[lint]` is no longer a trusted tag. - A stale report from an earlier review of an older commit could certify the current one if the lint step were skipped. The report records its `headSha` (`git rev-parse HEAD`); compose-review rejects it as stale when it disagrees with the plan's `fetchedSha`. * fix(review): third gate re-review — read the report before the owed-predicate, fail closed on all paths - The gate returned early on `hasExecutableScript` (path-only), so a shebang script the command DID lint (`.husky/pre-commit`, detected by `#!`) had its findings dropped — the gate never read the report. Read the report first; the path-predicate now only gates the no-report fail-closed case. Findings from any script the report names are processed. - The staleness guard was a no-op when `report.headSha` was absent (git unreadable): `planSha && report.headSha && …` short-circuited, so an unverifiable report certified new code. It now fails closed on a missing headSha when the plan names a commit. - The plan-parse catch failed OPEN (returned "nothing owed") while every other path fails closed. It now discloses "could not read the plan" as unreviewed. - hadolint had no config isolation. Point `HADOLINT_CONFIG` at an empty file so a PR-added `.hadolint.yaml` cannot suppress findings — an env var, benign if the tool ignores it (unlike the invalid `--no-config` flag from the last round). - `buildNote` hardcoded "not installed" for every skipped file; `skipped` now mixes reasons (missing tool, irregular file, deferred checker), so it summarises by tool and leaves the specific reason on each entry. * fix(review): R3 — deferred (non-capping) actionlint, local-flow staleness, disclosure - actionlint deferral capped EVERY workflow-touching PR (≈15% of merges): a deferred workflow went to `skipped` → unreviewed → no Approve, on a checker present but deliberately not run, which "install the tool" cannot fix. Split a third `deferred` state — disclosed in the note, but the gate never reads it, so it does not cap. `skipped`/`errored` still cap. - The staleness guard was PR-only: `fetchedSha` is written by fetch-pr, not capture-local, so a local review short-circuited the check and a stale local report could certify new code — the exact fail-open, in the local flow. capture-local now records the local HEAD as `fetchedSha`. - The local path was armed (reviewMode `local`) but SKILL's `--worktree` was unfillable there. SKILL now covers a local review (worktree = the project root) and the derived report name. - Nits: the skipped/irregular reasons no longer lead with the path (the gate prefixes it — was printing it twice); merged the two `./lib/roster.js` imports. - Tests: deferred does not cap; the staleness guard protects a local review too. * fix(review): R4 — disclose deferred lint without capping; bind report freshness to diff content B1 — the deferred checker was silent. R3 split a third `deferred` state so a workflow's embedded shell (actionlint, whose source-mapping this env can't verify) no longer caps the verdict — but the disclosure half was never wired: `scriptLintGate` returned only `{criticals, unreviewed}`, so a workflow-only PR went quiet on a file no checker examined. That is #10's original complaint ("only disclosed in prose") returning as disclosed nowhere. Add a third channel: the gate now returns `disclosed`, populated from `report.deferred`, and `composeReview` renders it in the body on every verdict — including Approve — without pushing it into `cappedBy`. SKILL.md's contract paragraph now documents the deferred state alongside unreviewed scope. B2 — the staleness guard keyed on HEAD. A local review is defined by uncommitted work, so `git rev-parse HEAD` is not a content identity for what it reviews: two reviews at the same HEAD with different working-tree content shared a `fetchedSha` and a stale clean report could certify broken code. Bind freshness to the diff's content instead: `runScriptLint` stamps the report with a sha256 of the captured diff (`diffHashOf`), and the gate re-hashes the plan's current diff and rejects a mismatch. Correct for a PR (a later commit → a different diff) and for local uncommitted work alike. `capture-local`'s `fetchedSha` plumbing is removed. Tests: the DEFERRED-only case now asserts the disclosure is present and non-capping (it previously pinned the silence). New composeReview gate tests kill the surviving mutants — the gate critical stands with no verifier (provenance, not the `[lint]` marker), and an errored checker caps a would-be Approve to Comment. New runScriptLint tests pin the diff-hash stamp. 972 review tests, ESLint --max-warnings 0, tsc, Prettier clean. * fix(review): R5 — freshness fails closed when neither side has a hash; pin config isolation The staleness guard was `report.diffHash !== planDiffHash`. When the plan names no readable diff AND the report carries no hash, both are `undefined` and `undefined !== undefined` is false — so the guard did not fire and an arbitrary hashless report was accepted as this review's, its findings promoted to `[lint]` Criticals. Every other branch of this gate fails closed; this one inverted, on the exact unverifiable case its own comment claimed to reject. Fixed with `!planDiffHash || …`, and the stale `headShaOf` JSDoc left stacked above `diffHashOf` is removed. The gate's happy-path tests were green *because* of that hole: `writePlan`/ `writeReport` (and the composeReview `gateReadyPlan`/`writeGateReport`) wrote no diff and no hash, so every test that didn't override them ran through the fail-open branch — proving the gate works on an *unverifiable* report, not a fresh one. The fixtures are now FRESH by default: a captured diff exists (`DIFF` is a real file; coverage only string-matches it, so this is transparent to it) and both plan and report bind to its hash. A freshness test overrides one side to model staleness, and a new test pins the both-undefined case directly. Also closes the last mutation gap R5 flagged three rounds running: the config isolation (`--norc`, `SHELLCHECK_OPTS` scrub, `HADOLINT_CONFIG` → empty file) is a security property — a PR-added linter config must not suppress the finding the gate blocks on — but it lived inside `runTool`, behind the spawn an injected runner bypasses. Extracted `buildToolInvocation` (pure argv + env) so all three defences are asserted without a binary; each was hand-mutated to confirm its test fails when the defence is deleted. 976 review tests, ESLint --max-warnings 0, tsc, Prettier clean. * test(review): pin the plan-parse disclosure — the last surviving gate mutant `scriptLintGate` on an unreadable plan fails closed and pushes its own reason into `unreviewed`. The coverage machinery already caps an unreadable plan, so the verdict and its cap are identical with or without this line — what it loses when deleted is the specific "could not read the plan" sentence in the body. That made it a disclosure guarantee with no test, and the one gate mutant left standing after five rounds. Pin it directly on the gate. 977 review tests, ESLint/tsc/Prettier clean. * fix(review): harden script-lint tmp-file + spawn, leak-proof the tests (R8 suggestions) Four suggestions from the re-review, all on script-lint: - Symlink race on the hadolint empty-config: it was written to a fixed `tmpdir()/qwen-review-hadolint-empty.yaml`, so on a shared runner a pre-planted symlink there would have `writeFileSync` follow it and truncate the target. Write it inside a fresh `mkdtempSync` directory instead — a 0700 dir with a random suffix that cannot pre-exist, so the write is safe and the path unpredictable. - `runTool`'s `spawnSync` had no `timeout`, unlike the sibling runners in build-test.ts and test-efficacy.ts: a crafted script that hangs a linter would block the review until the outer CI job timeout reclaimed the runner. Added a 120s bound; a timeout kills with SIGTERM, which the existing `r.signal` branch already turns into a fail-closed error. - Two test-cleanup leaks: script-lint.test.ts's symlink test made a second temp dir cleaned only by an inline `rmSync` a failing assertion would skip, and script-lint.mock.test.ts used inline `fresh()`/`clean()` with no hook. Both now tear down in `afterEach`, which runs even when a test throws. 977 review tests, ESLint --max-warnings 0, tsc, Prettier clean. * fix(review): hadolint isolation fails closed, not to a plantable path (R8) R8 caught that the previous commit's fallback reopened the vector it closed. When `mkdtempSync` failed, `emptyHadolintConfig` returned a FIXED `tmpdir()/qwen-review-hadolint-none/x`. That path is a config hadolint *reads*, so an attacker who plants an `ignored:` file there gets those ignores honoured — the opposite threat direction from the write-truncation the mkdtemp move fixed. There is now no predictable fallback: on failure `emptyHadolintConfig` returns `undefined`, the env var is set only for a hadolint run and only when a private config exists, and `runTool` fails the hadolint run CLOSED (errored) rather than lint against a config it cannot vouch for. Also from R8: - The timeout comment claimed the `r.signal` branch handles a timeout. On Node a timeout sets BOTH `r.error` (ETIMEDOUT) and `r.signal` (SIGTERM), and `r.error` is checked first — so it is reported through the error branch. Comment corrected; still fail-closed either way. - Both hardening changes were untested. `buildToolInvocation` now returns `timeoutMs` so the bound is one asserted value, and two tests pin it: HADOLINT_CONFIG is a fresh 0700 mkdtemp path (not the old fixed name), and the timeout is 120s. Each fails under the corresponding mutation. - The mkdtemp dir is swept at process exit, so the fixed-file-to-per-run-dir change does not leak into the OS tmpdir. 979 review tests, ESLint --max-warnings 0, tsc, Prettier clean. * test(review): pin the hadolint fail-closed guard in its own file (R9) The `if (tool === 'hadolint' && !emptyHadolintConfig())` guard is what the previous commit exists to add, yet deleting it left the suite green: `emptyHadolintConfig` caches at module scope, so by the time any test in script-lint.mock.test.ts runs the cache is warm and the failure path is unreachable from that file. A dedicated file gets a fresh module registry — it points TMPDIR at a path that does not exist before importing, so the first `emptyHadolintConfig()` call fails to mkdtemp and the guard fires. Two tests, split so a mutation attributes cleanly: (1) buildToolInvocation sets no HADOLINT_CONFIG (the env logic, holds either way); (2) runScriptLint errors a Dockerfile closed rather than lint it unisolated (the guard). Deleting the guard reddens (2) and leaves (1) green. Also from R9: register the process-exit cleanup right after mkdtempSync, before the write, so a writeFileSync that throws still leaves the temp dir swept, not leaked. 981 review tests, ESLint --max-warnings 0, tsc, Prettier clean. * fix(review): isolate hadolint via --config, not the ignored HADOLINT_CONFIG env (R10) Verified against real hadolint 2.14.0: the binary does not read `HADOLINT_CONFIG` (its `strings` list it among no `HADOLINT_*` config vars; `-V` reports "No configuration was specified"). It reads `--config`, then a `.hadolint.yaml` in the process CWD, then XDG. Because a local review runs with `--worktree .`, the linter ran inside the reviewed tree and honoured the diff's OWN `.hadolint.yaml` — so the env-based "isolation" was a silent no-op and a PR could suppress its own Dockerfile findings (a DL3018 `ignored:` made the finding vanish with nothing disclosed). Isolate on the channel hadolint actually reads: pass `--config <private neutral file>`, which overrides both the cwd and XDG configs. The config content is now `ignored: []` rather than empty — `--config` rejects an empty file ("empty YAML stream"). The mkdtemp 0700 dir and the fail-closed guard are kept (the config is a path hadolint reads, so both still matter); the no-op `HADOLINT_CONFIG` / `HADOLINT_NO_COLOR` env vars are dropped. Tests updated to assert the real channel: hadolint's argv carries `--config` at a fresh 0700 mkdtemp path holding `ignored: []`, and the isolation-unavailable path adds no `--config` (and still fails closed). shellcheck's `--norc` / `SHELLCHECK_OPTS` isolation is unchanged — it was verified sound against the real binary. 981 review tests, ESLint/tsc/Prettier clean. * fix(review): address maintainer review — worktree containment, hunk-fallback false positive, zsh/shebang docs From @yiliang114's review of the script-lint gate: - P1 shebang/zsh: `toolFor` intentionally omits zsh (and fish) — shellcheck refuses both (`SC1071: ShellCheck only supports sh/bash/dash/ksh`), so routing a `#!/usr/bin/env zsh` hook to it would make every zsh file a bogus SC1071 `[lint]` Critical on its shebang. Documented the deliberate exclusion rather than adding zsh. - P1 hunk fallback: `inDiff` fell back to the plan's context-inclusive hunks whenever `addedRanges.get(path)` missed — including when the diff parsed fine but a path was unmatched, which could promote a pre-existing finding on a context line to a blocker. Now a parsed diff that does not mention a path yields `[]` (nothing added → nothing blocks); the context-inclusive `hunksOf` fallback is used only when NO diff parsed at all (a report the freshness guard already rejects as stale), and that degraded path now logs a warning. - P2 worktree containment: `resolve(join(worktree, path))` now refuses a path that escapes the worktree (`../../etc/passwd`), disclosing it as skipped rather than stat-ing/linting a file outside the reviewed tree. - P2 roster/gate shebang gap: made explicit in the gate's no-report branch that a shebang-only script (which `hasExecutableScript` cannot see) is covered by the always-run contract, not the `owed` predicate. Two items left as tracked follow-ups per the reviewer's own framing (both flagged low-priority / design): an aggregate cross-file time budget (the per-file 120s bound is the main defense), and per-severity finding classification (the v1 all-or-nothing stance is documented and defensible). 983 review tests, ESLint/tsc/Prettier clean. * fix(review): harden the script-lint gate against the adversarial review round From the Codex GPT-5 /review pass. Five fixed: - Provenance by IDENTITY, not count (compose-review): the gate's `[lint]` criticals are now tracked as a separate list rather than pushed into `bodyCriticals` and removed by a count subtraction. The old `(filtered) − gateCount` misfired when a model claim carried a `[build]/[test]/[probe]` tag (filtered out before the subtract) or a gate finding's own text contained one — erasing an unrelated claim's verification requirement. Model criticals alone decide the verify count. - Distinguish unknown size from deletion (roster): `fileLines: 0` is a real post-image count only in pr-worktree; in local/diff-only the report builder writes 0 for EVERY file, so keying deletion on it read a surviving `deploy.sh` as deleted and let a missing report pass uncapped. `hasExecutableScript` now trusts `fileLines` only in pr-worktree and owes any path-detected script otherwise. - Hash the bytes used for range mapping (script-lint): the diff was read twice — ranges before the linters, hash after — a TOCTOU window where a concurrent recapture could pair snapshot A's ranges with snapshot B's hash. Read the diff once into one buffer and derive both from it; an unparseable diff now yields no `diffHash` (gate fails closed) instead of context-inclusive hunks. - Escape PR-controlled paths in the body (compose-review): a diff filename can carry a newline, `@mention`, HTML or Markdown; the gate's criticals/unreviewed/disclosed strings rendered it verbatim into the posted body. Paths and linter messages now render in an inline code span with backticks and newlines stripped (`mdField`). - Classify lstat failures (script-lint): only ENOENT/ENOTDIR is "missing" (deleted); EACCES/EIO/ELOOP is now `irregular` (skipped, fail closed) rather than silently read as a deletion into an `ok: true` "nothing changed". Three left as reasoned follow-ups: verifier-provenance for `[build]/[test]/[probe]` tags (a pre-existing verification-model concern, not this gate); actionlint's native workflow diagnostics (deferring the whole tool loses them, but separating native from embedded-shell output needs an actionlint-output parser this env can't verify); and cryptographic authentication of the report file (the orchestrator runs the deterministic command and overwrites any pre-planted file; full anti-forgery is a broader harness-trust change). 985 review tests, ESLint/tsc/Prettier clean. * fix(review): clean error from the script-lint handler on a bad plan Wrap `runScriptLint` in the command handler in try/catch, matching sibling build-test: a missing or invalid plan makes it throw, and yargs' default handler would print a stack trace the orchestrator has to parse. Emit the one-line message and a non-zero exit instead; the gate still fails closed on the absent report. * fix(review): scrub HADOLINT_* env, canonicalize worktree paths, portable isolation tests Follow-up adversarial round on the previous fixes: - Scrub HADOLINT_* from the child env (script-lint): hadolint 2.14 MERGES config from HADOLINT_IGNORE / HADOLINT_OVERRIDE_* / HADOLINT_CONFIG with the explicit --config, so an inherited one could suppress the findings the neutral config forces. Drop every HADOLINT_* var, mirroring the SHELLCHECK_OPTS scrub — configuration comes from our --config alone. Hostile-env regression test added (this also makes the HADOLINT_CONFIG assertion hermetic under an inherited value). - Canonicalize the worktree-containment check (script-lint): the lexical startsWith guard is defeated by a symlinked ANCESTOR — a directory replaced with a symlink leaves a child path lexically inside but resolving outside, and lstat/the linter follow it. Add a realpathSync check against the canonical worktree; a path that cannot be canonicalised is handled as missing downstream. Symlink-escape test added. - Portable isolation tests: override TEMP/TMP as well as TMPDIR (os.tmpdir() ignores TMPDIR on Windows, leaving the fail-closed path unexercised there), and guard the 0o700 mode-bit assertion with process.platform !== 'win32'. Left tracked: fail-closed on a PARTIALLY parsed diff — narrow (capture writes the diff in one writeFileSync, not incrementally) and already mitigated (a truncation that later completes changes the hash, so the freshness guard rejects the report). 987 review tests, ESLint/tsc/Prettier clean. --------- Co-authored-by: verify <verify@local>	2026-07-27 06:09:30 +00:00
qqqys	1acc511809	feat(core): add Goal v3 worker tools (#7729) ... * feat(core): add Goal v3 worker tools * fix(core): address Goal tool review blockers * fix(cli): complete Goal tool locale keys * test(core): cover Goal tool error propagation * fix(core): align Goal tool verification guidance * fix(core): harden Goal proposal validation * fix(core): align Goal blocker audit and verifier budget * fix(core): address Goal verification blockers * fix(core): keep Goal completion verifiable --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-27 04:28:39 +00:00
易良	d44030a4c0	feat(core): add model grade selection for subagent spawn (#7685) (#7702) ... * docs: add design placeholder for subagent model grade selection (#7685) * feat(core): add subagent model grade selection * test(subagent): cover resolveModelGrade deep guards and resume else branch - subagent-manager: add tests for non-string grade values, blank values, array-shaped modelGrades, and missing modelGrades (all return undefined) - background-agent-resume: assert configured subagent model is preserved (not forced to 'inherit') when launch flags (model + authType) are absent Addresses test-coverage review findings. * refactor(subagent): extract normalizeModelGradeSettings and merge model validate - Extract normalizeModelGradeSettings helper shared by resolveModelGrade and the Agent tool schema build, so the advertised grades and runtime resolution cannot drift (addresses duplicated shape invariant). - Merge the three model-parameter validate branches under a single `params.model !== undefined` guard. - Update agent.test.ts mock to preserve the real helper while still mocking SubagentManager. * refactor(core): simplify model grade resolution * fix(core): reject unknown model grades * docs(core): clarify model grade precedence * docs: explain subagent model grades * test(core): update subagent manager mock * fix(core): list available model grades * fix(core): trim model grade keys and cover schema removal Grade keys were checked for emptiness via grade.trim() but stored in the map and advertised in the tool schema enum untrimmed, while values were trimmed. A padded key like ' small ' published a padded enum name the model had to reproduce verbatim, and the allowlist check silently excluded it. Normalize the key before storing, allowlist matching, and schema publication. Also adds a test for the delete schema.properties.model branch that fires when grades transition from available to empty, so a regression that breaks the delete leaves no stale model enum in the tool schema. * fix(core): trim allowed model grade filters --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-26 16:23:54 +00:00
Shaojin Wen	d1988fd83d	feat(review): give the verifier a probe capability — run a runnable claim, don't just read it (#7756) ... Measured on this repo: read-only verification is where the review misses its hardest bugs. Handed the exact low-confidence finding for a `!` command that executes twice, the strongest model's Step-3 agents traced the mechanism and called it correct (0/3 across the full roster). Validated the fix directly — when the same models (3.7-max and 3.8-max-preview) were allowed to WRITE AND RUN a probe, both confirmed the double-execute from observed behaviour (`sendShellCommand called twice with ["git push"]`), both did the probe-validity self-check unprompted, and a plausible-but-false negative control was correctly refuted with no fabrication. So the verifier's brief now says: when a finding's failure scenario is a runnable claim about a named unit and the repo has a fast unit harness (vitest/jest/pytest), write a minimal probe, run it, and let the observed behaviour settle the verdict. Two rules keep it evidence rather than theatre — a mandatory self-check that the probe flips between buggy and correct, and leaving the tree exactly as found. A finding a probe confirmed carries `Source: [probe]`, which compose-review treats as deterministic (a run produced it) like `[build]`/`[test]`. This is Phase 1: the agent-driven loop the validation exercised, no new command. The deterministic runner + artifact and the finding-generation side (emitting a probeable low-confidence finding rather than concluding "correct") are follow-ups. Co-authored-by: verify <verify@local>	2026-07-26 16:08:40 +00:00
ovochouovo	60812d4cd3	fix(cli): show tool descriptions in multi-tool compact summaries (#7589) ... * fix(cli): show tool descriptions in multi-tool compact summaries (#6014) buildToolSummary() previously discarded descriptions when 2+ tools of the same category were grouped, showing only counts like "Read 3 files" or "Searched 2 patterns". Now shows actual descriptions inline when ≤3 tools (e.g. "Read a.ts, b.ts, c.ts"), and first 2 + "...and N more" when >3 tools. getActiveToolHint() skips the redundant ⎿ hint line when descriptions are already visible inline. * fix(cli): use i18n key for '...and N more' phrase Use existing '... and {{count}} more' translation key instead of hardcoded English. This ensures the phrase is localized correctly for all 9 shipped locales (fr, zh, de, zh-TW, ca, ja, ru, pt, en). Also updates test expectations to match the i18n format (space before 'and' instead of no space). Addresses CR suggestion on PR #7589.	2026-07-26 14:53:20 +00:00
jinye	9bdc62c74b	perf(cli): replace comment-json settings parser (#7747) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-26 14:42:51 +00:00
Jingyuan Tian	7422bf8797	fix(cli): handle escaped dollars around inline math (#7741) ... * fix(cli): handle escaped dollars around inline math * test(cli): add escaped-dollar TUI capture scenario --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-26 13:58:01 +00:00
Sparkle79	592b3d429d	fix(cli): complete repeated skill slash commands (#7720) ... * fix(cli): complete repeated skill slash commands * fix(cli): scope stacked skill completion * fix(cli): suppress invalid stacked skill ghost text * refactor(cli): centralize stacked skill eligibility * fix(cli): align stacked skill highlighting * test(cli): cover stacked skill completion guards --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-26 13:56:17 +00:00
Shaojin Wen	7c58f97d72	fix(review): recover bilingual register from the live PR when the plan omits the Han flag (#7739) ... * fix(review): recover the bilingual register from the live PR when the plan omits the Han flag The posted review body renders bilingually (English, with the full Chinese version collapsed under it) only when the plan `compose-review` reads carries `prDescriptionHasHan: true`. That flag is written in exactly one place — `fetch-pr` — and read in exactly one place, from a plan path the orchestrator supplies. Two gaps follow: a `plan-diff` plan never records the flag, and a run that improvises the pipeline can hand `compose-review` a plan that is not `fetch-pr`'s report at all. Either way the switch fails safe to English, and a Chinese-authored PR gets an English-only review — observed on #7686, where the four bot reviews off a proper plan were bilingual and the one off an improvised plan was not. When the flag is absent but the plan still names the PR (`ownerRepo` + `prNumber`), recover the signal from the live description with a single `gh pr view`, and test that recovered text for Han. This runs only on the absent-flag path: a recorded `true`/`false` is authoritative and spends no network, so every healthy `fetch-pr` review is unchanged. The signal stays the CLI's own — the real PR body, which the caller cannot forge — so the recovery tightens the "caller cannot toggle the register" property rather than loosening it, and any failure of the fetch falls back to English so the language can never take the review down. * refactor(review): reuse roster's isPositivePrNumber in the bilingual recovery `bilingualFromPlan`'s local `positivePrNumber` duplicated the exact PR-number validation already in `roster.ts` (positive integer, all-digit string, reject null/0/empty). Two copies of the same rule invite a silent divergence: a future change to how a PR number is validated, applied to only one, would make the bilingual recovery and `requiredAgents` disagree on the same plan's PR identity. Export the roster helper and reuse it, coercing to the string form the `gh pr view` call needs at the one call site. * fix(review): route compose-review's gh call via the PR host on GitHub Enterprise The bilingual body-language recovery added a `gh pr view` call inside compose-review, but its CLI handler never called `setGhHost` — unlike fetch-pr/submit/pr-context/comment-status/presubmit. On a GitHub Enterprise PR whose plan lacks `prDescriptionHasHan` but carries the PR identity, the recovery fetch would target github.com, fail, and compose an English-only body that disagrees with the bilingual body `submit` (which does route by host) posts. Give compose-review a `--host` option and call `setGhHost(host)` in the handler, mirroring the sibling subcommands; add it to the skill's Enterprise host list and the Step 6 invocation. Covered by a test that drives the handler with --host and asserts the routing took. * fix(review): strip the prBodyFetcher test seam at the compose-review boundary `prBodyFetcher` is a unit-test seam, but unlike `env` it was not stripped from the model-written state JSON. A state JSON carrying `"prBodyFetcher": "suppress"` survives `JSON.parse`, reaches `bilingualFromPlan`, is called, throws, and drops the Chinese fold through the fail-safe — letting the caller suppress a fold that the plan's own signal would have rendered. Strip it in the handler the same way `env` is stripped, and correct the field doc, which wrongly claimed a model could not supply one. * fix(review): strip prBodyFetcher at the submit boundary; pin fetchPrBodyViaGh and handler stripping with tests (#7739) * fix(review): pin the submit-boundary prBodyFetcher strip; soften SKILL.md wording (#7739) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-07-26 13:54:49 +00:00
Shaojin Wen	471141fcde	fix(review): correct the borrowed lenses and vacuous-test severity (follow-up to #7735/#7736) (#7746) ... * fix(review): correct and tighten the borrowed lenses per review Four findings on the lenses this PR adds: - **Agent 2 (subprocess injection), Critical.** The guidance said "terminate the argv with `--`", but `--` ends *option* parsing without neutralizing a *pathspec* — for an overloaded command it creates one (`git checkout -- release` restores a path instead of switching branch; `git checkout -- .` still discards changes). Reword: validate against the subcommand grammar; a `--` helps only where it keeps the operand's role, and the value allowlist is what closes it. - **Agent 1b (changed literal).** A default ripgrep skips hidden `.github/**`, so a marker consumed only by a workflow reads as "no consumer". Require a hidden-path search (`rg --hidden --glob '!.git/**' --fixed-strings`). - **Agent 4 (unreproducible benchmark).** "Flag as unverified" conflicts with the actionable-findings-only contract. Make it actionable when load-bearing (request the script/env/raw numbers) or no finding when incidental (record under not-verified), never a non-defect finding. - **Agent 5 (equivalent mutant).** Add a focused `buildRoleBrief(PLAN, '5')` test pinning the equivalent-mutant rule and its discriminating-input requirement, so a prompt-assembly regression cannot silently drop it. * fix(review): a vacuous test is a Suggestion, not a Critical for being the sole guard Agent 5's mutation lens graded a sole-guard vacuous test as Critical, but the shared severity ladder — and Agent 7's deterministic efficacy probe — grade an ineffective test as Suggestion. Step 4 keeps the higher severity, so the same inert guard arrived as Critical from Agent 5 and Suggestion from Agent 7, and the Critical won: a PR could be blocked solely for lacking an *effective* test, the exact inflation those shared rules exist to prevent. Align it with the dimension's own "name the bug, not the gap" rule: a vacuous test is a Suggestion, escalated to Critical only when it asserts the opposite of the intended behaviour, was weakened in-diff, or lets a specific incorrect behaviour ship (in which case that behaviour is the Critical, with the test as evidence). Mirrored in the test-matrix brief and the SKILL.md dimension table, and pinned by a buildRoleBrief(PLAN, '5') assertion so the semantic reversal cannot pass the generic word-presence check again. * fix(review): pin the Agent 2 and test-matrix brief corrections, sync the SKILL table --------- Co-authored-by: verify <verify@local>	2026-07-26 13:22:35 +00:00
Heyang Wang	d5a05749bc	fix(cli): keep IME cursor aligned after footer updates (#7711) ... * fix(cli): keep IME cursor aligned after footer updates Preserve the terminal cursor intent across independent UI renders so macOS input methods anchor their candidate window to the input prompt. - Submit text input cursor positions using Ink's render-time contract - Reassert the latest committed cursor before interactive frame writes - Cover standard, incremental, static, and fullscreen rendering paths - Keep cursor cleanup and hidden-state behavior intact * docs(cli): explain render-time cursor submission Document the non-obvious Ink hook ordering that requires cursor positions to be supplied during render rather than from another effect. - Prevent future refactors from reintroducing IME cursor misalignment --------- Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>	2026-07-26 11:42:06 +00:00
Shaojin Wen	27927c46d9	feat(review): borrow maintainer review lenses into the agent briefs (#7736) ... * feat(review): mutation-test the tests in the test-coverage pass (Agent 5) Agent 5 checked that tests exist and cover the diff's paths, but not that they would actually FAIL if the code were broken. A test that stays green no matter what the code does is worse than no test: it reads as coverage while blessing the exact regression it was written to catch. Add a mutation-testing discipline to Agent 5's brief (and its whole-diff test-matrix counterpart): for a test this diff adds or changes to pin a value or behaviour, name the one-line mutation to the code under it that should turn it red — if none does, the test is vacuous. Call out the recurring shapes: both sides of the assertion computed the same way so they move together (`expect(undefined).toBe(undefined)` from a loose/unanchored extraction); reading only the first of several sites the behaviour spans; an `expect(x).toBe(x)` tautology; a "does not throw" assertion for a bug that is a wrong value, not a throw. A vacuous test is a Suggestion on its own, but a Critical when it is the sole guard for a behaviour this diff changes. The agent briefs are CLI-generated (lib/agent-briefs.ts), so the behavioural change is there; the SKILL.md dimension table is synced for the human reader. * feat(review): add contract-shape, migration, equivalent-mutant, and perf-repro lenses Borrow four review disciplines a maintainer applies by hand, wiring them into the agent briefs (CLI-generated) so every review gets them: - Agent 1b (removed-behavior): a changed *literal* is a contract too — when the diff renames/reformats a value a distant consumer matches on its raw shape (a marker string, a key, a status code, regex text), grep the old shape, not the symbol; a rename that compiles can silently stop matching a filter in another file. And a rename/format/schema/default change must handle the data that already exists — an un-migrated population is a split-brain (orphaned records, double-writes), usually fixed by a two-line legacy fallback. - Agent 5 (test coverage): before calling a test vacuous, rule out the equivalent mutant — a mutation that leaves observable behaviour unchanged is not a coverage gap. Name the input that makes the mutation observable. - Agent 4 (performance): do not take the PR's own numbers on trust. Reproduce a cheap deterministic claim (bundle bytes, tree-shake) or report it doesn't; flag a runtime benchmark you can't re-run as unverified rather than endorsing it. SKILL.md dimension table synced for the human reader. Briefs only — no control flow changes. * feat(review): flag subprocess option/argument injection in the security pass Agent 2 listed shell/command injection but not the sink `execFile`/`spawn` leaves open: a user-controlled positional argument to git/gh/tar/ffmpeg that is reinterpreted as an option or special token. A value starting with `-` becomes a flag (`git log --output=<path>` overwrites an arbitrary file; `git checkout -f` discards the working tree) and `.`/`..` becomes a pathspec (`git checkout .` drops unstaged changes) — none of which shell-free spawning stops. Add it to the brief with the standard fix (validate + terminate the argv with `--`) and the call-site-asymmetry tell (one site validates, its sibling does not). SKILL.md security row synced. Briefs only. * feat(review): add the sibling-consistency lens to the code-quality pass When one member of a family of parallel paths carries a validation, guard, cleanup, or shape-check and its twin does not, the lone exception is usually accidental and the missing half is a latent asymmetric failure — harmless until the one input that path sees. Agent 3 now checks that siblings share the guard, names the divergent one, and escalates to the security pass when the missing guard is a validation on untrusted input (the `gitCheckout`-validates-but-its- sibling-does-not shape), rather than filing it as a style nit. SKILL.md quality row synced. Briefs only. --------- Co-authored-by: verify <verify@local>	2026-07-26 09:36:05 +00:00
Shaojin Wen	3fda7aebdd	feat(review): mutation-test the tests in the test-coverage pass (Agent 5) (#7735) ... Agent 5 checked that tests exist and cover the diff's paths, but not that they would actually FAIL if the code were broken. A test that stays green no matter what the code does is worse than no test: it reads as coverage while blessing the exact regression it was written to catch. Add a mutation-testing discipline to Agent 5's brief (and its whole-diff test-matrix counterpart): for a test this diff adds or changes to pin a value or behaviour, name the one-line mutation to the code under it that should turn it red — if none does, the test is vacuous. Call out the recurring shapes: both sides of the assertion computed the same way so they move together (`expect(undefined).toBe(undefined)` from a loose/unanchored extraction); reading only the first of several sites the behaviour spans; an `expect(x).toBe(x)` tautology; a "does not throw" assertion for a bug that is a wrong value, not a throw. A vacuous test is a Suggestion on its own, but a Critical when it is the sole guard for a behaviour this diff changes. The agent briefs are CLI-generated (lib/agent-briefs.ts), so the behavioural change is there; the SKILL.md dimension table is synced for the human reader. Co-authored-by: verify <verify@local>	2026-07-26 08:32:53 +00:00
Shaojin Wen	0446c576f9	feat(review): redefine medium effort as a balanced verified pass (#7733) ... * feat(review): redefine medium effort as a balanced verified pass The old medium was a thin inline Step 3C walk: no subagents, no build/test, no verification, no comment-status — cheaper than high but structurally unable to catch a whole class of bugs. Dogfooding measured it missing real Criticals that high caught, including a compile error CI had already flagged red (medium never built, so it could not see it). Redefine medium as a balanced, verified pass — the high pipeline with its most expensive passes removed: - Runs the Step 3A/3B fan-out over a reduced dimension set — issue fidelity (Agent 0), correctness (1a/1b/1c), security (Agent 2), quality (Agent 3), performance (Agent 4), test coverage (Agent 5), and build & test (Agent 7). Skips the adversarial personas (6a/6b/6c) and the Agent 8 diff-specialists. - Loads project rules (Step 2) and runs comment-status like high. - One verification pass (Step 4). Skips the reverse audit (Step 5), the incremental cache, and PR posting (--comment still forces high). - On a large diff, caps the 3B territory fan-out by re-running plan-diff with a coarser --max-chunk-lines. Measured against high on the same PR it lands at roughly one-third to one-half the time and tokens. It reliably catches mechanical defects (compile errors, failing tests — build-test is deterministic) and obvious correctness bugs, but is not an exhaustive correctness audit: a subtle Critical that only the reverse audit or the adversarial personas would surface can slip, so security-sensitive and pre-release reviews should still use --effort high. Because it runs no reverse audit, compose-review caps a clean medium verdict at Comment; a verified Critical still yields Request changes. low (thin inline pass) and high (full pipeline) are unchanged, as are the effort defaults (high for PRs, medium for local) and parse-args. SKILL.md only. * fix(review): make the roster and coverage gate effort-aware for medium The medium (balanced) tier deliberately skips the three adversarial personas (6a/6b/6c), but `requiredAgents()` added them unconditionally in the Step 3A (small-diff) roster. So `check-coverage` found no transcripts for the un-launched personas, flagged them missing, and exited 3 — and the skill says a non-zero exit halts before Step 4. A medium review of any small diff (the common case) would stall at Step 3D unless the model improvised past the gate. The SKILL-only redefinition of medium could not work without this. Thread the effort level through the roster/coverage machinery: - `requiredAgents(plan, effort?)` — at `medium`, do not require 6a/6b/6c in 3A. High (and the default) still require them; 3B is unchanged (personas are folded into the chunk agents there, never required as standalone roles). - `agent-prompt --roster --effort <level>` — builds the roster from the same effort-aware `requiredAgents`, so a medium roster prints only the 9 agents it launches instead of the full 12-then-skip. - `check-coverage --effort <level>` — holds the run to the same reduced set, so a balanced review is not flagged for personas it deliberately did not run. SKILL.md Step 3A/3D now pass `--effort medium` to both commands. Adds a roster test for the medium 3A drop. No behaviour change at high effort. * fix(review): carry effort in the plan so every reader agrees, and stop medium escalating itself The medium tier threaded `--effort` as a flag to `check-coverage` and `agent-prompt --roster`, but `compose-review` recomputes coverage itself and was never given it — so on every medium run its body disclosed the three personas it deliberately skipped as missing coverage, and its stderr FIX lines told the orchestrator to rebuild the full roster and run a reverse audit, turning the one mandated repair round into a silent escalation back to high. A caller-supplied `--effort` was also a roster the caller could shrink, against the invariant `roster.ts` opens with. Record the effort in the plan at capture time instead. `fetch-pr`, `capture-local` and `plan-diff` take `--effort` and write `plan.effort`; `requiredAgents` reads it from there and the `--effort` flag is gone from `agent-prompt`/`check-coverage`. The roster, `check-coverage` and `compose-review`'s recomputation now read one field and cannot disagree — fixing the personas half for free. For the reverse-audit half, `verificationGaps` reads `plan.effort`: at medium the absent reverse audit is a by-design omission that caps a clean verdict at Comment with an honest disclosure and no FIX line, not a repairable gap. Verify (Step 4) still runs and is still enforced at medium. Also: drop the medium 3B coarsening that re-ran `plan-diff` — on a same-repo PR it fed the diff back through the lightweight path, producing a plan with no worktree metadata that dropped Agents 7/1c and could clobber the fetch report Steps 3D/6/7 read. And correct the prose that still said compose-review runs only at high and that medium findings are unverified. Tests: roster reads plan.effort; plan-diff records it; agent-prompt --roster on a medium plan builds the reduced set (the command-boundary the pure-function test could not reach); compose-review caps a medium verdict at Comment with no reverse-audit FIX line while still requiring the verifier. * refactor(review): use the shared ReviewEffort type, and cover the medium coverage path Two review suggestions on the effort work: - The `'low' | 'medium' | 'high'` union was inlined in the capture commands where `parse-args` already exports `ReviewEffort`; import and reuse it so a fourth level cannot drift one copy out of sync. - Add a check-coverage test that reads the effort from the plan: a medium plan whose reduced roster was launched has no missing roles, while the same records under a high plan flag the personas — the integration point the roster unit test cannot reach, and the one whose regression would exit 3 on every medium review. --------- Co-authored-by: verify <verify@local>	2026-07-26 08:14:23 +00:00
jinye	8fa8085036	perf(core): Lazy-load first-use dependencies (#7686) ... * perf(core): Lazy-load first-use dependencies Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): Fix simple-git loader mock Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(core): Cover abort during xterm load Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Address lazy-loader review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Validate lazy dependency module shapes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-26 03:04:33 +00:00
yijie zhao	eddee6702c	test(cli): cover bottom-stuck virtualized list behavior (#7652) ... * fix(cli): collapse bottom-stuck virtualized list viewport * fix(cli): address virtualized list review feedback * test(cli): cover short virtualized list collapse * test(cli): keep virtualized list coverage only	2026-07-26 02:36:24 +00:00
samuelhsin	3a6c8e0c03	feat(skills): add overridable default-disabled state (#7357) ... * feat(skills): add overridable default-disabled state * fix(skills): address review feedback on default-disabled PR (#7357) - Fix disabledChanged comparison in SkillsManagerDialog to use previousDisabled (locked names filtered) instead of workspaceDisabled, preventing spurious settings writes when a skill is disabled at both workspace and higher scope - Import SettingScope as a value instead of string-casting literals in skill-settings.ts for compile-time safety - Add dual-key change test: enabling a workspace-hard-disabled default-disabled skill produces both skills.disabled and skills.enabled changes in one operation - Add legacy inactive-extension branch tests: reject when disabledReason is undefined and skill is not in settings disablements; allow when it is disabled by settings * fix(cli): address skills picker review feedback (#7357) Extract the skills picker's workspace persistence computation into a tested pure function so orphaned workspace disables (skills not currently loaded) are explicitly preserved and pinned by a regression test. Also add an integration test asserting a workspace-scope hard disable surfaces disabledReason 'hard' through the full loadSettings -> resolveSkillSettings -> mapSkillConfigToStatus pipeline. * fix(cli): resolve skill disablements in safe mode for status API (#7357) * fix(cli): dynamically import skill-settings in serve to keep fast-path closure clean (#7357) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix[bot]@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>	2026-07-25 18:29:20 +00:00
Shaojin Wen	c90748eae4	fix(review): don't overclaim a submit-gate bypass on a same-account write (#7718) ... * fix(review): don't overclaim a submit-gate bypass on a same-account write The cleanup bypass audit lists reviews and issue comments by the reviewing account inside the review window that the submit receipt does not vouch for. That set legitimately includes writes this review never made: a concurrent `/review`, a triage/autofix bot, or the user themselves, all posting under the same account, produce exactly the same shape — the audit cannot tell them apart from a genuine `gh pr review` bypass by metadata alone. Observed in practice: an observe-only (`no --comment`) quick pass that posted nothing had a concurrent same-account CHANGES_REQUESTED review land in its window, and the warning — relayed verbatim into the review summary — read "a write bypassed the submit gate", alarming over what was an external write. Reframe the footer so it no longer asserts a bypass as the default reading: the likely cause is benign (named account, another workflow, or a bot), and a write here is a real gate bypass only if its content is this review's own output. The detection logic and the per-write detail lines are unchanged, so nothing that was surfaced before is hidden now — only the conclusion the copy draws is corrected. * fix(review): third-person footer, sync SKILL.md, pin the copy in tests Address review feedback on the bypass-audit warning reframe: - The footer led with "you", but cleanup's stdout is read by the model as well as relayed to the user. To the model "you" is itself, and the model posting under the reviewing account is the exact bypass this tripwire catches — so the copy opened by calling that case benign to the one reader who might be the offender. Switch to third person ("the user (from another terminal), another workflow, or a bot"). - Requote the CLOCK_SKEW_MS docstring: it justified over-flagging by pointing at warning copy ("the user did this themselves") that this change removed. - Sync SKILL.md Step 9: it still framed a flagged write bypass-first with none of the new discriminator; lead with the external same-account reading and add "a write that bypassed the gate only if its content is this review's own output", matching the footer. - Tests: pin the interpolation shape `(reviewer)` instead of the bare word (the header also says "reviewing account", so the old assertion stayed green even with the account name dropped), and assert the "Relay this warning verbatim" sentence — the line that moves the warning to a human, previously covered by nothing. --------- Co-authored-by: verify <verify@local>	2026-07-25 17:23:39 +00:00
Shaojin Wen	a8a28a1137	fix(acp-bridge): raise live journal caps and expose as daemon config (#7715) ... The live journal (DAEMON-009) caps were too conservative for real-world agent turns: 2000 events / 2 MiB caused 79% event loss on a typical long turn (9647 events). Raise defaults to 10 000 events / 8 MiB and expose them as --max-journal-events / --max-journal-bytes CLI flags, following the same config path as --compacted-replay-max-bytes. Also fix stale docs that described the liveJournal as uncapped.	2026-07-25 14:14:09 +00:00
VectorPeak	8edfa31aab	fix(cli): parse heatmapDays strictly (#7218) ... * fix(cli): parse heatmapDays strictly Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> * test(cli): cover negative heatmapDays fallback Co-authored-by: qwen-code-ci-bot <253268222+qwen-code-ci-bot@users.noreply.github.com> --------- Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <253268222+qwen-code-ci-bot@users.noreply.github.com>	2026-07-25 13:53:53 +00:00
Shaojin Wen	5c7c2ec0d0	fix(web-shell): enable Changes and History dialogs for worktree sessions (#7695) ... * fix(web-shell): enable Changes and History dialogs for worktree sessions * fix(web-shell): add containment and gitCwd forwarding tests, fix SDK nits (#7695) * fix(cli): use realpathSync in resolveContainedCwd test assertions (#7695) On macOS, os.tmpdir() returns /var/folders/... (a symlink to /private/var/folders/...), but resolveContainedCwd resolves paths via fs.realpathSync. The test assertions compared against the raw os.tmpdir()-based paths, causing two tests to fail on macOS. Wrap the expected values in fs.realpathSync to match the function's actual return value on all platforms. * test(web-shell): cover gitCwd forwarding through log pagination (#7695) * test(sdk): cover cwd query encoding in workspace git client methods (#7695) * fix(sdk): bump browser bundle size limit for worktree cwd params --------- Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-25 12:47:02 +00:00
qwen-code-dev-bot	34a3d46006	fix(core): fire StopFailure hook on loop detection early returns (#7592) ... * fix(core): fire StopFailure hook on loop detection early returns (#7588) When loop detection (always-on safety or heuristic) terminates a turn early via `return turn`, the Stop hook code after the streaming loop was never reached. StopFailure hooks were only fired from the CLI layer for API errors, not from client.ts for loop detection. Added `loop_detected` to StopFailureErrorType and fire the StopFailure hook via MessageBus before each loop detection early return, so cleanup/notification hooks run regardless of how the turn ends. All 284 client tests and 682 hook tests pass. * fix(core): use direct hookSystem call for loop-detection StopFailure (#7588) The MessageBus bridge has no StopFailure case, so the hook never executed. Switch to config.getHookSystem()?.fireStopFailureEvent() (matching the CLI's API-error path), make it fire-and-forget per the StopFailure contract, drop the stale last_assistant_message that carried the previous turn's text, deduplicate via a private helper, update docs with loop_detected, regenerate the settings schema, and add regression tests for both loop-detection paths. * test(core): add negative-path test for StopFailure hook disable guard (#7592) * test(core): add negative-path tests for StopFailure hook guards (#7592) --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-07-25 11:29:02 +00:00
Shaojin Wen	7e2eeb90c2	fix(review): address post-merge P2 review feedback on the review skill (#7708) ... * fix(review): address post-merge P2 review feedback on the review skill Follow-up to #7690 and #7691, picking up six P2 review comments that landed just before those PRs merged: - comment-status: declare `host` on CommentStatusArgs so a future refactor that drops it becomes a type error, not a silent runtime regression. - comment-status: the degraded report now emits `prAuthor: null` (matching `worktreeHeadSha`) instead of '', so a total index failure is distinguishable from a legitimately absent (deleted) PR author. - comment-status: wrap the second head sample in its own try/catch and fall back to liveHeadBefore, so a transient failure after the comments are already fetched no longer discards them into a degraded report. - submit: write the audit receipt to a sibling tmp then renameSync over the target, so a crash mid-write can never leave a truncated receipt that parseReceiptIds reads as [] and drops every accumulated review id. - SKILL.md: the Step 1 comment-status guard is now worktree presence, not "the context file reports inline comments" — pr-context reports those in lightweight mode too, where no worktree exists. - SKILL.md: lead the Step 7 write-ban with a single compression-proof sentence; the enumeration stays as support beneath it. * fix(review): use atomicWriteFileSync for submit receipt (#7708) --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-07-25 11:24:23 +00:00
hogeheer499-commits	88782646ab	feat(core): configure stream rate-limit retry delays (#7666) ... Co-authored-by: JS van Dijk <267467744+hogeheer499-commits@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-25 09:43:25 +00:00
OrbitZore	62e009a952	feat(channels): GitHub polling adapter with notification-as-wakeup architecture (#7632) ... * feat(channels): add GitHub polling adapter with notification-as-wakeup architecture Introduce a GitHub channel adapter that monitors notifications and responds to @mentions on issues/PRs by posting comments. Uses last_read_at as a per-thread watermark for comment enumeration, replacing the unreliable latest_comment_url approach. Foundation changes to ChannelBase: - sendThreadMessage for thread-targeted delivery (IM adapters unchanged) - Envelope.metadata appended to prompt after command parsing - chat_thread session scope (channel:chatId:threadId) prevents cross-repo session collision - polling-helpers: testBotMention/stripBotMention (separate detection from stripping, no whitespace collapsing), cursor persistence, abortableSleep GitHub adapter design: - Notifications as wake-up signals only (unread filtering) - listComments enumeration with last_read_at watermark - Bot self-comment filtering, case-insensitive mention regex - In-memory recentlyProcessed set for mark-read failure dedup - First-contact: new issue body @bot triggers processing - Error comment + cursor advance on handleInbound failure - pollInterval minimum 60s, exponential backoff 2s-30s * refactor(channels): extract PollingChannelBase from polling-helpers Replace the loose polling-helpers module with a PollingChannelBase<Cursor> abstract class that encapsulates the poll loop, cursor persistence (JSON, atomic write), exponential backoff, and start/stop lifecycle. Subclasses implement only pollOnce() and createInitialCursor(). - Delete polling-helpers.ts (cursor fns + abortableSleep moved into base) - Move mention utilities (testBotMention/stripBotMention) to github pkg - GithubAdapter now extends PollingChannelBase<{ lastProcessedAt }> * fix(channels): remove Gitea/GitLab mention from sendThreadMessage JSDoc * fix(channels): match /pulls/N in notification subject URL GitHub PR notifications use /repos/{owner}/{repo}/pulls/{N} in subject.url, not /issues/{N}. The regex only matched /issues/, causing PR notifications to be skipped and marked read. Also sets threadId to 'pr:N' for PRs (was always 'issue:N'). * test(channels): add PR body first-contact unit test Verify that PR notifications with @mention in the body (not a comment) correctly trigger the first-contact path: extractFromSubjectUrl matches /pulls/N, listComments returns empty, tryFirstContactBody fetches the PR body and dispatches to handleInbound with threadId 'pr:N'. * feat(channels): read pollInterval from channel config in PollingChannelBase Move pollInterval config reading from GithubAdapter to the base class. The user's configured pollInterval in settings.json is now respected directly without a minimum enforcement. Defaults to 60000ms when not configured. * fix(channels): prepend metadata before prompt text Agent sees issue/PR context (type, title, URL) before the user's request, improving comprehension. Metadata is still appended after slash-command parsing so commands are not affected. * refactor(channels): route all ChannelBase delivery through sendThreadMessage Replace all internal sendMessage calls with sendThreadMessage, passing envelope.threadId (or target.threadId / undefined) so polling adapters can deliver to the correct thread. IM adapters are unaffected — the default sendThreadMessage falls through to sendMessage. * docs(channels): document sendThreadMessage delivery architecture * fix(channels): address review findings - Cap recentlyProcessed Set at 10k entries to prevent unbounded growth - Validate cursor JSON shape (non-null object) in loadCursorFromDisk - sendThreadMessage falls through to sendMessage when threadId is undefined instead of silently dropping - Remove duplicate pollInterval from GithubConfig (now in ChannelConfig) - Fix chat_thread routing key trailing colon when threadId is undefined * docs(channels): fix metadata JSDoc — prepended, not appended * fix(channels): use recentlyProcessed dedup for first-contact body Replace the fragile createdAt-vs-cursor check in tryFirstContactBody with the recentlyProcessed set. The cursor advances globally based on notification updated_at — when a different notification with a later updated_at is processed first, the cursor can advance past the issue's created_at, causing the first-contact check to incorrectly skip the issue body (forget reply bug, found in E2E TC-2b). * refactor(channels): two-layer dedup for GitHub adapter Layer 1: global cursor filters notifications by updated_at (sorted ascending, old first). Layer 2: server-side last_read_at filters comments by created_at (sorted ascending). - Delete recentlyProcessed Set (no longer needed) - Sort notifications by updated_at ascending before processing - Sort comments by created_at ascending before processing - Pass latest comment created_at to markThreadAsRead as last_read_at * fix(channels): address review findings on GitHub adapter Blockers: - sessionScope: add defaultSessionScope to ChannelPlugin, apply in parseChannelConfig so router and adapter agree on 'chat_thread' - channel-registry.test.ts: add 'github' to expected type list Should-fix: - Replace per-thread markThreadAsRead (PATCH) with bulk markNotificationsAsRead (PUT /notifications + last_read_at). API errors stop the batch without marking failed notifications read; handleInbound errors still advance (error comment posted). - connect() throws on bot identity failure instead of failing open - metadata appended after promptText (inside sender attribution) - isSharedSessionTarget includes 'chat_thread' scope Nits: - startPollLoop re-entrancy guard - clean-package-build-artifacts.js includes github - index.ts re-exports GithubChannel * fix(channels): use max updated_at of all fetched notifications as last_read_at Prevents re-fetching the same notifications in the next poll cycle. The bulk PUT /notifications marks all fetched notifications as read up to the max updated_at, regardless of per-notification success. * fix(channels): address review round 2 findings - #12: loadCursorFromDisk rejects arrays - #13: pollInterval validates positive finite number - #19: first-contact gate uses dispatchedMention flag (not newComments.length) - #25: stripBotMention no longer trims (preserves indentation) - #27: remove adapter-level requireMention, unify on GroupGate - #31: add chat_thread SessionRouter routing key tests - #33: clear metadata on collect-mode synthetic envelope - #35: fix PollingChannelBase.test import path - #36: add @octokit/rest to 15-channel-adapters.md dependencies * docs(channels): document known limitations for GitHub adapter - First start skips existing unread notifications (cursor = now) - Requires classic PAT (fine-grained PATs lack notifications API) - PR review comments not enumerated (issue comments only) * fix(channels): address review round 3 findings - #9: buildMetadata derives web URL from baseUrl (GHE support) - #12: sendThreadMessage throws on invalid threadId format - #19: mention lookbehind matches cc:@bot and "@bot" patterns - #23: cursor file name uses sha256 hash to prevent collision - #26: test verifies cursor persistence to disk - #31: postErrorComment double-failure logs to stderr - #45: tests use mkdtempSync isolation instead of real QWEN_HOME * fix(channels): pass threadId through pairing flow + sendResponseMessage test - #13+16: onPairingRequired receives envelope.threadId and passes it to sendThreadMessage, so pairing codes are delivered on threaded channels (GitHub) instead of throwing - #6: add test verifying sendResponseMessage resolves threadId from router.getTarget and passes it to sendThreadMessage * fix(channels): pass proxy to Octokit for daemon-worker environments - #44: read this.proxy from ChannelBaseOptions and pass HttpsProxyAgent to Octokit request.agent, matching the Telegram adapter pattern * fix(channels): address review findings — immutable senderId, comment time window, validateCursor, retry wrapper - senderId uses immutable user.id; allowedUsers resolved to IDs at connect - Comment filter upper bound: updated_at <= maxUpdatedAt (batch window) - Per-notification errors use continue (best-effort), not break - validateCursor() virtual hook for subclass cursor shape validation - sendThreadMessage/postErrorComment wrapped in githubApi() retry - webOrigin handles default api.github.com → github.com - Docs: classic PAT only, markNotificationsAsRead, dedup claims removed - Tests: threadId priority, metadata consumption, defaultSessionScope, QWEN_HOME isolation, persistent mock rejection Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(channels): mark notifications read before processing to prevent duplicate replies Bot's own replies bump notification updated_at past the pre-captured maxUpdatedAt, so markNotificationsAsRead(maxUpdatedAt) failed to mark them read — the next poll re-fetched the same comments and replied again. Move markNotificationsAsRead + cursor advance before the processing loop (best-effort delivery). This is safe because bot's own comments do not flip notifications back to unread. Update docs to reflect the new poll cycle order and best-effort semantics. * fix(channels): update sender gate after allowedUser ID resolution and harden tests Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(channels): cursor-based comment window to prevent duplicate replies PUT /notifications is async (202) with a last_read_at cutoff — the bot's reply bumps updated_at past the cutoff before the server processes the mark, so the notification is never marked read and gets re-fetched on the next poll, causing duplicate replies. Use the cursor value before advancement as an exclusive lower bound for the comment enumeration window: (windowSince, maxUpdatedAt]. Comments already eligible in a previous poll are excluded regardless of whether the mark succeeded. Zero new persistent state. * fix(channels): cursor-based comment window to prevent duplicate replies PUT /notifications is async (202) with a last_read_at cutoff — the bot's reply bumps updated_at past the cutoff before the server processes the mark, so the notification is never marked read and gets re-fetched on the next poll, causing duplicate replies. Use the cursor value before advancement as an exclusive lower bound for the comment enumeration window, with per-notification last_read_at as the preferred lower bound when available (server-side per-thread watermark). Comments already eligible in a previous poll are excluded regardless of whether the mark succeeded. Zero new persistent state. * fix(channels): address review findings — null guard, cursor validation, metadata dedup, abortable sleep, docs - Guard against null notification.subject.url in pollOnce - Validate lastProcessedAt is a parseable date in validateCursor - Add metadata: undefined to second collect-mode drain path - Refactor abortableSleep as protected method on PollingChannelBase - Fix docs: requireMention is nested under groups.* - Add tests: chat_thread shared session, dispatchedBodies eviction, cursor enumeration window, last_read_at in mention tests * docs(channels): sync docs with implementation — cursor shape, error handling, GitHub adapter tables, first-contact - Design doc: update Cursor to { lastProcessedAt, dispatchedBodies? }, add validateCursor date check, abortableSleep protected method, break-on-error semantics, subject.url null guard - Developer docs: add GitHub to adapter table and adapter matrix - User guide: add first-contact step to How It Works, clarify mark-before-process * fix(channels): address review round 2 — error dedup, abortable retry, backoff reset, window test - Record dispatchedBody on first-contact handleInbound failure to prevent duplicate error comments when mark-read async hasn't taken effect - Use abortableSleep instead of raw setTimeout in githubApi retry so disconnect() can interrupt rate-limit cooldowns - Reset consecutiveErrors in startPollLoop so stop/restart cycles don't inherit stale elevated backoff - Add test for cursor window client-side lower-bound exclusion filter * fix(channels): address review round 3 — cursor validation, error dedup, sender gate, bot-self body - validateCursor: normalize falsy non-array dispatchedBodies (false/0/""/null) to [] instead of passing them through to .includes() which throws TypeError - Set dispatchedMention after postErrorComment to prevent first-contact from posting a duplicate error comment on the same thread - Only set dispatchedMention when the sender passes the sender gate, so a disallowed commenter's mention no longer suppresses a valid first-contact body from an allowed issue author - Skip bot-authored issue bodies in tryFirstContactBody to prevent self-response loops under open sender policy * fix(channels): address review suggestions — test coverage, cursor filename, assertion precision - Pairing flow: add threadId pass-through regression test - pollInterval: add table-driven edge cases (0, -1, NaN, Infinity, string) - Add null-URL notification followed by valid notification batch test - Fix comment window test to assert paginate call 3 (listComments) not call 2 - Truncate cursor filename encoded prefix to 200 chars (filesystem 255 limit) - Assert mark-read uses batch maxUpdatedAt, not just { read: true } - Assert real GitHub plugin declares defaultSessionScope chat_thread - Add invocationCallOrder assertion for mark-before-process ordering * fix(channels): address review round 4 — allowedUsers throw on resolve failure, crash table fix, mark-read failure test * fix(channels): address review round 5 — created_at filter, retry-after NaN guard, retry/sendThreadMessage tests, docs fixes * fix(channels): address ci-bot review 4778587403 — reconnect idempotency, github type enumerations, retry/webOrigin tests * chore(channels): align channel-github version to 0.21.0 after upstream merge * chore(channels): update package-lock.json for channel-github 0.21.0 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: OrbitZore <orbitzore@users.noreply.github.com>	2026-07-25 09:31:50 +00:00