Skip to content

[Feat] surface spawnedBy in chat and agent broadcast payloads#63244

Merged
frankekn merged 4 commits intoopenclaw:mainfrom
samzong:feat/spawned-by-enrichment
Apr 29, 2026
Merged

[Feat] surface spawnedBy in chat and agent broadcast payloads#63244
frankekn merged 4 commits intoopenclaw:mainfrom
samzong:feat/spawned-by-enrichment

Conversation

@samzong
Copy link
Copy Markdown
Contributor

@samzong samzong commented Apr 8, 2026

Summary

  • Problem: Subagent sessions emit chat and agent broadcast events with only sessionKey but no spawnedBy field. Clients cannot determine session parentage from the first event without an extra sessions.subscribe round-trip, creating a structural race condition.
  • Why it matters: Downstream clients (e.g. ClawWork Desktop) are forced to maintain a workaround to discover subagent ownership, adding complexity and a timing-dependent failure mode.
  • What changed: Added a cached resolveSpawnedBy() helper inside createAgentEventHandler and injected spawnedBy into all 6 broadcast sites that were missing it: emitChatDelta, flushBufferedChatDeltaIfNeeded, emitChatFinal (done + error), agent seq gap error, and non-tool agent events.
  • What did NOT change (scope boundary): buildSessionEventSnapshot paths (already include spawnedBy), companion spawn metadata fields (spawnedWorkspaceDir, spawnDepth, etc.), and downstream client adaptations.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

N/A — new feature, not a bug fix.

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/gateway/server-chat.agent-events.test.ts
  • Scenario the test should lock in: All 6 broadcast paths include spawnedBy for subagent sessions and omit it for non-subagent sessions; cache resolves once per sessionKey.
  • Why this is the smallest reliable guardrail: Tests mock loadGatewaySessionRow and verify payload shape at each broadcast call site.
  • Existing test that already covers this (if any): Existing tests cover spawnedBy in buildSessionEventSnapshot paths; new tests cover the 6 previously uncovered broadcast sites.
  • If no new test is added, why not: 8 new tests added.

User-visible / Behavior Changes

  • WebSocket chat events (delta, final, error) and non-tool agent events now include a spawnedBy field when the emitting session is a subagent. Non-subagent sessions are unchanged (field absent).

Diagram (if applicable)

Before:
[subagent event] -> { sessionKey, state, ... }  (no spawnedBy)
[client]         -> sessions.subscribe -> discover parent -> route

After:
[subagent event] -> { sessionKey, spawnedBy, state, ... }
[client]         -> route immediately by spawnedBy

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: Node 22+
  • Model/provider: N/A
  • Integration/channel: Gateway WebSocket

Steps

  1. Start gateway with a subagent ensemble
  2. Observe chat and agent WebSocket events from subagent sessions
  3. Verify spawnedBy field is present in payloads

Expected

  • All chat and non-tool agent broadcasts from subagent sessions include spawnedBy

Actual

  • Confirmed via unit tests (40/40 pass)

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

  • Verified scenarios: All 6 broadcast sites via 8 unit tests, including delta, flush, final-done, final-error, seq gap, non-tool agent, non-subagent omission, and cache single-lookup behavior.
  • Edge cases checked: Non-subagent sessions (field omitted), cache hit on repeated events, error path caching.
  • What you did not verify: Live gateway end-to-end with real subagent ensemble.

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes (purely additive optional field)
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

  • Risk: Negative cache permanently stores null for sessions where spawnedBy is not yet set at first lookup.
    • Mitigation: Verified via subagent-spawn.ts that spawnedBy PATCH completes before agent starts emitting events. The immutability invariant in sessions-patch.ts guarantees the cached value never becomes stale.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M labels Apr 8, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 8, 2026

Greptile Summary

Adds a resolveSpawnedBy() helper with a per-handler cache inside createAgentEventHandler and spreads spawnedBy into the 6 previously uncovered broadcast sites (emitChatDelta, flushBufferedChatDeltaIfNeeded, emitChatFinal done + error, seq-gap error, and the non-tool agentPayload). Eight new unit tests cover all added paths including subagent/non-subagent discrimination and single-lookup caching behavior.

Confidence Score: 5/5

Safe to merge — additive, backward-compatible change with 8 targeted tests covering all new broadcast paths.

All changes are additive (optional field, absent for non-subagent sessions), implementation is correct, caching logic properly handles the undefined/null distinction, and every modified broadcast site is covered by new tests. The acknowledged null-cache risk for sessions where the spawnedBy PATCH hasn't completed is mitigated by the stated ordering invariant in subagent-spawn.ts, and no P0/P1 issues were found.

No files require special attention.

Vulnerabilities

No security concerns identified. The change is purely additive — it reads an already-persisted spawnedBy field from an existing session row and surfaces it on existing WebSocket events. No new network calls, no secret/token handling, no auth-boundary changes.

Reviews (1): Last reviewed commit: "feat(gateway): surface spawnedBy in chat..." | Re-trigger Greptile

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 732bf30c36

ℹ️ 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".

Comment thread src/gateway/server-chat.ts Outdated
@samzong samzong force-pushed the feat/spawned-by-enrichment branch from 732bf30 to 4ea8a0b Compare April 10, 2026 03:09
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ea8a0b80f

ℹ️ 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".

Comment thread src/gateway/server-chat.ts Outdated
@samzong
Copy link
Copy Markdown
Contributor Author

samzong commented Apr 15, 2026

@codex /review

@chatgpt-codex-connector
Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@samzong
Copy link
Copy Markdown
Contributor Author

samzong commented Apr 15, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7753208275

ℹ️ 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".

Comment thread src/gateway/server-chat.ts Outdated
@samzong samzong force-pushed the feat/spawned-by-enrichment branch from 7753208 to 94540b6 Compare April 16, 2026 16:31
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 94540b6532

ℹ️ 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".

Comment thread src/gateway/server-chat.ts
@samzong samzong force-pushed the feat/spawned-by-enrichment branch from 94540b6 to 249eb88 Compare April 16, 2026 16:52
@samzong
Copy link
Copy Markdown
Contributor Author

samzong commented Apr 16, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 249eb885fa

ℹ️ 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".

Comment thread src/gateway/server-chat.ts
@samzong samzong force-pushed the feat/spawned-by-enrichment branch 2 times, most recently from 94540b6 to ef4f577 Compare April 16, 2026 17:11
@openclaw-barnacle openclaw-barnacle Bot added the app: web-ui App: web-ui label Apr 16, 2026
@samzong
Copy link
Copy Markdown
Contributor Author

samzong commented Apr 16, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef4f57788e

ℹ️ 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".

Comment thread src/gateway/server-chat.ts Outdated
@samzong samzong force-pushed the feat/spawned-by-enrichment branch from ef4f577 to c5a46e9 Compare April 27, 2026 16:54
@openclaw-barnacle openclaw-barnacle Bot added the app: macos App: macos label Apr 27, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 28, 2026

Codex review: needs changes before merge.

Keep this PR open. Current main still does not implement the requested optional spawnedBy field on live chat delta/final/error broadcasts or non-tool agent broadcasts, and the Gateway event schemas plus generated Swift models still omit that field. The PR remains the active implementation candidate, with provided context showing the latest head added the requested per-session cache follow-up and only touches Gateway runtime/tests/protocol schema/generated Swift files.

Required change before merge:

Keep this PR open and review or land the latest version if it adds optional spawnedBy to subagent/ACP live chat and non-tool agent event payloads, updates AgentEventSchema and ChatEventSchema, regenerates both Swift protocol model copies, and keeps focused tests for all broadcast paths plus non-subagent omission and per-session cache behavior.

Best possible solution:

Keep this PR open and review or land the latest version if it adds optional spawnedBy to subagent/ACP live chat and non-tool agent event payloads, updates AgentEventSchema and ChatEventSchema, regenerates both Swift protocol model copies, and keeps focused tests for all broadcast paths plus non-subagent omission and per-session cache behavior.

What I checked:

  • Live chat broadcasts omit spawnedBy on current main: emitChatDelta, flushBufferedChatDeltaIfNeeded, and emitChatFinal build chat payloads with runId, sessionKey, seq, state/message/error fields, but no spawnedBy. (src/gateway/server-chat.ts:674, 71473e7448f9)
  • Non-tool agent broadcasts and sequence-gap errors omit spawnedBy on current main: The regular non-tool agentPayload is eventForClients plus sessionKey; the sequence-gap error payload includes sessionKey but no spawnedBy. (src/gateway/server-chat.ts:846, 71473e7448f9)
  • Existing lineage support is limited to session/tool snapshot paths: buildSessionEventSnapshot already includes spawnedBy, and current tests assert this for session/tool snapshot broadcasts, matching the PR's stated scope boundary and showing the remaining gap is live plain chat/non-tool agent payloads. (src/gateway/server-chat.ts:462, 71473e7448f9)
  • Protocol schemas still reject the proposed field on current main: AgentEventSchema and ChatEventSchema both use additionalProperties: false and neither schema includes optional spawnedBy. (src/gateway/protocol/schema/agent.ts:27, 71473e7448f9)
  • Generated Swift models still omit spawnedBy for AgentEvent and ChatEvent on current main: The generated Swift AgentEvent and ChatEvent structs do not include a spawnedby property or spawnedBy coding key; existing spawnedby hits in the file belong to other session-related models. (apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift:383, 71473e7448f9)
  • PR/security review pass: Provided PR file list is limited to src/gateway/server-chat.ts, src/gateway/server-chat.agent-events.test.ts, Gateway protocol schema files, and generated Swift protocol models. It does not touch CI workflows, dependencies, lockfiles, package scripts, secret handling, release metadata, or downloaded/generated third-party code beyond protocol model regeneration. (73b8211ccb0a)

Likely related people:

  • Peter Steinberger: Local blame and git log -S for server-chat.ts, the Gateway event schemas, and generated Swift models point to commit 07104c80b3bc879647d171f8877b2b5e792253ca as the recent current-main code shape for the affected paths. (role: recent maintainer of current Gateway event/protocol files; confidence: medium; commits: 07104c80b3bc; files: src/gateway/server-chat.ts, src/gateway/protocol/schema/agent.ts, src/gateway/protocol/schema/logs-chat.ts)
  • frankekn: The latest maintainer review comment in the provided PR context identifies the remaining hot-path caching requirement and records targeted Gateway/protocol validation evidence, making them a practical routing candidate for the next review step. (role: reviewer and likely follow-up owner; confidence: medium; files: src/gateway/server-chat.ts, src/gateway/server-chat.agent-events.test.ts, src/gateway/protocol/schema/agent.ts)

Remaining risk / open question:

  • This is an additive Gateway wire-contract change, so runtime payloads, TypeBox schemas, validators, generated Swift models, and tests need to land together.
  • The PR discussion had a concrete hot-path performance concern around repeated loadGatewaySessionRow(sessionKey) calls; the provided latest head includes a cache commit, but it still needs normal maintainer review before merge.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 71473e7448f9.

@frankekn
Copy link
Copy Markdown
Member

Thanks for the focused protocol/test coverage. One issue before we prepare this: the current resolveSpawnedBy() helper does not cache.

For subagent/ACP sessions it calls loadGatewaySessionRow(sessionKey) on each chat delta/final/error and non-tool agent event, and finalization can invoke it twice via flush + final. Since this is the live event path, please cache positive and null results per sessionKey inside createAgentEventHandler, and add a regression proving repeated events for the same subagent session only load the row once.

Local review evidence:

  • scripts/pr review-tests 63244 src/gateway/server-chat.agent-events.test.ts passed
  • pnpm protocol:check passed

samzong and others added 4 commits April 29, 2026 20:47
Subagent sessions emitted chat and agent broadcast events with only
sessionKey. Clients had to follow up with sessions.subscribe to learn
the parent, creating a timing race.

Inject spawnedBy at the six previously uncovered broadcast sites
(emitChatDelta, flushBufferedChatDeltaIfNeeded, emitChatFinal done +
error, seq gap error, non-tool agent event). resolveSpawnedBy reads
from the session store directly. It short-circuits for session keys
that cannot carry lineage (mirrors supportsSpawnLineage in
sessions-patch.ts), so the hot chat delta path does not touch the
store for normal sessions.

No cache: spawnedBy is immutable once set, and the only frequent
caller is the subagent/acp hot path which is already filtered by the
lineage key check.

Signed-off-by: samzong <samzong.lu@gmail.com>
Signed-off-by: samzong <samzong.lu@gmail.com>
@frankekn frankekn force-pushed the feat/spawned-by-enrichment branch from 73b8211 to ff0fe5d Compare April 29, 2026 12:48
@frankekn frankekn merged commit 443ca48 into openclaw:main Apr 29, 2026
69 checks passed
@frankekn
Copy link
Copy Markdown
Member

Merged via squash.

Thanks @samzong!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants