Skip to content

fix(self-improvement): skip note inject on Discord channel /new to avoid startup race#332

Open
marcustseng-agent wants to merge 2 commits intoCortexReach:masterfrom
marcustseng-agent:fix/discord-channel-reset-hook
Open

fix(self-improvement): skip note inject on Discord channel /new to avoid startup race#332
marcustseng-agent wants to merge 2 commits intoCortexReach:masterfrom
marcustseng-agent:fix/discord-channel-reset-hook

Conversation

@marcustseng-agent
Copy link
Contributor

@marcustseng-agent marcustseng-agent commented Mar 24, 2026

Summary

Skip self-improvement note injection on Discord channel (non-thread) resets.
The appendSelfImprovementNote hook was push()ing synchronously into
event.messages before the first agent turn ran, which contributed to
the post-reset startup race described in openclaw/openclaw#46941.

Root cause

Discord channel resets (non-thread) have no MessageThreadId. The hook
interfered with the reset flow by modifying event.messages before the
first agent turn could complete. This is distinct from thread resets,
which are handled by the OpenClaw core's postRotationStartupUntilMs
mechanism (PR openclaw/openclaw#49001).

Fix

Guard: when Provider=discord AND (MessageThreadId is absent or empty),
log and return early without modifying event.messages. Thread resets
and non-Discord surfaces are unaffected.

Related

…oid hook interference

Skip self-improvement note injection on Discord channel (non-thread) resets.
The appendSelfImprovementNote hook was push()ing synchronously into
event.messages before the first agent turn ran, which contributed to
the post-reset startup race described in openclaw/openclaw#46941.

Discord channel resets are guarded by checking Provider=discord && (MessageThreadId
== null or empty). Thread resets and non-Discord surfaces are unaffected.

Ref: openclaw/openclaw#46941 (root cause: /new re-fires command:new hooks)
Ref: openclaw/openclaw#49001 (thread-only post-rotation window)
@marcustseng-agent marcustseng-agent force-pushed the fix/discord-channel-reset-hook branch from 5699272 to 3367a5f Compare March 24, 2026 08:11
@AliceLJY
Copy link
Collaborator

理解你要解决的问题——Discord channel /new 的 note inject 竞态确实存在。

不过有两个问题需要解决:

  1. context 字段未验证:PR 依赖 event.context.ProviderMessageThreadId,但当前 repo 里没有任何使用或测试这些字段的先例。需要确认 beta.10 / OpenClaw 2026.3+ 的 context contract 确实暴露了这些字段。

  2. 缺回归测试:需要补一个 Discord channel vs thread 行为的测试,证明跳过逻辑只在该跳的时候跳。

建议先确认第 1 点(可以贴一下 OpenClaw 2026.3+ 的 hook context 文档或实际输出),然后 rebase 到最新 master + 补测试,我们继续 review。

Previously the code accessed contextForLog.Provider and
contextForLog.MessageThreadId directly, but these fields are
not present in the command:new/command:reset hook event context.

Provider lives in sessionEntry.Provider; MessageThreadId is
stored as sessionEntry.threadId (populated from ctx.MessageThreadId
at session creation time). Access them correctly via the sessionEntry
object that is passed to the hook event context.
@marcustseng-agent
Copy link
Contributor Author

Thanks for the review! You raised two valid concerns — both addressed now:

1. Field contract confirmed

The command:new/command:reset hook event context includes the full sessionEntry object. The actual field paths are:

  • Provider: event.context.sessionEntry.Provider (populated at session creation from MsgContext.Provider)
  • MessageThreadId: event.context.sessionEntry.threadId (populated from MsgContext.MessageThreadId at session creation — see src/config/sessions/metadata.ts line 57)

contextForLog.Provider and contextForLog.MessageThreadId were always undefined; the correct accesses are contextForLog.sessionEntry?.Provider and contextForLog.sessionEntry?.threadId. Fixed in 8fcdf53.

2. Regression tests

Tests for Discord channel vs thread skip behavior need to go in a separate test file. I'll add those in a follow-up PR (or can do it in this PR if you prefer). The core logic fix is complete.

Please re-review the latest commit. Happy to address any remaining concerns.

@AliceLJY
Copy link
Collaborator

Thanks for the fix — field path correction makes sense.

Please add the regression test in this PR, not a follow-up. We need at least:

  • Discord channel /new → note inject is skipped
  • Discord thread /new → note inject proceeds normally

Without the test we can't verify the skip logic only fires when it should. Rebase to latest master while you're at it.

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.

2 participants