mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
main
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Shaojin Wen
|
5d45b42940 |
feat(verify-pr): give the sandboxed round the PR's review threads (#16300)
* feat(verify-pr): give the sandboxed round the PR's review threads The sandboxed /verify round had no token and no review context. Its only inputs were the metadata snapshot and, on a follow-up round, the previous report. On #13652, Critical R1-26 was confirmed and then marked fixed three days before a round that published merge-ready without mentioning it. Eight hours later the review bot, at the head that round had certified, still could not rule out its residual. The verify job's trusted resolve step now writes review-threads.json beside pr.json. It holds every review thread, resolved ones included, since resolving is the author's claim, together with each reviewer's standing CHANGES_REQUESTED review. It comes from one paginated GraphQL query reshaped with jq. The snapshot is best-effort: on failure no file is written and the step warns. verify-pr-prepare.mjs sends the same query and writes the same shape for the local path. It removes a previous copy first, so a failed fetch cannot leave a stale file. A test runs the workflow's block verbatim and prepare's transform over the same pages and requires equal output. The skill adds report item 4, Existing review findings. Each term the verdict turns on is computed from snapshot fields: - a thread is blocking when its first comment is tagged Critical, Blocker, P0 or P1 by an owner, member or collaborator; - a blocking thread is open when it is unresolved, or when that comment belongs to a standing change request, resolved or not. That is the rule GitHub's review decision applies. An open blocking thread left unmeasured rules out merge-ready, and with nothing failed the verdict is inconclusive. Findings at fail 0 would publish as "passed with non-blocking findings". Other blocking threads are re-measured as budget allows, and the verdict line counts those left unmeasured. Change request bodies are listed but do not hold the verdict, because their severities are prose. The duty is also a line in the scope budget. A missing file reads as unknown, never as "nothing open". The file adds 2396 bytes, so the size baseline records the exact 378242. * test(verify-pr): pin the review snapshot's jq guard, copy order and odd answers - The CI/local parity test runs the step's real jq program. Like the file's four other jq cases, it now skips when jq is missing, instead of failing one assertion later with a message that names neither jq nor the missing tool. - The local recipe must clear the cached review-threads.json before it copies prepare's file. The test pinned the three lines but not their order. With rm after cp, every local round would read the review state as unknown. - prepare's two other failure arms are now driven through the CLI: an answer that is not JSON, and pages without a pull request. Each must leave no file and say why. An empty state there would read as "nothing open". * test(verify-pr): drive the review snapshot through the shared gh harness The review-snapshot case had its own copy of the executed-step harness: a temp dir, a stub gh on PATH and GitHub's bash flags. makeGhHarness exists so that fidelity contract lives in one place. The factory now records every call's argv, returns its temp dir, and has an empty-stdout arm for paginated reads, GH_STUB_EMPTY_LIST. The snapshot case runs through it. Its failure arms are the factory's GH_STUB_FAIL_LIST and GH_STUB_EMPTY_LIST. Deleting the shared stub's fail arm, its empty-stdout arm or its argv capture turns the case red, and so does recording only the first argument. |
||
|
Shaojin WenandQwen-Coder
|
68c9fa27fb |
feat(verify-pr): add local Docker recipe with a host-side model proxy (#16077)
* feat(verify-pr): add local Docker recipe with a host-side model proxy A local /verify-pr round needs the same isolation the CI lane provides, but the skill only documented the boundary check, not how to run the model credential through it. Add scripts/verify-pr-model-proxy.mjs, a host-side relay that holds the real key and lets the container call the model with a per-run bearer token, mirroring the CI lane's inline proxy (POST /chat/completions only, nonce health check, per-chunk idle watchdog). The listen address is a flag so the proxy can bind the Docker bridge, and readiness is printed as one JSON line. The SKILL recipe pairs it with a docker run template: the token travels as QWEN_VERIFY_PROXY_TOKEN (not OPENAI_API_KEY, which the isolation gate refuses), and the container's provider entry reads it via envKey. The CI lane keeps its inline copy — the proxy holds the real key, so it must not execute a script from the PR-head checkout. Verified: 11 new pin tests plus all existing verify-pr suites (120 passed), and a real docker smoke — the gate passes inside node:22-bookworm with the proxy token, refuses OPENAI_API_KEY with exit 4, and a container POST reaches a mock upstream with the real key injected and the /v1 prefix preserved. * fix(verify-pr): harden the model proxy after adversarial audit Audit round 1 found the relay forwarded client hop-by-hop headers (connection, transfer-encoding, expect, proxy-authorization, ...) to the upstream, which RFC 7230 forbids and which fights undici's own framing for the relayed stream. Strip them, with a chunked-body test that pins the behaviour. Round 1 also found the CLI entry had no coverage: add tests for the missing-config exit code and the printed JSON coordinates (token override, base-path prefix, no real-key leak, health nonce), and document that the printed baseUrl uses the listen address, so binding 0.0.0.0 yields a URL clients cannot dial. Rounds 2 and 3 (open-ended re-read plus adversarial passes over the updated diff) found nothing further. All verify-pr suites: 123 passed. * fix(verify-pr): address review findings in the model proxy and checker Round-1 review (12 inline findings at |
||
|
Shaojin Wen
|
e3f12f54f9 |
feat(verify-pr): reuse the current tree behind a clean-head prepare gate (#15913)
* feat(verify-pr): reuse the current tree behind a clean-head prepare gate A local round already runs the head side of its A/B in the current tree, but nothing said so and nothing checked it: preparing on the wrong branch silently measured code the report never names, and the round snapshot carried only the head OID, leaving every base reference to re-derive the HEAD^1 warning the skill repeated in three places. Prepare now refuses a HEAD other than the PR head (naming the checkout to run) and any uncommitted change, tracked or untracked, before it writes anything; it never stashes for the maintainer. The status probe pins --untracked-files=normal so a status.showUntrackedFiles=no config cannot hide files from the gate. The round snapshot records the full coordinates (repo, pr, baseRefOid, headRefOid), and the skill consolidates its three HEAD^1 warnings into one rule: local coordinates come from that snapshot, and the clean current tree is the head side, reusing its install and build. The isolation boundary is unchanged: the tree gate runs on the host coordinate path only, and --require-isolation still guards PR-code execution. * fix(verify-pr): make build reuse conditional on head freshness Review reproduced a clean head checkout whose gitignored dist still ran an earlier commit's code while prepare advertised unconditional build reuse. The gate attests source identity only, so the success line now says to verify or refresh the install/build first, the skill makes reuse conditional on establishing the artifacts were produced for this head (otherwise rebuild per the local install strategy), and the design document records the boundary in both languages. The PR body's risk section matches. * fix(verify-pr): close the index-flag hole and make refusals actionable Review found four gaps in the tree gate: skip-worktree / assume-unchanged entries (which sparse checkouts set en masse) hide tracked divergence from git status, so a partial tree passed with the gate's own clean stamp; the dirty-tree remedy looped because a plain git stash leaves untracked files behind; the printed checkout remedy's detached-HEAD round trip was untested; and the fixture re-declared the repository's tmp/ ignore rule instead of pinning it. The gate now refuses any index entry that is not a plain cached H, the dirty refusal names the entries and a stash form that clears them, a detached HEAD at the PR head is pinned to reach exit 0, and a coupling test fails if the real .gitignore stops ignoring the round's own artifacts. Each guard carries a mutation-verified test, and the four-step recipe was driven by hand against a scratch clone. |
||
|
jianyx1
|
f7885f9fa9 | fix(verify-pr): bind reports to their round snapshot (#15746) | ||
|
Shaojin Wen
|
2f3d1b2ff5 |
feat(verify-pr): add a local preflight and a verdict check (#15614)
* feat(verify-pr): add a local preflight and a verdict check A maintainer round resolves the metadata snapshot, the base and head OIDs, the artifact directory name, and the verdict/count agreement by hand. The CI lane supplies all four; a workstation supplies none, and every one of them fails silently rather than loudly — a snapshot from the wrong repository, a parent commit taken for the base, a directory the collector never sees, or counts that contradict the stated verdict. Add a preflight that resolves the coordinates and reports whether this process is isolated, and a checker that applies the publisher's consistency rules to a local artifact directory. Point the skill at both where the flow needs them. * fix(verify-pr): address the local-helper review round The macOS sandbox marker is not an isolation boundary: its default profile still reads host files, so it is reported as a caveat instead of a pass. The coordinate path no longer gates on isolation. It must reach `gh`, which the boundary excludes, so it always runs on the host; gating there fired on every round and made the override the normal path. The check moved to `--require-isolation`, meant to run inside the boundary. A null or array assertions.json is now rejected instead of skipping every count check, the missing-OID hint names the `--repo` repository rather than `origin`, and the untrusted PR title is stripped of control characters before it reaches a terminal. The shared entry-point check moved under scripts/lib, and a workflow meta-test pins the checker's verdict vocabulary to the publisher's case labels. * fix(verify-pr): gate on credential-free readiness, not a container marker The CLI's own Docker/Podman launcher mounts ~/.config/gcloud and the ADC file, copies model API keys it can see, and then sets SANDBOX to the container name. Counting that marker as a passing boundary let a credentialed container through, which is the same class of false pass the Seatbelt fix removed. A marker is now an observation. Passing requires the explicit QWEN_VERIFY_CREDENTIAL_FREE=1 attestation and no known credential signal (cloud, SSH, kube, docker, git and gh variables plus their well-known paths, and SANDBOX_MOUNTS, which can carry anything). An attestation contradicted by a visible signal exits 4. * fix(verify-pr): close the three findings from the verification round The boundary gate's credential list now names ~/.config/gh and ~/.npmrc — the persistent form of the gh token and a registry token, already covered in their environment form. A report now has to cite the verified head when the round snapshot is present, not merely some 40-hex token that a base OID satisfies. An unreadable snapshot is reported instead of silently downgrading the check. A boolean-looking SANDBOX value no longer counts as a container marker. |