gateway: the loop guard's end-to-end tests bound how soon the loop was cut by what reached the agent, not by how far the fake vendor wrote ahead of a slowed gateway (#1443)
Cause: loopCut failed when the fake vendor had sent more than 2000 lines
by the time it saw its request let go. That count isn't the guard's: the
cut is made in holdWriter.Write, on the write that trips it, and cancels
the vendor's request there. But the vendor's writes succeed as long as the
sockets' and transports' buffers take them, so it runs ahead of a gateway
that reads slowly. Under load (CI macOS -race, a full run at load ~35) it
got ~2,800-3,200 lines ahead of a cut the agent saw at the 259th line.
Fix (test only, the guard was already deterministic): loopingVendor sends
at most 2000 lines of its loop, then holds its stream open until its
request is let go (20s, else it says it wasn't). Being let go therefore
means the guard found the loop within those 2000 lines, however the test
is scheduled. How soon it cut is now checked on what reached the agent:
the loop lines in the stream are at most the guard's window (256) plus one
32K read of the vendor's stream, which the cut's write sends whole.
Run:
- Reproduced by injecting a 300us sleep into holdWriter.Write (not
committed): the old test failed 140 of 180 subtests (-race -count=5
-cpu 1,2,4, 16 busy loops beside it), every route and protocol but chat,
"let go only after 2754..3082 lines", the vendor up to 8322. The agent
got ~259 loop lines in each.
- The new test with the same sleep and load: -race -count=50 -cpu 1,2,4,
1800/1800 subtests pass (built-in and plugin, chat/anthropic/responses/
gemini, and TestLoopEndsTheReplyOnEveryProtocol). At most 470 loop lines
reached the agent (the bound is 583; 256 + 32768/141 is ~490).
- Without the sleep, under load: -race -count=20 -cpu 1,2,4 of these plus
TestLoopGuardOff and TestLongReplyThatDoesNotLoopGoesThrough, all pass.
- The test goes red when the guarantee breaks: with cutLoop's h.stop()
disabled ("the vendor's request was not let go", built-in anthropic,
responses, gemini; chat is still let go when the failed write ends its
copy), and with the guard's trips dropped for its first 800 writes ("the
loop was cut only after 1038 of its lines reached the agent").
- gofmt -l, go vet (darwin, GOOS=linux, GOOS=windows) ./internal/gateway,
go test -race ./internal/gateway under a temp HOME.
This commit is contained in:
1 parent
62d7cd7be3
commit
229ce19acc
1 file changed
+37
-5
@@ -252,6 +252,15 @@ func TestLoopGuardLeavesOrdinaryRepliesAlone(t *testing.T) {
|
||||
// loopingVendor is DeepSeek on its Chat API, its reasoning looping until
|
||||
// the request is let go, or, with lines set, for that many lines and then
|
||||
// the reply's end. It says whether the request was let go.
|
||||
//
|
||||
// Looping until let go, it sends loopMost lines and then waits, its stream
|
||||
// open, for its request to be let go. How many lines it has sent by the
|
||||
// time it sees that isn't how soon the guard cut: the lines it wrote ahead
|
||||
// into the sockets' and transports' buffers, which the gateway hadn't read
|
||||
// yet, count too. A gateway slowed by load let it get ~2,800 lines ahead
|
||||
// of a cut made at the 259th (#1443). Stopping at loopMost makes being let
|
||||
// go mean the guard found the loop within that many lines, however the
|
||||
// test is scheduled; loopCut bounds how soon by what reached the agent.
|
||||
type loopingVendor struct {
|
||||
reasoning string // what it reasons, else the reporter's loop
|
||||
lines int // 0: loop until let go
|
||||
@@ -259,6 +268,10 @@ type loopingVendor struct {
|
||||
letGo chan bool
|
||||
}
|
||||
|
||||
// loopMost is how many lines of its loop loopingVendor sends before it
|
||||
// waits to be let go.
|
||||
const loopMost = 2000
|
||||
|
||||
func (v *loopingVendor) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
io.ReadAll(r.Body)
|
||||
w.Header().Set("Content-Type", "text/event-stream")
|
||||
@@ -282,8 +295,13 @@ func (v *loopingVendor) ServeHTTP(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
} else {
|
||||
for i := 0; v.lines == 0 || i < v.lines; i++ {
|
||||
if i > 200000 {
|
||||
v.letGo <- false
|
||||
if v.lines == 0 && i == loopMost {
|
||||
select {
|
||||
case <-r.Context().Done():
|
||||
v.letGo <- true
|
||||
case <-time.After(20 * time.Second):
|
||||
v.letGo <- false
|
||||
}
|
||||
return
|
||||
}
|
||||
v.sent.Add(1)
|
||||
@@ -387,11 +405,15 @@ func loopCut(t *testing.T, v *loopingVendor, got, done, failed, provider string)
|
||||
if !letGo {
|
||||
t.Fatalf("the vendor's request was not let go (%d lines)", v.sent.Load())
|
||||
}
|
||||
case <-time.After(5 * time.Second):
|
||||
case <-time.After(30 * time.Second):
|
||||
t.Fatal("the vendor's request was not let go")
|
||||
}
|
||||
if n := v.sent.Load(); n > 2000 {
|
||||
t.Fatalf("let go only after %d lines", n)
|
||||
// the guard cut within its window of the loop's start: what reached
|
||||
// the agent of the loop is that window and at most one 32K read of the
|
||||
// vendor's stream more, which the cut's write sends whole. It doesn't
|
||||
// depend on how far the vendor got ahead of the gateway (#1443).
|
||||
if n := loopLines(got); n > loopWindow+(32<<10)/100 {
|
||||
t.Fatalf("the loop was cut only after %d of its lines reached the agent", n)
|
||||
}
|
||||
restingUntil.Lock()
|
||||
rests := len(restingUntil.m)
|
||||
@@ -412,6 +434,16 @@ func loopCut(t *testing.T, v *loopingVendor, got, done, failed, provider string)
|
||||
}
|
||||
}
|
||||
|
||||
// loopLines counts the lines of the reporter's loop in what the agent got,
|
||||
// each a delta of its own in any protocol.
|
||||
func loopLines(got string) int {
|
||||
var n int
|
||||
for _, l := range []string{"Now.", "Writing.", "Go.", "Let me write.", "OK.", "Here.", "Produce.", "Final.", "Producing."} {
|
||||
n += strings.Count(got, `"`+l+`\n\n"`)
|
||||
}
|
||||
return n
|
||||
}
|
||||
|
||||
// The reporter's route (#1359): WorkBuddy's DeepSeek v4.1 flash at max
|
||||
// effort, from dsh and Pi. A WorkBuddy subscription is served by magpie's
|
||||
// built-in, or, once moved, by the community plugin's fetch in the plugin
|
||||
|
||||
Reference in new issue
Block a user