Skip to content

fix(chat): emit SSE error and skip complete on prompt setup failures#2102

Open
shaun0927 wants to merge 3 commits into
danielmiessler:mainfrom
shaun0927:fix-chat-sse-complete
Open

fix(chat): emit SSE error and skip complete on prompt setup failures#2102
shaun0927 wants to merge 3 commits into
danielmiessler:mainfrom
shaun0927:fix-chat-sse-complete

Conversation

@shaun0927
Copy link
Copy Markdown

Summary

This PR fixes a REST /chat SSE contract bug where prompt/session setup failures could still end with a final complete event.

Problem

When chatter.Send() fails before any stream-originated error update is emitted (for example, when strategy.LoadStrategy() fails during request assembly), the handler currently:

  1. logs the error,
  2. closes the stream channel, and then
  3. emits a final complete event.

That means an SSE client can interpret the request as successful even though no valid response was produced.

Fix

  • forward an explicit StreamTypeError update when chatter.Send() returns an error that was not already surfaced through stream updates
  • suppress the final complete event whenever an error was emitted for that prompt
  • add regression coverage for the invalid-strategy path

Files changed

  • internal/server/chat.go
  • internal/server/chat_test.go

Tests

go test ./internal/server -v

Related issues

Closes #2100

The /chat handler previously relied on stream-originated error updates and
still emitted a terminal complete event when chatter setup failed before any
stream updates were sent. This change forwards a server-side error event for
those early failures, suppresses complete after errors, and adds regression
coverage for an invalid strategy path.

Constraint: Keep the fix narrow to the REST SSE path
Rejected: Broad chatter API redesign | unnecessary for a targeted contract fix
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Preserve the invariant that complete is emitted only for successful prompt runs
Tested: go test ./internal/server -v
Not-tested: End-to-end browser SSE client behavior outside package tests
Related: danielmiessler#2100
@@ -0,0 +1,17 @@
### PR [#2102](https://github.com/danielmiessler/Fabric/pull/2102) by [shaun0927](https://github.com/shaun0927): fix(chat): emit SSE error and skip complete on prompt setup failures

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @shaun0927 while I appreciate that you included this snippet, I'll give the same feedback I gave in your other PR:

The changelog snippet ends up in the ChangeLog and on the Release Notes. It needs to be short, concise, but also accurate. It shouldn't contain all the details or be a mini-PRD. Pleaese make this short and brief.
You can also use the --ai-summarize flag of the generate_changelog CLI tool.

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.

/chat SSE returns complete even when strategy loading fails

2 participants