Repository navigation
fix(workers): attach global sessions to the owning agent - #139216
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 10:37 AM ET / 14:37 UTC. ClawSweeper reviewWhat this changesPreserves the recorded agent owner when workers resolve global sessions, removes an unnecessary inference fallback, and adds transcript ownership regression coverage. Merge readiness✅ Ready for maintainer review This remains a useful, focused fix: current main and the latest release retain the faulty lookup. No actionable introduced defect was found, and the captured runtime evidence supports the repair. Likely related people: steipete (medium-confidence routing candidate). Priority: P2 Review scores
Verification
How this fits togetherThe Gateway attaches sessions to remote workers while retaining control of inference and conversation history. Its session resolver connects a worker’s session ID to the owning agent and canonical transcript store. flowchart TD
A[Worker session ID] --> B[Combined session view]
B --> C[Select session and recorded owner]
C --> D[Fresh canonical store lookup]
D --> E{Session ID still matches?}
E -->|Yes| F[Agent inference and transcript updates]
E -->|No| G[Reject unavailable session]
Before mergeNone. Agent review detailsSecurityNone. PR surfaceSource -1, Tests +60. Total +59 across 3 files. View PR surface stats
Review metricsNone. Technical reviewBest possible solution: Keep worker resolution on the existing canonical lookup while carrying the selected row’s agent owner through inference, transcript writes, and notifications. Do we have a high-confidence way to reproduce the issue? Yes: current main drops the selected global row’s owner before canonical lookup, and the captured baseline records the corresponding dispatch failure. No code or tests were executed during this read-only review. Is this the best way to solve the issue? Yes: carrying the existing ownership fact into the shared resolver fixes its consumers without changing configuration, persistence formats, or worker authorization. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 37aaf073776c. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (5 earlier review cycles)
|
79c7de3 to
7f6d80d
Compare
|
@clawsweeper re-review The exact head now includes behavior-level proof through the real worker transcript committer: it selects the non-default agent's global session, commits through the production path, and verifies transcript persistence. The PR body records the exact passing Actions job and the unrelated flaky shard. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Added exact parent/head connected-runtime proof to the PR body. The clean fork Actions run uses the production worker WebSocket handler, a real WorkerConnection, persisted environment/placement stores, the real authority gate, and the transcript committer. Parent: worker ready, session-not-attached, 0 persisted entries. PR head: one admitted worker, one worker.transcript.commit frame accepted, one entry persisted to work/global. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Expose the resolved session target's existing agent owner guarantee and remove inference's unreachable default-agent fallback. Verify global session commits preserve the owning transcript and publication scope. Co-authored-by: ben.li <li.yang6@xydigit.com>
|
Merged via squash.
|
…9216) * fix(workers): retain global session ownership * fix(workers): preserve canonical session owners Expose the resolved session target's existing agent owner guarantee and remove inference's unreachable default-agent fallback. Verify global session commits preserve the owning transcript and publication scope. Co-authored-by: ben.li <li.yang6@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: ben.li <li.yang6@xydigit.com>
What Problem This Solves
Fixes an issue where dispatching a non-default agent's global session to a worker failed with
Attached session target is unavailable, even though the session worked locally. Other worker activity could likewise treat that existing session as unattached.Why This Change Was Made
The combined session view already records the logical agent that owns each row. The worker resolver now carries that owner into the existing fresh canonical store lookup instead of trying to recover it from the reserved
globalkey. The resolved type exposes its existing required owner, allowing inference to remove an unreachable default-agent fallback.The same canonical store read, session-ID validation, and worker authority checks remain in place. Production changes total +11/−12 lines; regression coverage adds 60 lines. Thanks @ly85206559 for the original repair and regression.
User Impact
Worker attachment, inference, live events, transcript commits, and run lookup can retain the selected global session's owning agent. Worker output appears in that agent's history without entering another agent's conversation.
Evidence
76fb700f1322bc5d8d03ec6f9a63d0d637668aa7. A real Gateway and paired node were ready, and the explicit-agent local control turn succeeded. Dispatch then failed with exactlyAttached session target is unavailable; teardown reported zero failures. The focused real-committer regression also failed at its intended assertion.cdbdb27da0c55580fa0dfeba69893127a4f974c0found no actionable issues. The same three file hashes are preserved after merging protected main0ebb320906da7e1727934b3cf816cedf98270357to acquire fix(ci): preflight job outputs exceed 1 MiB on full-plan pull requests #140018's CI output-limit repair. Required upstream CI 34039432075 passed on that exact head, including preflight and every selected product job.The minimal synthetic provider does not implement embeddings, and the explicit test fleet has no ambient dreaming-job owner; their nonfatal background diagnostics occurred in both runs. Neither diagnostic caused the attachment failure or prevented successful local and fixed-worker turns.