Skip to content

Require routable ClawSweeper commands before fast ack#233

Merged
Takhoffman merged 1 commit into
openclaw:mainfrom
brokemac79:fix-fast-ack-command-parser
May 31, 2026
Merged

Require routable ClawSweeper commands before fast ack#233
Takhoffman merged 1 commit into
openclaw:mainfrom
brokemac79:fix-fast-ack-command-parser

Conversation

@brokemac79
Copy link
Copy Markdown
Contributor

@brokemac79 brokemac79 commented May 30, 2026

Summary

Fixes #232.

This makes the fast-ack paths reject comments that only mention @clawsweeper inline inside prose, unless the same comment is actually routable by the command parser.

Changes:

  • gates the deterministic comment webhook on parseCommand(...) before posting the visible fast ack
  • extracts the shared ClawSweeper command-line detector used by both the router parser and hosted dashboard worker
  • replaces the hosted dashboard worker's loose mention regex with that shared parser-compatible detector
  • keeps valid command forms working, including an own-line mention and the @clawsweeper then next-line re-review pattern used by issue authors
  • adds regression coverage for inline mentions versus real command-line mentions
  • normalizes a few workflow test reads so CRLF workflow files do not break unrelated assertions

Testing

  • pnpm run build:all
  • pnpm run lint
  • node --test test\\repair\\comment-router-core.test.ts test\\repair\\comment-webhook.test.ts test\\dashboard-worker.test.ts
  • node --test --test-name-pattern "sweep workflow_dispatch input count|sweep review continuations|repair workflows preserve" test\\clawsweeper.test.ts
  • pnpm exec oxfmt --check src/repair/comment-command-text.ts src/repair/comment-router-core.ts dashboard/worker.ts

Note: I also tried pnpm run check locally on Windows earlier. The relevant build/lint/regression parts pass, but the full suite still hits unrelated Windows/local-environment failures around spawning codex and POSIX file-mode assertions. A repository-wide format:check also reports existing unrelated formatting noise on this Windows checkout, while the touched files pass targeted format check.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 30, 2026

Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 3:33 PM ET / 19:33 UTC.

Summary
The branch adds a shared command-line detector, uses it to gate standalone and hosted fast-ack classification, and adds regression coverage plus CRLF-tolerant workflow test reads.

Reproducibility: yes. source-reproducible: current main's loose fast-ack regex matches the reported inline mention while parseCommand only accepts command-shaped lines. I did not run live webhook delivery.

Review metrics: 2 noteworthy metrics.

  • Fast-ack classifiers: 2 paths changed. Both the standalone webhook and hosted dashboard worker can affect live ClawSweeper command intake.
  • Changed surface: 8 files, +217/-97. The patch is bounded but touches source, hosted worker code, and regression tests.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until 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 redacted live output from a real signed issue_comment webhook or hosted-worker run showing inline prose rejected and own-line commands accepted.
  • After adding proof, update the PR body so ClawSweeper re-reviews automatically, or ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No redacted live signed-webhook or hosted-worker output is present; the contributor should add terminal/log/live output with private details redacted and update the PR body to trigger re-review.

Risk before merge

  • [P1] No redacted live webhook or hosted-worker output is supplied yet, so signed issue_comment delivery and visible fast-ack behavior are only covered by tests and source inspection.
  • [P1] The diff changes both deterministic and hosted fast-ack command classifiers; a production regression could suppress valid commands or acknowledge non-command prose.

Maintainer options:

  1. Require live command-routing proof (recommended)
    Ask for redacted output from a real signed issue_comment webhook or hosted-worker run showing inline prose rejected and own-line commands accepted before merge.
  2. Accept source-level coverage risk
    Maintainers can merge based on source inspection and regression tests if they are comfortable owning the remaining live-runtime uncertainty.

Next step before merge

  • [P1] Needs contributor-supplied real behavior proof and normal maintainer review; no narrow code repair is currently identified.

Security
Cleared: The diff changes command parsing and tests without adding dependencies, secrets handling, permissions, downloaded code, or package-resolution changes.

Review details

Best possible solution:

Merge a narrow parser-compatible fast-ack gate after redacted real webhook or hosted-worker proof shows inline prose is ignored and own-line re-review commands still dispatch.

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

Yes, source-reproducible: current main's loose fast-ack regex matches the reported inline mention while parseCommand only accepts command-shaped lines. I did not run live webhook delivery.

Is this the best way to solve the issue?

Yes, the shared parser-compatible command-line detector is the narrow maintainable fix. The remaining blocker is proof that the same behavior works in a real webhook or hosted-worker run.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 83cb99d7a283.

Label changes

Label justifications:

  • P2: This is a bounded automation bug fix with limited blast radius, but it affects visible command acknowledgement and routing.
  • merge-risk: 🚨 automation: The PR changes live fast-ack command classification, where a regression could misroute, suppress, or falsely acknowledge ClawSweeper commands.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No redacted live signed-webhook or hosted-worker output is present; the contributor should add terminal/log/live output with private details redacted and update the PR body to trigger re-review.
Evidence reviewed

What I checked:

  • AGENTS.md policy read: Repository policy was read fully and applied: ClawSweeper reviews should be conservative, evidence-backed, automation-safe, and use pnpm run check before handoff for code changes. (AGENTS.md:1, 83cb99d7a283)
  • Current main has the reported standalone fast-ack mismatch: On current main, COMMAND_PATTERN can match a ClawSweeper mention in prose, while parseCommand(...) is computed but not used to reject a null parse before accepted maintainer comments proceed to fast ack. (src/repair/comment-webhook.ts:174, 83cb99d7a283)
  • Current main has the same hosted-worker loose gate: The hosted dashboard worker currently uses CLAWSWEEPER_COMMAND_PATTERN directly before fast ack, so it can accept mention-shaped prose without checking the router grammar. (dashboard/worker.ts:508, 83cb99d7a283)
  • Router grammar accepts command-shaped lines: parseCommand scans lines and only accepts slash or mention commands when the line itself matches the command grammar, which is the behavior the PR is aligning fast ack with. (src/repair/comment-router-core.ts:1025, 83cb99d7a283)
  • PR gates both fast-ack paths on parser-compatible command text: The PR adds extractClawSweeperCommandLine(...) and commandTextForClawSweeperFastAck(...), then uses the helper in dashboard/worker.ts and adds a null-parse rejection in comment-webhook.ts. (src/repair/comment-command-text.ts:17, c636d95b5da4)
  • Regression tests cover the central command shapes: The PR adds tests rejecting inline prose mentions before visible ack, keeping own-line mention commands accepted, and checking the hosted worker false-positive case. (test/repair/comment-webhook.test.ts:39, c636d95b5da4)

Likely related people:

  • Peter Steinberger: Blame shows 9efa568 introduced the standalone webhook regex, hosted-worker regex, and router parser surface that this PR changes. (role: introduced affected surface; confidence: high; commits: 9efa568da092; files: src/repair/comment-webhook.ts, dashboard/worker.ts, src/repair/comment-router-core.ts)
  • Tak Hoffman: Recent commits changed the command webhook and router path, including the current parsedCommand use for stale closed PR command handling. (role: recent repair-command contributor; confidence: medium; commits: 9b0b0b391733, c01469876a85, 8db024ba0503; files: src/repair/comment-webhook.ts, src/repair/comment-router-core.ts, test/repair/comment-webhook.test.ts)
  • brokemac79: Prior merged history on current main changed re-review command parsing and duplicate fast-ack handling in the same command-routing area, beyond authoring this PR. (role: recent command-routing contributor; confidence: high; commits: 485f71abc664, 556268dff3d9; files: src/repair/comment-router-core.ts, dashboard/worker.ts, test/repair/comment-router-core.test.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.

@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: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels May 30, 2026
@brokemac79 brokemac79 force-pushed the fix-fast-ack-command-parser branch from 7386adb to 49d2fd5 Compare May 30, 2026 18:54
@brokemac79 brokemac79 force-pushed the fix-fast-ack-command-parser branch from 49d2fd5 to c636d95 Compare May 30, 2026 19:17
Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated to extract the shared command-line detector so the router parser and hosted dashboard worker no longer maintain separate command-shape regexes.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 30, 2026

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@Takhoffman
Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 30, 2026

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@Takhoffman Takhoffman merged commit a07fc1f into openclaw:main May 31, 2026
9 checks passed
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. 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.

Fast ack can queue inline @clawsweeper mentions that the router cannot parse

2 participants