Skip to content

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

Merged
vincentkoc merged 3 commits into
mainfrom
refactor/shared-autoreview-20261002
Oct 8, 2026
Merged

vincentkoc merged 3 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 retain the original P0–P3 review gate with --max-priority P3. The shared entrypoint remains identical to the canonical template. Refreshed main in c1fa2491515f0a98c3866cc71c26f5b9ab641116; the changelog conflict preserves both entries and leaves the reviewed migration unchanged. Diff checks and all required hosted checks pass. The organization admin explicitly authorized bypassing the missing code-owner review for this PR. Exact head c1fa2491515f0a98c3866cc71c26f5b9ab641116 has all required CI checks passing, a completed independent review with no findings or before-merge items, and no unresolved review threads. This exception applies only to the missing review; repository settings and CI requirements are unchanged.

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed October 8, 2026, 12:55 AM ET / 04:55 UTC (Revision 6).

ClawSweeper review

What this changes

Replace the repository’s copied autoreview helper and tests with shared-installation guidance while retaining its P0–P3 review gate.

Example: A contributor reviews a non-trivial change

  • Before: The contribution guide runs .agents/skills/autoreview/scripts/autoreview from the repository’s copied implementation.
  • After: The contribution guide runs the shared installed helper with --max-priority P3 after the documented setup.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, source-verified cutover with reported real installation checks and no remaining actionable defect supports normal good PR quality.
Proof confidence 🐚 platinum hermit (4/6) Not applicable: The MEMBER author is exempt from ordinary contributor proof. The target and merged upstream bodies report shared installer and helper dry runs from consumer repositories, including fresh and upgraded symlink/copy installations; source confirms caller-directory and threshold handling, but no execution transcript was independently inspected. No stored-data contract or material authority boundary changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Maintenance · Worth it: Yes
User problem: Contributors can receive outdated review behavior from repository-local copies that drift from the canonical tool.
Reason: Removing a large copied implementation reduces maintenance drift. Prior merged area history and the author’s explicit rollout direction support the documented shared-installation transition.

Merge readiness

✅ Ready for maintainer review

This PR remains useful and has no identified merge blocker. The refreshed head resolves the previous changelog conflict, preserves the review threshold, and uses the merged canonical installation contract.

Priority: P3
Reviewed head: c1fa2491515f0a98c3866cc71c26f5b9ab641116

Before merge

None.

Findings

None.

Agent review details

How this fits together

Autoreview checks proposed repository changes before contributors ship them. The repository supplies its working directory and review requirements to a shared helper, which returns structured findings.

flowchart TD
 A[Contributor changes] --> B[Repository review instructions]
 B --> C[Shared installation setup]
 C --> D[Installed review helper]
 E[Repository working directory] --> D
 F[P0 through P3 threshold] --> D
 D --> G[Structured review findings]
Loading

Technical review

Best possible solution:

Keep one upstream implementation and test owner, with explicit consumer setup and repository-specific review thresholds.

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

Not applicable: this is tooling consolidation rather than a reported runtime bug; source confirms the copied implementation remains on current main.

Is this the best way to solve the issue?

Yes. The existing canonical installer and working-directory-aware helper support this cutover without a parallel implementation, and explicit P3 arguments preserve the repository contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 5f1c98b00c69.

Provenance checked

  • .agents/skills/autoreview implementation and tests changes intended behavior with a stated reason (6912e6d: The canonical synchronization followed the foundation’s checked-in review skill; the scoped policy designated agent-skills as the canonical implementation and prohibited local variants.)
  • AGENTS.md review command keeps the original intent (031f919: The release-governance baseline established repository review guidance for non-trivial changes.)
  • CONTRIBUTING.md review command keeps the original intent (031f919: The release-governance change added root contribution guidance and exact validation reporting.)

Testing

Proof path: shipped entry point. Added test files: 0.

Security

None.

Evidence

What I checked:

  • Pinned introduced scope: The pinned main-to-head delta contains 11 files, 25 additions, and 15,056 deletions, confined to autoreview copies and repository guidance; product code, workflows, dependencies, and storage schemas are unchanged. (c1fa2491515f)
  • Complete local cutover: The tracked tree retains only the Markdown autoreview entrypoint. Repository-wide caller searches find the updated root and contribution instructions, with no surviving caller of the removed helper or harness. (CONTRIBUTING.md:47, c1fa2491515f)
  • Repository threshold preserved: Both contributor-facing commands explicitly pass --max-priority P3; the root guidance also explains preservation of the existing review gate. (AGENTS.md:72, c1fa2491515f)
  • Verified canonical dependency: The new entrypoint explicitly depends on this repository’s installer and helper. Its merged template matches the local entrypoint, and the installer supports the documented default location, symlink installation, copy mode, and forced replacement. (scripts/install-skills:17, 0595f725087f)
  • Shared helper contract: The canonical helper discovers the reviewed repository from the caller’s working directory and accepts P3 explicitly; its default threshold is P0, making the local override meaningful. (skills/autoreview/scripts/autoreview:7072, 0595f725087f)
  • Merged upstream rollout and validation: fix(autoreview): share one installation across repositories agent-skills#305 is merged. Its complete body records real-skill fresh and forced-upgrade installations in symlink and copy modes, invocation outside the source checkout, and preservation of downstream fixes. These are reported executions; no runtime transcript was independently inspected. (0595f725087f)

Likely related people:

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

Review metrics

Metric Value Why it matters
Copied implementation removed 6 executable or test files removed; 14,644 lines deleted Implementation and regression ownership moves upstream rather than maintaining another repository snapshot.

Labels

Label changes:

No label changes.

Label justifications:

  • P3: This is focused review-tooling maintenance with no product runtime change or urgent user-facing failure.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

History

Review history (5 earlier review cycles)
  • reviewed 2026-10-02T08:49:36.477Z sha b07819c :: needs maintainer review before merge. :: none
  • reviewed 2026-10-06T06:02:08.865Z sha b07819c :: needs changes before merge. :: none
  • reviewed 2026-10-06T18:51:15.798Z sha 61e2269 :: needs maintainer review before merge. :: none
  • reviewed 2026-10-07T14:03:10.788Z sha 61e2269 :: needs maintainer review before merge. :: none
  • reviewed 2026-10-08T01:08:09.086Z sha 61e2269 :: needs changes before merge. :: none

@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. labels Oct 6, 2026
@vincentkoc
vincentkoc requested a review from a team October 6, 2026 18:44
@vincentkoc
vincentkoc marked this pull request as ready for review 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.

ClawSweeper review complete

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

View the workflow run.

@vincentkoc
vincentkoc merged commit 23582fe into main Oct 8, 2026
10 checks passed
@vincentkoc
vincentkoc deleted the refactor/shared-autoreview-20261002 branch October 8, 2026 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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