Skip to content

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

Merged
vincentkoc merged 2 commits into
mainfrom
refactor/shared-autoreview-20261002
Oct 6, 2026
Merged

vincentkoc merged 2 commits into
mainfrom
refactor/shared-autoreview-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.

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

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

ClawSweeper review

What this changes

The branch replaces copied autoreview scripts and tests with a shared-skill entrypoint and updates contributor handoff commands and the changelog.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 3 items remain

The shared-installation cutover remains useful and is absent from current main. The previously reported severity-coverage regression remains unfixed; the member-authored PR should stay open.

Priority: P3
Reviewed head: d0015ba2d40f5a9dacaaa5b2055b0dd58e2d9292

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused cutover has useful validation context but still weakens the documented review gate.
Proof confidence 🌊 off-meta tidepool Not applicable: The ordinary contributor-proof gate is exempt for this member-authored tooling PR. The supplied dry-run claims concern shared-helper invocation; they do not resolve the independently source-proven severity mismatch. No stored-data contract changes.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The ordinary contributor-proof gate is exempt for this member-authored tooling PR. The supplied dry-run claims concern shared-helper invocation; they do not resolve the independently source-proven severity mismatch. No stored-data contract changes.
Evidence reviewed 8 items Pinned introduction and unchanged blocker: The introduced diff replaces both local handoff commands with the shared helper without a severity override. The previous completed review examined the identical head SHA, so its concrete finding remains applicable.
Explicit dependency boundary: The new entrypoint identifies openclaw/agent-skills as the canonical implementation and instructs consumers to run its installed scripts/autoreview. This explicitly makes the shared helper’s contract relevant to the replacement commands.
Verified shared severity contract: GitHub contents inspection at the recorded canonical main revision shows --max-priority defaulting to AUTOREVIEW_MAX_PRIORITY or P0 at lines 7071–7075. Lines 7337–7346 instruct the reviewer to omit lower priorities, and filter_findings_by_priority removes findings beyond the threshold.
Findings 1 actionable finding [P2] Preserve the existing review severity coverage
Security None None.

How this fits together

Autoreview checks contributors’ proposed changes before handoff. This cutover sends those changes to a separately installed shared helper, whose severity settings determine which findings block handoff.

flowchart LR
  A[Contributor changes] --> B[Repository handoff command]
  B --> C[Shared review helper]
  C --> D[Severity threshold]
  D --> E[Actionable findings]
  E --> F[Handoff result]
Loading

Before merge

  • Preserve the existing review severity coverage (P2) - With AUTOREVIEW_MAX_PRIORITY unset, this command inherits the shared helper’s P0 default, which instructs the reviewer to omit P1–P3 findings and filters any returned lower-priority findings. The removed helper covered all actionable priorities, so ordinary blocking defects can now escape the documented handoff gate. Add --max-priority P3 here and to the matching command in AGENTS.md. This finding was discovered late in an earlier cycle; the reviewed head is unchanged and it remains unfixed.
  • Resolve merge risk (P1) - Without an explicit P3 threshold, the documented handoff commands can report a clean review while ordinary blocking P1–P3 defects are omitted.
  • Complete next step (P2) - Add --max-priority P3 to the shared autoreview commands in AGENTS.md and CONTRIBUTING.md.

Findings

  • [P2] Preserve the existing review severity coverage — CONTRIBUTING.md:72
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Shared-tooling footprint 7 helper, harness, and implementation-test files removed Implementation and regression ownership move upstream rather than remaining in this consumer repository.

Merge-risk options

Maintainer options:

  1. Preserve the handoff threshold (recommended)
    Add --max-priority P3 to both documented shared-helper commands so the cutover retains the existing review coverage.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Add --max-priority P3 to the shared autoreview handoff commands in AGENTS.md and CONTRIBUTING.md; preserve the canonical shared installation and do not restore local helper copies.

Technical review

Best possible solution:

Use the canonical shared installation while explicitly preserving this repository’s full actionable-finding coverage.

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

Yes, from source: with AUTOREVIEW_MAX_PRIORITY unset, both replacement commands select the shared helper’s P0-only prompt and filtering. No helper or runtime execution was performed.

Is this the best way to solve the issue?

Yes, centralizing the copied implementation is appropriate, provided the two handoff commands explicitly retain P0–P3 coverage.

Full review comments:

  • [P2] Preserve the existing review severity coverage — CONTRIBUTING.md:72
    With AUTOREVIEW_MAX_PRIORITY unset, this command inherits the shared helper’s P0 default, which instructs the reviewer to omit P1–P3 findings and filters any returned lower-priority findings. The removed helper covered all actionable priorities, so ordinary blocking defects can now escape the documented handoff gate. Add --max-priority P3 here and to the matching command in AGENTS.md. This finding was discovered late in an earlier cycle; the reviewed head is unchanged and it remains unfixed.
    Confidence: 0.99
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 15e44ec7fb75.

Labels

Label changes:

No label changes.

Label justifications:

  • P3: This is contributor-tooling cleanup without a product-runtime change or urgent user-facing regression.
  • merge-risk: 🚨 automation: The replacement commands inherit a P0-only threshold and can weaken the automated review handoff gate.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The ordinary contributor-proof gate is exempt for this member-authored tooling PR. The supplied dry-run claims concern shared-helper invocation; they do not resolve the independently source-proven severity mismatch. No stored-data contract changes.

Evidence

Acceptance criteria:

  • [P1] git diff --check.
  • [P1] rg -n -- 'max-priority P3' AGENTS.md CONTRIBUTING.md.
  • [P1] pnpm check.

What I checked:

  • Pinned introduction and unchanged blocker: The introduced diff replaces both local handoff commands with the shared helper without a severity override. The previous completed review examined the identical head SHA, so its concrete finding remains applicable. (CONTRIBUTING.md:72, d0015ba2d40f)
  • Explicit dependency boundary: The new entrypoint identifies openclaw/agent-skills as the canonical implementation and instructs consumers to run its installed scripts/autoreview. This explicitly makes the shared helper’s contract relevant to the replacement commands. (.agents/skills/autoreview/SKILL.md:17, d0015ba2d40f)
  • Verified shared severity contract: GitHub contents inspection at the recorded canonical main revision shows --max-priority defaulting to AUTOREVIEW_MAX_PRIORITY or P0 at lines 7071–7075. Lines 7337–7346 instruct the reviewer to omit lower priorities, and filter_findings_by_priority removes findings beyond the threshold. (skills/autoreview/scripts/autoreview:7074, 3d3034af58d4)
  • Previous local coverage: The removed helper’s prompt requested every actionable defect without a severity threshold; the local skill documented nonzero exit when actionable findings remained. (.agents/skills/autoreview/scripts/autoreview:5797, c15ef395cdb6)
  • Current main and release retain the local helper: Fetched main still contains the local implementation and documents its local command. The v0.3.9 tag points at that same main revision; the shared-installation cutover is therefore not already implemented there. (AGENTS.md:79, 15e44ec7fb75)
  • Release identity: The tag check returned v0.3.9 for the fetched main revision. (15e44ec7fb75)

Likely related people:

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

  • Add --max-priority P3 to both repository handoff commands.

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 (9 earlier review cycles; latest 8 shown)
  • reviewed 2026-10-03T07:13:33.079Z sha d0015ba :: needs maintainer review before merge. :: none
  • reviewed 2026-10-03T11:09:43.649Z sha d0015ba :: needs maintainer review before merge. :: none
  • reviewed 2026-10-03T18:21:43.043Z sha d0015ba :: needs maintainer review before merge. :: none
  • reviewed 2026-10-04T00:46:43.608Z sha d0015ba :: blocked before merge. :: [P2] Preserve the existing review severity coverage
  • reviewed 2026-10-05T05:22:03.340Z sha d0015ba :: blocked before merge. :: [P2] Preserve the existing review severity coverage
  • reviewed 2026-10-05T09:45:35.131Z sha d0015ba :: blocked before merge. :: [P2] Preserve the existing review severity coverage
  • reviewed 2026-10-05T15:03:22.956Z sha d0015ba :: blocked before merge. :: [P2] Preserve the existing review severity coverage
  • reviewed 2026-10-05T17:47:27.595Z sha d0015ba :: blocked before merge. :: [P2] Preserve the existing review severity coverage

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed 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. labels Oct 3, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review October 6, 2026 18:45
@vincentkoc
vincentkoc requested a review from a team as a code owner October 6, 2026 18:45
@clawsweeper

clawsweeper Bot commented Oct 6, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@vincentkoc
vincentkoc merged commit 7458d69 into main Oct 6, 2026
11 of 12 checks passed
@vincentkoc
vincentkoc deleted the refactor/shared-autoreview-20261002 branch October 6, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant