mirror of
https://github.com/danielmiessler/fabric.git
synced 2026-09-12 16:46:35 -04:00
## Bug: Streaming Deadlock in Chatter.Send When a streaming error occurs, two goroutines can race to write to the same buffered(1) error channel: 1. The SendStream goroutine returns an error and writes to errChan 2. The stream-update loop receives a StreamTypeError update and also writes to errChan Since errChan has a buffer of 1, the second write blocks forever, causing a goroutine leak and a deadlock — the caller never returns. ### Fix Introduce `recordFirstStreamError()` which uses a non-blocking select/default to safely send only the first error, discarding subsequent ones. This prevents the deadlock while preserving the original error for the caller. ### Scenario that triggers the deadlock (before this fix): 1. Vendor stream emits a StreamTypeError update (e.g., rate limit) 2. The update loop writes the error to errChan (buffer fills) 3. SendStream also returns an error 4. SendStream goroutine tries to write to errChan → BLOCKS FOREVER 5. Chatter.Send never returns → user sees a hang ## Refactor: Unify Strategy Handling (Server + CLI) The REST API server (chat.go) was loading strategy prompts inline by reading JSON files directly with os.ReadFile, bypassing the core Chatter layer entirely. This caused: - Strategy prompts were prepended to UserInput instead of the system message, breaking the prompt architecture - The StrategyName was not passed to GetChatter(), so the core layer had no knowledge of the strategy - Duplicate strategy-loading logic between CLI and server paths ### Fix - Pass StrategyName through to GetChatter() and ChatRequest so the core Chatter.BuildSession() handles strategy loading uniformly - Extract `buildPromptChatRequest()` helper for clean request construction - Remove inline os.ReadFile strategy loading from the server handler ## Refactor: Clean System Message Assembly Replace raw string concatenation of context + pattern + strategy prompts with `joinPromptSections()` which: - Trims whitespace from each section - Skips empty sections (no double newlines from missing context) - Joins with a single newline separator ## Tests Added - `TestChatter_BuildSession_SeparatesSystemSections`: Verifies strategy, context, and pattern are joined with newline separators in the correct order - `TestChatter_Send_StreamingErrorUpdateAndReturnDoesNotDeadlock`: Regression test with 2-second timeout that catches the deadlock when both error paths fire simultaneously - `TestBuildPromptChatRequest_PreservesStrategyAndUserInput`: Verifies the extracted helper preserves all fields including StrategyName |
||
|---|---|---|
| .. | ||
| strategy.go | ||
| strategy_test.go | ||