Skip to content

fix(qqbot): support QQBOT_DATA_DIR environment variable#77218

Closed
Phymath wants to merge 3 commits into
openclaw:mainfrom
Phymath:fix/qqbot-data-dir
Closed

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

Conversation

@Phymath

@Phymath Phymath commented May 4, 2026 •
edited
Loading

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:

@Phymath
fix(qqbot): support QQBOT_DATA_DIR environment variable
836b0c8 
Allow users to customize QQBot data directory by setting QQBOT_DATA_DIR environment variable. Falls back to ~/.openclaw/qqbot when not set.

Fixes openclaw#39461
@openclaw-barnacle openclaw-barnacle Bot added channel: qqbot size: XS triage: blank-template Candidate: PR template appears mostly untouched. labels May 4, 2026
@clawsweeper

clawsweeper Bot commented May 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

Summary
The PR adds a QQBOT_DATA_DIR override to QQBot storage path resolution and documents it in the QQBot docs and changelog.

Reproducibility: yes. Source inspection gives a high-confidence path: with QQBOT_DATA_DIR set, current-main getQQBotDataPath() still resolves under getHomeDir()/.openclaw/qqbot, and diagnostics warning logic still keys off the Windows home path.

Next step before merge
The remaining blockers are narrow PR-branch repairs in QQBot path resolution, diagnostics, and focused tests.

Security
Cleared: The diff only reads an operator-controlled environment variable for QQBot state placement and adds docs/changelog text; it adds no dependencies, scripts, network calls, or secret-handling changes.

Review findings

  • [P2] Treat blank QQBOT_DATA_DIR as unset — extensions/qqbot/src/engine/utils/platform.ts:45-46
  • [P2] Make diagnostics honor the effective data dir — extensions/qqbot/src/engine/utils/platform.ts:53
Review details

Best possible solution:

Centralize effective QQBot data-dir resolution, treat blank overrides as unset, make diagnostics use the effective path, and add focused regression coverage while keeping the new docs and changelog entry.

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

Yes. Source inspection gives a high-confidence path: with QQBOT_DATA_DIR set, current-main getQQBotDataPath() still resolves under getHomeDir()/.openclaw/qqbot, and diagnostics warning logic still keys off the Windows home path.

Is this the best way to solve the issue?

No. The PR changes the right owner-owned helper and adds docs/changelog, but it should also handle blank env values, update diagnostics, and add regression tests before merge.

Full review comments:

  • [P2] Treat blank QQBOT_DATA_DIR as unset — extensions/qqbot/src/engine/utils/platform.ts:45-46
    This branches on the raw env value before normalizePath() trims it. If QQBOT_DATA_DIR is set to whitespace, normalizePath() returns an empty string and path.join('', ...) resolves to . or relative subpaths, so QQBot state can be written under the gateway working directory instead of falling back to ~/.openclaw/qqbot. Trim first and fall back when the override is empty.
    Confidence: 0.9
  • [P2] Make diagnostics honor the effective data dir — extensions/qqbot/src/engine/utils/platform.ts:53
    The linked Windows report is about diagnostics still surfacing the home-based path/warning after QQBOT_DATA_DIR is set. This redirects the storage helper, but runDiagnostics() still warns solely on homeDir and tells users to set QQBOT_DATA_DIR; with an ASCII override, that warning remains misleading. Update diagnostics to account for the effective overridden data dir and cover it with a test.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

  • pnpm test extensions/qqbot/src/engine/utils/platform-storage-laziness.test.ts extensions/qqbot/src/engine/utils/platform.test.ts extensions/qqbot/src/engine/utils/diagnostics.test.ts
  • pnpm exec oxfmt --check --threads=1 extensions/qqbot/src/engine/utils/platform.ts extensions/qqbot/src/engine/utils/diagnostics.ts extensions/qqbot/src/engine/utils/platform-storage-laziness.test.ts extensions/qqbot/src/engine/utils/platform.test.ts extensions/qqbot/src/engine/utils/diagnostics.test.ts docs/channels/qqbot.md CHANGELOG.md
  • pnpm check:changed in Testbox before handoff if runtime/docs/changelog files change

What I checked:

  • Current main ignores QQBOT_DATA_DIR for QQBot storage: On current main, getQQBotDataPath() always joins getHomeDir(), .openclaw, and qqbot; there is no QQBOT_DATA_DIR lookup in the helper used by session, ref, and credential storage. (extensions/qqbot/src/engine/utils/platform.ts:43, 89a15fddaf84)
  • Diagnostics still warn on the home path: runDiagnostics() reports dataDir from getQQBotDataDir(), but the Windows warning still keys only off homeDir and tells users to set QQBOT_DATA_DIR; the PR does not change this file. (extensions/qqbot/src/engine/utils/diagnostics.ts:59, 89a15fddaf84)
  • Blank override becomes relative storage: The PR branches on the raw env value before normalizePath() trims it. Since path.join("") is "." and path.join("", "data") is "data", a whitespace-only QQBOT_DATA_DIR would store QQBot state relative to the gateway working directory instead of falling back to ~/.openclaw/qqbot. (extensions/qqbot/src/engine/utils/platform.ts:147, 89a15fddaf84)
  • PR file scope after prior review: The provided PR file list shows changes to platform.ts, docs/channels/qqbot.md, and CHANGELOG.md; it does not include diagnostics or regression test changes after the earlier ClawSweeper comment requested those areas. (extensions/qqbot/src/engine/utils/platform.ts:43, a19a5b518212)
  • Related bug report matches the remaining diagnostic problem: The linked issue reports Windows QQBot diagnostics still using C:\Users\张家豪\.openclaw\qqbot after QQBOT_DATA_DIR=D:\qqbot; the PR redirects the storage helper but does not update the home-directory warning behavior.
  • History for routing: Blame shows the current QQBot platform and diagnostics helpers were introduced in commit 585ce38015ef2145974f0ee12d5c02594796f29d. (extensions/qqbot/src/engine/utils/platform.ts:42, 585ce38015ef)

Likely related people:

  • steipete: git blame and git show attribute the current platform.ts and diagnostics.ts helper introduction to commit 585ce38015ef2145974f0ee12d5c02594796f29d, authored by Peter Steinberger. (role: introduced current QQBot platform/diagnostics helpers; confidence: high; commits: 585ce38015ef; files: extensions/qqbot/src/engine/utils/platform.ts, extensions/qqbot/src/engine/utils/diagnostics.ts)
  • Pavan Kumar Gondhi: Recent current-main history for extensions/qqbot includes QQBot streaming command auth work, which is adjacent channel ownership but not the storage helper itself. (role: recent adjacent QQBot maintainer; confidence: low; commits: 1f724bc50b; files: extensions/qqbot)

Remaining risk / open question:

  • Whitespace-only QQBOT_DATA_DIR would redirect QQBot state to . or relative subpaths on the PR branch.
  • Windows diagnostics would still tell users to set QQBOT_DATA_DIR even when the override is already set.
  • There is no regression coverage yet for the data-dir override or diagnostic warning behavior.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 89a15fddaf84.

@Phymath
docs(qqbot): document QQBOT_DATA_DIR environment variable
4815526 
Add QQBOT_DATA_DIR to qqbot.md configuration docs and CHANGELOG.md.

Refs openclaw#39461
@openclaw-barnacle openclaw-barnacle Bot added the docs Improvements or additions to documentation label May 4, 2026
@Phymath Phymath closed this May 4, 2026
@Phymath
Phymath deleted the fix/qqbot-data-dir branch May 4, 2026 11:02
This was referenced Jun 16, 2026
Open
Open
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 size: XS triage: blank-template Candidate: PR template appears mostly untouched.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]：路径错误

1 participant

@Phymath