Skip to content

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

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. Remove the duplicate test launcher and its package/test wiring. Its useful launcher and harness fixes are preserved in the upstream PR.

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.
  • Changed TypeScript passes node --check; package JSON parses. Runtime tests/formatters were not run locally because the dependency installation is absent.

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

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review blocked

Automated review did not run, so no review verdict was produced.

Reason: The input-safety check rejected material in this revision. No detected value, path, or scanner output is reproduced here.

Automatic review is on hold, including after source changes. Request a fresh re-review after resolving the failure; maintainers can also release the hold through an intentional scanner-policy update.

Next step: If this is a genuine credential, remove and rotate it. If it is an intentional test fixture, a maintainer must review and qualify it.

View the workflow run.

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed October 2, 2026, 6:59 PM ET / 22:59 UTC (Revision 4).

ClawSweeper review

What this changes

Replace Crabline’s copied autoreview helper and tests with a shared-skill entrypoint, removing their local package verification wiring.

Merge readiness

⛔ Blocked before merge - 2 items remain

This remains useful cleanup absent from main. The earlier compatibility blocker is still unresolved, and the MEMBER-authored PR remains open for maintainer handling.

Priority: P3
Reviewed head: c0d39592a9588c0c3ae4670416e6c4f30d86dcd5

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused cleanup has no established correctness defect, while dependency landing order remains a merge prerequisite.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof requirements. Its entrypoint redirects reviews to the shared helper and reports consumer dry runs; this review inspected source without executing them. No material authority transfer or stored-data contract change is introduced.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER-authored PR is exempt from ordinary contributor proof requirements. Its entrypoint redirects reviews to the shared helper and reports consumer dry runs; this review inspected source without executing them. No material authority transfer or stored-data contract change is introduced.
Evidence reviewed 9 items Pinned introduced changes: The pinned main-to-head delta removes the local implementation, launchers, harness tests, and test launcher; package verification no longer runs their dedicated suite. The remaining entrypoint preserves the consumer working directory.
Affirmative dependency boundary: The changed entrypoint explicitly installs and executes openclaw/agent-skills. The PR body says Crabline’s downstream fixes are preserved in openclaw/agent-skills#305, making that dependency’s available behavior directly relevant.
Canonical transfer remains unmerged: GitHub reports openclaw/agent-skills#305 as open and unmerged at head 334cc560ec495b215ad5e59c35b413e07a88703b. Its patch transfers the Windows launcher, interpreter probing, complete malicious-fixture expectations, and cleanup failure handling.
Findings None None.
Security None None.

How this fits together

Autoreview supports contributor code review rather than Crabline’s provider runtime. The repository entrypoint directs agents to an installed shared helper, which reviews changes from the consumer repository’s working directory.

flowchart TD
 A[Repository changes] --> B[Local review entrypoint]
 B --> C[Shared skill installation]
 C --> D[Review helper]
 A --> D
 D --> E[Structured review results]
Loading

Before merge

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Local tooling removal 10 files removed; 4 files modified Implementation and test ownership move upstream, making transfer order relevant despite the small replacement entrypoint.

Merge-risk options

Maintainer options:

  1. Land the canonical transfer first (recommended)
    Merge fix(autoreview): share one installation across repositories agent-skills#305 before this consumer cleanup so the documented installation preserves the existing tooling guarantees.

Technical review

Best possible solution:

Keep one canonical implementation while preserving Crabline’s existing launcher and harness guarantees throughout the shared-installation transition.

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

Not applicable to this tooling cleanup; source and live upstream metadata establish the landing-order gap without executing a review.

Is this the best way to solve the issue?

Yes, centralizing the implementation uses the existing installer and avoids copy drift, provided the upstream transfer lands before consumer removal.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 48a8cc967026.

Labels

Label changes:

No label changes.

Label justifications:

  • P3: This is contributor-tooling cleanup with no changed Crabline product runtime behavior.
  • merge-risk: 🚨 compatibility: Canonical main does not yet contain the launcher and harness guarantees removed from the consumer.
  • 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: The MEMBER-authored PR is exempt from ordinary contributor proof requirements. Its entrypoint redirects reviews to the shared helper and reports consumer dry runs; this review inspected source without executing them. No material authority transfer or stored-data contract change is introduced.

Evidence

What I checked:

  • Pinned introduced changes: The pinned main-to-head delta removes the local implementation, launchers, harness tests, and test launcher; package verification no longer runs their dedicated suite. The remaining entrypoint preserves the consumer working directory. (.agents/skills/autoreview/SKILL.md:17, c0d39592a958)
  • Affirmative dependency boundary: The changed entrypoint explicitly installs and executes openclaw/agent-skills. The PR body says Crabline’s downstream fixes are preserved in fix(autoreview): share one installation across repositories agent-skills#305, making that dependency’s available behavior directly relevant. (.agents/skills/autoreview/SKILL.md:8, c0d39592a958)
  • Canonical transfer remains unmerged: GitHub reports fix(autoreview): share one installation across repositories agent-skills#305 as open and unmerged at head 334cc560ec495b215ad5e59c35b413e07a88703b. Its patch transfers the Windows launcher, interpreter probing, complete malicious-fixture expectations, and cleanup failure handling. (334cc560ec49)
  • Default-branch compatibility gap: Current canonical main remains edf1e2f436fa2a5375c98052180a090a57223f9e. Its Windows autoreview launcher is absent, and its harness requires only a command finding and treats cleanup failures as warnings, unlike Crabline’s removed implementation. (skills/autoreview/scripts/test-review-harness.py:249, edf1e2f436fa)
  • Existing installation mechanism: The existing installer supports the documented ~/.agents/skills default, symlink and copy modes, custom targets, and force replacement. This PR reuses that mechanism rather than adding a competing installer. (scripts/install-skills:19, edf1e2f436fa)
  • Re-review continuity: The previous completed review covered this same head, recorded no findings, and required landing the canonical transfer first. Live upstream metadata confirms that requirement remains unmet; no new line-level defect is established. (c0d39592a958)

Likely related people:

  • vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Peter Steinberger: 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.

  • Land the canonical transfer before removing the downstream tooling copies.

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 (3 earlier review cycles)
  • reviewed 2026-10-02T08:48:51.183Z sha c0d3959 :: blocked before merge. :: none
  • reviewed 2026-10-02T11:00:36.517Z sha c0d3959 :: blocked before merge. :: none
  • reviewed 2026-10-02T19:02:14.855Z sha c0d3959 :: blocked before merge. :: none

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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. labels Oct 2, 2026
@vincentkoc
vincentkoc marked this pull request as ready for review October 6, 2026 18:44
@vincentkoc
vincentkoc requested a review from a team as a code owner October 6, 2026 18:44
@vincentkoc
vincentkoc merged commit 45eb015 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: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant