Skip to content

fix(qqbot): respect OPENCLAW_HOME for media paths#83567

Closed
hclsys wants to merge 2 commits into
openclaw:mainfrom
hclsys:fix-qqbot-media-openclaw-home-83562
Closed

fix(qqbot): respect OPENCLAW_HOME for media paths#83567
hclsys wants to merge 2 commits into
openclaw:mainfrom
hclsys:fix-qqbot-media-openclaw-home-83562

Conversation

@hclsys

@hclsys hclsys commented May 18, 2026

Copy link
Copy Markdown

Fixes #83562.

Summary

  • Make QQ Bot data/media roots resolve from OPENCLAW_HOME when it is configured, matching OpenClaw core state path behavior.
  • Keep existing HOME behavior as the fallback when OPENCLAW_HOME is not set.
  • Add regression coverage proving a QQ Bot media file under $OPENCLAW_HOME/.openclaw/media/qqbot is accepted by the structured payload local-file guard.

Scout audit

  • Audit A (existing helper): core has resolveStateDir/home resolution in src/utils.ts, but this extension helper avoids importing core internals; local path helper reused by existing QQ Bot data/media functions.
  • Audit B (shared callers): getQQBotDataPath, getQQBotMediaPath, and shared media-root guard keep the same returned shape under .openclaw; only base home resolution changes when OPENCLAW_HOME is explicitly configured.
  • Audit C (broader rival): no open rival PR found for #83562 / qqbot media OPENCLAW_HOME.

Validation

  • pnpm test extensions/qqbot/src/engine/utils/platform.test.ts -> 9 passed
  • git diff --check -> passed
  • pnpm check:changed -> passed

@openclaw-barnacle openclaw-barnacle Bot added channel: qqbot size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 18, 2026
@clawsweeper

clawsweeper Bot commented May 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR changes QQ Bot media path and local-file allowlist helpers to use OPENCLAW_HOME for media roots, adds regression coverage, and leaves persisted QQ Bot data on the legacy home-derived root.

Reproducibility: yes. from source: current main derives QQ Bot media roots from getHomeDir() while core state/media roots honor OPENCLAW_HOME. I did not run a live QQ Bot send in this read-only review.

PR rating
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Summary: The patch is useful and focused, but missing real behavior proof plus the unresolved legacy media-root compatibility issue keep it out of merge-ready shape.

Rank-up moves:

  • Add redacted real QQ Bot custom-OPENCLAW_HOME media-send proof to the PR body.
  • Preserve/remap legacy HOME-root media reads or get an explicit maintainer decision with focused coverage.
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.

Real behavior proof
Needs real behavior proof before merge: The PR body and comments list unit/check commands only; it needs redacted terminal/log output, copied live output, a screenshot, or a recording from a real QQ Bot media send with custom OPENCLAW_HOME, with private paths, endpoints, phone numbers, and keys redacted before updating the PR body for re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge
Why this matters: - Existing custom-OPENCLAW_HOME installs can still have queued, transcript, or cached QQ Bot media paths under HOME/.openclaw/media; after this PR those paths fail the structured local-file containment guard unless legacy reads are preserved or explicitly migrated.

  • The PR still has no redacted real QQ Bot send proof from a custom OPENCLAW_HOME setup; the available evidence is focused unit/check output only.

Maintainer options:

  1. Preserve Legacy Media Reads (recommended)
    Keep writes under OPENCLAW_HOME but allow or remap the previous HOME/.openclaw/media root read-only for existing custom-home media references, with regression coverage.
  2. Accept The Media Cache Break
    Maintainers can intentionally accept that old HOME-root media references stop sending after upgrade if the stricter OpenClaw-home boundary is the desired behavior.
  3. Pause For QQ Bot Owner Decision
    Pause this PR if media-root compatibility and path-boundary semantics need QQ Bot owner review before changing shipped behavior.

Next step before merge
Human review is needed because the PR lacks contributor real-behavior proof and maintainers need to decide whether HOME-root media reads remain trusted during custom-home upgrades.

Security
Cleared: The diff does not add dependencies, workflows, downloads, secret handling, or new code-execution paths; the remaining concern is compatibility of path allowlists.

Review findings

  • [P2] Preserve legacy media reads during the root move — extensions/qqbot/src/engine/utils/platform.ts:110
Review details

Best possible solution:

Write new QQ Bot media under the OPENCLAW_HOME media root, preserve or explicitly migrate legacy HOME-root media reads for upgrade safety, and land with focused tests plus redacted real QQ Bot media-send proof.

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

Yes, from source: current main derives QQ Bot media roots from getHomeDir() while core state/media roots honor OPENCLAW_HOME. I did not run a live QQ Bot send in this read-only review.

Is this the best way to solve the issue?

Not yet: switching media roots fixes the central custom-home path, but the merge-ready solution should preserve or explicitly decide legacy HOME-root media reads and still needs real QQ Bot proof.

Label justifications:

  • P2: This is a concrete QQ Bot custom-home media delivery fix with limited channel-specific blast radius.
  • merge-risk: 🚨 compatibility: The PR moves QQ Bot media roots for OPENCLAW_HOME installs and can stop existing HOME-root media references from passing the local-file guard.
  • merge-risk: 🚨 message-delivery: The affected path gates QQ Bot image, voice, video, and file sends, so rejected local media paths can surface as failed media delivery.

Full review comments:

  • [P2] Preserve legacy media reads during the root move — extensions/qqbot/src/engine/utils/platform.ts:110
    When OPENCLAW_HOME is set, this changes the shared media allowlist from the old HOME-derived ~/.openclaw/media to $OPENCLAW_HOME/.openclaw/media. Existing QQ Bot downloads or queued <qqmedia> paths under the old HOME media root will now fail resolveQQBotPayloadLocalFilePath, so preserve/remap the legacy media root read-only or make the breaking migration explicit with coverage.
    Confidence: 0.83

Overall correctness: patch is incorrect
Overall confidence: 0.83

What I checked:

Likely related people:

  • steipete: The central QQ Bot platform helper, payload guard, outbound media sender, and related tests in current main all trace to commit 53773ccee47217ba5ec52b0233c3dc5785619b2a. (role: introduced current media path helpers; confidence: high; commits: 53773ccee472; files: extensions/qqbot/src/engine/utils/platform.ts, extensions/qqbot/src/engine/utils/platform.test.ts, extensions/qqbot/src/engine/messaging/outbound-media-send.ts)
  • Sliverp: Recent merged QQ Bot work includes typing keepalive changes and an earlier QQ Bot media host allowlist fix, making this a reasonable adjacent routing candidate. (role: recent adjacent QQ Bot contributor; confidence: medium; commits: e6e1696c28b2, ddb7a8dd80b8; files: extensions/qqbot/src/engine/gateway/typing-keepalive.ts, extensions/qqbot/src/utils/file-utils.ts)
  • vincentkoc: Recent QQ Bot media-security test work around symlink race setup is adjacent to the local-file containment guard changed by this PR. (role: adjacent media security test contributor; confidence: medium; commits: 8f0da7ef0678; files: extensions/qqbot/src/outbound.security.test.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 67f8683ca30a.

@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 backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 18, 2026
@hclsys

This comment was marked as low quality.

Sign in to view
@clawsweeper clawsweeper Bot added merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 18, 2026
@hclsys

This comment was marked as low quality.

Sign in to view
@hclsys

This comment was marked as low quality.

Sign in to view
@sliverp sliverp mentioned this pull request May 22, 2026
Merged
25 tasks
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 merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: qqbot media directory ignores OPENCLAW_HOME, hardcodes ~/.openclaw/media/

1 participant

@hclsys