diff --git a/.github/workflows/queue-build.yml b/.github/workflows/queue-build.yml index 12919098..60b98b5a 100644 --- a/.github/workflows/queue-build.yml +++ b/.github/workflows/queue-build.yml @@ -48,7 +48,9 @@ jobs: # Build concurrency and group size are both one in main's ruleset. [[ "$QUEUE_REF" =~ ^refs/heads/gh-readonly-queue/main/pr-([0-9]+)- ]] || { echo "::error::unexpected merge group ref" >&2; exit 1; } head_ref=$(gh api "repos/$REPO/pulls/${BASH_REMATCH[1]}" --jq .head.ref) - ./quest-base/bin/quest --root "$GITHUB_WORKSPACE" check --base "$BASE" --branch "$head_ref" + # The base's skills would reject a PR that adds, removes, or changes a skill's stub. The PR's + # own binary checks the stubs against the PR's skills in `just check` below, in this same job. + ./quest-base/bin/quest --root "$GITHUB_WORKSPACE" check --no-stubs --base "$BASE" --branch "$head_ref" - name: Install dependencies run: nix develop --command just install - name: Check diff --git a/README.md b/README.md index 33904db8..197faa53 100644 --- a/README.md +++ b/README.md @@ -280,6 +280,13 @@ detached, so pass the head branch: The check is a floor, not a review: any edit to the quest passes, so reviewers still judge whether a rewrite matches what remains. +`quest check` also compares the repository's skill stubs with the skills its +binary carries, and reports a missing, outdated, or retired one. A job that +checks a PR with a binary built from the base passes `--no-stubs`, since that +binary's skills would reject a PR that changes a stub; the PR's own binary +checks them in its own `quest check`. This repository's merge queue and the +`quest-merge` skill's preflight do so. + A link into a submodule that is not checked out, as in a fresh worktree or `actions/checkout` by default, resolves at the commit its gitlink pins when a local git directory holds that commit, such as the main worktree's. Otherwise diff --git a/assets/skills/merge.md b/assets/skills/merge.md index 3814327c..ee573231 100644 --- a/assets/skills/merge.md +++ b/assets/skills/merge.md @@ -29,8 +29,8 @@ Otherwise, the changes since the reviewed head need no new review only when they The repository's review policy decides whose record counts and may require more, such as a maintainer's review. Where it says nothing, only a record on a PR whose author has write access counts: anyone else writes their own PR body, so their head needs a local review that rewrites the record. -Before queueing, check the head with a `quest` the PR cannot modify: fetch the PR's base branch and its exact head, check the head out apart from your own checkout, such as in a detached worktree under `.tmp/`, and from a directory outside it run `quest --root check --base / --branch `. -The head's files are untrusted, and a stale local base can skip the check. +Before queueing, check the head with a `quest` the PR cannot modify: fetch the PR's base branch and its exact head, check the head out apart from your own checkout, such as in a detached worktree under `.tmp/`, and from a directory outside it run `quest --root check --no-stubs --base / --branch `. +The head's files are untrusted, and a stale local base can skip the check; `--no-stubs` is there because the PR's own checks verify its stubs. A finding means the PR leaves its quest inaccurate: it needs `/quest-iterate`. Queue the head with `quest merge --land `, adding `--squash` or `--rebase` when the repository's root instructions name that method for the PR's base. diff --git a/src/cli/main.ts b/src/cli/main.ts index 90cfef1f..0024edbd 100644 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -171,6 +171,9 @@ export function main(argv: string[], out: Output): number | Promise { "base with , the working tree must delete or edit it; a PR that finishes the quest deletes it, and " + "one that leaves work rewrites it to what remains. Any other branch adds no finding. Fetch " + "first: a stale base can predate the quest and skip it.\n\n" + + "A repository's own check also compares its skill stubs with the skills this binary carries, and " + + "reports a missing, outdated, or retired one. --no-stubs skips that, for a binary built from a base " + + "that checks a PR whose stubs follow the PR's own skills.\n\n" + "With --fix, first drop every Required, Related, or Closes entry that links to a quest deleted in " + "the history of HEAD, and of MERGE_HEAD during a merge, that the working tree lacks, as quest rm " + "does, such as the quests a merged base finished, and every epic's Required entry naming a quest in " + @@ -183,11 +186,12 @@ export function main(argv: string[], out: Output): number | Promise { ) .option("--base ", "Also check the quest branch's diff from its merge base with this ref") .option("--branch ", "The PR's head branch, when it differs from the current branch (needs --base)") + .option("--no-stubs", "Skip the comparison of the repository's skill stubs with this binary's skills") .option( "--fix", "First drop list links to quests the history deleted and epics' links to their own quests, and promote misleveled section headings", ) - .action((options: { base?: string; branch?: string; fix?: boolean }) => { + .action((options: { base?: string; branch?: string; fix?: boolean; stubs: boolean }) => { if (options.branch !== undefined && options.base === undefined) { throw new UsageError("--branch requires --base"); } @@ -201,7 +205,7 @@ export function main(argv: string[], out: Output): number | Promise { const findings = check(tree.docs, tree.exists); if (options.base !== undefined) findings.push(...landed(root(), options.base, options.branch)); // A tree nested in another checkout, such as an example, has no stubs of its own. - const stubs = questRoot(resolve(root())) === resolve(root()) ? stubFindings(root()) : []; + const stubs = options.stubs && questRoot(resolve(root())) === resolve(root()) ? stubFindings(root()) : []; if (findings.length === 0 && stubs.length === 0) { const count = collect(root()).length; print(theme ? theme.checked(count) : `quest: ${count} documents ok`); diff --git a/test/init.test.ts b/test/init.test.ts index 9b90fe96..021644f3 100644 --- a/test/init.test.ts +++ b/test/init.test.ts @@ -506,6 +506,13 @@ test("quest check reports stubs from a relative root", () => { const missing = run(dir); expect(missing.status).toBe(1); expect(missing.stderr).toContain("is missing"); + const skipped = spawnSync( + "bun", + ["run", "--silent", "quest", "--root", relative(process.cwd(), dir), "check", "--no-stubs"], + { encoding: "utf8" }, + ); + expect(skipped.status).toBe(0); + expect(skipped.stderr).toBe(""); init(dir); expect(run(dir).status).toBe(0); });