Rescue a tight tool-call loop sideways instead of killing the turn

internal/proxy/loop_detection.go

@@ -5,10 +5,13 @@ import (
 	"encoding/binary"
 	"fmt"
 	"net/http"
+	"slices"
+	"strings"
 	"time"
 
 	"workweave/router/internal/observability"
 	"workweave/router/internal/providers"
+	"workweave/router/internal/router/catalog"
 	"workweave/router/internal/router/sessionpin"
 	"workweave/router/internal/translate"
 
@@ -78,9 +81,16 @@ const (
 	loopDetectionMaxRepeats = 5
 )
 
+// 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": {},
+}
+
 // 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,219 @@ 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"
+)
+
+// 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
+// baseline and misattributes 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, sameEngineModels(pin.Model),
+			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 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
+	}
+	if pinnedProvider == "" {
+		return pinnedModel, requestedProvider
+	}
+	return pinnedModel, pinnedProvider
+}
+
+// handleToolCallLoopBreak is the last resort when no sideways rescue was
+// 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,
@@ -323,6 +545,7 @@ func (s *Service) handleToolCallLoopBreak(
 	role string,
 	loopingModel string,
 	loopingProvider string,
+	preserveForcedPin bool,
 	inputTokens int,
 ) error {
 	log := observability.FromContext(ctx)
@@ -351,7 +574,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)
 		}