Skip to content

chore(autoreview): use the shared canonical skill - #3897

Merged
vincentkoc merged 3 commits into
mainfrom
chore/autoreview-scanner-free-20261002
Oct 6, 2026
Merged

vincentkoc merged 3 commits into
mainfrom
chore/autoreview-scanner-free-20261002

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

What Problem This Solves

Repository-local autoreview copies drift and can retain retired prerequisites such as TruffleHog.

User Impact

Install autoreview once from openclaw/agent-skills using the setup linked in .agents/skills/autoreview/SKILL.md. Reviews run from this repository through that shared installation. Updating its source checkout updates all symlinked consumers; copy-mode users reinstall once. Product runtime behavior is unchanged.

Why This Change Was Made

Keep only the canonical Markdown entrypoint and remove the copied helper, fixtures, and implementation tests.

Canonical owner and preserved downstream fixes: openclaw/agent-skills#305. Shared changes belong upstream; review runs do not download or update code automatically.

Evidence

  • All 17 consumer entrypoints match the canonical template; each contains no local helper or implementation tests.
  • git diff --check passes; independent integration review checked callers and removed gates.
  • The shared installer and helper dry runs pass from consumer repositories without TruffleHog. Canonical regression tests and review are recorded in the upstream PR.
  • Product runtime suites were not run for this shared-tooling cutover.

Current-head static, unit, package, type/build, HTTP E2E, Playwright smoke, and every local-auth browser shard passed, including the previously failing generated-card test. The optional Vercel preview deployment failed; its build logs were not available through the installed tooling, so no deployment success is claimed.

Landing follow-up

Repository instructions explicitly retain the original P0–P3 review gate with --max-priority P3; direct invocation examples use that override. The shared entrypoint remains identical to the canonical template.

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@vercel

vercel Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
clawhub Error Error Oct 6, 2026 6:53pm UTC

Request Review

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge. Reviewed October 6, 2026, 1:33 PM ET / 17:33 UTC (Revision 22).

ClawSweeper review

What this changes

Replace ClawHub’s copied autoreview helper, fixtures, and tests with instructions for using the shared installation, and document the cutover.

Merge readiness

⛔ Blocked before merge - 3 items remain

The consolidation remains useful and is absent from current main. No blocking code finding emerged, but the previously identified review-threshold choice remains unresolved; MEMBER authorship also prevents automatic closure.

Priority: P3
Reviewed head: 662fe79692b78a29fc8f5cc8550e89fe547bbdc5
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused consolidation has normal good patch quality; the remaining threshold choice does not establish a code defect.
Proof confidence 🌊 off-meta tidepool Not applicable: MEMBER authorship exempts ordinary contributor proof. The body reports consumer installation and helper dry runs; these were not independently executed. No material authority or stored-data contract changes trigger additional proof requirements.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: MEMBER authorship exempts ordinary contributor proof. The body reports consumer installation and helper dry runs; these were not independently executed. No material authority or stored-data contract changes trigger additional proof requirements.
Evidence reviewed 8 items Pinned introduction and delegation boundary: The verified merge-base-to-head delta removes the copied implementation and retains a 23-line entrypoint. Lines 17–19 require running the shared helper from the reviewed repository and retaining local requirements; this is an affirmative dependency on openclaw/agent-skills.
Existing review gate: The original helper validates findings across P0–P3 and exits unsuccessfully when any retained finding exists. Root AGENTS.md requires no accepted/actionable findings before handoff. The introduced entrypoint supplies no explicit severity threshold, and the repository search found no AUTOREVIEW_MAX_PRIORITY setting.
Shared severity contract: The canonical parser defaults AUTOREVIEW_MAX_PRIORITY to P0 and supports an explicit --max-priority P3 override. Its result guidance says lower priorities remain auditable, but exit zero can accompany filtered findings and filtered status is not clean. This confirms the earlier threshold concern without claiming findings disappear entirely.
Findings None None.
Security None None.

How this fits together

Autoreview checks proposed repository changes before contributor handoff. The repository entrypoint supplies local requirements, while the installed shared helper selects Git changes and returns structured findings.

flowchart TD
 A[Repository changes] --> B[Local skill entrypoint]
 C[Repository requirements] --> B
 B --> D[Shared skill installation]
 D --> E[Review and severity filtering]
 E --> F[Findings and handoff result]
Loading

Decision needed

Question Recommendation
Should ClawHub preserve its P0–P3 autoreview handoff gate or intentionally adopt the shared helper’s P0-only default? Preserve the existing range: Record --max-priority P3 in ClawHub’s invocation guidance while retaining the shared implementation.

Why: The cutover changes which findings block handoff, and neither local policy nor discussion explicitly approves that narrower threshold.

Before merge

  • Resolve merge risk (P1) - Without an explicit ClawHub override, shared-helper adoption narrows the accepted finding range from P0–P3 to P0; P1–P3 findings remain auditable but can accompany exit zero.
  • Complete next step (P2) - Approve and record ClawHub’s severity threshold, with an explicit shared-helper invocation that implements that choice.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Diff size +16/-20,429 lines across 12 files Most deletions retire copied review tooling and its tests in favor of a thin shared-installation entrypoint.

Merge-risk options

Maintainer options:

  1. Retain ClawHub’s review threshold (recommended)
    Document an explicit P3 invocation so consolidation preserves the existing accepted finding range.
  2. Accept the narrower gate explicitly
    Approve P0-only acceptance and document the distinction between filtered findings and a clean review.

Technical review

Best possible solution:

Keep one upstream implementation and make ClawHub’s chosen handoff threshold explicit, preferably preserving P0–P3 through --max-priority P3.

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

Not applicable to a product bug: this is tooling consolidation. Source inspection establishes the old and shared severity behavior without executing either helper.

Is this the best way to solve the issue?

Yes, sharing the existing installer and helper is the right consolidation boundary. Keeping copied helpers would preserve drift; an explicit local threshold can preserve ClawHub policy without an implementation fork.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 671cd0a0bdcc.

Labels

Label changes:

No label changes.

Label justifications:

  • P3: This is bounded developer-tooling consolidation with no demonstrated product-runtime outage.
  • merge-risk: 🚨 compatibility: The shared default changes the review handoff contract from accepting P0–P3 findings to accepting P0 only.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: MEMBER authorship exempts ordinary contributor proof. The body reports consumer installation and helper dry runs; these were not independently executed. No material authority or stored-data contract changes trigger additional proof requirements.

Evidence

What I checked:

  • Pinned introduction and delegation boundary: The verified merge-base-to-head delta removes the copied implementation and retains a 23-line entrypoint. Lines 17–19 require running the shared helper from the reviewed repository and retaining local requirements; this is an affirmative dependency on openclaw/agent-skills. (.agents/skills/autoreview/SKILL.md:17, 662fe79692b7)
  • Existing review gate: The original helper validates findings across P0–P3 and exits unsuccessfully when any retained finding exists. Root AGENTS.md requires no accepted/actionable findings before handoff. The introduced entrypoint supplies no explicit severity threshold, and the repository search found no AUTOREVIEW_MAX_PRIORITY setting. (.agents/skills/autoreview/scripts/autoreview:11985, 01839ac5814a)
  • Shared severity contract: The canonical parser defaults AUTOREVIEW_MAX_PRIORITY to P0 and supports an explicit --max-priority P3 override. Its result guidance says lower priorities remain auditable, but exit zero can accompany filtered findings and filtered status is not clean. This confirms the earlier threshold concern without claiming findings disappear entirely. (skills/autoreview/scripts/autoreview:7072, 3d3034af58d4)
  • Existing installation and repository selection: The existing installer supports ~/.agents/skills, symlink and copy modes, and forced replacement. The shared helper discovers its reviewed repository from Path.cwd(), so the proposed working-directory instructions use an existing supported capability. Upstream fix(autoreview): share one installation across repositories agent-skills#305 remains open and owns additional rollout documentation and harness improvements. (skills/autoreview/scripts/autoreview:1995, 3d3034af58d4)
  • Intentional scanner policy: GitHub verifies that fix(autoreview): remove restored TruffleHog dependency agent-skills#240 merged as b86a9121adde5f949135841e92037f9593b7c694, with Patrick-Erichsen recorded as merger. Its explicit purpose accepts removal of pre-transmission TruffleHog screening; canonical documentation keeps approved scanning outside autoreview. This intentional tradeoff is not re-raised as a new defect. (b86a9121adde)
  • Current-main and release necessity: The fetched main revision still describes and contains the bundled helper. The latest GitHub release, v0.23.3, also retains the bundled entrypoint. Neither inspected version implements this shared-installation cutover. (.agents/skills/autoreview/SKILL.md:8, 671cd0a0bdcc)

Likely related people:

  • Patrick-Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Record the approved ClawHub severity threshold and matching shared-helper invocation.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (21 earlier review cycles; latest 8 shown)
  • reviewed 2026-10-05T12:50:41.826Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-05T15:37:58.536Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-05T17:36:41.923Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-05T17:46:58.420Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-06T01:11:53.462Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-06T03:42:17.962Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-06T08:04:12.617Z sha 662fe79 :: blocked before merge. :: none
  • reviewed 2026-10-06T12:47:58.550Z sha 662fe79 :: blocked before merge. :: none

@vincentkoc vincentkoc changed the title fix(autoreview): remove the retired TruffleHog prerequisite chore(autoreview): use the shared canonical skill Oct 2, 2026
@blacksmith-sh

This comment has been minimized.

@clawsweeper clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Oct 2, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review October 6, 2026 18:44
@clawsweeper

clawsweeper Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: chore(autoreview): use the shared canonical skill This is item 1/1 in the current shard. Shard 0/1.

This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@vincentkoc
vincentkoc merged commit a18bc74 into main Oct 6, 2026
30 of 33 checks passed
@vincentkoc
vincentkoc deleted the chore/autoreview-scanner-free-20261002 branch October 6, 2026 18:48

This branch had an error being deployed

1 failed deployment
Preview – clawhub — edd74132 Deployed Oct 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant