Fix bindSession ConflictException crashing control-plane#21
Merged
ajit-zer07 merged 2 commits intomainfrom Apr 22, 2026
Merged
Conversation
RunManagerService.bindSession let ConflictException from transitionTo
propagate unchanged. SessionDiscoveryService fires bindSession (and its
sibling transitions markStarted / markRunning) as unawaited `void` calls,
so any race — e.g., RunExecutor also advancing the run, or a re-emitted
session.created — surfaced as an unhandled promise rejection and killed
the Node process.
- run-manager.service.ts: catch ConflictException in bindSession, log,
and return the current run. Side-effects (session upsert, event emit)
already fired on the first successful bind, so they are skipped.
- session-discovery.service.ts: sequence markStarted / bindSession /
markRunning with await inside a try/catch. Any remaining conflict is
logged; subscribeSession + streamConsumer.start still run so the
WatchSessions loop keeps observing subsequent sessions.
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.
Summary
RunManagerService.bindSessionagainst races where the run hasalready advanced past
binding_session(e.g.,SessionDiscoveryServiceand
RunExecutorServiceboth processing the same session). The methodnow catches
ConflictExceptionfromtransitionTo, logs a warning, andreturns the current run — avoiding duplicate event emission and session
upserts.
SessionDiscoveryService.handleSessionCreatedfrom firing threeunawaited
voidstate transitions. They are now awaited sequentiallyinside a try/catch, so a transition failure on one discovered session
no longer kills the
WatchSessionsloop for every other session.Why
Previously,
void this.runManager.bindSession(...)insession-discovery.service.ts:98would surface anyConflictExceptionfrom the state-machine guard (
run.repository.ts:102) as an unhandledpromise rejection — crashing the control-plane. This was observed from
the UIConsole as a control-plane crash during session binding.
Root cause:
transitionTothrowsConflictExceptionwhen the current status isnot a valid predecessor for the target (e.g., run already
running).voidcalls (markStarted,bindSession,markRunning) runconcurrently and out of order relative to the state machine.
Changes
src/runs/run-manager.service.tsConflictExceptionfrom@nestjs/common.bindSession, wrapmarkBindingSessionin try/catch. OnConflictException, fetch the current run, log a warning with thecurrent status, and return it. Throws
NotFoundExceptiononly if therun genuinely disappeared.
src/runs/session-discovery.service.tsvoidstate transitions with sequentialawaitsinside a try/catch.
subscribeSession+streamConsumer.startstill execute — observation is the wholepoint of session discovery — and the outer
WatchSessionsloopstays alive for other sessions.
Test plan
npx tsc --noEmit -p tsconfig.json— clean.npx jest src/runs/run-manager.service.spec.ts— 19 / 19 pass.npx jest src/runs/session-discovery.service.spec.ts— 9 / 9 pass.npx jest src/runs/run-executor.service.spec.ts— 24 / 24 pass.npx jest(full suite) — 593 / 593 pass across 46 suites.(RunExecutor + SessionDiscovery on the same sessionId), confirm no
crash and a single
session.boundevent is emitted.Out of scope / follow-ups
markStartedandmarkRunningstill throwConflictExceptiononracing callers. They are now caught by the new try/catch in
session-discovery.service.ts, but could be made idempotent inRunManagerServicefor consistency withmarkCompleted/markFailed/markCancelled.void this.streamConsumer.start(...)inhandleSessionCreatedisstill unawaited; it's long-running by design, but a
.catchhandlerwould make rejections observable rather than unhandled.