Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

review: scope subdelegation control only

Browse Source
This commit is contained in:
Keisuke Hirata 2026-05-30 14:03:44 +09:00
parent 40ba8f5d73
commit 21c388e0d3
No known key found for this signature in database
2 changed files with 35 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
work-items/open/20260529-163047-pod-event-scope-subdelegation-control-only/item.md
View File

@ -7,7 +7,7 @@ kind: bug
priority: P2 priority: P2
labels: [pod, events, orchestration, context] labels: [pod, events, orchestration, context]
created_at: 2026-05-29T16:30:47Z created_at: 2026-05-29T16:30:47Z
updated_at: 2026-05-30T04:54:02Z updated_at: 2026-05-30T05:03:44Z
assignee: null assignee: null
legacy_ticket: null legacy_ticket: null
--- ---

34
work-items/open/20260529-163047-pod-event-scope-subdelegation-control-only/thread.md
View File

@ -46,4 +46,38 @@ Validation plan:
- Run `cargo test -p protocol pod_event`, `cargo test -p pod --test pod_events`, focused controller pod-event tests, and `cargo fmt --check`. - Run `cargo test -p protocol pod_event`, `cargo test -p pod --test pod_events`, focused controller pod-event tests, and `cargo fmt --check`.
---
<!-- event: review author: hare at: 2026-05-30T05:03:44Z status: approve -->
## Review: approve
Approve.
The implementation keeps `PodEvent::ScopeSubDelegated` on the typed IPC/control plane while removing it from the agent-visible notification/history/auto-run lane. The core change is an explicit `PodEvent::should_notify_agent()` classifier used by both controller event receive paths after side effects have already been applied.
Blocker findings: none.
Requirement coverage:
- `ScopeSubDelegated` side effects are still applied in both idle and running paths.
- Upward re-emission remains in `apply_event_side_effects`.
- `ScopeSubDelegated` no longer enters `NotifyBuffer`, does not append `SystemItem::PodEvent`, and does not auto-kick `RunForNotification`.
- `TurnEnded`, `Errored`, and `ShutDown` remain agent-visible.
- Wire serialization/protocol shape is unchanged.
- No new hidden history/context injection path was introduced.
Non-blocking follow-ups:
- Consider making misuse harder later by renaming/gating lower-level `push_pod_event_notify` / `NotifyBuffer::push_pod_event` APIs or adding debug assertions.
- Idle-path test does not directly assert registry side effect, but running-path and pod event side-effect tests cover it and the idle path calls the same side-effect function before gating.
Validation reviewed from coder report:
- `cargo fmt --check` — passed.
- `cargo test -p protocol pod_event` — passed.
- `cargo test -p pod --test pod_events_test` — passed.
- `cargo test -p pod --test controller_test pod_event` — passed.
- focused running-path tests for control-only and visible events — passed.
Final verdict: approve.
--- ---