Skip to content

fix(qqbot): tighten bundled skill guardrails#98032

Merged
Patrick-Erichsen merged 1 commit into
mainfrom
pe/claw-434-qqbot-scan-cleanup
Jun 30, 2026
Merged

fix(qqbot): tighten bundled skill guardrails#98032
Patrick-Erichsen merged 1 commit into
mainfrom
pe/claw-434-qqbot-scan-cleanup

Conversation

@Patrick-Erichsen

@Patrick-Erichsen Patrick-Erichsen commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

CLAW-434 tracks official OpenClaw plugins that should publish without suspicious ClawHub/ClawScan warnings while preserving intended behavior. QQBot still needed source-side cleanup for bundled skill wording and guardrails after the Matrix fix landed.

Why This Change Was Made

This keeps QQBot's intended channel, media, and reminder capabilities, but makes the agent-facing contract safer and clearer:

  • Narrows bundled reminder/media/channel skill activation language to explicit user intent.
  • Adds privacy/minimization guidance for member avatars and inbound QQ media paths.
  • Moves destructive QQ channel actions behind an explicit protected delete flow.
  • Enforces confirmed=true for QQ channel DELETE requests, and bulkConfirmed=true for deleting all announcements.
  • Canonicalizes percent-encoded path segments so encoded all cannot bypass the bulk-delete confirmation guard.

This is source-only. It does not deploy, publish, or bump any QQBot package/plugin version.

User Impact

QQBot keeps the same intended capabilities, but agents get clearer guardrails before scheduling reminders, handling media, showing profile images, or mutating/deleting QQ channel resources. Accidental destructive deletes are blocked unless the user-confirmed target is represented in the tool call.

Evidence

  • SKILLSPECTOR_PROVIDER=openai clawscan extensions/qqbot/skills/qqbot-channel --scanner skillspector --scanner clawscan-static --output /tmp/claw-434-qqbot-channel-after-autoreview.json -> issues_found: 0; SkillSpector issues 0, static findings 0, recommendation SAFE.
  • SKILLSPECTOR_PROVIDER=openai clawscan extensions/qqbot/skills/qqbot-remind --scanner skillspector --scanner clawscan-static --output /tmp/claw-434-qqbot-remind-after-autoreview.json -> issues_found: 0; SkillSpector issues 0, static findings 0, recommendation SAFE.
  • SKILLSPECTOR_PROVIDER=openai clawscan extensions/qqbot/skills/qqbot-media --scanner skillspector --scanner clawscan-static --output /tmp/claw-434-qqbot-media-after-autoreview.json -> issues_found: 0; SkillSpector issues 0, static findings 0, recommendation SAFE.
  • clawscan extensions/qqbot --scanner clawscan-static --output /tmp/claw-434-qqbot-root-static-after-autoreview.json -> issues_found: 0.
  • node scripts/run-vitest.mjs extensions/qqbot/src/engine/tools/channel-api.test.ts extensions/qqbot/src/bridge/tools/remind.test.ts extensions/qqbot/src/engine/tools/remind-logic.test.ts -> 3 files / 48 tests passed.
  • node_modules/.bin/oxfmt --check extensions/qqbot/src/bridge/tools/channel.ts extensions/qqbot/src/bridge/tools/remind.ts extensions/qqbot/src/engine/tools/channel-api.ts extensions/qqbot/src/engine/tools/channel-api.test.ts extensions/qqbot/src/engine/tools/remind-logic.ts -> passed.
  • git diff --check -> passed.
  • .agents/skills/autoreview/scripts/autoreview --mode local --prompt "Review the QQBot ClawScan cleanup after fixes..." -> clean, no accepted/actionable findings.

No ClawHub deploy or package rescan was run from this PR.

@clawsweeper

clawsweeper Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 1:32 AM ET / 05:32 UTC.

Summary
The PR tightens QQBot bundled skill wording and changes qqbot_channel_api so DELETE calls require confirmed, with bulk announcement deletes also requiring bulkConfirmed, plus regression tests for encoded bulk-delete paths.

PR surface: Source +75, Tests +109, Docs +25. Total +209 across 9 files.

Reproducibility: yes. source inspection gives a high-confidence behavior path: current main accepts DELETE calls through executeChannelApi, while the PR adds tests proving unconfirmed DELETE and encoded bulk-delete calls are denied before fetch.

Review metrics: 2 noteworthy metrics.

  • Tool contract surfaces changed: 2 new tool parameters added. confirmed and bulkConfirmed alter how agents must call the existing QQBot channel API for destructive operations.
  • Source-only publishing proof: 0 deploy/package rescans reported. The PR demonstrates local source and skill scans, but maintainers still need release or marketplace validation for the ClawHub warning goal.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P1] Get maintainer acceptance of the fail-closed QQBot DELETE contract or add an approved compatibility/upgrade note before merge.

Risk before merge

  • [P1] Merging this PR changes the existing QQBot channel API tool contract: DELETE calls that previously reached the QQ API will now fail closed until agents or saved instructions send confirmed: true, and bulk announcement deletes also need bulkConfirmed: true.
  • [P1] The PR body explicitly says no ClawHub deploy or package rescan was run, so the final marketplace warning status remains a release/publishing validation step outside the source-only proof.

Maintainer options:

  1. Approve the stricter QQBot delete contract
    A maintainer can explicitly accept that existing DELETE tool calls must add confirmed and bulk deletes must add bulkConfirmed, then land with the current focused tests and scan proof.
  2. Add upgrade-facing guidance before merge
    Keep the fail-closed behavior but add maintainer-approved release or plugin guidance explaining the new QQBot delete confirmation fields and the expected agent/tool-call update path.
  3. Preserve compatibility until an opt-in strict mode exists
    If breaking current DELETE callers is not acceptable, revise the PR so existing behavior remains the default and the stricter confirmation gate is introduced through an explicit maintainer-approved compatibility path.

Next step before merge

  • [P1] The remaining blocker is maintainer approval of the compatibility-sensitive QQBot DELETE fail-closed contract, not an automated code repair.

Security
Cleared: No concrete supply-chain or secret-handling regression is visible; the diff narrows destructive QQBot API behavior and adds confirmation checks without adding dependencies or workflow execution paths.

Review details

Best possible solution:

Land the QQBot guardrails only after a maintainer explicitly accepts the fail-closed DELETE contract and either documents the upgrade impact or confirms no shipped compatibility path is needed for existing QQBot tool callers.

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

Yes, source inspection gives a high-confidence behavior path: current main accepts DELETE calls through executeChannelApi, while the PR adds tests proving unconfirmed DELETE and encoded bulk-delete calls are denied before fetch.

Is this the best way to solve the issue?

Unclear. The implementation is a coherent safety hardening path, but the best product solution depends on maintainer acceptance of a compatibility-sensitive fail-closed tool contract for existing QQBot callers.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR is a bounded QQBot safety improvement with compatibility risk, but it does not show a current P0/P1 outage or urgent regression.
  • merge-risk: 🚨 compatibility: The diff changes an existing tool contract so current DELETE callers now fail unless they provide new confirmation fields.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix command output summaries for ClawScan/SkillSpector and focused QQBot tool tests, which is sufficient real behavior proof for this source-only skill/tool guardrail change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix command output summaries for ClawScan/SkillSpector and focused QQBot tool tests, which is sufficient real behavior proof for this source-only skill/tool guardrail change.
Evidence reviewed

PR surface:

Source +75, Tests +109, Docs +25. Total +209 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 4 83 8 +75
Tests 1 109 0 +109
Docs 4 91 66 +25
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 283 74 +209

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs extensions/qqbot/src/engine/tools/channel-api.test.ts extensions/qqbot/src/bridge/tools/remind.test.ts extensions/qqbot/src/engine/tools/remind-logic.test.ts.
  • [P1] node_modules/.bin/oxfmt --check extensions/qqbot/src/bridge/tools/channel.ts extensions/qqbot/src/bridge/tools/remind.ts extensions/qqbot/src/engine/tools/channel-api.ts extensions/qqbot/src/engine/tools/channel-api.test.ts extensions/qqbot/src/engine/tools/remind-logic.ts.
  • [P1] git diff --check.

What I checked:

  • Repository policy read: Read the full root AGENTS.md and applied the OpenClaw guidance that plugin APIs, config/defaults, setup, fallback behavior, and compatibility paths need maintainer-visible upgrade scrutiny. (AGENTS.md:1, 85ee71223f0d)
  • Maintainer note checked: No .agents/maintainer-notes/ directory exists in this checkout, so there were no matching internal maintainer decisions to apply for QQBot files.
  • Current main DELETE behavior: On current main, executeChannelApi validates method/path and builds the QQ API request without any confirmed or bulkConfirmed parameters, so existing DELETE tool calls are currently accepted if the method/path validation passes. (extensions/qqbot/src/engine/tools/channel-api.ts:36, 85ee71223f0d)
  • PR diff contract change: The PR adds confirmed and bulkConfirmed to ChannelApiParams and adds tests asserting unconfirmed DELETE requests and bulk announcement deletes return structured denials without reaching fetchWithSsrFGuard. (extensions/qqbot/src/engine/tools/channel-api.ts:36, 72556d748406)
  • Caller surface inspected: The bridge registers qqbot_channel_api as a tool through the plugin SDK and exposes the changed schema/description directly to agents, so the DELETE confirmation behavior is a user-facing tool contract rather than an internal helper-only change. (extensions/qqbot/src/bridge/tools/channel.ts:1, 85ee71223f0d)
  • Related QQBot feature provenance: QQBot was introduced as a bundled channel/plugin with qqbot-channel, qqbot-media, and qqbot-remind skills in merged pull request Feature/add qq channel #52986, making this PR a follow-up on an existing bundled tool surface. (extensions/qqbot/src/engine/tools/channel-api.ts:1, bf6f506dfae6)

Likely related people:

  • sliverp: The QQBot channel/plugin and the bundled qqbot-channel, qqbot-media, and qqbot-remind skills were introduced through merged pull request Feature/add qq channel #52986, which created the central QQBot tool surface this PR changes. (role: introduced behavior; confidence: high; commits: bf6f506dfae6; files: extensions/qqbot/src/engine/tools/channel-api.ts, extensions/qqbot/skills/qqbot-channel/SKILL.md, extensions/qqbot/skills/qqbot-remind/SKILL.md)
  • RomneyDa: Recent merged QQBot work in perf(qqbot): narrow tool discovery cold load #90780 touched the bridge/tool registration path adjacent to this PR's channel/remind tool changes. (role: recent area contributor; confidence: medium; commits: ebabf5022ffb; files: extensions/qqbot/src/bridge/tools/channel.ts, extensions/qqbot/src/bridge/tools/remind.ts)
  • Patrick-Erichsen: This person authored the current PR and provided source scan/test proof for the QQBot guardrail change; they are included for handoff context, not as current-main ownership provenance. (role: current proposed-change author with adjacent review context; confidence: medium; commits: 72556d748406; files: extensions/qqbot/src/engine/tools/channel-api.ts, extensions/qqbot/src/engine/tools/channel-api.test.ts, extensions/qqbot/skills/qqbot-channel/SKILL.md)
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 proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 30, 2026
@Patrick-Erichsen
Patrick-Erichsen merged commit 56c2d63 into main Jun 30, 2026
147 of 158 checks passed
@Patrick-Erichsen
Patrick-Erichsen deleted the pe/claw-434-qqbot-scan-cleanup branch June 30, 2026 05:48
@Patrick-Erichsen

Patrick-Erichsen commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

This was referenced Jun 30, 2026
Closed
Closed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 1, 2026
@Patrick-Erichsen
fix(qqbot): tighten bundled skill guardrails (openclaw#98032)
9e1d86b
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@Patrick-Erichsen @QiuYuang
fix(qqbot): tighten bundled skill guardrails (openclaw#98032)
3c0bc75
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@Patrick-Erichsen chenyangjun-xy
fix(qqbot): tighten bundled skill guardrails (openclaw#98032)
88f9cfe
@clawsweeper clawsweeper Bot mentioned this pull request Jul 2, 2026
Closed
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@Patrick-Erichsen @chenyangjun-xy
fix(qqbot): tighten bundled skill guardrails (openclaw#98032)
13562dd
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 6, 2026
@Patrick-Erichsen cron
fix(qqbot): tighten bundled skill guardrails (openclaw#98032)
604eebf 
(cherry picked from commit 56c2d63)
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 8, 2026
@Patrick-Erichsen cron
fix(qqbot): tighten bundled skill guardrails (openclaw#98032)
397e73f 
(cherry picked from commit 56c2d63)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: qqbot maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Patrick-Erichsen