fix(process): refuse to start a second session process for one id - #32
Open
xepozz wants to merge 1 commit into
Open
fix(process): refuse to start a second session process for one id#32xepozz wants to merge 1 commit into
xepozz wants to merge 1 commit into
Conversation
process.registry.register returns whether the name was taken, and the session process discarded it. When two spawns race for one session id both processes run: each opens its own writer, both append to the same conversation, and the one that lost the registration keeps serving a name that no longer routes to it, so its upstream updates go nowhere. Callers cannot close this themselves. wippy.session.process:plugin checks only its own in-memory map before spawning, so a second caller — another plugin instance, a channel bridge, an ensure-then-spawn path — has no way to see the first. The name is the only authority, and it is already being claimed here; this just stops ignoring the answer. Fails the start instead of continuing, matching how the surrounding code handles a session it cannot open.
xepozz
force-pushed
the
fix/session-registration-race
branch
from
August 24, 2026 15:52
c5a17ed to
7799083
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while tracing why a channel conversation was not appearing live in a Kickside UI. Not the cause of that, but a real race on the way.
The defect
src/process/session.luaclaims the session name and throws the answer away:process.registry.registerreports whether the name was already taken. Discarding that means two spawns for one session id both proceed. Each opens its ownwriter, both append to the same conversation, and the process that lost the registration keeps serving a name that no longer routes to it — so everything it sends upstream goes nowhere, silently.Why the caller cannot fix it
src/process/plugin.luachecks only its own in-memory map before spawning. A second caller — another plugin instance, a channel bridge, or anyensure-then-spawn path — has no way to observe the first. The registered name is the only authority, and this line is already claiming it.The change
Ten lines: keep the result, and fail the start when the name is taken. That matches how the surrounding code already treats a session it cannot open (
Failed to open session,Cannot open failed session).Verification
make lintreports the same 22 pre-existing errors and 1 warning before and after the change, none of them inwippy.session.process:session. I could not runmake testlocally — the harness stops atfailed to expand changeset: verified dependency evidence is unavailable during offline startup— so CI is the first real run.Happy to add a regression test if you can point me at how to spawn two processes for one id inside the harness; nothing under
test/currently exercises the session process directly.