Skip to content

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

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

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review in progress

ClawSweeper is reviewing this revision. This supersedes any previous blocked status.

View the workflow run.

@clawsweeper

clawsweeper Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed October 5, 2026, 12:51 PM ET / 16:51 UTC (Revision 8).

ClawSweeper review

What this changes

The branch replaces Carapace’s copied code-review helper and tests with shared installation guidance and updates the contributor review command.

Merge readiness

⛔ Blocked before merge - 3 items remain

The cutover remains useful because current main still vendors autoreview. No concrete introduced defect was found; the MEMBER-authored PR requires explicit handling of the shared-installation prerequisite.

Priority: P3
Reviewed head: f10d492a17a183cce15dc8f2c50b698a9cd19e7d
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused consolidation with no identified blocking code defect; draft status and the cutover decision do not reduce patch quality.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER author is exempt from ordinary contributor proof. The body reports shared installer/helper dry runs, and source confirms consumer-directory targeting; no runtime transcript was independently inspected. No stored-data contract or material authorization boundary changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The MEMBER author is exempt from ordinary contributor proof. The body reports shared installer/helper dry runs, and source confirms consumer-directory targeting; no runtime transcript was independently inspected. No stored-data contract or material authorization boundary changes.
Evidence reviewed 8 items Introduced cutover: The pinned delta removes the local helper and harness files, retains a thin Markdown entrypoint, and changes the contributor command to the shared installation. No runtime assets, workflows, dependencies, or package exports change.
Existing users must install the shared helper: The new instructions explicitly require a one-time installation, preserve the consumer working directory, support an operator-selected location and Windows copy mode, and require reinstalls for copied installations. The previously documented repository-local executable is removed.
Affirmative canonical dependency and existing installer: The changed entrypoint expressly delegates execution and review instructions to this repository. Its current-main installer already defaults to ~/.agents/skills and supports symlink, copy, custom-target, dry-run, and forced replacement modes; this cutover does not require a new installer.
Findings None None.
Security None None.

How this fits together

Autoreview supports contributor code reviews rather than Carapace’s shipped design assets. The repository supplies Git changes and local requirements to an installed shared helper, which produces structured findings.

flowchart TD
 A[Contributor changes] --> B[Repository review guidance]
 B --> C[Shared skill installation]
 C --> D[Review helper]
 A --> D
 D --> E[Structured findings]
Loading

Decision needed

Question Recommendation
Is Carapace ready to remove the local review executable and require contributors to install the shared skill? Approve the documented cutover: Confirm the shared installation is available to contributors and accept the documented one-time setup requirement.

Why: The replacement is technically supported, but readiness for the new operator prerequisite requires repository-owner intent.

Before merge

  • Resolve merge risk (P1) - Contributors using the existing repository-local command will lose that executable and must install the shared skill before their review workflow works again; confirm rollout readiness before removing the local copy.
  • Complete next step (P2) - Approve the shared-installation cutover after confirming contributor setup readiness.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Local executable and test removal helper −8,982 lines; tests and harnesses −5,662 lines Implementation and validation ownership move out of Carapace to the canonical skill repository.

Merge-risk options

Maintainer options:

  1. Confirm shared setup and accept the cutover (recommended)
    Verify contributor environments can use the documented shared installation before approving removal of the old command.
  2. Wait for rollout readiness
    Pause this PR while contributor environments complete the shared installation.

Technical review

Best possible solution:

Keep one canonical implementation with thin consumer guidance and an explicitly approved, documented installation cutover.

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

Not applicable: this is tooling consolidation rather than a reported product bug; source confirms the local command is replaced by an external installation.

Is this the best way to solve the issue?

Yes, centralizing the existing helper is a focused way to prevent copied implementations from drifting, provided the installation cutover is approved.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

No label changes.

Label justifications:

  • P3: This is contributor-tooling consolidation without evidence of a product runtime regression.
  • merge-risk: 🚨 compatibility: The documented local command disappears and existing contributors must complete shared installation.
  • 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 author is exempt from ordinary contributor proof. The body reports shared installer/helper dry runs, and source confirms consumer-directory targeting; no runtime transcript was independently inspected. No stored-data contract or material authorization boundary changes.

Evidence

What I checked:

  • Introduced cutover: The pinned delta removes the local helper and harness files, retains a thin Markdown entrypoint, and changes the contributor command to the shared installation. No runtime assets, workflows, dependencies, or package exports change. (CONTRIBUTING.md:55, f10d492a17a1)
  • Existing users must install the shared helper: The new instructions explicitly require a one-time installation, preserve the consumer working directory, support an operator-selected location and Windows copy mode, and require reinstalls for copied installations. The previously documented repository-local executable is removed. (.agents/skills/autoreview/SKILL.md:13, f10d492a17a1)
  • Affirmative canonical dependency and existing installer: The changed entrypoint expressly delegates execution and review instructions to this repository. Its current-main installer already defaults to ~/.agents/skills and supports symlink, copy, custom-target, dry-run, and forced replacement modes; this cutover does not require a new installer. (scripts/install-skills:17, 3d3034af58d4)
  • Consumer working-directory boundary: The canonical helper derives the reviewed repository from Path.cwd(), supporting execution through an externally installed helper while retaining Carapace as the review target. (skills/autoreview/scripts/autoreview:1995, 3d3034af58d4)
  • Intentional upstream scanner policy: The complete canonical skill documents intentionally scanner-free reviews while retaining isolation and source-integrity controls. GitHub verifies fix(autoreview): remove restored TruffleHog dependency agent-skills#240 is merged and explicitly records the pre-transmission screening tradeoff. This review did not run a target-bundled helper; ClawSweeper admission scanning is separate. (skills/autoreview/SKILL.md, b86a9121adde)
  • Current-main necessity and historical routing: Current main still documents the local executable and contains its implementation. History records several prior canonical synchronization and reviewer-hardening commits by Vincent Koc, establishing relevance beyond authorship of this proposal. (.agents/skills/autoreview/SKILL.md, 4356b06026d7)

Likely related people:

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

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 (7 earlier review cycles)
  • reviewed 2026-10-02T08:48:47.603Z sha f10d492 :: blocked before merge. :: none
  • reviewed 2026-10-02T14:50:58.591Z sha f10d492 :: blocked before merge. :: none
  • reviewed 2026-10-02T20:47:18.748Z sha f10d492 :: blocked before merge. :: none
  • reviewed 2026-10-03T06:04:06.983Z sha f10d492 :: blocked before merge. :: none
  • reviewed 2026-10-03T22:51:39.670Z sha f10d492 :: blocked before merge. :: none
  • reviewed 2026-10-04T23:44:25.751Z sha f10d492 :: blocked before merge. :: none
  • reviewed 2026-10-05T08:50:15.164Z sha f10d492 :: needs maintainer review 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 review from a team and Patrick-Erichsen as code owners October 6, 2026 18:44
@vincentkoc
vincentkoc merged commit 4f1c32f into main Oct 6, 2026
6 of 7 checks passed
@vincentkoc
vincentkoc deleted the refactor/shared-autoreview-20261002 branch October 6, 2026 18:47
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