Skip to content

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

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

Problem

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

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.

Change

Keep only the canonical Markdown entrypoint and remove the copied helper, fixtures, and implementation tests. Replace copy/sync procedures with shared installation/update guidance and retain Enterprise's explicit reviewer choice.

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

Verification

  • 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 Markdown links resolve. Workspace/docs checks were attempted but cannot start without the existing Prettier, slugify, and docs-renderer dependencies; hosted CI is still required.

Risks

Contributors need the shared skill installation before reviewing. The full installed skill and required references remain mandatory; review policy stays with its owning repository.

Landing follow-up

Refreshed main in a243bbaa7568ab247c80bb6c62a3e65bef656e13. The only conflict combined the new developer handoff/image-verification guidance with the shared autoreview installation paragraph. The reviewed migration is unchanged; the prior independent ClawSweeper review found no actionable defects. Shared entrypoint equality, catalog links, and diff checks pass. Local docs checks still cannot start because the docs dependencies are absent; current-head required hosted CI passed.

The current contribution policy permits a matching core-team author to use the documented self-merge bypass after independent review, green required CI, resolved conversations, and no specific holds. Using that documented self-merge route: writer and author are both the authorized core-team maintainer, current-head ClawSweeper review reports no findings or before-merge items, CI Required is green, and no unresolved review threads or specific holds remain. The separate required-CI rule is not bypassed.

Current-head required gate: https://github.com/openclaw/openclaw-enterprise/actions/runs/37729410678/job/113155904448. The optional First Agent Smoke job was still running at admission; its workflow explicitly excludes it from CI Required.

@freeqaz

freeqaz commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Free's clanker here (an AI agent reviewing on Free's behalf).

Checked head 5c877f8 against main 10a9756. The merge is clean: today's AGENTS.md change on main is in a different section. With the vendored helper, tests and fixtures removed, nothing in CI, workflows, scripts/ or package.json references the deleted files on the merged tree. The remaining references are docs pointing at the shared install under $HOME/.agents/skills/autoreview, which stays valid. docs:check passes on the merged tree, and CI Required is green on this head.

Verdict: ready for a maintainer to merge once it is out of draft. The one decision for the maintainer is policy: contributors will need the shared skill installed to run autoreview. Open #827 rewrites the same paragraph in docs/testing/autoreview.md, so whichever lands second has to re-apply its wording.

@vincentkoc
vincentkoc marked this pull request as ready for review October 6, 2026 18:43
@vincentkoc
vincentkoc requested a review from a team as a code owner October 6, 2026 18:43
@clawsweeper

clawsweeper Bot commented Oct 6, 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.

@clawsweeper

clawsweeper Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

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

ClawSweeper review

What this changes

Replace Enterprise’s copied autoreview implementation, fixtures, and tests with a canonical Markdown entrypoint and shared-installation guidance.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused consumer cutover supported by verified canonical contracts, reported real installation checks, and no actionable findings.
Proof confidence 🐚 platinum hermit (4/6) Not applicable: The MEMBER-authored maintenance PR is exempt from ordinary contributor proof. The merged canonical migration reports fresh and upgraded installations plus helper invocation from consumer directories; source inspection confirms the documented contract, but runtime transcripts were not independently inspected. No persisted-data or material authorization contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Maintenance · Worth it: Yes
User problem: Contributors must maintain repository-local copies of the same review tool, allowing instructions and prerequisites to drift.
Reason: The canonical owner has shipped the shared-installation path, and Enterprise already delegates shared behavior upstream. Removing its duplicate implementation is a focused maintenance improvement.

Merge readiness

✅ Ready for maintainer review

This PR remains useful: current main still vendors autoreview, while the canonical shared-installation migration has merged upstream. No actionable defect or additional contributor-facing blocker was found; repository policy and MEMBER authorship also require keeping it open.

Priority: P3
Reviewed head: a243bbaa7568ab247c80bb6c62a3e65bef656e13

Before merge

None.

Findings

None.

Agent review details

How this fits together

Autoreview supports contributor code review rather than the Enterprise application runtime. Enterprise supplies the repository, selected reviewer, and local requirements; the shared helper reads the selected changes and returns review results.

flowchart TD
  A[Enterprise checkout] --> B[Local skill entrypoint]
  B --> C[Shared skill installation]
  A --> D[Changes and repository requirements]
  C --> E[Review helper]
  D --> E
  E --> F[Review results]
Loading

Technical review

Best possible solution:

Keep one canonical autoreview implementation with thin consumer entrypoints, explicit installation instructions, and Enterprise-owned reviewer and validation requirements.

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

Not applicable: this is a tooling consolidation, not a reported application bug; current main’s vendored copy and the shared installation contract were verified.

Is this the best way to solve the issue?

Yes. Reusing the canonical installer removes downstream implementation drift while preserving Enterprise’s explicit reviewer choice and repository requirements.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • .agents/skills/autoreview implementation, fixtures, and tests changes intended behavior with a stated reason (chore: add enterprise developer review and testing skills #133: Provide Enterprise developer review through an unchanged canonical skill, preserving its tests, fixtures, license, and isolation behavior.)
  • docs/testing/autoreview.md synchronization procedure changes intended behavior with a stated reason (Sync canonical autoreview helper #457: Keep the complete canonical copy synchronized and validate updates without suppressing failures or performing unsafe deleting syncs.)
  • docs/testing/autoreview.md upstream branch selection keeps the original intent (docs: default contributor PRs to forks #827: Avoid assuming that origin/main identifies the upstream target in a contributor fork checkout.)

Testing

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

Security

None.

Evidence

What I checked:

  • Pinned introduced change: The verified main-to-head delta contains 25 files, with 46 additions and 22,646 deletions, confined to autoreview tooling and contributor guidance. Whitespace checking passed. (a243bbaa7568)
  • Current-main necessity: Current main still invokes the repository-local helper and documents complete vendored-directory synchronization. The shared-installation cutover remains unimplemented in this repository. (docs/testing/autoreview.md:11, a0d57c95b091)
  • Preserved local requirements: The revised guide retains explicit GPT-6 Astra selection, verified upstream branch targeting, isolation requirements, report handling, and the distinction between developer review and runtime approvals. Shared installation is an expressly documented prerequisite. (docs/testing/autoreview.md:11, a243bbaa7568)
  • Deleted-path callers: Searching workflows, scripts, package metadata, skills, and contributor docs found no remaining executable caller of the removed local helper or test harness. Remaining autoreview links point to the entrypoint or shared installation. (.agents/skills/autoreview/SKILL.md:17, a243bbaa7568)
  • Canonical migration and dependency signal: Enterprise explicitly delegates execution and installation to agent-skills. fix(autoreview): share one installation across repositories agent-skills#305 merged on 2026-10-06 and records fresh and forced-upgrade symlink/copy installations plus helper invocation outside its source checkout; these are reported results rather than independently inspected runtime transcripts. (0595f725087f)
  • Verified installation contract: The merged canonical entrypoint matches Enterprise’s entrypoint. The installer supports the documented ~/.agents/skills target, default symlink mode, copy mode, and forced reinstalls. Full root and autoreview-scoped dependency instructions were read and support thin downstream entrypoints. (scripts/install-skills:17, 0595f725087f)

Likely related people:

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

Review metrics

Metric Value Why it matters
Executable code reduction helper/launcher code −8,167 LOC; tests/harness/fixtures −14,205 LOC; executable additions 0 The large deletion retires a duplicated upstream implementation and its coverage rather than expanding Enterprise runtime behavior.

Labels

Label changes:

  • add P3: This is contributor-tooling maintenance with no demonstrated application outage or runtime behavior change.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This is contributor-tooling maintenance with no demonstrated application outage or runtime behavior change.
  • 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 (1 earlier review cycle)
  • reviewed 2026-10-06T18:48:58.275Z sha 558a752 :: needs maintainer review 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 8, 2026
@vincentkoc
vincentkoc merged commit 5b82d78 into main Oct 8, 2026
45 checks passed
@vincentkoc
vincentkoc deleted the refactor/shared-autoreview-20261002 branch October 8, 2026 04:57
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.

2 participants