AI-assisted: Gateway hook event routing respects target agent session by vincentkoc · Pull Request #39046 · openclaw/openclaw

vincentkoc · 2026-03-07T17:29:14Z

Summary

Problem: Hook completion and error system events were always enqueued to the default agent main session, even when the hook ran against another agent.
Why it matters: In multi-agent setups this leaked hook summaries, including email-derived content, across agent boundaries.
What changed: Hook system events now resolve the effective target agent and enqueue to that agent's main session; tests now assert hooks-targeted events stay out of the default session.
What did NOT change (scope boundary): Hook execution, delivery behavior, and wake-hook routing were not otherwise changed.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Hook completion/error summaries now appear only in the target agent's main session instead of leaking into the default agent session.

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? (Yes)
If any Yes, explain risk + mitigation:
This reduces data exposure by scoping hook-generated system events to the intended agent session. No broader data access was added.

Repro + Verification

Environment

OS: macOS
Runtime/container: Node 22 / pnpm workspace
Model/provider: N/A
Integration/channel (if any): Hook gateway endpoints
Relevant config (redacted): multi-agent config with default main agent and hooks agent

Steps

Configure hooks with a target agentId of hooks.
Trigger /hooks/agent with a non-delivered hook run.
Inspect queued system events for agent:main:main and agent:hooks:main.

Expected

The hook summary is queued only in the target agent's main session.

Actual

Before this change it was queued in the default agent's main session.

Evidence

Attach at least one:

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

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: explicit agentId: "hooks", agent-scoped request sessionKey, allowed-agent enforcement, wake hooks still landing in the default main session.
Edge cases checked: fallback to default agent for unknown target agent IDs.
What you did not verify: live Gmail hook traffic against a real multi-agent deployment.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert the two commits in this PR.
Files/config to restore: src/gateway/server/hooks.ts and src/gateway/server.hooks.test.ts
Known bad symptoms reviewers should watch for: hook summaries not appearing in the target agent session, or wake hooks no longer reaching the default main session.

Risks and Mitigations

Risk: Hook runs that infer the effective agent from session key could misroute if the session key parsing changes elsewhere.
- Mitigation: regression coverage now checks both explicit-agent and agent-scoped session-key flows.

AI-assisted: yes. Fully tested locally for the targeted hook suite and validated with pnpm build.

clawsweeper · 2026-05-19T18:50:06Z

Codex review: found issues before merge. Reviewed July 5, 2026, 5:24 AM ET / 09:24 UTC.

Summary
The PR branch reroutes gateway hook completion and error system events toward the hook target agent session and updates gateway hook tests around target-agent queues.

PR surface: Source +15, Tests +17. Total +32 across 2 files.

Reproducibility: yes. for the PR defect by source comparison: this branch infers the hook event target from sessionKey when agentId is omitted, while current main tests enforce omitted agentId as the default target. The original explicit-agent leak is no longer present in current main or v2026.6.11.

Review metrics: 1 noteworthy metric.

Agent target fallback: 1 added. The PR adds one sessionKey-derived fallback for hook event targeting, which changes which authority chooses the target agent when agentId is omitted.

Root-cause cluster
Relationship: superseded
Canonical: #73228
Summary: This protected draft and the merged replacement address the same hook completion/error cross-agent event leak; the open wake-hook PR is adjacent but targets a different hook path.

Members:

canonical: security(gateway): route hook completion events to target agent session #73228 - Merged replacement implements the central hook completion/error target-agent routing fix and closed the linked security report.
fixed_by_candidate: Security: Hook completion events leak cross-agent email content to default agent session #24693 - The issue reports the exact cross-agent hook completion event leak fixed by the merged replacement PR.
superseded: security(gateway): route hook completion events to the target agent's session #68667 - Closed source PR proposed the same central fix and was explicitly credited by the merged replacement.
adjacent_distinct: fix: route wake hooks to explicit sessions #70268 - Open wake-hook session targeting work touches nearby hook routing but targets wake events rather than agent hook completion/error events.
adjacent_distinct: Hook/cron lanes resolve auth from hardcoded 'main' agent path, ignoring default agent #24016 - Closed auth/session-scope report involved a related cross-agent hook lane but a different auth-context failure mode.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Have a maintainer close the protected draft as superseded by the merged replacement, or explicitly request a refresh for a unique remaining gap.
If refreshed, remove the sessionKey-derived target fallback and preserve current main's omitted-agent/default-target behavior.

Risk before merge

[P1] If this branch is revived as-is, omitted-agent hook requests can influence the event target through an agent-prefixed sessionKey, conflicting with current main's explicit/default target policy on a security-sensitive cross-agent boundary.
[P1] Because this is a draft, member-authored, maintainer-labeled PR, automated cleanup should not close it even though the central fix is shipped.

Maintainer options:

Retire the superseded protected draft (recommended)
A maintainer can close this draft after confirming the merged replacement remains canonical and there is no unique remaining current-main gap.
Refresh before merge
If maintainers want to keep the branch, update it against current main and drop the sessionKey-derived target fallback.
Accept a new routing policy intentionally
If sessionKey-targeted hook events are desired, own that as a separate compatibility/security policy change with explicit tests.

Next step before merge

[P2] Protected/member-authored draft cleanup requires maintainer choice; there is no safe autonomous repair lane while the central fix is already shipped and the remaining branch behavior changes target policy.

Maintainer decision needed

Question: Should this protected draft be closed as superseded by security(gateway): route hook completion events to target agent session #73228, or refreshed as a separate hook-targeting policy change?
Rationale: The central bug is already fixed and shipped, but the current PR is member-authored and maintainer-labeled, and its remaining sessionKey fallback changes a security-sensitive routing policy.
Likely owner: vincentkoc — They authored this draft and the merged replacement PR, and are assigned to this protected item.
Options:
- Close superseded draft (recommended): Close this PR after confirming the merged replacement remains the canonical fix and no unique current-main gap is being preserved here.
- Refresh against current policy: Rebase the branch and remove the sessionKey-derived fallback so omitted agentId continues to use the default target policy.
- Sponsor sessionKey targeting separately: Treat sessionKey-based hook target selection as a new explicit policy change with allowlist and security tests rather than incidental bug-fix behavior.

Security
Needs attention: The intended hardening is valid, but the PR's sessionKey-derived target fallback creates a concrete cross-agent routing concern before any refresh or merge.

Review findings

[P1] Preserve explicit hook-agent target policy — src/gateway/server/hooks.ts:76-80

Review details

Best possible solution:

Keep the behavior from #73228 and v2026.6.11 as canonical; a maintainer should either close this protected draft as superseded or refresh it without the sessionKey-derived target fallback.

Do we have a high-confidence way to reproduce the issue?

Yes for the PR defect by source comparison: this branch infers the hook event target from sessionKey when agentId is omitted, while current main tests enforce omitted agentId as the default target. The original explicit-agent leak is no longer present in current main or v2026.6.11.

Is this the best way to solve the issue?

No for merging this branch as-is. The central fix is already represented by the merged replacement, and the remaining sessionKey fallback should either be dropped to match current policy or explicitly sponsored as a separate policy change.

Full review comments:

[P1] Preserve explicit hook-agent target policy — src/gateway/server/hooks.ts:76-80
This branch falls back from an omitted agentId to resolveSessionAgentId(sessionKey). Current main treats omitted agentId as the default target and tests an agent:hooks request session key being rebound to the default target, so letting request sessionKey choose the event target can route hook summaries across the wrong agent boundary. Still unfixed from the prior ClawSweeper review.
Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 153ee2abbaff; fix evidence: release v2026.6.11, commit 6f38425e5c18.

Label changes

Label justifications:

P3: The central leak is already fixed and shipped, leaving protected draft cleanup plus a narrow policy conflict rather than an urgent active regression.
merge-risk: 🚨 compatibility: Reviving this branch could change current behavior for omitted-agent hook requests that provide agent-prefixed session keys.
merge-risk: 🚨 security-boundary: The changed path decides which agent session receives hook-derived summaries, including potentially sensitive hook payload content.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate is not applied because this PR is member-authored and maintainer-labeled; the PR body still records targeted local hook-suite verification.

Evidence reviewed

PR surface:

Source +15, Tests +17. Total +32 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	19	4	+15
Tests	1	24	7	+17
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	43	11	+32

Security concerns:

[medium] Session key can choose hook event target — src/gateway/server/hooks.ts:76
When agentId is omitted, the PR derives the system-event target from sessionKey; for externally supplied hook payloads this can make session-key input influence which agent receives hook summaries, contrary to current main's explicit/default target policy.
Confidence: 0.86

What I checked:

repository-policy: Root AGENTS.md and src/gateway/AGENTS.md were read fully; the protected-label/member-authored cleanup rule and gateway review depth requirements affect this review. (AGENTS.md:1, 153ee2abbaff)
live-pr-state: Live PR metadata shows this PR is open, draft, assigned to vincentkoc, and labeled maintainer, P3, compatibility risk, security-boundary risk, and waiting on author; the provided GitHub context identifies the author association as MEMBER. (913a1f61f77b)
pr-head-session-target-fallback: The PR head derives targetAgentId from resolveSessionAgentId({ sessionKey, config: cfg }) when agentId is omitted, allowing the request sessionKey to influence which agent receives hook summary events. (src/gateway/server/hooks.ts:76, 913a1f61f77b)
current-main-routing-source: Current main resolves hook event session keys from the explicit hook agentId only and falls back to the main session when agentId is omitted, preserving explicit/default target policy. (src/gateway/server/hooks.ts:33, 153ee2abbaff)
current-main-policy-test: Current main tests that an omitted agentId with an agent-prefixed request sessionKey still targets the default main queue and rebinds dispatch to the default agent rather than the request session's agent. (src/gateway/server.hooks.test.ts:902, 153ee2abbaff)
merged-replacement-provenance: Merged replacement security(gateway): route hook completion events to target agent session #73228 implemented the central hook completion/error target-agent routing fix and closed the linked security report. (src/gateway/server/hooks.ts:105, 6f38425e5c18)

Likely related people:

vincentkoc: Authored this protected draft and authored/merged the replacement PR that shipped the same central hook completion/error routing fix. (role: merged replacement author and likely follow-up owner; confidence: high; commits: 913a1f61f77b, 6f38425e5c18; files: src/gateway/server/hooks.ts, src/gateway/server.hooks.test.ts, src/gateway/server/hooks.agent-trust.test.ts)
bluesky6868: Authored the earlier closed source PR for the same cross-agent hook completion/error event leak, which the merged replacement explicitly credited. (role: source implementation author; confidence: medium; commits: df65e0df281f; files: src/gateway/server/hooks.ts, src/gateway/server.hooks.test.ts, src/gateway/server/hooks.agent-trust.test.ts)
steipete: Recent GitHub commit history shows repeated maintenance and refactors in the gateway hook server and tests after the replacement landed. (role: recent area contributor; confidence: medium; commits: f24ae91842d6, 326db5822920, c0fe7ab34ab8; files: src/gateway/server/hooks.ts, src/gateway/server.hooks.test.ts, src/gateway/server/hooks.agent-trust.test.ts)
eleqtrizit: Recent history includes default hook agent allowlist work in the same gateway hook policy/test surface. (role: adjacent hook policy contributor; confidence: medium; commits: e72621e5668e; files: src/gateway/server.hooks.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (4 earlier review cycles)

reviewed 2026-07-03T04:46:40.483Z sha 913a1f6 :: needs maintainer review before merge. :: none
reviewed 2026-07-04T15:13:03.353Z sha 913a1f6 :: found issues before merge. :: [P1] Preserve the explicit hook agent target policy
reviewed 2026-07-04T18:14:39.279Z sha 913a1f6 :: found issues before merge. :: [P1] Preserve the explicit hook agent target policy
reviewed 2026-07-05T00:33:46.005Z sha 913a1f6 :: found issues before merge. :: [P1] Preserve explicit hook-agent target policy

clawsweeper · 2026-05-19T23:15:59Z

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

steipete · 2026-07-06T00:59:55Z

Closing as superseded by #73228, merged in 6f38425e5c18ac9773e0802b56ab7e7302838411.

That canonical change routes hook completion and error events to the target agent main session in src/gateway/server/hooks.ts, keeps the default-session fallback, and adds both explicit-agent isolation coverage in src/gateway/server.hooks.test.ts and agent-trust coverage in src/gateway/server/hooks.agent-trust.test.ts. I compared this branch's two-file patch with the landed implementation and current main; there is no remaining unique behavior here worth replaying.

Thanks @vincentkoc for identifying and implementing the isolation fix. The maintained version is now #73228.

vincentkoc added 2 commits March 7, 2026 09:28

Gateway: route hook system events to target agent

87d9d06

Tests: cover hook event agent isolation

913a1f6

openclaw-barnacle Bot added the gateway Gateway runtime label Mar 7, 2026

vincentkoc self-assigned this Mar 7, 2026

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Mar 7, 2026

clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label May 19, 2026

clawsweeper Bot mentioned this pull request Jun 23, 2026

fix: route wake hooks to explicit sessions #70268

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

AI-assisted: Gateway hook event routing respects target agent session#39046

AI-assisted: Gateway hook event routing respects target agent session#39046
vincentkoc wants to merge 2 commits into
mainfrom
vincentkoc-code/fix-hook-system-events-routing

vincentkoc commented Mar 7, 2026

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 19, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

vincentkoc commented Mar 7, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hatch command

Uh oh!

steipete commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented May 19, 2026 •

edited

Loading

clawsweeper Bot commented May 19, 2026 •

edited

Loading