docs(openspec): propose the reconnect reload and session residency changes - #309
Merged
Merged
Conversation
…anges Two defects in the session layer, both measured against the tree rather than inferred, proposed as separate changes because one is a prerequisite of the other. **a-reconnect-reload-is-a-diff-not-a-wipe** — an AMI reconnect discards every tracked channel without telling anyone and re-adds the survivors as new. `VerbaraServer.OnReconnected` calls `Channels.Clear()`; `ChannelManager.Clear()` empties its dictionaries and raises no `ChannelRemoved`, so the session manager never learns the channels went away. The reload then re-adds survivors without passing `StatusEvent.LinkedId`, although the field exists and is populated. Measured with two tests written for the change and run against the unfixed code: a call that hung up during the outage leaves 1 active session still Connected with 0 CallEndedEvent, forever; a call whose two legs both survive becomes 3 active sessions. Affects both registration paths. One consequence decides the design and is recorded as a rejection: the reloaded legs land in Created, which is what the reconciliation sweep's 60-second orphan branch marks Failed — so registering that sweep where it does not run today would mark healthy calls dead after every reconnect. **a-call-that-ended-is-released-when-it-ends** — what the manager holds for a call is released only when another call completes, and the release can stop permanently. `EvictStaleCompleted` peeks the queue head and dequeues inside the loop body, so a head that names a session no longer held, or carries a null `CompletedAt`, exits the loop without being removed and nothing is ever released again. The duplicate enqueue that produces such a head is reachable: `OnSessionCompleted` enqueues unconditionally and is called even when both state transitions fail on an already-terminal session. `MaxCompletedSessions` is declared, defaulted and read by nothing. The default in-memory store holds the same object reference, so the manager's release frees a dictionary node and pins the rest. `BridgeManager` marks a destroyed bridge and never removes it. Both changes exclude, by requirement rather than by convention, any ageing of a call that is not terminal. Each carries a section recording what it does not fix and where that work belongs. Planning only: no production code is touched by this pull request.
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.
Two defects in the session layer, measured against the tree rather than inferred. Proposed as
separate changes because one is a prerequisite of the other.
Planning only — no production code is touched by this pull request.
1.
a-reconnect-reload-is-a-diff-not-a-wipe(MEDIANO, 16 tasks)An AMI reconnect discards every tracked channel without telling anyone, then re-adds the survivors as
if they had just appeared.
Statusreturns nothing)1 active session [linked=linked-001 state=Connected participants=2], 0CallEndedEventStatusreturns both, carryingLinkedid)Createdsession per legThe mechanism, read line by line:
VerbaraServer.OnReconnected(:122) callsChannels.Clear()(:130).ChannelManager.Clear()empties its two dictionaries and raises noChannelRemoved, so thesession manager's subscription hears nothing.
RequestInitialStateAsync(:158-168) re-adds survivors without passingStatusEvent.LinkedId,although that field exists (
StatusEvent.cs:19). Each leg defaults tolinkedId = uniqueId.This affects both registration paths, not just multi-server.
A rejection the measurement earns. The reloaded legs land in
Created, which is exactly whatSessionReconciliationService'sDialingTimeoutorphan branch marksFailed. Registering thatsweep where it does not run today would mark healthy calls dead after every reconnect. The
proposal records this rather than leaving the option open.
Ruled by the owner, 2026-09-24: a call ended because a reload proved it gone carries a marker
saying so and is attributed no hangup cause.
NotDefinedis not neutral downstream — a consumerclassifier treating anything other than
NormalClearingas abnormal would read every reconnect-lostcall as an abnormal hangup and act on it.
2.
a-call-that-ended-is-released-when-it-ends(MEDIANO, 19 tasks)What the manager holds for a call is released only when another call completes, and the release can
stop permanently.
A head that names a session no longer held, or carries a null
CompletedAt, exits the loop withoutbeing dequeued. It stays at the front for the life of the process and nothing is ever released
again.
The duplicate enqueue that produces such a head is reachable in code:
OnSessionCompletedenqueuesunconditionally on its first line and is called even when both state transitions fail on an
already-terminal session.
Also established:
MaxCompletedSessionsis declared, defaulted to 1000 and read by nothing; thedefault in-memory store holds the same object reference, so the manager's release frees a dictionary
node and pins the rest;
BridgeManagermarks a destroyed bridge and never removes it, soBridgeCountcounts every bridge ever created.Sequencing
The residency change edits the same method bodies as the reload change (
OnChannelRemoved,OnSessionCompleted,EvictStaleCompleted) and lands after it — its task 1.1 says to verifythat before starting. The reload change also reduces what the residency change must bound: calls
stranded in a connected state become terminal and therefore releasable.
What both changes exclude, by requirement
Any ageing or release of a call that is not terminal. That is a clock sweep over live calls; the
reload change rejects it with the measurement above, and the number that would make it arguable is
owed by another repository and unmeasured. Each change carries a final section recording what it does
not fix and where that work belongs — a deferred finding without a home is how the previous set was
lost.
Verification
openspec validate --all --strict— 15 passed, 0 failedpull request, because they belong to that change's first apply commit.
🤖 Generated with Claude Code