Skip to content

fix(autoreview): share one installation across repositories - #305

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

Copied autoreview implementations drift across repositories and can retain retired prerequisites such as TruffleHog.

User Impact

Install autoreview once from this repository. Consumer repositories keep a Markdown entrypoint and use the shared helper from their own working directory. Updating the shared checkout updates all symlinked consumers; copy-mode installs need one reinstall.

Why This Change Was Made

Make this repository the single implementation and test owner, document the existing install/update path, and provide a common repository entrypoint. Preserve the useful Crabline changes before retiring its copy: native PowerShell launching, Python 3.10 probing, complete fixture expectations, and cleanup failure reporting. Correct directory traversal recovery with a real nonempty read-only fixture.

The other audited variants match historical canonical snapshots. This does not change the existing scanner-free policy or add self-updating review runs.

Crabline source: Windows launcher, interpreter probing, cleanup, and fixture expectations. Thanks @vincentkoc.

Evidence

  • Installer tests: 10 passed; skill validator tests: 12 passed; check-runner tests: 8 passed.
  • Boundary/harness suite: 190 tests, 4 platform skips. The transferred regressions failed before the fixes; real nonempty mode-0600 directory cleanup now passes.
  • Skill metadata validation, Bash syntax, and git diff --check pass.
  • Independent autoreview: no accepted P0–P2 findings.
  • Shared-install dry runs pass from two consumer repositories without TruffleHog.
  • Native Windows boundaries CI passed, including the new launcher test. Ubuntu validation and macOS sandbox checks also passed.

Consumer Rollout

Each existing consumer replaces its implementation with the shared entrypoint. Repository-specific review policy remains local.

  • acpx — merged; landed entrypoint verified.
  • carapace — merged; landed entrypoint verified.
  • clawhub — merged; landed entrypoint verified.
  • clawrouter — merged; landed entrypoint verified.
  • clawscan — merged; landed entrypoint verified.
  • crabbox — merged; landed entrypoint verified.
  • crabline — merged; landed entrypoint verified.
  • crawlkit — merged; landed entrypoint verified.
  • discrawl — merged; landed entrypoint verified.
  • docs — merged; landed entrypoint verified.
  • gitcrawl — merged; landed entrypoint verified.
  • krillswitch — merged with the organization admin’s explicit approval to bypass the missing code-owner review; required CI and independent review passed; landed entrypoint and P3 policy verified.
  • libterminal — merged; landed entrypoint verified.
  • openclaw — merged through native recovery after current-head CI and security gates passed; landed entrypoint verified.
  • openclaw-atif — merged; landed entrypoint verified.
  • openclaw-enterprise — merged under the documented core-author self-merge policy after current-head independent review and required CI passed; landed entrypoint verified.
  • openclaw-windows-node — merged; landed entrypoint verified; the single UI retry and enforced CI Gate passed.

Landing follow-up

Fresh and forced-upgrade installations passed in both symlink and copy modes using the real skill; helper invocation from outside its source checkout passed. Current Windows boundary CI passes, including the launcher regression. The merge conflict only combined changelog entries; existing upstream helper changes were preserved.

Rollout status (2026-10-08): complete — master plus all 17 existing consumers merged, with landed entrypoints verified. Shared installation is updated to the landed master. Repository review thresholds are preserved locally. Autoreview uses the canonical scanner-free implementation; independent repository security checks remain in place.

@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: needs changes before merge. Reviewed October 4, 2026, 7:26 PM ET / 23:26 UTC (Revision 5).

ClawSweeper review

What this changes

Autoreview gains shared-installation guidance, a repository entrypoint template, a Windows launcher, and stronger interpreter selection, fixture checks, and cleanup handling.

Merge readiness

⛔ Needs changes before merge - 1 item remains

This PR retains useful work absent from current main, and no blocking introduced defect was found. Its MEMBER author also excludes automatic closure. GitHub confirms that the branch remains draft and conflicted.

Priority: P2
Reviewed head: 334cc560ec495b215ad5e59c35b413e07a88703b

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused implementation and relevant regression coverage support normal good patch quality; draft and conflict status do not reduce that rating.
Proof confidence 🌊 off-meta tidepool Not applicable: The MEMBER author is exempt from ordinary contributor proof. The body reports shared-install dry runs and native launcher validation, but no runtime transcript was independently inspected. The diff changes no stored-data contract or material authorization boundary.
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-install dry runs and native launcher validation, but no runtime transcript was independently inspected. The diff changes no stored-data contract or material authorization boundary.
Evidence reviewed 9 items Pinned introduced change: Read all introduced hunks across 13 files. The branch adds installation guidance and launch/harness changes without modifying the production review engine, installer, dependencies, or workflows.
Existing installation capability: The existing installer already supports a shared symlink target, custom targets, and forced copy reinstalls. The PR documents and standardizes this supported path rather than introducing a competing installer.
Consumer working-directory boundary: The unchanged helper discovers the reviewed repository from Path.cwd(), while the new Windows launcher locates the helper beside itself and forwards arguments without changing directories.
Findings None None.
Security None None.

How this fits together

Autoreview is the repository's shared code-review helper. Consumer repositories supply their working directory and review requirements; the installed helper selects the Git changes and produces structured review results.

flowchart TD
  A[Consumer repository] --> B[Markdown entrypoint]
  B --> C[Shared skill installation]
  C --> D[Python or Windows launcher]
  A --> E[Git changes and local requirements]
  D --> F[Review helper]
  E --> F
  F --> G[Structured review results]
Loading

Before merge

  • Complete next step (P2) - Resolve the reported merge conflicts against current main and mark the PR ready for review when the draft work is complete.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test growth production +34/-0; test and harness code +241/-23 Production growth is the Windows launcher; the remaining executable growth supports interpreter fallback and harness regression coverage.

Technical review

Best possible solution:

Keep one canonical implementation with thin consumer entrypoints, explicit installation instructions, and the transferred platform and cleanup safeguards.

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

Not applicable as a single current-main bug reproduction: this is consolidation with several harness repairs. Source and regression coverage were inspected; no target code or tests were executed.

Is this the best way to solve the issue?

Yes. Reusing the existing installer and working-directory-aware helper is a focused way to eliminate downstream copies while preserving repository-specific requirements.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

No label changes.

Label justifications:

  • P2: Shared review installation and platform reliability are bounded improvements without evidence of an urgent user-facing outage.
  • 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-install dry runs and native launcher validation, but no runtime transcript was independently inspected. The diff changes no stored-data contract or material authorization boundary.

Evidence

What I checked:

  • Pinned introduced change: Read all introduced hunks across 13 files. The branch adds installation guidance and launch/harness changes without modifying the production review engine, installer, dependencies, or workflows. (334cc560ec49)
  • Existing installation capability: The existing installer already supports a shared symlink target, custom targets, and forced copy reinstalls. The PR documents and standardizes this supported path rather than introducing a competing installer. (scripts/install-skills:17, 334cc560ec49)
  • Consumer working-directory boundary: The unchanged helper discovers the reviewed repository from Path.cwd(), while the new Windows launcher locates the helper beside itself and forwards arguments without changing directories. (skills/autoreview/scripts/autoreview:1995, 334cc560ec49)
  • Cleanup and regression coverage: Cleanup restores directory traversal before recursive deletion and reports retained fixtures as failures. Added coverage exercises nonempty restricted directories, cleanup failure status, and launcher fallback/argument forwarding; the new module is included in the boundaries group. (skills/autoreview/scripts/test-review-harness.py:261, 334cc560ec49)
  • Current-main necessity: Fetched main still recommends vendored autoreview snapshots and lacks both the repository-entrypoint template and new Windows launcher. The exact branch changes are therefore not already implemented there; the supplied latest release is the PR's base, v0.4.0. (README.md:135, 3d3034af58d4)
  • Live branch state: The REST API confirms the pinned head remains draft with mergeable=false and mergeable_state=dirty. No assignees or requested reviewers are recorded. No pinned test-merge result is available, so this review makes no claim that merging removes current-main behavior. (334cc560ec49)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Vincent Koc: 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 (4 earlier review cycles)
  • reviewed 2026-10-02T08:45:28.572Z sha 334cc56 :: needs maintainer review before merge. :: none
  • reviewed 2026-10-02T08:52:06.417Z sha 334cc56 :: needs maintainer review before merge. :: none
  • reviewed 2026-10-03T04:10:38.844Z sha 334cc56 :: needs maintainer review before merge. :: none
  • reviewed 2026-10-03T22:52:03.486Z sha 334cc56 :: needs maintainer review before merge. :: none

@vincentkoc
vincentkoc marked this pull request as ready for review October 6, 2026 18:41
@vincentkoc
vincentkoc merged commit 0595f72 into main Oct 6, 2026
13 of 14 checks passed
@vincentkoc
vincentkoc deleted the refactor/shared-autoreview-20261002 branch October 6, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. 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