mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-07 15:55:50 +00:00
447 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Shaojin Wen
|
d9f7e1fbe1
|
feat(autofix): auto-rerun a check that died on infrastructure, once (#7562)
* feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
80784e645c
|
fix(autofix): make the review-address report wrapper lines bilingual (#7569)
The agent's address-summary.md / no-action.md already ends with a collapsed Chinese translation, but the workflow-appended wrapper lines around it — the "Addressed/Reviewed the latest feedback" lead-in, the "Base-conflict check" line, and the "Re-review when you have a moment" footer — were English-only and sat outside that block. So the posted comment was only half translated, unlike the takeover-ack comments (full collapsed Chinese block) and the "model/模型" sign-off in this same report (already inline-bilingual). Give each wrapper line an inline Chinese translation, matching the model/模型 idiom. The English halves are preserved verbatim — the streak-reset detector globs on "Addressed the latest review feedback" and "no changes needed", and a test extracts these lines — so behaviour is unchanged and old English-only comments still match. A new test pins each English-Chinese pair so a future reword that drops the Chinese fails. The terminal handoff/failure comment is left English-only for now (SKILL.md keeps it so by design); that is a separate change. Co-authored-by: wenshao <wenshao@example.com> |
||
callmeYe
|
b436855a40
|
feat(core): propagate trusted daemon invocation context (#7279)
* feat(core): propagate trusted daemon invocation context * test(cli): update ACP startup expectation * refactor(core): centralize ACP capability env key * test(cli): update worktree ACP core mock * test(integration): run daemon context smoke on PRs * test(ci): update no-AK smoke expectation * test(core): cover invocation context isolation * fix(cli): compare ACP capability safely * fix(docs): restore GitHub action input names * fix(core): sanitize private ACP capability from child env * fix(core): reuse private ACP capability env constant * test(cli): cover malformed trusted invocation context * test(acp-bridge): assert exact child environment --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: 易良 <1204183885@qq.com> |
||
|
Shaojin Wen
|
5df71c8fd1
|
fix(autofix): retry an agent timeout instead of advancing past its feedback (#7563)
A timeout evaluated NOTHING — the agent ran out of budget before finishing, so nothing was committed and the feedback is unaddressed. It was treated as an evaluated verdict (real ts, watermark advances), which strands that feedback: the next scan sees "nothing new" and never retries. Observed on #7471 (round 13/100), a heavily-reviewed 1871-line PR: rounds 11 and 13 timed out, but round 12 pushed — so a timeout is transient far more often than not, and advancing past it left the round-13 feedback unhandled. run-agent.mjs now drops an `agent-timeout` signal on result.timedOut, and the handoff routes it like a pre-verdict crash: sentinel ts (feedback stays live) and a retry, with a headline that names the real fix at the cap (split the PR or raise the budget). A PR that PERSISTENTLY times out is bounded by the round cap and the consecutive-failure cap, so this cannot loop forever — it just stops treating a one-off budget blip as a verdict. The loop guard stays terminal (a tool-call loop is a real defect, not a budget blip). An API error still routes to its own model-key handoff; the timeout signal is written only when NOT an API error. Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
51cf26e30a
|
fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal (#7490)
* fix(autofix): retry a skipped-Prepare instead of stranding the PR terminal A base/infra failure BEFORE the agent runs was misread as an agent crash and terminated the PR forever. When an early step fails — installing or building the trusted base, checkout, node setup — the `Prepare branch and feedback` step is skipped, so NEWEST is empty, and the report step's "crashed before reading feedback" branch fired: MARK_ROUND=MAX_ROUNDS, terminal, scan skips it on every future tick. Observed: a web-shell TypeScript break on `main` failed `Install dependencies and build` (which builds the trusted base) across a whole scan batch, and SIX healthy PRs were stranded terminal at round=100 in one run — including ones at round 9 and 11 that had nothing to do with the break. `round=100` there is a terminal sentinel, not 100 attempts. NEWEST-empty now splits on steps.prepare.outcome: - 'skipped' (an earlier step failed, the agent never ran) is infra/base and transient: retry with a sentinel ts so the feedback stays live, incrementing the round so a PERSISTENTLY broken base is still bounded and stops at the cap (recoverable with /retry). - 'success'/'failure' (Prepare ran, no feedback produced) is a genuine pre-read agent crash: unchanged terminal behaviour. This is the reverse of the asymmetry #7482 addresses: that bounds a crash AFTER reading that retried forever; this stops a transient failure BEFORE reading from going terminal after one. * docs(autofix): note a pre-Prepare cancel also retries intentionally (#7490) * fix(autofix): also retry a cancelled/empty prepare outcome, not just skipped A previous review comment on this PR noted that a job cancelled before Prepare should retry too. It was right about the intent but the code did not do it: `steps.prepare.outcome` is 'cancelled' for a cancel and '' for a job that stopped before Prepare entered the step context — both DISTINCT from 'skipped', so `== 'skipped'` sent them to the terminal branch, the same over-termination this PR exists to fix. Match on "not a real Prepare run" (`!= 'success' && != 'failure'`) instead, so skipped, cancelled, and empty all retry; only a Prepare that actually ran to a verdict (success/failure) with no feedback stays terminal — the genuine pre-read agent crash. Test extended to drive the cancelled and empty cases (retry) and both real-run outcomes (terminal); mutation-verified that reverting to `== 'skipped'` reddens the cancelled case. * test(autofix): update the pre-read-crash case for the broadened retry The prior commit broadened NEWEST-empty retry to skipped/cancelled/empty but left the older 'replays the handoff decision' test asserting the old terminal behaviour for an unset PREPARE_OUTCOME (which now retries). That test's terminal cases now set PREPARE_OUTCOME=success/failure explicitly — the only outcomes that still terminate — so it exercises the genuine pre-read agent crash rather than the infra/cancel path. * test(autofix): anchor the skipped-Prepare extraction past the CONSEC block CI reddened `retries a skipped-Prepare` after main's consecutive-failure cap (#7482) merged into this branch: that block was inserted between this decision block and the report `{`, and it calls `gh api`. The test's `{`-anchored regex over-captured through it, so the extracted script ran the unstubbed `gh api` and failed. Anchor the end on the same `# Consecutive-failure` comment the sibling gate-crash test already uses, so the extraction stops at this decision block's own closing `fi`. * fix(autofix): exempt skipped-Prepare from the consecutive-failure breaker A broken base build skips Prepare, producing no API error file — so the consecutive-failure breaker ran on the new retry path and, after 5 scans, re-introduced the exact mass-stranding this PR exists to prevent. Exempt pre-agent infra failures (skipped/cancelled/empty outcome) from the breaker, mirroring the transient 429/5xx exemption: same failure class (not the PR's fault, self-heals, hits the whole batch). The round cap + sentinel-ts /retry recovery already bounds a persistently broken base. Also trim "checkout" from the retry headlines (checkout failures do not land in this branch) and hoist the duplicated MARK_TS assignment. * fix(autofix): reset the consecutive-failure streak on prior infra-failure markers The streak walker counted prior infra-failure headlines ("AutoFix could not start —…") as failures, inflating the consecutive-failure count on subsequent rounds. A PR with 3 real agent failures, then 3 rounds of base-build infra failures, then 1 more real failure would trip the cap-5 breaker even though only 4 rounds were the PR's fault. Add the two infra-failure headline patterns as reset strings in the streak walker, alongside the existing push and no-op resets. The genuine agent-crash headline ("AutoFix could not start evaluation —…") is deliberately excluded — it is a real failure and must still count. * fix(autofix): clarify infra-failure headlines and else-branch comment (#7490) Address review nits: the retry headline now mentions cancelled runs, the cap headline says 'reached the round cap' instead of overstating 'could not start for N rounds', the else-branch comment says 'prepare itself crashed' instead of 'agent crash', and the streak-reset pattern is simplified now that both infra headlines share the same prefix. --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
jinye
|
7c73768fa5
|
perf(startup): lazy-load Google GenAI SDK on first use (#7512)
* perf(startup): lazy-load Google GenAI SDK on first use Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#7512) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * codex: address PR review feedback (#7512) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
易良
|
ce40e33a65
|
fix(ci): autofix route checks existing labels on non-trigger label events (#7481)
* fix(ci): autofix route checks existing labels on non-trigger label events When triage adds multiple labels in sequence, per-issue concurrency cancels earlier runs. If the last label is not a trigger label (e.g. scope/build-system), the surviving run skips the issue phase even though the issue already has autofix/approved + status/ready-for-agent. Before ignoring a non-trigger label event, check ISSUE_LABELS_JSON for both required labels. If present and the issue is open, proceed with the issue phase. Trust was already established when the trigger labels were applied (both require triage+ permission). * fix(ci): require trusted sender for label fallback |
||
|
Shaojin Wen
|
ca084dd11f
|
feat(autofix): stop a PR that fails to push for N rounds in a row (#7482)
* feat(autofix): stop a PR that fails to push for N rounds in a row Under takeover the round cap is 100, which is right for a PR that needs many PRODUCTIVE rounds. It is wrong for one that fails every round: #6723 ran 7 consecutive failed rounds (3 agent timeouts at 50 min, 4 gate rejections whose fix broke tests) over 8 hours, heading for round 100, because it is a 5700-line, 47-file, 5-day-old PR racing a fast-moving main — every round re-resolves a conflict it cannot finish or that fails the gate. Retrying at the same per-round budget will not converge; a human has to rebase or split it. Adds CONSECUTIVE_FAILURE_CAP (5), distinct from the total round cap. The handoff step already runs only when a round did NOT push, so it counts the unbroken run of prior failure markers — stopping at the first push ("Addressed the latest review feedback") or legitimate no-op ("no changes needed"), either of which proves progress and resets the streak. At the cap it forces the terminal round even under takeover, with a handoff that names the real fix (rebase/split, then /retry). Cause- agnostic: a timeout and a gate rejection both count. * fix(autofix): address review feedback on consecutive-failure circuit breaker (#7482) - Fix misleading comment: the walk is oldest-first (API order) with reset-on-success, not newest-first with early stop - Prefer the already-fetched ic.json over a redundant gh api call, falling back to the API only when the file is missing - Filter eval markers by re-arm window (win=) so pre-re-arm failures do not immediately re-terminate a re-armed PR - Add test coverage for the MARK_ROUND == MAX_ROUNDS guard and for window-scoped streak counting * fix(autofix): exempt transient model errors from consecutive-failure breaker (#7482) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
cbf1c55595
|
docs(autofix): require evidenced pre-commit verification, not a bare "verified" (#7486)
* docs(autofix): require evidenced pre-commit verification, not a bare "verified" The skill already said to run build/typecheck/lint/Vitest before committing, but softly — and #7408 committed a fix with a TS error the gate then rejected while its summary claimed "verified all 3 commits". A self-assessment the gate contradicts wastes a whole round. Strengthens the address-review contract from "run the checks" to: - actually run them, do not assert them from reading the diff; - if typecheck or a touched-package test fails, do NOT commit — treat the feedback as unresolved (failure.md); - end address-summary.md with a `## Verification` section listing each command run and its result; a bare "verified" is not acceptable. The framing is structural, not etiquette: the deterministic gate re-runs the same commands and discards the round on any failure, so skipping them only moves the rejection later. Pinned by a test so it cannot soften back. This is the checkable half of "audit before committing" — the undirected/reverse-audit-until-clean practice does not transfer to an unsupervised agent (no verifiable stopping condition, and it worsens the timeouts seen on large PRs), but "run the gate's own checks first and show the evidence" does. * fix(autofix): clarify Verification section precedes collapsed Chinese translation (#7486) --------- Co-authored-by: wenshao <wenshao@example.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> |
||
|
易良
|
b1eeaae8c1
|
fix(release): exclude mobile-mcp from core version bump (#7474)
mobile-mcp has its own release cadence and CODEOWNER (@LaZzyMan). Bumping it in every core release PR forces an extra approval from LaZzyMan even though the change is a routine version bump. Exclude @qwen-code/mobile-mcp from the workspacesToExclude list in scripts/version.js, following the same pattern as @qwen-code/sdk. Closes #7462 |
||
|
Shaojin Wen
|
86b4b281ee
|
fix(autofix): keep a still-red check visible until its head is judged (#7438)
* fix(autofix): keep a still-red check visible until its head is judged A red check is a persistent STATE, but the scan only counted checks that failed AFTER the watermark. The moment the watermark passed the failure the PR went quiet while still red. Measured on the live fleet: #6451 watermark 10:55 3 reds completed 09:30, 09:30, 09:51 #7357 watermark 09:18 1 red completed 07:59 #7390 watermark 11:27:37 red completed 11:27:37 — a strict `>` hid it the instant it appeared All three sat red for hours while every scan logged "nothing new", and #6451 wrote two consecutive no-ops whose reasoning never mentions the three failures, because they were not in its feedback at all. A currently-red check now counts as feedback until the head it ran against has been evaluated. The address job records that head in its own `autofix-redcheck` marker — carried inside the eval comment, so no ts/acted/round parser changes and the agent still never sees it as feedback — and the scan skips a PR whose recorded head still matches. That bounds this to ONE look per head rather than every scan, which is what keeps a permanently-red PR from being re-selected forever. The head comes from the REMOTE, not local HEAD: after a rejected push the two differ, and recording a sha that never landed would suppress the reds on the head that actually exists. Empty on failure — matches no marker, so the reds stay visible. Two existing count assertions are replaced by the property they stood for: every check selector in the scan carries the address carve-out. * fix(autofix): pair REPORT_HEAD with the steps that emit its marker Review found the assignment had landed in issue-autofix's "Report dry-run / failure" step, which emits no redcheck marker and has no ${PR} in scope — dead code plus a malformed, swallowed API call. Verifying it surfaced a second half the review did not state: review-address's OWN handoff step emits the marker at line 3362 with REPORT_HEAD never assigned in that step, since shell variables do not cross step boundaries. Neither step sets `set -u`, so it expanded empty and the marker recorded no head — fail-open, but the handoff path never recorded one. Deletes the dead assignment, adds the missing one, and rewords the scan log so the two overlapping counts no longer read as a sum. The test now asserts the PAIRING per step block — emits iff defines — rather than counting each kind. Counting was what let this through: both counts were "right". The first fix for it keyed the sets by step NAME, which merged the two identically-named "Report dry-run / failure" steps and still passed with the bug reintroduced; keying by step block catches it. * fix(autofix): note fail-closed asymmetry on empty LIVE_HEAD (#7438) * fix(autofix): close three state-transition gaps in persistent red-check tracking (#7438) - Forward persistent red checks into agent feedback: the scan selects via N_RED_NOW but the prepare renderer only showed checks that failed AFTER the watermark, leaving the agent with an empty Failed checks section. Add a Still-red checks section with the complement filter. - Omit the redcheck marker on sentinel/retry handoffs: a sentinel ts means the agent evaluated nothing, so recording a judged head would suppress the retry the handoff promises. - Record the checked-out head, not the report-time remote head: capture the SHA in prepare before agent mutations and forward it as a step output, so a mid-run branch move cannot stamp an unevaluated head as judged. * fix(autofix): test empty-LIVE_HEAD fail-closed path (#7438) * fix(autofix): discard no-op same-head duplicates in the queued-job stale gate (#7438) Two near-simultaneous scans can both enqueue the same PR with the same watermark. When the first serialized job ends in a no-op, it records a redcheck marker for the head it judged but leaves both the eval timestamp and the round UNCHANGED — so the live-watermark/round revalidation never fires, and the second job re-runs the agent and posts a duplicate report for the same head. Parse the latest live redcheck marker during prepare (mirroring the scan's RED_HEAD parse) and add its head match against CHECKED_OUT_HEAD as a third stale-duplicate signature, reusing the existing "nothing newer" revalidation so newer feedback or a live conflict still keeps the target actionable. --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
jinye
|
042f6373b7
|
perf(startup): Load undici lazily behind package-local dynamic imports (#7455)
* perf(startup): load undici lazily behind package-local dynamic imports * fix(web-search): preload undici before building runtime fetch options Address review: web search builds fetch options outside the content generator preload path, so 33 web-search tests (and any standalone search invocation) hit the requireUndici fail-loud guard. Also redact and rethrow proxy dispatcher install failures, guard early promise rejections against unhandledRejection, and pin the guard message with a test. * test(cli): cover loadUndici interop and gitUtils proxy path Address review suggestions: add parameterized tests for the CJS unwrap normalization used by both core and cli loadUndici helpers, and verify getLatestGitHubRelease instantiates ProxyAgent when a proxy argument is passed. * fixup! test(cli): fix loadUndici test type errors Export UndiciModule type and loosen test helper typing so the cli package builds under tsc --build. |
||
|
Shaojin Wen
|
c06da8da28
|
feat(autofix): feed the gate's rejection back so the retry can fix what it broke (#7368)
* fix(autofix): retry a verification-gate crash instead of burying the agent's fix A gate failure had two very different meanings collapsed into one outcome. When the gate DECLARES a verdict (outcome=failed) it evaluated the agent's attempt and rejected it, so advancing the watermark is right — the same feedback would reproduce the same rejection, and MAX_ROUNDS bounds it. But when the gate dies WITHOUT a verdict it never judged the work at all, and advancing buries a fix the agent had already written: the next scan sees "nothing new" and the PR sits until a human deletes the marker by hand. That is exactly how the nested-package ENOENT stranded #7329 and #7336. Both agents had implemented the review feedback — the handoff even quoted the implemented changes — but the gate crashed on its own bug while resolving packages/channels/*, the commit was discarded, and the PRs read as "Could not address the latest feedback automatically". Two halves: - The review-address gate now declares every rejection it can legitimately reach: build, typecheck, lint and the per-package tests each call a `reject_fix` helper that writes outcome=failed before exiting. (The resolver call is deliberately left undeclared — a resolver error IS a gate bug.) - The handoff treats an EMPTY outcome on a non-success job as the gate's own crash and routes it to the existing sentinel/retry path, so the feedback stays live and the next scan retries. The round still increments, so a persistently crashing gate is bounded exactly as before, and the headline names the real cause ("hit a verification-gate error before reaching a verdict") and, on the final attempt, points at the gate logs. Unchanged: a declared rejection still advances and reads as before, a no-output crash keeps its own wording and retry, and a crash before the feedback was read stays terminal. Tests: the real extracted decision block is replayed under bash across declared rejection (advances to NEWEST), gate crash (sentinel + retry + round+1), no output (sentinel, original wording), the round cap (operator fix), and a successful job (never a crash); plus the reject_fix helper is driven for real to prove a rejection writes outcome=failed. Both mutation-verified — dropping the crash arm, or unwiring one known rejection, turns them red. * feat(autofix): feed the gate's rejection back so the retry can fix what it broke #7208 was handed to a human over a two-character fix. The agent implemented two review findings, the gate refused the commit because it did not compile (TS4111: `truncated` comes from an index signature, use `['truncated']`), and the loop stopped there — round 5/100, "A human should take over this PR". Nothing in the loop could have recovered on its own, because the reason was never carried anywhere the loop could read it: - the handoff comment showed only the agent's optimistic summary, so neither a human nor the next round could see WHY it was refused; - the feedback filter (correctly) excludes the bot's own comments, so a retry re-read only the original review points; - so `@qwen-code /retry` would have re-run the same agent against the same input and produced the same non-compiling change. The compiler had already said exactly what was wrong. The loop just threw it away. Three pieces carry it instead: - Each deterministic check now runs through `run_check`, which tees its output to a gate log; `reject_fix` writes the label plus the tail of that output to gate-rejection.md. (A four-backtick fence keeps captured ``` output from breaking out when this is posted as a comment.) - The handoff comment carries that block between `<!-- autofix-gate-rejection-start/end -->` markers, so a human sees the real reason next to the summary instead of a report that reads like success. - `Prepare branch and feedback` lifts it back out of the bot's newest comment and puts it at the top of the next round's feedback: "Your previous attempt was REJECTED by the verification gate — fix this first." So a mechanical rejection now closes inside the loop, which is the point of takeover. A rejection the agent cannot fix still burns rounds and ends at the same handoff, bounded exactly as before. Tests: the round trip is exercised end to end — a failing check's compiler output lands in gate-rejection.md with its label, the handoff delimits it, and the prepare step recovers the text (markers stripped) from the newest bot comment while a round that pushed yields nothing to replay. Both halves mutation-verified. #7351's verdict test is retargeted to run_check. * fix(autofix): declare the gate verdict before writing its detail file CI caught this and macOS could not: reject_fix wrote gate-rejection.md first and outcome=failed second, so a failure to write the detail took the verdict with it. An empty outcome on a failed job is the signal for "the gate never reached a verdict" — a crash, which is RETRIED — so a clean rejection whose detail write failed would be re-attempted every round instead of being reported once. The verdict is now written first and the detail write is non-fatal. The ordering is pinned by a STATIC assertion, not only the behavioural one: bash 3.2 suspends set -e through a `||`-invoked function and bash 5 does not, so the wrong order runs clean on macOS and aborts on a Linux runner. That is exactly how it shipped green locally and red in CI, and a guard that depends on the reviewer's bash would let it happen again. * fix(autofix): escape the gate-rejection detail for real The gate-rejection publish site used `sed 's/<!--/<!\-\-/g'` — single backslashes, which sed reads as escaped literal `-`, so the replacement is byte-identical to the match and the whole command is a no-op on both GNU and BSD sed. The other four publish sites use `\\-\\-` correctly. That mattered: the detail is `tail -c 3000` of build/typecheck/lint/test output, published verbatim in a bot-authored comment. The scan parses markers by matching the literal `<!-- autofix-eval ts=`, and it only counts markers in bot-authored comments — so any check output containing that string would have been parsed as a real eval marker. The existing test counted the CORRECT spelling and asserted there were four of them. A fifth site with the wrong spelling did not match the counted string, so the count stayed at four and the test stayed green. It now asserts every `s/<!--/…/g` site is byte-identical to the correct form, which fails on exactly this bug. Reported by qwen-code-ci-bot on PR #7368. * chore(autofix): correct stale "ALL FOUR" escape-site comment to five (#7368) * chore(autofix): document the head/tail byte-limit invariant (#7368) --------- Co-authored-by: wenshao <wenshao@example.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> |
||
|
Shaojin Wen
|
e6980841a3
|
feat(autofix): raise the strict round cap from 5 to 10 (#7412)
* feat(autofix): raise the strict round cap from 5 to 10 Measured across the last 40 bot-authored PRs: 17 finished at round 0, 12 at 1, 4 at 2, 2 at 3, 1 at 4, and 3 reached the cap of 5. All three that reached it merged AT it rather than stalling — and one of those spent two of its five rounds on the verify-gate ENOENT that #7330 has since fixed. So the ceiling was never the thing that stopped a PR, but it sat close enough to bind on a bad day with no headroom. 10 gives that headroom. The cap exists to stop an unproductive LOOP, not to ration ordinary iteration; a genuinely stuck PR still stops, just later. Deliberately not larger: retries for a transient model or gate failure increment the same counter, so the cap also bounds how much one bad provider window can spend. API_AUTH_MAX_ROUNDS stays at 3 and still short-circuits the errors only a maintainer can fix. Replaces the literal `MAX_ROUNDS: '5'` assertion with the ordering the numbers must satisfy — auth cap < strict cap < takeover cap — so the values stay tunable and a cap that stops binding fails instead. * fix(scripts): anchor round-cap regex with word boundary (#7412) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
837358f637
|
fix(ci): tell a triage action crash apart from a silent agent (#7418)
Both no-response cases produced the same error, and its advice — "check
the 'Run Qwen Triage' step stderr above for diagnostics" — pointed at
diagnostics that do not exist for one of them: the action installs the
CLI with `npm --silent`, so an install failure prints an exit code and
nothing else. Observed on a PR whose triage died at exit 243 during that
install, leaving a maintainer told to read an empty log.
The two causes need opposite responses, and steps.triage.outcome already
distinguishes them:
- action failed -> no model call happened, nothing about the PR can
explain it, and the fix is to re-run the job
- action succeeded, empty summary -> the agent ran and returned nothing,
which IS worth reading the step output for
Measured baseline for the retry advice: of 19 executed triage runs, 4
failed, 3 of them on unrelated PRs, and one PR both succeeded and failed
within an hour on the same head.
The outcome is passed through env like RESPONSE already is, and the
script is replayed under bash for both branches.
Co-authored-by: wenshao <wenshao@example.com>
|
||
|
Shaojin Wen
|
32ef628f95
|
perf(autofix): stop the feedback gate waiting on the LLM review check (#7416)
The scan skips any PR with checks in flight so a FAILED check can be read as feedback. `review-pr` is not that kind of check: its output is a REVIEW, delivered by its own real-time pull_request_review trigger and counted by the review path, so its conclusion carries nothing the loop acts on — but the PR stayed invisible to the scan for its whole duration. Measured over 32 completed review-pr runs: median 49 minutes, p75 78, p90 123, max 158. That is what a PR waited before autofix could touch it, even when it already had unaddressed feedback. Only `review-pr` is excluded, by name, via NON_BLOCKING_CHECKS. Build/test/lint still block, because a failed one IS feedback. So does `resolve-pr`, which mutates the branch. A test pins each excluded name to a real job id in qwen-code-pr-review.yml — a rename there would silently restore the wait with nothing failing. The behavioral replay caught a real bug before it shipped: the first version wrote `$nonblocking | index(.name)`, where `.` is already the array, so `.name` indexed the wrong object and jq errored out — which would have made HAS_PENDING_CHECKS empty and skipped nothing at all. Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
1b41cbb516
|
fix(ci): serialise the two workflows that push to a PR head branch (#7392)
`@qwen-code /resolve` and the autofix loop's own conflict path both merge the base branch and push to the PR's head, but they live in different workflows, so their per-PR concurrency groups only guarded each against itself. On #7355 they ran together: /resolve pushed at 03:51, the autofix leg pushed at 04:05 and was rejected `fetch first`, discarding a full agent run and leaving no marker behind. GitHub concurrency groups are repository-scoped, so both jobs now use the same `qwen-pr-head-write-<pr>` group and queue behind each other instead of racing. The prefix has to be a literal in both files — job-level `concurrency` cannot read the `env` context — so a test pins the two equal; renaming one side alone would silently re-open the race. Co-authored-by: wenshao <wenshao@example.com> |
||
|
jinye
|
636971efad
|
perf(telemetry): lazy-load the SDK and split OTLP exporter chains by protocol (#7276)
* perf(telemetry): lazy-load the SDK and split OTLP exporter chains by protocol * fix(telemetry): close lazy SDK init/shutdown races and make load failure non-fatal Addresses PR #7276 review feedback: shutdown now awaits an in-flight init before tearing down (was racing past the sync flag and leaking a started SDK whose buffered spans/logs never flushed); the dynamic imports now sit inside init's try so a chunk-load failure degrades telemetry instead of aborting daemon runtime startup. Also breaks the sdk<->sdk-impl import cycle via a leaf otlp-urls module, hardens the sdk-node exporter stub for thenable/interop probes with a unit-tested separator-independent resolve, lists the HTTP exporter packages explicitly in the bundle guard, and adds lazy-init lifecycle tests. |
||
|
Shaojin Wen
|
7b70d6df5f
|
perf(autofix): raise fleet simultaneity from 3 to 5 (#7396)
max-parallel is the only place different PRs wait on each other — the per-scan target budget (10) and the candidate-inspection budget (60) are both far from binding at the current pool of 15. On the scan that selected 7 PRs the legs ran exactly 3 at a time, each new one starting 3-4 seconds after a slot freed, so the 7th waited 81 minutes for a slot. Replaces the literal `max-parallel: 3` assertion with the invariant it was standing in for: a bound must exist and must still bind below MAX_TARGETS_PER_SCAN. Pinning the number only detected edits — it would not have caught the key being deleted outright, which is the actual regression, and it forced this tuning change to touch a test. Co-authored-by: wenshao <wenshao@example.com> |
||
Edenman
|
c33ca7227a
|
fix(web-shell): restore scheduled task reference interactions (#7313)
* fix(web-shell): restore scheduled task reference interactions * chore(web-shell): remove PR screenshot artifact * fix(web-shell): refine scheduled task tag removal --------- Co-authored-by: 克竟 <dingbingzhi.dbz@alibaba-inc.com> |
||
|
Shaojin Wen
|
0d5eab418a
|
fix(ci): stop /resolve reports from being guillotined mid-sentence (#7389)
* fix(ci): stop /resolve reports from being guillotined mid-sentence Every substantive /resolve summary was hitting the 2000-byte cap exactly and stopping mid-word: #2993, #4256 and #6206 all ended at 2100 bytes total, cut inside a sentence, with nothing saying the report had been clipped rather than abandoned. Two causes, both fixed: - The contract asked for a file-by-file inventory, which duplicates the diff and grows without bound. It now asks for what only the resolver knows — the root cause on the base branch, whether the merge was semantic or merely textual, what the resolution's correctness rests on, and what it could not verify (this command runs no tests and may not touch non-conflicted files, so a merge that breaks an untouched test can only be reported). - The cap was silent and too low. It is now 6000, above the 4000 the prompt asks for, and a report that still exceeds it says so. Also adds the project's collapsed Chinese section to the contract; no /resolve report had one. * fix(ci): make the truncation test fatal-decode real and link the run in the notice (#7389) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> |
||
|
Shaojin Wen
|
afd56117e4
|
fix(autofix): retry a model API error instead of stranding the PR (#7247)
* fix(autofix): retry a model API error instead of stranding the PR
When the agent's qwen subprocess dies on a model-side [API Error]
(403 access denied, a 429 quota, a 5xx), run-agent.mjs wrote a
handoff/failure.md, so the handoff step treated it as an EVALUATED
handoff — it advanced the watermark and the next scan saw 'nothing
new', stranding the PR until a manual re-arm. But the agent never
actually evaluated the feedback; the model was unreachable.
#7220 hit exactly this: fork-takeover engaged and ran the agent, the
model returned '[API Error: 403 Model access denied]' (the autofix key
lacks access to qwen3.8-max-preview), and the PR was left with an
advanced watermark that will not retry.
Fix, mirroring #7229's no-output-crash handling:
- run-agent.mjs extracts a [API Error: 4xx/5xx] from the captured
output tail, includes it in failure.md, and drops an
marker file.
- The handoff step reads that marker and routes the failure to the
sentinel-ts (retry) path — the watermark does NOT advance, so the
next scan retries; the round still increments so a PERSISTENT model
failure is bounded by MAX_ROUNDS. The headline names the model error
and, on the final attempt, tells the maintainer to check the autofix
model key/access and re-arm — instead of a generic crash message.
Tests: run-agent.mjs flags a model [API Error] (marker + failure.md)
and does NOT flag a generic failure; the handoff replay treats an
API-error handoff as sentinel|retry (not a watermark advance) with a
model-aware, cause-specific headline. 62/62 + 12/12.
* fix(autofix): scope + broaden the retryable model-API detection (review)
Addresses wenshao's review on #7247:
- Behavioral (1): the agent-api-error marker was written on ANY non-zero
exit whose output tail contained an API-error string — so a loop
guard, a timeout, or an agent-written failure.md (a real verdict)
would wrongly retry and, worst case, silently discard a verdict. The
write is now scoped to the bare-failure branch and guarded by
!timedOut, so only an un-evaluated model failure retries.
- Coverage (2): the old regex only matched a LEADING status digit, so
it missed the canonical rate-limit render, the (Status: …) form, the
bad-key 401, the Chinese quota text, and the unwrapped Qwen OAuth
quota — i.e. most real errors this targets. Detection is now a
whitelist of RECOVERABLE errors (401/402/403/429/5xx + rate-limit /
quota / api-key / RESOURCE_EXHAUSTED / overloaded phrasings, plus the
standalone OAuth-quota form); a 400/404 stays terminal.
- Test gap (3): a writer↔reader contract test now runs the REAL
run-agent.mjs to write the marker, then the extracted workflow reader
block against that same workdir — a rename on either side (proven
with the YAML-only mutation) now fails the suite.
- Smaller: API_ERROR_DETAIL is comment-escaped (sed) and capped
(cut -c1-200) since it derives from agent stdout; the marker match is
single-line ([^]\n]) so a multi-line render can't smuggle a newline;
agent-api-error is added to the run-artifacts list.
Non-recoverable 4xx (400/404) deliberately stay terminal; the live
401/403 config cases retry and self-heal once the key/access is fixed.
79/79 across both suites.
* test(autofix): cover the timeout guard and the OAuth-quota fallback (review)
Two coverage gaps from the ci-bot review on #7247:
- The !result.timedOut guard was only asserted indirectly — no test
emitted an [API Error] AND timed out. Added a case (spawnSync +
QWEN_TIMEOUT_MS=100): qwen streams [API Error: 503] then hangs past
the budget → killed → no marker. A refactor to !loopDetected now
fails here.
- The standalone Qwen-OAuth-quota fallback (unwrapped, no [API Error:])
had no test. Added a case emitting bare 'Qwen OAuth quota exceeded
(limit: 100/min)' → marker written, wrapped as
'[API Error: Qwen OAuth quota exceeded …]'.
* fix(autofix): anchor the API-error code, split retry budget by cause, keep the headline UTF-8
Addresses the review on #7247.
Classifier (points 2 and 4): the status code is now read from its POSITION in
the render (`[API Error: <code>`) instead of matched anywhere in the message.
Matching anywhere retried permanent failures forever — `400 Invalid value for
max_tokens: must be <= 512` matched a bare \b5\d\d\b and `400 context length
exceeded` matched a bare `exceeded`. `exceeded` now only counts as part of
`quota`. A 404 whose message says the model "does not exist or you do not have
access to it" — the OpenAI-compatible render of what a 403 reports — is no
longer terminal.
Retry budget (point 3): the marker now carries the cause class. A transient
429/5xx self-heals and keeps the full round budget; an auth/access error that
only a maintainer can fix is capped at API_AUTH_MAX_ROUNDS (3) and then goes
terminal with the "check the autofix model key/access, then re-arm" headline —
instead of ~100 agent runs and ~100 PR comments over ~17h on a takeover PR.
The terminal round is stamped so the scan's round gate skips the PR while the
sentinel ts keeps the feedback live for a re-arm.
Headline (point 1): `cut -c` counts bytes under GNU coreutils and the
classifier deliberately matches CJK renders, so the 200-byte cap could split a
multi-byte character and emit invalid UTF-8. Guarded with
`iconv -f utf-8 -t utf-8 -c || true`, matching the sibling publish site (the
`|| true` is required — iconv -c exits 1 when it discards).
Minor (point 5): documented that detection is best-effort because apiError is
derived from the last 20 KB of output; `head -1` -> `head -n 1`; tests added
for a permanent 400 carrying a 3-digit number >= 500 and for a >200-byte CJK
render staying valid UTF-8.
* test(autofix): cover the auth-capped retry budget and Chinese API-error patterns (#7247)
* fix(autofix): short-circuit 400 as terminal and classify only the last API error (#7247)
* fix(autofix): treat transport-level API failures as retryable
#7365 stranded at round 2/100 on this render:
[API Error: terminated (cause: read ECONNRESET)]
The connection to the model dropped mid-run. That is as transient as a 429, but
the classifier never saw it that way: a transport failure never got far enough
to have an HTTP status, so it fell through to the keyword arm, and the keyword
arm only knew about rate limits and quotas. It was classified terminal, the
watermark advanced, and a PR that needed nothing but a re-run was handed to a
human.
Verified against the shipped classifier before the fix — every transport render
came back terminal:
terminated (cause: read ECONNRESET) -> terminal
fetch failed -> terminal
socket hang up -> terminal
connect ETIMEDOUT -> terminal
Adds a transport arm to the code-less branch: ECONNRESET, ECONNREFUSED,
ETIMEDOUT, EPIPE, EAI_AGAIN, socket hang up, fetch failed, terminated.
ENOTFOUND is deliberately excluded. A hostname that does not resolve is a
misconfigured endpoint, which repeats forever — the same reasoning that keeps a
bad model name terminal.
Coded errors are unaffected: the arm sits after the status-code branch, so the
400 short-circuit added in
|
||
|
易良
|
10f14547a3
|
fix(cli): update npm installs safely in background (#7322)
* fix(cli): update npm installs safely in background * refactor(cli): simplify managed npm updates * refactor(cli): simplify background update completion * fix(cli): align managed updates with npm package layout * fix(cli): harden managed update lifecycle * fix(cli): invalidate managed updates after reinstall * fix(cli): validate staged npm updates * fix(cli): match home env parsing in launcher * fix(cli): harden managed background updates * fix(cli): bound managed npm updates * fix(cli): keep update notices live * test(cli): avoid ctime race in update test * fix(cli): gate screen reader update notice * fix(cli): show update notice on agent tabs * fix(cli): preserve managed update stderr * fix(cli): address managed update review * fix(cli): handle tilde qwen home without homedir * test(cli): make managed update paths portable * fix(cli): tolerate unavailable home directory |
||
|
Shaojin Wen
|
4e87922504
|
fix(autofix): refuse a non-main takeover out loud instead of only in the job log (#7382)
Applying autofix/takeover to a PR that targets another branch left no visible trace: the label stuck, the pull_request:labeled route run went green, and the only record was one line in a job log. A stacked PR then looked exactly like a managed one while nothing was managing it. The route now emits a 'base-refused' ack, and the ack job posts a bilingual explanation naming the base it was refused against. The refusal deliberately reads no live PR state, so the one ack whose whole purpose is to explain silence cannot itself be silenced by an unrelated API failure. Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
8065355ac3
|
feat(autofix): render the managed fleet into the scan's run summary (#7355)
* feat(autofix): render the managed fleet into the scan's run summary Seeing whether the loop was healthy meant reconstructing it by hand: list the bot's PRs, fetch each one's comments, regex the autofix-eval markers for round and watermark, then cross-check gh pr checks and the fork/takeover state. That is how today's triage of #7246, #7259, #7329, #7333 and #7336 was done, and it is why a stalled PR stayed invisible until somebody went looking for it. The scan already computes every one of those facts while deciding what to process — it just wrote them to a job log nobody reads. Each per-PR terminal decision now also records a row, and the step renders one markdown table into the run summary: | PR | State | Detail | | #7329 | SELECTED | 1 review + 5 inline new (round 0/5) | | #7333 | idle | nothing new since 2026-07-20T13:54:18Z | | #7262 | waiting | active checks in flight | | #7208 | round-capped | round 100/100 - needs a human or @qwen-code /retry | States cover every branch that ends a PR's inspection: busy, skipped, unknown, waiting, round-capped, idle and SELECTED — so a PR cannot drop out of the table by returning early, which is exactly the invisibility this fixes. No new API calls (the data is already in hand), no writes outside the run summary, and the helper is defined at the top of the step so it stays clear of the BUSY_PRS/INSPECTED proximity guard that keeps the free busy-skip from consuming the inspection budget. Tests: the real helper and render block are replayed over fixtures (table structure, one row per state, and an empty fleet still rendering a table), plus each decision branch is pinned to its fleet_row. Mutation-verified: dropping one branch's row turns it red. * fix(autofix): use temp file for fleet test replay; cover fork-head skip (#7355) * test(autofix): assert each skipped fleet_row call site individually (#7355) * fix(autofix): record fleet rows for both budget-break paths (#7355) The candidate-inspection budget break incremented INSPECTED but never called fleet_row, so the PR that tripped the budget was silently absent from the fleet table. The target-budget break left all remaining candidates invisible with no truncation signal. Add a per-PR deferred row before the inspection-budget break and a summary deferred row before the target-budget break so the fleet table stays complete in both cases. * fix(autofix): harden fleet summary render and clean up temp file (#7355) Address review feedback: - Escape '|' in detail values to prevent broken table columns - Render budget summary row (PR '-') as em dash instead of '#-' - Add trap for FLEET_FILE cleanup on early exit paths - Document deferred summary row semantics in test comment * fix(autofix): use summary row for candidate-inspection budget break (#7355) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
Shaojin Wen
|
96a39f4cb9
|
fix(autofix): retry a verification-gate crash instead of burying the agent's fix (#7351)
* fix(autofix): retry a verification-gate crash instead of burying the agent's fix A gate failure had two very different meanings collapsed into one outcome. When the gate DECLARES a verdict (outcome=failed) it evaluated the agent's attempt and rejected it, so advancing the watermark is right — the same feedback would reproduce the same rejection, and MAX_ROUNDS bounds it. But when the gate dies WITHOUT a verdict it never judged the work at all, and advancing buries a fix the agent had already written: the next scan sees "nothing new" and the PR sits until a human deletes the marker by hand. That is exactly how the nested-package ENOENT stranded #7329 and #7336. Both agents had implemented the review feedback — the handoff even quoted the implemented changes — but the gate crashed on its own bug while resolving packages/channels/*, the commit was discarded, and the PRs read as "Could not address the latest feedback automatically". Two halves: - The review-address gate now declares every rejection it can legitimately reach: build, typecheck, lint and the per-package tests each call a `reject_fix` helper that writes outcome=failed before exiting. (The resolver call is deliberately left undeclared — a resolver error IS a gate bug.) - The handoff treats an EMPTY outcome on a non-success job as the gate's own crash and routes it to the existing sentinel/retry path, so the feedback stays live and the next scan retries. The round still increments, so a persistently crashing gate is bounded exactly as before, and the headline names the real cause ("hit a verification-gate error before reaching a verdict") and, on the final attempt, points at the gate logs. Unchanged: a declared rejection still advances and reads as before, a no-output crash keeps its own wording and retry, and a crash before the feedback was read stays terminal. Tests: the real extracted decision block is replayed under bash across declared rejection (advances to NEWEST), gate crash (sentinel + retry + round+1), no output (sentinel, original wording), the round cap (operator fix), and a successful job (never a crash); plus the reject_fix helper is driven for real to prove a rejection writes outcome=failed. Both mutation-verified — dropping the crash arm, or unwiring one known rejection, turns them red. * fix(autofix): clarify retry-branch comments per review nits (#7351) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
c42b940685
|
feat(autofix): resolve the review threads whose findings it implemented (#7364)
* feat(autofix): resolve the review threads whose findings it implemented A human re-reviewing a managed PR currently has to re-read every thread to work out what the bot already handled. #7308 shows the cost: 17 review threads, 13 still open, with no way to tell which of those were fixed and which were declined. The agent already decides per finding — it records each one in address-summary.md as implemented or declined with a reason. What was missing was a way to act on that: the agent's sandbox carries no GitHub token, so it cannot resolve anything itself, and feedback.md gave it no stable handle to point at even if it could. Three small pieces close that: - feedback.md now renders each inline comment with its id (`- [rc:<id>] …`), giving the agent a handle it can echo back. - The SKILL asks the agent to write resolved-comments.txt: one id per line, for findings it IMPLEMENTED only. A declined or deferred finding must stay unresolved so its recorded reason actually gets read. - After a successful push, the step that already holds the PAT maps each id to its review thread and resolves it. Deliberately narrow: only threads the agent claims it implemented, only ones not already resolved, and entirely best-effort — a resolve failure warns and never fails a good push. Tests: the real extracted block is driven with a stubbed gh over fixture threads — an implemented finding's open thread is resolved, a DECLINED finding's thread is left open, an already-resolved thread is skipped, and an unknown id matches nothing. Mutation-verified: dropping the isResolved guard turns it red. * fix(autofix): harden review-thread resolution per review feedback - Guard --jq against null pullRequest (// {nodes:[]}) so a transient API inconsistency cannot crash the step after a successful push - Add pageInfo{hasNextPage} and emit :⚠️: when threads exceed the first-100 page cap - Tolerate rc: prefix and trailing CR in resolved-comments.txt - Emit :⚠️: when a valid numeric id matches no open thread - Match production set -euo pipefail flags in the extracted-block test --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code <qwen-code@users.noreply.github.com> |
||
|
Shaojin Wen
|
91d60a9812
|
fix(ci): stop a slow patrol classifier from killing every flaky rerun (#7358)
* fix(ci): stop a slow patrol classifier from killing every flaky rerun The CI Failure Patrol has been effectively offline. Across the last 30 scheduled runs, 28 were cancelled and only 2 succeeded — and both survivors ran at 00:0x, in ~2 minutes, when the model was idle. Step timings show why. Setup, checkout, node and the scan finish in 28 seconds; the model step then runs 9m39s and is killed by the job's 10-minute timeout. Because the job dies there, Validate and Upload never run, the `act` job is skipped on `classify.result != 'success'`, and nothing is ever re-run. One slow step was taking down the whole patrol, every cycle, for hours. That is why #7333 still carries a red `web-shell E2E Smoke` whose failure is `No space left on device` — a textbook infra flake, inside the patrol's own TARGET_WORKFLOW, that the patrol never got far enough to re-run. - The classifier step is now bounded at 5 minutes, below the job's 10, and marked continue-on-error: a slow model costs one patrol cycle instead of the patrol, and the next tick simply tries again. - An empty classifier result is reported (has_decisions=false) rather than failing the job, so the run finishes cleanly instead of looking like a broken patrol. - Upload and the `act` job are gated on decisions actually EXISTING, not merely on the classify job having survived — otherwise a no-decision cycle would look actionable. Tests: the step's timeout is asserted to be strictly below the job's, plus the continue-on-error and the has_decisions gating on Upload and `act`; and the Validate step's bash is replayed for real with and without a decisions file, asserting exit 0 and the right flag in both. Mutation-verified — removing the step bound, or the decisions step id, turns it red. * fix(ci): validate JSON syntax in patrol decisions before acting (#7358) --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
0717a2a0b6
|
feat(autofix): pick up managed fork PRs in real time instead of waiting for the throttled schedule (#7350)
* feat(autofix): pick up managed fork PRs in real time instead of waiting for the throttled schedule The `pull_request_review` trigger already routes feedback straight to the PR it arrived on, but it admitted ONLY in-repo bot PRs — every fork under takeover had to wait for the scheduled scan. That schedule is far slower than it looks: the cron says `*/10`, but GitHub throttles scheduled events on this repo hard enough that the observed interval is 40-70 minutes (and in the same window `pull_request_review` fired 17 times while `schedule` fired once). So the takeover PRs a maintainer is actively iterating on were the ones waiting longest for their feedback to be picked up. Real-time pickup now applies the SAME admission the scheduled scan uses for a fork: allow-edits on, and either the bot's own fork or an explicit autofix/takeover label. Nothing about *what* may run changes — this event runs in base-repo context, and review-address independently re-verifies allow-edits, a live write+ author and a matching live head repo before it touches the branch. Only *when* the same gated work happens changes. Unchanged: non-main targets, untrusted senders, human in-repo PRs and forks that are neither the bot's own nor takeover-labelled are all still ignored, and only `pull_request_review:submitted` triggers (not per-comment events). Tests: a behavioural replay drives the extracted route block with a stubbed gh across eight cases — in-repo bot admitted, in-repo human rejected, bot fork and takeover-labelled fork admitted and routed to that PR, and no-allow-edits, unlabelled fork, non-main base and untrusted sender all rejected. Mutation- verified: restoring the blanket fork rejection turns it red. * fix(autofix): admit real-time fork PRs in review-scan's forced predicate (#7350) The route step now admits managed fork PRs for real-time review pickup, but review-scan's forced-PR predicate still required `.isCrossRepository == false`, so every fork was rejected there: targets=[] / has_targets=false and review-address never started — the feature was silently discarded for the very PRs it added. Admit forced fork PRs under the scheduled scan's OWN fork rules (allow-edits on, plus a live write+ author check mirroring the scan's per-candidate gate); in-repo PRs keep the fail-closed `.isCrossRepository == false` test. review-address still re-verifies allow-edits, a live write+ author and a matching head repo before pushing. Also exercise the route step's metadata-read-failure branch (fails closed) in the workflow tests. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
2493248858
|
fix(review): make agent launches and cleanup resilient (#7259)
* fix(review): make agent launches and cleanup resilient Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): address worktree cleanup feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): harden cleanup follow-ups Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(ci): sync autofix workflow assertions Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(review): close cancellation cleanup gaps Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(review): cover remaining review-cleanup feedback suggestions (#7259) --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: 易良 <1204183885@qq.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
eb53ebed5b
|
feat(autofix): re-arm a stranded PR with @qwen-code /retry instead of deleting a marker (#7354)
* feat(autofix): re-arm a stranded PR with @qwen-code /retry instead of deleting a marker Recovering a stranded managed PR meant running `gh api -X DELETE` against the bot's own autofix-eval marker comment. That needed raw API access and the comment id, erased the audit trail, and was undiscoverable unless you had read the workflow — it came up twice while triaging #7246, #7329 and #7336. `@qwen-code /retry` now posts a single `<!-- autofix-rearm -->` marker, which does both halves of what the deletion did: - The scan's watermark ignores eval markers written BEFORE the newest re-arm, so the feedback those markers buried is read again. The watermark stays global otherwise — this is an explicit, maintainer-issued exception, which is exactly what the deletion was, only recorded instead of destructive. - The marker also opens a fresh counting window (it joins the engage ack in REARM_KEY), so the round counter resets and a terminal round stops skipping the PR. That also means the existing "a re-arm supersedes queued old-window jobs" guard covers /retry for free. The address job's live recheck mirrors both, so a run selected before a re-arm still discards itself instead of stamping an old-sequence marker. Authorization is the takeover command's, unchanged and reused rather than reinvented: exact body match, live permission lookup, in-repo-only author privilege. The route prefilter now admits the second command. The job verifies CI_DEV_BOT_PAT authenticates as the bot before commenting, because both scanners only count markers authored by it. The marker is registered as a control comment so the agent never sees the re-arm as feedback to address. Tests: the real extracted scan block is replayed over synthetic comment fixtures — stranded (watermark held, round 2), after /retry (watermark released, window reset, round 0), a marker written after the re-arm counting again, and a re-arm from a non-bot author correctly ignored. Both halves mutation-verified. * test(autofix): add behavioral test for address-side re-arm stale check (#7354) * fix(autofix): generalize remaining command-ignored messages and assert all filter sites (#7354) * test(autofix): add behavioral test for the retry-command re-arm marker job (#7354) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> |
||
|
Shaojin Wen
|
eca654f365
|
fix(autofix): resolve owning package for nested paths; report verify-failed handoffs as not pushed (#7330)
* fix(autofix): resolve owning package for nested paths; report verify-failed handoffs as not pushed The verify gate mapped each changed file to a flat `packages/<dir>` and read `<dir>/package.json`, which ENOENT-crashed on nested packages such as packages/channels/base — the container packages/channels has no package.json. Walk each changed file up to its nearest package.json in both the issue-fix and review-address verify steps, and skip any candidate that still has none. When such a verify failure follows an agent commit, the review-address handoff rendered the agent's optimistic address-summary.md (which can cite a commit SHA) under a neutral "what I found" heading, so a maintainer chased a commit that was discarded with the runner workspace. An EXIT trap now records any post-commit non-zero exit as outcome=failed, and the handoff states plainly that the change did NOT pass the gate and was NOT pushed. Tests: walk-up detection over a nested package tree, the outcome=failed trap, and the not-pushed handoff wording — each mutation-verified. * refactor(autofix): extract owning-package resolver to a shared staged script Addresses review on #7330. Extract the changed-file → owning-package walk into .github/scripts/resolve-owning-packages.sh, staged to RUNNER_TEMP from the trusted base alongside check-settings-schema.sh and invoked from both verify gates, so the two gates cannot drift into resolving packages differently (the 8-line walk was otherwise duplicated verbatim in each). Updates the package-scripts test that pinned the old inline grep. Narrow the verify-failed handoff lead-in to "This change was NOT pushed": four paths set outcome=failed BEFORE the deterministic gate runs (agent abort via failure.md, dirty tree, unchanged branch, missing address-summary.md), so the previous "did NOT pass the verification gate" claim was factually wrong for them. The specific reason stays in the headline and the quoted summary. * style(autofix): brace variable references in resolve-owning-packages.sh The repo's shellcheck gate runs --enable=all --severity=style, under which bare $f/$d references trip SC2250 (prefer ${var}). Brace them to match the convention already used in check-settings-schema.sh, and update the script content assertions accordingly. Verified with shellcheck 0.11.0 using the exact CI flags: clean. * fix(autofix): resolve owning workspace via npm query; key unpushed-handoff on commit existence Addresses the deeper review on #7330. Blocking issue: the "nearest package.json" resolver mapped a change under a workspace's fixture/example package (e.g. packages/cli/src/commands/extensions/examples/starter) to that fixture, whose test script is not Vitest — silently SKIPPING packages/cli's own tests, a coverage regression invisible in the log. Resolve against the authoritative `npm query .workspace` set instead and take each file's longest-prefix workspace: nested workspaces (packages/channels/base) match exactly, fixtures and non-workspace paths (packages/sdk-python, packages/README.md, the excluded packages/desktop) drop. Also harden the resolver against a final line with no trailing newline and against an unmatched last line, which under `set -o pipefail` would otherwise abort the script. Handoff wording: keying "was NOT pushed / commit discarded" on outcome=failed was wrong for the abort paths (failure.md, dirty tree, unchanged branch, missing address-summary.md), which set outcome=failed before ever making a commit. Record committed=true right after checkout — before any gate can fail — and key the wording on that; the abort/no-op paths keep the neutral framing. This removes the EXIT trap entirely (its only observable effect was that wording), so it no longer mislabels pre-commit failures either. * fix(autofix): expand workspaces on-disk so branch-added packages are tested; harden resolver Addresses the re-review on #7330. The resolver sourced its workspace set from `npm query .workspace`, which reads node_modules — installed from the BASE checkout. A workspace the PR branch ADDS (a new channel adapter, a new sdk — the issue-fix job's whole purpose) was invisible, so its tests were silently skipped, and for a nested new package the ENOENT crash this PR fixes turned into a silent skip. Expand the set from the on-disk root package.json `workspaces` globs instead (shallow `dir/*` + literals, honouring `!` negations, keeping dirs with a package.json): it reflects the branch, matches what `npm run --workspace` accepts downstream, and needs no install. Verified to reproduce `npm query`'s set exactly on the current tree. Also from the review: - Fail the gate loudly on an empty/unreadable workspace set instead of the silent "no package changes" skip, and drop the now-unneeded `|| true` at both resolver call sites (the resolver already exits 0 on legitimate no-match). - Record committed=true at the TOP of the step (ref-only diff), covering an agent that commits then aborts, and count only `git diff --quiet` exit 1 as a commit (128 is a git error, not a discarded commit). - Correct the two call-site comments that still described the superseded nearest-package.json approach. Also hardens the resolver against a final changed-path with no trailing newline and an unmatched last line under `set -o pipefail`. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
3cd9561c8b
|
fix(ci): tighten API error detection to avoid false positive on review prose (#7328)
* fix(ci): tighten API error detection to avoid false positive on review prose The result-text classifier matched *"[API Error"* which hits review summaries that quote the pattern in prose (e.g. reviewing PR #7247 whose summary mentions "[API Error: ...]" and "quota … limit"). The quota grep then fired on the coincidental "quota … limit" substring, falsely reporting quota exhaustion on a successful review. Require a digit after "[API Error: " so only real API error messages (e.g. "[API Error: 429 …]") trigger the failure path. * fix(ci): anchor API error detection on tail position, not status-code shape The status-code glob from the previous commit would silently miss real aborts whose message has no leading digit (Connection error, Status: suffix, Chinese rate-limit text) — trading a loud false positive for a silent false negative. Anchor on position instead: an aborted run renders the API error as (or at the very end of) the result text, while a successful review that discusses API errors quotes them mid-prose. Checking only the last 600 bytes separates the two without constraining the error message format. Also adds fixtures for the non-digit-leading shapes and for prose quoting a real status code mid-body, and removes a stale comment. * fix(ci): catch aborts longer than the tail window; grep full text for quota Add a whole-result check (case "$RESULT_TEXT" in "[API Error: "*) alongside the tail check so errors whose body exceeds 600 bytes are still detected — the prefix falls outside the tail window but the result starts with it. Move the quota grep back onto the full RESULT_TEXT so a long error with quota wording early in the message is still classified as quota (not downgraded to retryable). * fix(ci): anchor abort detection on trailing shape, not byte window Replace the prefix+tail-window split with an ends-with check: the stream-json adapter appends the formatted API error last, so an aborted run's result ENDS with "[API Error: …]" optionally followed by a rate-limit guidance suffix. Strip the three known suffixes, rtrim, then match *"[API Error: "*"]". This catches the production abort shape (partial review + appended error) at any error length, without a byte-window constant to tune or leave untested. Prose that quotes the pattern mid-body keeps writing afterwards and does not end with "]". Adds fixtures for the production shape (appended error, long appended error, rate-limit suffix after the bracket). * test(ci): pin suffix sync with errorParsing.ts; document ]-ending trade-off Add a sync test that reads RATE_LIMIT_MESSAGE_BY_AUTH from errorParsing.ts and asserts all three suffixes appear in the workflow — prevents silent drift if someone rewords one. Add a KNOWN-limitation fixture documenting that prose ending with ] after quoting the pattern is a false positive (accepted trade-off; the durable fix is checking that the bot comment landed). |
||
|
易良
|
5e183dda18
|
test(autofix): sync workflow assertions with split model vars (#7297)
The autofix workflow now plumbs QWEN_AUTOFIX_MODEL (with a QWEN_PR_REVIEW_MODEL fallback) into the report steps, and the prepare step documents the verification gate's git diff --quiet check in a comment. Update the two stale assertions so they match the workflow again without dropping their original intent. |
||
|
Shaojin Wen
|
0f1700b8a4
|
feat(autofix): auto-manage the bot's own fork PRs without a label (#7243)
A fork PR the autofix bot itself opened (its codex flow pushes to qwen-code-dev-bot/qwen-code) is the bot's own generated work — same author, same code provenance, and the bot holds write+ — so it is trust-equal to an in-repo bot PR. Requiring a manual autofix/takeover label on it was redundant: in-repo bot PRs are auto-managed with no label, and the takeover label exists to authorize EXTERNAL (human) fork authors, not the bot's own. Now a fork authored by AUTOFIX_BOT with 'Allow edits from maintainers' is admitted and managed WITHOUT a label: - Scan: fork candidates are unioned from bot-prs.json (the bot's own forks — --author AUTOFIX_BOT, so no label needed) AND the takeover-labeled list (non-bot forks, explicit opt-in). Both still require allow-edits and pass the per-candidate live write+ gate. - Eligibility: the fork chain no longer demands the takeover label when the author is the bot (the author check already exempts it); it still demands allow-edits + a live write+ author + a matching live head repo. autofix/skip still opts any such PR out. Non-bot forks are unchanged — they still need the explicit label. Tests: the fork-candidate union admits a bot fork (no label) + a labeled human fork, dropping no-allow-edits/in-repo/skip; the eligibility replay makes a bot fork with allow-edits eligible without a label and discards it without allow-edits. 62/62 + 12/12. Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
076427650d
|
feat(ci): auto-open a deflake fix issue for confirmed flaky tests (#7231)
Some checks are pending
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run
E2E Tests / cron-interactive E2E (nightly) (push) Waiting to run
E2E Tests / web-shell Browser Regression (push) Waiting to run
* feat(ci): auto-open a deflake fix issue for confirmed flaky tests
The CI Failure Patrol reruns flaky failures but never fixes them, so
the same tests flake forever on a rerun treadmill. This closes the
loop: when the patrol classifies a rerun as a nondeterministic TEST
(not infra), it now also opens ONE deflake issue that the existing
autofix issue pipeline develops into a reviewable stabilization PR.
- ci-flaky-patrol SKILL: a rerun decision whose cause is a specific
named flaky test carries an optional flakyTest {file, name}; infra
reruns (ENOSPC, network, runner death) never do.
- ci-flaky-rerun.mjs: validates flakyTest (malformed → the whole
decision is rejected, so a bad classification can't open a bogus
issue); after a rerun, ensureDeflakeIssue upserts a deflake issue
deduped by a stable (file, name) marker — one open issue per flaky
test across all PRs — labeled status/ready-for-agent + autofix/
approved so the scheduled autofix scan picks it up.
- .qwen/skills/deflake/SKILL.md: constrains the fix to four
assertion-preserving patterns (raise timeout/poll budget, stabilize
timing/waiting, make randomness/time deterministic, isolate
interference) and forbids skipping/deleting/loosening the check;
write failure.md if none applies or the failure looks like a real
bug. The produced PR is reviewable, never auto-merged.
Tests: deflakeKey stability/collision-freedom, the bilingual issue
body, one-issue-per-test dedup, no issue for infra reruns, and
malformed-flakyTest rejection. 34/34 across both patrol suites.
* fix(ci): deflake review hardening — rerun survives bad metadata, no markup injection
Addresses the two Criticals + suggestions on #7231:
- **Critical: a malformed/over-length flakyTest no longer kills the
rerun.** flakyTest validation is removed from validDecision (which
gated the PRIMARY action on secondary metadata — a >200-char nested
test name or a null silently dropped a valid rerun). Well-formedness
is now checked in ensureDeflakeIssue, which simply skips the deflake
issue when the metadata is bad; the rerun always stands.
- **Critical: markup/mention injection via the test path/name.** file
and name are code-span-stripped of backticks (which cannot be escaped
inside a span and would break out into live Markdown, turning
into a mention in a bot-created issue) and both now sit in
code spans. safeReason alone did not close this (it does not touch
backticks).
- Best-effort deflake: ensureDeflakeIssue is wrapped in try/catch so a
transient createIssue failure — after the marker is already posted —
no longer surfaces as a misleading "skipping PR" and permanently
suppresses the deflake; it retries on the next flaky occurrence.
- Run link uses the patrol's own repo (client.repo) instead of the dead
target.repo, so deflake issues on a fork don't 404.
- Body reworded: it no longer claims the rerun already passed (it runs
right after the rerun is triggered) — it says a real deterministic
failure is NOT flakiness and must not be stabilized.
- SKILL: bound file/name to 200 chars, and note a malformed one is
ignored (never drops the rerun).
Tests: malformed flakyTest keeps rerun (no createIssue); long title
truncates ≤240; backtick path/name cannot inject; run link honors the
repo; a throwing createIssue leaves the rerun intact. 38/38.
---------
Co-authored-by: wenshao <wenshao@example.com>
|
||
|
Shaojin Wen
|
a7bac6a433
|
test(autofix): exercise the SKILL stage↔resolve contract end-to-end (#7227)
* test(autofix): exercise the SKILL stage↔resolve contract end-to-end Follow-up to #7225, implementing the reviewer's non-blocking suggestions. The staging guard #7225 added pins the mirrored LAYOUT but re-implements run-agent.mjs's `<dir>/../SKILL.md` convention in the test. If that coupling ever moves in the RUNNER (e.g. ../../SKILL.md), the string test stays green while prod breaks again — the same class of blind spot that let #7165 ship. This adds the one check that exercises the contract for real: stage the actual runner into a mirrored tmp layout, run it with --print-prompt, and assert it reads the staged SKILL (sentinel body + resolved skill dir). The negative case — the flat layout #7165 shipped — is asserted to crash with ENOENT, proving the test catches that regression. Also replaces the brittle fixed-width `[\s\S]{0,200}` bound between `core.hooksPath .husky` and the runner invocation with a direct ordering assertion (indexOf), so adding a comment between the two lines can no longer fail the test spuriously. 61/61 + 12/12. * test(autofix): harden the stage↔resolve integration test per review Applies all four inline suggestions on #7227: - spawn process.execPath, not the bare 'node' string, so a version-manager shim or a PATH without node can't turn the test into an opaque 'null !== 0'. - nest the flat-layout runner under dir/flat/ so its ../SKILL.md resolves to dir/SKILL.md (never created) instead of a shared tmpdir()/SKILL.md that a concurrent job could leave behind and make the negative case pass spuriously — a real flake in the deflake-test itself. - reuse the existing withRunnerDir helper instead of duplicating its mkdtemp/try/finally/rmSync. - bound each spawnSync with timeout: 10_000 so a hung runner fails the test instead of the whole CI job (spawnSync blocks the event loop, so vitest's async timeout can't fire). 61/61 + 12/12. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
1e28b9a089
|
feat(review): retry transient API failures once; surface quota clearly (#7233)
* feat(review): retry transient API failures once; surface quota clearly The PR-review workflow failed permanently on any single API hiccup — a 502/503, a dropped connection, a rate limit — with only a fallback comment, so an idle PR sat without a review until someone re-ran `@qwen-code /review` by hand. Now the qwen invocation runs in a budget-guarded retry loop: - A transient outcome (non-quota API error, dropped/aborted run, empty output, error result) retries ONCE after a 60s backoff. - All attempts SHARE the review timeout budget, and a retry is capped at 5 minutes — a cleared transient succeeds fast, and a still-failing retry can't burn another hour (the observed quota run took 61 min, so an unbounded second attempt could blow the job timeout). - A quota-exhausted 429 is NOT retried in-run: its reset is typically hours out. It fails with kind=quota and the fallback comment now states the reset time and how to re-run once it resets — a clear recovery instead of a dead end. - A real timeout or a hard/config failure never retries (unchanged). Detection is exactly as before; only the disposition (retry / quota kind) is new. Behavioral test spawns the extracted loop under bash with a scripted stub qwen: success→1 try, transient→2 then success, persistent-transient→2 then fail, quota→1 try + quota kind + reset time, error-result→retry, hard-exit→no retry. 7/7. * fix(ci): guard the quota-detail grep, sync the timeout assertion, harden the retry tests Review follow-through on #7233's two Criticals and every suggestion: - The quota-detail grep ran unguarded in an assignment under `set -euo pipefail`: a 429 whose message lacks "reset at" exited the step before fail() wrote failure_kind, so the quota-aware fallback never fired. Guarded with `|| true`, and a quota_noreset scenario reproduces the exact message shape that died. - qwen-resolve-workflow.test.js still asserted the pre-refactor literal `fail "… ${QWEN_TIMEOUT} minutes." 1 "timeout"`; it now pins the OUTCOME/REASON pattern the loop actually uses. - The timeout REASON reported the total budget even when a 5-minute retry cap fired; it now names the attempt's own timeout beside the budget, so the fallback's --timeout advice matches what actually expired. - Quota matching tightened to quota-plus-context (exhaust/exceed/limit/ reset) so a transient "quota configuration" style error keeps its retry; the new test file uses the root `yaml` dependency instead of a hoisted js-yaml, carries the license header, anchors the loop extraction on the retry-budget comment instead of lastIndexOf, and gains the two uncovered scenarios: a real timeout is not retried, and an attempt with under 30s of budget never starts. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
58103b614a
|
fix(autofix): a no-output crash must not advance the review watermark (#7229)
* fix(autofix): a no-output crash must not advance the review watermark When a review-address run crashes AFTER prepare (so NEWEST is set) but BEFORE the agent writes any verdict — no address-summary.md, no-action.md, or failure.md — the handoff stamped the marker with ts=NEWEST, advancing the feedback watermark as if the feedback had been evaluated. It hadn't. The next scan then saw 'nothing new since <NEWEST>' and never retried, stranding the PR on a purely transient crash. That is exactly what happened to #7219 during the #7165 SKILL-staging outage: the run crashed at promptFor (ENOENT) at 09:50, the handoff advanced the watermark to 09:50:56, and even after #7225 fixed the crash the loop considered all prior feedback 'addressed' and would not re-engage. Fix: on a no-output crash (NEWEST set, DETAIL_FILE empty) stamp the sentinel ts instead — it is excluded from EVAL_WM, so the watermark does not move and the next scan retries the same feedback. The round still increments, so a PERSISTENT crash is bounded by MAX_ROUNDS and ends in a terminal handoff rather than looping forever. Agent-produced handoffs (verify failed after real output) keep advancing the watermark as before. Replay test extended to assert BOTH MARK_TS and MARK_ROUND across all three shapes: output+verify-fail → advance; no-output crash → sentinel (retry); pre-prepare crash → terminal. 60/60 + 12/12. * fix(autofix): correct the final-attempt crash headline per review Two review findings on the no-output-crash handoff: - The headline promised 'it will retry on the next scan' even on the final attempt, but at MARK_ROUND == MAX_ROUNDS the scan's round-cap gate skips the PR and the cap-reached notice is takeover-only — so a maintainer was told a retry was coming that never comes. The headline now branches: 'it will retry' only while MARK_ROUND < MAX_ROUNDS, otherwise 'this was the last automatic attempt; a human should take over'. - It embedded a Run log URL that the report block already appends to every handoff, duplicating it in the comment. Removed from the headline. Replay test extended: mid-attempt headline promises retry and carries no Run log; final-attempt headline says human-takeover and never 'retry'. 60/60 + 12/12. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
4998cb16d6
|
feat(autofix): surface the running model in every autofix report (#7226)
* feat(autofix): surface the running model in every autofix report Every visible autofix comment now carries a footer naming Qwen Code and the model it ran — for diagnosis (which model produced this) and as a small attribution for our own model. Four surfaces: the review-address fixed report, the no-action report, the handoff report, and the issue-phase PR's E2E comment. The model comes from the QWEN_PR_REVIEW_MODEL repo variable — already the agent's OPENAI_MODEL, a variable not a secret, so it is safe to echo into a public comment. Each reporting step plumbs it in and computes MODEL_DISPLAY with a 'default' fallback so an unset variable never renders a bare backtick pair. The footer sits with the report body (before the eval marker), and on the E2E path it is appended after the model's file, never injected mid-generation. Contract test pins the env plumbing and the footer on all four surfaces (twice in push-and-report, which carries both bodies), plus the append-after ordering on the E2E path. 61/61 + 12/12. * Update scripts/tests/qwen-autofix-workflow.test.js Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci@service.alibaba.com> |
||
|
Shaojin Wen
|
68c9032cd5
|
feat(autofix): direct takeover of maintainer-fork PRs (#7213)
* feat(autofix): direct takeover of maintainer-fork PRs Maintainer-approved v2: many maintainers work from personal forks, and adoption-snapshotting breaks their local workflow. A fork PR is now directly manageable when three live conditions hold — the takeover label, 'Allow edits from maintainers' (org-owned forks cannot enable it; adoption remains their path), and a fork author who holds write+ RIGHT NOW (the same live-privilege rule as the comment command, so an ex-member's fork can never summon secret-bearing runs). Plumbing: - Scan: fork takeover candidates are admitted per candidate (allow- edits + no-skip filtered in jq; the author's live write+ gate is one permission call each — a rare set); every matrix target now carries its head repo. - Address: prepare fetches the fork branch (origin has no copy) and checks out FETCH_HEAD with hooks already severed; the eligibility gate re-verifies takeover + allow-edits + author write+ live; the report step pushes back to the fork via the allow-edits grant. - Triggers: fork pull_request label events carry NO secrets, so the route notes them and the next scheduled scan engages (≤10m); the comment command now toggles fork PRs too (write+ senders only — fork authors stay silently dropped) and refuses only when allow-edits is missing, with the actionable ask. The scan posts a first-pickup engage ack (identity-verified, deduped on any existing ack, ic.json re-fetched so the same scan counts under the fresh window key) — closing the fork/manual-label ack gap and anchoring the round window. Behavioral coverage: fork-candidate admission jq (allow-edits, skip, in-repo exclusion, tsv rows), eligibility across fork+takeover+allow-edits+write / no-allow-edits / read-author, the toggle's fork split (refusal vs managed), plus plumbing pins (fork fetch/push forms, head_repo threading, first-pickup ack dedup). 60/60 + 12/12. * fix(autofix): strip stray patch-artifact quotes after two fi keywords Two inserted blocks ended 'fi"' — the quotes balanced against each other inside the same script, so bash -n stayed green while runtime would have lexed 'fi' as a command word and swallowed the span between them (the fork head-repo resolution tail and the engage-ack block) into one string. Removed both, and pinned the artifact class in the suite: a lone fi/done/esac followed by a quote now fails the tests. 61/61 + 12/12. * fix(autofix): author-filtered, re-armable first-pickup engage ack Reverse-audit findings on the scan-side ack: - Dedup was a raw grep over ic.json — a forged human comment carrying the engaged marker would have suppressed the real ack (and with it the window anchor). Dedup now selects bot-authored engaged acks via jq, same author rule as the window key itself. - Fork PRs get NO ack job (label events carry no secrets), so the documented re-arm gesture — remove and re-add the label to reset the round window — silently kept the old window: any historical ack blocked a fresh one. When a bot ack already exists, the scan now compares it against the takeover label's latest application time (issue events, fetched only in that rare case); a newer application posts a fresh ack, resetting window and cap as documented. Coverage: verbatim jq replays for both selections (forged-marker and released-marker exclusion, label-name filter, sort|last) plus the lexicographic re-arm gate pin. 61/61 + 12/12. * fix(autofix): review round 1 — ack ordering, dry-run, ghost-engage gate Addresses the maintainer review on #7213 (all findings confirmed): - Critical: the first-pickup ack read ic.json BEFORE the per-PR fetch — the first takeover candidate killed the whole scan step (missing file under -eo pipefail; every in-repo label-forced scan regressed), and later candidates dedup'd against the PREVIOUS PR's comments (bot PR ahead → fresh ack every 10min → window reset → cap never binds). The block now sits directly AFTER the fetch; its post-ack re-fetch keeps the downstream MARKERS/window-key reads fresh. A contract pin asserts the fetch precedes the first ack-timestamp read. - Medium: the ack now honors DRY_RUN (log only, window key untouched). - Medium: the command refused forks only for missing allow-edits — a below-write fork author was a silent ghost engagement (label sticks, no ack, nothing ever manages it). The command now mirrors the scan's author write+ gate with an actionable bilingual refusal. Found while fixing it: PR_INFO never fetched maintainerCanModify (or author), so EVERY fork toggle refused regardless of allow-edits — the test stub carried the field and masked the gap. Both engage-side fork gates are now also scoped to 'add': release is never blocked. - Low: the two new paginated jq reads are slurped (add-merged) so >100 comments/events cannot scramble the timestamp comparisons; replays now feed two concatenated page-documents. Fork fetch pins refs/heads/ (tag shadowing); HEAD_REPO_FULL guards each component (deleted fork = owner XOR name empty); fork-rotation caveat documented; forced-path refusal mentions the scheduled fork path. - Security caveat adopted: prepare proves fork push access with a --dry-run push right after checkout (allow-edits rides the classic-PAT grant only) and discards gracefully instead of 403ing after a full agent round. 61/61 + 12/12; YAML parses; every run block passes bash -n. * fix(autofix): fork targets keep base/branch invariants + 3 hardening follow-ups Blocking (yiliang114): the last fork elif ends the eligibility ladder for every eligible fork, so the LIVE_BASE/LIVE_BRANCH re-checks were unreachable for exactly the PR class the loop fetches and pushes — a labeled fork retargeted off main (or head-renamed) between scan and address would have had conflicts resolved against the wrong base. The base/branch invariants now sit ABOVE the fork chain (comment explains why the order is load-bearing), with replay cases pinning a retargeted and a renamed fork to the discard path. Follow-ups from the same review, all adopted: - PR_LIVE re-reads headRepositoryOwner/headRepository; a fork renamed or transferred since the scan discards at the live re-check (moved or unresolved, fail-closed) instead of fetching and token-pushing a stale path. The replay's fork fixture now carries its head repo and the harness provides the matrix HEAD_REPO to compare against. - The fork fetch failure (force-push/rename race) discards through the standard no-action path instead of a red run. - The first-pickup engage ack defers to the in-repo label event's DEDICATED ack job within a 3-minute grace after the label lands, so a concurrent ack job is never double-posted (which would shift the round-window anchor); a failed ack job is still healed by the next scan, and forks (no ack job) keep immediate pickup. Events are read once, before the branch split. Both hooks-order regex windows widened to span the new fork-arm guards (the assertions are about order; one hooksPath site genuinely covers both checkout arms). 61/61 + 12/12. * test: raise timeout ceiling for I/O-bound tests flaky under CI contention The self-hosted CI runners are heavily oversubscribed (core runs maxThreads: 16), and a recurring class of tests blows vitest's 5s default timeout purely under that contention — not from any logic fault. Observed repeatedly across unrelated PRs (#7213, #7219, and noted in prior sessions): - packages/core/src/utils/shell-ast-parser-lazy.test.ts — fully mocked, but the dynamic import + async coordination exceeds 5s when 16 threads contend. - packages/cli/src/serve/workspace-registration-store.test.ts — tempdir round-trip. - packages/core/src/extension/github.test.ts > extractFile — its waitForFileData helper polled a FIXED 1_000 setImmediate turns, which elapse in <100ms while the tar extraction I/O is still catching up, throwing 'Timed out waiting for extracted data'. Fixes: - testTimeout: 15000 in the core and cli vitest configs — 3x the default. Assertions still fail instantly; only the timeout ceiling grows, so this masks no logic bug (a real hang still fails, just later, and the job timeout still bounds it). - waitForFileData now polls a real ~10s wall-clock budget (2_000 x 5ms) instead of a fixed iteration count, so a slow extraction is awaited rather than raced. Stays under the 15s ceiling. These are the deterministic root-cause fixes for the flake class the autofix loop and CI Failure Patrol were papering over with reruns. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
5c7a5a84e0
|
fix(scripts): allow multiple dev:daemon instances by probing Vite port (#7212)
* fix(scripts): allow multiple dev:daemon instances by probing Vite port The Vite dev server port was hardcoded to 5173 with --strictPort, so a second npm run dev:daemon would crash immediately. Now the launcher probes for an available port (starting from 5173) and passes it explicitly via --port, matching how the daemon port is already handled. * fix(scripts): fail cleanly when the dev Vite/daemon port probe is exhausted The daemon and Vite port probes were bare top-level awaits, so an exhausted range (e.g. 5173–5182 all taken by other dev:daemon instances) rejected as an unhandled promise rejection — a raw stack instead of an actionable message. Wrap both probes and print the launcher's usual `[daemon-dev] <message>` before exiting 1, matching the validateLauncherArgs handling. findAvailablePort already rejects with the exhausted range, so the message names exactly which ports to free. * fix(scripts): let Vite handle port selection atomically Remove the probe-then-bind approach for the Vite port. The probe had a TOCTOU race (two launchers could pick the same port before either binds) and a host mismatch (probed 127.0.0.1 while Vite binds localhost which may resolve to ::1). Instead, drop --strictPort and let Vite auto-increment from its configured port 5173. Vite's --open uses the actual bound port, so the token URL is always correct. * fix(scripts): don't print a potentially wrong Vite URL in banner The banner hardcoded http://localhost:5173/ but with --strictPort removed the actual port may differ. Point at Vite's own output instead, which always prints the real bound port. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
易良
|
4e21f82c28
|
fix(ci): consolidate issue triage ownership (#7180)
* fix(ci): consolidate issue triage ownership Resolves #4786 * chore(vscode-ide-companion): regenerate NOTICES.txt to fix CI drift * fix(ci): close triage ownership review gaps * fix(ci): restore need-info label cleanup * docs(ci): document issue retriage triggers --------- Co-authored-by: Shaojin Wen <szujobs@gmail.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> |
||
|
Shaojin Wen
|
89db3d6aa0
|
fix(autofix): stage SKILL.md beside run-agent.mjs so review-address boots (#7225)
P0 regression from #7165. The review-address agent invokes a trusted staged copy of run-agent.mjs to avoid executing the PR branch's version on the host with the model key. run-agent.mjs resolves its instructions as `<own dir>/../SKILL.md`, but the staging was flat (${RUNNER_TEMP}/run-agent.mjs), so ../SKILL.md pointed at ${RUNNER_TEMP}/../SKILL.md = /home/runner/work/SKILL.md — which does not exist. Every review-address run since #7165 merged crashed with ENOENT before reading any feedback: the entire feedback-addressing and takeover path was down on main. Fix: stage the runner AND its SKILL in a mirrored layout (${RUNNER_TEMP}/autofix-skill/{SKILL.md,scripts/run-agent.mjs}) and invoke the staged runner from there, so ../SKILL.md resolves to the staged SKILL. This also closes a latent gap — the model's instructions now come from the trusted base too, never the checked-out PR branch. Regression guard: the suite pinned the cp string and the invocation but never checked SKILL.md was resolvable from the staged location. The test now derives the staged runner path from the invocation, computes <dir>/../SKILL.md, and asserts a cp stages exactly that — a flat re-stage fails. 60/60 + 12/12. Co-authored-by: wenshao <wenshao@example.com> |
||
|
Shaojin Wen
|
6acfc9a22f
|
feat(autofix): label-driven takeover and release; fix forced-dispatch green no-op (#7165)
* feat(autofix): label-driven takeover and release; fix forced-dispatch green no-op Takeover, exactly as designed: applying autofix/takeover (GitHub triage+ only — the permission gate is GitHub's own) summons the loop onto a PR, human-authored included; removing it releases the PR. The autofix/skip label opts any PR out at every engagement path — the autofix scan, the forced path, and the Fleet Shepherd walk — and wins when both labels are present. Every toggle gets a bilingual ack from the PAT-verified bot identity. The comment-command surface stays CLOSED: the pinned no-comment-commands contract test is untouched. Plumbing: pull_request labeled/unlabeled triggers; label events share the per-PR route group (the event class is triage-gated); the group expression also carries the #7163 payload trust prefilter so the two open PRs converge on the same final text in either merge order; scan candidates become bot PRs ∪ takeover PRs minus skip PRs (deduped); fork label events are logged and dropped (no secrets to even post a rejection). The forced-validation replay immediately caught a latent bug live since #6528: '(.isCrossRepository // true) | not' is false for EVERY input (jq's // treats false as empty), so every workflow_dispatch pr_number=N run — including the shepherd's conflict dispatches — validated to targets=[] and exited GREEN having done nothing. Fixed to '.isCrossRepository == false' (fail-closed on a missing field) with a replay case pinning the trap. Tests: autofix 55/55 (label engage/release pins, candidate-selection replay with skip-wins and fork cases, forced-validation replay across author/takeover/skip/closed/fork/missing-field); shepherd 12/12 (skip-filter replay). * feat(autofix): '@qwen-code /takeover' comment sugar over the takeover label Maintainer-mandated reopening of the comment surface, in the narrowest possible form: '@qwen-code /takeover' applies TAKEOVER_LABEL and '@qwen-code /takeover stop' removes it — nothing else. The label stays the single source of truth: engagement and release happen exclusively via the pull_request label events, so a manual label edit and the command are one mechanism with two entry points, and the command's whole blast radius is one label toggle. Gates: an expression-level startsWith prefilter keeps ordinary comments from ever starting a job; the body must match the constant EXACTLY after trimming (no parsing, no arguments); allowed senders are the PR author (who may lack label access — this is who the sugar is FOR) or a write+ collaborator via the same permission API used by review routing; closed PRs, non-PR comments, and the bot itself are ignored. The toggle job is PAT-verified and presence-aware (no-op toggles are explicit, since they fire no label event). The 'does not expose comment-triggered autofix commands' contract test is REWRITTEN into pinning this gated design, documenting the deliberate reversal. New behavioral replay drives the extracted command branch with a PATH-stubbed permission API across eight author/write/read/ exact-match/non-PR/closed/self cases. 56/56. * feat(autofix): raise the round cap to 50 while a PR is under takeover Large managed PRs routinely need dozens of feedback rounds — that is the point of takeover — so the unattended MAX_ROUNDS=5 would strangle exactly the PRs the label exists for. While TAKEOVER_LABEL is present the effective cap is TAKEOVER_MAX_ROUNDS=50: the circuit breaker stays (a bot/review-bot ping-pong is still bounded and every round still requires new trusted feedback or a conflict), it is just sized for explicitly delegated work. Removing the label restores the strict cap on the next scan. The scan computes the effective cap from the candidate's live labels and stamps it into the matrix target; the address job shadows the workflow-level MAX_ROUNDS with the matrix value, so every round message, marker, and cap gate uses the same number consistently (including #7163's address-time cap discard once both merge). 57/57 with a verbatim cap-selection replay (labeled → 50, plain → 5). * feat(autofix): re-armable round windows, cap raised to 100, visible cap pause The round counter is DERIVED state, stored nowhere but in the bot's eval-marker comments on the PR — and counting is now windowed by the latest '<!-- takeover-ack engaged -->' comment. Re-engaging (label off→on, or repeating the takeover command on an already-managed PR, which now posts a re-arm ack instead of a silent no-op) starts a fresh window: a PR that exhausted its rounds continues under management with one human action, auditable in the PR timeline. The WATERMARK stays global across windows — feedback already addressed is never replayed — and a PR never taken over has no ack, so strict lifetime counting is unchanged. The prepare-side live round is windowed identically, so pre-reset markers can neither trip the cap nor look like same-ts round-advance duplicates (replay-proven). TAKEOVER_MAX_ROUNDS rises to 100 per maintainer sizing, and pausing at the cap is now VISIBLE on managed PRs: a bilingual notice with re-arm guidance, once per counting window (marker-deduped past the latest re-arm; a failed post retries next scan). 58/58: rearm windowing replay (no ack → lifetime; ack → round 0 with watermark preserved; new rounds count from 1; latest ack wins), the stale-gate re-arm interplay case, cap-selection at 100, and cap-notice dedup pins. * feat(autofix): collapsed-Chinese bilingual takeover comments; fix ESLint regex-spaces Every takeover-flow comment — engage ack, release ack, re-arm ack, and the cap-pause notice — now follows the project convention: English body plus Chinese collapsed under <details><summary>中文说明</summary> (pinned at exactly four sites). Bodies are built via printf so no workflow indentation leaks into the markdown: the previous literal multi-line strings embedded 10 leading spaces, which would have rendered the trailing marker comment as a visible code block. Also fixes the CI failure at |
||
|
Shaojin Wen
|
030e335651
|
ci(autofix): harden the address path against stale targets and untrusted route events (#7163)
* ci(autofix): harden the address path against stale targets and untrusted route events Follow-up to #7127 addressing the five Critical findings of its post-merge review (4728499913): - Route group trust prefilter: the per-PR concurrency group is entered before any step runs, so an arbitrary commenter's review could cancel a queued legitimate route and then die in Decide phases. Reviews whose payload does not already look trusted (repo association or the review bot) now get a run-unique group — cancel nothing, still fully authorized inside. Decide phases remains the real permission gate. - Address-time eligibility recheck: a matrix job can start hours after its scan; a PR closed/merged meanwhile (or with changed author/repo/base/branch) is discarded BEFORE the PR branch checkout — no secret-bearing agent run, no push, no comment, no marker. A failed fetch discards too (unknown is not eligible). - Non-stale duplicates adopt the live watermark and round: a sibling may have evaluated F1 through T1 while this job carried watermark W; rendering from W would replay handled feedback to the agent, and reusing the matrix round would double-write a marker round. Both reporters consume the effective round. - Live round cap: when live markers already sit at MAX_ROUNDS the run discards (the scan itself skips capped PRs before conflict checks) — no round MAX+1 work, no second capped marker. - Stale discard suppresses the failure-path handoff: a late always() step failure after stale=true no longer converts a deliberate no-comment/no-marker discard into a handoff that consumes a round. Contract tests 53/53: W/T1/T2 replay proving only-F2 rendering via the adopted watermark, live-round adoption and cap-discard replays, sentinel-ts non-adoption, stale-suppressed handoff replay, and eligibility ordering/coverage pins. * ci(autofix): honor engagement labels in the address-time eligibility gate Forward-compatible with the incoming label-takeover feature so the two changes commute in any merge order: the eligibility recheck now reads labels live — autofix/takeover exempts a human-authored PR from the bot-author requirement, and autofix/skip (which wins over takeover) withdraws consent even if applied while the job sat queued. Both are enforced at the moment the secret-bearing run starts, not at scan time. Inert until the labels exist. * test(autofix): behaviorally replay the eligibility recheck Review suggestion, adopted: the discard path was only string-pinned — a future edit dropping the stale=true echo would leave every toContain green while STALE arrived empty downstream, letting a late always() failure post a spurious handoff for a discarded job. The recheck now runs VERBATIM under a PATH-stubbed gh across nine states: healthy bot PR proceeds writing nothing; closed-while-queued discards AND writes all four outputs later gates read; live takeover label exempts a human author; live skip label withdraws consent even for the bot's own PR; fork head, renamed branch, and a failed fetch (unknown is not eligible) all discard. 54/54. * fix(autofix): scan-side skip filter, honest labels comment, infra-distinct discard Review round on the hardening PR (six Suggestions, all adopted): - The scan candidate list now excludes skip-labeled PRs: the address-gate discard writes no marker, so an unfiltered scan would re-emit a skip-labeled PR into a full address job (checkout, npm ci, build) every tick forever. - The engagement-labels env comment no longer overpromises: in THIS change the labels are honored at the eligibility gate and scan filter; the scan-side widening that makes takeover summon human PRs ships with the takeover feature PR (the two commute either way). - A failed eligibility fetch now discards with an infra-distinct message (metadata fetch failed (API error) — fail-closed) instead of masquerading as state='unknown', mirroring the scan-side wording. - Tests: the non-main-base discard is now exercised behaviorally (not just pinned); the fetch-failure case asserts its distinct message; the full parenthesized route trust expression is pinned as ONE string (Actions binds && tighter than ||, so dropped parens would invert the grouping); and the terminal-sentinel adoption guard is exercised on a path that actually reaches the adoption block (live conflict skips the stale gate) instead of passing via the discard. 54/54. --------- Co-authored-by: wenshao <wenshao@example.com> |
||
|
jinye
|
86ad532deb
|
perf(cli): Defer TUI runtime from ACP startup (#7182)
* perf(cli): Defer TUI runtime from ACP startup Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(cli): Preserve state on lazy import failures Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * test(cli): Colocate API error classifier coverage Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> |
||
|
Shaojin Wen
|
1cacbb1bcc
|
ci(shepherd): add Fleet Shepherd — automated unblocking of the bot-PR fleet (#7142)
* ci(shepherd): add Fleet Shepherd — automated unblocking of the bot-PR fleet
A scheduled janitor that applies, per open bot PR, the smallest lever that
unblocks it — each lever was validated by hand across the fleet before being
automated here:
- conflict → dispatch the autofix loop for that PR (its --conflict path
merges base and resolves), deduped per conflicted head SHA via a marker
comment, with a bilingual notice on the PR
- stale base → update-branch when ≥25 commits behind main (fresh CI signal
and propagates workflow/skill fixes to the branch tree; self-limiting
because behind_by resets to zero after the sync), never while checks run
- known-flake red → rerun failed jobs only when EVERY failing test parsed
from the job log matches .github/known-flakes.txt (one unknown failure
blocks the rerun), only after the run fully concluded, max 2 attempts per
run id; the registry seeds the three repeatedly-observed flakes
- scan liveness → if no autofix full scan ran in 60 minutes and none is in
flight, dispatch one (GitHub cron was observed silent for 16h on a */10
schedule)
A single "Fleet Shepherd Dashboard" issue is edited in place each tick for
observability. Safety rails: bot-authored in-repo main-targeting PRs only,
per-tick action caps (3 syncs / 2 dispatches), dry-run dispatch input,
FLEET_SHEPHERD_DISABLED repository-variable kill switch, PAT identity
verified before any write; dispatches/reruns ride the workflow token while
comments/update-branch/dashboard use the bot PAT so synced branches still
trigger CI. Contract test (8 cases) pins triggers, scoping, credential
split, idempotency markers, caps, liveness, dashboard, and validates every
registry line as a compiling test-file regex.
* ci(shepherd): review fixes — gate reruns across ALL failed jobs, behavioral tests
Addresses the /review findings:
- The flake gate now enumerates EVERY failed job of the run via the jobs API
and parses failing tests from all their logs before deciding — matching the
header's "every failing test" promise, since `gh run rerun --failed` reruns
them all. A flaky Ubuntu test can no longer green-light a run that also has
a genuine Windows failure. Failed jobs exposing no parseable failing tests
(e.g. a coverage comment failing downstream of the test job) neither allow
nor block on their own — documented tradeoff. This also removes the URL
job-id parse entirely, mooting the /job-vs-/jobs concern (for the record:
statusCheckRollup detailsUrl is the web URL and uses singular /job/, as the
campaign's live extractions confirmed — but not parsing URLs is better).
- Behavioral tests: the log parser and the flake-gate pipeline are extracted
VERBATIM from the workflow (the test fails if either drifts) and executed
under bash — parser fixture → exact file list; gate matrix all-known→RERUN,
mixed→BLOCK, unknown→BLOCK, empty→SKIP. 10/10.
* ci(shepherd): review round 2 — atomic markers, dashboard lookup fix, hardened plumbing
Two criticals and the standing suggestions from review:
- act() now propagates the wrapped command's real exit status (still set -e
safe at the if-wrapped call sites), and every dedup marker is posted ONLY
when its primary action succeeded — a transient dispatch/rerun failure no
longer plants a marker that freezes the PR at that head or burns phantom
rerun attempts.
- Dashboard lookup used `--jq --arg`, which gh does not support (single
expression only) — DASH_NUM was always empty, creating a new dashboard
issue every tick. Fixed to an exact-title search with a plain expression.
- Scan-liveness signal now counts SCHEDULE runs plus the shepherd's own
liveness dispatches (recorded as a watermark in the dashboard body), so a
conflict dispatch — also a workflow_dispatch — can no longer satisfy the
watchdog and silently starve full scans; one run-list call now feeds both
the age and in-flight computations.
- Fleet metadata comes from ONE gh pr list call (no N+1 pr view loop);
actions reads (run state, jobs, logs) ride the workflow token whose
actions scope is guaranteed; update-branch passes expected_head_sha as a
compare-and-swap against races with the loop's own pushes; PENDING now
counts WAITING/REQUESTED; RUN_ID extraction is pipefail-guarded; the
registry filter is a trichotomy — rc≥2 (invalid pattern) BLOCKS reruns
instead of falling through to the rerun branch.
- Behavioral replay extended: invalid-ERE registry → ERRBLOCK (fail-safe
direction proven under bash). 10/10.
* ci(shepherd): review round 3 — cede rerun ownership to CI Failure Patrol; consumer-parity gates
The decisive review finding (raised independently by two reviewers): the
flake-rerun lever created a SECOND scheduled owner for the same failed CI
runs, unserialized against the existing Qwen CI Failure Patrol
(qwen-ci-flaky-rerun.yml) — exactly the rerun-vs-rerun race class observed
live during the fleet campaign. Single-owner principle applied: the lever,
its registry (.github/known-flakes.txt), and its tests are removed; the
shepherd now only REPORTS red CI on the dashboard, and the header documents
the non-goal. This also moots the sibling findings against the lever's
internals (stale first-FAILURE selection, unparseable-job neutrality, missing
fleet-wide rerun budget).
Remaining findings fixed:
- Conflict dispatch now mirrors its consumer's predicate: the autofix scan
skips PRs with checks in flight, so the shepherd defers the dispatch (and
its dedup marker) until checks are quiet instead of wasting the dispatch
and freezing the head behind a marker for a scan that never ran.
- A failed marker read now SKIPS the PR for the tick — an empty comment
history must never masquerade as "no markers" and double-fire actions.
- New behavioral test: act() is extracted verbatim from the workflow and run
under bash, proving a failing primary action returns nonzero (marker
skipped) and a succeeding one returns zero (marker posted). 9/9.
* ci(shepherd): honest budget accounting on marker failure; dry-run behavioral proof
Review round 4 (two suggestions):
- A successful dispatch now counts against MAX_DISPATCHES_PER_TICK the moment
it happens, with the marker posted afterwards; if the marker post fails the
note says so honestly and the possible next-tick re-dispatch is absorbed by
downstream dedup (the autofix scan's busy-PR skip and nothing-new gate).
The reviewer's ordering — counting only after a successful marker — would
let a marker outage overspend the per-tick dispatch budget.
- The act() behavioral replay now also proves the dry-run branch: with
DRY_RUN=true and `false` as the primary command, act() returns 0 without
executing it (DRY-OK), so an inverted condition or dropped return can no
longer pass on string presence alone. 9/9.
* ci(shepherd): scope-named conflict cap, wider liveness window, no empty-fleet overwrite
Review body-level confirms addressed:
- MAX_DISPATCHES_PER_TICK renamed to MAX_CONFLICT_DISPATCHES_PER_TICK: the
cap budgets conflict dispatches only, BY DESIGN — the liveness dispatch is
separately self-limited (dashboard watermark + 60-minute age + in-flight
gate ⇒ at most one per tick and usually one per hour), and sharing one
budget would let a liveness fire starve conflict handling. The name now
states the scope instead of inviting the bypass reading.
- Liveness run-list window widened to 50 so a review-event storm can't push
schedule runs or an in-flight scan out of view; the residual worst case
(one unnecessary dispatch) stays bounded by the watermark and the autofix
scan's own busy/nothing-new gates.
- A failed fleet enumeration now skips the walk AND the dashboard update —
the previous dashboard body is preserved instead of being overwritten with
a misleading empty table. 9/9.
* test(shepherd): rename to qwen-fleet-shepherd-workflow.test.js per convention
Every other workflow contract test in scripts/tests/ carries the -workflow
suffix; match it so workflow-test globs include this file.
* ci(shepherd): if-wrap every act(), exact-title dashboard, drop vestigial checkout
Review round 7:
- act() propagates real exit codes, so under the runner's set -eo pipefail a
BARE failing call aborts the whole tick — the reviewer's sharpened framing
of the marker-call issue (that call was already if-wrapped in the previous
round; the residue was the two dashboard writes). Both are now if-wrapped
with retry-next-tick warnings, and the test asserts no bare act call
remains anywhere.
- Dashboard lookup: exact-title equality via standalone jq --arg (in:title is
a substring search — a bystander issue merely containing the title must
never be hijacked and overwritten), and a FAILED lookup now skips the
dashboard for the tick instead of minting a duplicate via create-on-failure
(same fail-closed rule as the fleet and marker reads).
- Removed the vestigial checkout step: with the rerun lever ceded to the
Patrol, the run step reads no repo files — every read goes through the API.
10/10.
* ci(shepherd): fail-closed run snapshot, liveness-scoped in-flight, shepherd-own busy-set
Review round: three Criticals against the run-snapshot section.
- A failed autofix run-list read sets SCAN_RUNS_OK=false and skips the
liveness lever AND every conflict dispatch for the tick, instead of
falling back to '[]' — an empty snapshot both zeroed the in-flight
count and blanked the schedule signal, so the watchdog could stack a
duplicate scan on top of a live one.
- SCAN_INFLIGHT counts only SCHEDULE runs plus workflow_dispatch runs
whose createdAt sits within 180s of our recorded liveness watermark.
Forced conflict dispatches can hold a run in progress for up to two
hours; counting them starved the watchdog and froze full scans exactly
when the fleet was busiest.
- The shepherd builds its OWN busy-set from live runs' review-address
matrix jobs and defers a conflict dispatch while one is live or
queued (unknown snapshot => every dispatch deferred). The
marker-failure retry note claimed 'downstream dedup' that lives in an
unmerged PR; the dedup now travels with this workflow.
Contract tests: 11/11, including a verbatim-extracted jq replay proving
a foreign forced dispatch (created far from the liveness watermark)
neither satisfies nor starves the watchdog.
* ci(shepherd): declare set -eo pipefail explicitly
The act() contract, every if-wrapper, and the behavioral replay all
assume strict-mode semantics. Actions' default bash gives -e (bash -e
{0}) but NOT pipefail — declare both so production matches the tested
contract and a future bare act() call fails loudly instead of silently
posting its marker. The one real pipeline in the tick (the liveness
watermark grep) already carries || true.
* ci(shepherd): platform-blind CI-red on the dashboard; document first-tick edge
Review notes (non-blocking, adopted): the dashboard's ci-red state
matched only 'Test (ubuntu' — a Windows- or macOS-only regression
stayed invisible on the health view. Widen to any Test platform
(reruns stay with the Patrol either way) and pin it. Also record the
first-tick liveness edge at the in-flight computation: with no
watermark yet a manual dispatch in flight is not counted and one
duplicate scan may go out; the scan's own busy/nothing-new gates
absorb it, and every later tick has the watermark.
* ci(shepherd): unknown watermark and partial busy-set fail closed
Review round: two remote-read failure paths still turned unknown state
into actionable empty state.
- The liveness watermark lives in the dashboard body, so a failed body
read (not just a failed lookup) now flips DASH_LOOKUP_OK: the
dashboard is not overwritten (that would destroy the stored
watermark) and the liveness lever is skipped — otherwise, with cron
already stale, a PAT outage would dispatch a duplicate full scan
every tick for its duration. The lever now requires known watermark
state, a good run snapshot, expired age, and zero in-flight.
- Every busy-set jobs read is tracked: BUSY_OK inherits SCAN_RUNS_OK
and flips false on the first failed gh run view or jq parse, because
a partial enumeration is unknown busy-state, not a smaller busy-set
(a gh failure inside process substitution never trips the parent
shell). Conflict dispatches now gate on BUSY_OK.
New behavioral replay (PATH-stubbed gh over the verbatim-extracted
walk): failed jobs read → BUSY_OK=false with an empty set; successful
read → the queued review-address PR lands in the busy-set; no live
runs → known-empty set with dispatches enabled. 12/12.
* ci(shepherd): fetch markers only for conflicting PRs
Review suggestion, adopted: MY_MARKS was fetched (paginated) for every
PR in the fleet but consumed only by the conflict lever's dedup —
~100 wasted comments fetches per tick on a 50-PR fleet — and a failed
fetch on a NON-conflicting PR dropped the PR from the dashboard,
stale-base sync, and CI-red reporting via the shared continue. The
read now lives inside the CONFLICTING branch behind MARKS_OK: a failed
read defers just the conflict dispatch (empty history still never
masquerades as no markers), and every other lever keeps working.
* ci(shepherd): link the dashboard ci-red state to the failing job
Review suggestion, adopted: FAILED_TEST_URL was extracted per PR and
then only tested for non-emptiness — the dashboard said 'ci red' with
no way to reach the failure, forcing exactly the per-PR navigation the
dashboard exists to eliminate. The state cell is now a markdown link
to the failing Test job.
* ci(shepherd): attribute in-flight liveness by run id, not timestamp proximity
Review round: the ±180s attribution window necessarily miscounted a
conflict dispatch fired later in the SAME tick — created seconds from
the liveness watermark, its two-hour address run then suppressed the
age>=60m dispatch on every later tick, the exact starvation the header
claims to prevent. And the behavioral test only exercised a foreign
dispatch 40 minutes outside the window, masking the reachable case.
The liveness dispatch now records its run id: captured right after the
dispatch (a correlation window in which no other dispatch can exist —
conflict dispatches fire later in the walk), persisted in the
dashboard marker as 'run=<id>', and matched by id equality next tick.
No proximity window exists at all. Unknown id (capture failure or a
pre-id marker) attributes nothing, so the failure mode is one absorbed
duplicate scan — never starvation. The replay now proves the
reviewer's in-window case: a conflict dispatch created 5s after the
watermark counts 0, and our completed run plus that live conflict run
also counts 0. 12/12.
---------
Co-authored-by: wenshao <wenshao@example.com>
|
||
|
Shaojin Wen
|
ee0cc79739
|
ci(autofix): fan out review targets and stop route-scan starvation (#7127)
* ci(autofix): fan out review targets and stop route-scan starvation
Two throughput fixes for the review loop, both observed live:
- review-scan emitted ONE newest-first target per scan ("single-target
worker"). With sparse cron ticks this starves older armed PRs for hours —
an armed PR sat unprocessed for 16h while newer PRs took every tick. Emit
EVERY eligible target instead: the address matrix's max-parallel (3) bounds
simultaneity and the per-PR concurrency groups already prevent duplicate
same-PR runs, so one surviving scan drains the whole backlog.
- route used a single shared concurrency group with cancel-in-progress. Under
runner backlog a route job sits QUEUED for minutes, and any newer event
(review submissions arrive constantly) cancelled it — five consecutive
dispatched scans died this way; during event storms no full scan survived
at all. Cron ticks keep deduping through a shared 'route-cron' group, but
dispatches and review/issue events now get unique per-run groups: route is
a seconds-long job, so never cancelling it costs nothing and every trigger
is guaranteed to route.
Contract test updated: fan-out asserted (no single-target break, matrix
max-parallel), new route concurrency expression pinned. 50/50.
* ci(autofix): cap targets emitted per scan (review defense-in-depth)
Review note on the fan-out: bound the scan's output for a pathological
backlog. Clarifications recorded in-thread — the loop lives in review-scan
(timeout 15m), not route (5m), and the pre-change worst case already walked
the full candidate list (break fired on the first ELIGIBLE PR, not the first
candidate) — but an explicit bound is good hygiene: emit at most
MAX_TARGETS_PER_SCAN (10) targets, LOG the deferral (never a silent cap),
and let the next scan pick up the remainder since their signals persist.
Contract test pins the cap, the deferral log, and the slice.
* ci(autofix): review round 2 — per-target route coalescing, busy-PR skip, in-loop budget
Both criticals and the suggestion from review, each verified against live
campaign observations:
- Route concurrency is now keyed by TARGET: cron ticks still coalesce with
each other; review events coalesce PER PR (near-simultaneous reviews on one
PR route once — the one useful side effect of the old shared group,
restored — without events on other PRs cancelling this one); issue events
coalesce per issue; dispatches stay unique and are never cancelled. This
keeps the starvation fix while closing the duplicate-forced-scan window the
per-run_id grouping had opened.
- The scan now skips any PR whose review-address job is RUNNING OR QUEUED in
a live autofix run (one runs-list plus a jobs-view per live run). A
fanned-out matrix holds queued jobs past a 10-minute tick and
schedule/dispatch runs never surface in the PR's checks, so without this
the next scan re-emitted the same PRs and per-PR groups accumulated
duplicates that later replayed stale watermarks — the exact duplicate-round
behavior observed live on the fleet.
- The per-scan target budget now BREAKS the candidate loop instead of slicing
after it, so it genuinely bounds scan runtime and API usage (each candidate
costs several serial reads); the deferral is logged and the remainder keeps
its signals for the next scan.
Contract test updated for all three (route expression per target, busy-skip
message + capture regex, in-loop budget break). 50/50.
* ci(autofix): discard stale duplicate targets via live-watermark revalidation
Review: the busy-set closes the queued-matrix window but not the pre-matrix
one — two near-simultaneous same-PR triggers can both scan before either has
emitted a matrix job, so both emit the PR with the same stale watermark, and
the per-PR address group QUEUES (not discards) the duplicate.
That queueing is exactly what makes revalidation sound: address jobs for one
PR run strictly one at a time, so when the duplicate reaches prepare, the
first job's eval marker is already posted. Prepare now recomputes the
watermark from LIVE markers; if it advanced past the matrix watermark and
nothing (reviews / inline / issue comments / failed checks) is newer — and
there is no conflict — the run marks itself stale and the address + verify
steps are skipped entirely: no agent run, no marker, no comment, no push.
Contract test pins the revalidation, both step gates, and the now-three
shared address-carve-out sites. 50/50.
* ci(autofix): filter live runs server-side in the busy-set listing
A client-side status filter over the 15 newest runs loses a long-lived
fanned-out run once cron traffic (~6 runs/hr) pushes it past the
window — its queued review-address PRs silently stop looking busy and
the next scan re-emits them with a stale watermark. Query in_progress
and queued server-side instead, so the limit applies to LIVE runs only
(at most a handful) and the window cannot be starved by completed runs.
One status query failing does not hide the other (|| true per query);
an empty set stays fail-open by design — the address-side live-marker
revalidation is the second line of defense.
* ci(autofix): document route-group cases and busy-set fail-open contract
Review notes (non-blocking, adopted): a four-case summary above the
chained route-concurrency ternary for the next reader hitting it in a
blame, and an explicit contract at the busy-set listing — a double
status-query failure is deliberately fail-open because the skip is an
optimization and the address-side live-marker revalidation is the
correctness gate; if that revalidation is ever removed, this read must
become fail-closed.
* ci(autofix): discard conflict-only duplicates; fix dead recount in the stale gate
Review round: the ts-only revalidation missed a conflict-only
duplicate. Two overlapping scans emit the same conflicted PR with
watermark W; the first serialized job resolves the conflict and — with
no newer feedback — its marker keeps ts=W while its round advances.
The second job then sees CONFLICT=false live but LIVE_EVAL_WM == W, so
the strict > gate never fired and the agent re-ran against resolved
work. The gate now also extracts LIVE_MAX_ROUND and treats
same-ts-with-newer-round (conflict cleared) as a duplicate signature,
still subject to the nothing-newer recount.
The behavioral replay the reviewer asked for immediately caught a
latent bug in the previous fix: the recount jq opened with '((' and
never closed it, so it failed to compile, LIVE_NEW stayed empty, and
the whole stale gate was dead code in production. Fixed to a single
paren; the replay now proves five transitions (conflict-only duplicate
discards, first conflict job proceeds, live conflict always proceeds,
ts-advanced duplicate discards, round-advanced-with-new-feedback
proceeds).
---------
Co-authored-by: wenshao <wenshao@example.com>
|
||
|
Shaojin Wen
|
582fb49603
|
feat(web-shell): git status chip, visual working-tree diff, and sidebar git status (#7054)
* feat(web-shell): git status chip, visual working-tree diff, and sidebar git status
Bring working-tree Git awareness to the Web Shell (browser daemon session UI):
- Toolbar branch chip becomes a live status indicator: dirty (staged/unstaged/
untracked), ahead/behind upstream, stash count, detached HEAD, in-progress
operation (merge/rebase/cherry-pick/revert/bisect), and conflict count, each
with a non-color cue.
- Read-only "Changes" dialog: working-tree-vs-HEAD file list with per-file,
line-level, per-side syntax-highlighted diffs; opens via /diff or a dirty
chip; untracked files expand as fully-added and deleted files still diff.
- Per-workspace git status in the sidebar: a compact icon-only chip per trusted
workspace (status dot + hover tooltip); click opens that workspace's dialog.
All git access goes through the daemon REST API with per-workspace trust
gating; new SDK status fields are optional and additive (v2).
* fix(web-shell): themed tooltips and git-chip review follow-ups
Tooltips now render on the themed popover surface (bg-popover /
text-popover-foreground / border + fill-popover arrow) instead of the
inverted bg-foreground default, so they read dark-on-dark rather than a
bright box on the dark theme. Fixing the shared primitive corrects the
git branch tooltip in the composer toolbar and sidebar, plus every other
tooltip, at once.
Also addressing review feedback on the git integration:
- Replace the hand-drawn detached/conflict/stash SVG icons with
lucide-react (CircleDot / TriangleAlert / Layers) per the web-shell
icon convention.
- Gate the tooltip "Working tree clean" message on an enriched status
(computedAt) so a branch-only status no longer asserts clean.
- Include the file path in the diff dialog row aria-label so screen
readers can distinguish files.
- Reset the toolbar git chip on workspace switch so it never shows the
previous repo's branch/counts while the new fetch resolves.
- Log a sidebar git poll failure only on the success->failure transition
to avoid spamming a long-lived tab.
- Correct the SDK doc for DaemonWorkspaceGitDiffFile.added/removed
(0, not undefined, for binary files).
* fix(web-shell): address git-integration review suggestions
Follow-ups from the /review pass on the git integration:
- GitBranchIndicator: include the short SHA in the detached-HEAD tooltip
title, and add the "Working tree clean" status to the aria-label (gated
on an enriched status, matching the tooltip) so the two never drift.
- WorkspaceSection: keep the last known git status on a transient poll
failure instead of blanking the chip for a whole interval.
- App: surface a toast for `/diff` when no workspace is available instead
of silently consuming the composer input.
- Tests: cover the diff dialog's list-load and per-file load error paths,
and detectGitOperation's revert/bisect branches.
- Design doc: align the getGitWorkingTreeStatus spec text with the
decision (transient states return status with `operation`; null is
reserved for non-repo / git failure).
* fix(web-shell): focus-visible ring for git chip button; align doc poll interval
- Add a :focus-visible outline to .gitBranchChipButton so keyboard users
get a visible focus indicator (the chip resets UA button chrome).
- Design doc: align the active-workspace poll-interval references at 30s
to match the implementation.
* fix(web-shell): surface capped diffs, catch row-build failures, cover degradation paths
Address the remaining review findings on the git integration:
- Truncation is no longer silent: fetchGitDiffHunksForFile now returns
{ hunks, truncated } — the parser records files that actually lost
lines to MAX_LINES_PER_FILE (tracked path), and the untracked
synthesis reports its byte/line caps. The route forwards an additive
`truncated` flag on the hunks response (absent when not truncated, so
older clients and daemons are unaffected), and the Changes dialog
renders a "Diff truncated" note under the visible window.
- DiffHunks catches an unexpected buildRows rejection (e.g. malformed
hunk lines) and shows the per-file error instead of leaving an
unhandled rejection and a silently empty diff area.
- New tests: untracked and tracked truncation at the core caps, the
route's truncated passthrough (and its absence when clean), the
branch-only degradation when the working-tree summary throws, the
malformed-hunks error path, and the Shiki success path (a fake
tokenizer proving add rows pull new-side tokens and del rows pull
old-side tokens, not the plain-text fallback).
* fix(web-shell): drop dialog backdrop-blur that froze the page on open
The dialog and alert-dialog overlays applied `backdrop-blur-xs`, which
forces the browser to rasterize and blur the entire content behind the
overlay when a dialog opens. With a long transcript behind it, that
main-thread paint+blur froze the whole page — e.g. clicking the git
branch chip to open the Changes dialog. Keep the bg-black/10 scrim for
separation and drop the blur.
* fix(core): guard synthesizeUntrackedHunk against non-regular files
synthesizeUntrackedHunk opened an untracked path before checking its
type, so an untracked FIFO (listed by `ls-files --others`) would block
on open() forever waiting on a writer — hanging the daemon's event loop
and leaving the Web Shell Changes dialog stuck on a permanent loading
state. lstat-gate on regular files before opening, matching the existing
guard in countUntrackedLines. Adds a FIFO regression test.
* fix(web-shell,core): rename expansion, no-newline marker, chip measurement
Round-5 review Criticals:
- core: key renamed diff entries by the real (post-rename) path and carry
the old path for display, so renamed rows can be expanded — the synthetic
`old => new` key was sent to git as a nonexistent literal path. The diff
dialog renders the rename as `old → new`.
- core: preserve Git's `\ No newline at end of file` marker through the hunk
parser so a trailing-newline-only edit isn't shown as identical
removed/added lines (the viewer already renders it as a meta row).
- web-shell: the toolbar's hidden git-chip measurement replica now renders
the full chip content via the extracted GitBranchChipContent, so the
expanded width includes the status indicators and the compact/expanded
toggle no longer oscillates near the responsive threshold.
* fix(build): generate git-commit info even when prepare build is skipped
The review tooling runs `npm ci` with QWEN_SKIP_PREPARE=1 (to skip the
heavy prepare build) and then builds only the changed workspaces. Because
`prepare` exited before generating the gitignored git-commit.ts, a
per-workspace build of packages/cli failed at the unchanged systemInfo.ts
on the missing `../generated/git-commit.js` module. Generate the git-commit
info in the skip path too — it is cheap and never fails hard — so a later
per-workspace build or typecheck finds the module. The non-skip path still
generates it via `npm run build`.
* fix(web-shell,cli): address round-6 review suggestions
- cli: carry the pre-rename path (oldPath) through DiffRenderRow and show
renamed files as `old → new` in both the Ink and plain-text renderers.
The rename-keying fix updated the daemon and web-shell dialog but not the
CLI `/diff` renderer, which silently dropped the old path.
- web-shell: key DiffFileRow by workspace + path so switching workspace
remounts the row instead of reusing another workspace's hunks/open state
for a path both workspaces share.
- web-shell: show a loading placeholder in DiffHunks while rows are (re)built
(e.g. after a theme switch) instead of an empty, jumpily-resized box.
- web-shell: cover the /diff local intercept in App.test.tsx (opens the
Changes dialog and is not forwarded to the agent).
* fix(web-shell,cli,core): address round-7 review suggestions
- cli: sanitize the rendered filename (and pre-rename oldPath) in the Ink
DiffStatsDisplay via sanitizeFilenameForDisplay, matching the plain-text
renderer so a crafted path can't inject into the interactive view.
- cli: apply the read headers before awaiting the per-file diff fetch (as
handleDiffList does) so error responses also carry no-store/nosniff.
- cli + web-shell: strip Unicode bidi embedding/isolate controls
(U+202A-202E, U+2066-2069) in the filename/control-char sanitizers so a
crafted filename can't visually spoof its extension.
- core: guard countStashEntries with an lstat type check before readFile, so
a symlink-to-FIFO at logs/refs/stash can't block the event loop (the same
hazard already guarded in the untracked-file readers).
- core: cover fetchGitDiffHunksForFile's transient-state guard with a test
(the sibling helpers already had one).
* fix(web-shell,cli,core): address round-8 review suggestions
- core: pass --no-optional-locks to the ls-files call in
fetchGitDiffHunksForFile, matching the other runGit calls so it doesn't
contend for an optional index-refresh lock alongside concurrent git
add/commit.
- cli: add a route test asserting a rename's oldPath survives serialization
end-to-end (keyed by the new path, old path carried alongside).
- web-shell: add a GitDiffDialog test for the hiddenCount>0 "N more files
not shown" note (every payload previously used hiddenCount: 0).
- web-shell: drop the nonexistent primaryLabel prop from the WorkspaceSection
test (it is not a WorkspaceSectionProps member).
- docs: correct the plan doc — large-diff virtual scrolling was explicitly
descoped (core caps + per-file lazy loading), not implemented in Phase 2.
* fix(cli,web-shell): address round-9 review findings
- cli: propagate the pre-rename oldPath through DiffDialog's
perFileToUnified and render renamed files as `old → new` in the
interactive diff viewer (the rename-keying fix had updated the daemon,
the web-shell dialog, and the /diff stats, but not this viewer).
- cli: cover DiffStatsDisplay's rename (`old → new`) rendering and the
sanitizeFilenameForDisplay path for hostile filenames carrying control
characters.
- web-shell: guard the GitBranchIndicator test afterEach against
double-unmounting an already-unmounted root (the localization tests
assert on getTranslator without calling render()).
* fix(core,cli,web-shell): rename-aware single-file diff (old→new)
fetchGitDiffHunksForFile pathspec-limited the diff to the new path, which
defeats git's rename detection — a renamed file was reported as fully
added (every line +) instead of its actual edit. Thread an optional
pre-rename path through the single-file endpoint (core → route → SDK →
dialog) and diff old→new with -M when it is present, so expanding a
renamed file shows its real content change.
* fix(cli): address round-10 review suggestions
- DiffDialog: split the path-width budget between old and new paths for a
rename (reserving the " → " separator) so the combined width stays within
maxPathChars instead of overflowing the row layout.
- textUtils: extend MULTILINE_CONTROL_CHARS_REGEX with the Unicode bidi
ranges (matching FILENAME_CONTROL_CHARS_REGEX) and add a test that
sanitizeFilenameForDisplay strips bidi embedding/isolate controls.
- workspace-git-diff route: add a test that ?oldPath= is parsed and
forwarded to fetchGitDiffHunksForFile.
* test(sdk),docs: cover diff client methods; align design doc
- sdk: add DaemonClient unit tests for workspaceGitDiff() and
workspaceGitDiffFile(path, oldPath?) — URL construction (incl. urlEncode
on path/oldPath, with and without oldPath, plus the workspace-qualified
route) and response deserialization, mirroring the existing workspaceGit()
test.
- docs: add the oldPath? param to the workspaceGitDiffFile API spec; record
that the diff client methods now have unit tests (correcting the claim
that workspaceGit() had none); attribute the bundle-limit bump to
packages/sdk-typescript/scripts/build.js; clarify ahead/behind are relative
to upstream (0, and ↑N/↓N not shown, without one).
* fix(web-shell,core): address round-11 review suggestions
- GitBranchIndicator: count conflicted entries as dirty — a merge where every
changed file is conflicted (staged=unstaged=untracked=0) is still
uncommitted, so the expanded chip's dirty dot / data-dirty now reflect it.
- core: split the status branch line at the last "..." (the branch/upstream
separator) so a dotted branch name isn't truncated at the first "...".
- GitDiffDialog: guard DiffFileRow's in-flight fetch against unmount via a
cancelled ref, matching DiffHunks / GitDiffDialog.
- tests: forward oldPath when expanding a renamed file in the web-shell
dialog; bidi-strip coverage for the web-shell sanitizeControlChars;
untrusted-guard coverage on the single-file diff route; conflicted-only
dirty; branch-line "..." split.
* fix(web-shell,cli): address round-12 review suggestions
- DiffDialog: only render the rename "old → new" when there's room for both
sides (≥19 cols, so each gets ≥8); otherwise fall back to the new path
alone, so a narrow terminal no longer overflows the row (the Math.max(8,…)
floor could exceed maxPathChars).
- GitBranchIndicator test: guard afterEach container.remove() for non-render
tests run in isolation, and make the compact-mode ↑-suppression assertion
non-vacuous by giving the fixture an ahead count.
- App: compute the active workspace once (useMemo) and share it between the
git-status effect and the Changes-dialog entry point, so the chip and the
dialog can't drift onto different repos.
* fix(core,docs): address round-13 review suggestions
- core: add a rebase-apply detection test (git am / an interrupted
`rebase --apply` creates rebase-apply, which detectGitOperation also maps
to 'rebase'); previously only rebase-merge was exercised.
- docs: correct section 5 to describe the actual diff-dialog mechanism
(diffWorkspaceCwd state, not the stale activePanel design).
* test(core): cover stray no-newline marker before any hunk header
parseGitDiff's pre-hunk guard already skips a "\ No newline at end of
file" marker that appears before any @@ header, so a malformed/truncated
diff can't throw on a null currentHunk and lose subsequent files' hunks;
add a regression test pinning that behavior.
* fix(web-shell): unstick per-file diff loading and skip non-path git poll
- DiffFileRow: reset the cancelled-fetch flag on mount so StrictMode's
mount/unmount/mount replay no longer leaves it latched at true, which
dropped the fetched hunks and froze the row on "Loading changes…" despite
a 200 response.
- WorkspaceSection: skip the git status poll when the workspace cwd is not an
absolute path. A synthetic fallback workspace carries a display name there,
which the cwd-qualified route rejects with a 400.
* fix(web-shell,cli): address review suggestions on the git diff surface
- GitDiffDialog: highlight each diff side independently so a small side
keeps syntax highlighting even when the other side exceeds the size cap
(the old guard dropped both as soon as either was too large).
- ChatEditor: complete the .gitBranchChipButton reset (font/color/padding/
margin) so the clickable dirty-tree chip matches the read-only output chip
instead of picking up UA button styling.
- DiffDialog: cover the interactive rename display (old to new on a wide
terminal), mirroring the rename tests DiffStatsDisplay and GitDiffDialog
already have.
* test(web-shell,cli): cover git chip clean/reload/traversal paths, fix doc
- GitDiffDialog: add the missing expect(header).not.toBeNull() guard to the
three expand-file tests that lacked it, matching the others in the block.
- GitBranchIndicator: cover the known-clean aria-label branch (computedAt set
and every change counter zero).
- WorkspaceSection: verify a reloadToken change re-fetches git status instead
of waiting for the next 60s poll.
- workspace-git-diff route: verify a traversal oldPath is forwarded to core
and surfaced as available:false rather than escaping the workspace.
- Design doc: /diff is handled via setDiffWorkspaceCwd, not setActivePanel.
* fix(core): allow literal `..foo` paths in diff normalization
- toRepoRelativePath: reject only a real climb-out (`..` or `../…`), not a
literal `..foo` filename at the repo root, which the bare startsWith('..')
over-rejected, leaving the diff viewer unable to render such a file.
- parseGitDiff: cover the truncatedPaths output set directly (it was only
exercised indirectly through fetchGitDiffHunksForFile).
---------
Co-authored-by: wenshao <wenshao@example.com>
Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
|