From 6abbf5a0652c3ca6ec13c6e7413fc81d24b12ba3 Mon Sep 17 00:00:00 2001 From: Steven Tohme Date: Tue, 25 Aug 2026 19:43:40 +0000 Subject: [PATCH 1/5] Rescue a tight tool-call loop sideways instead of killing the turn --- internal/proxy/loop_detection.go | 205 +++++++++++++++++- internal/proxy/loop_sideways_internal_test.go | 178 +++++++++++++++ internal/proxy/service.go | 15 +- internal/proxy/turnloop.go | 2 +- internal/translate/force_model.go | 6 + 5 files changed, 398 insertions(+), 8 deletions(-) create mode 100644 internal/proxy/loop_sideways_internal_test.go diff --git a/internal/proxy/loop_detection.go b/internal/proxy/loop_detection.go index b6b95f9fc..8ef6ae220 100644 --- a/internal/proxy/loop_detection.go +++ b/internal/proxy/loop_detection.go @@ -9,6 +9,7 @@ import ( "workweave/router/internal/observability" "workweave/router/internal/providers" + "workweave/router/internal/router/catalog" "workweave/router/internal/router/sessionpin" "workweave/router/internal/translate" @@ -78,9 +79,18 @@ const ( loopDetectionMaxRepeats = 5 ) +// pollToolNames read a side channel that changes underneath a byte-identical +// request: draining a background shell's buffer returns new output every call +// with the same arguments. Repetition is the tool's normal usage, not loop +// evidence — prod 2026-08-25 stopped a healthy session on 5x `shell_output`. +var pollToolNames = map[string]struct{}{ + "shell_output": {}, "get_output": {}, "BashOutput": {}, +} + // detectToolCallLoop reports whether the same (tool_name, args) signature // repeats loopDetectionMaxRepeats+ times within the last loopDetectionWindowSize // tool calls, returning the signature and count for logs/the stop message. +// Poll-style tools are counted for window position but never trip the break. func detectToolCallLoop(env *translate.RequestEnvelope) (looped bool, sig translate.ToolCallSig, count int) { sigs := env.AssistantToolCallSignatures() if len(sigs) < loopDetectionMaxRepeats { @@ -94,6 +104,9 @@ func detectToolCallLoop(env *translate.RequestEnvelope) (looped bool, sig transl counts := make(map[string]int, len(window)) keys := make(map[string]translate.ToolCallSig, len(window)) for _, s := range window { + if _, isPoll := pollToolNames[s.Name]; isPoll { + continue + } key := s.Name + "\x00" + s.InputHash counts[key]++ keys[key] = s @@ -308,10 +321,191 @@ func (s *Service) handleLoopEscalation( } } -// handleToolCallLoopBreak short-circuits a runaway tool-call loop: writes a -// synthetic end_turn response and expires the session pin so the next turn -// re-routes instead of re-anchoring on the looping model. Pin expiry is -// best-effort — a write failure logs but doesn't block the response. +// loopSidewaysResult reports whether the rescue re-pinned the session and, for +// the caller's fallback path, who was looping and whether the pin is a user's. +type loopSidewaysResult struct { + Moved bool + LoopingModel string + LoopingProvider string + UserForced bool +} + +// Sideways-rescue action taxonomy for a tight tool-call loop. Exactly one applies. +const ( + // loopSidewaysMoved: the session was re-pinned onto a different arm and + // this same turn dispatches there. + loopSidewaysMoved = "moved" + // loopSidewaysNoPin: no pin to read, so neither the looping model nor its + // cluster is known — nothing to move sideways from. + loopSidewaysNoPin = "no_pin" + // loopSidewaysUserForced: a /force-model pin outranks the automatic move. + loopSidewaysUserForced = "user_forced" + // loopSidewaysAlreadyMoved: this session was already rescued onto another + // arm; looping again there is a task problem, not a misroute. + loopSidewaysAlreadyMoved = "already_moved" + // loopSidewaysNoTarget: no dispatchable arm above or beside the pin's cluster. + loopSidewaysNoTarget = "no_target" + // loopSidewaysDisabled: the loop-escalation kill switch is off. + loopSidewaysDisabled = "disabled" +) + +// handleToolCallLoopSideways rescues a tight tool-call loop by re-pinning the +// session onto a different arm — the cheapest cluster above the pin's own, else +// a sideways arm within it — instead of stopping the turn. It writes no +// response, so routing picks the new pin up and dispatches this same turn. +// +// The result also names the model that was actually looping (the pin), which is +// NOT the client's requested model: this runs before routing, so the caller's +// feats.Model is the inbound baseline and misattributes the loop on any +// re-routed session. +func (s *Service) handleToolCallLoopSideways( + ctx context.Context, + sig translate.ToolCallSig, + count int, + installationID uuid.UUID, + sessionKey [sessionpin.SessionKeyLen]byte, + role string, +) loopSidewaysResult { + log := observability.FromContext(ctx) + + if s.pinStore == nil || installationID == uuid.Nil { + return loopSidewaysResult{} + } + pin, found, err := s.pinStore.Get(ctx, sessionKey, role) + if err != nil { + log.Error("loop-sideways: pin lookup failed", "err", err) + return loopSidewaysResult{} + } + if !found { + log.Info("router.loop_sideways", + "action", loopSidewaysNoPin, + "loop_tool", sig.Name, + "repeat_count", count, + "session_key_prefix", shortSessionKey(sessionKey), + "role", role, + ) + return loopSidewaysResult{} + } + + action := loopSidewaysMoved + var target, targetCluster, targetProvider string + switch { + case !s.ResolveLoopEscalationEnabled(ctx): + action = loopSidewaysDisabled + case isUserForcedReason(pin.Reason): + action = loopSidewaysUserForced + case pin.Reason == translate.ReasonLoopSideways || + pin.Reason == translate.ReasonLoopEscalation || + pin.Reason == translate.ReasonStruggleEscalation: + action = loopSidewaysAlreadyMoved + case pin.Model == "" || pin.PolicyGroup == "" || s.struggleEscalationRoster == nil: + action = loopSidewaysNoTarget + default: + t, cluster, rosterErr := s.struggleEscalationRoster.EscalationTarget( + ctx, pin.PolicyGroup, pin.Model, nil, + func(model string) bool { + if s.availableModels != nil { + if _, ok := s.availableModels[model]; !ok { + return false + } + } + return true + }, + ) + if rosterErr != nil { + log.Error("loop-sideways: roster lookup failed", "err", rosterErr) + action = loopSidewaysNoTarget + break + } + m, mok := catalog.ByID(t) + if t == "" || !mok || len(m.Providers) == 0 { + action = loopSidewaysNoTarget + break + } + target, targetCluster, targetProvider = t, cluster, m.Providers[0].Provider + // context.Background(): the request ctx may already be canceled; the + // pin must land or this turn dispatches back onto the looping model. + upsertErr := s.pinStore.Upsert(context.Background(), sessionpin.Pin{ + SessionKey: sessionKey, + Role: role, + InstallationID: installationID, + Provider: m.Providers[0].Provider, + Model: target, + Reason: translate.ReasonLoopSideways, + TurnCount: 1, + PinnedUntil: time.Now().Add(pinSessionTTL), + PolicyGroup: targetCluster, + LastServedModel: pin.LastServedModel, + }) + if upsertErr != nil { + log.Error("loop-sideways: pin upsert failed", "err", upsertErr) + action = loopSidewaysNoTarget + target, targetCluster, targetProvider = "", "", "" + } + } + + log.Info("router.loop_sideways", + "looping_model", pin.Model, + "looping_provider", pin.Provider, + "action", action, + "escalation_target", target, + "escalation_provider", targetProvider, + "escalation_cluster", targetCluster, + "policy_group", pin.PolicyGroup, + "loop_tool", sig.Name, + "loop_input_hash", sig.InputHash, + "repeat_count", count, + "window_size", loopDetectionWindowSize, + "session_key_prefix", shortSessionKey(sessionKey), + "role", role, + ) + + if action != loopSidewaysMoved { + return loopSidewaysResult{ + LoopingModel: pin.Model, + LoopingProvider: pin.Provider, + UserForced: action == loopSidewaysUserForced, + } + } + if s.loopEscalationStore != nil { + event := LoopEscalationEvent{ + InstallationID: installationID.String(), + SessionKey: sessionKey[:], + Role: role, + LoopingModel: pin.Model, + Action: struggleActionSideways, + EscalationTarget: target, + LoopTool: sig.Name, + LoopInputHash: sig.InputHash, + RepeatCount: int32(count), + WindowSize: loopDetectionWindowSize, + } + if err := s.loopEscalationStore.InsertLoopEscalationEvent(context.Background(), event); err != nil { + log.Error("loop-sideways: event insert failed", "err", err) + } + } + return loopSidewaysResult{Moved: true, LoopingModel: pin.Model, LoopingProvider: pin.Provider} +} + +// loopAttribution names the model that was actually looping. The pinned model +// is authoritative — loop detection runs before routing, so the inbound +// requested model is only a fallback for a session with no pin to read. +func loopAttribution(pinnedModel, pinnedProvider, requestedModel, requestedProvider string) (model, provider string) { + if pinnedModel == "" { + return requestedModel, requestedProvider + } + if pinnedProvider == "" { + return pinnedModel, requestedProvider + } + return pinnedModel, pinnedProvider +} + +// handleToolCallLoopBreak is the last resort when no sideways rescue was +// available: it writes a synthetic end_turn response and expires the session +// pin so the next turn re-routes instead of re-anchoring on the looping model. +// Pin expiry is best-effort — a write failure logs but doesn't block the +// response — and is skipped for a user-forced pin, which outranks automatic +// eviction just as it outranks the sideways move. func (s *Service) handleToolCallLoopBreak( ctx context.Context, w http.ResponseWriter, @@ -323,6 +517,7 @@ func (s *Service) handleToolCallLoopBreak( role string, loopingModel string, loopingProvider string, + preserveForcedPin bool, inputTokens int, ) error { log := observability.FromContext(ctx) @@ -351,7 +546,7 @@ func (s *Service) handleToolCallLoopBreak( // Expire the pin in Postgres (not just the in-proc cache) so a racing // reader on another pod can't repopulate the LRU from the stale row. - if s.pinStore != nil && installationID != uuid.Nil { + if s.pinStore != nil && installationID != uuid.Nil && !preserveForcedPin { if err := s.expireSessionPinAndHMMHistory(ctx, installationID, sessionKey, role, "tool_call_loop_break"); err != nil { log.Error("loop-break: pin store upsert failed", "err", err) } diff --git a/internal/proxy/loop_sideways_internal_test.go b/internal/proxy/loop_sideways_internal_test.go new file mode 100644 index 000000000..e369e4da2 --- /dev/null +++ b/internal/proxy/loop_sideways_internal_test.go @@ -0,0 +1,178 @@ +package proxy + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "workweave/router/internal/router/sessionpin" + "workweave/router/internal/translate" +) + +// newLoopSidewaysSvc wires the pieces handleToolCallLoopSideways touches: the +// pin store it reads and re-pins, the roster it picks a target from, and the +// event store the rescue is recorded in. +func newLoopSidewaysSvc(pins *stubPinStore, events *recordingLoopStore, clusters map[string][]string) *Service { + return NewService(nil, nil, nil, false, nil, pins, false, "anthropic", "claude-haiku-4-5", nil). + WithLoopEscalationConfig(true, 0). + WithLoopEscalationStore(events). + WithStruggleEscalationRoster(NewStruggleRoster(fakeRosterSource{clusters: clusters})) +} + +var loopSidewaysClusters = map[string][]string{ + "balanced": {"anthropic/claude-haiku-4.5", "anthropic/claude-sonnet-4-5"}, + "high": {"anthropic/claude-opus-5"}, +} + +func loopSidewaysPin(model, reason string) sessionpin.Pin { + return sessionpin.Pin{ + Model: model, + Provider: "openai", + PolicyGroup: "balanced", + Reason: reason, + } +} + +func TestHandleToolCallLoopSideways_RePinsOntoAnotherArm(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("claude-haiku-4-5", "hmm_sticky"), true + events := &recordingLoopStore{} + svc := newLoopSidewaysSvc(pins, events, loopSidewaysClusters) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(1), "default") + + assert.True(t, res.Moved, "a loop with a dispatchable target must be rescued, not stopped") + require.Len(t, pins.upserts, 1, "the rescue must land a pin so this same turn dispatches elsewhere") + pin := pins.upserts[0] + assert.Equal(t, "claude-opus-5", pin.Model) + assert.Equal(t, translate.ReasonLoopSideways, pin.Reason) + assert.Equal(t, "high", pin.PolicyGroup) + assert.NotEmpty(t, pin.Provider, "the pin must name the provider that serves the target") + + require.Len(t, events.events, 1) + assert.Equal(t, "claude-haiku-4-5", events.events[0].LoopingModel, "telemetry names the served model, not the client baseline") + assert.Equal(t, "claude-opus-5", events.events[0].EscalationTarget) +} + +func TestHandleToolCallLoopSideways_LeavesUserForcedPinAlone(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("claude-haiku-4-5", translate.ReasonUserForceModel), true + svc := newLoopSidewaysSvc(pins, &recordingLoopStore{}, loopSidewaysClusters) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(2), "default") + + assert.False(t, res.Moved) + assert.True(t, res.UserForced, "the caller must skip pin eviction for a /force-model session") + assert.Empty(t, pins.upserts, "an explicit force-model pin outranks the automatic move") +} + +func TestHandleToolCallLoopSideways_DoesNotRescueTwice(t *testing.T) { + for _, reason := range []string{translate.ReasonLoopSideways, translate.ReasonLoopEscalation, translate.ReasonStruggleEscalation} { + t.Run(reason, func(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("claude-haiku-4-5", reason), true + svc := newLoopSidewaysSvc(pins, &recordingLoopStore{}, loopSidewaysClusters) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(3), "default") + + assert.False(t, res.Moved, "looping again after a rescue is a task problem; stop the turn") + assert.False(t, res.UserForced) + assert.Empty(t, pins.upserts) + }) + } +} + +func TestHandleToolCallLoopSideways_NoPinMeansNoAttribution(t *testing.T) { + pins := newStubPinStore() + svc := newLoopSidewaysSvc(pins, &recordingLoopStore{}, loopSidewaysClusters) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(4), "default") + + assert.False(t, res.Moved) + assert.Empty(t, res.LoopingModel, "with no pin the caller falls back to the requested model") +} + +func TestHandleToolCallLoopSideways_NoDispatchableTargetFallsBack(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("claude-opus-5", "hmm_sticky"), true + svc := newLoopSidewaysSvc(pins, &recordingLoopStore{}, map[string][]string{"balanced": {"anthropic/claude-opus-5"}}) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(5), "default") + + assert.False(t, res.Moved, "the only arm in the roster is the one looping") + assert.Equal(t, "claude-opus-5", res.LoopingModel, "the fallback stop must still be attributed to the pin") + assert.Equal(t, "openai", res.LoopingProvider) + assert.Empty(t, pins.upserts) +} + +func TestHandleToolCallLoopSideways_DisabledFallsBack(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("claude-haiku-4-5", "hmm_sticky"), true + svc := newLoopSidewaysSvc(pins, &recordingLoopStore{}, loopSidewaysClusters). + WithLoopEscalationConfig(false, 0) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(6), "default") + + assert.False(t, res.Moved) + assert.Empty(t, pins.upserts, "the kill switch must stop the pin write, not just the logging") +} + +func TestHandleToolCallLoopSideways_PinWriteFailureFallsBack(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("claude-haiku-4-5", "hmm_sticky"), true + pins.upsertErr = assert.AnError + events := &recordingLoopStore{} + svc := newLoopSidewaysSvc(pins, events, loopSidewaysClusters) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(7), "default") + + assert.False(t, res.Moved, "an unwritten pin means the turn would dispatch back onto the looping model") + assert.Empty(t, events.events, "no rescue happened, so no rescue event") +} + +func TestLoopAttribution_PrefersThePin(t *testing.T) { + model, provider := loopAttribution("gpt-5.6-luna", "openai", "claude-fable-5", "anthropic") + assert.Equal(t, "gpt-5.6-luna", model) + assert.Equal(t, "openai", provider) + + model, provider = loopAttribution("", "", "claude-fable-5", "anthropic") + assert.Equal(t, "claude-fable-5", model, "with no pin the requested model is all we know") + assert.Equal(t, "anthropic", provider) +} + +func TestDetectToolCallLoop_PollingToolsDoNotTrip(t *testing.T) { + for name := range pollToolNames { + t.Run(name, func(t *testing.T) { + calls := make([]toolCall, 0, 8) + for range 8 { + calls = append(calls, toolCall{name: name, input: map[string]any{"shell_id": "abc"}}) + } + env, err := translate.ParseAnthropic(buildBodyWithToolCalls(t, calls)) + require.NoError(t, err) + + loop, _, _ := detectToolCallLoop(env) + assert.False(t, loop, "draining a background shell returns new output on every identical call") + }) + } +} + +func TestDetectToolCallLoop_PollingDoesNotMaskARealLoop(t *testing.T) { + // A genuine repeat interleaved with polls still trips: the exemption drops + // poll signatures from the counts, not the whole window. + calls := []toolCall{} + for range 5 { + calls = append(calls, + toolCall{name: "shell_output", input: map[string]any{"shell_id": "abc"}}, + toolCall{name: "ls", input: map[string]any{"path": "/tmp"}}, + ) + } + env, err := translate.ParseAnthropic(buildBodyWithToolCalls(t, calls)) + require.NoError(t, err) + + loop, sig, _ := detectToolCallLoop(env) + assert.True(t, loop) + assert.Equal(t, "ls", sig.Name) +} diff --git a/internal/proxy/service.go b/internal/proxy/service.go index bfdf466e4..5565d4803 100644 --- a/internal/proxy/service.go +++ b/internal/proxy/service.go @@ -592,6 +592,7 @@ const ( markerReasonUserForced = "pinned by force-model" markerReasonLoopEscalated = "escalated due to loop" markerReasonStruggleEscalated = "picked a different model to break a grind" + markerReasonLoopSideways = "switched model to break a tool-call loop" markerReasonSwitched = "switched for positive EV after cache eviction" markerReasonStayed = "stayed on your last pick" markerReasonTierUpgrade = "upgraded to a stronger tier" @@ -638,6 +639,8 @@ func routingReasonShort(res turnLoopResult) string { return markerReasonLoopEscalated case translate.ReasonStruggleEscalation: return markerReasonStruggleEscalated + case translate.ReasonLoopSideways: + return markerReasonLoopSideways } return markerReasonBestPick } @@ -2657,7 +2660,11 @@ func (s *Service) ProxyMessages(ctx context.Context, body []byte, w http.Respons if loop, sig, count := detectToolCallLoop(env); loop { loopRole := roleForTier(catalog.TierFor(feats.Model)) log.Info("ProxyMessages tool-call loop detected", "tool_sig", sig, "repeat_count", count, "role", loopRole) - return s.handleToolCallLoopBreak(ctx, w, env, sig, count, installationID, sessionKey, loopRole, feats.Model, providers.ProviderAnthropic, feats.Tokens) + rescue := s.handleToolCallLoopSideways(ctx, sig, count, installationID, sessionKey, loopRole) + if !rescue.Moved { + loopingModel, loopingProvider := loopAttribution(rescue.LoopingModel, rescue.LoopingProvider, feats.Model, providers.ProviderAnthropic) + return s.handleToolCallLoopBreak(ctx, w, env, sig, count, installationID, sessionKey, loopRole, loopingModel, loopingProvider, rescue.UserForced, feats.Tokens) + } } } } @@ -5140,7 +5147,11 @@ func (s *Service) ProxyOpenAIChatCompletion(ctx context.Context, body []byte, w if loop, sig, count := detectToolCallLoop(env); loop { loopRole := roleForTier(catalog.TierFor(feats.Model)) log.Info("ProxyOpenAIChatCompletion tool-call loop detected", "tool_sig", sig, "repeat_count", count, "role", loopRole) - return s.handleToolCallLoopBreak(ctx, w, env, sig, count, installationID, sessionKey, loopRole, feats.Model, providers.ProviderOpenAI, feats.Tokens) + rescue := s.handleToolCallLoopSideways(ctx, sig, count, installationID, sessionKey, loopRole) + if !rescue.Moved { + loopingModel, loopingProvider := loopAttribution(rescue.LoopingModel, rescue.LoopingProvider, feats.Model, providers.ProviderOpenAI) + return s.handleToolCallLoopBreak(ctx, w, env, sig, count, installationID, sessionKey, loopRole, loopingModel, loopingProvider, rescue.UserForced, feats.Tokens) + } } } diff --git a/internal/proxy/turnloop.go b/internal/proxy/turnloop.go index 6886ebe83..9a124665a 100644 --- a/internal/proxy/turnloop.go +++ b/internal/proxy/turnloop.go @@ -734,7 +734,7 @@ func (s *Service) runTurnLoop( // the scorer call further down constrains the fresh decision to this tier // instead of collapsing to the cheap tier-default. TierUnknown = no constraint. forcedTierFloor := catalog.TierUnknown - if pinFound && (isUserForcedReason(pin.Reason) || pin.Reason == translate.ReasonLoopEscalation || pin.Reason == translate.ReasonStruggleEscalation) { + if pinFound && (isUserForcedReason(pin.Reason) || pin.Reason == translate.ReasonLoopEscalation || pin.Reason == translate.ReasonStruggleEscalation || pin.Reason == translate.ReasonLoopSideways) { _, excluded := req.ExcludedModels[pin.Model] _, providerEnabled := req.EnabledProviders[pin.Provider] providerEligible := req.EnabledProviders == nil || providerEnabled diff --git a/internal/translate/force_model.go b/internal/translate/force_model.go index c5cb5d101..312e272be 100644 --- a/internal/translate/force_model.go +++ b/internal/translate/force_model.go @@ -17,6 +17,12 @@ const ReasonUserForceModel = "user_forced" // ReasonUserForceModel, so the session doesn't re-route back into the loop. const ReasonLoopEscalation = "loop_escalation" +// ReasonLoopSideways marks a session pin created when the tight tool-call loop +// detector moves a looping session onto a different arm instead of stopping the +// turn. Immutable sticky like ReasonLoopEscalation, so the session can't +// re-route back onto the model it was looping on. +const ReasonLoopSideways = "loop_sideways" + // ReasonStruggleEscalation marks a session pin created when the struggle // detector arms an early sideways move (turns >= 30, wall >= 10m). Immutable // sticky like ReasonLoopEscalation. From 790df14b835dd557bbd88156b3d6c1f11ae63d10 Mon Sep 17 00:00:00 2001 From: Steven Tohme Date: Tue, 25 Aug 2026 19:47:43 +0000 Subject: [PATCH 2/5] Tighten loop-sideways comments per review --- internal/proxy/loop_detection.go | 22 +++++++--------------- internal/translate/force_model.go | 7 +++---- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/internal/proxy/loop_detection.go b/internal/proxy/loop_detection.go index 8ef6ae220..721968b2d 100644 --- a/internal/proxy/loop_detection.go +++ b/internal/proxy/loop_detection.go @@ -79,10 +79,8 @@ const ( loopDetectionMaxRepeats = 5 ) -// pollToolNames read a side channel that changes underneath a byte-identical -// request: draining a background shell's buffer returns new output every call -// with the same arguments. Repetition is the tool's normal usage, not loop -// evidence — prod 2026-08-25 stopped a healthy session on 5x `shell_output`. +// pollToolNames holds tools that drain a side-channel buffer — repetition is +// their normal usage, not loop evidence. var pollToolNames = map[string]struct{}{ "shell_output": {}, "get_output": {}, "BashOutput": {}, } @@ -350,14 +348,9 @@ const ( ) // handleToolCallLoopSideways rescues a tight tool-call loop by re-pinning the -// session onto a different arm — the cheapest cluster above the pin's own, else -// a sideways arm within it — instead of stopping the turn. It writes no -// response, so routing picks the new pin up and dispatches this same turn. -// -// The result also names the model that was actually looping (the pin), which is -// NOT the client's requested model: this runs before routing, so the caller's -// feats.Model is the inbound baseline and misattributes the loop on any -// re-routed session. +// session onto a different arm instead of stopping the turn. Returns a result +// naming the looping model (the pin, not feats.Model, which is the pre-routing +// baseline and misattributes on any re-routed session). func (s *Service) handleToolCallLoopSideways( ctx context.Context, sig translate.ToolCallSig, @@ -487,9 +480,8 @@ func (s *Service) handleToolCallLoopSideways( return loopSidewaysResult{Moved: true, LoopingModel: pin.Model, LoopingProvider: pin.Provider} } -// loopAttribution names the model that was actually looping. The pinned model -// is authoritative — loop detection runs before routing, so the inbound -// requested model is only a fallback for a session with no pin to read. +// loopAttribution prefers the pinned model: loop detection runs before routing, +// so the inbound requested model misattributes on re-routed sessions. func loopAttribution(pinnedModel, pinnedProvider, requestedModel, requestedProvider string) (model, provider string) { if pinnedModel == "" { return requestedModel, requestedProvider diff --git a/internal/translate/force_model.go b/internal/translate/force_model.go index 312e272be..e5cd8db33 100644 --- a/internal/translate/force_model.go +++ b/internal/translate/force_model.go @@ -17,10 +17,9 @@ const ReasonUserForceModel = "user_forced" // ReasonUserForceModel, so the session doesn't re-route back into the loop. const ReasonLoopEscalation = "loop_escalation" -// ReasonLoopSideways marks a session pin created when the tight tool-call loop -// detector moves a looping session onto a different arm instead of stopping the -// turn. Immutable sticky like ReasonLoopEscalation, so the session can't -// re-route back onto the model it was looping on. +// ReasonLoopSideways marks a session pin created by the tight tool-call loop +// detector to re-route onto a different arm. Immutable sticky: scorer/planner +// are bypassed so the session can't route back onto the looping model. const ReasonLoopSideways = "loop_sideways" // ReasonStruggleEscalation marks a session pin created when the struggle From ef3a5f9325895bd2d2d2d9c7ba65003e5a9f73a6 Mon Sep 17 00:00:00 2001 From: Steven Tohme Date: Tue, 25 Aug 2026 19:50:51 +0000 Subject: [PATCH 3/5] Tighten two more comments per review --- internal/proxy/loop_detection.go | 8 +++----- internal/proxy/loop_sideways_internal_test.go | 4 +--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/proxy/loop_detection.go b/internal/proxy/loop_detection.go index 721968b2d..93338295e 100644 --- a/internal/proxy/loop_detection.go +++ b/internal/proxy/loop_detection.go @@ -493,11 +493,9 @@ func loopAttribution(pinnedModel, pinnedProvider, requestedModel, requestedProvi } // handleToolCallLoopBreak is the last resort when no sideways rescue was -// available: it writes a synthetic end_turn response and expires the session -// pin so the next turn re-routes instead of re-anchoring on the looping model. -// Pin expiry is best-effort — a write failure logs but doesn't block the -// response — and is skipped for a user-forced pin, which outranks automatic -// eviction just as it outranks the sideways move. +// available: writes a synthetic end_turn and expires the session pin so the +// next turn re-routes. Pin expiry is best-effort (write failure logs only) +// and is skipped when preserveForcedPin — user-forced pins outrank eviction. func (s *Service) handleToolCallLoopBreak( ctx context.Context, w http.ResponseWriter, diff --git a/internal/proxy/loop_sideways_internal_test.go b/internal/proxy/loop_sideways_internal_test.go index e369e4da2..6583f34c5 100644 --- a/internal/proxy/loop_sideways_internal_test.go +++ b/internal/proxy/loop_sideways_internal_test.go @@ -12,9 +12,7 @@ import ( "workweave/router/internal/translate" ) -// newLoopSidewaysSvc wires the pieces handleToolCallLoopSideways touches: the -// pin store it reads and re-pins, the roster it picks a target from, and the -// event store the rescue is recorded in. +// newLoopSidewaysSvc wires the pin store, roster, and event store that handleToolCallLoopSideways touches. func newLoopSidewaysSvc(pins *stubPinStore, events *recordingLoopStore, clusters map[string][]string) *Service { return NewService(nil, nil, nil, false, nil, pins, false, "anthropic", "claude-haiku-4-5", nil). WithLoopEscalationConfig(true, 0). From 6b61ce24acfabc73a2afd49715d11424d80e507d Mon Sep 17 00:00:00 2001 From: Steven Tohme Date: Tue, 25 Aug 2026 20:06:29 +0000 Subject: [PATCH 4/5] Sideways rescue must change model, not just effort level --- internal/proxy/loop_detection.go | 40 ++++++++++++++++++- internal/proxy/loop_sideways_internal_test.go | 34 ++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/internal/proxy/loop_detection.go b/internal/proxy/loop_detection.go index 93338295e..0dcae5657 100644 --- a/internal/proxy/loop_detection.go +++ b/internal/proxy/loop_detection.go @@ -5,6 +5,8 @@ import ( "encoding/binary" "fmt" "net/http" + "slices" + "strings" "time" "workweave/router/internal/observability" @@ -347,6 +349,42 @@ const ( loopSidewaysDisabled = "disabled" ) +// effortVariantSuffixes name catalog IDs that are the same engine dialed to a +// different reasoning budget rather than a genuinely different model. +var effortVariantSuffixes = []string{"pro", "thinking"} + +// sameEngine reports whether candidate is the looping model wearing a different +// effort level ("gpt-5.6-luna" vs "gpt-5.6-luna-pro" or "…:high"). More +// thinking on the same weights repeats the same tool call, so a rescue that +// only changes effort is not a rescue. +func sameEngine(current, candidate string) bool { + current, candidate = baseModelOf(current), baseModelOf(candidate) + if current == candidate { + return true + } + long, short := current, candidate + if len(short) > len(long) { + long, short = short, long + } + suffix, ok := strings.CutPrefix(long, short+"-") + if !ok { + return false + } + return slices.Contains(effortVariantSuffixes, suffix) || translate.IsValidEffort(suffix) +} + +// sameEngineModels is the exclusion set for a sideways rescue: the looping +// model plus every catalog entry that is only an effort variant of it. +func sameEngineModels(current string) map[string]struct{} { + exclude := map[string]struct{}{current: {}} + for _, m := range catalog.Models { + if sameEngine(current, m.ID) { + exclude[m.ID] = struct{}{} + } + } + return exclude +} + // handleToolCallLoopSideways rescues a tight tool-call loop by re-pinning the // session onto a different arm instead of stopping the turn. Returns a result // naming the looping model (the pin, not feats.Model, which is the pre-routing @@ -395,7 +433,7 @@ func (s *Service) handleToolCallLoopSideways( action = loopSidewaysNoTarget default: t, cluster, rosterErr := s.struggleEscalationRoster.EscalationTarget( - ctx, pin.PolicyGroup, pin.Model, nil, + ctx, pin.PolicyGroup, pin.Model, sameEngineModels(pin.Model), func(model string) bool { if s.availableModels != nil { if _, ok := s.availableModels[model]; !ok { diff --git a/internal/proxy/loop_sideways_internal_test.go b/internal/proxy/loop_sideways_internal_test.go index 6583f34c5..ca9bfc257 100644 --- a/internal/proxy/loop_sideways_internal_test.go +++ b/internal/proxy/loop_sideways_internal_test.go @@ -55,6 +55,40 @@ func TestHandleToolCallLoopSideways_RePinsOntoAnotherArm(t *testing.T) { assert.Equal(t, "claude-opus-5", events.events[0].EscalationTarget) } +func TestHandleToolCallLoopSideways_SkipsEffortVariantsOfTheLoopingModel(t *testing.T) { + pins := newStubPinStore() + pins.getPin, pins.getFound = loopSidewaysPin("gpt-5.6-luna", "hmm_sticky"), true + svc := newLoopSidewaysSvc(pins, &recordingLoopStore{}, map[string][]string{ + "balanced": {"openai/gpt-5.6-luna"}, + "high": {"openai/gpt-5.6-luna-pro", "anthropic/claude-opus-5"}, + }) + + res := svc.handleToolCallLoopSideways(context.Background(), loopTestSig, 5, uuid.New(), loopTestKey(8), "default") + + assert.True(t, res.Moved) + require.Len(t, pins.upserts, 1) + assert.Equal(t, "claude-opus-5", pins.upserts[0].Model, + "more effort on the same engine repeats the same tool call — the rescue must change model") +} + +func TestSameEngine(t *testing.T) { + cases := []struct { + current, candidate string + want bool + }{ + {"gpt-5.6-luna", "gpt-5.6-luna", true}, + {"gpt-5.6-luna", "gpt-5.6-luna-pro", true}, + {"gpt-5.6-luna", "gpt-5.6-luna:high", true}, + {"gpt-5.6-luna", "gpt-5.6-terra", false}, + {"gpt-5.6-luna", "gpt-5.6-sol-pro", false}, + {"gpt-5.4", "gpt-5.4-mini", false}, + {"claude-opus-5", "claude-opus-4-5", false}, + } + for _, c := range cases { + assert.Equal(t, c.want, sameEngine(c.current, c.candidate), "sameEngine(%q, %q)", c.current, c.candidate) + } +} + func TestHandleToolCallLoopSideways_LeavesUserForcedPinAlone(t *testing.T) { pins := newStubPinStore() pins.getPin, pins.getFound = loopSidewaysPin("claude-haiku-4-5", translate.ReasonUserForceModel), true From ad272214580963f84a7634025de4b231d3fb2be6 Mon Sep 17 00:00:00 2001 From: Steven Tohme Date: Tue, 25 Aug 2026 20:09:20 +0000 Subject: [PATCH 5/5] Shorten sameEngine comment per review --- internal/proxy/loop_detection.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/proxy/loop_detection.go b/internal/proxy/loop_detection.go index 0dcae5657..bbc1642ca 100644 --- a/internal/proxy/loop_detection.go +++ b/internal/proxy/loop_detection.go @@ -353,10 +353,9 @@ const ( // different reasoning budget rather than a genuinely different model. var effortVariantSuffixes = []string{"pro", "thinking"} -// sameEngine reports whether candidate is the looping model wearing a different -// effort level ("gpt-5.6-luna" vs "gpt-5.6-luna-pro" or "…:high"). More -// thinking on the same weights repeats the same tool call, so a rescue that -// only changes effort is not a rescue. +// sameEngine reports whether candidate is the looping model at a different +// effort level ("gpt-5.6-luna" vs "gpt-5.6-luna-pro" or "…:high") — same +// weights repeat the same tool call, so changing only effort is not a rescue. func sameEngine(current, candidate string) bool { current, candidate = baseModelOf(current), baseModelOf(candidate) if current == candidate {