Commit graph

6376 commits

Author SHA1 Message Date
github-actions[bot]
cbda99bb9b chore(release): v0.21.0-nightly.20260727.c003e1718 2026-07-27 00:43:59 +00:00
易良
d44030a4c0
feat(core): add model grade selection for subagent spawn (#7685) (#7702)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* docs: add design placeholder for subagent model grade selection (#7685)

* feat(core): add subagent model grade selection

* test(subagent): cover resolveModelGrade deep guards and resume else branch

- subagent-manager: add tests for non-string grade values, blank values,
  array-shaped modelGrades, and missing modelGrades (all return undefined)
- background-agent-resume: assert configured subagent model is preserved
  (not forced to 'inherit') when launch flags (model + authType) are absent

Addresses test-coverage review findings.

* refactor(subagent): extract normalizeModelGradeSettings and merge model validate

- Extract normalizeModelGradeSettings helper shared by resolveModelGrade
  and the Agent tool schema build, so the advertised grades and runtime
  resolution cannot drift (addresses duplicated shape invariant).
- Merge the three model-parameter validate branches under a single
  `params.model !== undefined` guard.
- Update agent.test.ts mock to preserve the real helper while still
  mocking SubagentManager.

* refactor(core): simplify model grade resolution

* fix(core): reject unknown model grades

* docs(core): clarify model grade precedence

* docs: explain subagent model grades

* test(core): update subagent manager mock

* fix(core): list available model grades

* fix(core): trim model grade keys and cover schema removal

Grade keys were checked for emptiness via grade.trim() but stored in the
map and advertised in the tool schema enum untrimmed, while values were
trimmed. A padded key like ' small ' published a padded enum name the
model had to reproduce verbatim, and the allowlist check silently
excluded it. Normalize the key before storing, allowlist matching, and
schema publication.

Also adds a test for the delete schema.properties.model branch that fires
when grades transition from available to empty, so a regression that
breaks the delete leaves no stale model enum in the tool schema.

* fix(core): trim allowed model grade filters

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-26 16:23:54 +00:00
Shaojin Wen
d1988fd83d
feat(review): give the verifier a probe capability — run a runnable claim, don't just read it (#7756)
Measured on this repo: read-only verification is where the review misses its
hardest bugs. Handed the exact low-confidence finding for a `!` command that
executes twice, the strongest model's Step-3 agents traced the mechanism and
called it correct (0/3 across the full roster). Validated the fix directly — when
the same models (3.7-max and 3.8-max-preview) were allowed to WRITE AND RUN a
probe, both confirmed the double-execute from observed behaviour
(`sendShellCommand called twice with ["git push"]`), both did the probe-validity
self-check unprompted, and a plausible-but-false negative control was correctly
refuted with no fabrication.

So the verifier's brief now says: when a finding's failure scenario is a runnable
claim about a named unit and the repo has a fast unit harness (vitest/jest/pytest),
write a minimal probe, run it, and let the observed behaviour settle the verdict.
Two rules keep it evidence rather than theatre — a mandatory self-check that the
probe flips between buggy and correct, and leaving the tree exactly as found. A
finding a probe confirmed carries `Source: [probe]`, which compose-review treats
as deterministic (a run produced it) like `[build]`/`[test]`.

This is Phase 1: the agent-driven loop the validation exercised, no new command.
The deterministic runner + artifact and the finding-generation side (emitting a
probeable low-confidence finding rather than concluding "correct") are follow-ups.

Co-authored-by: verify <verify@local>
2026-07-26 16:08:40 +00:00
Yufeng He
ebf8f7510d
fix(core): route id-less continuation chunks to a colliding tool-call opener's slot (#6981)
* fix(core): route id-less continuation chunks to a colliding tool-call opener's slot

StreamingToolCallParser remaps a provider index to a fresh slot when a second
tool call reuses an already-completed index. When that second call's id and
name arrive together on an empty opener delta (the standard OpenAI streaming
shape, function: { name, arguments: "" }), the index -> slot remap was never
recorded, because it was guarded by !meta.id and the id was already set. The
following id-less argument chunks then failed to find the remapped slot and
were routed to a brand-new orphan slot: the tool call was emitted with empty
args {}, the real arguments (in a nameless slot) were dropped by
getCompletedToolCalls, and the nameless slot made converter.ts raise
InvalidStreamError('MALFORMED_TOOL_CALL'), forcing up to four wasted retries of
an otherwise valid response.

Record the remap whenever the actual slot differs from the provider index, not
only before an id is known, so id-less continuations follow it. To keep a
genuinely new tool call from hijacking that slot, only adopt a pending remap
for a new id when the remapped slot has not itself been claimed by an id yet
(the existing id-arrives-late path); otherwise fall through to normal collision
handling.

Adds a regression test for the id+name-on-opener collision shape, which the
existing "late stable ID" collision tests did not cover.

* test(core): cover the block direction of the tool-call remap guard

Adds a case where a brand-new tool-call id reuses an index that already has a
0->N remap to an id-claimed slot: it must not hijack that slot and instead gets
its own. This exercises the `!toolCallMeta.get(remap)?.id` guard directly (the
prior test only covered id-less continuation routing). Fails without the guard.

* test(core): broaden tool-call collision coverage and refresh the remap doc

Per review: add a case for the content-bearing colliding opener (arguments
fragment arriving with the id/name, exercising the line-239 remap-record path
that the empty-opener test skipped), assert call_1's arguments survive the
collision in the id-less-continuation test, and update the pendingIndexRemaps
doc to describe its dual role (post-id adoption and id-less continuation
routing).

* refactor(core): drop the now-dead pending-remap delete on id adoption

Once an id adopts a remapped slot, the common-path re-registration keeps
the index->slot mapping alive for later id-less continuation chunks, so
the delete in the adoption branch is immediately re-created and has no
runtime effect. Remove it and note why the remap is intentionally kept,
so the mapping's lifetime reads straight through.

* test(core): pin id-less continuation routing across three colliding openers

The unconditional remap overwrite is deliberate: an id-less continuation after
the newest colliding opener must route to that opener's slot. A regression test
covers the three-call case so the overwrite is not later 'guarded' back into
misrouting the third call's continuation to an earlier slot.
2026-07-26 15:56:08 +00:00
ovochouovo
60812d4cd3
fix(cli): show tool descriptions in multi-tool compact summaries (#7589)
* fix(cli): show tool descriptions in multi-tool compact summaries (#6014)

buildToolSummary() previously discarded descriptions when 2+ tools of
the same category were grouped, showing only counts like "Read 3 files"
or "Searched 2 patterns". Now shows actual descriptions inline when ≤3
tools (e.g. "Read a.ts, b.ts, c.ts"), and first 2 + "...and N more"
when >3 tools. getActiveToolHint() skips the redundant ⎿ hint line
when descriptions are already visible inline.

* fix(cli): use i18n key for '...and N more' phrase

Use existing '... and {{count}} more' translation key instead of
hardcoded English. This ensures the phrase is localized correctly
for all 9 shipped locales (fr, zh, de, zh-TW, ca, ja, ru, pt, en).

Also updates test expectations to match the i18n format (space before
'and' instead of no space).

Addresses CR suggestion on PR #7589.
2026-07-26 14:53:20 +00:00
jinye
9bdc62c74b
perf(cli): replace comment-json settings parser (#7747)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-07-26 14:42:51 +00:00
Jingyuan Tian
7422bf8797
fix(cli): handle escaped dollars around inline math (#7741)
* fix(cli): handle escaped dollars around inline math

* test(cli): add escaped-dollar TUI capture scenario

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-26 13:58:01 +00:00
Sparkle79
592b3d429d
fix(cli): complete repeated skill slash commands (#7720)
* fix(cli): complete repeated skill slash commands

* fix(cli): scope stacked skill completion

* fix(cli): suppress invalid stacked skill ghost text

* refactor(cli): centralize stacked skill eligibility

* fix(cli): align stacked skill highlighting

* test(cli): cover stacked skill completion guards

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-26 13:56:17 +00:00
Shaojin Wen
7c58f97d72
fix(review): recover bilingual register from the live PR when the plan omits the Han flag (#7739)
* fix(review): recover the bilingual register from the live PR when the plan omits the Han flag

The posted review body renders bilingually (English, with the full Chinese
version collapsed under it) only when the plan `compose-review` reads carries
`prDescriptionHasHan: true`. That flag is written in exactly one place —
`fetch-pr` — and read in exactly one place, from a plan path the orchestrator
supplies. Two gaps follow: a `plan-diff` plan never records the flag, and a run
that improvises the pipeline can hand `compose-review` a plan that is not
`fetch-pr`'s report at all. Either way the switch fails safe to English, and a
Chinese-authored PR gets an English-only review — observed on #7686, where the
four bot reviews off a proper plan were bilingual and the one off an improvised
plan was not.

When the flag is absent but the plan still names the PR (`ownerRepo` +
`prNumber`), recover the signal from the live description with a single
`gh pr view`, and test that recovered text for Han. This runs only on the
absent-flag path: a recorded `true`/`false` is authoritative and spends no
network, so every healthy `fetch-pr` review is unchanged. The signal stays the
CLI's own — the real PR body, which the caller cannot forge — so the recovery
tightens the "caller cannot toggle the register" property rather than loosening
it, and any failure of the fetch falls back to English so the language can never
take the review down.

* refactor(review): reuse roster's isPositivePrNumber in the bilingual recovery

`bilingualFromPlan`'s local `positivePrNumber` duplicated the exact PR-number
validation already in `roster.ts` (positive integer, all-digit string, reject
null/0/empty). Two copies of the same rule invite a silent divergence: a future
change to how a PR number is validated, applied to only one, would make the
bilingual recovery and `requiredAgents` disagree on the same plan's PR identity.
Export the roster helper and reuse it, coercing to the string form the
`gh pr view` call needs at the one call site.

* fix(review): route compose-review's gh call via the PR host on GitHub Enterprise

The bilingual body-language recovery added a `gh pr view` call inside
compose-review, but its CLI handler never called `setGhHost` — unlike
fetch-pr/submit/pr-context/comment-status/presubmit. On a GitHub Enterprise PR
whose plan lacks `prDescriptionHasHan` but carries the PR identity, the recovery
fetch would target github.com, fail, and compose an English-only body that
disagrees with the bilingual body `submit` (which does route by host) posts.

Give compose-review a `--host` option and call `setGhHost(host)` in the handler,
mirroring the sibling subcommands; add it to the skill's Enterprise host list and
the Step 6 invocation. Covered by a test that drives the handler with --host and
asserts the routing took.

* fix(review): strip the prBodyFetcher test seam at the compose-review boundary

`prBodyFetcher` is a unit-test seam, but unlike `env` it was not stripped from
the model-written state JSON. A state JSON carrying `"prBodyFetcher": "suppress"`
survives `JSON.parse`, reaches `bilingualFromPlan`, is called, throws, and drops
the Chinese fold through the fail-safe — letting the caller suppress a fold that
the plan's own signal would have rendered. Strip it in the handler the same way
`env` is stripped, and correct the field doc, which wrongly claimed a model
could not supply one.

* fix(review): strip prBodyFetcher at the submit boundary; pin fetchPrBodyViaGh and handler stripping with tests (#7739)

* fix(review): pin the submit-boundary prBodyFetcher strip; soften SKILL.md wording (#7739)

---------

Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
2026-07-26 13:54:49 +00:00
jinye
06df2410b6
fix(core): reliably deliver manual plan-exit notices (#7744)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-07-26 13:25:56 +00:00
Shaojin Wen
471141fcde
fix(review): correct the borrowed lenses and vacuous-test severity (follow-up to #7735/#7736) (#7746)
* fix(review): correct and tighten the borrowed lenses per review

Four findings on the lenses this PR adds:

- **Agent 2 (subprocess injection), Critical.** The guidance said "terminate the
  argv with `--`", but `--` ends *option* parsing without neutralizing a
  *pathspec* — for an overloaded command it creates one (`git checkout -- release`
  restores a path instead of switching branch; `git checkout -- .` still discards
  changes). Reword: validate against the subcommand grammar; a `--` helps only
  where it keeps the operand's role, and the value allowlist is what closes it.
- **Agent 1b (changed literal).** A default ripgrep skips hidden `.github/**`, so
  a marker consumed only by a workflow reads as "no consumer". Require a
  hidden-path search (`rg --hidden --glob '!.git/**' --fixed-strings`).
- **Agent 4 (unreproducible benchmark).** "Flag as unverified" conflicts with the
  actionable-findings-only contract. Make it actionable when load-bearing (request
  the script/env/raw numbers) or no finding when incidental (record under
  not-verified), never a non-defect finding.
- **Agent 5 (equivalent mutant).** Add a focused `buildRoleBrief(PLAN, '5')` test
  pinning the equivalent-mutant rule and its discriminating-input requirement, so
  a prompt-assembly regression cannot silently drop it.

* fix(review): a vacuous test is a Suggestion, not a Critical for being the sole guard

Agent 5's mutation lens graded a sole-guard vacuous test as Critical, but the
shared severity ladder — and Agent 7's deterministic efficacy probe — grade an
ineffective test as Suggestion. Step 4 keeps the higher severity, so the same
inert guard arrived as Critical from Agent 5 and Suggestion from Agent 7, and
the Critical won: a PR could be blocked solely for lacking an *effective* test,
the exact inflation those shared rules exist to prevent.

Align it with the dimension's own "name the bug, not the gap" rule: a vacuous
test is a Suggestion, escalated to Critical only when it asserts the opposite of
the intended behaviour, was weakened in-diff, or lets a specific incorrect
behaviour ship (in which case that behaviour is the Critical, with the test as
evidence). Mirrored in the test-matrix brief and the SKILL.md dimension table,
and pinned by a buildRoleBrief(PLAN, '5') assertion so the semantic reversal
cannot pass the generic word-presence check again.

* fix(review): pin the Agent 2 and test-matrix brief corrections, sync the SKILL table

---------

Co-authored-by: verify <verify@local>
2026-07-26 13:22:35 +00:00
Nothing Chan
a68b9c12d4
fix(core): redact the plan argument from history after an approved exit_plan_mode (#7197)
* fix(core): redact the plan argument from history after an approved exit_plan_mode

The full plan text a model submits to exit_plan_mode stays in the
conversation history as its own functionCall arguments. On long
conversations models occasionally regurgitate chunks of that blob into
later responses, mixing stale plan sections into answers (#6237).

After an APPROVED exit (keyed off the approval llmContent prefixes, so
rejected/no-action results keep their plan text for revision), the tool
scheduler now swaps the `plan` argument in the model turn's functionCall
for a short pointer to the plan file that Config.savePlan already
persisted. The rewrite is a targeted, immutable replacement by callId in
GeminiChat history: sibling parts and other arguments survive, and the
partial-push markers are unaffected (they compare by index and role).
returnDisplay.plan on the UI side is untouched.

Verified with a PTY E2E against a local fake OpenAI-compatible server
inspecting the exit_plan_mode arguments of every outgoing request:
before, the full plan text rides along on every post-approval request;
after, those requests carry only the reference. The scheduler regression
test fails on the unpatched source. Known limit: the chat-recording
JSONL keeps the original args, so a resumed session re-feeds the full
plan text until its next approved exit.

Fixes #6237

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

* fix(core): verify the saved plan file and redact approved plans on history load

Review follow-up on #7197, addressing both inline findings.

Pointer honesty: savePlanBestEffort swallows filesystem errors, so the
scheduler previously could replace the only remaining in-memory plan
with a pointer to a file that was never written. The redaction now reads
the plan file first and redactApprovedPlanFromHistory requires the
in-history plan to equal the on-disk content byte-for-byte — a missing,
unreadable, or stale file skips the rewrite entirely.

Resume leak: the chat-recording JSONL captures the assistant turn (full
plan argument) before the tool runs, so --resume/--continue re-fed the
text the in-session redaction removed. GeminiChat.setHistory and the
constructor now run a history-wide pass (redactApprovedPlansInHistory)
that rewrites approved exit_plan_mode calls under the same
file-must-match rule. The pointer text moved to a shared
approvedPlanRedactionText helper so the write and load sides cannot
drift. Known bound, stated in code: with several approved plans in one
session the file holds only the last, so earlier calls rehydrate
unredacted — safe, just not minimal.

New tests: scheduler save-failure case (plan retained), pure-function
matrix (approval detection, stale-file guard, non-approval), and
setHistory/constructor wiring both with and without the plan file.
557/557 across the three touched suites.

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

* test(core): cover leader-approved and stale-plan branches; restore orphaned doc

Review follow-up on #7197 (three inline findings): move the two plan-
redaction helpers below redactStructuredOutputArgsForRecording so its
doc comment stays attached; add the expectedPlan-mismatch unit test
(stale on-disk plan blocks the rewrite, matching plan still rewrites);
add a scheduler test for the 'Leader approved.' llmContent prefix so the
teammate approval path cannot silently drop out of the redaction.

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

* fix(core): reattach the recording-redaction doc; canonicalize plan tool names on load

Review follow-up on #7197 (second inline round): remove the duplicated
redactStructuredOutputArgsForRecording JSDoc copies stranded by the
earlier helper move and reattach the single copy to its function; route
every load-side tool-name match through a local ToolNamesMigration
mirror (canonicalPlanToolName — kept local to avoid a geminiChat ->
coreToolScheduler import cycle) so a future exit_plan_mode rename keeps
the resume-time redaction working for sessions recorded under the old
name; add the approved+rejected same-plan-text regression test pinning
the per-call-id approval gate. The per-call-id plan *file* suggestion is
deferred as a follow-up — it changes the Config.savePlan single-file
contract shared with other plan consumers.

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

* fix(core): attach the pointer-text doc to its function; log the skipped load-side redaction

Review follow-up on #7197 (third inline round): the "single source of
the pointer text" JSDoc landed above canonicalPlanToolName instead of
approvedPlanRedactionText — moved to its function; the load-side catch
now emits a debugLogger.debug line when the plan file is unavailable so
a --resume that silently skips the redaction is traceable under DEBUG,
matching the write-side scheduler's logging.

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

* fix(core): trace silent plan-redaction skips on both surfaces

The write side discarded redactApprovedPlanFromHistory's boolean and the
load side discarded redactApprovedPlansInHistory's null, so a no-match
call id, a non-string plan argument, and an expectedPlan mismatch were
indistinguishable from a redaction that ran. Both sides now emit a debug
log when the rewrite leaves history unchanged (review finding #2).

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

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.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-26 11:48:06 +00:00
Heyang Wang
d5a05749bc
fix(cli): keep IME cursor aligned after footer updates (#7711)
* fix(cli): keep IME cursor aligned after footer updates

Preserve the terminal cursor intent across independent UI renders so
macOS input methods anchor their candidate window to the input prompt.

- Submit text input cursor positions using Ink's render-time contract
- Reassert the latest committed cursor before interactive frame writes
- Cover standard, incremental, static, and fullscreen rendering paths
- Keep cursor cleanup and hidden-state behavior intact

* docs(cli): explain render-time cursor submission

Document the non-obvious Ink hook ordering that requires cursor positions
to be supplied during render rather than from another effect.

- Prevent future refactors from reintroducing IME cursor misalignment

---------

Co-authored-by: heyang.why <heyang.why@alibaba-inc.com>
2026-07-26 11:42:06 +00:00
pratik wayase
60be40e8f9
feat(web-shell): persist terminal history pagination errors (#7709)
* feat(web-shell): persist terminal history pagination errors

* fix: address review feedback for pagination error state
2026-07-26 11:31:36 +00:00
Shaojin Wen
27927c46d9
feat(review): borrow maintainer review lenses into the agent briefs (#7736)
* feat(review): mutation-test the tests in the test-coverage pass (Agent 5)

Agent 5 checked that tests exist and cover the diff's paths, but not that
they would actually FAIL if the code were broken. A test that stays green no
matter what the code does is worse than no test: it reads as coverage while
blessing the exact regression it was written to catch.

Add a mutation-testing discipline to Agent 5's brief (and its whole-diff
test-matrix counterpart): for a test this diff adds or changes to pin a value
or behaviour, name the one-line mutation to the code under it that should turn
it red — if none does, the test is vacuous. Call out the recurring shapes:
both sides of the assertion computed the same way so they move together
(`expect(undefined).toBe(undefined)` from a loose/unanchored extraction);
reading only the first of several sites the behaviour spans; an
`expect(x).toBe(x)` tautology; a "does not throw" assertion for a bug that is
a wrong value, not a throw. A vacuous test is a Suggestion on its own, but a
Critical when it is the sole guard for a behaviour this diff changes.

The agent briefs are CLI-generated (lib/agent-briefs.ts), so the behavioural
change is there; the SKILL.md dimension table is synced for the human reader.

* feat(review): add contract-shape, migration, equivalent-mutant, and perf-repro lenses

Borrow four review disciplines a maintainer applies by hand, wiring them into
the agent briefs (CLI-generated) so every review gets them:

- Agent 1b (removed-behavior): a changed *literal* is a contract too — when the
  diff renames/reformats a value a distant consumer matches on its raw shape (a
  marker string, a key, a status code, regex text), grep the old shape, not the
  symbol; a rename that compiles can silently stop matching a filter in another
  file. And a rename/format/schema/default change must handle the data that
  already exists — an un-migrated population is a split-brain (orphaned records,
  double-writes), usually fixed by a two-line legacy fallback.
- Agent 5 (test coverage): before calling a test vacuous, rule out the
  equivalent mutant — a mutation that leaves observable behaviour unchanged is
  not a coverage gap. Name the input that makes the mutation observable.
- Agent 4 (performance): do not take the PR's own numbers on trust. Reproduce a
  cheap deterministic claim (bundle bytes, tree-shake) or report it doesn't;
  flag a runtime benchmark you can't re-run as unverified rather than endorsing
  it.

SKILL.md dimension table synced for the human reader. Briefs only — no control
flow changes.

* feat(review): flag subprocess option/argument injection in the security pass

Agent 2 listed shell/command injection but not the sink `execFile`/`spawn`
leaves open: a user-controlled positional argument to git/gh/tar/ffmpeg that is
reinterpreted as an option or special token. A value starting with `-` becomes a
flag (`git log --output=<path>` overwrites an arbitrary file; `git checkout -f`
discards the working tree) and `.`/`..` becomes a pathspec (`git checkout .`
drops unstaged changes) — none of which shell-free spawning stops. Add it to the
brief with the standard fix (validate + terminate the argv with `--`) and the
call-site-asymmetry tell (one site validates, its sibling does not).

SKILL.md security row synced. Briefs only.

* feat(review): add the sibling-consistency lens to the code-quality pass

When one member of a family of parallel paths carries a validation, guard,
cleanup, or shape-check and its twin does not, the lone exception is usually
accidental and the missing half is a latent asymmetric failure — harmless until
the one input that path sees. Agent 3 now checks that siblings share the guard,
names the divergent one, and escalates to the security pass when the missing
guard is a validation on untrusted input (the `gitCheckout`-validates-but-its-
sibling-does-not shape), rather than filing it as a style nit.

SKILL.md quality row synced. Briefs only.

---------

Co-authored-by: verify <verify@local>
2026-07-26 09:36:05 +00:00
Shaojin Wen
3fda7aebdd
feat(review): mutation-test the tests in the test-coverage pass (Agent 5) (#7735)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
Agent 5 checked that tests exist and cover the diff's paths, but not that
they would actually FAIL if the code were broken. A test that stays green no
matter what the code does is worse than no test: it reads as coverage while
blessing the exact regression it was written to catch.

Add a mutation-testing discipline to Agent 5's brief (and its whole-diff
test-matrix counterpart): for a test this diff adds or changes to pin a value
or behaviour, name the one-line mutation to the code under it that should turn
it red — if none does, the test is vacuous. Call out the recurring shapes:
both sides of the assertion computed the same way so they move together
(`expect(undefined).toBe(undefined)` from a loose/unanchored extraction);
reading only the first of several sites the behaviour spans; an
`expect(x).toBe(x)` tautology; a "does not throw" assertion for a bug that is
a wrong value, not a throw. A vacuous test is a Suggestion on its own, but a
Critical when it is the sole guard for a behaviour this diff changes.

The agent briefs are CLI-generated (lib/agent-briefs.ts), so the behavioural
change is there; the SKILL.md dimension table is synced for the human reader.

Co-authored-by: verify <verify@local>
2026-07-26 08:32:53 +00:00
Shaojin Wen
0446c576f9
feat(review): redefine medium effort as a balanced verified pass (#7733)
* feat(review): redefine medium effort as a balanced verified pass

The old medium was a thin inline Step 3C walk: no subagents, no build/test,
no verification, no comment-status — cheaper than high but structurally unable
to catch a whole class of bugs. Dogfooding measured it missing real Criticals
that high caught, including a compile error CI had already flagged red (medium
never built, so it could not see it).

Redefine medium as a balanced, verified pass — the high pipeline with its most
expensive passes removed:

- Runs the Step 3A/3B fan-out over a reduced dimension set — issue fidelity
  (Agent 0), correctness (1a/1b/1c), security (Agent 2), quality (Agent 3),
  performance (Agent 4), test coverage (Agent 5), and build & test (Agent 7).
  Skips the adversarial personas (6a/6b/6c) and the Agent 8 diff-specialists.
- Loads project rules (Step 2) and runs comment-status like high.
- One verification pass (Step 4). Skips the reverse audit (Step 5), the
  incremental cache, and PR posting (--comment still forces high).
- On a large diff, caps the 3B territory fan-out by re-running plan-diff with a
  coarser --max-chunk-lines.

Measured against high on the same PR it lands at roughly one-third to one-half
the time and tokens. It reliably catches mechanical defects (compile errors,
failing tests — build-test is deterministic) and obvious correctness bugs, but
is not an exhaustive correctness audit: a subtle Critical that only the reverse
audit or the adversarial personas would surface can slip, so security-sensitive
and pre-release reviews should still use --effort high. Because it runs no
reverse audit, compose-review caps a clean medium verdict at Comment; a verified
Critical still yields Request changes.

low (thin inline pass) and high (full pipeline) are unchanged, as are the
effort defaults (high for PRs, medium for local) and parse-args. SKILL.md only.

* fix(review): make the roster and coverage gate effort-aware for medium

The medium (balanced) tier deliberately skips the three adversarial personas
(6a/6b/6c), but `requiredAgents()` added them unconditionally in the Step 3A
(small-diff) roster. So `check-coverage` found no transcripts for the
un-launched personas, flagged them missing, and exited 3 — and the skill says a
non-zero exit halts before Step 4. A medium review of any small diff (the common
case) would stall at Step 3D unless the model improvised past the gate. The
SKILL-only redefinition of medium could not work without this.

Thread the effort level through the roster/coverage machinery:

- `requiredAgents(plan, effort?)` — at `medium`, do not require 6a/6b/6c in 3A.
  High (and the default) still require them; 3B is unchanged (personas are folded
  into the chunk agents there, never required as standalone roles).
- `agent-prompt --roster --effort <level>` — builds the roster from the same
  effort-aware `requiredAgents`, so a medium roster prints only the 9 agents it
  launches instead of the full 12-then-skip.
- `check-coverage --effort <level>` — holds the run to the same reduced set, so a
  balanced review is not flagged for personas it deliberately did not run.

SKILL.md Step 3A/3D now pass `--effort medium` to both commands. Adds a roster
test for the medium 3A drop. No behaviour change at high effort.

* fix(review): carry effort in the plan so every reader agrees, and stop medium escalating itself

The medium tier threaded `--effort` as a flag to `check-coverage` and
`agent-prompt --roster`, but `compose-review` recomputes coverage itself and
was never given it — so on every medium run its body disclosed the three
personas it deliberately skipped as missing coverage, and its stderr FIX lines
told the orchestrator to rebuild the full roster and run a reverse audit,
turning the one mandated repair round into a silent escalation back to high.
A caller-supplied `--effort` was also a roster the caller could shrink, against
the invariant `roster.ts` opens with.

Record the effort in the plan at capture time instead. `fetch-pr`,
`capture-local` and `plan-diff` take `--effort` and write `plan.effort`;
`requiredAgents` reads it from there and the `--effort` flag is gone from
`agent-prompt`/`check-coverage`. The roster, `check-coverage` and
`compose-review`'s recomputation now read one field and cannot disagree —
fixing the personas half for free.

For the reverse-audit half, `verificationGaps` reads `plan.effort`: at medium
the absent reverse audit is a by-design omission that caps a clean verdict at
Comment with an honest disclosure and no FIX line, not a repairable gap. Verify
(Step 4) still runs and is still enforced at medium.

Also: drop the medium 3B coarsening that re-ran `plan-diff` — on a same-repo PR
it fed the diff back through the lightweight path, producing a plan with no
worktree metadata that dropped Agents 7/1c and could clobber the fetch report
Steps 3D/6/7 read. And correct the prose that still said compose-review runs
only at high and that medium findings are unverified.

Tests: roster reads plan.effort; plan-diff records it; agent-prompt --roster on
a medium plan builds the reduced set (the command-boundary the pure-function
test could not reach); compose-review caps a medium verdict at Comment with no
reverse-audit FIX line while still requiring the verifier.

* refactor(review): use the shared ReviewEffort type, and cover the medium coverage path

Two review suggestions on the effort work:

- The `'low' | 'medium' | 'high'` union was inlined in the capture commands
  where `parse-args` already exports `ReviewEffort`; import and reuse it so a
  fourth level cannot drift one copy out of sync.
- Add a check-coverage test that reads the effort from the plan: a medium plan
  whose reduced roster was launched has no missing roles, while the same records
  under a high plan flag the personas — the integration point the roster unit
  test cannot reach, and the one whose regression would exit 3 on every medium
  review.

---------

Co-authored-by: verify <verify@local>
2026-07-26 08:14:23 +00:00
chinesepowered
7959fdb272
fix(core): stop humanReadableCron naming intervals that never happen (#7529)
* fix(core): stop humanReadableCron naming intervals that never happen

humanReadableCron turned any `*/N` step into "Every N ..." without
checking N against the field. parseCron accepts those expressions and
cron creation validates only through parseCron, so the misleading string
reached the user:

  */90 * * * *  -> "Every 90 minutes"  (minute set is {0}: hourly)
  0 */30 * * *  -> "Every 30 hours"    (hour set is {0}: once a day)
  0 0 */40 * *  -> "Every 40 days"     (day set is {1}: monthly)

In-range steps could lie too. The minute and hour fields restart at the
top of the next hour/day, so "every N" only holds when N divides that
unit evenly: */25 fires at :00, :25, :50 and then :00 again, a 10-minute
gap rather than 25.

Show the friendly string only when it is true, and fall back to the raw
expression otherwise, which is what the function already does for
malformed steps. Day-of-month keeps a range check instead of a
divisibility one because months vary in length, so no step there is ever
exactly "every N days".

* fix(core): label a day-of-month step only when it is every day

No */N above 1 on day-of-month keeps its interval across the month
rollover, so the range check was too weak rather than mis-tuned: */15
fires on days 1, 16 and 31, a 1-day gap, and */31 matches day 1 alone.

* fix(core): also fall back when the step spans the whole field

evenStepOf accepted N === unit, so `*/60` on minutes kept "Every 60
minutes" and `*/24` on hours kept "Every 24 hours". Both clear the field
in a single stride, leaving one matching value: they are the hourly
`0 * * * *` and the daily `0 0 * * *` written in a form that invites being
read as something else. Require the step to be strictly smaller than its
unit so those fall back to the raw expression.

Requested in review.

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-26 06:10:24 +00:00
jinye
8fa8085036
perf(core): Lazy-load first-use dependencies (#7686)
* perf(core): Lazy-load first-use dependencies

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

* test(core): Fix simple-git loader mock

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

* test(core): Cover abort during xterm load

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

* fix(core): Address lazy-loader review feedback

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

* fix(core): Validate lazy dependency module shapes

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-07-26 03:04:33 +00:00
yijie zhao
eddee6702c
test(cli): cover bottom-stuck virtualized list behavior (#7652)
* fix(cli): collapse bottom-stuck virtualized list viewport

* fix(cli): address virtualized list review feedback

* test(cli): cover short virtualized list collapse

* test(cli): keep virtualized list coverage only
2026-07-26 02:36:24 +00:00
chinesepowered
f43a2e46c8
fix(web-shell): parse 256-color and truecolor SGR sequences in parseAnsi (#7620)
* fix(web-shell): parse 256-color and truecolor SGR sequences in parseAnsi

parseAnsi read every ';'-separated SGR parameter as a standalone code,
but the arguments of 38/48/58 (extended foreground/background/underline
color) are not codes. `38;5;<n>` and `38;2;<r>;<g>;<b>` were fed back
into the code loop, so:

- `38;5;2` set dim (from color index 2) and produced no color;
- any truecolor value with a zero channel hit the `code === 0` branch
  and wiped the color, bold and dim already set on the line;
- a background such as `48;5;22` fed 22 to the reset-intensity branch,
  silently un-bolding the text.

Consume the 38/48/58 arguments instead of reading them as codes, and
resolve the foreground to hex: 0-15 map onto the existing themed palette,
16-231 onto the 6x6x6 cube, 232-255 onto the grayscale ramp. Background
and underline color are parsed but not rendered (Segment has no such
field) so their arguments still cannot leak into the code stream.

parseAnsi feeds shell tool-output rendering in ToolGroup, so this
affects any 256-color CLI. Adds ansi.test.ts (none existed); the four
new cases fail on the previous parser.

* fix(web-shell): keep the current color when an extended-color sequence is malformed

An unreadable 38/48/58 sequence was assigning its undefined result straight
to the color, so `\x1b[31m\x1b[38;5;300m` dropped the red that code 31 had
already set. Ignore the sequence instead and leave the color as-is; a
well-formed one still replaces it.

Also widen toHex/xterm256 to accept `number | undefined` so the truncated
arguments they are actually handed match their declared types, and drop the
non-null assertions that were hiding that mismatch from the compiler.

Add the regression case the existing malformed-sequence test could not catch:
it started from no color, so a cleared color was invisible to it.

* test(web-shell): pin the 58 branch and the truecolor channel guard

The 58;5;2 case asserted only bold, so dropping 58 from the extended-color
trio left the test green: the leaked 2 argument turns on dim, not bold.
Assert the whole segment instead.

No case used an out-of-range truecolor channel either, so toHex's 0-255
guard was unpinned. Add 38;2;999;0;0 to both malformed-sequence loops.

Both gaps found by wenshao's mutation run in review.
2026-07-26 02:25:20 +00:00
qqqys
df54a7d252
feat(webui): add workspace Channel management hook (#7728)
* feat(webui): add workspace channel management hook

* test(webui): cover manual Channel workspace reload

* test(webui): cover Channel mutation delegation

* test(webui): cover Channel hook failures

* fix(webui): preserve Channel mutation state
2026-07-26 02:17:02 +00:00
OrbitZore
4895726600
fix(channels): use username as senderId in GitHub adapter to fix allowlist gate (#7727)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
* fix(channels): use username as senderId in GitHub adapter to fix allowlist gate

isAuthorizedForSharedSessionTarget compares config.allowedUsers (logins)
against envelope.senderId — but senderId was a numeric ID resolved via
getByUsername, so every allowlisted user was rejected from /who, /clear,
/status, /loop, and channel memory commands.

Fix by using user.login as senderId throughout, assuming GitHub users
don't change usernames. This also removes the getByUsername resolution
step that made connect() non-idempotent on daemon reconnect.

- Remove botUserId field; bot self-filter uses botUsername
- Remove allowedUsers login-to-ID resolution in connect()
- Pass config.allowedUsers logins directly to gate
- senderId in envelopes uses user.login

* fix(channels): normalize allowlist/senderId to lowercase for case-insensitive matching

GitHub logins are case-insensitive but Set.has/Array.includes are not.
Without normalization, allowedUsers: ['Alice'] silently rejects a
commenter whose canonical login is 'alice' — a regression from the old
getByUsername round-trip which normalized casing implicitly.

- Normalize config.allowedUsers and gate to lowercase in connect()
- Lowercase senderId at both envelope assignment sites
- Remove dead != null guard in bot self-comment filter
- Add case-insensitive gate test and connect idempotency test
- Add senderId/allowedUsers comparability guard to dispatch test
- Document username-based allowlist rename risk in security section
2026-07-26 00:21:22 +00:00
hogeheer499-commits
9d19eafa97
fix(core): avoid required tools in DashScope thinking (#7661)
Co-authored-by: JS van Dijk <267467744+hogeheer499-commits@users.noreply.github.com>
2026-07-26 00:16:13 +00:00
Harjoth Khara
ecd86421c5
fix(core): fall back to system rg when bundled ripgrep cannot run (#7203)
* fix(core): fall back to system rg when bundled ripgrep cannot run

The bundled ripgrep is selected on file existence alone, so a binary that
exists but aborts on exec — such as on arm64 kernels with 64K pages — made
the health check throw and degraded the whole session to the JS grep, even
with a working system rg on PATH.

Verify the selection actually runs, and retry with system rg when the
bundled binary fails. When system rg is unusable too, the bundled failure
is reported as the root cause instead of a misleading "rg not found".

Fixes #2676

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

* fix(core): treat an unidentified ripgrep probe as unhealthy

Callers distinguish healthy from unhealthy only by the throw, so a probe
that exits zero without identifying itself as ripgrep read as success. The
new fallback could then cache such a binary and report it as usable.

Also cover the path where neither the bundled nor the system binary exists.

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

* fix(core): keep ripgrep probe diagnostics from being discarded

The health error now carries the probe's exit code and output, so a wrapper
or an unrelated tool named rg is identifiable without rerunning it by hand,
and a failing system rg is logged instead of being dropped for the bundled
error it is reported behind.

Also cover the fallback path where no system rg is installed at all.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-25 18:31:55 +00:00
destire-mio
d8787ba1b8
fix(core): prevent updates to extension-provided agents (#7245)
Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-25 18:29:57 +00:00
samuelhsin
3a6c8e0c03
feat(skills): add overridable default-disabled state (#7357)
* feat(skills): add overridable default-disabled state

* fix(skills): address review feedback on default-disabled PR (#7357)

- Fix disabledChanged comparison in SkillsManagerDialog to use
  previousDisabled (locked names filtered) instead of workspaceDisabled,
  preventing spurious settings writes when a skill is disabled at both
  workspace and higher scope
- Import SettingScope as a value instead of string-casting literals in
  skill-settings.ts for compile-time safety
- Add dual-key change test: enabling a workspace-hard-disabled
  default-disabled skill produces both skills.disabled and
  skills.enabled changes in one operation
- Add legacy inactive-extension branch tests: reject when
  disabledReason is undefined and skill is not in settings
  disablements; allow when it is disabled by settings

* fix(cli): address skills picker review feedback (#7357)

Extract the skills picker's workspace persistence computation into a tested pure function so orphaned workspace disables (skills not currently loaded) are explicitly preserved and pinned by a regression test. Also add an integration test asserting a workspace-scope hard disable surfaces disabledReason 'hard' through the full loadSettings -> resolveSkillSettings -> mapSkillConfigToStatus pipeline.

* fix(cli): resolve skill disablements in safe mode for status API (#7357)

* fix(cli): dynamically import skill-settings in serve to keep fast-path closure clean (#7357)

---------

Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix[bot]@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
2026-07-25 18:29:20 +00:00
曹潇缤
c1c9cd9b4f
fix(qqbot): restore AcpBridge session loading — return input sessionId, patch catch path (#7722)
AcpBridge.loadSession() has been returning response.sessionId since
introduction, but ACP's LoadSessionResponse schema does not include a
sessionId field — so response.sessionId is always undefined.

The QQChannel originally worked around this with fixRestoredSessions()
that reads the raw persist file and patches the router maps. However,
after SessionRouter added validation in PR #5978, restoreSessions()
throws on undefined sessionId, and the .catch() handler never called
fixRestoredSessions().

Before PR #6457 this was masked by WebSocket RESUME — the QQ-level
session recovery meant restoreSessions() was never reached on reconnect.
PR #6457 added tryResume=false on non-1000 close, triggering full
cold-start restore on every reconnect.

Fix:
1. AcpBridge.loadSession() returns sessionId directly (ACP protocol
   design: LoadSessionResponse omits sessionId because client knows it)
2. QQChannel READY catch handler now calls fixRestoredSessions() so
   the workaround runs regardless of restoreSessions() outcome

Fixes #7721
2026-07-25 17:43:05 +00:00
Shaojin Wen
c90748eae4
fix(review): don't overclaim a submit-gate bypass on a same-account write (#7718)
* fix(review): don't overclaim a submit-gate bypass on a same-account write

The cleanup bypass audit lists reviews and issue comments by the reviewing
account inside the review window that the submit receipt does not vouch for.
That set legitimately includes writes this review never made: a concurrent
`/review`, a triage/autofix bot, or the user themselves, all posting under the
same account, produce exactly the same shape — the audit cannot tell them
apart from a genuine `gh pr review` bypass by metadata alone. Observed in
practice: an observe-only (`no --comment`) quick pass that posted nothing had a
concurrent same-account CHANGES_REQUESTED review land in its window, and the
warning — relayed verbatim into the review summary — read "a write bypassed the
submit gate", alarming over what was an external write.

Reframe the footer so it no longer asserts a bypass as the default reading: the
likely cause is benign (named account, another workflow, or a bot), and a write
here is a real gate bypass only if its content is this review's own output. The
detection logic and the per-write detail lines are unchanged, so nothing that
was surfaced before is hidden now — only the conclusion the copy draws is
corrected.

* fix(review): third-person footer, sync SKILL.md, pin the copy in tests

Address review feedback on the bypass-audit warning reframe:

- The footer led with "you", but cleanup's stdout is read by the model as well
  as relayed to the user. To the model "you" is itself, and the model posting
  under the reviewing account is the exact bypass this tripwire catches — so the
  copy opened by calling that case benign to the one reader who might be the
  offender. Switch to third person ("the user (from another terminal), another
  workflow, or a bot").
- Requote the CLOCK_SKEW_MS docstring: it justified over-flagging by pointing at
  warning copy ("the user did this themselves") that this change removed.
- Sync SKILL.md Step 9: it still framed a flagged write bypass-first with none
  of the new discriminator; lead with the external same-account reading and add
  "a write that bypassed the gate only if its content is this review's own
  output", matching the footer.
- Tests: pin the interpolation shape `(reviewer)` instead of the bare word
  (the header also says "reviewing account", so the old assertion stayed green
  even with the account name dropped), and assert the "Relay this warning
  verbatim" sentence — the line that moves the warning to a human, previously
  covered by nothing.

---------

Co-authored-by: verify <verify@local>
2026-07-25 17:23:39 +00:00
Shaojin Wen
596abd9664
fix(web-shell): allow pin and group for secondary workspace sessions (#7716)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
Organization actions (pin/group) only mutate display metadata and never
execute code or touch the filesystem, so they are safe for any trusted
workspace — not just locked ones. Decouple them from the stricter
canUseWorkspaceQualifiedActions gate into a new canUseOrganizationActions
check that allows primary, locked, and restricted scopes (rejecting only
unknown/untrusted). Destructive actions (rename, delete, export) remain
gated behind the original locked-or-primary requirement.
2026-07-25 14:14:16 +00:00
Shaojin Wen
a8a28a1137
fix(acp-bridge): raise live journal caps and expose as daemon config (#7715)
The live journal (DAEMON-009) caps were too conservative for real-world
agent turns: 2000 events / 2 MiB caused 79% event loss on a typical
long turn (9647 events). Raise defaults to 10 000 events / 8 MiB and
expose them as --max-journal-events / --max-journal-bytes CLI flags,
following the same config path as --compacted-replay-max-bytes.

Also fix stale docs that described the liveJournal as uncapped.
2026-07-25 14:14:09 +00:00
VectorPeak
8edfa31aab
fix(cli): parse heatmapDays strictly (#7218)
* fix(cli): parse heatmapDays strictly

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

* test(cli): cover negative heatmapDays fallback

Co-authored-by: qwen-code-ci-bot <253268222+qwen-code-ci-bot@users.noreply.github.com>

---------

Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <253268222+qwen-code-ci-bot@users.noreply.github.com>
2026-07-25 13:53:53 +00:00
Shaojin Wen
5c7c2ec0d0
fix(web-shell): enable Changes and History dialogs for worktree sessions (#7695)
* fix(web-shell): enable Changes and History dialogs for worktree sessions

* fix(web-shell): add containment and gitCwd forwarding tests, fix SDK nits (#7695)

* fix(cli): use realpathSync in resolveContainedCwd test assertions (#7695)

On macOS, os.tmpdir() returns /var/folders/... (a symlink to
/private/var/folders/...), but resolveContainedCwd resolves paths
via fs.realpathSync. The test assertions compared against the raw
os.tmpdir()-based paths, causing two tests to fail on macOS. Wrap
the expected values in fs.realpathSync to match the function's
actual return value on all platforms.

* test(web-shell): cover gitCwd forwarding through log pagination (#7695)

* test(sdk): cover cwd query encoding in workspace git client methods (#7695)

* fix(sdk): bump browser bundle size limit for worktree cwd params

---------

Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
2026-07-25 12:47:02 +00:00
yijie zhao
2049d50823
test(web-shell): cover restored-history pagination retries (#7657) 2026-07-25 11:30:51 +00:00
qwen-code-dev-bot
34a3d46006
fix(core): fire StopFailure hook on loop detection early returns (#7592)
* fix(core): fire StopFailure hook on loop detection early returns (#7588)

When loop detection (always-on safety or heuristic) terminates a turn
early via `return turn`, the Stop hook code after the streaming loop
was never reached. StopFailure hooks were only fired from the CLI
layer for API errors, not from client.ts for loop detection.

Added `loop_detected` to StopFailureErrorType and fire the
StopFailure hook via MessageBus before each loop detection early
return, so cleanup/notification hooks run regardless of how the
turn ends.

All 284 client tests and 682 hook tests pass.

* fix(core): use direct hookSystem call for loop-detection StopFailure (#7588)

The MessageBus bridge has no StopFailure case, so the hook never
executed. Switch to config.getHookSystem()?.fireStopFailureEvent()
(matching the CLI's API-error path), make it fire-and-forget per the
StopFailure contract, drop the stale last_assistant_message that
carried the previous turn's text, deduplicate via a private helper,
update docs with loop_detected, regenerate the settings schema, and
add regression tests for both loop-detection paths.

* test(core): add negative-path test for StopFailure hook disable guard (#7592)

* test(core): add negative-path tests for StopFailure hook guards (#7592)

---------

Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-25 11:29:02 +00:00
Shaojin Wen
7e2eeb90c2
fix(review): address post-merge P2 review feedback on the review skill (#7708)
* fix(review): address post-merge P2 review feedback on the review skill

Follow-up to #7690 and #7691, picking up six P2 review comments that landed
just before those PRs merged:

- comment-status: declare `host` on CommentStatusArgs so a future refactor
  that drops it becomes a type error, not a silent runtime regression.
- comment-status: the degraded report now emits `prAuthor: null` (matching
  `worktreeHeadSha`) instead of '', so a total index failure is
  distinguishable from a legitimately absent (deleted) PR author.
- comment-status: wrap the second head sample in its own try/catch and fall
  back to liveHeadBefore, so a transient failure after the comments are
  already fetched no longer discards them into a degraded report.
- submit: write the audit receipt to a sibling tmp then renameSync over the
  target, so a crash mid-write can never leave a truncated receipt that
  parseReceiptIds reads as [] and drops every accumulated review id.
- SKILL.md: the Step 1 comment-status guard is now worktree presence, not
  "the context file reports inline comments" — pr-context reports those in
  lightweight mode too, where no worktree exists.
- SKILL.md: lead the Step 7 write-ban with a single compression-proof
  sentence; the enumeration stays as support beneath it.

* fix(review): use atomicWriteFileSync for submit receipt (#7708)

---------

Co-authored-by: verify <verify@local>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
2026-07-25 11:24:23 +00:00
Matt Van Horn
a4d8845567
fix(core): silence xterm.js parser diagnostics from headless shell terminals (#7663)
Fixes #7631

Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
2026-07-25 10:20:41 +00:00
hogeheer499-commits
88782646ab
feat(core): configure stream rate-limit retry delays (#7666)
Co-authored-by: JS van Dijk <267467744+hogeheer499-commits@users.noreply.github.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
2026-07-25 09:43:25 +00:00
OrbitZore
62e009a952
feat(channels): GitHub polling adapter with notification-as-wakeup architecture (#7632)
* feat(channels): add GitHub polling adapter with notification-as-wakeup architecture

Introduce a GitHub channel adapter that monitors notifications and
responds to @mentions on issues/PRs by posting comments. Uses
last_read_at as a per-thread watermark for comment enumeration,
replacing the unreliable latest_comment_url approach.

Foundation changes to ChannelBase:
- sendThreadMessage for thread-targeted delivery (IM adapters unchanged)
- Envelope.metadata appended to prompt after command parsing
- chat_thread session scope (channel:chatId:threadId) prevents
  cross-repo session collision
- polling-helpers: testBotMention/stripBotMention (separate detection
  from stripping, no whitespace collapsing), cursor persistence,
  abortableSleep

GitHub adapter design:
- Notifications as wake-up signals only (unread filtering)
- listComments enumeration with last_read_at watermark
- Bot self-comment filtering, case-insensitive mention regex
- In-memory recentlyProcessed set for mark-read failure dedup
- First-contact: new issue body @bot triggers processing
- Error comment + cursor advance on handleInbound failure
- pollInterval minimum 60s, exponential backoff 2s-30s

* refactor(channels): extract PollingChannelBase from polling-helpers

Replace the loose polling-helpers module with a PollingChannelBase<Cursor>
abstract class that encapsulates the poll loop, cursor persistence (JSON,
atomic write), exponential backoff, and start/stop lifecycle. Subclasses
implement only pollOnce() and createInitialCursor().

- Delete polling-helpers.ts (cursor fns + abortableSleep moved into base)
- Move mention utilities (testBotMention/stripBotMention) to github pkg
- GithubAdapter now extends PollingChannelBase<{ lastProcessedAt }>

* fix(channels): remove Gitea/GitLab mention from sendThreadMessage JSDoc

* fix(channels): match /pulls/N in notification subject URL

GitHub PR notifications use /repos/{owner}/{repo}/pulls/{N} in
subject.url, not /issues/{N}. The regex only matched /issues/,
causing PR notifications to be skipped and marked read.

Also sets threadId to 'pr:N' for PRs (was always 'issue:N').

* test(channels): add PR body first-contact unit test

Verify that PR notifications with @mention in the body (not a comment)
correctly trigger the first-contact path: extractFromSubjectUrl matches
/pulls/N, listComments returns empty, tryFirstContactBody fetches the
PR body and dispatches to handleInbound with threadId 'pr:N'.

* feat(channels): read pollInterval from channel config in PollingChannelBase

Move pollInterval config reading from GithubAdapter to the base class.
The user's configured pollInterval in settings.json is now respected
directly without a minimum enforcement. Defaults to 60000ms when not
configured.

* fix(channels): prepend metadata before prompt text

Agent sees issue/PR context (type, title, URL) before the user's
request, improving comprehension. Metadata is still appended after
slash-command parsing so commands are not affected.

* refactor(channels): route all ChannelBase delivery through sendThreadMessage

Replace all internal sendMessage calls with sendThreadMessage, passing
envelope.threadId (or target.threadId / undefined) so polling adapters
can deliver to the correct thread. IM adapters are unaffected — the
default sendThreadMessage falls through to sendMessage.

* docs(channels): document sendThreadMessage delivery architecture

* fix(channels): address review findings

- Cap recentlyProcessed Set at 10k entries to prevent unbounded growth
- Validate cursor JSON shape (non-null object) in loadCursorFromDisk
- sendThreadMessage falls through to sendMessage when threadId is
  undefined instead of silently dropping
- Remove duplicate pollInterval from GithubConfig (now in ChannelConfig)
- Fix chat_thread routing key trailing colon when threadId is undefined

* docs(channels): fix metadata JSDoc — prepended, not appended

* fix(channels): use recentlyProcessed dedup for first-contact body

Replace the fragile createdAt-vs-cursor check in tryFirstContactBody
with the recentlyProcessed set. The cursor advances globally based on
notification updated_at — when a different notification with a later
updated_at is processed first, the cursor can advance past the issue's
created_at, causing the first-contact check to incorrectly skip the
issue body (forget reply bug, found in E2E TC-2b).

* refactor(channels): two-layer dedup for GitHub adapter

Layer 1: global cursor filters notifications by updated_at (sorted
ascending, old first). Layer 2: server-side last_read_at filters
comments by created_at (sorted ascending).

- Delete recentlyProcessed Set (no longer needed)
- Sort notifications by updated_at ascending before processing
- Sort comments by created_at ascending before processing
- Pass latest comment created_at to markThreadAsRead as last_read_at

* fix(channels): address review findings on GitHub adapter

Blockers:
- sessionScope: add defaultSessionScope to ChannelPlugin, apply in
  parseChannelConfig so router and adapter agree on 'chat_thread'
- channel-registry.test.ts: add 'github' to expected type list

Should-fix:
- Replace per-thread markThreadAsRead (PATCH) with bulk
  markNotificationsAsRead (PUT /notifications + last_read_at).
  API errors stop the batch without marking failed notifications
  read; handleInbound errors still advance (error comment posted).
- connect() throws on bot identity failure instead of failing open
- metadata appended after promptText (inside sender attribution)
- isSharedSessionTarget includes 'chat_thread' scope

Nits:
- startPollLoop re-entrancy guard
- clean-package-build-artifacts.js includes github
- index.ts re-exports GithubChannel

* fix(channels): use max updated_at of all fetched notifications as last_read_at

Prevents re-fetching the same notifications in the next poll cycle.
The bulk PUT /notifications marks all fetched notifications as read
up to the max updated_at, regardless of per-notification success.

* fix(channels): address review round 2 findings

- #12: loadCursorFromDisk rejects arrays
- #13: pollInterval validates positive finite number
- #19: first-contact gate uses dispatchedMention flag (not newComments.length)
- #25: stripBotMention no longer trims (preserves indentation)
- #27: remove adapter-level requireMention, unify on GroupGate
- #31: add chat_thread SessionRouter routing key tests
- #33: clear metadata on collect-mode synthetic envelope
- #35: fix PollingChannelBase.test import path
- #36: add @octokit/rest to 15-channel-adapters.md dependencies

* docs(channels): document known limitations for GitHub adapter

- First start skips existing unread notifications (cursor = now)
- Requires classic PAT (fine-grained PATs lack notifications API)
- PR review comments not enumerated (issue comments only)

* fix(channels): address review round 3 findings

- #9: buildMetadata derives web URL from baseUrl (GHE support)
- #12: sendThreadMessage throws on invalid threadId format
- #19: mention lookbehind matches cc:@bot and "@bot" patterns
- #23: cursor file name uses sha256 hash to prevent collision
- #26: test verifies cursor persistence to disk
- #31: postErrorComment double-failure logs to stderr
- #45: tests use mkdtempSync isolation instead of real QWEN_HOME

* fix(channels): pass threadId through pairing flow + sendResponseMessage test

- #13+16: onPairingRequired receives envelope.threadId and passes it
  to sendThreadMessage, so pairing codes are delivered on threaded
  channels (GitHub) instead of throwing
- #6: add test verifying sendResponseMessage resolves threadId from
  router.getTarget and passes it to sendThreadMessage

* fix(channels): pass proxy to Octokit for daemon-worker environments

- #44: read this.proxy from ChannelBaseOptions and pass
  HttpsProxyAgent to Octokit request.agent, matching the
  Telegram adapter pattern

* fix(channels): address review findings — immutable senderId, comment time window, validateCursor, retry wrapper

- senderId uses immutable user.id; allowedUsers resolved to IDs at connect
- Comment filter upper bound: updated_at <= maxUpdatedAt (batch window)
- Per-notification errors use continue (best-effort), not break
- validateCursor() virtual hook for subclass cursor shape validation
- sendThreadMessage/postErrorComment wrapped in githubApi() retry
- webOrigin handles default api.github.com → github.com
- Docs: classic PAT only, markNotificationsAsRead, dedup claims removed
- Tests: threadId priority, metadata consumption, defaultSessionScope,
  QWEN_HOME isolation, persistent mock rejection

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

* fix(channels): mark notifications read before processing to prevent duplicate replies

Bot's own replies bump notification updated_at past the pre-captured
maxUpdatedAt, so markNotificationsAsRead(maxUpdatedAt) failed to mark
them read — the next poll re-fetched the same comments and replied
again.

Move markNotificationsAsRead + cursor advance before the processing
loop (best-effort delivery). This is safe because bot's own comments
do not flip notifications back to unread. Update docs to reflect the
new poll cycle order and best-effort semantics.

* fix(channels): update sender gate after allowedUser ID resolution and harden tests

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

* fix(channels): cursor-based comment window to prevent duplicate replies

PUT /notifications is async (202) with a last_read_at cutoff — the
bot's reply bumps updated_at past the cutoff before the server
processes the mark, so the notification is never marked read and gets
re-fetched on the next poll, causing duplicate replies.

Use the cursor value before advancement as an exclusive lower bound
for the comment enumeration window: (windowSince, maxUpdatedAt].
Comments already eligible in a previous poll are excluded regardless
of whether the mark succeeded. Zero new persistent state.

* fix(channels): cursor-based comment window to prevent duplicate replies

PUT /notifications is async (202) with a last_read_at cutoff — the
bot's reply bumps updated_at past the cutoff before the server
processes the mark, so the notification is never marked read and gets
re-fetched on the next poll, causing duplicate replies.

Use the cursor value before advancement as an exclusive lower bound
for the comment enumeration window, with per-notification last_read_at
as the preferred lower bound when available (server-side per-thread
watermark). Comments already eligible in a previous poll are excluded
regardless of whether the mark succeeded. Zero new persistent state.

* fix(channels): address review findings — null guard, cursor validation, metadata dedup, abortable sleep, docs

- Guard against null notification.subject.url in pollOnce
- Validate lastProcessedAt is a parseable date in validateCursor
- Add metadata: undefined to second collect-mode drain path
- Refactor abortableSleep as protected method on PollingChannelBase
- Fix docs: requireMention is nested under groups.*
- Add tests: chat_thread shared session, dispatchedBodies eviction,
  cursor enumeration window, last_read_at in mention tests

* docs(channels): sync docs with implementation — cursor shape, error handling, GitHub adapter tables, first-contact

- Design doc: update Cursor to { lastProcessedAt, dispatchedBodies? }, add
  validateCursor date check, abortableSleep protected method, break-on-error
  semantics, subject.url null guard
- Developer docs: add GitHub to adapter table and adapter matrix
- User guide: add first-contact step to How It Works, clarify mark-before-process

* fix(channels): address review round 2 — error dedup, abortable retry, backoff reset, window test

- Record dispatchedBody on first-contact handleInbound failure to prevent
  duplicate error comments when mark-read async hasn't taken effect
- Use abortableSleep instead of raw setTimeout in githubApi retry so
  disconnect() can interrupt rate-limit cooldowns
- Reset consecutiveErrors in startPollLoop so stop/restart cycles don't
  inherit stale elevated backoff
- Add test for cursor window client-side lower-bound exclusion filter

* fix(channels): address review round 3 — cursor validation, error dedup, sender gate, bot-self body

- validateCursor: normalize falsy non-array dispatchedBodies (false/0/""/null)
  to [] instead of passing them through to .includes() which throws TypeError
- Set dispatchedMention after postErrorComment to prevent first-contact from
  posting a duplicate error comment on the same thread
- Only set dispatchedMention when the sender passes the sender gate, so a
  disallowed commenter's mention no longer suppresses a valid first-contact
  body from an allowed issue author
- Skip bot-authored issue bodies in tryFirstContactBody to prevent
  self-response loops under open sender policy

* fix(channels): address review suggestions — test coverage, cursor filename, assertion precision

- Pairing flow: add threadId pass-through regression test
- pollInterval: add table-driven edge cases (0, -1, NaN, Infinity, string)
- Add null-URL notification followed by valid notification batch test
- Fix comment window test to assert paginate call 3 (listComments) not call 2
- Truncate cursor filename encoded prefix to 200 chars (filesystem 255 limit)
- Assert mark-read uses batch maxUpdatedAt, not just { read: true }
- Assert real GitHub plugin declares defaultSessionScope chat_thread
- Add invocationCallOrder assertion for mark-before-process ordering

* fix(channels): address review round 4 — allowedUsers throw on resolve failure, crash table fix, mark-read failure test

* fix(channels): address review round 5 — created_at filter, retry-after NaN guard, retry/sendThreadMessage tests, docs fixes

* fix(channels): address ci-bot review 4778587403 — reconnect idempotency, github type enumerations, retry/webOrigin tests

* chore(channels): align channel-github version to 0.21.0 after upstream merge

* chore(channels): update package-lock.json for channel-github 0.21.0

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

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: OrbitZore <orbitzore@users.noreply.github.com>
2026-07-25 09:31:50 +00:00
chinesepowered
0a17239453
fix(core): convert subschemas of properties named after schema keywords (#7546)
convertTypes checked the constraint-keyword branches (minimum, maximum,
multipleOf, minLength, maxLength, minItems, maxItems) before the object
recursion branch, and each of those branches copies non-string values
verbatim. A tool parameter literally NAMED after one of those keywords
therefore had its subschema passed through unconverted:

  properties: { maximum: { type: 'INTEGER', minimum: '5' } }
  -> { type: 'INTEGER', minimum: '5' }   // neither lowercased nor coerced
  properties: { normalProp: { type: 'STRING' } }
  -> { type: 'string' }                  // converted correctly

Recurse on any object value first, so the keyword branches only ever see
primitives — which is all they were meant to coerce. The old
'typeof value === object' branch became reachable only for null, whose
convertTypes(null) is null, i.e. identical to the final else, so it is
dropped.

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-25 09:25:39 +00:00
chinesepowered
5bb53eb645
fix(core): stringify const-derived enums in toOpenAPI30 (#7547)
toOpenAPI30 maps const to a single-value enum, then separately stringifies
enums because — per its own comment — Gemini strictly requires enums to be
strings. The two never met: the stringification keys off source['enum'],
which a const-only schema never sets, so the const value went through raw.

  { const: 5 }     -> { enum: [5] }        // number, breaks the rule
  { const: true }  -> { enum: [true] }     // boolean, likewise
  { enum: [1, 2] } -> { enum: ['1', '2'] } // the intended behavior

Build the const-derived enum with String() so both paths produce the same
kind of value.

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
2026-07-25 09:25:15 +00:00
chinesepowered
3a85d48e77
fix(desktop): scale formatBytes past GB so terabyte sizes don't render as "undefined" (#7623)
formatBytes only had B/KB/MB/GB units and indexed the array with an
unclamped log-based exponent, so any value >= 1 TB overflowed to
"1.0 undefined". This is reachable in api-tools: the "Response too
large" guard formats an untrusted remote Content-Length, which a
server can report in the terabytes.

Extend the unit table through EB, clamp the exponent to the last unit,
and guard non-finite / sub-1 inputs so the function never emits
"undefined" or "NaN". Add formatBytes coverage to the existing
binary-detection test (range/limit guards plus the TB/PB/EB and
non-finite cases that fail on the old code).
2026-07-25 09:21:15 +00:00
destire-mio
dbadf49c6c
feat(stats): show generation timing metrics (#7677)
Co-authored-by: destire-mio <248462155+destire-mio@users.noreply.github.com>
2026-07-25 09:06:33 +00:00
Dragon
16ead53675
refactor(core): assemble the system prompt through one layered builder (#7707)
* perf(core): keep the volatile auto-memory section last in the system prompt

The managed auto-memory section (instructions + MEMORY.md indexes) is
rewritten in-session on every memory save, but it was pre-concatenated
into the middle of the userMemory blob — ahead of appendSystemPrompt and
git status, which are stable for the whole session. Every save therefore
invalidated the prompt-cache prefix from the middle of the system prompt,
and the stable/context/volatile layers were indistinguishable in code.

Store the auto-memory section separately on Config (getAutoMemoryPrompt)
and have every assembly site append it after all stable and context
content, yielding a stable -> context -> volatile layout: base prompt,
QWEN.md hierarchy + rules, append prompt, git status, auto-memory. Also
count the section in /context (previously dropped by the marker parser)
and keep /memory show and the context-size warning covering both layers.

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

* refactor(core): reuse buildSystemPromptSuffix for subagent auto-memory + add regression tests

- agent-core.ts: replace the inline auto-memory separator with
  buildSystemPromptSuffix so the '---' separator and trimming stay in
  sync with the other assembly sites (client.ts x2, ArenaManager.ts).
- client.test.ts: add coverage that a non-empty getAutoMemoryPrompt is
  appended as the last block of the main-session system instruction.
- contextCommand.test.ts: add coverage that a non-empty
  getAutoMemoryPrompt surfaces an 'auto memory' row in the /context
  memory breakdown.

* refactor(core): assemble the system prompt through one layered builder

Follow-up to the auto-memory reordering: the stable -> context -> volatile
order existed only as a convention spread across four call sites (client,
subagents, Arena, custom-instruction path), so a new segment could silently
be appended in the wrong position. Introduce assembleSystemPrompt with one
named slot per segment (base, contextFiles, appendPrompt, gitStatus,
autoMemory); it is now the single place that knows the order, and
getCoreSystemPrompt / getCustomSystemPrompt delegate to it so there is one
join implementation. buildSystemPromptSuffix returns to module-private.

Pure refactor: every assembly site produces byte-identical output, so no
prompt caches are invalidated by this change.

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

* fix(core): stop duplicating the auto-memory section in Arena workers

ArenaManager pre-appended buildSystemPromptSuffix(getAutoMemoryPrompt())
onto the in-process worker's systemPrompt, but AgentCore.buildChatSystemPrompt
already appends the auto-memory section itself when the worker builds its
system instruction (and the per-agent Config inherits a non-empty
getAutoMemoryPrompt() from the base via Object.create). The section therefore
appeared twice in the worker's prompt.

Drop the ArenaManager append so the volatile auto-memory layer is added exactly
once, by AgentCore, keeping it last. Add regression tests: ArenaManager no
longer embeds the auto-memory marker, and generateContent's per-call
systemInstruction branch still appends it.

* refactor(core): make buildSystemPromptSuffix module-private

It has no external importers after routing every assembly site through
assembleSystemPrompt, so dropping the export prevents a future caller
from bypassing the enforced layer order — matching the PR's stated intent.

* chore: drop unrelated lightningcss lockfile churn

Restore the "peer": true entries on the lightningcss optional deps that
an incidental npm install had removed; keeps this refactor's lockfile
diff empty so it does not mislead bisects on lightningcss resolution.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-25 09:03:56 +00:00
yijie zhao
0b5116a1bb
feat(core): configure stream rate-limit retry delays (#7674) 2026-07-25 08:56:21 +00:00
jinye
c4859627a7
feat(serve): Hot-reload workspace trust changes (#7268)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
SDK Java / ubuntu-latest / Java 11 (push) Waiting to run
SDK Java / ubuntu-latest / Java 17 (push) Waiting to run
SDK Java / macos-latest / Java 21 (push) Waiting to run
SDK Java / ubuntu-latest / Java 21 (push) Waiting to run
SDK Java / windows-latest / Java 21 (push) Waiting to run
SDK Java / Real daemon E2E / Java 11 (push) Waiting to run
* feat(serve): hot-reload workspace trust changes

Rebuild workspace runtime generations when trust policy changes, fail closed across daemon routes, and expose reconciliation status to SDK and Web Shell clients.

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

* codex: address PR review feedback (#7268)

Document the trust hot-reload capability and reuse the daemon environment fallback so the serve process environment guard remains satisfied.

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

* fix(cli): cache workspace trust status snapshots

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

* fix: address trust reload race regressions

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

* fix(serve): avoid repeated runtime containment

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

* fix(serve): harden workspace generation boundaries

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

* fix(serve): restore stale session owner fallback

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

* fix(serve): preserve workspace metadata across trust reloads

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

* fix(serve): align hot-reload trust semantics

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

* fix(serve): stop git-state watcher on dispose only, fix git chip test (#7268)

beginDrain stopped the git-state watcher but cancelDrain had no way to
restart it, leaving the watcher disposed until the next lazy poll.
disposeRuntime already stops git-state when the drain is committed, so
the beginDrain stop was redundant — remove it.

Also fix the WorkspaceSection git chip test that broke when the trigger
changed from <button> to <span role="button"> inside DropdownMenuTrigger:
use closest('[role="button"]') and interact with the dropdown menu item.

* fix(serve): address review feedback on trust polling and setValue assertion (#7268)

* fix(cli): correct daemon trust policy settings precedence and drain continuation (#7268)

* fix(serve): address review feedback on fork cleanup, persist simplification, sync guard, and a11y (#7268)

* fix(serve): assert before mutate in setValue, add pre-mutation guard, trust-before-generation ordering (#7268)

* fix(serve): honor system defaults in trust policy

Apply the documented settings precedence to daemon folder trust evaluation and keep workspaces outside configured trust rules fail-closed.

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

* fix(serve): preserve managed scratch trust during reloads

Keep daemon-created scratch workspaces trusted across policy reloads while retaining controlled-root validation, and reject trust mutations that cannot apply to these fixed-trust runtimes.

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

* fix(serve): guard auth provider persistence by generation

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

* refactor(serve): remove Web Shell trust UI

Keep this PR focused on daemon and SDK trust reconciliation; the Web Shell integration can follow separately.

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

* fix(sdk): restore workspace trust bundle budget

Preserve the merge-only browser bundle allowance required by the additive workspace trust v2 SDK surface after rebasing.

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

* fix(cli): handle trusted folder write failures

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

* fix(cli): keep capabilities available during trust reload

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

* fix(cli): address review feedback for workspace trust hot reload (#7268)

Drop the closed generation guard before retrying dynamic workspace
runtime creation so the retried runtime starts with a fresh, open guard
instead of inheriting the one closed during the abandoned attempt. Make
the /workspace/reload trust reconcile fire-and-forget with a swallowed
rejection (failures are reported separately), reuse sendGenerationClosedError
for the memory write error path, and assert the subagent deletion commit
boundary once before unlinking so a closed generation fails atomically.
Add coverage for the blocked-entry deep health probe and the /session/:id/cd
generation-close-during-flight path.

* fix(serve): close trust reload cleanup gaps

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

* fix(serve): use fire-and-forget for trust reconcile in workspace-qualified reload (#7268)

* fix(serve): address review feedback on generation guard and trust reconciler (#7268)

* fix(serve): use shared helpers for untrusted/generation-closed responses (#7268)

* fix(serve): continue cleanup after drain commit errors

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

* fix(cli): retry transient trust policy disappearance

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

* fix(serve): align status provider trust default with route-level check (#7268)

* fix(serve): clean up worktree on generation guard abort (#7268)

* fix(cli): guard tool and skill settings commits

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

* test(cli): add discriminating persistent-ENOENT test for trust policy read (#7268)

* fix(serve): close runtime generation gaps

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

* fix(serve): preserve scheduled task cap errors

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

* fix(serve): address review feedback on trust reconciler, settings guard, and route simplification (#7268)

* fix(serve): preserve containment retry semantics

Restore the last verified trust-reconciliation and generation-guard behavior after the automated review fix marked an unconfirmed disposal as contained and removed per-scope commit checks. Defer the remaining late-round suggestions to avoid expanding the PR.

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

---------

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: Qwen Autofix <qwen-autofix@alibaba-inc.com>
Co-authored-by: Qwen Code Autofix <qwen-code-autofix@users.noreply.github.com>
2026-07-25 08:43:07 +00:00
Jingyuan Tian
cb36659e39
fix(cli): align inline math recognition (#7701)
* fix(cli): align inline math recognition

* fix(cli): tighten inline code span handling

* test(cli): cover escaped inline math closer
2026-07-25 08:30:33 +00:00
Shaojin Wen
b9db14cb13
feat(review): enforce the submit-only write contract with a cleanup tripwire (#7691)
* feat(review): enforce the submit-only write contract with a cleanup tripwire

The /review skill's posting ban named one API route — `gh api
.../pulls/<n>/reviews` — and a run that had lost the surrounding prose to
four context compressions walked around it: it decided its findings were
all duplicates, never called submit, and hand-posted a consolidated
summary with `gh pr comment`. No authorisation gate, no downgrade
semantics, no `posted` fact, no completion line; nothing downstream could
tell it had happened.

Three layers close this:

- SKILL.md Step 7 now bans every write path to the PR (`gh pr comment`,
  `gh pr review`, `gh issue comment`, mutating `gh api` calls, comment
  edits/deletes) and routes prose wrap-ups into the review body via
  compose-review; the overlap-drop branch and Step 9 restate it at the
  two decision points where runs improvised.
- fetch-pr stamps `fetchedAt` (and `host`) into the fetch report — the
  review window's opening time, on the carrier that exists on every PR
  run.
- cleanup audits that window before sweeping: issue comments by the
  reviewing account are flagged with warning lines the skill must relay
  (submit posts reviews, never issue comments, so the overlap with
  sanctioned output is zero). Best-effort by design — offline or
  unauthenticated runs skip it silently, and an audit failure never
  fails the cleanup.

Validated against the real bypass: with a window opening at the original
run's fetch time, the audit names the exact hand-posted comment.

* fix(review): bypass-audit review follow-ups — shared-account filter, edit class, named skips

Addresses the review on the tripwire design:

- Medium: in CI the reviewing account is the bot that precheck/triage
  also post from, so their marker-stamped comments (<!-- qwen-… -->) are
  filtered out and the warning prose gains the third reading (another
  workflow under the same account). Without this, every mid-review push
  produced a false accusation the skill is told to relay verbatim.
- Low-Medium: producer contract tests — fetch-pr's report is now pinned
  to carry fetchedAt (a real timestamp) and --host, since dropping either
  silently turns the audit off with output identical to a clean window.
- Low: every skip path names itself on stderr (note: bypass audit
  skipped (…)), so the tripwire's off state is distinguishable from its
  all-clear state.
- Low: pre-window comments edited inside the window are flagged as an
  edit class of their own — ?since= filters on updated_at, so the rows
  were already fetched. Verified empirically that reactions do not bump
  an issue comment's updated_at before adding this.
- Low: the empty-window early return skips the currentUser() round trip;
  report-derived prNumber is cross-checked against the cleanup target and
  ownerRepo is shape-checked before either reaches a gh api path.
- Nits: use the tmpFile helper instead of re-deriving its path; SKILL.md's
  fetch-pr snippet now shows --host (the report records it and the audit
  queries it — a dropped host silently audits github.com).

* test(review): cover the github.com (host: null) path of the bypass audit

* fix(review): harden the bypass audit — review channel, boundary integrity, provenance

Addresses the second review round on the tripwire:

- Review-channel coverage: submit now records a receipt (the one review
  id it was authorised to create, parsed from the POST response), and
  cleanup flags any in-window review by the reviewing account the
  receipt does not vouch for — `gh pr review` and direct POSTs to
  pulls/<n>/reviews were bannable but invisible before. No receipt
  vouches for nothing (fail-safe).
- Boundary integrity across restarts: fetch-pr preserves the earliest
  window opening as auditSince when it overwrites its own report (the
  head-drift rule reruns it), so writes made during an abandoned attempt
  stay inside the audit window. A window from a different PR left at the
  same path is not inherited.
- Clock skew: the audit boundary backs off two minutes from the recorded
  opening — fetchedAt is local time compared against GitHub's server
  timestamps, and a fast local clock could otherwise hide the first
  moments of the review. Errs toward over-flagging.
- Marker provenance: the automation filter is anchored to the body
  START, so a hand-posted summary that merely quotes a marked bot
  comment (or hides the marker mid-body) stays visible to the tripwire.
- Named skip fidelity: ENOENT alone means "no fetch report" (any other
  read failure names its code); gh failures surface the first non-empty
  stderr line instead of the generic Command-failed wrapper.
- Tests: rogue-review flag + receipt exclusion, skew boundary, auditSince
  window, edited-warning rendering through runCleanup, malformed-report
  table, EACCES vs ENOENT, stderr extraction, marker-quoting visibility,
  and producer tests for auditSince preservation.

* fix(review): distinguish a corrupt prior fetch report from an absent one

The auditSince-preservation block swallowed every read/parse error as "no
previous report". A crash mid-write leaves truncated JSON: on the next
drift restart JSON.parse threw, the catch reset auditSince to the new
fetch, and a bypass write from the abandoned attempt escaped the audit
window silently. ENOENT is still the silent first-attempt path; a
non-ENOENT read failure and an unparseable existing report each warn on
stderr that the window may not reach an earlier attempt. Tests cover
corrupt-JSON, ENOENT-silent, and EACCES-named.

* fix(review): accumulate submit-receipt ids across a window, and test the producer

Two gaps in the review-channel bypass audit's receipt contract:

- The receipt was overwritten on every submit, but the audit window spans
  drift restarts (fetch-pr preserves auditSince), so two sanctioned
  submits could fall in one window — the single-id receipt then vouched
  only for the last, and cleanup flagged the earlier legitimate review as
  a bypass (a false positive for a write submit itself made). The receipt
  now accumulates ids (read prior, append, dedupe); cleanup reads the set
  and excludes all of them. Both sides migrate a legacy single `reviewId`.
- The producer half had no test: submit.test.ts's ghMock returns '', so
  JSON.parse(response) threw and the receipt write always hit its catch —
  the happy path never ran. Added producer tests (id/event/timestamp
  written, accumulation across two submits, legacy migration) driven from
  inside the fixture dir, plus a cleanup test that spares every id in a
  multi-id receipt.

* fix(review): correct SKILL.md prose-wrap-up guidance; de-fragilize fetch-pr mock reset

- SKILL.md told the orchestrator to route a free-form prose wrap-up
  through compose-review's input, but ComposeReviewInput has no free-text
  body field — the body is computed from structured state, by design (the
  model does not author PR-facing prose). Following that instruction would
  force the model to misuse a structured field or forge a `body` submit
  refuses. Both passages now say the recap belongs in the TERMINAL
  summary; the PR receives only submit's computed body plus inline
  comments. This aligns the guidance with the submit-only principle rather
  than contradicting it.
- fetch-pr.test.ts's producer beforeEach re-set git/gh but not
  readFileSync, and clearAllMocks does not reset implementations — so a
  mockReturnValue from one test could leak into a later test relying on
  the default. The beforeEach now re-asserts the ENOENT default, removing
  the ordering dependency.

The receipt-overwrite and receipt-untested suggestions from the same
review were already addressed in 87554647c (this review ran against the
prior commit).

* fix(review): address maintainer review — host restore, shared receipt parser, auditSince bound

Three findings from @yiliang114:

- cleanup's audit set the gh host for the PR but never restored it —
  harmless today (cleanup runs last) but a leak if any code runs after.
  It now saves the prior host (new getGhHost) and restores it in a
  finally, so the Enterprise override is scoped to the audit block.
- submit's readReceiptIds and cleanup's readSubmitReceipt duplicated the
  receipt-shape parse (reviewIds[] + legacy reviewId migration). Extracted
  to lib/receipt.ts parseReceiptIds so a schema change is one edit both
  inherit; the two call sites now differ only in their empty wrapper
  ([] vs Set). Unit-tested directly.
- fetch-pr's auditSince adoption: documented that `prevSince < auditSince`
  (auditSince = fetchedAt = now) is also the upper bound — a corrupted
  far-future timestamp is rejected because the window only ever moves
  backward, so it cannot push the audit ahead of every real comment.

Tests: host-restore assertion in cleanup, parseReceiptIds table.

* test(review): exercise the auditSince-preference branch in the producer test

The drift-restart producer test seeded only fetchedAt, leaving the
auditSince-over-fetchedAt branch (the third-restart case, where a prior
report already carries an earlier auditSince) untested. Added a case
seeding both. Reported by @yiliang114.

* fix(review): parseReceiptIds must not throw on valid non-object JSON

JSON.parse('null') (or a bare number/string/array) succeeds, and the
next field access threw a TypeError — breaking the documented never-throws
contract. Both callers wrap it in try/catch today, but the contract
invites a future caller to omit the guard. Now guards value === null ||
typeof value !== 'object' and returns []. Tests cover null/number/string/array.

* feat(review): detect head drift at presubmit and cap the verdict (#7692)

* feat(review): detect head drift at presubmit and cap the verdict

A PR that advances while /review runs carries commits no agent read, and
an Approve issued past them certifies unreviewed code. Dogfooded on a
live PR whose head moved four times in one day: the only run that
noticed did so by accident (a context compression triggered a re-fetch),
and the 422-recovery path — the one place drift was handled — only fires
after a submission already failed.

presubmit now fetches the live head alongside the author (one jq
projection), compares it to the reviewed fetchedSha, and on drift adds a
headDrift report field plus a downgrade reason through the existing
machinery — an Approve is capped to COMMENT automatically, naming both
SHAs. A best-effort compare call annotates the delta (ahead_by, touched
files, diverged = force-push); the drift verdict never depends on it.

SKILL.md turns the report into one deterministic size call: if the
unreviewed commits touched files the findings anchor to — or the compare
is unavailable or diverged — apply the 422-recovery rule proactively and
restart at the new SHA; otherwise submit against the reviewed SHA with
the downgrade sentence saying so. Restarts cannot loop: they only happen
when the new commits touched the very files under review, which is
exactly when re-reading them is the review rather than overhead.

* fix(review): head-drift review follow-ups — compute anchorsAtRisk fail-safe in code

Addresses the review at be2cbb91:

- Blocker: the 50-file cap turned SKILL.md's intersection test into a
  silent fail-open — on a real 283-file base-merge drift the surviving
  alphabetically-first paths contained no packages/cli or packages/core
  file, so the gate read "safe" on exactly the largest drifts. The
  decision is now computed in presubmit as headDrift.anchorsAtRisk,
  fail-safe on every hole a hand intersection falls into: local
  truncation, the compare API's own 300-file ceiling, diverged history,
  an unavailable compare, or a missing findings list. filesTotal carries
  the real count and the downgrade reason reports it instead of the cap.
- Medium: restarts are bounded — at most one per review; a restarted run
  that reaches presubmit drifted again submits at its reviewed SHA with
  the drift named. The Approve cap holds either way.
- Low: a deleted PR author (author: null) no longer kills presubmit —
  the projection parse is fail-soft, restoring the bare-string
  behaviour the jq change had dropped.
- Low: an abbreviated commit_sha no longer reports false drift — an
  identical/zero-ahead compare is the endpoint's own proof that nothing
  is unreviewed, and it outranks a SHA-string mismatch.
- Tests: truncation and the API-cap fail-safe pinned; exactly-one-gh-call
  pinned on the happy path; a throwing compare pinned to keep the drift
  verdict; the --host fixture back to a neutral author; the
  paren-absence assertion replaced with the exact message.

* fix(review): head-drift round 2 — rename paths, metadata fail-closed, shared restart bound

Addresses the follow-up review:

- Renamed files: the compare projection now keeps previous_filename
  alongside filename, so an unreviewed commit that renamed a finding's
  anchor file (under its old path) still intersects and marks
  anchorsAtRisk. filesTotal dedupes the union.
- PR-metadata read failure now fails CLOSED: a thrown pulls fetch
  (transport/auth/404 on that endpoint) leaves the head unknown, so
  neither self-PR nor drift can be checked — the run emits a downgrade
  reason and caps the Approve rather than silently proceeding as if it
  had verified. A successful author:null response stays fail-soft (that
  case is answerable — not a self-PR).
- The one-restart-per-review bound now explicitly covers BOTH the
  proactive drift restart AND the reactive 422 recovery; the 422 branch
  no longer restarts unconditionally once the bound is spent.
- --new-findings must carry body-only Criticals' file paths too, so a
  drift touching an unmappable Critical's file is not read as safe.
- Tests: metadata-failure downgrade, rename-path intersection.

Not changed (design boundaries, reasoning in the PR thread): anchorsAtRisk
gates ANCHOR validity, not finding-truth (the Approve cap covers stale
findings; an inline comment on the reviewed SHA is acceptable and GitHub
marks it outdated); the presubmit→submit TOCTOU can only make the verdict
more conservative, never less.

* fix(review): validate the --new-findings file before trusting it as a drift safety proof

The findings list gates anchorsAtRisk — a disjoint intersection lets a
review submit past head drift — so a malformed file must not be able to
prove a false all-clear. parseFindingsFile now collapses to null (unknown
→ fail-safe at-risk) when the file will not parse, is not an array, or
holds an entry without a string path; a bad entry rejects the WHOLE file
rather than silently shortening the set (a dropped finding would read as
disjoint). A missing line normalizes to 0 (path is what the intersection
uses). Pure-validator table tests plus a handler test proving
anchorsAtRisk stays true on disjoint files when the findings are garbage.

* fix(review): surface a malformed --new-findings file instead of degrading silently

parseFindingsFile returning null made the drift path fail safe
(anchorsAtRisk: true) but ALSO silently emptied newFindingKeys, disabling
the existing-comment overlap check — so a run could not tell "no
overlaps" from "the dedup input was garbage" and might re-post comments a
prior run already made. presubmit now sets findingsFileInvalid on the
report and pushes a downgrade reason (and caps the Approve) when a
--new-findings path was given but did not parse, so the double-directional
degradation is visible and the skill knows to regenerate the file. Tests:
flag + downgrade with no drift, and no-flag on a valid/absent file.

* fix(review): treat a `behind` force-push-to-earlier as drift; render capped totals as a lower bound

Two review suggestions:

- classifyHeadDrift proved-same any non-diverged compare with aheadBy 0,
  which wrongly included `behind` — a head force-pushed BACK to an earlier
  commit. The reviewed SHA is then ahead of the head and off the PR's
  line, so its anchors may not exist in the head at all: that is drift.
  provedSame is now `status === 'identical'` only; `behind` drifts, forces
  anchorsAtRisk, and gets its own reason ("PR head moved to an earlier
  commit") instead of the self-contradictory "+0 unreviewed commit(s)".
- The public downgrade reason printed the exact filesTotal even at
  GitHub's 300-file compare cap, where the count may be an undercount. It
  now renders "300+ file(s)" at the cap.

---------

Co-authored-by: verify <verify@local>

---------

Co-authored-by: verify <verify@local>
2026-07-25 08:27:41 +00:00
Shaojin Wen
a470ba626c
feat(review): add comment-status helper for existing-thread triage (#7690)
* feat(review): add comment-status helper for existing-thread triage

One deterministic pass over a PR's existing inline comments, replacing
the per-comment `gh api` fetches the orchestrating model used to make
during /review: anchor validity at the live head (outdated detection,
with a file-level exemption), whether the anchored file changed in the
reviewed worktree since each comment's commit and which commits touched
it (the re-check's candidate "fixed by" list), reply participation and
PR-author response, the blocker signal (same carriesBlockerSignal as
pr-context, so the two surfaces agree by construction), and
worktree-vs-live head drift.

Measured on a heavily discussed PR (72+ inline comments), a single
review run burned 20+ model turns re-deriving exactly these fields one
comment id at a time. SKILL.md now runs the subcommand in Step 1 and
routes the Step 6 re-check's status questions at the report; comment
bodies stay in the pr-context file under its untrusted-data preamble,
and a comment-status failure only warns — it is an index, not the
evidence, so it never sets the context-unavailable state.

* test(review): add comment-status to the subcommand registry expectations

* fix(review): comment-status review follow-ups — size warning, --host wiring, scope clauses

Addresses the review at d098e4feb:

- High: warn when the report exceeds read_file's truncation threshold,
  mirroring pr-context — measured 53k chars on the benchmark PR, where a
  single read lost 36 of 71 threads (24 blocker-flagged) and the cut JSON
  did not parse. The warning points at jq first: the file is
  machine-shaped in a way the Markdown context file is not.
- Medium: thread --host through — the SKILL.md command block now says to
  pass it (each subcommand is its own process, so a host set elsewhere
  cannot carry over), and comment-status joins the host-required lists in
  SKILL.md and the code-review docs.
- Minor: Step 6's routing sentence now states both scope limits — the
  report exists only when Step 1 wrote it (worktree mode), and it indexes
  inline threads only; issue-/review-level blockers keep the context-file
  walk.
- Minor: touchedByTotal exposes the real commit count behind the capped
  touchedBy list, so a cut list is visible instead of reading as "the fix
  is not among them".
- Nits: drop the never-read `side` field; guard authorReplied against a
  deleted-author/deleted-replier '' === '' match; correct the force-push
  doc comment (a shared object database usually retains the old commit,
  so the range widens — fail-safe — rather than going unknown).

* fix(review): comment-status second-round follow-ups — CWD-safe pathspec, shared walk, stale flag

Addresses the LGTM-with-suggestions round:

- The git probe's pathspec is anchored with `:(top)`: run from a
  subdirectory of the worktree, the old CWD-relative form returned empty
  output with exit 0 and every thread read as "untouched since the
  comment" — silent, and pointing the one direction this index must not
  fail in. A real-git integration test now drives the probe from the
  repo root AND a subdirectory (plus the cap, the memo, and the
  missing-commit gate — none of which the injected-probe unit tests
  could see).
- findRootId is imported from pr-context (made generic and exported)
  instead of duplicated — the thread walk now agrees by construction,
  like the blocker signal already did.
- Head drift is denormalized onto every thread as code.staleWorktree, so
  a jq consumer of threads[] cannot skip the top-level flag by
  construction.
- Commit existence is memoized per SHA (it never depended on the path),
  halving git spawns on thread-heavy PRs; summarizeThreads gets a named
  return interface like every other exported shape here.
- DESIGN.md gains the missing section: why comment-status is a separate
  subcommand and why the second fetch of pulls/{n}/comments is
  deliberate (process boundary — pr-context must stay pure-API for
  lightweight mode; this one exists to join API facts with worktree git).

* fix(review): harden comment-status against untrusted-PR inputs

Addresses the security review round:

- Symlink --out: the command now runs from the trusted main checkout (so
  a relative --out cannot be redirected through a symlink an untrusted PR
  planted in its own worktree) and scopes its git queries to the worktree
  with `git -C <worktreePath>`, which it locates itself. SKILL.md no
  longer cd's into the worktree for it. The report lands in the main
  checkout's .qwen/tmp alongside every sibling report.
- Non-ancestor comment commit: after a force-push the anchor commit can
  survive in the shared object store without being on HEAD's history, so
  `sinceSha..HEAD` is empty and a changed file reads as changed:false —
  the one direction this index must not fail in. A single
  `merge-base --is-ancestor` gate now returns 'unknown' for a
  non-ancestor OR a missing commit, replacing the cat-file existence
  check (one git process instead of two).
- Literal pathspec: the GitHub-supplied path is passed as
  `:(top,literal)<path>` so a value like `:(exclude)a.ts` cannot be read
  as pathspec magic and inspect unrelated files.
- Fetch-race drift: the live head is sampled before AND after the
  comments fetch; a push landing mid-fetch (which would pair newer
  anchor mappings with a stale comparison) is now detected, recorded as
  both samples, and warned on distinctly from ordinary worktree lag.
- pr-context renders the root comment id in the Open and Already-discussed
  sections, giving Step 6 a stable join key back to comment-status's
  per-thread rootId (the blocker renderer already did this).
- Handler-level tests (mocked gh/git/fs) for the three drift outcomes,
  plus real-git integration cases for the non-ancestor gate and the
  literal pathspec. Multi-page pagination is a non-issue: gh api
  --paginate merges top-level arrays into one (verified on the live
  93-comment PR).

* fix(review): comment-status round 3 — precise staleWorktree, worktree-missing warning, discriminating pathspec test

Addresses three Suggestions:

- staleWorktree is now keyed on worktreeStale alone, not the headDrift
  union. A head that merely moved between the two samples while the
  worktree already matches the final head is NOT a superseded checkout,
  so its threads no longer carry staleWorktree:true against the field's
  documented meaning. headMovedDuringFetch stays a separate top-level
  flag + warning.
- A missing worktree (comment-status run before fetch-pr or after
  cleanup) now sets worktreeMissing on the report and prints a warning —
  previously every thread degraded to code:'unknown' silently, readable
  as "nothing changed".
- The literal-pathspec test now uses a discriminating pathspec
  (`:(glob)pkg/**`): magic would match the changed file (true), literal
  is a nonexistent filename (false), so asserting false actually fails if
  the `:(top,literal)` prefix is dropped — the old `:(exclude)…` read
  false under both interpretations. A plain-path control proves the probe
  is live.

* test(review): make the comment-status negation test actually exercise negation

The body 'No blockers here' matched no BLOCKER_PATTERN (the bare plural
never triggers /\bblocking\b/ etc.), so isBlocker returned false before
the negation branch ran — false coverage. It now uses 'No blocking
issues', which matches the signal and must be suppressed by the leading
'No', plus an un-negated control that asserts true.

* test(review): assert per-thread staleWorktree and --host wiring; document ghApiAll merge contract

Two test gaps + one recurring-review clarification:

- The worktree-lag drift test now includes a thread and asserts
  staleWorktree:true is denormalized onto its code object — the old
  positive case had zero threads, so the denormalization loop iterated
  over nothing and would pass even if the block were deleted.
- A --host test now asserts setGhHost is called with the argv host,
  matching the presubmit analog; without it a dropped setGhHost would
  silently target github.com for a GHE review.
- ghApiAll's doc now explains why one JSON.parse is correct on multi-page
  output: gh --paginate MERGES top-level arrays into one (it does not emit
  one array per page); the per-page-concat failure only affects
  key-nested arrays, which is exactly why ghApiAllNested exists. Verified
  on a 4-page 97-comment response.

* fix(review): comment-status degrades gracefully on failure per its SKILL.md contract

SKILL.md promises this command is an index, not evidence — "if it fails
(auth, network), warn and continue" — but runCommentStatus had no
try/catch, so an ensureAuthenticated() or gh throw propagated as an
unhandled rejection and killed the whole review. The runtime body is now
wrapped: any throw writes a minimal empty report ({prNumber, ownerRepo,
error, threads: []}) so downstream jq still parses, prints a
"comment-status failed" warning, and exits 0. Handler test pins the
auth-failure path (no throw, empty report, warning). Reported by
@yiliang114.

* test(review): pin comment-status owner_repo guard and truncation-size warning

Two untested paths flagged in review: the owner_repo-without-slash guard
(a caller error that must still throw, distinct from the runtime
graceful-degradation path) and the report-size warning that fires when
the JSON crosses read_file's truncation threshold.

* fix(review): comment-status degraded report carries the full shape, not a stripped one

The graceful-degradation path wrote { prNumber, ownerRepo, error,
threads: [] }, omitting headDrift/summary/headMovedDuringFetch/etc. A
consumer reading report.headDrift then got undefined (falsy = "no
drift"), silently mistaking a total index failure for a clean "nothing
moved" — and the review orchestrator keys code-fact warnings on exactly
that field. The degraded report now emits the same shape as the success
report with safe defaults plus `error`, so a consumer that checks `error`
sees the failure and one that reads a fact gets a neutral value, never a
misleading one. Reported by @doudouOUC.

* fix(review): degraded report's worktreeMissing must not contradict worktreeHeadSha: null

The catch-block report hardcoded worktreeMissing: false beside
worktreeHeadSha: null — a positive "worktree present" assertion the
success path (worktreeMissing = worktreeHeadSha === null) would never
make for a null head. A consumer reading worktreeMissing without gating
on error would conclude the worktree exists on a run where nothing is
known. Now true, matching the null head and the fail-safe reading (code
facts unavailable). Reported by qwen-code-ci-bot.

---------

Co-authored-by: verify <verify@local>
2026-07-25 08:26:08 +00:00
Shaojin Wen
d61b0ea475
perf(web-shell): paint the composer git chip before git status completes (#7680)
* perf(web-shell): paint the composer git chip before git status completes

New sessions gated the chip on a full `git status --porcelain` subprocess
behind GET /workspaces/:ws/git, so the branch chip appeared hundreds of
milliseconds (worst case seconds) after the composer was ready.

The daemon now keeps a per-workspace last-known summary with in-flight
dedup and a 2s background-refresh throttle: the default GET returns the
cached status (branch-only on a cold start) immediately and recomputes in
the background, publishing git_status_changed over SSE only on a delta,
while ?wait=1 keeps the previous blocking semantics. The composer fetches
both paths concurrently — the fresh GET also covers the no-session state,
which has no per-session SSE stream — so the branch paints in ~3ms and
the counters land when the computation finishes. The sidebar keeps
wait:true since it has no SSE fill-in path.

* fix(web-shell): add exhaustiveness guard and worktree git-status test (#7680)

* fix(web-shell): add exhaustiveness guard and worktree git-status test (#7680)

* fix(web-shell): add exhaustiveness guard and worktree git-status test (#7680)

* fix(cli): use writeStderrLineSafe in git-status refresh error path (#7680)

* fix(web-shell): add debug trail to fresh-path catch and test branch-watcher dispose guard (#7680)

* fix(cli): assert writeStderrLineSafe in git-status refresh failure test (#7680)

---------

Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
2026-07-25 07:05:52 +00:00