Skip to content

Make autofix reviews action-first#374

Draft
proxynico wants to merge 1 commit into
openclaw:mainfrom
proxynico:clawsweeper/action-first-autofix-orchestrator
Draft

Make autofix reviews action-first#374
proxynico wants to merge 1 commit into
openclaw:mainfrom
proxynico:clawsweeper/action-first-autofix-orchestrator

Conversation

@proxynico

Copy link
Copy Markdown

Summary

  • make public ClawSweeper review comments action-first and stop emitting copyable bot commands/details dumps
  • trust deployed Dita/Nico ClawSweeper app bot identities by default
  • let trusted exact-head fix-required markers auto-opt normal PRs into clawsweeper:autofix
  • keep fix-only branch repairs out of the automerge continuation path unless merge is explicitly allowed

Proof

  • pnpm run build
  • pnpm run build:repair
  • node --test dist/repair/comment-router-core.test.js test/repair/comment-router-config.test.ts test/pr-comment-action-policy.test.ts

Live Dita proof on proxynico/quinela26#17: Dita pushed repair commit 6bec27c1631a, CI and CodeRabbit are green, and the current Dita comments have no 403, no copyable @clawsweeper automerge command, and no active fix-required marker.

Notes

Direct push to openclaw/clawsweeper is blocked for dita-clawsweeper[bot] with GitHub 403, so this goes through the proxynico/clawsweeper fork.

@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 7:38 AM ET / 11:38 UTC.

Summary
The branch makes PR review comments more action-first, adds Dita/Nico app bots to trusted defaults, lets trusted exact-head repair markers opt normal PRs into autofix, and prevents fix-only branch repairs from continuing automerge unless merge is allowed.

Reproducibility: not applicable. This PR changes automation behavior and policy; the important validation is source review plus inspectable live proof of the Dita autofix-only path.

Review metrics: 2 noteworthy metrics.

  • Files affected: 8 files, +330/-182. The diff crosses public review rendering, repair routing, repair execution, trusted defaults, and tests.
  • Trusted defaults: 2 bot identities added. Default trusted bot changes affect which GitHub app comments can drive ClawSweeper automation.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add accessible redacted live proof showing the Dita autofix-only path after this change.
  • Get maintainer confirmation that the Dita/Nico bot identities should be trusted by default.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body lists build/test commands and references live Dita proof, but proxynico/quinela26 was not resolvable here and no inspectable logs or artifacts are included; add redacted live output or a public artifact and update the PR body to trigger re-review.

Risk before merge

  • [P1] The PR broadens DEFAULT_TRUSTED_BOTS, so maintainers should confirm the Dita/Nico app identities are owner-controlled and intended to emit trusted structured automation markers by default.
  • [P1] Trusted exact-head repair markers would be able to label normal PRs with clawsweeper:autofix and dispatch branch repair, changing the default automation path for PRs that were not explicitly opted in by a maintainer.
  • [P1] The referenced live Dita proof is not accessible here, so the after-fix autofix-only behavior has not been independently inspected.

Maintainer options:

  1. Confirm default trust boundary
    A maintainer with deployment ownership context should confirm Dita/Nico app bot identities are controlled by OpenClaw and may emit trusted structured repair markers by default.
  2. Keep new bots deployment-configured
    Remove Dita/Nico from DEFAULT_TRUSTED_BOTS and rely on CLAWSWEEPER_TRUSTED_BOTS until default trust is explicitly approved.
  3. Add inspectable live proof
    Ask for redacted logs, terminal output, or a public linked artifact showing Dita applied an autofix-only repair and did not continue automerge.

Next step before merge

  • [P1] Human review is needed because the PR broadens trusted automation identities and the live proof reference is not inspectable.

Security
Needs attention: The diff expands trusted automation identity and repair opt-in behavior, so maintainer ownership approval is needed before merge.

Review details

Best possible solution:

Land only after maintainers confirm the Dita/Nico default trust boundary and the PR body includes accessible redacted live proof; otherwise keep those bot identities deployment-configured.

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

Not applicable. This PR changes automation behavior and policy; the important validation is source review plus inspectable live proof of the Dita autofix-only path.

Is this the best way to solve the issue?

Unclear. The implementation path is coherent, but the best solution depends on maintainer approval for default trust and accessible after-fix proof.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ae63b16d6c74.

Label changes

Label changes:

  • add P2: This is a normal-priority automation improvement with meaningful merge risk but no evidence of an urgent live regression.
  • add merge-risk: 🚨 security-boundary: Default trusted bot identities determine which GitHub app comments can trigger structured ClawSweeper repair and close handling.
  • add merge-risk: 🚨 automation: The diff changes autofix opt-in, trusted repair dispatch, public review action text, and post-repair automerge continuation behavior.

Label justifications:

  • P2: This is a normal-priority automation improvement with meaningful merge risk but no evidence of an urgent live regression.
  • merge-risk: 🚨 security-boundary: Default trusted bot identities determine which GitHub app comments can trigger structured ClawSweeper repair and close handling.
  • merge-risk: 🚨 automation: The diff changes autofix opt-in, trusted repair dispatch, public review action text, and post-repair automerge continuation behavior.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body lists build/test commands and references live Dita proof, but proxynico/quinela26 was not resolvable here and no inspectable logs or artifacts are included; add redacted live output or a public artifact and update the PR body to trigger re-review.
Evidence reviewed

Security concerns:

  • [medium] Default trust expands automation authority — src/repair/config.ts:15
    Adding Dita and Nico app bots to DEFAULT_TRUSTED_BOTS lets those accounts' structured comments drive trusted ClawSweeper automation markers; maintainers should confirm account ownership and intended authority before this ships.
    Confidence: 0.84

What I checked:

  • Repository policy read: AGENTS.md was read fully; its automation-safety guidance applies because this PR changes review, repair, trusted marker, and post-repair routing behavior. (AGENTS.md:1, ae63b16d6c74)
  • Trusted identity default changes: The PR adds dita-clawsweeper[bot] and nico-clawsweeper[bot] to DEFAULT_TRUSTED_BOTS, changing which GitHub app comments can be parsed as trusted structured automation. (src/repair/config.ts:15, c84b12d77bc0)
  • Trusted marker authority on current main: Current main's parseTrustedAutomation maps trusted authors' structured close, repair, human-review, and fix-required markers into executable router intents, making default-trusted author changes security-boundary relevant. (src/repair/comment-router-core.ts:1567, ae63b16d6c74)
  • Autofix opt-in routing change: The PR changes classifyCommand so a trusted exact-head ClawSweeper auto-repair marker can add clawsweeper:autofix and dispatch repair even when the PR was not already opted into the repair loop. (src/repair/comment-router.ts:794, c84b12d77bc0)
  • Autofix jobs remain merge-blocked by default: Current automerge repair job frontmatter blocks merge and sets allow_merge: false; the PR's continuation guard aligns with that model for fix-only branch repairs. (src/repair/comment-router-core.ts:319, ae63b16d6c74)
  • Live proof inaccessible: The PR body references live Dita proof on proxynico/quinela26, but gh could not resolve that repository or commit from this review environment, so the proof was not inspectable.

Likely related people:

  • RomneyDa: Blame and PR metadata tie the current review rendering, trusted marker parsing, and repair routing surfaces to the merged automation implementation in fix: rerender advisory label review comments #315. (role: introduced behavior and recent area contributor; confidence: high; commits: 4e5c4d47c83f; files: src/clawsweeper.ts, src/repair/comment-router.ts, src/repair/comment-router-core.ts)
  • brokemac79: Authored merged PR close-routing work in [codex] Route PR close reviews to autoclose #371, adjacent to this PR's trusted marker behavior. (role: recent trusted-routing contributor; confidence: medium; commits: 84796f0c9b0c; files: src/clawsweeper.ts, src/repair/comment-router.ts, src/repair/comment-router-core.ts)
  • vincentkoc: Authored recent merged branch-repair dispatch settling work in fix(repair): settle stale branch pushes before dispatch #347, adjacent to this PR's post-repair continuation changes. (role: recent branch-repair contributor; confidence: medium; commits: ab37a53656df, babd6a435afb; files: src/repair/execute-fix-artifact.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@proxynico proxynico force-pushed the clawsweeper/action-first-autofix-orchestrator branch from 753e704 to c84b12d Compare June 27, 2026 11:32
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jun 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant