Fix Telegram topic-bound subagent session spawns#42707
Conversation
openclaw-barnacle
Bot
added
channel: telegram
Greptile SummaryThis PR wires up the missing Telegram subagent lifecycle hooks ( Key changes:
One test has an order-dependent initialization that could cause it to fail if run in isolation. Confidence Score: 5/5
Last reviewed commit: a0784d5 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("shares adapter and service state through globalThis", () => { | ||
| registerSessionBindingAdapter({ | ||
| channel: "discord", | ||
| accountId: "default", | ||
| capabilities: { | ||
| placements: ["current"], | ||
| }, | ||
| bind: async (input) => createRecord(input), | ||
| listBySession: () => [], | ||
| resolveByConversation: () => null, | ||
| unbind: async () => [], | ||
| }); | ||
|
|
||
| const globalAdapters = Reflect.get( | ||
| globalThis, | ||
| "__openclawSessionBindingAdaptersByChannelAccount", | ||
| ); | ||
| const globalService = Reflect.get(globalThis, "__openclawSessionBindingService"); | ||
|
|
||
| expect(globalAdapters).toBeInstanceOf(Map); | ||
| expect((globalAdapters as Map<string, unknown>).has("discord:default")).toBe(true); | ||
| expect(globalService).toBe(getSessionBindingService()); | ||
| }); |
There was a problem hiding this comment.
This test has a hidden order dependency. globalThis.__openclawSessionBindingService is only written the first time getSessionBindingService() is called (lazy initialization). The adapters map is written at module-load time, but the service is not.
If this test runs in isolation (e.g. via it.only, test randomization, or in a test run before other tests that invoke getSessionBindingService()), the globalService variable would be undefined, and the final assertion would fail:
expect(undefined).toBe(getSessionBindingService()) // fails
Initialize the service first to ensure globalThis is populated:
| it("shares adapter and service state through globalThis", () => { | |
| registerSessionBindingAdapter({ | |
| channel: "discord", | |
| accountId: "default", | |
| capabilities: { | |
| placements: ["current"], | |
| }, | |
| bind: async (input) => createRecord(input), | |
| listBySession: () => [], | |
| resolveByConversation: () => null, | |
| unbind: async () => [], | |
| }); | |
| const globalAdapters = Reflect.get( | |
| globalThis, | |
| "__openclawSessionBindingAdaptersByChannelAccount", | |
| ); | |
| const globalService = Reflect.get(globalThis, "__openclawSessionBindingService"); | |
| expect(globalAdapters).toBeInstanceOf(Map); | |
| expect((globalAdapters as Map<string, unknown>).has("discord:default")).toBe(true); | |
| expect(globalService).toBe(getSessionBindingService()); | |
| }); | |
| const service = getSessionBindingService(); // Ensure globalThis is populated | |
| const globalAdapters = Reflect.get( | |
| globalThis, | |
| "__openclawSessionBindingAdaptersByChannelAccount", | |
| ); | |
| const globalService = Reflect.get(globalThis, "__openclawSessionBindingService"); | |
| expect(globalAdapters).toBeInstanceOf(Map); | |
| expect((globalAdapters as Map<string, unknown>).has("discord:default")).toBe(true); | |
| expect(globalService).toBe(service); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/infra/outbound/session-binding-service.test.ts
Line: 202-224
Comment:
This test has a hidden order dependency. `globalThis.__openclawSessionBindingService` is only written the first time `getSessionBindingService()` is called (lazy initialization). The adapters map is written at module-load time, but the service is not.
If this test runs in isolation (e.g. via `it.only`, test randomization, or in a test run before other tests that invoke `getSessionBindingService()`), the `globalService` variable would be `undefined`, and the final assertion would fail:
```
expect(undefined).toBe(getSessionBindingService()) // fails
```
Initialize the service first to ensure globalThis is populated:
```suggestion
const service = getSessionBindingService(); // Ensure globalThis is populated
const globalAdapters = Reflect.get(
globalThis,
"__openclawSessionBindingAdaptersByChannelAccount",
);
const globalService = Reflect.get(globalThis, "__openclawSessionBindingService");
expect(globalAdapters).toBeInstanceOf(Map);
expect((globalAdapters as Map<string, unknown>).has("discord:default")).toBe(true);
expect(globalService).toBe(service);
```
How can I resolve this? If you propose a fix, please make it concise.![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0784d52f6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const target = rawTo.startsWith("channel:") | ||
| ? rawTo.slice(8) | ||
| : rawTo.startsWith("telegram:") | ||
| ? rawTo.slice(9) | ||
| : rawTo; |
There was a problem hiding this comment.
resolveTelegramConversationId only strips channel: and telegram:, so valid Telegram target forms like telegram:group:-100123:topic:55 (and tg: aliases) are converted into conversation IDs such as group:-100123:topic:55 instead of -100123:topic:55. That malformed ID then propagates into completion routing (e.g. channel:group:-100123), which the Telegram send path cannot resolve to a real chat, so thread-bound subagent session replies can be dropped for those supported target formats.
Useful? React with 👍 / 👎.
|
This pull request has been automatically marked as stale due to inactivity. |
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. This stale PR should close as superseded: current main still lacks the Telegram subagent hook implementation, but the same remaining work is now tracked by the canonical issue and a newer active PR that rewrites the approach around the current bundled-channel boundary. The useful global session-binding part is already present on main, while the old branch still has correctness findings and missing live Telegram proof. Canonical path: Keep #42986 as the tracker and review, land, or replace #82023 as the canonical current-boundary implementation with redacted live Telegram proof. So I’m closing this here and keeping the remaining discussion on #42986 and #82023. Review detailsBest possible solution: Keep #42986 as the tracker and review, land, or replace #82023 as the canonical current-boundary implementation with redacted live Telegram proof. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main still gates thread-bound native subagent sessions on subagent_spawning hooks, and Telegram registers no such hooks while its binding adapter exists. I did not run a live Telegram reproduction in this read-only review. Is this the best way to solve the issue? No. The old branch is not the best current solution because it uses the obsolete plugin.register/core-internal boundary and duplicates work now covered by the newer registerFull/per-kind-capability PR. Security review: Security review cleared: No concrete security or supply-chain issue was found; the diff adds no dependencies, workflows, lifecycle scripts, downloaded artifacts, or new secret storage. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 33685e14748b. |
clawsweeper
Bot
added
the
mantis: telegram-visible-proof
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
clawsweeper
Bot
added
mantis: telegram-visible-proof
1 participant
Summary
Fix Telegram topic-bound
sessions_spawn(..., thread=true, mode="session")for subagent sessions.Problem
Telegram thread-bound subagent spawns could fail with
Thread bindings are unavailable for telegram.orSession mode is unavailable for this target.because the Telegram extension was missing subagent lifecycle wiring, requester topic context could be lost onagent --session-idruns, and session-binding state could be split across module instances.Fix
Register Telegram subagent lifecycle hooks, hydrate requester context from persisted session delivery state, share session-binding adapters and services across module instances, and prewarm the Telegram thread-binding manager before capability checks.
Validation
pnpm exec vitest run extensions/telegram/index.test.ts extensions/telegram/src/subagent-hooks.test.tspnpm exec vitest run src/infra/outbound/session-binding-service.test.ts src/telegram/thread-bindings.test.tspnpm exec vitest run src/commands/agent.test.ts -t "hydrates telegram topic routing from persisted session delivery context on session resume"pnpm buildpnpm checkcurrently still fails on an unrelated repo error atsrc/cron/isolated-agent/run.ts(207,12):Cannot find name 'CommandLane'.