345 commits

Author	SHA1	Message	Date
Shaojin Wen	7091b8c761	fix(review): lock the PR review worktree lease against concurrent sessions (#9211) ... * fix(review): lock the PR review worktree lease against concurrent sessions The /review worktree lives at a fixed path per PR number, and the lease recording its owning session was only consulted by the end-of-session crash sweep. A second session reviewing or finishing the same PR deleted the first session's worktree, branch, and side files mid-run (#9205). Make the lease double as a lock: fetch-pr refuses with an actionable error before touching anything when another session holds it, and cleanup skips the whole target with a note. Ownership is per session, so drift restarts and later rounds of a multi-prompt review are not locked out. A missing worktree now fails repo-context with a re-run-fetch-pr message instead of a bare ENOENT. * fix(review): roll back the lease on fetch-pr failures and scope the missing-worktree remedy Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): shield live leases from the cleanup sweep and roll back on any fetch-pr failure * fix(review): re-check the lease after cleanup's audit to close a TOCTOU (#9211) - Re-read the lease after the network-bound bypass audit and before any destructive step, so a session that acquires the lease during the audit is skipped, not destroyed (R2-10). - Narrow the cleanup lease-skip guard to the real lease shape so a target named 'lease' still has its own side files swept (R2-1). - Make the fetch-pr lease rollback best-effort via tryRemove so an un-removable lease file cannot mask the original failure (R2-5). - Pin the lease-lock wiring and success/rollback invariants in tests (R2-2, R2-7, R2-8, R2-11). * fix(review): validate fetch-pr's number and release leases off side-file residue (#9211) The lease gate only engaged `pr-\d+` targets while cleanStale destroyed worktreePath(prNumber) for any input, so a malformed number bypassed the lock and deleted a live holder's worktree; refuse non-positive-integer pr_number before the gate like the sibling commands. Cleanup now releases the lease once the worktree and branch steps succeed instead of holding it on an un-deletable side file, which wedged every later review of the PR. The lease-file grammar is one shared predicate (isReviewLeaseFile) across the writer, the sweep guard, and the finalizer scan, and the lease tests pin the arguments and ordering the mocks previously left blind. * fix(review): acquire review leases atomically and fail closed on identity (#9211) Close the round-5/6 lease-lock findings: - Create the lease with `flag: 'wx'` so two concurrent fetch-prs that both pass the gate's read cannot clobber each other's lease; on EEXIST, same-session re-fetch rewrites, a foreign holder refuses (R6-1). - Roll the lease back on failure only when this run created it, and compare ownership before deleting so a re-fetch keeps the session's live lease and a lease acquired during a stuck run survives (R6-2). - Refuse fetch-pr before any state when QWEN_CODE_SESSION_ID / QWEN_CODE_PROMPT_ID are absent instead of running lease-less (R6-3). - Register the lease inside the rollback try (R6-5). - Track the platform separator in the lease assertion (R6-4) and gate the POSIX-only ENOTDIR test off Windows (R5-1). - Pin the `Number(prNumber) <= 0` validation disjunct (R5-2) and arm the side-file sweep in the lease-skip test (R4-3). --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-autofix[bot] <qwen-code-autofix[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-17 00:48:00 +00:00
Shaojin Wen	2610f6ed6d	feat(review): apply the huge round reduction only when the run has a clock (#9203) ... * feat(review): let an operator lower the reverse-audit round cap The round cap is chosen from the diff topology, which is right for the cost question it answers but leaves no way for someone paying for reviews to say "spend less on this loop". `review.reverseAuditRounds` is that knob: a standing operator policy that lowers whichever tier applies. It can only lower, and the asymmetry is the point rather than caution about configuration. A single operator-chosen count is exactly what tiering removed — a round is one agent on a small diff and about ninety minutes on a huge one, so one number is wrong for at least one topology, and most wrong for the one whose cap exists to stop six-hour reviews that post nothing. Lowering carries no matching hazard: it can only end the loop sooner. The floor stays at the huge tier's three, for the reason the plan reader already refuses one and two — a cap below that pre-empts the two-consecutive-dry rule and buys a capped verdict rather than a cheaper review. The two things an operator means by "let it run longer" both have direct expressions that a round count only approximates: a ceiling longer than the huge tier assumes is a deadline, which the admission gate already prices a round against, and "keep going while it is still finding real defects" is a property of the findings rather than of a number chosen before the review starts. It is a setting, not a flag, and it resolves in the capture command rather than at the gate — so it lands in the plan and every reader sees one number without learning a setting was involved. That satisfies the module's standing rule that a budget the caller passes is a budget the caller can inflate, rather than making an exception to it. The reader needs no new code: a lowered value is inside the tier's band, which the existing clamp already honours. Operator scopes only, like the other review policy settings — a repository must not choose how deeply the pipeline verifies it, which the new tests pin by putting the setting in a workspace file and asserting it does nothing. Two things fall out of adding a fourth caller, both fixed here: - The plan builder now takes the ceiling as a REQUIRED parameter. Three capture commands build a plan, an optional one is a parameter a call site can quietly omit, and a setting that applies to two of the three review entry points is worse than one that applies to none. Passing undefined is how a caller says "no ceiling", visibly. - Reading the settings can throw: loading raises a fatal error when any settings file fails to parse, and this is now read while the diff is being captured — the review's first step. A stray comma in a file none of these settings had to come from would have ended the whole review. It degrades to the defaults and discloses instead, and every default is the conservative side: attribution on, no auto-posting, no effort or round-cap override. * chore(review): regenerate the settings JSON Schema for the new review setting The schema file is generated from the settings schema and checked in; adding a setting without regenerating it fails the CI check that keeps the two in step. No behaviour change — the file is derived output. * fix(review): stop the settings degrade from being killed by its own announcement The try/catch added here exists so a corrupt or unreadable settings file cannot end a review: loading throws a fatal error when any settings file fails to parse, and this is read while the diff is being captured, the review's first step. It then announced the degrade through the *throwing* stderr helper, so the announcement could end the review the degrade was written to save. Both halves are reachable together, and the second is ordinary rather than exotic: `process.stderr.write` throws on EPIPE or a closed fd — whenever the reader goes away (`qwen … | head`) or a daemon redirects its stderr — which is why the safe sibling exists and says so in its own docstring. With a broken settings file and no reader, the throw propagates out of the catch and all three capture commands crash before writing a plan. Switched to the safe writer, with the reason recorded at the call site. The test that covers it needed the mock repaired first, because the mock was hiding the bug: it mapped both writers to one non-throwing spy, which makes the throwing and safe helpers interchangeable and mocks away the entire distinction the degrade depends on. The safe one is now a spy that swallows what the underlying write throws, matching the real contract — so a test can make the write fail and see which helper the code chose. Reverting to the throwing helper turns the new test red; before this, it turned nothing red. * fix(review): correct two rationales that contradict the code they sit beside Both are claims about mechanism, both false, and both contradicted by documentation already in the same file — which is what makes them worth fixing past the round where only correctness fixes land: in this codebase the comment is the design record, and a wrong one outlives the round that shipped it. **Why a cap of one or two is refused** was stated in four places as "it forces a non-converged stop where two-consecutive-dry would have converged on its own". That is false for two: the convergence check runs before the cap gate, so an all-dry loop reaches CONVERGED under any cap of two or more — which the huge-tier constant's own docstring, two functions away, already said. The reasons are real but different for each value. One refuses the convergence pair's second member, so the loop can never produce the two dry audits convergence is defined by. Two lets an all-dry loop converge but leaves no round for a loop that reports anything, so the first finding makes the stop non-converged. Both end in a capped verdict rather than a cheaper review; only the mechanism was wrong. **When loading settings throws** was described as "any settings file fails to read or parse", with a stray comma as the example. Malformed JSON is the one case that does not throw: it is copied aside and recovered, under a comment that says "Never crash due to a corrupted settings file". The throw comes from a file that cannot be read, which is enough on its own to justify the degrade — this is read while the plan is being captured — so the correction narrows the claim without weakening the reason for the guard. * feat(review): apply the huge round reduction only when the run has a clock Three is the one tier lower than the topology beneath it, and read as a statement about auditing it is backwards. A huge diff has more defects and more territory than a chunked one, converges later, and on recall deserves more rounds rather than fewer — the standing counterexample is a 5,801-line PR that took eight review rounds and was still surfacing Criticals in code present since its first commit. It was never a statement about auditing. It is a statement about a wall: a reverse-audit round on a 4,000-line PR is about ninety minutes, five of them are 450, and a six-hour CI ceiling does not hold that plus the fan-out and the tail. The survey behind it measured absent reviews rather than slow ones — twenty-six timed-out jobs in one window, about 122 hours of compute, nothing posted. Three rounds reported beat five rounds lost. That argument is sound exactly where the wall is. A local run exports no review deadline, nothing kills it at six hours, and the reduction there trades recall away to fit a ceiling that does not exist — on the tier where recall matters most, and by a number calibrated against somebody else's CI. So the reduction now applies only when the run has a deadline at all. With a clock a huge diff caps at three, as before; without one it is simply a large chunked diff and caps at five. The clock is read where the settings ceiling is read — in the capture command, passed into the budget, recorded in the plan — so the two facts the cap depends on arrive the same way and the budget module keeps its property of touching neither the environment nor the settings. Both now travel as one context object rather than a growing parameter list, still required at the plan builder so a capture command cannot silently omit them. The admission gate asks the same question through the same parse the deadline gates already use, so "has a deadline" and "a deadline will be enforced" cannot come apart. All four capture/gate clock combinations are safe and covered: a plan captured without a clock records five and is honoured at five; read later under a clock its band closes to three and it is cut to three, which is the conservative direction when a wall turns out to exist after all. Two things this does not pretend to fix, both recorded in the design note. The deadline gate falls back to a flat thirty-minute round estimate until it has measured one, so on exactly the runs that time out it under-prices the first two rounds threefold and cannot refuse them — that, not the round count, is why a static reduction was needed on top of a working gate, and a size-aware first estimate is what would retire the reduction entirely. And chunk retirement can only begin at round three, so under a three-round cap only one round can ever shrink: the arithmetic that justifies the cap is an arithmetic the cap guarantees stays true. * fix(review): correct the claims this stack got wrong, and cover its untested seams Round-4 review of the three stacked changes, all of it comment-and-coverage rather than behaviour. Grouped by what was actually wrong. **A doc comment detached from what it documents.** The context interface was inserted between `reviewBudget`'s doc block and the function, so the whole block — including the input-laundering contract that opens it — attached to the interface and the function was left undocumented. The interface moves up beside its sibling with its own doc; the function keeps its contract. Its new paragraph also called both context fields environment values, when only one is — the other is resolved from settings. **Four test comments that argued for their assertions with false reasons.** The assertions were right and the rationales were not, which in a codebase whose comments are its design documentation is the more durable error. The integer guard is tested before the floor, not after, so the fractional value cited says nothing about what the floor would have caught alone. Not every value in the coercible-garbage list becomes zero — a negative stays negative and two strings become real counts, one of them large enough to land on the huge tier rather than the fallback. A single global bound of ten honours only two of the three clamp assertions, not all three; the third is an edge case, not a discriminator. And one comment justified its cases by naming a writer that does not exist on that branch. **A guarantee that is not guaranteed.** The settings fallback was documented as always degrading toward more work; that holds for three of the four fields and not for effort, where losing an operator's `high` returns the built-in rule and a local review drops to medium — less work, not more. Named as the exception it is, which is also why the fallback discloses on stderr. **Five prose sites that never learned the cap is conditional.** The step's stop rule, the clock module's own header, the large-diff cost model, the setting's schema description and the user-facing feature doc all still enumerated the cap unconditionally, contradicting the change one file away. The schema description additionally advised setting a deadline "to let a productive loop run longer", which is backwards: on a huge diff a deadline lowers the cap from five to three. **Two prose sites that overstated scope.** The setting is honoured from three operator scopes, not user only, and a value below the floor is ignored rather than clamped to it; and it cuts the cap for high-effort reviews only, since medium skips the reverse audit and low runs none. A cross-reference named a heading that does not exist. **The two seams nothing covered.** The environment boundary — whether this run has a deadline — was never exercised with the variable actually set: the budget tests pass the flag as a literal and the gate tests delete it. The operator ceiling's write path was likewise untested end to end; every builder call in the unit tests passes it as absent. Both now run through the real capture handler against a real environment and a mocked settings source, and both were mutation-checked: inverting the environment predicate and ignoring the ceiling each turn the new tests red. * fix(review): finish propagating the clock, and close the seams round 5 found Round-5 review of the stack. One real defect, one unsafe writer, and the rest is narration that stopped short plus coverage that could not see the change it was meant to cover. **A mock that leaked into every test after it.** The handler test added last round set an operator ceiling on a module-level settings mock and never restored it, so every later test in that file — including the whole trailing block — ran the real handler with an undeclared ceiling of twenty in play. Inert today because nothing downstream asserts on it, which is exactly how it would have survived to matter. Reset in the file's `beforeEach`. **The degrade announced itself with a writer that can destroy the degrade.** The settings fallback wrote its NOTE through the throwing stderr helper, whose safe sibling exists for precisely this case and says so in its own docstring: the write is incidental to the work in hand, and failing it would take down the fallback the guard exists to provide. Switched, with the reason recorded. **Five narration sites still stated the huge reduction unconditionally** — the field doc, the cap reader's own bullet, the enforcement-site comment above the round gate, the skill's version-skew paragraph, and the design note's "what an operator means by let it run longer". The last one was the most wrong: it named a deadline as the way to say "my ceiling is larger than six hours", but the check is for a deadline's *presence*, so any deadline however generous reads as three, and the cap is evaluated before the deadline arithmetic — setting one lowers the cap rather than raising it. Saying that would need the tier to read the deadline's size, which is now written down as the missing capability rather than implied to exist. **The setting was documented everywhere except the reference table.** The hand-maintained settings reference lists the review section exhaustively and did not list this one; the generator only writes the JSON schema, so it does not self-heal. Its schema description also never mentioned that only whole numbers are honoured, which matters because JSON Schema has no integer type here — a fraction validates in an editor and is discarded at runtime. **Two coverage gaps where the change was invisible to mutation.** Every gate test used the unsized fixture, whose tier is the fallback whatever the clock says, or forced a cap by storing one — so hardcoding the clock argument at all four call sites left the suite green. A sized huge plan is the only shape where the flag decides anything, and it now drives the gate on both sides. And two of the three capture commands had no budget assertion at all, so dropping either context field from their call sites compiled clean; the local one now asserts both. Both new tests were mutation-checked against exactly those edits.	2026-08-16 03:26:59 +00:00
Shaojin Wen	dc7e234876	feat(review): absorb prose gh commands into platform-backed subcommands (#9096) ... * docs(design): /review platform provider abstraction (GitHub + Aone Code) * feat(review): absorb prose gh commands into platform-backed subcommands The skill prose and agent briefs carried raw gh commands for the model to execute (repo resolution, head-SHA fetches, issue evidence, lightweight diffs, truncated-body refetches) — the prose-carried class that keeps shipping parsing bugs and drops the Enterprise host unless a prose rule remembers GH_HOST. Four new subcommands absorb them, built on a review-platform reader seam (lib/platform) whose first provider is GitHub over lib/gh.ts: - meta: repo identity + live headSha/webUrl (was gh repo view / gh pr view) - issue-context: closing-issue evidence file for Agent 0, incl. cross-repo issues and --issue for referenced-but-unlinked targets - fetch-diff: lightweight-mode diff to file (was gh pr diff redirects) - comment-body: one comment body by kind; pr-context truncation notes now name this command (with --host baked in) instead of a gh api route SKILL.md, the Agent 0 brief, and the role-0 generated prompt no longer contain model-executed gh calls; the GH_HOST prefixing prose rule is gone. * test(review): pin the bare-number host source as review meta Step 1 now derives a bare PR number's owner/repo/host with the meta subcommand instead of a prose gh repo view; the pin follows. * fix(review): address PR #9096 review findings Critical: - tests: resolve() expectations on Windows-asserted --out paths - issue-context: same-repo-keyed closing/extra dedup, extras self-dedup, and a failed single-issue fetch degrades to an explicit section instead of aborting the whole evidence file - meta: apply the URL-discovered host to gh routing before the PR call, validate --repo without requiring a number, usage errors exit 2 - agent-prompt: shellQuotePath the welded --out evidence path; plan-diff gains --host so a lightweight run welds it into the Agent 0 command - lib/gh: ghRaw (no trim) for diff/comment-body payloads whose edges are content; resolveGhHost normalizes an empty --host flag - SKILL.md: restore the constructable Posted:-link fallback; scope the no-model-run-gh-calls claim (Step 4 scratch-repo carve-out named) - issue-context: actionable error when gh < 2.72.0 lacks closingIssuesReferences Suggestions: drop dead host fields from run-function arg interfaces, exit-2 consistency, pin the previously unpinned contracts (setGhHost ordering x4, buildMarkdown host baking, welded GHE command, mkdirSync guards, no-comments placeholder, GH_HOST save/restore, 422 meta pin), fetch-diff handler tests, ClosingIssueRef dead fields removed, design-doc corrections (D1 subset note, D2 cell, D7 amend-delta rule, testing strategy wording, carve-out exemption), code-review.md --out fix. * fix(review): address PR #9096 round-2 review findings Critical: - lib/gh: ghRaw now returns bytes untouched — the unconditional CRLF rewrite would strip blob-content \r from every hunk of a CRLF-file diff (heavy mode's raw-bytes policy; the justification comment was wrong) - SKILL.md: Step 7's head-SHA fallback meta call carries the Enterprise --host annotation like every sibling call site - SKILL.md: the render-adjudication carve-out runs in a verifier subagent's shell — the Enterprise note now says exported-GH_HOST only, otherwise adjudication is unavailable (a --host note here cannot reach the subagent); the GHE enumeration also names submit Suggestions: - agent-prompt welds the plan's pr/ownerRepo/host only after re-validation (the plan is a file on disk; compose-review already re-validates) - plan-diff validates --host against HOSTNAME_RE before recording it - pins: full emitted-command prefix at all three sites, setGhHost ordering now includes ensureAuthenticated (x4), ghRaw no-trim/no-rewrite, plan-diff host write side, closing-ref repository-less fallback, --issue handler wiring, comment-body --out JSON marker + malformed-repo exit 2 + usage-error preempts auth, meta cwd-branch flag precedence, buildMarkdown host baking for inline/issue kinds - agent-briefs: --issue extras fetch from the PR's own repo — disclosed - pr-context: fix the resolveGhHost comment (env host IS baked) - docs: design doc corrections (gh.ts not-unchanged note, plan-diff in the inventory + D8, Phase 1 is new-implementation-not-refactor note, carve-out row/phase-3 ownership), review DESIGN.md issue-fetch path * fix(review): address PR #9096 round-3 review findings Critical: - lib/gh: HOSTNAME_RE now requires an alphanumeric first char and REPO_SEGMENT rejects a leading dash — flag-shaped values (--help, -evil/repo) no longer pass validation only to be misparsed as CLI options downstream of the unquoted weld Suggestions: - agent-prompt weld: the plan re-validation (digit prNumber, isOwnerRepo, HOSTNAME_RE-gated host) is now pinned by tampered-plan tests - setGhHost trims once so raw and resolved --host inputs agree - all four subcommands validate --repo before the auth gate (usage error exit 2, never preempted by an auth failure), pinned with ensureAuthenticated-not-called assertions - issue-context: bodies render untrimmed (leading-indent log pastes keep their code block); closing/extra dedup compares repos case-insensitively; a failed closing-issue discovery degrades into a named section (with the gh >= 2.72.0 hint) while --issue extras still fetch; numeric args get positive-integer validation with exit 2 (also --issue, id, --pr) - agent-briefs: retry-once guidance extended — unfetchable sections mean re-run with --issue before declaring evidence unavailable - SKILL.md: Step 5's lightweight block no longer re-fetches the diff Step 1 already wrote (one fetch, no head-advance race); SKILL.test.ts gains the revert guard for the lightweight capture + host note - meta: env-GH_HOST label for explicit --repo pinned * fix(review): address PR #9096 round-4 review findings Critical: - agent-briefs: the retry rule no longer sends unfetchable CLOSING refs through --issue (extras resolve in the PR's own repo — a cross-repo closing number would fetch the same-numbered unrelated issue); a plain re-run is the retry, closing refs are re-fetched every run - fetch-diff: an empty PR diff writes a 0-byte file, not a one-blank-line file that plan-diff dies on with a coverage error instead of taking the designed empty-plan branch Suggestions: - setGhHost: only genuinely-absent input resets; a non-empty all-whitespace --host now fails validation instead of silently restoring the default - agent-prompt weld trims the plan host before re-validating (fetch-pr records the raw flag); pr-context validates the resolved host against HOSTNAME_RE before baking it into emitted refetch commands - empty --out is a usage error (exit 2) classified before any fetch, in comment-body/fetch-diff/issue-context; plan-diff's handler maps the new --host usage error to exit 2 instead of an uncaught crash - issue-context: extras section header no longer claims NOT-in-closing when the closing set is UNKNOWN (discovery failed) - SKILL.md: Step 1's lightweight item spells out the fetch-diff failure stop rule; the Enterprise enumeration now lists every --host subcommand (adds plan-diff, test-plan, publish-assets); code-review.md matches - design doc: D1 names the ensureAuthenticated gate; the D2 carve-out row describes the shipped behavior (exported-GH_HOST only), not a welded prefix that never existed - pins: full-wrapper assertions extended, numeric usage gates at all three remaining handlers, --pr success-path plumbing, --out JSON marker, setGhHost TypeError class + trim/whitespace behavior, ghRaw byte fidelity, unfetchable extras in the JSON, cross-repo ownerRepo in the JSON, untrimmed body rendering, extras-section absence, discovery-failed header wording, runPrContext-level host baking (flag + env + rejected alias), SKILL revert guards for Step 7's meta rewiring * fix(review): address PR #9096 round-5 review findings Critical: - fetch-pr records the TRIMMED host into the fetch report, so the two downstream readers that re-validate it (compose-review's plan identity, the agent-prompt weld) see the canonical form — a padded-but-valid GHE host no longer drops to github.com anchor links - a non-empty all-whitespace --host no longer silently falls through to the env/default in resolveGhHost (it is returned as '', not swallowed), and publish-assets validates the raw flag via setGhHost before resolving — the Contents-API write can no longer be retargeted at github.com by a whitespace-only flag; match-remote now fails closed (exit 6) on the same input instead of matching github.com Suggestions: - plan-diff: drop the doubled `plan-diff:` prefix from the two thrown TypeErrors (the handler prepends it once); reject a whitespace-only --host instead of dropping it from the plan - new shared assertWritableOutPath (lib/paths): empty/whitespace AND directory --out targets are classified as usage errors BEFORE any fetch in comment-body/fetch-diff/issue-context (the directory case previously died EISDIR after the fetches and exit-coded as a runtime failure) - resolveRepo fetches `parent` and prefers it when the resolved repo is a fork — gh's default-repo preference is a remote literally named `upstream`, not an API fork check, so an origin-only fork clone no longer targets a fork's same-numbered PR - scope the comment-body exit-2 comment to the handler-level guards (yargs -layer missing-arg / invalid-choice failures exit 1 — a known gap) - SKILL.test revert guards: rule-4 issue-context weld + absence of the pre-absorption `--json closingIssuesReferences` syntax; the 422 `commit_id` comparison clause and the `fetch-diff`-output rename; the Step 6 tail-fetch `--out` sentence and the Posted: fallback grounding; the lightweight-capture host note - pins: malformed-host handler exit-2 in fetch-diff/issue-context/meta; issue-context exit-1 auth branch; padded-host weld trim; pr-context setGhHost routing (flag + env); whitespace-only --out in all three; numeric-gate tests reset process.exitCode between invocations and add non-integer cases; plan-diff asserts the metacharacter host is never recorded into the plan * fix(review): address PR #9096 round-5 findings (meta host guard, plan-diff stderr) - meta's discovery branch validates the routed host against HOSTNAME_RE before setGhHost: a host gh tolerates but the subcommands reject (underscore intranet aliases, IPv6 literals) is an environmental condition, so it now names the actual source (--host flag vs discovered repo-URL host) and fails exit 1, never as a usage error blaming a flag the caller never passed - plan-diff's handler catch uses writeStderrLineSafe (a broken stderr must not let the throw escape and lose the exit-2/exit-1 classification) * fix(review): address PR #9096 round-6 Critical findings - lib/gh: split the byte/text raw modes. execGhWithRetry gains a mode ('default' | 'bytes' | 'text'); the bytes mode runs with encoding 'buffer' and decodes latin1, so a diff of a non-UTF-8 (Latin-1/Shift-JIS) file no longer loses every invalid byte to U+FFFD. ghRaw is the bytes mode (fetch-diff writes it back with latin1 — byte fidelity end to end); new ghRawText is UTF-8-with-edges-preserved, which comment-body uses (comment bodies are always valid UTF-8 from the API; the leading-indent code-block fidelity holds, but bytes are not corrupted into mojibake) - lib/gh: split the leading-dash ban per segment — owners cannot start with a hyphen but REPO names can (yezhaodan/-Git exists), so a leading dash on the repo half is no longer rejected (the ban only protected against the flag-shaped OWNER half anyway) - github resolveRepo: take the host from the resolved repo's OWN url — gh's `parent` field carries no url (only id/name/owner), so reading target.url crashed every origin-only fork clone with TypeError; the meta.test fork fixture now matches the real gh shape - publish-assets: the round-5 raw-flag validation guarded on `trim() !== ''`, which skipped exactly the whitespace-only host it exists to refuse — guard on presence instead so setGhHost(' ') throws the documented TypeError (exit-3 refusal, no silent Contents-API retarget at the env/default host) * fix(review): address PR #9096 round-6 gpt-5.6-sol Critical findings - agent-prompt weld: a present-but-invalid plan host now fails closed (throws) instead of being silently dropped to null — a tampered host can no longer quietly reroute the evidence fetch to github.com's same-named repo (a missing host stays optional) - comment-body: read `.body` off the JSON-parsed response instead of `--jq '.body // ""'` — the jq form appends a trailing newline (a body not ending in one gained a byte; an empty body became "\n"); JSON parse returns the exact bytes GitHub stores - issue-context: --issue now accepts `owner/repo#123` as well as `123`, so a referenced issue living in a DIFFERENT repo is fetched there instead of silently reading the PR repo's same-numbered unrelated issue; dedup is by (repo, number) pair, case-insensitively, which also fixes the cross-repo-closing-shadows-same-repo-extra edge uniformly - lib/gh: drop the now-unused ghRawText text mode (comment-body moved to the JSON parse) * fix(review): address PR #9096 round-7 review findings Critical: - R7-1: the Agent 0 brief, SKILL.md rule 4, and code-review.md still taught "issue-context cannot fetch a referenced issue in a different repo — declare it unavailable", contradicting the cross-repo `--issue owner/repo#123` capability shipped in round 6. All three carriers now teach the qualified form, and the wrong-issue warning / retry ban is narrowed to bare numbers (a qualified retry is a correct retry) Suggestions (all directly pin or harden this PR's changes): - agent-prompt weld fails closed on a present-but-NON-STRING host and on a present-but-whitespace-only host (both were silently dropped to null, rerouting the evidence fetch), matching the sibling identity fields - gh.test.ts: the ghRaw byte-fidelity test now returns a real Buffer with an invalid-UTF-8 byte (0xE9) — the latin1 decode genuinely executes (the previous string mock made String.prototype.toString an identity call) - meta: the explicit-`--repo` branch gates the emitted host with HOSTNAME_RE, same as the discovery branch (an unroutable GH_HOST env value no longer emits a host label every sibling rejects) - publish-assets: pin the round-6 whitespace-host refusal (exit 3, no gh call, `(from --host)` in stderr) - issue-context: pin the documented `--issue owner/repo#n` grammar end to end through the handler regex - code-review.md: the GHE `--host` enumeration adds match-remote (the pipeline's first host-sensitive step) * fix(review): address PR #9096 round-8 review findings Critical: - R8-1: the round-7 non-string-host guard threw on `host: null` — which fetch-pr writes unconditionally into every same-repo github.com plan (`args.host?.trim() || null`), so every ordinary review would have failed at the roster build. null is now tolerated (only a present non-null non-string host throws); regression test added - R8-2: comment-body validates `--kind` is a single admitted token before any platform call — a duplicated `--kind` arrives as an array that passes yargs' element-wise choices, and String() would coerce it to 'review,inline' into the wrong API collection Suggestions: - assertWritableOutPath rejects a trailing-separator --out (the POSIX directory spelling that resolve() normalizes away) - comment-body prints the body via process.stdout.write (byte-exact, no invented trailing newline) - agent-prompt weld prNumber guard strengthened (rejects 0 and unsafe integers, matching the welded handler's contract) - pins: meta explicit-branch HOSTNAME_RE gate, isOwnerRepo dash asymmetry both directions, ghRaw retry with buffer stderr, whitespace-only and null plan hosts, issue-context qualified-grammar rejection side, corrected the misleading case-insensitive dedup test, fs mocks no longer consult ambient /tmp state (existsSync/statSync overridden) - R8-13 (extras-header double-render assertion) deferred to #9194 per the reviewer's own note	2026-08-15 09:16:48 +00:00
qqqys	8517fa9d47	feat(web-shell): redesign Channel policy and workspace management (#8848) ... * feat(web-shell): expose channel access policies * test(cli): cover shared Channel management fields * feat(web-shell): clarify channel policy controls * feat(web-shell): select channel workspace * feat(web-shell): redesign channel management * fix(web-shell): align channel manager with shell tabs * fix(web-shell): prioritize conversation settings * fix(web-shell): preserve legacy channel defaults * fix(channels): address management review blockers * fix(channels): address editor review blockers * fix(channels): preserve workspace action and route state * fix(web-shell): prevent stale channel editor state * fix(channels): preserve stored group settings * fix(channels): preserve group behavior settings * fix(web-shell): reset channel workspace UI state * test(web-shell): assert restored channel scope * fix(web-shell): preserve legacy channel scope * fix(web-shell): preserve inherited channel defaults * fix(channels): preserve compatible legacy settings * fix(web-shell): keep workspace navigation available * fix(channels): default new channels to pairing --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com>	2026-08-14 07:41:44 +00:00
Shaojin Wen	97ec96ec54	feat(cli): Add review settings for attribution, default effort, and default comment (#8994) ... * feat(cli): Add review settings for attribution, default effort, and default comment * fix(cli): resolve review settings from operator scopes and close gate gaps (#8994) Address review feedback on the review settings: - Resolve review.attribution/effort/comment from operator-controlled settings scopes only (system defaults, user, system); a repository's .qwen/settings.json is content under review and must not control whether findings publish, whether the review names its model, or how deeply the pipeline verifies. - Normalize the configured review.effort through the same case- insensitive validation as the --effort flag, so "Low" cannot miss the exact comparisons the forcings run and invalid values cannot leak into the verdict. - Gate the modelId requirement and footer-safety validation on attribution: with the footer gated off, the field has no consumer and must not refuse the run. - Pass the standing review.comment setting into publish-assets' call of the shared authorisation gate, so both callers agree on what authorises a run. - Make presubmit's self-comment detection footer-independent by also matching the reviewing account's own top-level comments, so attribution-off posts still dedup. - Align SKILL.md's Step 7 gate and every --comment branch on comment.effective, and add handler-level wiring tests for all configured defaults. * test(cli): pin the review-settings operator defaults with unit tests (#8994) * fix(cli): share the guarded footer strip and pin the gate audit text (#8994) * fix(cli): raise the repository-context array bound to 256 (#8994) * fix(cli): validate review setting values and tighten the review gates (#8994) * fix(cli): align presubmit dedup with severityOf and normalize auto effort (#8994) * fix(cli): show the review settings in the settings dialog (#8994) * fix(cli): bound the footer strip tail and match refusal advice to its class (#8994) --------- Co-authored-by: qwen-code-autofix[bot] <qwen-code-autofix[bot]@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com>	2026-08-13 13:27:39 +00:00
易良	8858d4340b	feat(cli): add native multi-agent coordination (#8804) ... * feat(core): add native multi-agent coordination * feat(cli): add agent view pty workers * fix(cli): harden agent view pty workers * fix(cli): harden agent view pty host teardown * fix(cli): fail fast on pty host auth rejection * test(cli): cover pty host remote exit polling * fix(cli): address agent view pty worker review nits * fix(cli): harden pty host socket fallback * fix(cli): harden agent view pty workers * fix(cli): harden agent view pty workers * test(cli): cover pty host spawn contract * fix(cli): guard agent view pty host socket takeover and races * fix(cli): fit agent view pty logs under wire cap, strip host token after merge * feat(cli): manage agent view session lifecycle * fix(cli): preserve agent view lifecycle state * fix(cli): harden agent view lifecycle persistence * fix(cli): harden agent view lifecycle * fix(cli): harden agent view lifecycle recovery * feat(cli): expose agent view commands * fix(cli): wire agent view command safeguards * fix(cli): resolve agent view command integration * feat(cli): add agent view roster ui * feat(cli): add durable multi-agent coordination * fix(cli): harden multi-agent coordination * fix(cli): complete native coordination flows * fix(cli): wait for agent view host cold starts * fix(cli): allow coordination startup time * fix(cli): persist missing coordination results * fix(cli): enforce exact Agent View answers * refactor(cli): reuse existing agent team coordination * docs(cli): clarify homogeneous coordination * feat(core): complete native team coordination * fix(agents): enforce teammate coordination boundaries * fix(agents): close teammate lifecycle gaps * fix(agents): align empty teammate names with routing * fix(core): close coordinator review gaps * fix(core): keep read-only teammates off writer tasks --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-12 17:39:03 +00:00
jinye	4980a2c20d	fix(cli): bound headless tool result content (#9012) ... Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-12 16:22:36 +00:00
易良	70672f8fb8	fix(cli): avoid duplicate context usage in footer and status line (#8749) ... * fix(cli): avoid duplicate context usage in footer and status line The built-in default status line preset includes `context-used`, and the footer renders its own context indicator unless `hideContextIndicator` is set, so context usage was shown twice out of the box. Treat `ui.statusLine.hideContextIndicator` as tri-state: an explicit boolean still wins in both directions, and when it is unset a preset status line containing `context-used` or `context-remaining` hides the footer indicator. Command status lines are unchanged — their output is opaque, so it is never inspected for context information. Fixes #8695 * fix(cli): preserve status line context override * fix(cli): preserve status line context semantics * fix(cli): keep context visible in narrow footers * fix(cli): keep context visible when status line clips * fix(cli): preserve context indicator visibility * fix(cli): match status line wrap layout	2026-08-11 07:18:48 +00:00
Shaojin Wen	af372e5a21	perf(review): guarantee compose survives a reverse-audit budget stop (#8791) ... * perf(review): guarantee compose survives a reverse-audit budget stop PR #8687 — a 4,269-line cross-worktree git guard — timed out after six hours and posted nothing, holding ~20 E2E-confirmed Critical bypasses. The deadline gate worked: it refused round 3 correctly with ~110 minutes and the whole reserve in hand. The tail after the stop was the killer — a single hand-rolled verification agent re-running a 15-family shell/git bypass battery with real filesystem E2E consumed all of it, and the wall hit mid-verification before compose-review ever ran. The reserve was one number covering "verification + compose + submit", which is right for a normal per-finding re-trace but wrong for a security PR where verification cost is unbounded (real E2E per finding) while compose and submit stay bounded. So a distinct, smaller compose FLOOR is carved out and the VERIFIER — not the reverse-audit builder — is gated on it: below the floor `agent-prompt --role verify` refuses to build (VERIFY BUDGET, exit 4), the findings keep their `— [unverified]` tag for compose-review to cap, and compose runs. The floor is strictly below the reserve, so a healthy run reaches the reverse-audit gate first and never sees it; it is the cover for the one span the reserve cannot bound. The prose closes the bypass the gate cannot see: the post-stop tail verifies only through the gated builder, never a hand-rolled agent, and invents no fresh re-verification pass for findings already confirmed — compose and submit are non-negotiable. DESIGN.md records the incident; the RA budget message and SKILL Step 5 tail are rewritten to match. * fix(review): close the round-1 gaps in the compose-floor gate - R2-2 (Critical): the documented `0` escape hatch did not disable the verify gate past the deadline — `remainingSeconds` goes negative there and `negative >= 0` is false, firing the supposedly-disabled gate. verifyBudgetExhausted now returns null the moment the effective floor is 0, before the comparison. Pinned with a past-deadline case. - R2-1 (Critical): the gate bounds prompt CONSTRUCTION, not the wall time of an already-admitted verifier that then runs a long E2E past the floor — and agent-prompt builds prompts, it cannot cancel a running agent. The SKILL tail now tells the orchestrator to bound the WAIT: when the deadline is within the compose floor and a verifier batch has not returned, stop waiting on it, keep its findings unverified, and compose. The remaining execution-time cancellation is a harness capability, noted as such (same layer boundary as the hand-rolled-agent caveat). - R2-3: the agent-prompt exit-code help now documents both the BUDGET and VERIFY BUDGET exit-4 refusals. - R2-4: the reverseAuditBudgetMessage test now pins the new tail rules (gated verifier only, no hand-rolled agent, no re-verification). - R2-5: docs/users/features/code-review.md documents the compose floor — default, env var, reserve nesting, exit-4 behaviour, zero hatch. * fix(review): round-2 fixes for the compose-floor gate - R3-1 (Critical): the verify gate admitted at exactly the floor, where the first work crosses below it — the floor is compose-only with no margin, so it now refuses at equality (`> floor`, unlike the RA reserve which admits at exact cover). Exact-boundary test flipped. - R3-2 (Critical): the refusal message and SKILL claimed unverified findings "post as needing human review", but the confirmed-only rule keeps tagged details terminal-only. Reworded to the true contract: compose-review caps the verdict and discloses the verification gap; the tagged details stay terminal-only; what posts is the earlier rounds' confirmed findings plus that gap. - R3-5: extracted readDeadlineSeconds / readNonNegativeSeconds, shared by both gates so the fail-open contract lives in one place. - R3-3: pinned the verify gate's fail-open branches (malformed/non-positive deadline, past-deadline negative remaining, negative-floor fallback). - R3-4: pinned the floor-minutes rendering (a field swap to remainingSeconds would misstate the protected floor). - R3-7: pinned that a refused verifier writes no budget-stop marker and no admission stamp. - R3-8: pinned validation-before-gate (a malformed verify call under the floor throws, not exit 4). R3-6 needs no change: the SKILL.test pointer<->heading gate already covers the DESIGN section (a dangling pointer fails it). * fix(review): round-3 cheap fixes for the compose-floor gate Low-risk corrections; the two edge-case Criticals (R4-1 broken-plan masking, shared with the RA gate; R4-2 compose-review relaunch FIX) are left as follow-ups — noted on the threads. - R4-4: the readDeadlineSeconds extraction stranded reverseAuditBudgetExhausted's contract JSDoc above the helper; moved it back onto the function. - R4-5: the round-2 "terminal-only, never posted" wording contradicted compose-review's own verdict line ("posted, disclosed as unverified") — a pre-existing contract ambiguity this PR should not relitigate. Reworded the message and SKILL to the invariant both readings share: an unverified finding is never treated as a confirmed blocker; the verdict is capped. - R4-7: "below the N-minute floor" contradicted the exact-equality refusal (the gate admits on `> floor`); now "at or below", in the message and the user docs. - R4-3: pinned that a blank/whitespace floor override falls back to the default (only explicit 0 disables). - R4-6: pinned the negative-remaining clamp in verifyBudgetMessage. --------- Co-authored-by: verify <verify@local>	2026-08-10 02:50:45 +00:00
qqqys	88a325bce9	feat(workflows): add cooperative pause and resume (#8320) ... * feat(workflows): add cooperative pause and resume * fix(workflows): restrict pause to background runs * fix(cli): clarify foreground workflow pause errors * fix(core): preserve dispatch errors across cancellation * test(core): cover late workflow state callbacks * fix(workflows): address review suggestions (#8320) - Rename misleading `terminal` local to `presentation` in BackgroundTasksDialog - Fix vacuous `toContain('p')` assertion to `toContain('Background tasks + p')` - Fix vacuous gate assertion with macrotask yield in scheduler test - Add over-count cap test for `onAgentCompleted` past dispatched count - Add pausing-state approval parking test - Remove dead `concurrencyLimiter` module (no production consumers) * test(workflows): pin review-flagged mutation-surviving branches (#8320) * test(cli): use valid agent status in detail-view reset test (#8320) * test(workflows): harden pause-gate settle probes with a full flush (#8320) * fix(workflows): address round-5 review findings (#8320) * test(ci): sync review timeout assertions with repository variables (#8320) * fix(workflows): address round-6 review findings (#8320) * fix(workflows): address round-7 review findings (#8320) * fix(workflows): address round-8 review findings (#8320) * fix(workflows): address round-9 review findings (#8320) * fix(workflows): address round-10 review findings (#8320) * fix(workflows): address round-11 review findings (#8320) * fix(workflows): address round-12 review findings (#8320) * fix(workflows): address round-13 review findings (#8320) * fix(workflows): address round-14 review findings (#8320) * fix(workflows): address round-15 review findings (#8320) --------- Co-authored-by: qwen-code-dev-bot <269191875+qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-08 04:21:21 +00:00
destire-mio	b34a08d16f	fix(core): separate hook context from transcript display (#7948) ... * fix(core): separate hook context from transcript display * test(ci): gate desktop transcript projection * revert: keep desktop CI scope unchanged * test: cover transcript display fallbacks * fix(transcript): address review feedback * fix(transcript): reconcile post-merge provenance paths * fix(webui): preserve legacy transcript concatenation * test(transcript): cover projection consumers * fix(transcript): consolidate hook context projection * fix(transcript): support single-field display provenance * fix(transcript): strip hook context with invalid metadata * test(acp): cover empty replay display text --------- Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com> 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-08-08 02:03:20 +00:00
jinye	26352fcc6a	feat(external-context): Add optional Mem0 memory writes (#8507) ... * feat(external-context): Add optional Mem0 memory writes Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(hooks): Preserve confirmation content visibility Render PreToolUse confirmation reasons literally and keep long confirmations accessible through the virtualized TUI. Add unit and interactive regression coverage for Mem0 write confirmations. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(external-context): Address memory write review findings Align Hook and MCP argument handling, distinguish definitive Provider rejections from ambiguous outcomes, improve deployment diagnostics, and document the write-back trust boundary. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(hooks): Refine plain-text confirmations Render URLs consistently, avoid persistent virtual viewport gaps, and document the literal-rendering and managed deployment boundaries. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(external-context): Support Auto Edit write confirmation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(external-context): Harden write confirmations Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): Measure virtual row height directly Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): Preserve YOLO Hook confirmation content 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> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-08-07 16:52:33 +00:00
BaboBen	028747aa41	feat(feishu): enrich observed contact labels (#8569) ... * docs: design feishu observed contact enrichment * docs: add Chinese Feishu enrichment design * feat(feishu): enrich observed contact labels * fix(feishu): preserve enriched contact labels * fix(feishu): harden observed-contact label enrichment lifecycle * fix(feishu): bound label caches, honor observation recency, silence enrichment token failures (#8569) - hydrate runtime label caches from the newest observation per contact so stale group membership labels cannot overwrite more recent ones - cap the user/chat label, in-flight lookup, and write-dedup maps at 500 entries (matching the persisted registry) and evict oldest entries - route best-effort label lookups through a silent token refresh path so enrichment failures no longer write to stderr - add tests for silent token refresh, newest-label hydration, cache cap, and the persisted-observation reject path in hook ordering * fix(feishu): address observed-contact label review feedback (#8569) * Track core (non-silent) waiters on the shared tenant-token refresh so a silent-initiated refresh still logs token errors for joined delivery callers. * Short-circuit label lookups on the resolved names cache so evicted lookup entries do not trigger redundant API requests. * Re-hydrate label caches from the persisted registry after an in-lifetime cache eviction so the next initial write cannot clobber a persisted label with the raw ID. * Add mutation-proof regression tests for the channel-isolation filter, the list-failure swallow, the silent HTTP-error branch, and the 'unknown' label guard. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com>	2026-08-07 16:41:37 +00:00
BaboBen	edb420393e	fix(channels): manage DingTalk interactive card config (#8517) ... * fix(channels): manage DingTalk interactive card config * test(cli): cover nested channel object validation * fix(channels): harden nested management metadata * fix(channels): isolate invalid management descriptors * fix(channels): isolate invalid management metadata from channel runtime * fix(channels): reject reserved unknown keys in management config upserts * fix(channels): harden channel management validation and editor checks Reject management descriptors that lack a fields array at registration so broken plugins are stripped to unmanageable instead of being advertised as manageable and failing every upsert with an unmapped TypeError. Reserve the top-level "type" field key and require enum fields to declare at least one option, both of which the settings store could never accept. Treat whitespace-only number drafts as empty in the channel editor, consistent with the module's other emptiness checks. Also give the SDK descriptor mirror test a runtime wire-shape walk over the built-in catalog, add the parser's timeout rejection boundary, and restore the exact built-in catalog membership assertion. * fix(channels): validate management field shapes and editor bounds (#8517) * fix(channels): align management validation layers and pin gate behavior (#8517) Read envResolvable by truthiness in the settings store so it matches the registration gate and the editor, instead of rejecting the advertised environment references of untyped plugins. Fail closed at registration on non-finite exclusiveMinimum values, empty object property lists, and async validateConfig functions, all of which would otherwise advertise a field or save path that can never succeed. Strip invalid management metadata over a prototype-preserving copy so class-instance plugins keep their createChannel implementation. Move the unchanged-value preservation exemption ahead of the object shape rejection so a stored non-record value (for example a hand-written null) no longer locks every unrelated management edit of that channel. Clamp DingTalk question-card timeouts at the maximum setTimeout delay, since Node treats larger delays as one millisecond and would expire cards instantly. Pin the previously untested load-bearing behaviors: per-key previous threading in the recursive validation, the preservation exemption's precedence over nested required enforcement, nested "type" properties, depth-2 nesting rules, and the nested-only constraints of the daemon descriptor wire contract. * fix(channels): close reserved-key preservation gaps and pin gate behavior (#8517) * test(cli): tolerate IPv6-less hosts in serve ::1 bind tests (#8517) The self-hosted CI containers can have no IPv6 loopback, where the two runQwenServe tests that bind ::1 fail with EADDRNOTAVAIL. Probe the interfaces once and skip only the IPv6-dependent binds there; every assertion still runs on IPv6-capable hosts. * fix(channels): align descriptor type contracts with runtime validation (#8517) The registry already rejects object fields without a non-empty properties array and enums without unique options, but the descriptor types still admitted both, so TS-authored plugins only learned about it when registration stripped their management surface. Make `properties` required, give enums a dedicated descriptor member with required `options`, and drop the never-honored `envResolvable` flag from number descriptors, in both channel-base and the SDK mirror, and export the descriptor sub-types through the webui barrels. Also map a throwing `validateConfig` to the usual invalid-config error and pin the store contracts that had no distinguishing tests: omitting a parent object drops the stored object without checking its nested required, writes replace nested values wholesale, unchanged stored scalars are still re-validated, and valid plugins register by original reference. * fix(channels): defuse validateConfig rejection leak and close descriptor gate gaps (#8517) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-07 15:47:20 +00:00
qqqys	3edecac116	feat(channels): support group pairing (#8440) ... * feat(channels): support group pairing * fix(channels): address group pairing review * fix(web-shell): show group pairing management * fix(channels): recheck group pairing before history backfill * test(channels): verify group approval isolation * fix(channels): address group pairing review findings and pin behaviors (#8440) - Grandfather the group allowlist file in PairingStore legacy migration - Offer the pairing groupPolicy option in github/gitlab descriptors - Re-export DaemonChannelPairingSubject from the webui barrels - Refresh the GroupGate doc comment and channel docs rows - Pin the unpinned group pairing behaviors called out in review: subject dedup, trigger matrix, notification content/cap/failure/ thread routing, DM negative space under groupPolicy pairing, stored DM loop authz, pairing-enabled guard negative space, approval/revocation HTTP bodies, descriptor-driven gate branch, and the web-shell group approval mirrors - Add a compile-time assertion for the revocation request union * fix(channels): address group pairing review findings (#8440) - Accept 'pairing' in the GitLab connect warning, descriptor help text, and gitlab.md: todos dispatch after one-time group approval. - Model group approvals in the web-shell e2e mock daemon (approve by subject type, GET returns senderIds+groupIds, DELETE accepts groupId) and exercise the group pairing flow in the channels spec. - Add 'pairing' to the groupPolicy enumerations in the plugins and per-channel docs (telegram, feishu, dingtalk, qqbot, wecom). - Update the channel pairing CLI help to cover group requests. - Cap pending pairing requests at one per sender so a single member cannot occupy every shared pending slot. * fix(channels): address group pairing review findings round 7 (#8440) * fix(channels): address group pairing review findings round 8 (#8440) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(channels): address group pairing review findings round 9 (#8440) --------- Co-authored-by: qqqys <266654365+qqqys@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.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> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-08-07 09:20:18 +00:00
Shaojin Wen	7edc16ba11	feat(review): say so when the bundle is older than the review it runs (#8390) ... * feat(review): say so when the bundle is older than the review it runs Every `qwen review …` step runs the BUILT bundle, not the working tree. So editing a review command, or switching to a branch that contains one, changes nothing about the run until someone rebuilds -- and the failure is silent and total: the run behaves like the last build, and every conclusion drawn from it is a conclusion about that build. Measured on 2026-08-02, dogfooding /review against #8368 from a checkout whose bundle was fourteen hours old. Three things were invalidated at once and none announced itself: `drive` and `mock-provider` had merged that morning and were absent from the binary, so "the agent never reached for them" measured nothing; and #8345's guard against scoring a mutant `survived` when its own collocated test was red had merged too, so the run reproduced the bug it fixed and filed three findings the current code holds as `inconclusive`. The round was discarded and re-run after a rebuild. `parse-args` is the first command of every review, which makes it the only place a notice reaches a reader before they act on a result. It names the file that is ahead, by how much, what actually runs from the bundle, and the command to rebuild -- "rebuild" without evidence is advice nobody can check. mtime, not git: the question is whether this bundle was built from this source, and a git comparison answers a different one. A margin absorbs a checkout, which writes everything at once in no guaranteed order. An installed package has no sources beside it, finds nothing to compare, and stays silent -- a check that cannot see the files must not accuse the build. Also documents `findings --test-delta` for users: it can lower a severity, and therefore change what the verdict is computed from, so it belongs beside `--outcomes` rather than only in the skill. * fix(review): watch the file every subcommand is registered in `packages/cli/src/commands/review.ts` is where all 30-odd subcommands are imported and registered, and it sits beside the directory rather than in it -- so a new command, or a changed dispatch, was exactly the change this check could not see. A root may now be a single file, which is what that one is. Confirmed end to end: with `review.ts` three hours ahead of a fresh bundle, the warning names it. Also two comments that did not match the code: symlinks of every kind are skipped, not only directories (`isFile()` is false for a symlinked file too), and the module now says what `QWEN_CODE_CLI` already covers -- talking to a different program -- so it is clear this guards the other half, the right program built before the change. * fix(review): compare content, because a timestamp check cried wolf The first version compared the bundle's mtime against the newest review source, and it was wrong in the direction that matters most. `git checkout` rewrites every file that differs between two commits, so returning to the branch a bundle was built from re-stamps exactly those files and the check calls a byte-for-byte correct bundle stale. Measured: with the sources untouched and the bundle two minutes older, it warned. A line that fires when nothing is wrong teaches its reader to skip the line, which would have made this worse than absent. The build now stamps a digest of the review sources it bundled into `dist/review-sources.sha256`, and the check re-derives that digest from the tree and compares. No margin to tune, no clock to trust, and no answer but the true one. Verified end to end across all five cases: a clean tree is silent, a source touched but unchanged is silent, and a real change under any of the three roots -- the command directory, the `review.ts` that registers them, the bundled skill -- warns. The digest is now one rule stated twice, since the build script cannot import the package it runs before building. `scripts/tests/review-source-digest.test.ts` holds the two equal, on this repo and on a synthetic tree that exercises the file-shaped root; a package test may not reach into `scripts/`, so it lives on the side of the boundary that may. Paths are folded relative to the repo root with separators normalised, and the file list is sorted -- `readdir` order is a property of the filesystem, so without it a bundle built in CI and a tree cloned locally would hash the same source differently and every run would warn. * fix(review): a diagnostic must not kill the run, and tests are not the bundle Two Criticals and five suggestions from review, all verified before changing anything. `writeStderrLine` throws on EPIPE, so stderr piped to `head` would have killed the review before it parsed a single argument -- a warning that destroys the run it was warning about, and the opposite of this change's own invariant. `writeStderrLineSafe` is the convention for diagnostics in this subsystem and is what it calls now. `reviewSourceRoots` builds paths with the platform `join`, and the test asserted forward-slash literals, so all three elements would have failed on the merge queue's Windows leg -- which the pull_request event never runs, so the green CI here proved nothing about it. Test files left the digest. esbuild follows imports from the CLI entry and no test is reachable that way, so folding them in fired the warning for an edit that cannot change a byte of the bundle -- the false positive this module already rejected once. 112 files became 61, and a test-only edit is now silent while a production one still warns. The handler wiring is tested at last, against a real temp tree rather than a mock of the reads under test: the derivation from `process.argv[1]`, the stamp read, and the warning. All three mutations the review named -- dropping the call, reading the stamp from the wrong directory, collapsing repoRoot to distDir -- now redden it. Also: the stamp's filename is pinned across the boundary it crosses (the build wrote a literal while the check read `DIGEST_FILE`, so a one-sided rename would have silenced the feature with every test green); the digest is computed only when there is a stamp to compare it against, instead of hashing a hundred files for a value the first guard discards; the `rebuildCommand` parameter no caller ever set is gone; and the build script's comment no longer claims a code-sharing relationship that does not exist. * fix(review): fixtures are not in the bundle either The same false positive, a third time and one directory over. Excluding tests from the digest was right and incomplete: `review/__fixtures__` holds four files — three responder modules and a captured comment — that a test loads at runtime, from no import the bundler follows. Measured against `dist`: none of the four appears in it, so editing one changed the digest while the bundle stayed byte-identical and the warning claimed a review command had changed. Both walks skip the directory now, and the parity test's synthetic tree grows a fixture and a `.spec.tsx` so the two implementations are held equal on the whole exclusion, not just the part the first case exercised. Reverting one side reddens the local case AND both parity cases, which is what that guard is for. Verified the other direction too, since an exclusion can overshoot: every review source that reaches `dist` is still covered. `DESIGN.md` and `SKILL.md` both ship and both remain in the digest — checked, not assumed, after two rounds of this exact mistake. Six cases end to end after a rebuild: a clean tree, a test edit and a fixture edit are silent; a production edit, a `review.ts` edit and a `DESIGN.md` edit each warn. * fix(review): allowlist the stamp, and stop guessing what the bundle holds The Critical first: `create-standalone-package.js` fails on any top-level dist entry outside its allowlist, and `review-sources.sha256` was on neither list. The next release would have aborted the standalone archive on all five targets, and no PR-time job runs the packager, which is why this suite is green. Allowlisted -- shipping it is harmless, since a standalone install has no `packages/` to compare against and the check stays silent there. `lib/test-utils.ts` was in the digest: test support with a production-looking name, imported by two test files and nothing else. That is the fourth patch to one rule -- `.test.ts`, then `__fixtures__/`, then this, plus `.DS_Store` -- and each was found by a reviewer after it shipped. So the rule stops being a list somebody remembers to extend: a new test asserts the property the list approximates, that every file the digest folds in is reachable from production code and nothing reachable is left out. Dropping `test-utils.ts` from the exclusion reddens it, which is the fifth instance failing in CI instead of in a review. Three branches that no test reached, each with a mutant the review measured surviving the whole suite: the walk's symlink skip (a directory cycle would send the first command of every review into unbounded recursion), the read-failure path (hashing the survivors of a concurrent checkout would accuse a tree that is merely mid-change), and the build's stamp call site (removing it left the scripts suite green while `npm run bundle` silently stopped writing the stamp). All three now redden. And `unmeasured` had no reader, so the one edge this check cannot measure but can see -- sources present, stamp absent -- passed in silence. That is the state of every existing checkout the moment this ships, and it is exactly the silent failure the change was written to end. It now says so, while an installed package, which has no sources either, still says nothing. * fix(review): the guard was shallower than the property it claimed The guard added last round asserts that every file in the digest is reachable from production code. It did not: a file imported by nothing passed, because the filter also required some test to import it; only `.ts` was inspected, so a test-only `.tsx` or `.mts` helper walked through; and it read static imports only, while this directory has nine `await import('./…')` edges. It asserts the property now — every extension, orphans included, dynamic edges seen — and the tree has no violators, so the strictness cost nothing today and is there for the next file. `__snapshots__` joins the exclusions. `vitest --update` regenerating a snapshot would have moved the digest with the bundle byte-identical; none exists under the review roots today only by chance, and 120 `toMatchSnapshot()` calls live elsewhere in this package. Three couplings that no test held: - the allowlist entry that fixed the release-breaking R2-1 -- reverting those five lines left the whole scripts suite green, and the next failure would have been a release aborting on all five targets. `isAllowedDistEntry` is exported and the stamp's own name is asserted against it, so a one-sided rename fails here instead; - the `.DS_Store` member of `NOT_BUNDLED_FILE`, absent from the repo and so from the parity tree -- one-sided removal stayed green while a macOS checkout would digest differently on the two sides forever; - each `unmeasured` reason. Swapping the two arguments at the single call site kept all 76 tests green while telling a pre-stamp checkout its sources were missing. And two comments that said the opposite of the code beneath them: the digest is computed unconditionally on purpose (the pre-stamp notice needs it), and `NOT_BUNDLED_FILE` helpers are deliberately not importers, since nothing reaches the bundle through a file the bundle does not contain. The two stderr diagnostics are documented for users, beside the sibling paragraph this PR already added. * fix(review): measure only the layout that can carry a stamp `npm start` launches `node <root>/packages/cli`, and node sets `argv[1]` to that directory -- so the derivation found sources under `<root>` with no stamp beside them and printed "could not check" on every review, forever, with advice that could never make it stop. That is the fires-when-nothing-is-wrong failure this change argues against, on the path `start.js` sets `QWEN_CODE_CLI` to precisely so reviews reach that build. Only a `<root>/dist/cli.js` layout is measured now; anything else has no stamp to find and no way to grow one. The build-side digest could kill `npm run bundle` where the check side degrades gracefully: a file vanishing mid-walk threw out of the hash loop, and the stamp is the copier's last step, so the build would fail with every asset already in place. Caught and skipped -- a missing stamp is `unmeasured`, which the runtime already treats as an acceptable answer. The skill now says what to do with the warning, which is the half that makes it reach a human: `parse-args` runs inside an agent's shell tool, the user reads the agent's summary rather than raw stderr, and a line nobody repeats is a line nobody sees -- which is how the 2026-08-02 round went wrong in the first place. It also records that the instruction cannot help the run that needs it, since the skill comes from the same bundle. And the scope is stated where silence could be over-read: the digest covers the review commands, the file that registers them, and the bundled skill -- not the shared helpers those import. A quiet run means the review code matches the bundle, not that the tree does. * fix(review): refuse to certify a bundle the copier may not describe The stamp described the tree as the COPIER saw it, and the copier runs after esbuild -- so a source edited in between, or `copy_bundle_assets.js` run on its own (it self-executes), wrote a digest certifying a `cli.js` built from something else. Silence then means "verified fresh" when it is not, and that is the only direction here where a quiet run is affirmatively wrong rather than merely uninformative: every other gap degrades to `unmeasured`. Timestamps are the wrong tool for judging staleness and the right one for judging whether this stamp can be honest at all, so the build refuses when any source is newer than the bundle it would attest to, and says why. Driven for real: touching a review source and running the copier alone now prints "skipped the source digest rather than certify a bundle it may not describe". `it('counts the same files')` compared nothing -- it asserted `> 50` on the build side while the check side exposes no count, so the title claimed a parity the body never checked, and the margin over the real 56 made it a future false alarm in `scripts/` for an unrelated change. Removed; the digest parity already holds the file set. "Root is a file" was inferred from `readdirSync` raising ENOTDIR, an assumption about every platform's libuv on the one root that is a file -- `review.ts`, where "a new subcommand was registered" lives. `statSync(root).isFile()` says it instead. And the check itself moves out of the handler into `bundleStalenessNotices`, which is where the rest of it already lived. `parse-args` is about parsing arguments again, the wording is testable without the yargs harness, and a second caller -- an agent resuming a review never runs step 1 -- is one line. * fix(review): align the twin walk, and stop a test from passing on nothing The build side still inferred "this root is a file" from `readdirSync` raising ENOTDIR, one commit after the check side stopped doing exactly that and said why. A platform that maps the case differently would drop `commands/review.ts` from one digest and not the other, and a byte-for-byte correct bundle would warn on every review forever, on that platform alone, with rebuilding reproducing the same one-sided walk. Both sides ask `statSync(...).isFile()` now. Fixing one half of a pair and not the other is the mistake this file keeps making. The filename parity test had been passing on nothing since the previous commit: it matched `writeFileSync(join(distDir, '…'))` against the script's source, the literal moved into a `stampPath` variable, and the regex returned `undefined` so the assertion compared against nothing. It runs the build against a fixture now and reads the name off `dist/`, so it measures what the build does instead of what its source looks like. Renaming the stamp on one side reddens it. Also from review: the duplicated comment block in `parse-args`; an unreadable source now says the check could not run rather than passing in the same silence as an installed package, which is what the docstring already promised; the "could not check" line no longer asserts that the checkout predates the feature, since the build has three refusal paths and one of them means the opposite; every refusal removes an existing stamp, because leaving an older attestation beside a newer bundle is a weaker form of the certifying it refuses; and `drive` calls the check, which the module comment argued for and the diff had not done -- a resumed review never runs step 1, and that is where the long work starts. * fix(review): pin the regex group the parity tree missed, and say source, not command * fix(review): allowlist what the bundle holds, and cover the drive notice (#8390) * fix(review): treat unreadable review sources as unmeasured (#8390) * test(review): pin the stamp guard mutations that survived the suite (#8390) * fix(review): close staleness-check gaps and pin the round-4 survivors (#8390) * fix(review): close round-5 staleness gaps for parity, refusals, and partial checkouts (#8390) * fix(review): close round-6 gaps in the clause classifier, symlink layout, and pin honesty (#8390) * fix(review): close round-7 gaps in the closure oracle, parity pin, and refusal pins (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-8 gaps from the maintainer review (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close round-9 nits from the maintainer review (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): pin the lease root in the synthetic digest parity case (#8390) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Autofix <autofix@qwen-code.dev> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-07 03:21:26 +00:00
qqqys	03eb5043cc	fix(dingtalk): keep status cards continuous and attributable (#8565) ... * fix(dingtalk): keep status cards continuous during runs * fix(dingtalk): render attributable markdown replies * fix(dingtalk): harden status card attribution and fallback paths (#8565) * fix(dingtalk): deliver boundary content reliably and harden card fallbacks (#8565) * fix(dingtalk): halt refreshes on dead cards, keep delivered content (#8565) The per-second status chain kept pushing metadata updates after the content stream latched failed, and once the 3-failure breaker tripped an idle card could never revive. Stop the chain on a latched stream failure and keep a low-frequency probe so a recovered metadata API revives it. Also re-send boundary content declared delivered via the card when a failed/cancelled terminal overwrites the continuity card, give a re-latched status context a fresh segment id after input_requested so a later failure still reaches the card failure UX, dedup inline sender echoes, and require the 'I' of IMAGE in the partial-marker regex so a bare trailing '[' survives in final fallback text. Consolidate the duplicated content-cap constants and pin the reviewed delivery, drain, breaker, and attribution behaviors with tests. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-06 22:42:42 +00:00
Shaojin Wen	e76dff1c6b	feat(review): add declarative repository-context manifest (#8401) ... * feat(review): add OpenJDK repository context Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * refactor(review): extract repository context foundation Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): repair CI type guard and add manifest repository context Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): harden repository context per maintainer review Address both maintainer reviews on the repository-context PR: - repo-context: a PR plan whose merge base never resolved (mergeBaseSha: null) now degrades to a null artifact without consulting the worktree, instead of throwing a misleading "invalid plan" error or falling back to the PR head. - Identity reads return the same shape in PR and local modes (CRLF->LF, trimmed) and fail closed: absence yields null, a present-but-unreadable file throws. - Context-required roles can no longer override the roster's effort, topology, and mode gates. - The relatedPaths scan bound rises from 1024 visited entries to 16384 and is documented, so honestly scoped manifests no longer abort reviews. - A present-but-invalid repositoryContext now fails closed in every consumer; the gate no longer silently drops the disclosure. - The duplicated validators and bounds are shared between the wire format and the manifest provider; the context role allow-list is derived from a single const; manifest arrays no longer require hand-sorting (uniqueness only). - Nits: dead mkdir removed, output message names the provider, escape-message fix, unsafe changed paths skip instead of aborting, segment-glob regexes memoised, list helper hoisted. - Docs: user-facing manifest section, trust-boundary residuals and foundation status in the design doc, fail-closed exit guidance in the skill. * fix(review): skip unsafe related paths in manifest context (#8401) * fix(ci): align review timeout helper test with externalized variables (#8401) * fix(review): harden repository context bounds and base identity reads (#8401) * fix(review): bound manifest matching work and pin round-2 review gaps (#8401) * fix(test): isolate serve streaming suite from stray workspace settings (#8401) * fix(review): cap identity reads, bill match work by length, pin round-3 gaps (#8401) --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-08-06 15:13:31 +00:00
Shaojin Wen	477859bb3f	feat(channels): support local gh authentication (#8461) ... * feat(channels): support local gh authentication Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(channels): align registry catalog test and visuals with optional GitHub token (#8461) * fix(channels): address review feedback for GitHub local gh auth (#8461) Treat a blank replacement of an optional secret as a clear so an existing GitHub channel can no longer ship an empty or whitespace-only PAT to the daemon. Reuse the shared missing-field predicate in the editor's GitHub credential validation, wrap malformed baseUrl failures in an actionable channel error, and surface sanitized gh stderr in local authentication failures. * fix(channels): address second-round review feedback for GitHub local gh auth (#8461) Pin the whitespace-only token gate, the bounded gh stderr sanitization, and the required-secret blank-replacement guard with mutation-resistant tests. Log the authenticated account identity on channel connect so an out-of-band gh auth switch is visible to operators. Align test secret-source fixtures with the SDK union and complete the design doc's change footprint. * fix(channels): address third-round review feedback for GitHub local gh auth (#8461) * fix(channels): address fourth-round review feedback for GitHub local gh auth (#8461) * fix(channels): address fifth-round review feedback for GitHub local gh auth (#8461) * fix(channels): address sixth-round review feedback for GitHub local gh auth (#8461) * fix(channels): address seventh-round review feedback for GitHub local gh auth (#8461) * fix(channels): address eighth-round review feedback for GitHub local gh auth (#8461) --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-05 14:05:00 +00:00
Shaojin Wen	a123d0030a	ci(review): prepare evidence-image tooling for GitHub-triggered reviews (#8454) ... * ci(review): prepare evidence-image tooling for GitHub-triggered reviews Reviews triggered on GitHub cannot attach images today for three reasons; this wires the two that live in the workflow: - Install tmux and freeze (pinned, checksum-verified) before the review runs. Both are optional by contract — the evidence ladder degrades honestly without them (png -> ans-only -> refused, recorded in the capture manifest) — so the step never fails the review; it only decides which rung the runner can reach. tmux mirrors the tolerant install qwen-autofix.yml already uses; freeze falls back to ~/.local/bin when passwordless sudo is absent. - Pass QWEN_REVIEW_ASSETS_REPO from a repository variable to the review step. Publishing stays OPT-IN by design: with the variable unset the env is empty and publish-assets refuses (parseAssetsRepo trims and rejects empty), so nothing changes until a maintainer sets the variable. When set, evidence images land on commit-pinned pr-assets/<pr>-review branches — already covered by the visuals cleanup workflow — pushed with the same CI_BOT_PAT the step uses. The third reason is release lag: the capture producer (capture-tui, #8388) has to merge and ship in a release before rendering claims can generate images on CI at all. This change is inert until then. * fix(ci): capture-tools step review fixes — enforced tolerance, version pin, cached fallback R1-1: the never-fails contract is now enforced twice — continue-on-error at the YAML level (the belt) and set +e with a trailing exit 0 inside (the suspenders); under the runner's default bash -e several statements (mktemp, install, sudo install with an empty path) could previously abort the step and fail the review the comment promised never to fail. R1-6: probe the VERSION, not just the binary — on a persistent self-hosted runner an installed freeze made any FREEZE_VERSION/SHA bump a silent no-op; the pin now forces a refresh when the cached binary does not match. Cached-fallback fix: put ~/.local/bin on PATH (and GITHUB_PATH) before the probe — a sudo-less runner otherwise re-downloads the tarball on every review run forever. R1-3: the step comment says capture-tui is UPCOMING (#8388, not in the released CLI) and names qwen review drive as today's tmux consumer, so the step cannot be mistaken for stale dead weight and deleted from under the follow-up. R1-4: the retention comment scopes the cleanup-workflow claim to the same-repository designation; a fork or scratch destination manages its own retention (docs updated to match, plus a note documenting the repository VARIABLE a maintainer sets to enable publishing). R1-5: the step's real bash now runs in the workflow behavioural harness under bash -e with stubbed sudo/apt/curl/sha256sum/tar/uname: worst-runner and checksum-reject scenarios exit 0 installing nothing, the no-sudo happy path pins the ~/.local/bin + GITHUB_PATH pairing, and the version-pin probe is pinned from both sides (wrong version re-downloads, matching version skips). Real freeze/sudo on a developer machine are shadowed so the tests are deterministic and can never install to /usr/local/bin. Nit: both sudo guards now check sudo -n true. * fix(ci): capture-tools step review fixes — step-owned tool dir, anchored probe, honest failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): capture-tools test harness — shadow tmux, don't blank its PATH dir The harness dropped every host PATH directory that ships a tmux so the step's apt branch would depend on the scenario, not on the machine hosting the suite. On GitHub-hosted ubuntu runners tmux lives in /usr/bin, so the filter blanked /usr/bin wholesale — bash, grep, mkdir, and tar included — and execFileSync('bash') died of ENOENT: all seven capture-tools tests failed in the Test (ubuntu-latest Node 22.x) job while passing on tmux-less dev machines. Replace the directory-level drop with an entry-level shadow: each tmux-bearing directory is mirrored (symlinks) into a scratch dir minus the tmux entry, in place, preserving PATH order and the empty-entry stripping the old filter did. Hosts without tmux take the map through unchanged, and Windows (no tmux in its PATH, no symlink branch) keeps its current behavior exactly. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): capture-tools test review fixes — faithful stubs, env shape pins, missing-branch scenarios Round-3 review findings: the harness executed several of the step's branches but asserted nothing about them, so probe-verified mutants (dropped tmux guard, deleted warning/degradation messages, malformed or missing FREEZE_VERSION/FREEZE_SHA256, wrong hash variable, dropped URL `v` prefix or curl `-L`, severed tarball paths, broken regex boundary, leaked mktemp dir) all shipped green. - Make the curl/sha256sum/tar stubs model their real contracts: exact pinned URL, pinned checksum over a file curl actually wrote, existing -xzf operand - Pin FREEZE_VERSION/FREEZE_SHA256 shape in captureToolsSource - Pin the full curl flag set and the three-site tarball path agreement - Assert the stale-renderer warning (fires on degraded re-download, silent on the happy path) and the tmux-unavailable message - Pin TMPDIR and assert the mktemp cleanup leaves it empty - Add the two missing scenarios: tmux-present skips apt, cached version extending the pin with a leading digit re-downloads Verified by 13 mutation probes: every named mutant now turns the suite red (13/13 killed), baseline 34/34 green. * fix(ci): capture-tools step review fixes — hash-verified cache, per-run PATH promotion * fix(ci): capture-tools step review fixes — verified-bytes-only installs, step timeout Review findings on the capture-tools step: - Drop the PATH-trust branch: a freeze already on PATH was accepted on its own --version and executed to probe it — exactly the self-report the FREEZE_BIN_SHA256 comment declares attacker-controllable, from dirs writable between jobs on both runner classes. The checksummed download always runs now; the cache makes it free after the first run. - Guard $tools_bin in the download branch: with mktemp failing, the unguarded install resolved to /freeze — harmless unprivileged, but a root-in-container self-hosted runner writes it and reports success with nothing on PATH. - Copy-then-verify the cache: install into the fresh per-run dir FIRST, verify THOSE bytes, delete both copies on mismatch — the verified bytes are the bytes later steps execute, closing the check-then-copy race for free. This makes the separate pre-verify block redundant; it is deleted. - Add timeout-minutes: 5 — continue-on-error bounds failure, not duration, and a stalled `sudo apt-get update` mirror had no other bound under the 300-minute job cap. - Report block: say the resolved freeze is likely broken when its --version produces nothing, instead of echoing a blank line and calling it stale; the mismatch wording is direction-neutral now. Tests: replace the PATH-trust scenario with a planted-PATH one (marker outside the scenario dir proves the plant never executes), add the mktemp-failure scenario (the install stub succeeds like root would, so the unguarded mutant is caught) and the promoted-dir 0700 assertion; re-anchor the two digit-boundary tests on the report's warning. 41/41 green; both fix mutants verified killed. * fix(ci): capture-tools review fixes — stale-dir cleanup, pinned guards Address round-5 review: - R5-1 (Critical): the per-run qwen-review-tools.* dir under RUNNER_TEMP was never removed; RUNNER_TEMP survives across jobs on the shared pool, so every review run accumulated one dir + one Go binary, unbounded. 'Clean stale agent state' now removes stale dirs before the install step creates the current run's dir, matching the qwen-triage.yml convention. The harness comment claiming the dirs were runner-cleaned is corrected. - R5-6: the cache re-verification rejection branch now logs why it deletes the cached binary instead of degrading silently. - R5-7: bump-checklist note beside the freeze pins — the harness stubs key on the same env values, so a transposed hash pair must be caught against the real release artifacts at bump time. - R5-2/R5-3/R5-4/R5-5: four unpinned step properties now pinned (the if: guard, the sudo -n probe flag, install-after-context ordering, and the cache branch's tools_bin guard via a new mktemp-fails scenario); six mutation probes confirm each pin kills its mutant. * fix(ci): capture-tools review fixes — curl budget, swept scratch dir, wiring pins * fix(ci): capture-tools review fixes — harness mutation pins, pin-pair self-check Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): capture-tools review fixes — shadow-farm cleanup, backoff budget term * fix(ci): capture-tools review fixes — report probes only installed freeze, age-gated sweep --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-08-05 13:16:56 +00:00
Shaojin Wen	1a2bb10298	fix(review): admit evidence images by content, not by name — magic-byte sniffing (#8459) ... * fix(review): admit evidence images by content, not by name — magic-byte sniffing The publish-assets allowlist was extension-based, and an extension is a claim anyone can make: combined with a prompt-injected review run, whatever could name a file evidence.png could host up to the size cap of arbitrary bytes at a github.com URL through the evidence push (raised in #8454's review as an enablement consideration). sniffImageFormat reads the four admitted signatures (PNG, JPEG, GIF87a/89a, RIFF+WEBP — RIFF alone is not enough, AVI and WAV share the container prefix) and validateAssetContent rules the content against the format the extension claims, fail-closed: an unrecognized signature refuses even when the extension is allowed. publish-assets applies the ruling to every file's first bytes before anything is uploaded — all-or-nothing, same refusal contract as the other gates. Pinned: the four signatures, truncated/empty headers, the RIFF/AVI near-miss, extension-format mismatch, fail-closed unknown extensions, and end-to-end: a shell script named evidence.png refuses with exit 3 and nothing pushed. * fix(ci): re-pin review timeout tests to vars externalization (#8459) * fix(review): single-source the asset allowlist and pin sniff depth (#8459) Address round-1 review suggestions: - ASSET_EXTENSIONS now derives from EXTENSION_FORMAT, so admitting a format is a one-place change the batch gate and the content gate cannot drift on; the duplicated extension extraction moves into one claimedExtension helper shared by both gates. - Pin what the mutation probes showed unpinned: the GIF87a branch (sniff + admission), uppercase extensions at the content gate, the refusal message direction, the full depth of every signature (near-miss negatives), and the publish-time 16-byte slice end to end (a WEBP publishes through runPublishAssets; verified the test fails when the slice shrinks to 8). * fix(review): pin every sniff check and guard the allowlist lookup (#8459) * fix(review): pin every sniff byte and the two-gate format coupling (#8459) * fix(review): align the two-gates comment with the pin that enforces it (#8459) * fix(review): sharpen evidence-gate diagnostics and pins (#8459) - Content refusals now name the JSON-quoted full path, so two same-named files from different directories are tellable apart (the sibling read-error refusal already spent the path). - Export ASSET_HEADER_BYTES from the lib that owns the sniff depth; the publish call site uses it and the two-gates pin slices canonical headers to it, so a future longer signature fails the pin, not real publishes. - One shared refusal builder keeps the allowlist message identical in validateAssetFile and validateAssetContent. - Narrow the sniffImageFormat threat model to what magic bytes buy: binds the claimed type to the leading bytes, does not stop prefixed payloads. - One-byte-off matrix becomes a labeled it.each table (failures name the exact corrupted byte); imports re-alphabetized. * test(review): pin the shared extension refusal across both asset gates (#8459) * test(review): pin lastIndexOf extension parsing for multi-dot asset names (#8459) * fix(review): admit WEBP by its fourcc and name refused files once (#8459) --------- 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>	2026-08-05 02:23:25 +00:00
Shaojin Wen	ac67de2e46	fix(review): stop the reverse-audit loop while there is still time to report (#8468) ... * fix(review): stop the reverse-audit loop while there is still time to report Measured on CI run #8368 (+1699 lines): the iterative reverse audit ran to its 5-round cap, each round a per-chunk fan-out whose findings then went back through verification, and the loop consumed 3.5 of the job's 4 budgeted hours. The outer GNU-timeout kill arrived while round 5's findings were still being verified. The review died holding every confirmed finding it had; nothing reached the pull request. The loop's rounds are driven by the orchestrator, but every round begins at the same place: agent-prompt building the round's prompts. So the builder becomes the loop's clock. When the environment carries a review deadline (QWEN_REVIEW_DEADLINE_EPOCH, exported per attempt by the review workflow) and the remaining time is inside the reserve kept for the last verification, compose-review and submission (default 60 minutes, QWEN_REVIEW_DEADLINE_RESERVE_SECONDS to override), a reverse-audit round is refused: a BUDGET line on stderr, exit code 4, no prompt built and no record written. The message carries the exact unreviewedDimensions entry to file, so the disclosure that caps the verdict is the CLI's text, and Step 6 proceeds with the findings already confirmed. Local runs have no deadline and are untouched. A malformed deadline fails open — the outer kill still bounds the run, and a broken variable must degrade to today's behaviour rather than wedge every budgeted review at round 1. The verifier is deliberately not gated: the reserve exists so it can run. * fixup: scale the deadline reserve to the externally-chosen budget The budget is not this workflow's to assume: it arrives from a repository variable, a workflow input, or a /review --timeout=N comment. A fixed 60-minute reserve would consume most of a 70-minute budget and refuse the audit loop outright on a 30-minute one. The workflow now passes a reserve of a quarter of the attempt, floored at 10 minutes and capped at 60; the CLI constant remains only the fallback for a caller that sets a deadline without a reserve. * review feedback: admit the round only if IT fits, and cap deterministically Three findings from review, all taken: 1. The gate budgeted for the tail but not for the round it admits — the terminal round is by construction the one that starts closest to the boundary, so the killed-mid-verification failure survived one round wide. The gate now requires remaining >= round + reserve, where the round's cost is the previous round's, measured admission-to-admission from a stamp the builder writes (one per round; a same-round rebuild is not a round), falling back to a 30-minute constant for round 1, which starts with the most headroom. 2. The refusal was deterministic; the disclosure that caps the verdict was prose the orchestrator had to carry. The builder now records a budget-stop marker beside the prompt records and compose-review synthesizes the unreviewedDimensions entry from it — deduped against a relayed copy — so a run that drops the sentence still cannot approve past a truncated audit. 3. Exit code 4 is documented in the command's describe. Also restores the Step 5 bullet the previous commit's edit displaced (new findings merge into the cumulative list before the next round). * review feedback: pin the budget gate's all-chunks refusal and ordering Cover the two behaviours the review noted were only asserted on the bare --findings form: an exhausted budget refuses the loop's real --all-chunks round before ANY of the per-chunk records is written, and a malformed call (--round 0) still gets its validation error first — exit 4 is for a well-formed round the budget refuses, never a replacement error. Also name what the code already does: reserve=0 is the deliberate escape hatch (the gate shrinks to the round estimate alone), and the workflow's 3600s cap mirrors DEFAULT_RESERVE_SECONDS. * docs(review): describe the soft-deadline env vars for time-budgeted runs The review noted the two new variables appeared in no user-facing doc; the reserve in particular is an operator-facing knob. State what each does, the fail-open posture, and how the refusal surfaces in the verdict. * fix(cli): align budget-stop disclosure with the gate's refusal (#8468) A round-1 budget refusal left no reverse-audit records, so the Step 4/5 floor reported the deliberate stop as a rogue/unlaunched audit with a rebuild FIX the same gate deterministically rejects; the refusal's own disclosure was swallowed by the caller-echo dedup. The floor now stands down when the budget-stop marker exists, and compose-review renders the disclosure structurally, bilingually, from the marker. Also: `--role reverse-audit` requires `--round <k>` (an unlabeled admission stamps an entry no estimate can attribute), the budget gate runs after the plan/findings reads (a broken plan or unreadable findings deserves its own error, and nothing is stamped ahead of a buildable call), and the gate's admission boundary, measured-cost behaviour, and the workflow env contract are pinned by tests. * review: a budget stop excuses only the round it refused The budget-stop suppression keyed on the marker's existence alone, so every reverse-audit gap shape went silent once any round was refused — including the shapes that describe rounds which RAN before the budget hit. A hand-written round-1 launch is exactly as undelivered when round 3 later hits the budget, and suppressing its disclosure let 'stopped before round 3' imply the rounds that did run were faithful. Exactly one shape is by design under a marker: not-built — the refusal writes no record, so an audit with no records IS the audit the gate stopped, and its FIX (rebuild the round) would be refused by the same gate. The suppression now names that shape and no other; a rewritten, unlaunched or brief-unread round keeps its disclosure and its repair. The new test pins the operative halves: the verdict stays capped, the marker's disclosure posts, and the operator channel carries the rewritten round's exact repair. (The posted body collapses same-subject disclosures — both say 'reverse audit' — so the author sees the stop; repairs are acted on from stderr, where the rewritten fix rides.) * fix(review): fence budget state per run, and let gate errors beat budget stops Address the round-2 review threads on the reverse-audit budget gate: - Fence budget-rounds.json and budget-stop.json by the plan's own mtime. Every run rewrites the plan at its Step 1 capture, so records older than the plan belong to a previous run of the same PR: a run killed before cleanup no longer prices the next run's rounds off stale stamps (an hours-old stamp read as an hours-long round refused round 1 of a fresh budget) and no longer caps a later run's verdict on a stop that did not happen in it (R2-1, R2-2). - Refuse a structurally unbuildable plan (no chunks[], duplicate or non-integer ids) with its own error ahead of the budget gate, so the same corruption gets the same diagnosis whatever the clock says, and no budget-stop marker is written over a corrupt plan (R2-5). - Stamp a round admitted only after its build succeeds: a build that throws leaves no stamp, so the next round's cost is never measured from a build that produced nothing and floored to 600s (R2-6). - Keep the budget entry's 'reverse audit' subject out of the caller-echo prefix filter: other reverse-audit scopes the orchestrator disclosed (a twice-whiffed chunk from the rounds that DID run) are no longer silently dropped in the marker's shadow; the marker's own relays stay deduped by the phrase splice (R2-7). - Render --round unbracketed in the reverse-audit rebuild fix — the CLI refuses a round-less reverse-audit call, so the paste-and-run repair must not present the flag as optional (R2-14). - Document the deliberate one-verification overlap between the measured round estimate and the tail reserve, at both definitions (R2-13). - Test hardening, each assertion mutation-probed to fail its named mutant: a reshaped relay only the marker-phrase splice dedups (R2-8); the stamp's round label and the verifier's no-stamp invariant (R2-9); whole-line, unit-arithmetic and reserve-cap pins on the CI wiring contract (R2-10); the first-wins stamp survivor (R2-11); the reserve=0 escape hatch (R2-12). --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-04 13:58:15 +00:00
Dragon	48d37cdf70	docs: document headless Goal workflows (#8503)	2026-08-04 03:30:26 +00:00
Shaojin Wen	7dfc554dff	feat(review): Add structured Web Shell review results (#8402) ... * feat(review): add Web Shell review artifacts Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(web-shell): add code review artifact visual scenario (#8402) * fix(review): address Web Shell review artifact feedback (#8402) * save-artifact: document why paths resolve against the daemon workspace root (QWEN_CODE_PROJECT_DIR) instead of cwd, and cover the relative-path form the skill documents with a test where the two roots differ. * CLI/renderer contract: the renderer hand-duplicates the findings vocabulary and fails closed on unknown values, so name the renderer as a second consumer beside the CLI's lists and check in a contract fixture generated through the real pipeline (validateFindings -> buildReport -> save-artifact) that exercises every source, severity, confidence and outcome. Exporting the vocabulary through the SDK stays deferred: it is a public cross-package API change beyond this PR's seam. * resolve-anchors now validates `line` exactly like `findings` does (positive safe integer); the two validators in one pipeline no longer disagree. Note: an in-flight `.qwen/tmp` findings file carrying `line: 0` fails where it previously did not. * The renderer validates markdownReportPath (relative, no ".." segments, .md suffix) before it becomes a readWorkspaceFile call, resets the severity/confidence filters when switching artifacts, and surfaces heldByMeasurement so a nonzero Held count is attributable. * save-artifact refuses low effort structurally (choices and library guard) instead of by prose, stats the Markdown report before reading it so a directory reports "not a file", and the component no longer shadows the DOM `document` global. * The case-insensitive alias test now skips visibly on case-sensitive filesystems instead of passing vacuously. * Comment the kept `turnOutputs.review` key and document the JSON companion in the user docs. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): address second Web Shell review artifact feedback round (#8402) --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-03 16:08:13 +00:00
jinye	0cb109f513	fix(core): Avoid replaying unsafe MCP tool calls (#8387) ... * fix(core): Avoid replaying unsafe MCP tool calls Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): Revalidate MCP replay after reconnect Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-03 11:04:38 +00:00
Shaojin Wen	186812694c	feat(review): publish evidence images to a user-designated assets repo (#8351) ... * feat(review): publish-assets — evidence images for PR review comments GitHub's API cannot attach images to review comments (the web UI's drag-and-drop upload has no API equivalent), so a review whose evidence is a screenshot — a TUI rendering, a before/after comparison — had no way to show it. New `qwen review publish-assets` hosts evidence images in a user-designated repository and hands back URLs a comment can embed. Grew from the maintainer's manual workflow (screenshots pushed to `pr-assets/<PR>-verify` branches over HTTPS), and inherits the shape of the skill's only other public write (`submit`) deliberately: - Designated destination: writes only to QWEN_REVIEW_ASSETS_REPO, an owner/repo the user set by hand — the reviewed repo for maintainers, a fork or scratch repo otherwise (fork-vs-in-repo becomes a configuration difference, not two code paths). A separate variable from QWEN_REVIEW_SCRATCH_REPO on purpose: that contract forbids PR-derived content, and evidence screenshots are exactly that. Unset → exit 3. - Authorised run: the same args-file re-parse and target binding as submit, now extracted to a shared lib/authorization.ts so the two gates cannot drift (the target-binding lesson lives in one place). Since an effective --comment forces high effort, low/medium runs can never publish. - Images only, capped, all-or-nothing: extension allowlist (SVG excluded — a script container), per-file and per-batch size caps, one refused file refuses the batch before anything is pushed. - Immutable references: files land on pr-assets/<pr>-review via the Contents API (HTTPS via gh; no clone, no SSH), content-hash-named so re-runs are idempotent, and every URL is pinned to the commit — a posted comment's evidence cannot be changed from under it. The web-host /raw/ URL form works unchanged on GitHub Enterprise. - Auditable: a manifest names every file pushed and the landing commit, swept by cleanup with the other review artifacts. The findings artifact gains per-finding `assetFiles` (local evidence paths) and `assets` (published URLs); `publish-assets --findings/--findings-out` publishes everything referenced and weaves the URLs back in, so the comment builder reads the artifact rather than hand-carrying URLs. What the command cannot check is stated in SKILL.md instead: image content. Publish only evidence the review itself produced — never a capture of the user's own terminal, which can hold an env dump in the scrollback. Tests: 45 files / 1394 assertions — new suites for the assets naming and validation rules and the command's gates (refusal without designation, refusal without authorisation, target binding, branch creation, idempotent re-run, batch refusal, findings weaving); submit's 42 pass unchanged on the extracted gate. * fix(review): publish-assets round-1 self-review — six findings Round-1 review of this branch, walked with the angles the author-side pass does not cover: - submit.ts kept its parseReviewArgs import after the authorization extraction; vitest does not typecheck, `tsc --build` does, and CI's build leg failed on TS6133. (The whole first CI round's failures cascade from this one break.) - ensureBranch %2F-encoded the slashed ref path; GitHub's documented form is literal slashes and %2F routes inconsistently across endpoints — a 404 here reads as "branch missing" and turns every re-run into a 422 on the create. Ref paths are now literal (the branch name is built from a validated integer, so interpolation is safe); the contents `?ref=` query VALUE keeps its encoding, which is the correct position for it. - The authorization gate bound URL-shaped `--comment` arguments against the ASSETS repo, refusing legitimately authorised runs whenever the assets repo is a fork rather than the reviewed repo. The shared gate's repo binding is now optional — submit still always binds it; publish-assets binds the PR number (and host) alone, with a new optional --reviewed-repo to restore the stronger binding when the orchestrator knows the reviewed repo. - URLs were pinned to the last PUT response's commit.sha; on an identical-content update that field's shape is GitHub's to decide, not ours to assume. The head is now read from the branch ref after the uploads — one extra call for independence from the response shape. - putContent's catch-all retried EVERY failure through the exists path, answering a 401 with a confusing secondary error from the sha lookup; the retry now fires only on the 422/needs-sha shape and rethrows the rest. - --findings without --findings-out silently skipped the URL weaving; it now warns, and --findings-out implies --findings. New tests: literal-ref assertion, non-exists rethrow, URL-shaped authorisation without assets-repo binding, --reviewed-repo mismatch refusal. 45 files / 1399 assertions green; `tsc --build` clean. * fix(review): publish-assets round-2 — empty-findings no-op, reviewed-repo hint Round-2 findings on this branch: - A findings artifact carrying no assetFiles is the ORDINARY case for most reviews, but publish-assets answered it with exit 3 — a refusal an orchestrator calling the command unconditionally on every posting run would read as a failure to repair. It is now a no-op (exit 0, {published:false, count:0}); a bare --files with nothing named keeps the exit-3 refusal, because there the emptiness IS the caller error. - SKILL.md's example now names --reviewed-repo for URL-target reviews, so the stronger authorisation binding is used where the orchestrator knows the reviewed repo. 44 files / 1387 assertions green; tsc --build clean. * test(review): fix invalid two-argument expect in assets.test.ts Round-3 sweep: vitest's expect takes one argument — the message-style second argument was a lint error and a weak assertion both. The offending value now rides inside the asserted object, so a regression names which shape slipped through instead of reporting 'expected true'. * test(review): pin the findings schema's evidence-asset validation directly Round-4 sweep: assetFiles/assets were exercised only through publish-assets' weaving test — the schema's own rejection paths (non-array, empty-string entry, empty-array drop) had no direct case, so a validation regression would have surfaced as a confusing weaving failure two layers up. * fix(review): address all six findings from the automatic review (R1-1..R1-6) The /review pipeline's own round-1 findings on this PR, each confirmed and fixed: - R1-1 (the real catch): the host-binding check sat nested inside the `req.repo !== undefined` guard, so a caller omitting --reviewed-repo also silently skipped the HOST binding — contradicting the documented "binds the PR number (and host) alone". The host check now stands on its own; a new test pins an Enterprise-host mismatch refusal with the repo binding absent. - R1-2: --pr accepted whatever yargs `type:'number'` passed through (NaN, 0, 3.5), and --user-authorized bypasses the gate that would have re-parsed the target — `pr-assets/NaN-review` was reachable. A Gate-0 positive-integer check now refuses first, matching submit's sibling discipline. - R1-3: the suite drove the skillArgs seam without clearing QWEN_CODE_SESSION_ID, so running it inside an active Qwen Code session spuriously failed eight tests. beforeEach now saves/clears the variable and afterEach restores it. - R1-4: the 40MB aggregate cap was enforced inline and untested (a mutation deleting it stayed green). The per-file rules and the total cap now live in one pure ruling, validateAssetBatch, unit-tested with five 9MB sizes and no fixtures. - R1-5: the asset_files snake_case alias was the one untested member of an otherwise-tested alias family; pinned. - R1-6: the setGhHost wiring had no command-level assertion; a GHE test now pins both the call and the host-carrying manifest URLs. 44 files / 1397 assertions green; tsc --build and eslint clean. * fix(review): address all ten round-2 findings from the automatic review Round-2 of the /review pipeline on this PR: 2 Critical, 8 Suggestions, every one confirmed against the code and fixed. Criticals: - The round-2 test block added for the empty-findings no-op omitted the QWEN_CODE_SESSION_ID save/delete/restore its two sibling blocks perform, so the suite spuriously failed inside an active Qwen Code session — the exact dogfooding environment this repo reviews from. - The gh routing and the returned URLs read the host from two different sources: with --host absent, gh children inherit an operator-exported GH_HOST (routing at Enterprise) while rawAssetUrl defaulted to github.com — every returned URL a 404. One effectiveHost (flag, then GH_HOST env) now feeds both. Suggestions: - putContent's retry discriminator matched a bare `422` anywhere in err.message — which execFileSync fills with the full command line, including the PR-numbered remote path: evidence for PR #4220 would read a 401 as "already exists". Anchored to `HTTP 422`. - ensureBranch's bare catch read every ref-lookup failure (401, 403 rate-limit) as "branch missing"; only HTTP 404 takes the create path now, and an empty assets repo — whose default_branch resolves while its head ref 404s — is named as the condition it is, with the fix stated. - Validation refusals threw (yargs exit 1, stack trace, empty stdout) while every other gate in the command answers exit 3 + {"published": false}; unreadable files and batch refusals now speak the same refusal language. - The command's idempotent writes (content-hashed PUTs, a ref create whose duplicate is tolerated) now go through a new ghWithInputRetried — sharing gh()'s transient-error retry — and ghWithInput's no-retry docstring names the two-caller split instead of claiming a sole caller. - parseAssetsRepo admitted dot-segment repos (`owner/..`) its docstring claimed were path-safe; segments now exclude `.`/`..`, mirroring submit's isRepo. - stringArray accepted whitespace-only evidence paths; trim(), matching the sibling asString. - The GHE test asserted setGhHost was called but not WHEN; it now asserts the call precedes the first API invocation. 44 files / 1403 assertions green; tsc --build and eslint clean. * refactor(review): one refusal helper for every publish-assets gate Round-2 of this branch's fresh review: the refuse() helper existed below seven inline copies of the identical three-line refusal — the drift shape where one site eventually forgets the exit code. Hoisted to the top of runPublishAssets and used by every gate; message content unchanged where tests pin it. 26/26; tsc clean. * fix(review): address the round-3 review — bidirectional host binding and 14 more The automatic review's third round on this PR: 1 Critical + 14 Suggestions, each verified and addressed. The Critical (host binding, both halves): - The gate's `req.host &&` guard bound the host in one direction only — an Enterprise-URL authorisation admitted a host-less write routed at github.com (or wherever GH_HOST pointed). The gate now compares the authorised host against the write's EFFECTIVE host, defaulting an absent req.host to github.com: a host is a host, not an exemption. - Both callers fed the gate the flag rather than the route: publish-assets computed effectiveHost (--host ?? GH_HOST) AFTER the gate and bound args.host; submit bound args.host while its gh child inherited GH_HOST. publish-assets now resolves effectiveHost before Gate 2 and binds it; submit binds the same resolution. The rest: - pr-assets/<N>-review registered in the asset-branch cleanup workflow, per its own every-producer-must-be-added-here rule — a branch nothing deletes is permanent. - ghWithInputRetried had been inserted between ghWithInput and its JSDoc, leaving the does-NOT-retry comment attached to the function that DOES retry; each function now carries its own doc. - putContent's retry-path contents-GET is wrapped: when the 422 was not the sha-missing shape and the path does not exist, the GET's 404 no longer replaces the PUT error the user needs. - stringArray treats null as absent like every sibling parser, so an artifact rendering "no assets" as null canonicalizes instead of crashing. - Test isolation, all four describe blocks: GH_HOST save/delete/restore, setGhHostMock.mockReset (a sibling's persistent throwing implementation survives mockClear — the malformed-host test also switched to mockImplementationOnce), and full mock resets in the blocks that lacked them. - The two regression-pin tests the review measured vacuous now discriminate: each fails only the one call under test and asserts the pipeline stopped THERE (no contents PUT after a bad create; exactly one gh call after a 403 lookup). - New positive pins: a double-fired create ("Reference already exists") succeeds; the canonical report shape this command's own --findings-out writes round-trips; an Enterprise-URL authorisation refuses a host-less write while a github.com-URL one passes it. Not changed: the finding that reverting the Finding-interface hunk leaves tests green — the fields are type-level and their removal fails tsc --build (the CI leg that caught this branch's own TS6133); a runtime pin would duplicate what the round-trip tests already exercise. 47 files / 1495 assertions green; tsc --build 0 errors; actionlint clean on the cleanup workflow. * fix(review): address the round-4 review — empty-GH_HOST passthrough and four test pins Round 4 came back COMMENTED (down from CHANGES_REQUESTED), 5 Suggestions, 0 Criticals — all five confirmed and fixed: - An exported-but-empty GH_HOST ("" from an unset workflow var) survives `??`, being non-nullish: effectiveHost became "" and the gate compared the authorised host against "", refusing a legitimate github.com write. Both call sites now collapse an empty trim to undefined (`|| undefined`, parenthesized). - The gate's URL-shaped repo/host binding was exercised only via publish-assets' suite; submit.test.ts now pins both directions of the host binding and the repo binding at its own call site. - ghWithInputRetried had no retry-contract test; gh.test.ts adds the symmetric block to ghWithInput's does-NOT-retry pin (transient 500 retried once then succeeds; non-transient 401 single call). - The publish-assets mock aliased ghWithInput and ghWithInputRetried to one mock, hiding which variant a write used; they are two mocks now, and the happy path asserts the non-retrying variant is never touched. - The Prepared interface's dead `name` field is gone. 46 files / 1476 assertions green; tsc --build and eslint clean.	2026-08-02 15:49:32 +00:00
Shaojin Wen	09d818867e	fix(review): follow output language in Tip lines and saved reports (#8370) ... * fix(review): follow output language in Tip lines and saved reports The /review skill's critical rule 2 already states that terminal output follows the user's output language preference, but three areas lacked explicit guidance, causing the model to output them in English even with a Chinese output language configured: - Follow-up Tip lines (e.g. "Tip: type post comments to ...") were specified as English templates with no translation instruction - The Step 8 saved report file used English section headings and prose - Step 6 labels (e.g. "Balanced review (effort: medium)") had no translation note Add explicit output-language guidance at each point of use, with a Chinese example for Tip lines and Chinese section heading examples for the saved report. Command keywords (post comments, fix these issues) stay verbatim since they are trigger phrases the user types back. * fix(review): disambiguate findings-artifact language boundary and complete low-effort translation notes (#8370) --------- Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-02 11:48:58 +00:00
易良	eea0a2b3b2	fix(github-channel): recover interrupted inbound tasks (#8306) ... * fix(github-channel): recover interrupted inbound tasks * fix(github-channel): make inbound recovery bounded * test(github-channel): cover delivery-failure lifecycle and audit-hit recovery Add a direct test for the onTaskLifecycle failed/delivery -> reply_pending transition and a recovery test for the publication-audit match-and-remove path. Restore the blank line between the constructor and createInitialCursor. * fix(github-channel): preserve cancelled inbound tasks and fail closed on bookkeeping (#8306) * fix(github-channel): harden inbound task lifecycle against partial persistence failures (#8306) * fix(github-channel): close crash-window duplicates and make tests load-bearing (#8306) * test(github-channel): add recovery test for suppressed audit outcome (#8306) * fix(github-channel): persist errorCommentPosted after post, add review test coverage (#8306) --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-02 07:27:44 +00:00
Dragon	e967cc9037	docs: document compaction and image model selection (#8348) ... * docs: document auxiliary model selection * docs: align model option labels * docs: clarify image model HTTPS requirement	2026-08-02 06:44:54 +00:00
qwen-code-dev-bot	8d6d2ab56a	feat(cli): /summary supports custom export path (#8116) ... * feat(cli): /summary supports custom export path (#8113) `/summary` now accepts an optional path argument, matching `/export`'s behavior. When a path is provided, the summary is saved there instead of the default `.qwen/PROJECT_SUMMARY.md`. - `/summary` → saves to `.qwen/PROJECT_SUMMARY.md` (unchanged) - `/summary docs/summary.md` → saves to `docs/summary.md` - `/summary /absolute/path/summary.md` → saves to absolute path - `/summary docs/` → saves to `docs/PROJECT_SUMMARY.md` If the path is a directory (existing or ending with `/`), the default filename `PROJECT_SUMMARY.md` is appended. Parent directories are created automatically. * fix(cli): summary custom path dir detection and i18n key (#8116) * test(cli): assert relative display path in summary tests (#8116) * fix(cli): summary path containment, early validation, and mkdir hardening (#8116) * fix(cli): defer summary mkdir to save time so failed generation leaves no empty dir (#8116) * fix(cli): normalize path separators in summary test and assert file content (#8116) * test(cli): assert directory permission mode in summary test (#8116) * fix(cli): resolve symlinks in summary path containment check (#8116) * fix(cli): reject broken symlinks escaping project root in /summary (#8116) * fix(cli): guard /summary overwrite and expand tilde in path (#8116) * fix(cli): re-validate appended default filename for symlink escape in /summary (#8116) * fix(cli): harden /summary symlink chain walk, file mode, and overwrite guard (#8116) * fix(cli): address review feedback on /summary custom path (#8116) - Fix CRLF false-negative in overwrite guard by normalizing line endings - Allow overwriting empty pre-created files (zero-length bypass) - Detect trailing separator on existing file and report clearly - Log chmod failures via debugLogger matching exportCommand convention - Add comment explaining mkdir mode asymmetry - Update docs: /summary usage table and custom-path welcome-back note - Add i18n key for trailing-separator error in all 9 locales - Add tests for CRLF, empty file, and trailing separator cases * fix(cli): address review feedback on /summary custom path (#8116) - Skip symlink-escape check for the default .qwen/ target so a symlinked .qwen/ directory (shared team config, overlay mounts) keeps working, and the check no longer runs after the LLM call - Re-run the overwrite guard immediately before writing to close the TOCTOU window across the slow generation step - Determine isDefaultTarget by comparing the resolved path against the default so `/summary .qwen/` gets the same 0o700 permissions - Only chmod 0o600 on file creation; preserve existing permissions on regeneration - Return empty content in interactive-mode errors to avoid double rendering (failInteractive already adds the error to history) - Tighten the overwrite-guard regex to require `**Update time**: ` after the Summary Metadata heading, preventing false positives - Fix the realpathNearestExisting comment to document the missing containment-during-walk guard vs export/stats copies - Add tests: symlink cycle, default target with symlinked .qwen, TOCTOU overwrite guard, explicit .qwen/ permissions, chmod preservation, interactive error content, regex false-positive * fix(cli): address review feedback on /summary custom path (#8116) * test(cli): cover post-LLM symlink re-check and interactive error UI (#8116) * fix(cli): address review feedback on /summary custom path (#8116) --------- 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 Bot <qwen-code-bot@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-08-02 02:19:31 +00:00
Shaojin Wen	dbb0349351	feat(review): borrow recall, a fix loop, and a size-derived budget from Claude /review (#8315) ... * feat(review): borrow recall, a fix loop, and size-derived budget from Claude /review Three changes, from a comparison of this skill against Claude Code's `/code-review`. The orchestration half of that comparison went the other way — nothing there has the worktree isolation, the transcript-backed coverage gate, or the single computed verdict — so what is borrowed is what it does better: how much it surfaces, what a finding *is*, and what a small diff costs. Recall ------ The Exclusion Criteria are a filter on what KIND of thing is a finding. Read as a confidence bar — which is how an agent under "silence is better than noise" reads them — they license dropping anything half-believed, and that drop is invisible: no later stage sees a candidate that was never filed. Every stage after the finders removes wrong findings; none can add a missing one. Each finder brief now carries the counterweight explicitly, and the Step 4 verifier deliberately does not get it. Code quality was one agent holding six unrelated checks — the shape this skill already refuses for invariant agents, on measured evidence (PR #6457: one agent with an eight-item checklist found 1 of 5 defects; the same model split three ways found all 5). Split into 3a reuse/duplication, 3b altitude/abstraction fit, 3c consistency/clarity. 12 -> 14 agents in 3A. Low was one undirected pass capped at 8, and its only alternative was a nine-subagent fan-out. It is now an angle rotation in one context — line-by-line, removed behaviour, language pitfalls, wrapper routing, reuse/dead code, sibling consistency, then a gap sweep — dedup-only, no re-judging, cap 10. Still zero subagents. --fix and findings as data -------------------------- `--fix` is `--comment` reflected and gated on the opposite target: `--comment` writes to a pull request, `--fix` writes to a working tree, so a PR review (whose tree is the ephemeral worktree Step 9 deletes) ignores it with a warning. An effective `--fix` floors the effort at medium — editing the user's files on an unverified finding is the same mistake as posting one. New `qwen review findings` canonicalizes the findings into a JSON artifact the terminal report, the saved report and the review JSON all read, instead of three transcriptions of one list. With `--outcomes` it merges the fixer's ledger and REFUSES one that does not account for every finding: a fixer that applies six of nine and reports six has not lied about any of them, it has silently shortened the list. Size-derived budget ------------------- New `plan.budget`, computed from srcDiffLines the way the topology gate is and recorded in the plan rather than passed as a flag, so every reader sees one number. Scopes the low tier's angle count and sweep, the Agent 8 ceiling (0 below 80 source lines — "one domain dominates" is a judgement, and one made about forty lines finds a dominant domain every time), and the verify shard width. It never scales a dimension away: that is the roster's answer and the roster reads effort. Not included: per-model prompt routing. Claude's table exists because it was measured per model family; shipping an invented mapping into this skill is the kind of change its own review would flag. Tests: 39 files, 1215 assertions. * docs(review): align counts and level descriptions left stale by the 3a/3b/3c split Round-1 self-review findings on this branch: - SKILL.md medium tier still named 'quality (Agent 3)'; the Step-1 low bullet and Step 3C heading hardcoded six angles though plan.budget scales them 3-6; and no fallback was stated for a plan written by an older CLI that carries no budget field (falls back to the flat pre-budget behaviour — more coverage, never less). - DESIGN.md still labelled the 12-agent roster '(current)', kept the ten-lens / crosses-twelve topology arithmetic beside the updated fourteen-agent copy, keyed the re-gating cost row to the 12-agent roster, and described low as one pass ≤8 and medium as unverified inline angles — both contradicting the SKILL.md this PR ships. The LLM-call-budget and Fork-Subagent sections were still summed for 12 agents. - findings.ts: validateFindings accepted outcome but dropped outcomeNote, so the canonical artifact did not round-trip — a skipped finding fed back through --input kept its outcome and lost its reason. Tests: 40 review files green, including two new round-trip cases. * docs(review): one id per finding across the cache ledger and the findings artifact The rebase onto #8218 left two id schemes for one finding: the incremental cache's cross-round ledger names findings R<round>-<n>, while the findings artifact accepted any unique id. Same defect, two names, and the outcome ledger and next round's report could no longer be joined. The artifact now uses the R-ids whenever the run writes the cache ledger. Conflict resolutions from the rebase itself: review.ts keeps both new subcommands (test-delta from main, findings from this branch); the documentation-parity check #8218 added to the old Agent 3 brief lands in 3c, the consistency slice that owns sibling-parity checks. * test(cli): add findings to the pinned review subcommand surface review.test.ts pins the exact subcommand list and sits one directory above the review/ glob the branch's local runs used, so the new findings subcommand never met it until CI. Ubuntu was the only matrix leg that ran. --------- 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-08-01 15:26:58 +00:00
Dragon	8673151ebd	docs: document skill learning and live reload (#8298)	2026-08-01 15:23:08 +00:00
Shaojin Wen	6f8ad2b4a5	feat(review): Include CLI version in attribution (#8294) ... * feat(review): Include CLI version in attribution Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(review): Decouple footer test from package version Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-08-01 09:40:57 +00:00
Dragon	e569734a1e	feat(skills): add auto-skill curator (#7846) ... * feat(skills): add auto-skill curator * fix(i18n): translate /curator command descriptions for zh and zh-TW The strict-parity locale coverage test failed because the new /curator command and its status/run/restore subcommands fell back to English descriptions in zh-CN and zh-TW. Add Simplified and Traditional Chinese translations for the curator command descriptions and its user-facing output strings. * fix(i18n): add English source keys for /curator command The strict-parity i18n check failed because zh and zh-TW defined the new /curator command keys while en.js (the source of truth) did not, producing extra-key parity errors. Add the matching English source entries so all supported locales share the same key set. * fix(skills): align curator lifecycle safeguards * fix(skills): harden curator trust and name guards * fix(skills): guard curator mutations by workspace trust * test(skills): cover curator stale-to-active reactivation path * test(skills): cover curator rollback and restore-collision paths Add coverage for two previously untested error paths in the auto-skill curator: - restoreArchivedAutoSkill refusing to overwrite an existing active directory, leaving both the reused directory and the archived copy intact. - runAutoSkillCurator rolling back an archive rename when the post-move state persistence fails, returning the skill to the live library and leaving nothing stranded in the archive (new isolated test file that mocks atomicWriteJSON to fail once). * test(cli): cover curator command errors and stacked auto-skill usage - Add mockRejectedValue error-path tests for the /curator status, run, restore, and pin commands, asserting each surfaces messageType 'error' and that a failed run/restore skips skill-discovery refresh. - Add positive stacked auto-skill tests to both the non-interactive and interactive slash-command paths, asserting recordAutoSkillUsage is called once per successful stacked skill carrying project skillDetail. * fix(skills): reject control-byte auto-skill directory names isManagedDirectoryName only checked the auto-skill- prefix and basename, so a crafted directory whose name embeds ANSI/control bytes was treated as a managed skill and its name printed verbatim by the non-interactive /curator output (which, unlike the TUI, does not run escapeAnsiCtrlCodes), enabling terminal control-sequence injection from a cloned repo. Require the directory name to match SKILL_NAME_PATTERN. A managed dir is always auto-skill-<name> where <name> passes validateSkillName and the prefix chars are within the same charset, so this never rejects a legitimately generated directory (including Unicode skill names, which an ASCII-only guard would wrongly drop) while excluding ESC/control bytes. Add a regression test covering a crafted directory with a valid manifest name so only the directory-name guard can exclude it. * fix(skills): guard curator state reads and clarify restore errors Align the curator state read path with the noFollow/lstat guards every write already uses: refuse a symlinked or non-regular-file state file (which could otherwise be followed to an external path, /dev/zero, or a FIFO, causing OOM or a boot hang in untrusted workspaces) and cap the read size. Also distinguish a present-but-ineligible archived skill from a missing one in restore error messages. * fix(skills): close curator TOCTOU reads and preserve seeding baseline Address review feedback on the auto-skill curator: - readManagedSkill previously read the manifest via Promise.all([lstat, lstat, readFile]); the readFile ran concurrently with the lstat guards, so a symlinked SKILL.md pointing at /dev/zero could start an unbounded read before the guard rejected it. Read the manifest with O_NOFOLLOW + an fstat size bound instead (shared readRegularFileNoFollow helper), refusing symlinks atomically and bounding the read. - readState had the same lstat->readFile TOCTOU window; the O_NOFOLLOW read closes it while keeping the existing friendly error messages. - First-run seeding overwrote firstSeenAt/lastActivityAt with now even when recordAutoSkillUsage had already created a record, resetting the inactivity clock. Preserve an existing baseline (like useCount/pinned/ lastUsedAt), while a brand-new skill still gets a fresh now baseline. Adds regression tests for the seeding-baseline preservation and for refusing a symlinked manifest. * fix(core): harden auto-skill curator per review feedback (#7846) - Resolve node:fs constants lazily so importing the curator does not crash tests that mock node:fs without a constants export. - Preserve the original error via cause when a rollback also fails. - Apply the skill-name charset guard to archived directory names reserved in the review-agent task prompt. * test(core): cover curator restore rollback and re-read guard (#7846) * fix(core): record auto-skill usage on re-invocation (#7846) * fix(skills): preserve curator read failures * fix(skills): harden curator lifecycle guards * test(skills): cover curator safety paths * fix(skills): address curator review findings (#7846) - Ignore future manifest mtimes in lastActivityMs so a bogus timestamp cannot make a skill permanently un-curatable - Skip archived status entries whose directory is also live, preventing contradictory double-listing in /curator status - Check the weekly interval before acquiring the cross-process lock in maybeRunAutoSkillCurator so most boots skip the lock entirely - Use handle.readFile() instead of a single handle.read() to avoid silent truncation on short reads * fix(cli): localize curator messages * test(skills): cover curator usage safeguards * fix(skills): address curator review feedback (#7846) - Isolate per-skill rename failures so a transient error no longer aborts the whole pass and prevents state persistence (boot-loop fix) - Make usage recording fire-and-forget (void instead of await) since it is already best-effort and nothing consumes the result - Skip state file creation when no auto-skills exist - Prune dead records whose directory exists in neither root - Sanitize user-supplied directory names in error messages (JSON.stringify) to close the ANSI control-sequence echo path - Split reserved skill names into active/archived lists in the review-agent task prompt - Make collision output actionable with remediation guidance - Fix rollbackMoves mutating its argument (moved.reverse → copy) - Add null guard to isMissing for non-object rejections - Add locale keys for skippedErrors output (9 locales) --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Autofix <qwen-autofix@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-08-01 07:37:20 +00:00
ChiGao	907c7dea70	fix(cli): stabilize thinking block height, replace transcript overlay with inline Ctrl+O toggle (#8077) ... * fix(cli): hide streaming thinking preview, rebind Ctrl+O to inline fullDetail toggle The streaming thinking block showed a 4-line preview that varied in height due to empty lines in the model's reasoning output, causing constant page reflow and flicker during generation. Changes: - ThinkBody now renders nothing when collapsed (both streaming and committed), keeping the block at a stable 1-line header height. - Ctrl+O now toggles inline fullDetail mode (like Claude Code): all thinking blocks, tool groups, and tool results expand/collapse in the main conversation view — no alternate-screen overlay. - Alt+T preserved as hidden shortcut (same toggle, not shown in UI). - MainContent passes fullDetail to HistoryItemDisplay via the existing ThoughtExpandedContext, so the toggle works in both VP and Static rendering paths. - Removed TranscriptView overlay rendering, transcriptItems memo, StreamingContext import, and EMPTY_HISTORY_ITEMS constant. - Removed dead code: tailVisualLines, grow-only height tracker, MAX_STREAMING_THINKING_VISUAL_LINES, openTranscript callback. * refactor(cli): remove orphaned transcript full-detail infrastructure (#8077) * fix(cli): update Ctrl+O help text and add thinking-expansion integration test (#8077) * fix(cli): update docs, help text, and remove transcript dead code (#8077) * fix(cli): strengthen Ctrl+O full-detail tests and refresh stale docs (#8077) * fix(cli): address review feedback on test isolation, dead i18n keys, and missing negative case (#8077) * fix(cli): add M1 mutation-killing assertion to Ctrl+O test (#8077) The existing test only asserted clearTerminal (a refreshStatic side effect) and never verified the setThoughtExpanded state flip. Under the vi.mock('ink') harness the mocked App never re-renders from a directly-called handler, so a behavioural allExpanded assertion is not possible. Add a structural guard on the handler source that fails when setThoughtExpanded is removed (mutation M1 verified). * fix(cli): strengthen Ctrl+O toggle assertion and document non-VP redraw (#8077) Tighten the structural M1 assertion from .toContain to a regex that matches the (prev) => !prev updater pattern, catching mutations like (prev) => true that the old check would miss. Document the non-VP scrollback redraw in keyboard-shortcuts.md per maintainer request. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com> 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-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com>	2026-08-01 02:39:53 +00:00
destire-mio	412eae24b4	feat(core): add project-level fork profiles (#8148) ... * feat(core): add project-level fork profiles * fix(core): harden fork profile loading --------- Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com>	2026-08-01 02:20:51 +00:00
destire-mio	4dc50b18e9	feat(memory): protect pinned files during forked Dream (#7714) ... * feat(memory): protect pinned files during forked Dream * fix(memory): harden pinned path protection * perf(memory): avoid repeated pinned path resolution * fix(memory): protect pinned memory during extraction --------- Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>	2026-08-01 00:55:45 +00:00
Baby Blue Viper	e3479a6251	docs: worked example for a PreToolUse HTTP hook backed by an external judgment service (#8202) ... * docs: worked example for a PreToolUse HTTP hook backed by an external judgment service The existing remote-security-check config example points at a service that has to already exist, without showing what that service actually looks like. Adds a minimal, stdlib-only, runnable adapter (invinoveritas /review as the judgment source) implementing the exact contract PreToolUse HTTP hooks expect -- verified live against the real production API, not just written to look plausible: a genuinely destructive shell command returns permissionDecision: "deny" with a real explanation, a benign one returns "allow", and the adapter fails open on any judgment-service-side error so an outage never blocks legitimate tool calls. * docs: address review feedback -- disclose affiliation, note swap point clearly * docs: fix timeout mismatch, add data-handling note, make backend URL configurable, log fail-open state --------- Co-authored-by: babyblueviper1 <babyblueviper1@users.noreply.github.com>	2026-07-31 13:56:33 +00:00
sunday	cf547b6a3c	feat(hooks): add SessionDelete event (#8059) ... * feat(hooks): add SessionDelete event * test(hooks): cover SessionDelete failure paths * fix(hooks): standardize SessionDelete failure handling * fix(hooks): include session id in SessionDelete hook failure logs (#8059) * docs(hooks): note that transcript_path is empty over ACP for SessionDelete (#8059) * refactor(cli): deduplicate SessionDelete hook dispatch (#8059) * fix(cli): keep SessionDelete hook dispatch ACP-safe * fix(cli): align SessionDelete short description with other references (#8059) --------- Co-authored-by: 欢伯 <ri.xur@alibaba-inc.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-ci-bot <qwen-code-ci-bot@users.noreply.github.com>	2026-07-31 13:05:44 +00:00
Dragon	c50137cf86	fix(core): prevent subagents from asking users (#8219)	2026-07-31 12:39:04 +00:00
OrbitZore	153d781a34	feat(gitlab-channel): add transient 👀 award emoji while agent is working (#8119) ... * feat(gitlab-channel): add transient 👀 award emoji while agent is working Adds a working-reaction feature to the GitLab channel adapter, mirroring the GitHub adapter's eyes reaction (PR #8061). When the agent starts processing a note mention, a 👀 award emoji is added to the note; it is removed when the run completes, fails, or is cancelled. Both operations are best-effort and never block the response. Also replaces the custom Todo interface with gitbeaker's TodoSchema, introduces GitlabTarget to consolidate target info, and simplifies processTodo/buildMetadata signatures by passing the parsed target directly instead of redundant targetType/threadId parameters. Co-Authored-By: Qwen Code <noreply@alibaba.com> * fix(gitlab-channel): guard null target and fix reactions leak Restore null guard for todo.target (dropped in the TodoSchema refactor) to prevent TypeError when GitLab returns a todo without target. Use try/finally instead of try/catch for reactions cleanup so entries are removed on all handleInbound return paths, not just throws. Co-Authored-By: Qwen Code <noreply@alibaba.com> * test(gitlab-channel): add poll-driven reaction tests, fix fragile sequencing Add ReactingGitlabChannel that drives real pollOnce → handleInbound → onPromptStart/onPromptEnd path, covering #note_ parse, key derivation, and finally cleanup. Replace the tautological description-mention test with one that exercises the real isNoteMention guard. Replace bare Promise.resolve() microtask waits with vi.waitFor in the award-failure test. Co-Authored-By: Qwen Code <noreply@alibaba.com> * test(gitlab-channel): pin double-award guard with dedup test Add test that calls startPromptForTest twice on the same messageId and asserts award is called exactly once, pinning the `|| entry.award` guard in onPromptStart against surviving mutations. Co-Authored-By: Qwen Code <noreply@alibaba.com> --------- Co-authored-by: Qwen Code <noreply@alibaba.com>	2026-07-31 10:45:38 +00:00
易良	9a4e924cf1	fix(github-channel): retry definite no-write deliveries (#8087) ... * fix(github-channel): retry definite no-write deliveries * fix(github-channel): preserve concurrent pending deliveries * fix(github-channel): harden pending retry updates * fix(github-channel): avoid duplicate pending retries on reconnect * fix(github-channel): audit recovered deliveries before cleanup * fix(github-channel): avoid duplicate recovered comments * fix(github-channel): skip malformed audit entries * fix(github-channel): bound pending retry recovery * fix(serve): restore session service import * fix(serve): remove unused session service import * fix(github-channel): harden pending delivery recovery * docs(github): update channel state paths * fix(github-channel): audit ambiguous pending retries * fix(github-channel): guard legacy state migration * fix(github-channel): avoid pending delivery id collisions	2026-07-31 08:06:28 +00:00
destire-mio	079ce5346a	feat(agent): add fork tool execution allowlist (#8066) ... * feat(agent): add fork tool execution allowlist * fix(agent): address fork allowlist review feedback --------- Co-authored-by: destire-mio <248462155+destire-mio@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-30 13:58:51 +00:00
zjgzx1988	953c9d8177	feat(core): tag UserPromptSubmit hook context and record display provenance (#7956) ... * feat(core): tag UserPromptSubmit hook context and record display provenance UserPromptSubmit additionalContext was appended to the request as a bare text part and persisted verbatim, so hook-injected text was indistinguishable from user-authored text in the transcript, polluted resumed sessions, telemetry, and auto-memory recall queries. - Wrap injected context in a reserved <qwen:user-prompt-submit-context> tag (hook output already escapes angle brackets, so the tag cannot be forged from inside). - Record the pre-injection user prompt as systemPayload.displayText plus the injected string as hookContext on the user record; the model-bound message stays verbatim for faithful resume replay. - Use the pre-injection prompt text for telemetry prompt attributes and managed auto-memory recall. - Resume projection prefers displayText, strips a trailing whole-part tagged block when no payload exists, and leaves legacy bare-injected records unchanged. - Apply the same tag wrapping on the ACP session injection path, which already records the pre-injection prompt. Closes #7940 Co-authored-by: Cursor <cursoragent@cursor.com> * docs: note UPS promptText TDZ ordering and sole-part resume guard Document the conflict-resolution constraint that promptText must be declared before the injection assignment, and the sole-part read-path guard that keeps a user-authored whole-tag message intact. Co-authored-by: Cursor <cursoragent@cursor.com> * test(cli): cover at_command resume with tagged UPS context Confirm the at_command branch still prefers payload.userText when a paired user record carries a trailing tagged hook-context part, and falls back to the tag-stripping projection only when userText is absent. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(core): address PR 7956 review findings and Goal recording spy Omit the optional UserPromptRecordPayload third arg when no hook injected, so Goal admission spies expecting two args stay exact and CI client-goal.test.ts passes. Project plain UserPromptSubmit-augmented records through transcript-replay with the same displayText / trailing-tag strip fallback as the TUI, covering ACP/export surfaces. Strengthen the displayText preference fixture so it disagrees with the tag-strip path, and use the named UserPromptRecordPayload type in resume. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(acp-bridge): import UPS tag helper via Node-free package export transcript-replay is inlined into the browser daemon/transcript SDK bundle. Importing isUserPromptSubmitContextPartText from the core package barrel pulled the whole Node-bound core graph into that bundle and failed CI (esbuild Could not resolve "node:*") across Test, web-shell E2E, and Real daemon E2E. Export the pure helper as @qwen-code/qwen-code-core/userPromptSubmitContext and import that path instead. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(test): alias userPromptSubmitContext for Vitest source resolution CLI and acp-bridge Vitest configs already map goalWire/transcriptRecords to TypeScript sources; without the same alias the new package export fails import analysis and breaks dozens of CLI suites. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(acp-bridge): keep images when projecting displayText user records Preferring UserPromptSubmit displayText previously returned early and skipped projectMessageParts, dropping multimodal inlineData. Rebuild parts so displayText replaces text while images keep their order. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(core): drop unused hookContext and cover image-only displayText UserPromptRecordPayload.hookContext had no read sites; keep displayText only and recover injected text from the tagged message part. Also cover the image-only !replaced append path and simplify the recording guard. Co-authored-by: Cursor <cursoragent@cursor.com> * test: cover remaining UserPromptSubmit provenance Suggestions Share stripTrailingUserPromptSubmitContextPart between TUI resume and ACP replay, assert ACP Session tags additionalContext, and lock telemetry to the pre-injection prompt text. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> 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> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-30 11:45:23 +00:00
易良	f005f3eee8	feat(github-channel): add transient working reaction (#8061) ... * feat(channels): acknowledge GitHub requests * fix(channels): remove transient GitHub reactions * test(github-channel): cover reaction failures * test(github-channel): cover pending reaction dedup * docs(github-channel): clarify final-only output --------- 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-30 03:00:29 +00:00
Shaojin Wen	0a9db38221	feat(review): add review run — headless review with a machine-readable verdict (#7983) ... * feat(review): add `review run` — headless review with a machine-readable verdict The review pipeline already runs non-interactively: `qwen --prompt "/review …"` expands the bundled skill, launches the dimension agents, and honors the approval mode. What that path lacks is a contract. The verdict lives in the model's prose and in files whose names the caller must simply know, the exit code says nothing about the outcome, and piped stdin silently defeats slash-command detection (the runner prepends piped input, so the leading `/` is no longer first). Anyone who wants "run a review, tell me what it decided" ends up scraping a terminal. `qwen review run [target]` is that contract and nothing more. It assembles the /review invocation from typed flags (--effort, --comment), re-enters this build's own CLI in a child process with stdin closed, streams the child's progress to stderr, and then reads the verdict from the artifact compose-review wrote — the same JSON the skill treats as the verdict authority — never from anything the model printed. stdout carries only the result (human lines, or the full JSON with --json). Exit codes make the outcome scriptable without parsing: 0 = the review completed (whatever it decided), 1 = it never reached a verdict (child failure, timeout, or no composed artifact — a clean child exit without one is a run that wandered off, not an approve), 3 = completed AND --fail-on request-changes AND the event is REQUEST_CHANGES, so a CI gate can tell "blocking verdict" from "the tool broke". Artifact discovery is scoped to this run (mtime cutoff with a small slack for coarse filesystem clocks): a stale composed JSON from an earlier review says whatever THAT review decided, which is exactly the wrong thing to republish. * fix(cli): harden review run against EPIPE, target injection, and drift (#7983) - Use writeStderrLineSafe in the timeout and spawn-error handlers and guard the progress stream, so an EPIPE on stderr can no longer skip the child kill, hang the promise, or orphan the review. - Reject a review target carrying whitespace or a leading dash before it is re-tokenized by the child CLI (e.g. `123 --comment` silently authorising posting). - Constrain --approval-mode to the same choices as the top-level CLI. - Capture the child's exit signal and surface it (OOM/SIGKILL vs spawn fail). - Sync the top-level `qwen --help` review description with the command. - Register `run` in the review.test.ts subcommand expectation and add tests for the timeout branch, the readComposed guard, and target rejection. * fix(cli): kill process group on review run timeout, harden edge cases (#7983) The CLI relaunches itself in a child process (for --max-old-space-size), so child.kill() only reached the relaunch wrapper — the real review was reparented to PID 1 and kept burning API calls. Spawn with detached:true and kill the process group (-pid) so the timeout actually terminates the review. Also: clamp negative --timeout-minutes to a 1-minute floor, distinguish a corrupt composed artifact from a missing one in human-readable output, and add test coverage for the default (non-JSON) output path. * fix(cli): use specific MockInstance type for process.kill spy (#7983) * fix(cli): capture review run verdict before cleanup, forward signals (#7983) * fix(cli): reject quoted review targets, pin signal forwarding (#7983) * fix(cli): keep captured review verdict when timeout fires after compose (#7983) --------- Co-authored-by: verify <verify@local> 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-30 02:40:10 +00:00
易良	3d5924bd2f	fix(github-channel): validate and document reasonFilter (#8035) ... * feat(github-channel): add reasonFilter config to skip unwanted notification reasons Adds an optional `reasonFilter` allowlist to the GitHub channel config. When set, notifications whose `reason` is not in the list are skipped before any lane dispatch, reducing unnecessary API calls and agent work for notification types the operator does not care about. - New `reasonFilter?: string[]` field on `GithubConfig` - O(1) Set lookup (`reasonFilterSet`); undefined = no filter (all reasons) - Early-skip in the poll loop, before subject URL extraction and lane dispatch - Two tests: filtered reasons skipped, unset filter processes all Default behavior is unchanged (undefined = process all reasons). * fix(channels): log github reason filter skips * fix(channels): validate github reason filter * fix(channels): address github reason filter comments * fix(github-channel): reject invalid reason filters * fix(github-channel): validate reason filters on connect --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>	2026-07-30 01:17:44 +00:00
Nothing Chan	a771e4449e	fix(channels): reject unusable GitHub self-allowlists (#8055)	2026-07-29 15:48:56 +00:00
Mark Xian	c19d321d1f	feat(github-channel): filter notification reasons (#8031)	2026-07-29 15:34:16 +00:00
OrbitZore	ec9c36ef82	feat(channels): add GitLab polling channel adapter (#7862) ... * feat(channels): add GitLab polling channel adapter Poll GitLab todos via @gitbeaker/rest, dispatch notes through the existing PollingChannelBase pipeline. Key design points: - action_prompt_template config drives event filtering and metadata rendering (unconfigured actions are skipped) - Per-repo cursor (repo[chatId].last_read) as notes window lower bound, global lastProcessedAt for todo-level dedup - mark_done after successful processing; failure skips mark_done for retry on next poll - Mention gating delegated to base GroupGate (adapter only sets isMentioned flag) - First-contact body fallback for todos with no notes (e.g. mention in issue description) * fix(channels/gitlab): persist cursor after each successful todo Call saveCursor() immediately after advancing lastProcessedAt so that progress is durable even if the process crashes mid-poll. Also removes the local watermark variable in favor of direct assignment. * fix(channels/gitlab): persist cursor on every advancement including skips * fix(channels/gitlab): address review critical issues - Remove non-functional proxyAgent (gitbeaker doesn't support it) - Construct repo_url from host + path (API doesn't return web_url) - Handle directly_addressed action (falls back to mentioned template) - First-contact fetches target description instead of using todo.body - Move todo.project dereference inside try block - Filter confidential notes - Update channel-registry.test.ts for gitlab entry * fix(channels/gitlab): address review suggestions - Warn on connect if action_prompt_template is not configured - Guard todo.target.iid before use - Skip paths now mark_done (best-effort) to clean GitLab UI - Remove postErrorComment (avoids duplicate comments on retry) - Fetch only first page of notes (desc, maxPages:1, perPage:100) instead of paginating entire note history - Extract fetchRecentNotes for single-page windowed enumeration * refactor(channels/gitlab): simplify to todo.body dispatch, add description mention support - Remove notes API fetching; dispatch todo.body directly - Detect description mentions via target_url anchor (#note_ absence) - Always fetch target description for %description% metadata - Remove per-repo cursor; dedup via cursor + mark_done only - Cursor advances regardless of success/failure (no retry) - Use zod for cursor validation - Rename template vars to GitLab terminology: %project% %project_url% %target_type% %iid% %title% %description% %todo_id% - Support %% escape for literal percent * docs(channels): add GitLab adapter documentation - New user guide: docs/users/features/channels/gitlab.md - Update _meta.ts navigation - Update developer adapter matrix and SDK list * fix(channels/gitlab): use correct Issues.show(issueIid, { projectId }) signature * chore: regenerate NOTICES.txt for new gitlab channel dependencies * fix(channels/gitlab): address review suggestions - Add todo.project null guard (item 2) - Single-pass regex for %% escape + %var% substitution (item 4) - sendThreadMessage throws directly on undefined threadId (item 5) - Dedup fetchDescription with per-poll cache (item 6) - Remove per-todo saveCursor; base class saves after pollOnce (item 7) - Add undefined threadId test (item 8) - Expand confidential notes limitation in docs (item 3) * test(channels/gitlab): add mention tests, directly_addressed coverage, skip assertions, temp cleanup - New mention.test.ts: 14 cases for testBotMention/stripBotMention/escapeRegex - Add directly_addressed fallback test - Skip tests now assert TodoLists.done + cursor advancement - afterEach cleans up mkdtempSync temp dirs * fix(channels/gitlab): address review round 4 - Non-mention actions (assigned, etc.) set forceMentioned=true to bypass GroupGate - Merge dead note-filter tests into single 'skips todo authored by bot' - Log fetchDescription errors to stderr instead of silent swallow - Post error comment on issue/MR when handleInbound fails (best-effort) * fix(channels/gitlab): always force isMentioned=true, remove regex re-derivation The action_prompt_template config is already the event filter, and GitLab has already decided the mention when creating the todo. Re-deriving isMentioned via regex on todo.body causes permanent message loss when the regex misses (description mention + fetch failure, group mentions). Always set forceMentioned=true so GroupGate never drops a todo that passed the template filter. * fix(channels/gitlab): propagate fetchDescription errors for description mentions For note mentions, description is metadata-only — fetch failure is logged and swallowed. For description mentions, description IS the message — fetch failure now propagates to the outer catch, which posts the ⚠️ error comment so the user knows to re-mention. * perf(channels/gitlab): clean up stale todos, skip unnecessary fetchDescription - Mark stale todos (updated_at <= cursor) as done on each poll to prevent perpetual re-fetching of pre-existing pending todos - Skip fetchDescription for note mentions when template does not contain %description%, saving one API call per todo - Update docs: stale todo cleanup, error comment on failure * docs(channels/gitlab): clarify requireMention is bypassed, template is the real filter * Apply suggestions from code review Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(channels/gitlab): use todo ID cursor instead of timestamp to eliminate equal-timestamp loss Timestamp-based cursors (second granularity) could silently destroy todos sharing the same updated_at as the cursor boundary. Switch to monotonically increasing todo IDs which are unique and collision-free. Add initialized flag to preserve first-start drain semantics: pre-existing pending todos are marked done without dispatch on the first poll cycle. * fix(channels/gitlab): harden first-poll drain, add ordering tests, fix lockfile - Replace Math.max(...spread) with reduce to avoid RangeError on large backlogs (~100k+ todos). Move initialized=true after the drain work so any throw retries the drain instead of falling through to dispatch. - Add unit tests: identical-timestamp delivery and id-order-when-updated_at-disagrees (kills M2 sort mutant). - Align lockfile: file:../base → ^0.21.0 for channel-base dep. * fix(channels/gitlab): include dot in mention lookahead for GitLab usernames GitLab usernames may contain dots (e.g. bot.name). The lookahead character class inherited from GitHub omitted '.', causing @bot.name to match as @bot. Add '.' to the negated class. * docs(channels/gitlab): align docs with ID cursor and drain semantics - Add first-poll drain as step 2 in How It Works - Clarify GroupGate always passes (isMentioned forced true) - Document initialized flag in Known Limitations * Apply suggestions from code review Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> * fix(channels/gitlab): align package version and channel-base dependency to 0.21.1 Bump version from 0.21.0 to 0.21.1 to match other channel packages after upstream merge. Pin @qwen-code/channel-base to exact 0.21.1 instead of ^0.21.0, matching the convention used by other published channels. * fix(channels/gitlab): regenerate lockfile to match package.json versions Manually add only gitlab-related lockfile entries (workspace, @gitbeaker packages, transitive deps, channel-gitlab link) without unrelated npm normalization churn. * test(channels/gitlab): add regression tests for first-poll drain hardening Two tests that kill the M1 (Math.max spread RangeError) and M2 (flag ordering) mutants which survived the original 46-test suite: - 150k todo drain verifies reduce() handles large backlogs without RangeError and without dispatching - Drain throw verifies initialized stays false so the next poll retries the drain instead of falling through to dispatch Test file duration: ~40ms → ~170ms. * docs(channels/gitlab): clarify groupPolicy must be "open" and add runtime warning The default groupPolicy "disabled" silently drops all mentions — todos are marked done and cursor advances, but no dispatch occurs. Fix misleading docs that said "GroupGate always passes" (only true at groupPolicy: "open") and add a connect()-time warning when groupPolicy is not "open". * fix(channels/gitlab): correct xcase integrity hash in lockfile The manually added xcase entry had a typo in the sha512 hash (ys → ks), causing npm ci EINTEGRITY failures in CI. * fix(channels/gitlab): correct requester-utils integrity hash in lockfile Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(channels/gitlab): allow groupPolicy "allowlist" in warning and docs The groupPolicy warning and docs incorrectly stated that groupPolicy must be "open". In reality "allowlist" with the project listed also works because isMentioned is forced true and GroupGate only requires the group to be listed. Also fix the inaccurate "no error is logged" claim — ChannelBase logs preflight rejected reason=group_disabled. Fixes R5-🟡3 from PR #7862 review. --------- Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>	2026-07-29 14:28:31 +00:00