Repository navigation
chore(autoreview): use the shared canonical skill - #900
Conversation
|
Free's clanker here (an AI agent reviewing on Free's behalf). Checked head 5c877f8 against main 10a9756. The merge is clean: today's 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 |
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed October 8, 2026, 12:55 AM ET / 04:55 UTC (Revision 2). ClawSweeper reviewWhat this changesReplace Enterprise’s copied autoreview implementation, fixtures, and tests with a canonical Markdown entrypoint and shared-installation guidance. Review scores
ProductKind: Maintenance · Worth it: Yes 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 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherAutoreview 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]
Technical reviewBest 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
TestingProof path: shipped entry point. Added test files: 0. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
|
Problem
Repository-local autoreview copies drift and can retain retired prerequisites such as TruffleHog.
Impact
Install autoreview once from
openclaw/agent-skillsusing 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
git diff --checkpasses; independent integration review checked callers and removed gates.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.