mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
Shaojin Wen
7f91bc2f24
* feat(verify-pr): configure the assets host and branch once A publish round still had to confirm where the images go, even though the branch already resolved from a flag, a variable, then the repository's assets/pr<n>-validation convention. The host is the last coordinate that had to be repeated per round. settings.json gains verifyPr.assetsRepo and verifyPr.assetsBranch. At startup they fill QWEN_VERIFY_ASSETS_REPO and QWEN_VERIFY_ASSETS_BRANCH when those are unset, so the skill keeps reading the variables it documents and a one-off export still wins. These values choose where a maintainer's evidence is published, so a cloned repository must not be able to set them. Three paths are closed: verifyPr is a workspace-restricted root (the merge strips it, the warnings channel reports it ignored, and the daemon refuses a workspace write, including a nested verifyPr.assetsRepo), and both variables are excluded from what a project .env or settings.env may supply — the environment loader applies those before the settings bridge, so without the exclusion a repository could set the variable itself and the configured value would never win. The skill also states that a published filename is spent: raw.githubusercontent serves a URL from cache, so a republished image needs a new name and the body and comments must move to it. * fix(verify-pr): drop the unreachable guard and rewrap the settings table Review found two mechanical blockers. Prettier rejected the settings table: the two new rows were pasted with padding that did not match the column widths, so Lint & Static failed before "Generate settings schema" ran and the hand-edited schema was never checked. Both rows are rewrapped against the table's own alignment; every other row is byte-identical. The daemon test asserted workspace_restricted_setting for verifyPr.assetsRepo, but the route answers disallowed_key: the allowlist check runs first, and verifyPr is not offered to workspace writes at all. That also made the new root-segment branch unreachable for every restricted root today, so the guard and its test are dropped. The merge strip and the env exclusion are what protect the value. The new shared-env-keys test also no longer separates the TLS trust-anchor comment from the test it introduces. * fix(verify-pr): warn on a rejected settings.env entry and name the branch call Review R1-1: the hardcoded exclusion is key-based, so it also drops a User-scope settings.env entry — a channel that worked before this PR, silently, because reportRejectedSettingsEnv only reports loader-class keys. The rule stays scope-blind: the merged env map cannot tell a workspace entry from an operator one, and only the operator may choose a publishing target. It is no longer silent: the rejection names verifyPr.* and the launch environment, and an environment.test.ts case pins both the unset variable and the warning. Review R1-2: the skill said the branch is "created when absent" while its only upload was a contents PUT, which answers 422 Branch not found for a ref that does not exist. The paragraph now names the GET ref / POST git/refs step (404 only) before the first upload, and the scripts assertion pins it. The design doc and the PR description record the settings.env restriction, so "breaking changes: none" is no longer claimed. * test(cli): isolate the publishing-coordinate variables in environment tests The new warning test asserts the variable stays unset, so the two names belong in TRACKED_ENV rather than depending on the runner's environment. * docs(verify-pr): name the default-branch lookup the branch creation needs Review R2-3: the creation step required "the default branch's head sha" without saying where either comes from, and `default_branch` appeared nowhere in the file. An executor would guess the PR checkout's origin/main, whose sha need not exist in the assets repository — a fork behind upstream then answers 422 Object does not exist and every later upload fails after the round is spent. The paragraph now names both lookups (`.default_branch`, then `.object.sha` of that repository) and the empty-repository 404, matching the in-repo publisher. The scripts assertion pins `.object.sha` so the lookup cannot be dropped again. * fix(cli): fold case in the rejected-coordinate warning and read back from the assets repo Review R2-1: the diagnostic looked the key up with its canonical spelling while the gate folds case, so `qwen_verify_assets_branch` was dropped and never reported — the silent loss this block exists to prevent. The lookup now folds case and names the spelling that was actually written; the environment test runs both spellings, and the lowercase case reds before this change. Review R2-2: the post-upload read-back addressed `<owner>/<repo>`, which this file uses for the PR repository, while the upload went to `<assets-repo>`. With a fork as the image host the confirmation queried the wrong repository: a 404 over a correctly published file, or another blob's size and sha. It now reads back from the repository it wrote to, and the scripts assertion pins the placeholder. * fix(cli): make the coordinate bridge refresh on reload and tighten its typing Four of the deferred review items. The bridge was add-only: a long-lived process kept the value it had written even after the setting changed or disappeared, so the round published to the old host. Writes are now tracked per environment object — a value this function wrote is refreshed on change and removed when the settings drop it, while an exported value is never touched. Three tests pin the three cases. Both leaves used a bare `default: undefined`, so Settings['verifyPr'] inferred as undefined; they now use the `as string | undefined` form the neighbouring optional strings use. The hardcoded project-exclusion list derives its two coordinate names from VERIFY_PR_COORDINATE_ENV, so a third coordinate cannot be gated nowhere. The workspace-restricted root membership is pinned by a test: deleting that entry would let a cloned repository choose the publishing host, and nothing noticed before.