Skip to content

fix(qqbot): support QQBOT_DATA_DIR environment variable#78253

Open
Phymath wants to merge 2 commits into
openclaw:mainfrom
Phymath:fix/qqbot-data-dir
Open

fix(qqbot): support QQBOT_DATA_DIR environment variable#78253
Phymath wants to merge 2 commits into
openclaw:mainfrom
Phymath:fix/qqbot-data-dir

Conversation

@Phymath

@Phymath Phymath commented May 6, 2026

Copy link
Copy Markdown

Summary

Allow users to customize QQBot data directory by setting QQBOT_DATA_DIR environment variable. Falls back to ~/.openclaw/qqbot when not set.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

For bug fixes or regressions, explain why this happened, not just what changed. Otherwise write N/A. If the cause is unclear, write Unknown.

  • Root cause:
  • Missing detection / guardrail:
  • Contributing context (if known):

Regression Test Plan (if applicable)

For bug fixes or regressions, name the smallest reliable test coverage that should catch this. Otherwise write N/A.

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file:
  • Scenario the test should lock in:
  • Why this is the smallest reliable guardrail:
  • Existing test that already covers this (if any):
  • If no new test is added, why not:

User-visible / Behavior Changes

List user-visible changes (including defaults/config).
If none, write None.

Diagram (if applicable)

For UI changes or non-trivial logic flows, include a small ASCII diagram reviewers can scan quickly. Otherwise write N/A.

Before:
[user action] -> [old state]

After:
[user action] -> [new state] -> [result]

Security Impact (required)

  • New permissions/capabilities? (Yes/No)
  • Secrets/tokens handling changed? (Yes/No)
  • New/changed network calls? (Yes/No)
  • Command/tool execution surface changed? (Yes/No)
  • Data access scope changed? (Yes/No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS:
  • Runtime/container:
  • Model/provider:
  • Integration/channel (if any):
  • Relevant config (redacted):

Steps

Expected

Actual

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios:
  • Edge cases checked:
  • What you did not verify:

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

  • Backward compatible? (Yes/No)
  • Config/env changes? (Yes/No)
  • Migration needed? (Yes/No)
  • If yes, exact upgrade steps:

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

  • Risk:
    • Mitigation:

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: qqbot size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 6, 2026
@clawsweeper

clawsweeper Bot commented May 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 12, 2026, 2:44 AM ET / 06:44 UTC.

Summary
The branch makes QQBot diagnostics and remaining legacy/download paths honor a new QQBOT_DATA_DIR environment variable, adds focused path tests, and documents the variable as QQBot storage configuration.

PR surface: Source +9, Tests +69, Docs +1. Total +79 across 3 files.

Reproducibility: yes. at source level: current diagnostics recommend QQBOT_DATA_DIR, but the current helper ignores it, and the linked issue supplies concrete Windows steps. No live current-main or after-fix Windows run was performed or attached.

Review metrics: 2 noteworthy metrics.

  • Environment surfaces: 1 added, 0 changed, 0 removed. The new variable becomes a public filesystem-routing contract with compatibility and security implications.
  • Affected path consumers: 3 runtime consumers. The implementation affects diagnostics and selected downloads, not general QQBot runtime storage as the docs claim.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #39461
Summary: The linked issue is the canonical report and this PR is its active candidate fix; earlier same-purpose PRs are closed unmerged, while the SQLite migration is adjacent ownership work.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
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:

  • Obtain the QQBot data-root product decision from the likely owner.
  • [P1] Fix path validation and documentation scope for the chosen contract.
  • [P1] Attach redacted Windows Gateway or QQBot runtime proof of the after-fix behavior.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR contains focused tests and CI results but no after-fix Windows Gateway or QQBot runtime output showing the reported directory and surviving path consumers use the configured value; add redacted terminal or log proof, update the PR body to trigger review, and ask a maintainer for @clawsweeper re-review if it does not rerun automatically.

Risk before merge

  • [P1] Merging as written creates a second public data-directory contract whose documentation conflicts with current SQLite state and OPENCLAW_HOME-managed media ownership.
  • [P1] Whitespace-only or relative values can resolve to the process working directory or another unintended location, redirecting directory creation, write diagnostics, and legacy download access.
  • [P1] The PR has no after-fix Windows Gateway evidence showing that diagnostics and the actual surviving path consumers use the configured directory.

Maintainer options:

  1. Use canonical path ownership (recommended)
    Remove the stale variable recommendation rather than adding a second general QQBot storage contract.
  2. Narrow and validate the override
    Reject blank and relative values, limit the variable to surviving legacy/download consumers, correct the docs, and add Windows runtime proof.

Next step before merge

  • [P1] A maintainer must first choose the permanent QQBot path contract; after that, the contributor must address the two findings and supply real Windows behavior proof.

Maintainer decision needed

  • Question: Should QQBot remove the stale QQBOT_DATA_DIR recommendation and use canonical SQLite/media paths, or support it permanently as a narrowly scoped legacy/download override?
  • Rationale: Current main has no general QQBot filesystem state directory, so accepting this variable establishes a new public configuration and security boundary that code inspection cannot authorize.
  • Likely owner: steipete — The recent QQBot SQLite migration defines the current storage boundary and explicitly identifies this variable as separate work.
  • Options:
    • Keep canonical directories (recommended): Remove the unsupported recommendation and direct users to the existing SQLite state and OpenClaw-managed media model.
    • Approve a narrow override: Limit QQBOT_DATA_DIR to named diagnostics and legacy/download consumers with absolute-path validation, exact docs, and live Windows proof.

Security
Needs attention: The new environment value redirects real filesystem operations without an explicit safe absolute-local-path contract.

Review findings

  • [P1] Reject blank and relative data-directory values — extensions/qqbot/src/engine/utils/platform.ts:91-93
  • [P1] Document the variable's actual legacy scope — docs/channels/qqbot.md:75
Review details

Best possible solution:

Close this stale PR as superseded: Use canonical path ownership. Remove the stale variable recommendation rather than adding a second general QQBot storage contract.

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

Yes at source level: current diagnostics recommend QQBOT_DATA_DIR, but the current helper ignores it, and the linked issue supplies concrete Windows steps. No live current-main or after-fix Windows run was performed or attached.

Is this the best way to solve the issue?

No, not as currently framed: calling this general QQBot storage conflicts with current SQLite and media ownership. The narrow maintainable choices are removing the stale promise or approving a validated legacy/download-only override.

Full review comments:

  • [P1] Reject blank and relative data-directory values — extensions/qqbot/src/engine/utils/platform.ts:91-93
    Trim and validate this new public path before using it. A whitespace-only value normalizes to an empty string, and a relative value remains relative, so diagnostics and download staging can create or access directories under the Gateway working directory instead of the intended absolute data root.
    Confidence: 0.96
  • [P1] Document the variable's actual legacy scope — docs/channels/qqbot.md:75
    Do not describe this as the directory for all QQBot storage. Current runtime state is in plugin SQLite KV and managed media uses a separate OPENCLAW_HOME root; this variable only reaches diagnostics and selected legacy/download paths, so the current wording gives users a false configuration contract.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The source mismatch is real but limited to QQBot path diagnostics and selected filesystem workflows rather than an urgent channel outage.
  • merge-risk: 🚨 compatibility: The new environment contract overlaps existing SQLite state and OPENCLAW_HOME media ownership and may confuse upgrades or configuration.
  • merge-risk: 🚨 security-boundary: The configured value selects the root used for directory creation, write checks, and permitted legacy download paths.
  • 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 real behavior proof before merge: The PR contains focused tests and CI results but no after-fix Windows Gateway or QQBot runtime output showing the reported directory and surviving path consumers use the configured value; add redacted terminal or log proof, update the PR body to trigger review, and ask a maintainer for @clawsweeper re-review if it does not rerun automatically.
Evidence reviewed

PR surface:

Source +9, Tests +69, Docs +1. Total +79 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 1 11 2 +9
Tests 1 70 1 +69
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 82 3 +79

Security concerns:

  • [medium] Unvalidated environment-controlled filesystem root — extensions/qqbot/src/engine/utils/platform.ts:91
    Whitespace-only and relative values can redirect directory creation, write checks, and legacy download access to unintended working-directory locations.
    Confidence: 0.96

What I checked:

  • recommended close path: the latest review's recommended merge-risk option is Use canonical path ownership, categorized as pause_or_close.
  • stale PR: PR was opened 2026-05-06T03:35:01Z, which is older than the 30-day stale promotion threshold.
  • no human follow-up: live comments and timeline hydrated by apply contain no non-automation activity after the ClawSweeper review.

Likely related people:

  • steipete: Authored the merged SQLite migration that defines current QQBot state ownership and explicitly left this environment-variable contract as separate work. (role: recent QQBot storage contributor; confidence: high; commits: 24d94a54a516; files: extensions/qqbot/src/state-migrations.ts, extensions/qqbot/src/engine/session/session-store.ts, extensions/qqbot/src/engine/config/credential-backup.ts)
  • sliverp: Authored the nearby merged OPENCLAW_HOME media-path change and tests, establishing the distinction between managed media and the legacy data root. (role: adjacent QQBot path contributor; confidence: high; commits: 0d23c3b4e133; files: extensions/qqbot/src/engine/utils/platform.ts, extensions/qqbot/src/engine/utils/platform.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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-12T06:01:41.731Z sha 3feef83 :: needs real behavior proof before merge. :: [P1] Reject blank and relative data-directory values | [P1] Document the variable's actual legacy scope

@Phymath
Phymath force-pushed the fix/qqbot-data-dir branch from 50e8eaf to cb6be91 Compare May 8, 2026 00:56
This was referenced May 9, 2026
Open
Closed
Closed
Closed
@Phymath
Phymath force-pushed the fix/qqbot-data-dir branch from cb6be91 to 9e46536 Compare May 17, 2026 04:15
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. impact:security Security boundary, credential, authz, sandbox, or sensitive-data risk. impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. impact:auth-provider Auth, provider routing, model choice, or SecretRef resolution may break. labels May 17, 2026
@Phymath
Phymath force-pushed the fix/qqbot-data-dir branch from 9e46536 to 28d78b3 Compare May 29, 2026 09:34
@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. 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. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed impact:session-state Session, memory, transcript, context, or agent state can drift or corrupt. impact:security Security boundary, credential, authz, sandbox, or sensitive-data risk. impact:auth-provider Auth, provider routing, model choice, or SecretRef resolution may break. labels May 29, 2026
@clawsweeper clawsweeper Bot removed 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. labels Jun 20, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 20, 2026
@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. labels Jun 20, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Merged
@brokemac79 brokemac79 mentioned this pull request Jun 25, 2026
Merged
@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jul 2, 2026
@Phymath
Phymath force-pushed the fix/qqbot-data-dir branch from 4bbb356 to 5ddb021 Compare July 2, 2026 06:50
@Phymath
fix(qqbot): support QQBOT_DATA_DIR environment variable
3feef83 
Allow users to customize QQBot data directory by setting QQBOT_DATA_DIR environment variable. Falls back to ~/.openclaw/qqbot when not set.
Closes [Bug]：openclaw#39461
@Phymath
Phymath force-pushed the fix/qqbot-data-dir branch from 5ddb021 to 3feef83 Compare July 12, 2026 05:53
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 docs Improvements or additions to documentation merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. 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-pr-context Candidate: external PR body lacks required problem context or evidence. 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]：路径错误

1 participant

@Phymath