mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-08 16:25:10 +00:00
eca654f365
* 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>
73 lines
3.2 KiB
Bash
Executable file
73 lines
3.2 KiB
Bash
Executable file
#!/usr/bin/env bash
|
|
# Owning-workspace resolver, shared by the qwen-autofix verify steps
|
|
# (.github/workflows/qwen-autofix.yml) so the two gates cannot drift apart.
|
|
#
|
|
# Reads changed file paths on stdin (one per line, e.g. the output of
|
|
# `git diff --name-only`) and emits, sorted and unique on stdout, the OWNING
|
|
# npm workspace of each: the workspace whose location is the LONGEST matching
|
|
# path prefix of the file.
|
|
#
|
|
# The workspace set is expanded from the ON-DISK root package.json `workspaces`
|
|
# globs, NOT from `npm query`/node_modules: node_modules reflects the BASE
|
|
# checkout the gate installed, so a workspace the PR branch ADDS (a new channel
|
|
# adapter, a new sdk — the issue-fix job's whole purpose) would be invisible and
|
|
# its tests silently skipped. It is also NOT "any ancestor dir with a
|
|
# package.json": a fixture/example package inside a workspace's src tree (e.g.
|
|
# packages/cli/src/commands/extensions/examples/starter) has a package.json but
|
|
# is not a workspace, so resolving a change there to the fixture would skip
|
|
# packages/cli's own tests. Expanding the globs (shallow `dir/*` + literals,
|
|
# honouring `!` negations, keeping dirs that contain a package.json) matches
|
|
# what `npm run --workspace` accepts downstream and reflects the branch.
|
|
#
|
|
# Invoked with the repository as the working directory. Staged to RUNNER_TEMP
|
|
# from the trusted base checkout (never the PR branch) alongside
|
|
# check-settings-schema.sh.
|
|
set -euo pipefail
|
|
|
|
workspaces="$(node -e '
|
|
const fs = require("fs");
|
|
const path = require("path");
|
|
const pkg = JSON.parse(fs.readFileSync("package.json", "utf8"));
|
|
let globs = pkg.workspaces || [];
|
|
if (!Array.isArray(globs)) globs = globs.packages || [];
|
|
const positive = [];
|
|
const negative = [];
|
|
for (const g of globs) (g[0] === "!" ? negative : positive).push(g.replace(/^!/, ""));
|
|
const hasManifest = (d) => {
|
|
try { return fs.statSync(path.join(d, "package.json")).isFile(); }
|
|
catch { return false; }
|
|
};
|
|
const expand = (g) => {
|
|
const star = g.indexOf("*");
|
|
if (star === -1) return [g];
|
|
const parent = g.slice(0, star).replace(/\/$/, "");
|
|
let entries = [];
|
|
try { entries = fs.readdirSync(parent, { withFileTypes: true }); }
|
|
catch { return []; }
|
|
return entries.filter((e) => e.isDirectory()).map((e) => path.posix.join(parent, e.name));
|
|
};
|
|
const dirs = new Set();
|
|
for (const g of positive) for (const d of expand(g)) if (hasManifest(d)) dirs.add(d);
|
|
for (const g of negative) { for (const d of expand(g)) dirs.delete(d); dirs.delete(g); }
|
|
process.stdout.write([...dirs].sort().join("\n"));
|
|
')"
|
|
|
|
if [[ -z "${workspaces}" ]]; then
|
|
echo "resolve-owning-packages: no workspaces resolved from package.json" >&2
|
|
exit 1
|
|
fi
|
|
|
|
while IFS= read -r f || [[ -n "${f}" ]]; do
|
|
[[ -n "${f}" ]] || continue
|
|
best=''
|
|
while IFS= read -r w; do
|
|
[[ -n "${w}" ]] || continue
|
|
if [[ "${f}" == "${w}"/* && "${#w}" -gt "${#best}" ]]; then
|
|
best="${w}"
|
|
fi
|
|
done <<< "${workspaces}"
|
|
# `if`, not `[[ ]] && printf`: an unmatched file (best empty) must leave the
|
|
# loop body's exit status 0, or under `set -o pipefail` a no-match on the LAST
|
|
# line makes `while … | sort` fail and (with `set -e`) aborts the script.
|
|
if [[ -n "${best}" ]]; then printf '%s\n' "${best}"; fi
|
|
done | sort -u
|