Require inferred-vs-verified disclosure in reviews; deny turbo in the review shell - #37
Merged
Conversation
Two additions to the reusable review workflow, both surfaced by reviewers on downstream PRs (isapp/ht-platform#3602, isapp/AIDLC#214). Both are generic — they apply to every repo that consumes this workflow — so they belong here rather than in any single repo's extra_prompt. 1. Inferred-vs-verified disclosure rule in the base prompt. The reviewer runs in a sandbox with no execution, so nearly everything it reports is inferred from reading. The rule requires it to say which part of a finding it could not confirm by reading, instead of presenting the inference as verified. Prose-only by nature; there is nothing to enforce mechanically. 2. Bash(turbo:*) in --disallowedTools. pnpm/npm/npx/yarn were already denied; turbo — the task runner those repos front build, test, lint and migrations with — was not. Placed next to Bash(pnpm:*) and matching the list's existing Bash(...)-patterns-only constraint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gobetti
approved these changes
Aug 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Two additions to
.github/workflows/claude-review.yml, the reusable workflow every repo's review job consumes. Both were raised by reviewers on downstream PRs; both are generic, so they belong upstream here where all repos get them, rather than in one repo'sextra_prompt(@gobetti's structural point on isapp/ht-platform#3602 — that PR was closed as redundant because its 22 lines duplicated this base prompt).Neither addition touches AIDLC's canonical review body: per isapp/AIDLC#214 a parity test asserts that surface stays byte-identical to ht-platform's
extra_prompt, and an environment-constraints rule and a--disallowedToolsentry live in neither.1. Inferred-vs-verified disclosure rule (
## Environment constraints)The reviewer runs in a sandbox that cannot execute anything, so nearly everything it reports is inferred. It matters more here than for a local review, where a human can just run the thing. @jdrorrer's framing: this is the right shape of rule for a layer that has none, and it needs no technical enforcement to be worth having.
Verified absent before this change:
grep -ciE 'inferred|verified|distinguish'over the whole file returns0. The nearest existing text — "If something genuinely requires a blocked tool, note it once in the final comment instead of retrying" — is about blocked tools, not about marking inference.Scoped deliberately to findings the reviewer is already writing up. A draft also said "'I could not verify X' is a legitimate thing to report", covering the case where it clears code it could not check. I dropped that: it cuts against the existing Behavior rule "if you initially find an issue but then determine no fix is needed, do not comment on it", and would push comment volume up. Easy to add back if reviewers want the clearing direction covered too.
2.
Bash(turbo:*)in--disallowedToolsVerified gap:
Bash(npm:*),Bash(npx:*),Bash(yarn:*)andBash(pnpm:*)were all denied;turbowas not. Placed next toBash(pnpm:*)— it's the JS-monorepo runner that pairs with them — and it is aBash(...)pattern, never an MCP tool name, per the constraint in the comment above the list.Is
turboeven reachable from this job? Not today, as far as I can verify — so this entry is defense-in-depth, not a closed hole:reviewjob runs exactly three steps (harden-runner,actions/checkout,claude-code-action) onubuntu-latest. Nosetup-node, no dependency install, and installs are themselves denied.turbois a devDependency (package.json,"turbo": "^2.9.17") resolved fromnode_modules/.bin/turbo, with a rootturbo.jsonand elevenpackage.jsonscripts fronted byturbo run(build, test, lint, typecheck, codegen, thedb:*family).turbois therefore not onPATHin the job. I did not inspect the GitHub-hosted runner image; I'm reasoning from "no install step ran and the binary lives innode_modules". Confirming it would take a run that echoescommand -v turbo.So the value is (a) drift protection — the day a caller adds a setup/install step, or a repo vendors or globally installs turbo, the deny is already there; and (b) it removes the invitation: a repo whose
turbo.jsonand scripts advertiseturbo run lintis inviting the reviewer to spend a turn trying it. That's also why it belongs in the deny list rather than in prose, which @jdrorrer noted "only works if the agent chooses to follow it."Same best-effort caveat the whole list already carries: a prefix pattern on
turbodoes not fence a path-qualifiednode_modules/.bin/turbo, exactly asBash(pnpm:*)does not. The header already frames this list as scope guard plus defense-in-depth, not a boundary.Ticket
None — follow-ups from review comments on isapp/ht-platform#3602 and isapp/AIDLC#214.
Type of Change
Testing Done
There is no test harness in this repo, so verification was static:
promptblock scalar keeps uniform 12-space indentation and the${{ inputs.extra_prompt }}placeholder is intact.claude_argsis still a single folded line; the deny list parses to 43 comma-separated entries, all of the formBash(...), no duplicates, withBash(turbo:*)present.claude_args) describe it categorically — "package managers, build/test runners, and network tools" — and enumerate nothing, so neither goes stale.README.mdandCLAUDE.mddo not mention it.Not exercised by this PR:
claude-review.ymlisworkflow_call-only and this repo has no caller for it, so the changed prompt and deny list do not run against this PR. Consumers reference it at@main(e.g. ht-platform'suses: isapp/.github/.github/workflows/claude-review.yml@main), so merging takes effect on every consumer's next review with no staging step. First real signal is the next downstream PR review.Checklist
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.