From fdcf02f20d62153efa4c6424066adff989f2a45f Mon Sep 17 00:00:00 2001 From: matheus1lva <831308+matheus1lva@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:33:03 -0300 Subject: [PATCH 1/3] feat: add option to skip tests and lint on workflow --- skills/review-pr-workflow/SKILL.md | 11 +++++++---- .../scripts/check-workflow.mjs | 19 +++++++++++++++++++ skills/review-pr-workflow/workflow.js | 13 +++++++++++-- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/skills/review-pr-workflow/SKILL.md b/skills/review-pr-workflow/SKILL.md index 839c66b..7a7b9e5 100644 --- a/skills/review-pr-workflow/SKILL.md +++ b/skills/review-pr-workflow/SKILL.md @@ -18,13 +18,14 @@ Same scope as `review-pr`: **web/frontend projects** (React, TypeScript, Next.js ## Arguments ``` -/review-pr-workflow [verify-agent=claude|codex] [tier=auto|full|light|skip] +/review-pr-workflow [verify-agent=claude|codex] [tier=auto|full|light|skip] [run-checks=true|false] ``` | Arg | Default | Meaning | |-----|---------|---------| | `verify-agent` | `claude` | Which agent verifies findings in Phase 2. `claude` uses the session model. `codex` shells out to the `codex` CLI. | | `tier` | `auto` | How much horsepower to spend. `auto` detects from the diff (see Tier Detection). Anything else overrides detection. | +| `run-checks` | `true` | Phase 0 lint and tests. Omitted or `true` runs them. Only explicit `false` skips — then neither the main loop nor any workflow agent runs lint, tests, or typecheck. | Example: `/review-pr-workflow https://github.com/yearn/kong/pull/412 verify-agent=codex tier=full` @@ -52,7 +53,7 @@ Do all of this yourself. Subagents share one working directory; if they check ou 2. **Read the PR body for instructions** — author's review notes, linked issues (`Closes #123`, `Fixes #456`, URLs). 3. **Fetch linked issues** — read each issue body for the original spec. The review is graded against this, not against the PR description. 4. **Checkout the PR branch locally.** Do this once, here. Every workflow agent is read-only from this point. -5. **Run project linters** — `bun run lint`, `npm run lint`, whatever the project defines. Capture the output; it goes into the workflow as context so five agents don't each re-run it. +5. **Run project linters and tests** unless the user passed `run-checks=false`. Omitted means run. Lint: `bun run lint`, `npm run lint`. Tests: `bun test`, `npm test`, whatever the project defines. Capture both; they go into the workflow as `lintOutput` and `testOutput` so agents don't each re-run them. Pass `runChecks` as a real boolean (`true`/`false`). If skipped, do not run lint, tests, or typecheck; set both outputs to `(skipped — run-checks=false)` — never leave them empty; empty is interpolated as `(clean)`. 6. **Detect new dependencies** — if `package.json` changed, list newly added packages. These feed the dependency lens. 7. **Detect the tier** (below) and **state it out loud with its reason** before spawning anything. @@ -127,10 +128,12 @@ Pass these as real JSON values, never a JSON-encoded string. | `baseRef` | no | Base to diff against, e.g. `origin/main`. Defaults to `origin/HEAD`; pass it explicitly when the PR targets anything else. | | `diffStat` | no | Output of `gh pr diff --stat` | | `changedFiles` | no | Array of paths | -| `lintOutput` | no | Phase 0's lint result, so five agents don't each re-run it | +| `lintOutput` | no | Phase 0's lint result, so five agents don't each re-run it. On `run-checks=false`, the skip sentinel — not empty. | +| `testOutput` | no | Phase 0's test result. Same skip-sentinel rule as `lintOutput`. | | `newDeps` | no | Newly added package names | | `tier` | no | `full` or `light`. `skip` throws — run `review-pr` inline instead. | | `verifyAgent` | no | `claude` (default) or `codex` | +| `runChecks` | no | Real boolean. Default `true`. `false` forbids lint/test/typecheck in every agent prompt. | ### returns @@ -186,7 +189,7 @@ Agent count scales with findings, not diff size. A PR yielding 2 blockers and 5 4. **Preview the review for the user.** Output the full review as plain markdown text in the conversation. Do not skip this. Do not substitute a tool-call preview. - Then, in **no more than four lines** outside the review body, report counts only: tier and why, `verify-agent`, `stats.refuted`, `stats.discarded`, `stats.advisories`, any duplicate collapse from step 2, and `stats.unverified`. Counts, not contents — do not summarise a refuted claim. Two exceptions to the line budget — if `gaps` is non-empty, list each gap in one line so the user can decide whether to re-run a lens; and if `stats.unverified` is non-zero, list those findings explicitly and say plainly that the review is not exhaustive; raising `MAX_VERIFY_PER_LENS` or re-running that lens is the fix. + Then, in **no more than four lines** outside the review body, report counts only: tier and why, `verify-agent`, `run-checks`, `stats.refuted`, `stats.discarded`, `stats.advisories`, any duplicate collapse from step 2, and `stats.unverified`. Counts, not contents — do not summarise a refuted claim. Two exceptions to the line budget — if `gaps` is non-empty, list each gap in one line so the user can decide whether to re-run a lens; and if `stats.unverified` is non-zero, list those findings explicitly and say plainly that the review is not exhaustive; raising `MAX_VERIFY_PER_LENS` or re-running that lens is the fix. 5. **Post only after explicit approval.** Do not call any GitHub write tool before the user approves this specific review. A prior approval, a plan that mentioned posting, or this skill's own existence does not count. diff --git a/skills/review-pr-workflow/scripts/check-workflow.mjs b/skills/review-pr-workflow/scripts/check-workflow.mjs index e91ece2..160395e 100644 --- a/skills/review-pr-workflow/scripts/check-workflow.mjs +++ b/skills/review-pr-workflow/scripts/check-workflow.mjs @@ -37,14 +37,17 @@ const FAKE = { const isAdv = f => Boolean(f.advisory) let agentCalls = 0 let labels = [] +let prompts = [] function makeEnv({ refuteAll = false } = {}) { agentCalls = 0 labels = [] + prompts = [] async function agent(prompt, o = {}) { agentCalls++ labels.push(o.label) + prompts.push({ label: o.label, prompt }) if (o.label?.startsWith('review:')) return { findings: FAKE[o.label.split(':')[1]] ?? [] } if (o.label?.includes('verify:')) { const refuted = refuteAll || o.label.includes('bugs3') @@ -170,5 +173,21 @@ threw = null try { await run({}) } catch (e) { threw = e.message } check('[edge] missing args.pr is rejected', /args\.pr is required/.test(threw || ''), threw) +const specPrompt = (runArgs) => run(runArgs).then(() => + prompts.find(p => p.label === 'review:spec')?.prompt || '') + +const defaultPrompt = await specPrompt({ ...BASE, tier: 'light' }) +check('[checks] omitted runChecks still forbids re-running lint/test', + /already run — do not re-run lint or tests/.test(defaultPrompt) && + /Test output, already run/.test(defaultPrompt), + defaultPrompt.slice(0, 400)) + +const skippedPrompt = await specPrompt({ ...BASE, tier: 'light', runChecks: false }) +check('[checks] runChecks=false forbids lint/test/typecheck and does not claim they ran', + /run-checks=false/.test(skippedPrompt) && + /Do not run lint, tests, typecheck/.test(skippedPrompt) && + !/already run/.test(skippedPrompt), + skippedPrompt.slice(0, 400)) + console.log(fails ? `\n${fails} failing` : '\nall checks passed') process.exit(fails ? 1 : 0) diff --git a/skills/review-pr-workflow/workflow.js b/skills/review-pr-workflow/workflow.js index f925552..0490c9e 100644 --- a/skills/review-pr-workflow/workflow.js +++ b/skills/review-pr-workflow/workflow.js @@ -37,11 +37,21 @@ const { diffStat = '', changedFiles = [], lintOutput = '', + testOutput = '', newDeps = [], tier = 'full', verifyAgent = 'claude', + runChecks = true, } = input +const CHECKS = runChecks + ? `Lint output, already run — do not re-run lint or tests: +${lintOutput || '(clean)'} + +Test output, already run — do not re-run it: +${testOutput || '(clean)'}` + : `run-checks=false. Do not run lint, tests, typecheck, or any project script. Review the diff only.` + // Verify at most this many findings per lens, highest severity first. Bounds the // agent count; the selection is deterministic so resumes hit cache. const MAX_VERIFY_PER_LENS = 4 @@ -72,8 +82,7 @@ ${changedFiles.join('\n')} Diffstat: ${diffStat} -Lint output, already run — do not re-run it: -${lintOutput || '(clean)'} +${CHECKS} The PR branch is already checked out. You are READ-ONLY: do not checkout, commit, stash, start a dev server, or modify any file. From 67deec48aae9c15b851b9914e0acaa118bd4fa6a Mon Sep 17 00:00:00 2001 From: matheus1lva <831308+matheus1lva@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:30:51 -0300 Subject: [PATCH 2/3] fix(review-pr-workflow): stop claiming tests passed when none ran Empty lintOutput/testOutput interpolated as "(clean)" under the default runChecks=true, telling every review and verify agent a suite had run and was clean. Report each stream as "not run" when no output is supplied, and cover the empty case in the check script. Addresses https://github.com/yearn/webops-skills/pull/24#pullrequestreview-3304019519 --- skills/review-pr-workflow/SKILL.md | 6 +++--- .../scripts/check-workflow.mjs | 19 ++++++++++++++----- skills/review-pr-workflow/workflow.js | 14 ++++++++++---- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/skills/review-pr-workflow/SKILL.md b/skills/review-pr-workflow/SKILL.md index 7a7b9e5..b9555a1 100644 --- a/skills/review-pr-workflow/SKILL.md +++ b/skills/review-pr-workflow/SKILL.md @@ -53,7 +53,7 @@ Do all of this yourself. Subagents share one working directory; if they check ou 2. **Read the PR body for instructions** — author's review notes, linked issues (`Closes #123`, `Fixes #456`, URLs). 3. **Fetch linked issues** — read each issue body for the original spec. The review is graded against this, not against the PR description. 4. **Checkout the PR branch locally.** Do this once, here. Every workflow agent is read-only from this point. -5. **Run project linters and tests** unless the user passed `run-checks=false`. Omitted means run. Lint: `bun run lint`, `npm run lint`. Tests: `bun test`, `npm test`, whatever the project defines. Capture both; they go into the workflow as `lintOutput` and `testOutput` so agents don't each re-run them. Pass `runChecks` as a real boolean (`true`/`false`). If skipped, do not run lint, tests, or typecheck; set both outputs to `(skipped — run-checks=false)` — never leave them empty; empty is interpolated as `(clean)`. +5. **Run project linters and tests** unless the user passed `run-checks=false`. Omitted means run. Lint: `bun run lint`, `npm run lint`. Tests: `bun test`, `npm test`, whatever the project defines. Capture both; they go into the workflow as `lintOutput` and `testOutput` so agents don't each re-run them. Pass `runChecks` as a real boolean (`true`/`false`). If skipped, or if the project defines no such script, leave the output empty — the workflow then tells every agent the check was not run instead of claiming it passed. 6. **Detect new dependencies** — if `package.json` changed, list newly added packages. These feed the dependency lens. 7. **Detect the tier** (below) and **state it out loud with its reason** before spawning anything. @@ -128,8 +128,8 @@ Pass these as real JSON values, never a JSON-encoded string. | `baseRef` | no | Base to diff against, e.g. `origin/main`. Defaults to `origin/HEAD`; pass it explicitly when the PR targets anything else. | | `diffStat` | no | Output of `gh pr diff --stat` | | `changedFiles` | no | Array of paths | -| `lintOutput` | no | Phase 0's lint result, so five agents don't each re-run it. On `run-checks=false`, the skip sentinel — not empty. | -| `testOutput` | no | Phase 0's test result. Same skip-sentinel rule as `lintOutput`. | +| `lintOutput` | no | Phase 0's lint result, so five agents don't each re-run it. Empty when lint did not run — agents are told so. | +| `testOutput` | no | Phase 0's test result. Empty when tests did not run — agents are told so, never that they passed. | | `newDeps` | no | Newly added package names | | `tier` | no | `full` or `light`. `skip` throws — run `review-pr` inline instead. | | `verifyAgent` | no | `claude` (default) or `codex` | diff --git a/skills/review-pr-workflow/scripts/check-workflow.mjs b/skills/review-pr-workflow/scripts/check-workflow.mjs index 160395e..aec04f5 100644 --- a/skills/review-pr-workflow/scripts/check-workflow.mjs +++ b/skills/review-pr-workflow/scripts/check-workflow.mjs @@ -176,11 +176,20 @@ check('[edge] missing args.pr is rejected', /args\.pr is required/.test(threw || const specPrompt = (runArgs) => run(runArgs).then(() => prompts.find(p => p.label === 'review:spec')?.prompt || '') -const defaultPrompt = await specPrompt({ ...BASE, tier: 'light' }) -check('[checks] omitted runChecks still forbids re-running lint/test', - /already run — do not re-run lint or tests/.test(defaultPrompt) && - /Test output, already run/.test(defaultPrompt), - defaultPrompt.slice(0, 400)) +const emptyPrompt = await specPrompt({ ...BASE, tier: 'light' }) +check('[checks] no output supplied claims nothing ran, and still forbids running', + !/already run/.test(emptyPrompt) && + /Lint output: not run/.test(emptyPrompt) && + /Test output: not run/.test(emptyPrompt) && + /do not assume it passed/.test(emptyPrompt), + emptyPrompt.slice(0, 400)) + +const ranPrompt = await specPrompt({ ...BASE, tier: 'light', lintOutput: 'lint clean', testOutput: '46/46 passing' }) +check('[checks] supplied output is reported as already run', + /Lint output, already run/.test(ranPrompt) && + /Test output, already run/.test(ranPrompt) && + /46\/46 passing/.test(ranPrompt), + ranPrompt.slice(0, 400)) const skippedPrompt = await specPrompt({ ...BASE, tier: 'light', runChecks: false }) check('[checks] runChecks=false forbids lint/test/typecheck and does not claim they ran', diff --git a/skills/review-pr-workflow/workflow.js b/skills/review-pr-workflow/workflow.js index 0490c9e..8c417a8 100644 --- a/skills/review-pr-workflow/workflow.js +++ b/skills/review-pr-workflow/workflow.js @@ -44,12 +44,18 @@ const { runChecks = true, } = input +// Only claim a check ran when its output is actually here. An empty output used to +// interpolate as "(clean)", telling every agent a suite passed that may never have run. +const checkLine = (label, out) => + out + ? `${label}, already run — do not re-run it: +${out}` + : `${label}: not run. Do not run it, and do not assume it passed.` + const CHECKS = runChecks - ? `Lint output, already run — do not re-run lint or tests: -${lintOutput || '(clean)'} + ? `${checkLine('Lint output', lintOutput)} -Test output, already run — do not re-run it: -${testOutput || '(clean)'}` +${checkLine('Test output', testOutput)}` : `run-checks=false. Do not run lint, tests, typecheck, or any project script. Review the diff only.` // Verify at most this many findings per lens, highest severity first. Bounds the From 6e749adea4b050c95017413d74ace1fa6ba94005 Mon Sep 17 00:00:00 2001 From: matheus1lva <831308+matheus1lva@users.noreply.github.com> Date: Fri, 11 Sep 2026 09:31:37 -0300 Subject: [PATCH 3/3] fix(review-pr-workflow-batch): forward run-checks to each PR session The runner forwarded only tier and verify-agent, so run-checks=false on /review-pr-workflow-batch or /triage-reviews was silently dropped and every session still ran lint and tests. Adds REVIEW_RUN_CHECKS, threads it into the per-PR prompt, and documents the flag in both skills. Addresses https://github.com/yearn/webops-skills/pull/24#pullrequestreview-5175368936 --- skills/review-pr-workflow-batch/SKILL.md | 6 +++--- skills/review-pr-workflow-batch/scripts/run-batch.sh | 7 +++++-- skills/triage-reviews/SKILL.md | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/skills/review-pr-workflow-batch/SKILL.md b/skills/review-pr-workflow-batch/SKILL.md index 565630d..b5edc24 100644 --- a/skills/review-pr-workflow-batch/SKILL.md +++ b/skills/review-pr-workflow-batch/SKILL.md @@ -14,12 +14,12 @@ For a single PR, run `/review-pr-workflow` directly. This skill is a batch drive ## Arguments ``` -/review-pr-workflow-batch , , ... [tier=auto|full|light] [verify-agent=claude|codex] [model=] +/review-pr-workflow-batch , , ... [tier=auto|full|light] [verify-agent=claude|codex] [run-checks=true|false] [model=] ``` `` accepts `https://github.com/OWNER/REPO/pull/N`, `OWNER/REPO#N`, or `OWNER/REPO/N`. Separate with commas or whitespace. PRs may span different repos. -Trailing flags apply to every PR in the batch and are passed straight through to `/review-pr-workflow`; `tier=auto` lets each session run its own tier detection, which is almost always what you want. +Trailing flags apply to every PR in the batch and are passed straight through to `/review-pr-workflow`; `tier=auto` lets each session run its own tier detection, which is almost always what you want. `run-checks=false` skips Phase 0 lint and tests in every session — use it when CI already covers them. ## What it guarantees @@ -54,7 +54,7 @@ Trailing flags apply to every PR in the batch and are passed straight through to Launch the driver in the background — a full-tier review runs well past any foreground command timeout: ``` -INVOCATION_DIR="$PWD" REVIEW_TIER= REVIEW_VERIFY_AGENT= \ +INVOCATION_DIR="$PWD" REVIEW_TIER= REVIEW_VERIFY_AGENT= REVIEW_RUN_CHECKS= \ scripts/run-batch.sh "$RUN_DIR" ``` diff --git a/skills/review-pr-workflow-batch/scripts/run-batch.sh b/skills/review-pr-workflow-batch/scripts/run-batch.sh index 839044a..9e60502 100755 --- a/skills/review-pr-workflow-batch/scripts/run-batch.sh +++ b/skills/review-pr-workflow-batch/scripts/run-batch.sh @@ -7,6 +7,8 @@ # REVIEW_TIER pass-through tier for /review-pr-workflow (default: auto). # A manifest entry with a "tier" field overrides it for that PR. # REVIEW_VERIFY_AGENT claude | codex (default: claude) +# REVIEW_RUN_CHECKS true | false (default: true). false tells every session to skip +# Phase 0 lint and tests — the CI-covered batch case. # REVIEW_MODEL optional --model override for the review sessions # SESSION_CWD worktree (default) | invocation | # INVOCATION_DIR where the batch was invoked from (default: $PWD) @@ -19,6 +21,7 @@ RESULTS="$RUN_DIR/results.jsonl" TIER="${REVIEW_TIER:-auto}" VERIFY_AGENT="${REVIEW_VERIFY_AGENT:-claude}" +RUN_CHECKS="${REVIEW_RUN_CHECKS:-true}" SESSION_CWD="${SESSION_CWD:-worktree}" INVOCATION_DIR="${INVOCATION_DIR:-$PWD}" @@ -35,7 +38,7 @@ DENIED='Bash(gh pr review:*),Bash(gh pr comment:*),Bash(gh pr merge:*),Bash(gh p : > "$RESULTS" COUNT=$(jq length "$MANIFEST") -echo "run-batch: $COUNT PR(s), tier=$TIER verify-agent=$VERIFY_AGENT" >&2 +echo "run-batch: $COUNT PR(s), tier=$TIER verify-agent=$VERIFY_AGENT run-checks=$RUN_CHECKS" >&2 for i in $(seq 0 $((COUNT - 1))); do eval "$(jq -r ".[$i] | @sh \" @@ -63,7 +66,7 @@ Run the /review-pr-workflow skill on pull request #$NUM of $SLUG. Title: $TITLE Worktree: $WT Base ref: origin/$BASE_REF - Args: tier=$PR_TIER verify-agent=$VERIFY_AGENT + Args: tier=$PR_TIER verify-agent=$VERIFY_AGENT run-checks=$RUN_CHECKS The worktree above is already checked out at the PR head in detached HEAD, and origin/$BASE_REF is fetched. Do NOT run 'gh pr checkout' or switch branches — diff --git a/skills/triage-reviews/SKILL.md b/skills/triage-reviews/SKILL.md index 1975d48..202111d 100644 --- a/skills/triage-reviews/SKILL.md +++ b/skills/triage-reviews/SKILL.md @@ -10,7 +10,7 @@ Use this skill when: - User asks to pick from their review queue and review the selection Optional org argument narrows the queue, same as `my-reviews` (e.g. `/triage-reviews yearn`). -Trailing `tier=`, `verify-agent=`, `model=` flags pass through to the batch untouched. +Trailing `tier=`, `verify-agent=`, `run-checks=`, `model=` flags pass through to the batch untouched. ## Overview