Skip to content

hooks: extract shared lifecycle hook runner into src/hooks/lifecycle.ts#20

Open
Arry8 wants to merge 21 commits intomainfrom
feature/10-lifecycle-hooks-shared-module
Open

hooks: extract shared lifecycle hook runner into src/hooks/lifecycle.ts#20
Arry8 wants to merge 21 commits intomainfrom
feature/10-lifecycle-hooks-shared-module

Conversation

@Arry8
Copy link
Copy Markdown
Owner

@Arry8 Arry8 commented Mar 15, 2026

Closes #10

Summary

  • Added LifecycleHookEntry and LifecycleHookRunResult to src/config/types.hooks.ts — the generic base types for all workflow hook domains
  • Created src/hooks/lifecycle.ts exporting runLifecycleHooks, loadHookModule, isValidJobHookPath, createTimeout — moved verbatim from src/cron/hooks.ts with cron-specific naming stripped
  • Updated CronHookEntry in src/config/types.cron.ts to extend LifecycleHookEntry (adds cron-specific filter field)
  • Refactored src/cron/hooks.ts to delegate to runLifecycleHooks and re-export runCronHooks / isValidJobHookPath / loadHookModule as back-compat aliases

Also fixed a pre-existing silent bug: resolveUserPath was being called with a 4th basePath arg it didn't accept (JS silently ignored it). The lifecycle module now correctly anchors relative script paths to basePath via path.join before resolution.

Test plan

  • pnpm build — clean
  • pnpm check — 0 warnings, 0 errors
  • pnpm test -- src/cron/hooks.test.ts — 25/25 passed

Authored by: cc (Claude Code) | 2026-03-15

Summary by CodeRabbit

  • New Features

    • Cron hooks can be filtered by workflow, job ID, job name, and agent ID.
  • Refactor

    • Hook execution moved to a unified lifecycle runner for consistent loading, timeouts, and abort behavior across hooks.
    • Cron hook declarations now reuse the shared lifecycle hook shape.
  • Bug Fixes

    • Improved hook loading, timeout enforcement, abort handling, and error reporting to reduce silent failures.

Arry8 and others added 18 commits March 15, 2026 04:54
Add a hook system that runs user-defined scripts at cron job lifecycle
points (beforeRun, afterComplete, onFailure, afterRun). Hooks are
configured via openclaw.json cron.hooks (global) and per-job hooks
in the job store. Supports priority ordering, job/agent filtering,
skipGlobal overrides, and abort capability on beforeRun hooks.
* fix(node-host): harden pnpm approval binding

* fix(node-host): harden perl approval binding

* docs(plugins): clarify workspace shadowing

* fix(ui): keep shared auth on insecure control-ui connects (openclaw#45088)

Merged via squash.

Prepared head SHA: 99eb3fd
Co-authored-by: velvet-shark <126378+velvet-shark@users.noreply.github.com>
Co-authored-by: velvet-shark <126378+velvet-shark@users.noreply.github.com>
Reviewed-by: @velvet-shark

* docs: add Claude Code token efficiency playbook

* docs: compress playbook for agent consumption

* docs: restructure playbook for long-context patterns

* pre-commit: add incremental repo-map updater

Auto-updates docs/repo-map.json exports and dependencies when
src/ or extensions/ .ts files are committed. Preserves existing
purpose fields, removes deleted files, adds new files with empty
purpose. Regex-based extraction, no dependencies.

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
Co-authored-by: Radek Sienkiewicz <mail@velvetshark.com>
Co-authored-by: velvet-shark <126378+velvet-shark@users.noreply.github.com>
- Enforce workflow filter in matchesFilter() instead of leaving it as
  a no-op. The workflow string is now threaded from loadHookEntries.
- Treat undefined job.agentId as non-matching when filter.agentId is
  set, preventing hooks from firing on unintended agent-less jobs.
- Clear setTimeout handle after hook completion via createTimeout()
  pattern, preventing stale timer accumulation.
- Add tests for all three fixes (14 total).
- runStartupCatchupCandidate now runs the full beforeRun/afterComplete/
  onFailure/afterRun hook lifecycle, matching runDueJob behavior.
  Previously, jobs replayed after a gateway restart silently bypassed
  all hooks, creating inconsistent auditing behavior.

- loadHookModule checks path.isAbsolute() before the URL-scheme regex
  so Windows drive-letter paths (e.g. C:\hooks\audit.cjs) are treated
  as file paths rather than URL specifiers.
- Security: validate per-job hook paths (reject absolute + traversal)
- Add configurable timeoutMs per hook entry (default 10s)
- Distinguish error vs warn log level (runtime throw vs module-not-found)
- Resolve hook scripts from OC home (storePath parent) not process.cwd()
- Wire lifecycle hooks into finishPreparedManualRun (ops.ts)
- Add hooks field to TypeBox CronJobSchema (fixes gateway rejection)
- Add jobName filter for case-insensitive substring matching
- Document lifecycle ordering on CronLifecycleHookPoint type
- Add Node-only comment on inlineHook test helper
- 7 new tests (path traversal, jobName filter, isValidJobHookPath)
Add optional `base` parameter to resolveUserPath so config-derived paths
can resolve against a stable root (e.g. OC home) instead of process.cwd().
Hook module loading now uses this instead of manual path.resolve.
When a beforeRun hook aborted a manual run via finishPreparedManualRun,
the early return skipped the locked cleanup that clears runningAtMs.
This left the job stuck as "running" until STUCK_RUN_MS timed out.

Now the abort path applies a "skipped" result and clears state under
lock, matching the timer-path behavior.
Track .claude/CLAUDE.md via gitignore exception. Fix pre-commit filter
to exclude dot-directory files that oxfmt silently skips.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 83710df6-9526-433c-afe7-51815d8710d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key(s) in object: 'tools'
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

Introduces a shared lifecycle hook runner and types: moves hook loading/execution into src/hooks/lifecycle.ts, adds LifecycleHookEntry/LifecycleHookRunResult types, updates cron types to extend the shared entry, and makes src/cron/hooks.ts delegate to and re-export lifecycle utilities. (≤50 words)

Changes

Cohort / File(s) Summary
Shared lifecycle
src/hooks/lifecycle.ts
New module implementing runLifecycleHooks, loadHookModule, isValidJobHookPath, createTimeout, and LifecycleLogger; handles per-hook timeouts, abort semantics, module loading, and logging.
Hook types
src/config/types.hooks.ts, src/config/types.cron.ts
Adds LifecycleHookEntry and LifecycleHookRunResult; updates CronHookEntry to extend LifecycleHookEntry and adds optional filter (workflow?, jobId?, jobName?, agentId?).
Cron hooks adapter
src/cron/hooks.ts
Reworks cron hook execution to delegate to runLifecycleHooks; re-exports isValidJobHookPath and loadHookModule; exposes CronHookRunResult as an alias for back-compat and preserves entry filtering/sorting.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides a clear summary of changes, includes a test plan with passing results, and mentions a bug fix, but is missing several required template sections like Change Type checkboxes, Scope, User-visible Changes, Security Impact, and Verification steps. Complete the PR description by filling in all template sections, particularly Security Impact and Human Verification which are marked required.
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the primary change: extracting shared lifecycle hook functionality into a new module.
Linked Issues check ✅ Passed The PR successfully implements all acceptance criteria from issue #10: exports required functions, adds LifecycleHookEntry type, creates back-compat aliases, passes all existing tests, and completes the implementation plan.
Out of Scope Changes check ✅ Passed All changes are scoped to issue #10 requirements: adding shared types, creating the lifecycle module, refactoring cron hooks, and fixing a pre-existing bug with basePath handling. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/10-lifecycle-hooks-shared-module
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/cron/hooks.ts (1)

26-30: Prefer a true alias for CronHookRunResult to avoid type drift.

CronHookRunResult currently duplicates the lifecycle result shape. Alias it directly to the shared type so future changes stay in sync automatically.

♻️ Proposed refactor
 import type { CronConfig, CronHookEntry, CronLifecycleHookPoint } from "../config/types.cron.js";
+import type { LifecycleHookRunResult } from "../config/types.hooks.js";
 import { isValidJobHookPath, loadHookModule, runLifecycleHooks } from "../hooks/lifecycle.js";

 // Back-compat alias: CronHookRunResult is the same shape as LifecycleHookRunResult.
-export type CronHookRunResult = {
-  aborted: boolean;
-  reason?: string;
-};
+export type CronHookRunResult = LifecycleHookRunResult;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cron/hooks.ts` around lines 26 - 30, Replace the duplicated shape for
CronHookRunResult with a true alias to the shared type so they stay in sync:
change the declaration of CronHookRunResult to alias LifecycleHookRunResult
(i.e., export type CronHookRunResult = LifecycleHookRunResult) instead of
redefining aborted and reason fields; remove the duplicated property list so
future changes to LifecycleHookRunResult automatically apply to
CronHookRunResult.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/hooks/lifecycle.ts`:
- Around line 118-127: isValidJobHookPath currently allows URL-scheme specifiers
(e.g. data:, http:) which can be imported by loadHookModule and bypass the
intended local-file restriction; update isValidJobHookPath to also reject any
input that begins with a URI scheme (pattern like /^[A-Za-z][A-Za-z0-9+.-]*:/)
and reject paths containing backslashes or other separators before a scheme
check so scheme-prefixed strings (e.g. data:, http:, file:) are refused; keep
the existing absolute/path-traversal checks and ensure loadHookModule only
receives paths that pass isValidJobHookPath.

---

Nitpick comments:
In `@src/cron/hooks.ts`:
- Around line 26-30: Replace the duplicated shape for CronHookRunResult with a
true alias to the shared type so they stay in sync: change the declaration of
CronHookRunResult to alias LifecycleHookRunResult (i.e., export type
CronHookRunResult = LifecycleHookRunResult) instead of redefining aborted and
reason fields; remove the duplicated property list so future changes to
LifecycleHookRunResult automatically apply to CronHookRunResult.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9a8797f0-2ce2-433b-8463-48f510cb557f

📥 Commits

Reviewing files that changed from the base of the PR and between b0d8654 and bddc9d1.

⛔ Files ignored due to path filters (1)
  • docs/repo-map.json is excluded by !docs/repo-map.json
📒 Files selected for processing (4)
  • src/config/types.cron.ts
  • src/config/types.hooks.ts
  • src/cron/hooks.ts
  • src/hooks/lifecycle.ts

Comment thread src/hooks/lifecycle.ts
@Arry8
Copy link
Copy Markdown
Owner Author

Arry8 commented Mar 15, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Arry8
Copy link
Copy Markdown
Owner Author

Arry8 commented Mar 15, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

✅ Actions performed

Full review triggered.

@Arry8
Copy link
Copy Markdown
Owner Author

Arry8 commented Mar 15, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/hooks/lifecycle.ts (1)

69-77: Keep the original Error in the log payload.

String(err) drops the stack and code, which makes shared hook failures much harder to diagnose.

🔎 Proposed refinement
       const logFn = isModuleError ? ctx.log.warn : ctx.log.error;
       logFn.call(
         ctx.log,
-        { hookPoint, script: entry.script, err: String(err) },
+        {
+          hookPoint,
+          script: entry.script,
+          err: err instanceof Error ? err : String(err),
+        },
         "lifecycle hook: script failed, continuing",
       );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/lifecycle.ts` around lines 69 - 77, The log payload is converting
the caught error to String(err), which loses stack and code; in the catch block
around isModuleError and the log call that uses ctx.log.warn / ctx.log.error,
pass the original Error object (e.g., err) in the payload instead of String(err)
so the logger receives the full Error (stack, code, etc.), while keeping the
same fields hookPoint and entry.script and the existing message "lifecycle hook:
script failed, continuing".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/hooks/lifecycle.ts`:
- Around line 3-5: Add a re-export for the LifecycleHookRunResult type from the
shared lifecycle entry so consumers can import it from the lifecycle module;
specifically, in the lifecycle module add an export like: export type {
LifecycleHookRunResult } from "../config/types.hooks.js" (or the equivalent
re-export syntax) so code that currently imports LifecycleHookRunResult from
elsewhere can instead import it from the lifecycle entry alongside
LifecycleHookEntry and other exports.

---

Nitpick comments:
In `@src/hooks/lifecycle.ts`:
- Around line 69-77: The log payload is converting the caught error to
String(err), which loses stack and code; in the catch block around isModuleError
and the log call that uses ctx.log.warn / ctx.log.error, pass the original Error
object (e.g., err) in the payload instead of String(err) so the logger receives
the full Error (stack, code, etc.), while keeping the same fields hookPoint and
entry.script and the existing message "lifecycle hook: script failed,
continuing".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 27d5dabb-d854-4e72-a28f-a1cb2293d2e2

📥 Commits

Reviewing files that changed from the base of the PR and between b0d8654 and 90653b0.

⛔ Files ignored due to path filters (1)
  • docs/repo-map.json is excluded by !docs/repo-map.json
📒 Files selected for processing (4)
  • src/config/types.cron.ts
  • src/config/types.hooks.ts
  • src/cron/hooks.ts
  • src/hooks/lifecycle.ts

Comment thread src/hooks/lifecycle.ts
@Arry8
Copy link
Copy Markdown
Owner Author

Arry8 commented Mar 15, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/hooks/lifecycle.ts (1)

3-5: ⚠️ Potential issue | 🟠 Major

Re-export LifecycleHookRunResult from the shared entry point.

This extracted module still keeps the result type as a private import, so consumers have to split imports between src/hooks/lifecycle.ts and src/config/types.hooks.ts. That misses the intended public surface for the shared lifecycle API.

🔧 Proposed fix
 import path from "node:path";
 import type { LifecycleHookEntry, LifecycleHookRunResult } from "../config/types.hooks.js";
+export type { LifecycleHookRunResult } from "../config/types.hooks.js";
 import { resolveUserPath } from "../utils.js";
 import { importFileModule, resolveFunctionModuleExport } from "./module-loader.js";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/lifecycle.ts` around lines 3 - 5, Export the lifecycle result type
from the public hooks entry so consumers don't need to import from the private
config file: remove or stop relying on the private-only import of
LifecycleHookRunResult in this module and add a re-export like "export type {
LifecycleHookRunResult } from '../config/types.hooks.js'" (or add it to the
shared public barrel) so that LifecycleHookRunResult is available from the
shared lifecycle entry alongside LifecycleHookEntry; update imports in this file
to use the public re-export instead of directly importing the type from the
config types module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/hooks/lifecycle.ts`:
- Around line 85-91: The type-guard isAbortResult currently treats any truthy
abort as an actual abort, so update it to only return true when the abort
property strictly equals boolean true; in function isAbortResult change the
check that inspects (value as { abort: unknown }).abort from a Boolean(...)
coercion to a strict === true comparison and keep the other object/type checks
intact so only { abort: true } (and optional reason) will short-circuit the
lifecycle.

---

Duplicate comments:
In `@src/hooks/lifecycle.ts`:
- Around line 3-5: Export the lifecycle result type from the public hooks entry
so consumers don't need to import from the private config file: remove or stop
relying on the private-only import of LifecycleHookRunResult in this module and
add a re-export like "export type { LifecycleHookRunResult } from
'../config/types.hooks.js'" (or add it to the shared public barrel) so that
LifecycleHookRunResult is available from the shared lifecycle entry alongside
LifecycleHookEntry; update imports in this file to use the public re-export
instead of directly importing the type from the config types module.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cab89644-def3-4360-9a9f-544b314c9a04

📥 Commits

Reviewing files that changed from the base of the PR and between b0d8654 and 59c1c25.

⛔ Files ignored due to path filters (1)
  • docs/repo-map.json is excluded by !docs/repo-map.json
📒 Files selected for processing (4)
  • src/config/types.cron.ts
  • src/config/types.hooks.ts
  • src/cron/hooks.ts
  • src/hooks/lifecycle.ts

Comment thread src/hooks/lifecycle.ts
@Arry8 Arry8 marked this pull request as ready for review March 15, 2026 12:32
@Arry8 Arry8 force-pushed the main branch 3 times, most recently from 073d284 to 90b44b8 Compare March 29, 2026 20:29
Arry8 pushed a commit that referenced this pull request Apr 5, 2026
* feat: add QQ Bot channel extension

* fix(qqbot): add setupWizard to runtime plugin for onboard re-entry

* fix: fix review

* fix: fix review

* chore: sync lockfile and config-docs baseline for qqbot extension

* refactor: 移除图床服务器相关代码

* fix

* docs: 新增 QQ Bot 插件文档并修正链接路径

* refactor: remove credential backup functionality and update setup logic

- Deleted the credential backup module to streamline the codebase.
- Updated the setup surface to handle client secrets more robustly, allowing for configured secret inputs.
- Simplified slash commands by removing unused hot upgrade compatibility checks and related functions.
- Adjusted types to use SecretInput for client secrets in QQBot configuration.
- Modified bundled plugin metadata to allow additional properties in the config schema.

* feat: 添加本地媒体路径解析功能,修正 QQBot 媒体路径处理

* feat: 添加本地媒体路径解析功能,修正 QQBot 媒体路径处理

* feat: remove qqbot-media and qqbot-remind skills, add tests for config and setup

- Deleted the qqbot-media and qqbot-remind skills documentation files.
- Added unit tests for qqbot configuration and setup processes, ensuring proper handling of SecretRef-backed credentials and account configurations.
- Implemented tests for local media path remapping, verifying correct resolution of media file paths.
- Removed obsolete channel and remind tools, streamlining the codebase.

* feat: 更新 QQBot 配置模式,添加音频格式和账户定义

* feat: 添加 QQBot 频道管理和定时提醒技能,更新媒体路径解析功能

* fix

* feat: 添加 /bot-upgrade 指令以查看 QQBot 插件升级指引

* feat: update reminder and qq channel skills

* feat: 更新remind工具投递目标地址格式

* feat: Refactor QQBot payload handling and improve code documentation

- Simplified and clarified the structure of payload interfaces for Cron reminders and media messages.
- Enhanced the parsing function to provide clearer error messages and improved validation.
- Updated platform utility functions for better cross-platform compatibility and clearer documentation.
- Improved text parsing utilities for better readability and consistency in emoji representation.
- Optimized upload cache management with clearer comments and reduced redundancy.
- Integrated QQBot plugin into the bundled channel plugins and updated metadata for installation.

* OK apps/macos/Sources/OpenClaw/HostEnvSecurityPolicy.generated.swift

> openclaw@2026.3.26 check:bundled-channel-config-metadata /Users/yuehuali/code/PR/openclaw
> node --import tsx scripts/generate-bundled-channel-config-metadata.ts --check

[bundled-channel-config-metadata] stale generated output at src/config/bundled-channel-config-metadata.generated.ts
 ELIFECYCLE  Command failed with exit code 1.
 ELIFECYCLE  Command failed with exit code 1.

* feat: 添加 QQBot 渠道配置及相关账户设置

* fix(qqbot): resolve 14 high-priority bugs from PR openclaw#52986 review

DM routing (7 fixes):
- #1: DM slash-command replies use sendDmMessage(guildId) instead of sendC2CMessage(senderId)
- #2: DM qualifiedTarget uses qqbot:dm:${guildId} instead of qqbot:c2c:${senderId}
- #3: sendTextChunks adds DM branch
- #4: sendMarkdownReply adds DM branch for text and Base64 images
- #5: parseAndSendMediaTags maps DM to targetType:dm + guildId
- #6: sendTextToTarget DM branch uses sendDmMessage; MessageTarget adds guildId field
- #7: handleImage/Audio/Video/FilePayload add DM branches

Other high-priority fixes:
- #8: Fix sendC2CVoiceMessage/sendGroupVoiceMessage parameter misalignment
- #9: broadcastMessage uses groupOpenid instead of member_openid for group users
- #10: Unify KnownUser storage - proactive.ts delegates to known-users.ts
- #11: Remove invalid recordKnownUser calls for guild/DM users
- #12: sendGroupMessage uses sendAndNotify to trigger onMessageSent hook
- #13: sendPhoto channel unsupported returns error field
- #14: sendTextAfterMedia adds channel and dm branches

Type fixes:
- DeliverEventContext adds guildId field
- MediaTargetContext.targetType adds dm variant
- sendPlainTextReply imgMediaTarget adds DM branch

* fix(qqbot): resolve 2 blockers + 7 medium-priority bugs from PR openclaw#52986 review

Blocker-1: Remove unused dmPolicy config knob
- dmPolicy was declared in schema/types/plugin.json but never consumed at runtime
- Removed from config-schema.ts, types.ts, and openclaw.plugin.json
- allowFrom remains active (already wired into framework command-auth)

Blocker-2: Gate sensitive slash commands with allowFrom authorization
- SlashCommand interface adds requireAuth?: boolean
- SlashCommandContext adds commandAuthorized: boolean
- /bot-logs set to requireAuth: true (reads local log files)
- matchSlashCommand rejects unauthorized senders for requireAuth commands
- trySlashCommandOrEnqueue computes commandAuthorized from allowFrom config

Medium-priority fixes:
- #15: Strip non-HTTP/non-local markdown image tags to prevent path leakage
- #16: applyQQBotAccountConfig clears clientSecret when setting clientSecretFile and vice versa
- #17: getAdminMarkerFile sanitizes accountId to prevent path traversal
- #18: URGENT_COMMANDS uses exact match instead of startsWith prefix match
- #19: isCronExpression validates each token starts with a cron-valid character
- #20: --token format validation rejects malformed input without colon separator
- #21: resolveDefaultQQBotAccountId checks QQBOT_APP_ID environment variable

* test(qqbot): add focused tests for slash command authorization path

- Unauthorized sender rejected for /bot-logs (requireAuth: true)
- Authorized sender allowed for /bot-logs
- Non-requireAuth commands (/bot-ping, /bot-help, /bot-version) work for all senders
- Unknown slash commands return null (passthrough)
- Non-slash messages return null
- Usage query (/bot-logs ?) also gated by auth check

* fix(qqbot): align global TTS fallback with framework config resolution

- Extract isGlobalTTSAvailable to utils/audio-convert.ts, mirroring core
  resolveTtsConfig logic: check auto !== 'off', fall back to legacy
  enabled boolean, default to off when neither is set.
- Add pre-check in reply-dispatcher before calling globalTextToSpeech to
  avoid unnecessary TTS calls and noisy error logs when TTS is not
  configured.
- Remove inline as any casts; use OpenClawConfig type throughout.
- Refactor handleAudioPayload into flat early-return structure with
  unified send path (plugin TTS → global fallback → send).

* fix(qqbot): break ESM circular dependency causing multi-account startup crash

The bundled gateway chunk had a circular static import on the channel
chunk (gateway -> outbound-deliver -> channel, while channel dynamically
imports gateway). When two accounts start concurrently via Promise.all,
the first dynamic import triggers module graph evaluation; the circular
reference causes api exports (including runDiagnostics) to resolve as
undefined before the module finishes evaluating.

Fix: extract chunkText and TEXT_CHUNK_LIMIT from channel.ts into a new
text-utils.ts leaf module. outbound-deliver.ts now imports from
text-utils.ts, breaking the cycle. channel.ts re-exports for backward
compatibility.

* fix(qqbot): serialize gateway module import to prevent multi-account startup race

When multiple accounts start concurrently via Promise.all, each calls
await import('./gateway.js') independently. Due to ESM circular
dependencies in the bundled output, the first import can resolve
transitive exports as undefined before module evaluation completes.

Fix: cache the dynamic import promise in a module-level variable so all
concurrent startAccount calls share the same import, ensuring the
gateway module is fully evaluated before any account uses it.

* refactor(qqbot): remove startup greeting logic

Remove getStartupGreetingPlan and related startup greeting delivery:
- Delete startup-greeting.ts (greeting plan, marker persistence)
- Delete admin-resolver.ts (admin resolution, greeting dispatch)
- Remove startup greeting calls from gateway READY/RESUMED handlers
- Remove isFirstReadyGlobal flag and adminCtx

* fix(qqbot): skip octal escape decoding for Windows local paths

Windows paths like C:\Users\1\file.txt contain backslash-digit sequences
that were incorrectly matched as octal escape sequences and decoded,
corrupting the file path. Detect Windows local paths (drive letter or UNC
prefix) and skip the octal decoding step for them.

* fix bot issue

* feat: 支持 TTS 自动开关并清理配置中的 clientSecretFile

* docs: 添加 QQBot 配置和消息处理的设计说明

* rebase

* fix(qqbot): align slash-command auth with shared command-auth model

Route requireAuth:true slash commands (e.g. /bot-logs) through the
framework's api.registerCommand() so resolveCommandAuthorization()
applies commands.allowFrom.qqbot precedence and qqbot: prefix
normalization before any handler runs.

- slash-commands.ts: registerCommand() now auto-routes by requireAuth
  into two maps (commands / frameworkCommands); getFrameworkCommands()
  exports the auth-required set for framework registration; bot-help
  lists both maps
- index.ts: registerFull() iterates getFrameworkCommands() and calls
  api.registerCommand() for each; handler derives msgType from ctx.from,
  sends file attachments via sendDocument, supports multi-account via
  ctx.accountId
- gateway.ts (inbound): replace raw allowFrom string comparison with
  qqbotPlugin.config.formatAllowFrom() to strip qqbot: prefix and
  uppercase before matching event.senderId
- gateway.ts (pre-dispatch): remove stale auth computation; commandAuthorized
  is true (requireAuth:true commands never reach matchSlashCommand)
- command-auth.test.ts: add regression tests for qqbot: prefix
  normalization in the inbound commandAuthorized computation
- slash-commands.test.ts: update /bot-logs tests to expect null
  (command routed to framework, not in local registry)

* rebase and solve conflict

* fix(qqbot): preserve mixed env setup credentials

---------

Co-authored-by: yuehuali <yuehuali@tencent.com>
Co-authored-by: walli <walli@tencent.com>
Co-authored-by: WideLee <limkuan24@gmail.com>
Co-authored-by: Frank Yang <frank.ekn@gmail.com>
@Arry8 Arry8 force-pushed the main branch 2 times, most recently from bbbcf3e to 1ce87cd Compare April 11, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement]: hooks: extract shared lifecycle hook runner into src/hooks/lifecycle.ts

1 participant