fix(gateway): stabilize inter-session completion wake prompts#63096
fix(gateway): stabilize inter-session completion wake prompts#63096afurm wants to merge 4 commits into
Conversation
Greptile SummaryThis PR fixes a prompt-cache stability regression in the gateway The fix adds Confidence Score: 5/5Safe to merge — the fix is targeted, the logic is correct, and the two new seam tests lock in the intended behavior. No P0 or P1 issues found. The new No files require special attention.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9be0e8bc6
ℹ️ 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".
b9be0e8 to
7343d9c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7343d9c09e
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 993ab77153
ℹ️ 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".
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Keep open: current main does not implement the completion-wake prompt reconstruction and the linked cache-stability issue remains open, but this PR is not merge-ready because it is dirty against main, only partially mirrors the current prompt contract, and lacks real behavior proof. Canonical path: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. So I’m closing this here because the remaining work is already tracked in the canonical issue. Review detailsBest possible solution: Close this stale PR. The latest review rated it F, the branch still lacks merge-ready proof, and there has been no human follow-up after the durable review. Do we have a high-confidence way to reproduce the issue? Yes at source level: current gateway completion wakes can enter the agent path with inter-session task-completion events while normal turns assemble richer session-derived prompt context. I did not run a live Anthropic cache trace in this read-only review. Is this the best way to solve the issue? No, not as currently patched. The maintainable fix should reuse or exactly mirror current normal-turn prompt assembly instead of adding a partial parallel reconstruction, then prove provider-visible cache stability. Security review: Security review cleared: The diff is limited to gateway TypeScript prompt construction and tests, with no dependency, workflow, secret, package-resolution, install, or code-download changes. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against e0018382eb00. |
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?
|
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
Summary
Describe the problem and fix in 2–5 bullets:
task_completionwakes sent through the gatewayagentpath did not rebuild the session-backedInbound Context/Group Chat Contextsuffix that normal chat turns include.src/gateway/server-methods/agent.tsnow synthesizes persisted session context for inter-session completion wakes when no explicitextraSystemPromptis provided, and preserves explicit caller-provided prompt context when it is present.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
agentwithinputProvenance.kind="inter_session"andtask_completioninternal events, but unlike the normal chat path it did not reconstruct the session-derivedextraSystemPromptsections that containInbound ContextandGroup Chat Context.## Runtime, so missing them changes the system prompt digest even when session state and transcript history are otherwise unchanged.Regression Test Plan (if applicable)
src/gateway/server-methods/agent.test.tstask_completionwake for an existing session should rebuild persisted inbound/group prompt context whenextraSystemPromptis omitted, and should preserve explicitextraSystemPromptwhen one is provided.agentingress seam, where session metadata, provenance, and internal events are combined before the run is dispatched.does not create task rows for inter-session completion wakescovers adjacent routing behavior, but not prompt-context reconstruction.User-visible / Behavior Changes
Diagram (if applicable)