feat(cli): add agent-id and session-key to system event wake#57199
feat(cli): add agent-id and session-key to system event wake#57199janicduplessis wants to merge 1 commit into
Conversation
Adds --agent-id and --session-key to openclaw system event, threading both through the wake request validation and cron wake handling so a system event can target a specific agent session. - CLI: --agent-id <agentId> and --session-key <sessionKey> options - Schema: WakeParamsSchema gains optional agentId + sessionKey - Cron handler: extracts and forwards both to cron.wake() - Timer: passes agentId/sessionKey to enqueueSystemEvent + requestHeartbeatNow - Tests: coverage for both parameters
Greptile SummaryThis PR extends Key observations:
Confidence Score: 5/5Safe to merge; only a minor schema consistency suggestion remains. The change is narrowly scoped and all new parameters are optional — no existing behavior is broken. The only finding is a P2 style suggestion to use NonEmptyString for sessionKey in the schema to match agentId and prevent silent no-ops for direct API callers. No logic errors, data loss, or security concerns were identified. src/gateway/protocol/schema/agent.ts — minor schema type inconsistency for sessionKey.
|
| Filename | Overview |
|---|---|
| src/cli/system-cli.ts | Adds --agent-id and --session-key options with whitespace normalization; correct camelCase mapping from Commander.js. |
| src/gateway/protocol/schema/agent.ts | WakeParamsSchema gains optional agentId (NonEmptyString) and sessionKey (plain String); minor inconsistency in that sessionKey allows empty strings. |
| src/gateway/server-methods/cron.ts | Cron wake handler extracts and forwards agentId/sessionKey from validated params correctly. |
| src/cron/service/timer.ts | wake() passes agentId/sessionKey to both enqueueSystemEvent and requestHeartbeatNow via falsy guards. |
| src/cron/service.ts | CronService.wake() signature extended with optional agentId/sessionKey, forwarded to ops.wakeNow. |
| src/cron/service/ops.ts | wakeNow() signature extended to accept and pass through agentId/sessionKey. |
| src/cli/system-cli.test.ts | Tests cover --agent-id alone and --agent-id + --session-key together; CLI layer forwarding verified. |
Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/gateway/protocol/schema/agent.ts
Line: 138
Comment:
**Schema inconsistency: `sessionKey` accepts empty strings**
`agentId` correctly uses `NonEmptyString` to reject blank values at the schema layer, but `sessionKey` uses plain `Type.String()` which allows `""`. An empty string passes validation and then reaches `timer.ts`, where it is silently discarded by the falsy guard (`opts.sessionKey ? ...`). For direct API callers who bypass the CLI this means the parameter is accepted without error but has no effect.
For consistency, consider using `NonEmptyString` here as well, the same way `agentId` is declared on the line above.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(cli): add agent-id and session-key ..." | Re-trigger Greptile
| mode: Type.Union([Type.Literal("now"), Type.Literal("next-heartbeat")]), | ||
| text: NonEmptyString, | ||
| agentId: Type.Optional(NonEmptyString), | ||
| sessionKey: Type.Optional(Type.String()), |
There was a problem hiding this comment.
sessionKey accepts empty strings
agentId correctly uses NonEmptyString to reject blank values at the schema layer, but sessionKey uses plain Type.String() which allows "". An empty string passes validation and then reaches timer.ts, where it is silently discarded by the falsy guard (opts.sessionKey ? ...). For direct API callers who bypass the CLI this means the parameter is accepted without error but has no effect.
For consistency, consider using NonEmptyString here as well, the same way agentId is declared on the line above.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/protocol/schema/agent.ts
Line: 138
Comment:
**Schema inconsistency: `sessionKey` accepts empty strings**
`agentId` correctly uses `NonEmptyString` to reject blank values at the schema layer, but `sessionKey` uses plain `Type.String()` which allows `""`. An empty string passes validation and then reaches `timer.ts`, where it is silently discarded by the falsy guard (`opts.sessionKey ? ...`). For direct API callers who bypass the CLI this means the parameter is accepted without error but has no effect.
For consistency, consider using `NonEmptyString` here as well, the same way `agentId` is declared on the line above.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa9a68af7b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| state.deps.enqueueSystemEvent(text, { | ||
| ...(opts.agentId ? { agentId: opts.agentId } : {}), | ||
| ...(opts.sessionKey ? { sessionKey: opts.sessionKey } : {}), |
There was a problem hiding this comment.
Derive agent from session key before enqueueing wake event
Forwarding sessionKey to enqueueSystemEvent without an agentId breaks the new --session-key-only flow: in buildGatewayCronService (src/gateway/server-cron.ts), enqueue resolution uses resolveCronAgent(opts?.agentId) and defaults to the main agent when agentId is missing, so a call like openclaw system event --session-key agent:coding:... queues the event into agent:main:* while requestHeartbeatNow may wake coding. This can route wake/event to different sessions and lose the intended target unless --agent-id is always provided.
Useful? React with 👍 / 👎.
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open. The requested CLI Canonical path: Close this PR as superseded by #83738. So I’m closing this here and keeping the remaining discussion on #83738. Review detailsBest possible solution: Close this PR as superseded by #83738. Do we have a high-confidence way to reproduce the issue? Yes, at source level. Current main exposes Is this the best way to solve the issue? No. The direction is useful, but this exact branch is not the best merge shape because it is stale against current Security review: Security review cleared: The diff changes CLI parsing, wake protocol plumbing, cron wake forwarding, and tests only; it does not add dependencies, workflows, install scripts, release paths, or secret handling. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 7bc4a333aa09. |
The cron `wake` MCP tool currently forwards only `{mode, text}` to the
gateway. Every wake then enqueues a system event with no sessionKey /
agentId, so the cron service falls back to the heartbeat / main
default. Wakes scheduled from a non-main session (Telegram thread,
Discord channel, multi-agent setup) silently route to the wrong
conversation lane — and on CLI runtimes the woken session burns
tokens generating output that no caller can route back.
Origin capture (closes the upstream half of openclaw#46886 and openclaw#64556):
- `src/agents/tools/cron-tool.ts` — the `wake` case now resolves
`opts.agentSessionKey` through `resolveInternalSessionKey` and
`resolveSessionAgentId`, matching the existing `add` action at
L569-583. Explicit `sessionKey` / `agentId` params on the tool
call take precedence over the inferred values so cross-session
wakes remain expressible.
- `src/gateway/protocol/schema/agent.ts` — `WakeParamsSchema` now
declares optional `sessionKey` and `agentId` (NonEmptyString)
so the gateway-level validator types them explicitly. The
schema's `additionalProperties: true` continues to accept
forward-compat metadata unchanged.
- `src/gateway/server-methods/cron.ts` — the wake handler reads
both fields, trims them, and forwards to `context.cron.wake`.
- `src/cron/service.ts` + `src/cron/service/ops.ts` +
`src/cron/service/timer.ts` — `wake()` accepts optional
`sessionKey`/`agentId` and threads them into
`enqueueSystemEvent` and `requestHeartbeatNow`. Missing /
whitespace-only fields fall through to the dep's default so
pre-existing call sites with no origin keep behaving the same
way (backwards compatible).
Tests:
- `src/cron/service/wake-origin.test.ts` (new, 6 tests) —
direct seams on `wake()`: origin forwarded on `mode: "now"`,
queued on `mode: "next-heartbeat"`, default fallback when
origin omitted, whitespace-only origin treated as omitted,
empty text still rejected.
- `src/gateway/protocol/index.test.ts` — extends
`validateWakeParams` coverage with the new optional fields
accepted + empty-string rejected.
Out of scope (deliberate split):
- Channel/thread/topic capture on the job's `delivery` block —
follow-up PR once this contract lands. The minimum-viable fix
here is session routing, which unblocks the CLI runtime's
`--resume <session>` path and the embedded session-resolution
path without a schema rewrite of the delivery contract.
- The 5 stalled wake-related PRs (openclaw#70268, openclaw#57199, openclaw#82767,
openclaw#79869, openclaw#63096) each fix downstream specifics. This PR fixes
the upstream origin-capture they all silently assume.
The cron `wake` MCP tool currently forwards only `{mode, text}` to the
gateway. Every wake then enqueues a system event with no sessionKey /
agentId, so the cron service falls back to the heartbeat / main
default. Wakes scheduled from a non-main session (Telegram thread,
Discord channel, multi-agent setup) silently route to the wrong
conversation lane — and on CLI runtimes the woken session burns
tokens generating output that no caller can route back.
Origin capture (closes the upstream half of openclaw#46886 and openclaw#64556):
- `src/agents/tools/cron-tool.ts` — the `wake` case now resolves
`opts.agentSessionKey` through `resolveInternalSessionKey` and
`resolveSessionAgentId`, matching the existing `add` action at
L569-583. Explicit `sessionKey` / `agentId` params on the tool
call take precedence over the inferred values so cross-session
wakes remain expressible.
- `src/gateway/protocol/schema/agent.ts` — `WakeParamsSchema` now
declares optional `sessionKey` and `agentId` (NonEmptyString)
so the gateway-level validator types them explicitly. The
schema's `additionalProperties: true` continues to accept
forward-compat metadata unchanged.
- `src/gateway/server-methods/cron.ts` — the wake handler reads
both fields, trims them, and forwards to `context.cron.wake`.
- `src/cron/service.ts` + `src/cron/service/ops.ts` +
`src/cron/service/timer.ts` — `wake()` accepts optional
`sessionKey`/`agentId` and threads them into
`enqueueSystemEvent` and `requestHeartbeatNow`. Missing /
whitespace-only fields fall through to the dep's default so
pre-existing call sites with no origin keep behaving the same
way (backwards compatible).
Tests:
- `src/cron/service/wake-origin.test.ts` (new, 6 tests) —
direct seams on `wake()`: origin forwarded on `mode: "now"`,
queued on `mode: "next-heartbeat"`, default fallback when
origin omitted, whitespace-only origin treated as omitted,
empty text still rejected.
- `src/gateway/protocol/index.test.ts` — extends
`validateWakeParams` coverage with the new optional fields
accepted + empty-string rejected.
Out of scope (deliberate split):
- Channel/thread/topic capture on the job's `delivery` block —
follow-up PR once this contract lands. The minimum-viable fix
here is session routing, which unblocks the CLI runtime's
`--resume <session>` path and the embedded session-resolution
path without a schema rewrite of the delivery contract.
- The 5 stalled wake-related PRs (openclaw#70268, openclaw#57199, openclaw#82767,
openclaw#79869, openclaw#63096) each fix downstream specifics. This PR fixes
the upstream origin-capture they all silently assume.
The cron `wake` MCP tool currently forwards only `{mode, text}` to the
gateway. Every wake then enqueues a system event with no sessionKey /
agentId, so the cron service falls back to the heartbeat / main
default. Wakes scheduled from a non-main session (Telegram thread,
Discord channel, multi-agent setup) silently route to the wrong
conversation lane — and on CLI runtimes the woken session burns
tokens generating output that no caller can route back.
Origin capture (closes the upstream half of openclaw#46886 and openclaw#64556):
- `src/agents/tools/cron-tool.ts` — the `wake` case now resolves
`opts.agentSessionKey` through `resolveInternalSessionKey` and
`resolveSessionAgentId`, matching the existing `add` action at
L569-583. Explicit `sessionKey` / `agentId` params on the tool
call take precedence over the inferred values so cross-session
wakes remain expressible.
- `src/gateway/protocol/schema/agent.ts` — `WakeParamsSchema` now
declares optional `sessionKey` and `agentId` (NonEmptyString)
so the gateway-level validator types them explicitly. The
schema's `additionalProperties: true` continues to accept
forward-compat metadata unchanged.
- `src/gateway/server-methods/cron.ts` — the wake handler reads
both fields, trims them, and forwards to `context.cron.wake`.
- `src/cron/service.ts` + `src/cron/service/ops.ts` +
`src/cron/service/timer.ts` — `wake()` accepts optional
`sessionKey`/`agentId` and threads them into
`enqueueSystemEvent` and `requestHeartbeatNow`. Missing /
whitespace-only fields fall through to the dep's default so
pre-existing call sites with no origin keep behaving the same
way (backwards compatible).
Tests:
- `src/cron/service/wake-origin.test.ts` (new, 6 tests) —
direct seams on `wake()`: origin forwarded on `mode: "now"`,
queued on `mode: "next-heartbeat"`, default fallback when
origin omitted, whitespace-only origin treated as omitted,
empty text still rejected.
- `src/gateway/protocol/index.test.ts` — extends
`validateWakeParams` coverage with the new optional fields
accepted + empty-string rejected.
Out of scope (deliberate split):
- Channel/thread/topic capture on the job's `delivery` block —
follow-up PR once this contract lands. The minimum-viable fix
here is session routing, which unblocks the CLI runtime's
`--resume <session>` path and the embedded session-resolution
path without a schema rewrite of the delivery contract.
- The 5 stalled wake-related PRs (openclaw#70268, openclaw#57199, openclaw#82767,
openclaw#79869, openclaw#63096) each fix downstream specifics. This PR fixes
the upstream origin-capture they all silently assume.
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
ClawSweeper applied the proposed close for this PR.
|
Summary
Adds
--agent-id <agentId>and--session-key <sessionKey>toopenclaw system event, threading both through the wake request validation and cron wake handling so a system event can target a specific agent and session.Why
On multi-agent setups,
openclaw system eventalways targets the default agent's main session. This makes it impossible to:codinginstead ofmain)With both params, commands like:
route the event and heartbeat wake to the correct agent and session.
Changes
--agent-id <agentId>and--session-key <sessionKey>options with whitespace normalizationWakeParamsSchemagains optionalagentId(NonEmptyString) andsessionKey(String)cron.wake()agentId/sessionKeyto bothenqueueSystemEventandrequestHeartbeatNow--agent-idand--session-keyforwardingHow it works
agentIddetermines which agent handles the event (workspace, model, heartbeat config)sessionKeydetermines which conversation session the event lands in--agent-idalone → agent's default main session--session-keyalone → agent inferred from the key prefixTesting
pnpm vitest run src/cli/system-cli.test.tspnpm vitest run src/cron/