feat(review): drop registry-refutable findings from review output

[coletebou]

Summary

• add opt-in npm registry verification for review findings
• narrow suppression to matching single-package, whole-title-and-reasoning public npm publication claims
• keep all compound, multi-package, private-registry, missing, failed, or ambiguous cases
• add request bounds, response validation, lookup caching, CLI/CI opt-out, docs, and changelog
• remove the unrelated build-fix commit from this PR

Validation

• pnpm test src/registry-verifier.test.ts src/review-validation.test.ts src/app.test.ts
• pnpm typecheck
• pnpm lint
• pnpm format:check
• pnpm build
• autoreview: clean

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed June 9, 2026, 11:24 PM ET / 03:24 UTC.

Summary
The PR adds a default-on npm registry post-validator for review findings, documents and configures a registryVerifier.enabled / --no-registry-verify opt-out, extends dropped-finding metadata and tests, and changes the build script to chmod dist/cli.js.

Reproducibility: yes. for the review findings by source inspection: the default config enables the verifier, ciCommand passes only reviewFlagSubset into reviewCommand, and that subset omits noRegistryVerify. I did not run build or tests to keep the checkout read-only.

Review metrics: 3 noteworthy metrics.

• Patch Size: 12 files changed, +1200/-9. The PR touches review behavior, config, CLI help, docs, tests, and package build behavior, so review needs more than unit-test confidence.
• Default Behavior Changed: 1 new verifier enabled by default. Existing users without new config would get outbound registry checks during review after upgrade.
• Build Script Changed: 1 package script changed. Package build portability is affected by adding a shell-specific chmod command.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Make registry verification opt-in or require an explicit configured registry, with docs/tests for upgrade and restricted-network behavior.
• Forward noRegistryVerify through CI review flag handling and cover the clawpatch ci --no-registry-verify path.
• [P1] Replace shell chmod with a cross-platform Node implementation and add redacted terminal/log proof of real review behavior.

Proof guidance:

• [P1] Needs real behavior proof before merge: No after-fix real behavior proof is present; the PR body states local checks but does not include terminal output, logs, screenshots, recordings, or artifacts showing the verifier and opt-out behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] Default-on public npm requests can disclose private package names or versions from provider findings and can break restricted-network review workflows after upgrade.
• [P1] clawpatch ci --no-registry-verify is accepted and documented but currently dropped before the review step, so CI users may not be able to use the advertised escape hatch.
• [P1] The build-script chmod is POSIX-only and can break Windows builds or package preparation.
• [P1] The contributor has not provided inspectable after-fix real behavior proof; the PR body only states local checks and scan results.

Maintainer options:

1. Make Registry Lookups Opt-In (recommended)
   Default the verifier off, or require an explicit registry setting, so existing reviews do not start public npm requests after upgrade.
2. Accept Default-On Only With Privacy Approval
   Maintainers could intentionally keep default-on verification, but the PR should document the upgrade/privacy tradeoff and prove restricted-network behavior before merge.
3. Repair CI And Build Compatibility
   Forward noRegistryVerify through reviewFlagSubset and replace shell chmod with a Node-based chmod path before merge.

Next step before merge

• [P1] Needs contributor proof and a maintainer decision on the external registry lookup default before automation should repair or merge the branch.

Security
Needs attention: The patch introduces a default-on outbound registry lookup path that can leak private dependency names or versions from review findings.

Review findings

• [P1] Make registry verification opt-in before external lookups — src/config.ts:64
• [P2] Forward the CI opt-out into review — src/cli.ts:186
• [P2] Keep the build script cross-platform — package.json:16

Review details

Best possible solution:

Land a narrower verifier that is explicit opt-in or registry-configured, honors both review and CI opt-outs, preserves cross-platform builds, and includes redacted real review/CI proof.

Do we have a high-confidence way to reproduce the issue?

Yes for the review findings by source inspection: the default config enables the verifier, ciCommand passes only reviewFlagSubset into reviewCommand, and that subset omits noRegistryVerify. I did not run build or tests to keep the checkout read-only.

Is this the best way to solve the issue?

No. The registry-verifier direction is maintainable, but the safer merge path is opt-in or explicitly configured external lookup, a forwarded CI opt-out, and a cross-platform chmod implementation.

Full review comments:

• [P1] Make registry verification opt-in before external lookups — src/config.ts:64
  With registryVerifier.enabled defaulting to true, every existing project without a new config starts running the verifier, and reviewFeature sends extracted pkg@semver values from provider findings to registry.npmjs.org. That can disclose private package names/versions and adds a new network dependency during review, so keep this off by default or require an explicit registry setting before public lookups.
  Confidence: 0.89
• [P2] Forward the CI opt-out into review — src/cli.ts:186
  The PR accepts and documents clawpatch ci --no-registry-verify, but ciCommand builds reviewFlags with reviewFlagSubset, which does not copy noRegistryVerify. In air-gapped CI the advertised opt-out is dropped before reviewCommand, so registry verification stays enabled unless config is changed.
  Confidence: 0.93
• [P2] Keep the build script cross-platform — package.json:16
  Adding chmod +x directly to the npm build script will fail on Windows shells, while the existing cleanup step uses Node. Use a Node chmodSync/postbuild helper so pnpm build remains portable.
  Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 98f51b3daf23.

Label changes

Label changes:

• add P2: This is a normal-priority feature PR with concrete merge blockers, not a live regression already affecting users.
• add merge-risk: 🚨 compatibility: Merging as-is could break restricted-network CI opt-outs and Windows package builds.
• add merge-risk: 🚨 security-boundary: Merging default-on registry verification could send private package names and versions from review findings to the public npm registry.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; the PR body states local checks but does not include terminal output, logs, screenshots, recordings, or artifacts showing the verifier and opt-out behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P2: This is a normal-priority feature PR with concrete merge blockers, not a live regression already affecting users.
• merge-risk: 🚨 security-boundary: Merging default-on registry verification could send private package names and versions from review findings to the public npm registry.
• merge-risk: 🚨 compatibility: Merging as-is could break restricted-network CI opt-outs and Windows package builds.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real behavior proof is present; the PR body states local checks but does not include terminal output, logs, screenshots, recordings, or artifacts showing the verifier and opt-out behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

Security concerns:

• [medium] Default public registry lookup can disclose package metadata — src/config.ts:64
registryVerifier.enabled defaults to true and the review path calls the registry verifier when enabled, so existing projects may send extracted package names and versions from provider findings to registry.npmjs.org without prior opt-in.
  Confidence: 0.89
• [medium] CI opt-out is accepted but not honored — src/cli.ts:186
  The new ci flag is parsed, but the review flag subset omits it, which can leave restricted CI environments making registry requests even after using the documented opt-out.
  Confidence: 0.9

What I checked:

• Repository policy read: AGENTS.md was read fully and applied for TypeScript CLI scope, focused tests, generated-output hygiene, and conservative provider-output review expectations. (AGENTS.md:1, 98f51b3daf23)
• Current main lacks this capability: Current main has no registryVerifier, registry-verifier, or noRegistryVerify implementation, so the PR is not obsolete on main. (98f51b3daf23)
• Verifier is enabled for existing default config: The PR defaults registryVerifier.enabled to true, so projects without an explicit new config setting would start running registry verification after upgrade. (src/config.ts:64, 5b68fdc3fe90)
• Verifier runs in the review path: reviewFeature installs buildRegistryVerifierValidator() when config.registryVerifier.enabled is true, which makes validated findings eligible for outbound registry checks. (src/app.ts:738, 5b68fdc3fe90)
• CI opt-out is parsed but not forwarded: The PR accepts noRegistryVerify for ci, but reviewFlagSubset only forwards provider flags, since, limit, jobs, rateLimitPerMinute, and includeDirty to reviewCommand. (src/cli.ts:186, 5b68fdc3fe90)
• Build script adds POSIX chmod: The PR appends chmod +x dist/cli.js to the npm build script, which is not portable to Windows shells. (package.json:16, 5b68fdc3fe90)

Likely related people:

• Peter Steinberger: Current main blame points the core review/config/CLI files to the v0.5.0 release commit, and Peter also authored the prior ci review-flag forwarding fix in 4518c3a. (role: recent area contributor; confidence: high; commits: d407835d91c7, 4518c3a8e672; files: src/app.ts, src/cli.ts, src/config.ts)
• coletebou: Prior merged history under this handle authored review jobs/rate-limit and validation-drop partitioning changes touching the same review and provider-output paths. (role: prior adjacent contributor; confidence: medium; commits: 0abe1afda4cb, d981d6751f7b, 5f83cd2a5deb; files: src/app.ts, src/cli.ts, src/review-validation.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.