mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-01 12:24:51 +00:00
5a35435df1
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
* ci(autofix): teach the review loop about generated-artifact gates and stop silent stalls
The autofix bot stalled on PRs that edit settingsSchema.ts without
regenerating settings.schema.json (e.g. #6984): a CI-only freshness gate
it could neither run, see, nor recover from.
- Give the agent the tool and the instruction to regenerate: add
`npm run generate:settings-schema` to the develop-issue and address-review
coreTools allowlists, and a SKILL rule to regenerate + commit a source's
generated artifact.
- Mirror CI's "Check settings schema is up-to-date" step in both verify
gates so a stale artifact fails locally instead of red-on-CI after push.
- Inject the actual failing STEP name + a log excerpt into feedback.md so
the agent diagnoses from the real failure instead of guessing from local
test runs. SKILL now forbids "pre-existing"/environment excuses without
evidence.
- Decouple the feedback watermark from base-sync pushes: use the last eval
marker (what the agent evaluated), not the head commit date, so an
"Update branch" merge can no longer bury unaddressed maintainer feedback.
Use PR createdAt as the pre-first-eval floor.
- Bound the pending-check skip so a check wedged pending can't strand a PR
forever; always post a handoff comment + eval marker on failure so the
loop never goes silent; add an issue_comment trigger so an @-mention from
a trusted maintainer re-triggers the review pass promptly.
* ci(autofix): drop comment-trigger and raw-log injection; keep them within existing safety guards
Respects two deliberate, tested design decisions that the first cut collided
with (caught by scripts/tests/qwen-autofix-workflow.test.js):
- Drop the issue_comment @-mention trigger and its route branch: the workflow
intentionally does not expose comment-triggered autofix (only
pull_request_review:submitted) to avoid redundant runs and comment-command
surface. The scheduled scan plus the watermark fix already re-target a PR
after maintainer feedback.
- Drop the raw CI-log injection into feedback.md: feedback fed to the model is
deliberately sanitized and must not pull in URLs / raw context (a
prompt-injection surface). Keep only the sanitized check-name rendering
(.name // .workflowName, still gsub+truncated).
Update the assertions that the retained improvements (watermark decoupling,
pending-check staleness bound, always-post-handoff-on-failure) legitimately
changed. Workflow test: 48/48 green.
* ci(autofix): address review — handle cancellation, fold a PR fetch, cover schema commands
Addresses the three inline /review suggestions on the PR:
- Handoff on any non-success end, not just "failure". A 120-minute job-timeout
cancellation sets job.status = "cancelled", which the "== failure" check
missed — leaving no marker and no comment, so the next scan re-targeted the
same feedback with the same round (an invisible loop). Use "!= success";
the step only runs on failure()/cancelled()/dry-run and dry-run is excluded.
- Fold the createdAt fetch into the existing statusCheckRollup gh pr view call
(statusCheckRollup,createdAt), removing one GitHub API round-trip per PR
scanned.
- Add test coverage the earlier diff lacked: assert generate:settings-schema is
in both agent allowlists and that both verify gates run the schema-freshness
check, so a future edit can't silently drop the guard this PR adds.
* ci(autofix): fix handoff/staleness design flaws from review (4 critical + 2 suggestions)
Addresses the CHANGES_REQUESTED review of the handoff (E-4) and pending-check
staleness (E-3) logic:
- Suppress the handoff once a run published a result (OUTCOME fixed/noop), so a
later always() step failing the job (e.g. artifact upload) can no longer post
a contradictory acted=false handoff over a reported success.
- Bound the agent step at 80m, well under the 120m job timeout, so a runaway
agent fails the STEP (not the job) and the always() report step still runs and
hands off — a job-level timeout would cancel that step too and go silent.
- On a pre-prepare crash (empty NEWEST) the watermark can't advance, so write a
terminal marker (round = MAX_ROUNDS) and skip on the highest marker round
(not last-by-ts), so the scan stops re-handing-off instead of repeating until
MAX_ROUNDS.
- Raise the pending-staleness bound from 30m to 240m so an active check
(review-pr ~50m, review-address up to 120m) is never aged out mid-flight and
the same feedback double-processed; only truly-dead checks are ignored.
- Prefer the agent's detailed failure.md over the generic handoff.md wrapper.
Adds a bash-replay test that extracts the actual POST_HANDOFF decision and
MARK_ROUND logic from the workflow and exercises the state transitions
(published+late-failure, dry-run, verify failure, pre-verify crash,
cancellation; terminal vs incremental round). Workflow test: 49/49.
* ci(autofix): address review nits — symmetric schema-gate outcome, softer SKILL wording
Two non-blocking review suggestions:
- The issue-phase verify gate's schema-freshness check now writes
outcome=failed before exit 1, matching the address-review gate, so the
issue-phase step summary shows outcome=failed instead of outcome=unknown.
- Reword the SKILL rule from "do not invent environment excuses" to "do not
skip a failing check by attributing it to the environment without evidence,"
which keeps the intent (no hand-waving a real failure as an env issue)
without discouraging the agent from reporting a genuine infra failure.
* ci(autofix): close review edge cases — structural schema gate, immutable floor, robust handoff
Review round 3 (2 critical + 3 suggestions):
- Run the review gate's settings-schema freshness check BEFORE the no-op/
unchanged return, so a stale-schema PR the agent wrongly no-ops fails
(outcome=failed) instead of being reported as evaluated while CI stays red —
the exact motivating bug. Single check now covers every path; ordering is
asserted in the test.
- Never fall back to the mutable head commit date for the pre-first-eval
watermark floor: if the PR metadata query fails, use an empty (over-inclusive,
never-buries) floor. A base-sync HEAD as the floor would recreate the burial
bug. Removes the now-unused HEAD_SHA lookup.
- Guard the terminal handoff marker's timestamp (MARK_TS=${NEWEST:-${WATERMARK:-unknown}})
so a cascading API failure that blanks WATERMARK can't emit an unparseable
`ts=` that the scan regex skips, defeating the terminal-round guard.
- Truncate failure.md through `iconv -f utf-8 -t utf-8 -c` so a byte-level
head -c can't split a multi-byte sequence and corrupt the comment body.
- A pre-prepare crash (empty NEWEST) now says "could not start evaluation"
instead of "round 5/5", which would imply MAX_ROUNDS attempts were made.
Workflow test: 49/49.
* test(autofix): assert regression-catching invariants flagged in review
Four review suggestions, test-only: assert the else-branch floor
(EFF_WM=${CREATED_WM}, not the old PUSH_WM), the staleness jq filter
(.startedAt // ... // $cut), the JOB_STATUS env declaration (else it is always
empty → over-eager handoffs), and the .name // .workflowName feedback format —
so a regression on any of these is caught rather than passing silently.
* ci(autofix): fix iconv silent-abort, fold branch fetch, tighten staleness filter
Review round with a real regression in my own round-3 UTF-8 fix:
- CRITICAL: `iconv -c` exits 1 whenever it discards a byte split by `head -c`,
and under the step's `set -eo pipefail` that aborts before the eval marker +
gh pr comment run — a silent stall, the exact failure this block prevents.
Add `|| true`; the cleaned text is already emitted, so the handoff continues.
- Fold headRefName into the PR_META fetch (headRefName,statusCheckRollup,
createdAt) and derive BRANCH from it — one fewer API call per scanned PR.
- Simplify the pending-staleness clock to `.startedAt // $cut`: statusCheckRollup
has no updatedAt and pending checks have no completedAt, so those fallbacks
were dead and contradicted the comment. Now a check blocks only if it actually
started within the bound; comment matches the code.
- Tests: assert `.startedAt // $cut) > $cut` (the comparison, not just the
constant) and the `|| true` guard, so a flipped comparison or a dropped guard
is caught. 49/49.
* ci(autofix): review round — skip empty branch, hoist staleness vars, robust sentinel
Six review suggestions (no criticals):
- Skip a candidate PR when the metadata fetch fails (empty branch) instead of
falling through to an address job that fails on `git checkout -B "" origin/`
and posts a misleading handoff. This also means CREATED_WM is only reached
with populated metadata (subsumes the empty-floor warning suggestion).
- Hoist the invariant PENDING_STALE_MIN / PENDING_CUTOFF out of the per-PR loop
(one `date` fork instead of one per candidate).
- Replace the MARK_TS "unknown" sentinel with a far-future ISO-8601 date, so it
is non-empty AND sorts above real timestamps without relying on an
undocumented lexicographic quirk of a bare word.
- Cross-reference comment on the positional eval-marker regex noting it must
stay in lockstep with every write site (ts= acted= round=).
- Tests: document OUTCOME="" + JOB_STATUS=success → no handoff, and assert the
empty-branch skip guard.
* ci(autofix): DRY schema check via --check, widen handoff detail, honest terminal recovery
Three review suggestions:
- Replace both duplicated schema-freshness blocks with the generator's in-process
`--check` mode (verified: exit 1 when stale, 0 when fresh; no disk write, so
the review gate's later no-op git-diff is unaffected). Single source of truth
with CI; a future change to the check lives in one place.
- Widen the handoff DETAIL_FILE search to address-summary.md/no-action.md: when
the agent succeeds but a post-agent verify gate fails (e.g. the schema gate),
OUTCOME=failed with only the success outputs present, and "Push and report" is
skipped — so this was posting a false "crashed or timed out" and dropping the
agent's real summary.
- Correct the terminal-crash headline: the marker makes the scan skip forever
(even forced dispatch), so "re-trigger if transient" was misleading; the
headline now states the real recovery — delete the terminal autofix-eval
marker comment, then re-trigger. (Keeps the terminal design a prior review
asked for; only the advertised recovery is fixed.)
* ci(autofix): revert schema gate off --check (removed from main by #7031); jq replay
- CRITICAL: `--check` was reverted from main's generator by #7031 (3d4601489),
after this branch's base. Since this PR doesn't touch the generator, the
merged code would run main's argument-ignoring generator and both freshness
gates would go fail-open — the exact stale-artifact stall this PR prevents.
Revert both gates to regenerate + `git status --porcelain` (restoring the file
on failure), which mirrors CI's actual "Check settings schema is up-to-date"
step and works with any generator version. Verified no --check on current
origin/main.
- Add a behavioral jq replay of the pending-staleness filter (started-before vs
started-after cutoff, and no-startedAt) so a flipped comparison is caught, not
just string-matched.
* ci(autofix): set outcome=failed explicitly if the schema generator crashes
Both verify gates ran the generator unguarded: if it crashes (e.g. a type error
the agent introduced in the schema source), set -eo pipefail aborts the step
before outcome=failed is written, leaving OUTCOME unset (the handoff still fires
via job.status, but the outcome is inferred rather than explicit). Wrap the
generator in `if ! ...; then outcome=failed; exit 1; fi` so the failure is
explicit and does not depend on the job.status fallback. Test asserts the guard.
* ci(autofix): extract the settings-schema gate into a shared script
Address review: the 16-line schema-freshness gate (generator crash
guard, porcelain check, restore, outcome=failed) was duplicated verbatim
between the issue-fix verify step and the triage-and-address verify
step, so an edit to either copy could silently diverge from the other.
Move it to .github/scripts/check-settings-schema.sh and call it from
both sites; the site-specific rationale comments stay at the call sites,
the shared mechanics and the crash-guard rationale live in the script.
The script preserves the exact step contract, verified with a PATH-shim
harness over a temp git repo: generator ok + fresh schema exits 0 with
no output written; a generator crash writes outcome=failed to
GITHUB_OUTPUT and exits 1; a stale schema prints the diff, restores the
file, writes outcome=failed, and exits 1.
* test(autofix): update verify-gate assertions for the extracted schema-check script
The main-merge extracted the inline schema-freshness block into the shared
.github/scripts/check-settings-schema.sh (both verify gates now invoke it), which
broke the test that asserted the inline generate/porcelain strings in the step.
Assert the step invokes the script and that the SCRIPT holds the logic
(regenerate, crash guard, git status --porcelain, no --check, outcome=failed),
and that the review gate's script call precedes the no-op return.
---------
Co-authored-by: wenshao <wenshao@example.com>
40 lines
1.6 KiB
Bash
Executable file
40 lines
1.6 KiB
Bash
Executable file
#!/usr/bin/env bash
|
|
# Settings-schema freshness gate, shared by the qwen-autofix verify steps
|
|
# (.github/workflows/qwen-autofix.yml) so the two gates cannot drift apart.
|
|
#
|
|
# Mirrors CI's "Check settings schema is up-to-date" step EXACTLY: regenerate,
|
|
# then fail if the committed artifact changed. Uses regenerate +
|
|
# `git status --porcelain` (NOT the generator's --check, which was reverted
|
|
# from main by #7031 — after merge this runs against main's generator, which
|
|
# ignores args and would make --check fail-open). Stale schemas are invisible
|
|
# to build/typecheck/lint/vitest.
|
|
#
|
|
# On failure: prints the diff, restores the schema file, writes
|
|
# `outcome=failed` to $GITHUB_OUTPUT (when set, matching the calling step's
|
|
# contract), and exits 1.
|
|
set -uo pipefail
|
|
|
|
SCHEMA_FILE='packages/vscode-ide-companion/schemas/settings.schema.json'
|
|
|
|
fail() {
|
|
if [[ -n "${GITHUB_OUTPUT:-}" ]]; then
|
|
echo "outcome=failed" >> "${GITHUB_OUTPUT}"
|
|
fi
|
|
exit 1
|
|
}
|
|
|
|
# Guard the generator itself: if it CRASHES (e.g. a type error the agent
|
|
# introduced in the schema source), a caller running under set -eo pipefail
|
|
# would abort before outcome=failed is written, leaving OUTCOME unset. Handle
|
|
# it here so the failure is explicit, not inferred from job.status.
|
|
if ! npm run generate:settings-schema; then
|
|
echo "❌ Settings schema generator failed to run."
|
|
fail
|
|
fi
|
|
|
|
if [[ -n "$(git status --porcelain "${SCHEMA_FILE}")" ]]; then
|
|
echo "❌ ${SCHEMA_FILE} is out of date. Run: npm run generate:settings-schema"
|
|
git --no-pager diff -- "${SCHEMA_FILE}" || true
|
|
git checkout -- "${SCHEMA_FILE}" || true
|
|
fail
|
|
fi
|