fix(kap-server): release session event resources - #1652
Conversation
🦋 Changeset detectedLatest commit: a41120b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@codex review |
commit: |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 962365123a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (const d of state.lifecycleDisposables) d.dispose(); | ||
| for (const d of state.agentDisposables.values()) d.dispose(); | ||
| await state.queue; | ||
| state.targets.clear(); |
There was a problem hiding this comment.
When a session is archived or closed while a WebSocket remains connected, this clears the broadcaster's target set without notifying WsConnectionV1, whose own subscriptions map is only updated by explicit unsubscribe/close. If that same session is later restored in the same socket lifetime, the connection still believes it is subscribed but the fresh broadcaster owner has no target for it, so subsequent session events are silently missed until the client reconnects or manually subscribes again. Either keep enough subscription state to reattach on restore, or send/remove the subscription explicitly before dropping these targets.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
6aaf8af to
a41120b
Compare
1 participant
Related Issue
No standalone issue. This is a focused resource-lifecycle follow-up requested during the long-running server audit. It is stacked on #1623 because that PR changes the same broadcaster tests and snapshot state.
Problem
The v1 WebSocket broadcaster kept every activated session state until server shutdown. A closed or archived session therefore retained its journal tail, targets, agent/session listeners, and journal writer. Concurrent first activation could also create two owners for the same session (or global stream), causing duplicate listeners, missed subscribers, and duplicate sequence advancement.
Lifecycle release also had persistence races: an initialization or core event already awaiting could publish or append after the old owner began closing, allowing an immediate restore to overlap the previous journal writer.
What changed
/api/v1/wscursor-continuity contract.This PR currently targets #1623's head branch (
8fae3d8fc) so its diff contains only this follow-up. It can be retargeted tomainafter #1623 merges.Validation
@moonshot-ai/kap-serversuite, single worker: 56 files / 657 tests.@moonshot-ai/kap-servertypecheck.@moonshot-ai/agent-core-v2lint:domain(848 files).git diff --check.pnpm changeset status --since=8fae3d8fc9e025437547841d80d000cecd07830c(CLI patch).3×8 diagnostic samples (not pass/fail RSS thresholds):
gen-docswas evaluated. This internal lifecycle fix adds no command, configuration, or user workflow to document; the skill's expected changelog sync helper is absent on the base branch, so this PR does not invent or modify documentation tooling.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.