Skip to content

NOJIRA-Sandbox-install-reliability-sentinel-stt-fix - #1147

Merged
pchero merged 3 commits into
mainfrom
NOJIRA-Sandbox-install-reliability-sentinel-stt-fix
Jul 30, 2026
Merged

NOJIRA-Sandbox-install-reliability-sentinel-stt-fix#1147
pchero merged 3 commits into
mainfrom
NOJIRA-Sandbox-install-reliability-sentinel-stt-fix

Conversation

@pchero

@pchero pchero commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Fix two boot-crash bugs found while investigating why a customer's VoIPBin sandbox install failed: bin-call-manager and bin-timeline-manager unconditionally subscribe to a RabbitMQ exchange owned by bin-sentinel-manager and crash-loop when it's not deployed (true for any non-Kubernetes deployment, including sandbox), and bin-transcribe-manager crashes at boot when no STT provider credentials are configured. Both are now non-fatal. Full investigation and design in the sandbox repo: docs/plans/2026-07-30-sandbox-install-reliability-design.md.

  • bin-call-manager: Declare the sentinel-manager exchange (idempotent, matches sentinel-manager's own declaration) before subscribing to it, instead of crashing when the exchange doesn't exist
  • bin-timeline-manager: Same fix as bin-call-manager, since it has the identical sentinel-subscribe pattern
  • bin-call-manager: Add docs/architecture.md Event Subscriptions section documenting the fix
  • bin-timeline-manager: Add docs/architecture.md Event Subscriptions section documenting the fix
  • bin-transcribe-manager: NewStreamingHandler now returns a working disabled implementation instead of a bare nil when no STT provider is configured, so the service boots normally with only the streaming path degraded
  • bin-transcribe-manager: Remove the now-unreachable streamingHandler == nil fatal checks in both cmd/transcribe-manager/main.go and cmd/transcribe-control/main.go
  • bin-transcribe-manager: Disabled streaming handler returns a structured STT_NOT_CONFIGURED VoipbinError (matching this codebase's error convention) instead of a plain error, so API callers get a typed response instead of an opaque 500
  • bin-transcribe-manager: Update CLAUDE.md and docs/operations.md to describe the new degrade-instead-of-crash behavior

pchero added 3 commits July 30, 2026 12:50
call-manager and timeline-manager both include the sentinel-manager event
exchange in their subscribe-target lists, but sentinel-manager requires the
Kubernetes API and is only deployed in Kubernetes environments. In any other
deployment nothing declares that exchange, so QueueSubscribe's underlying
QueueBind fails with an AMQP 404, which closes the channel shared by all of the
queue's bindings and makes the service exit fatally at boot. Both services now
declare the exchange themselves immediately before that one target's bind, using
the same durable fanout parameters sentinel-manager's own notifyhandler uses, so
the declare is an idempotent no-op when sentinel-manager is deployed.

- bin-call-manager: Call sockHandler.TopicCreate for the sentinel target in
subscribeHandler.Run() before its QueueSubscribe; return the error on failure,
matching the existing subscribe-failure handling in the same loop
- bin-call-manager: Add subscribehandler Run() tests covering the sentinel
topic declare, the no-sentinel-target case, and the topic-create and
subscribe failure paths
- bin-call-manager: Document the event subscriptions and the sentinel
declare-before-bind behavior in docs/architecture.md
- bin-timeline-manager: Same guard in its Run() loop, comparing against
commonoutline.QueueNameSentinelEvent directly since its subscribeTargets is
[]commonoutline.QueueName
- bin-timeline-manager: Add tests asserting the sentinel topic is declared
before the sentinel subscribe and that a failed declare is returned; add the
TopicCreate expectation to the existing bind-ordering regression test
- bin-timeline-manager: Document the sentinel declare-before-bind behavior and
the pre-existing boot-order sensitivity of the other targets in
docs/architecture.md
transcribe-manager exited at boot whenever neither the GCP nor the AWS STT
client could be initialized: NewStreamingHandler returned a bare nil interface
and cmd/transcribe-manager turned that into a fatal error. That is a
disproportionate response, since every non-streaming capability of the service
works fine without an STT provider, and it is easy to hit accidentally (for
example a placeholder GCP key file and no AWS credentials). The constructor now
returns a disabled StreamingHandler instead, so the service boots and only the
streaming transcribe path reports the failure, per request.

- bin-transcribe-manager: Add pkg/streaminghandler/disabled.go with
NewDisabledStreamingHandler and the exported ErrSTTNotConfigured sentinel
("STT_NOT_CONFIGURED: no STT provider available"); Run() is a no-op so startup
completes, Start()/Stop() return ErrSTTNotConfigured
- bin-transcribe-manager: Return NewDisabledStreamingHandler() from
NewStreamingHandler when both providers are unavailable, instead of nil
- bin-transcribe-manager: Remove the now-unreachable streamingHandler == nil
fatal check from cmd/transcribe-manager/main.go
- bin-transcribe-manager: Add Test_NewDisabledStreamingHandler covering the
no-op Run() and the ErrSTTNotConfigured Start()/Stop() contract; relax
Test_NewStreamingHandler_NoProviders to the non-nil constructor contract, which
no longer depends on whether ambient GCP credentials happen to exist
- bin-transcribe-manager: Replace the "at least one provider must be configured
at startup" invariant in CLAUDE.md and docs/operations.md with the
degrade-instead-of-crash behavior, and add an STT_NOT_CONFIGURED row to the
failure-modes table
- bin-transcribe-manager: Remove the stale streamingHandler == nil fatal check
  left in transcribe-control (identical to the one already removed from
  transcribe-manager's main.go) - the constructor can no longer return nil
- bin-transcribe-manager: Replace the disabled streaming handler's plain
  errors.New sentinel with a structured cerrors.Unavailable VoipbinError
  (reason STT_NOT_CONFIGURED), matching this codebase's existing error
  convention so the API's errorResponse() translator returns a typed error
  instead of an opaque 500
@pchero
pchero merged commit 0e7c6c3 into main Jul 30, 2026
2 of 3 checks passed
@pchero
pchero deleted the NOJIRA-Sandbox-install-reliability-sentinel-stt-fix branch July 30, 2026 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant