Skip to content

test(e2e): marker-routed multi-turn scripts + observer-scoped fixture clients (RIG-3528) #1029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rigel-mintaka wants to merge 1 commit into from
Open

test(e2e): marker-routed multi-turn scripts + observer-scoped fixture clients (RIG-3528) #1029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/designs/DECISIONS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,5 +460,6 @@ check enforces the mechanical half. Full rationale:
| DL-322 | Error/abort emit rule: `reason=error` emits `SessionError(ERROR)` AND the existing `ERRORED` lifecycle transition (additive — board/presence/delivery key off `ERRORED`); `reason=aborted` emits `SessionError(ABORTED)` with NO lifecycle transition (abort is not a crash) and replaces the prior counted-`UnmappedEvent` staging | Active (Matt, 2026-09-02) | [error/abort surfacing](agent/compass-agent-error-abort-surfacing/design.md) |
| DL-323 | The `SessionError` trace frame rides the FrameSink never-drop PRIORITY lane (not the bounded drop-oldest trace queue), via an `isSessionError` classifier extending the `frame-sink.ts` priority predicate — matching the `SessionInjection` never-drop carve-out, so surfaced failure content is as durable-on-the-spine as the lifecycle transition it reports | Active (Matt, 2026-09-02) | [error/abort surfacing](agent/compass-agent-error-abort-surfacing/design.md) |
| DL-341 | Compass UI visual regression is gated by built-in Playwright `toHaveScreenshot` (not a hosted visual-diff service), with baseline PNGs committed in-repo and regenerated ONLY by a pinned nix Chromium in CI via a `regen-visual-baselines` `workflow_dispatch` lane, never from a developer machine; wired as a `visual-gate` moon task on @playwright/test 1.62.1. Ruled at the design-PR gate: base `maxDiffPixelRatio` 0.001 with per-shot overrides `10/90` (state-dot) and `25/21357` (bridge-card) only; intentional visual changes regenerate baselines by dispatching the lane ON THE FEATURE BRANCH so the bot PR targets that branch; adjudication is GitHub's native committed-PNG image diff (2-up/swipe/onion-skin) on that bot PR, with the CI failure artifact retained only as the diagnostic path for an UNINTENDED red (untracked `e2e/.output` has no committed file to diff); hard gate from first landing, no advisory period; all 11 shots at v1 (RIG-2154) | Active (Matt, 2026-09-07) | [visual regression gate](ui/compass-visual-regression-gate/design.md) |
| DL-347 | Multi-actor comms coverage is tiered: cross-agent conversation, fan-out/isolation, and offline redelivery are asserted at the podman e2e tier with agent authorship carried by the real Runner session→account binding (no per-agent credential — `relay_comms.go:7-15`), and a non-admin observer client proves the credentialed transport path; the D9 visibility/leak matrix stays at the pgtest tier with e2e proving only one transport positive+negative. every agent-reachable relay arm is covered by a unit test at the runnerhub tier (podman is spent on the tool→relay→server JOIN, never on arm dispatch), and each native agent tool has at least one real-server execution. No new build tag or CI tier — new legs ride the existing e2e skip-guard | Active (Matt, 2026-09-08) | [multi-actor comms coverage](server/compass-comms-multi-actor-test-coverage/design.md#resolved-decisions) |
| DL-348 | The UI emits the PostHog session id on outbound Connect requests via a `sessionIdInterceptor` in `@compass/client` (sibling to `traceResponseInterceptor`), completing the J1 correlation-key seam whose inbound half stamps semconv `session.id` on backend spans. `X-POSTHOG-SESSION-ID` ONLY — `X-POSTHOG-DISTINCT-ID` is permanently excluded because it identifies a person and backend spans land in Grafana/Tempo, the plane J1 keeps identity out of; identity resolution stays PostHog-side where `identify()` holds it. Supersedes the 2026-09-05 HOLD (RIG-3233), whose two reasons — no inbound consumer, and a lazy-getter-over-nothing boot workaround — are discharged by the server merge and by the boot reorder respectively. posthog-js's own `TracingHeaders` extension is rejected: it monkey-patches global fetch/XHR and also sends the distinct-id header | Active (Matt, 2026-09-07) | [outbound session header](ui/compass-outbound-session-header/design.md#the-outbound-interceptor) |
| DL-349 | Boot constructs analytics BEFORE the live clients (`createAnalytics` → `createLiveClients` → `bootCaller` → `identify`), so the transport's session-id getter closes over a real `Analytics` rather than a mutable ref slot or a forward `let`. Rejected alternatives: mirroring the `clients.traceId` sink (a workaround for a construction-order problem the reorder deletes) and a forward `let analytics` (a TDZ-shaped `undefined` window every reader must guard). Accepted trade, stated not silent: on the WhoAmI-failure early return an analytics-enabled deployment now emits an anonymous PostHog session (init-time remote-config egress) where it previously emitted nothing — the reorder is unavoidable while the transport needs the getter at construction, and the egress itself is declined-not-absent (`advanced_disable_flags` would suppress it, at the cost of remote config), with nothing captured either way. The sender-side guard is printable ASCII + `.length ≤ 200`, STRICTER than the server's `≤200 bytes` + valid-UTF-8 pair and NARROWER than `Headers.set` itself, because `Headers.set` is a WebIDL ByteString: a well-formed id above U+00FF THROWS inside the interceptor and would fail the RPC, and U+0080–U+00FF is accepted by `set` but a browser then emits it as a SINGLE RAW HIGH BYTE on the wire, which fails Go's `utf8.ValidString`, so `sessionIDFromHeader` returns `""` and the id is DROPPED — silent loss, the same failure class as every other rejected value, NOT a wrong correlation key the server accepts (measured: raw-TCP wire bytes `736573732de9` from Node/undici, which serializes like a browser; and real Chromium → real Go `net/http` running a verbatim `sessionIDFromHeader` copy — `len` 6, `utf8.ValidString` false, result `""`. A Bun-client-to-`Bun.serve` round-trip measures Bun's own encode/decode pair, not the wire, and is NOT valid evidence here) | Active (Matt, 2026-09-07) | [outbound session header](ui/compass-outbound-session-header/design.md#the-boot-reorder-chosen--approved-by-matt) |
200 changes: 149 additions & 51 deletions go/e2e/cannedmodel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ func CannedToolCall(toolName, argsJSON string) CannedTurn {
// already routed.
type cannedMarker struct {
marker string
reply string
// turns is the ordered sequence this marker route serves, one per matching
// request. A one-element script is the single-text-reply form
// (newCannedMarker); a longer one is a multi-turn script
// (newCannedMarkerScript) whose TERMINAL element repeats once exhausted —
// see newCannedMarkerScript for why repeating (not 500ing) is the only
// terminating semantics for a marker route.
turns []CannedTurn
}

// newCannedMarker builds an off-script body-marker route: a request whose body
Expand All @@ -146,8 +152,36 @@ type cannedMarker struct {
// Use it via WithCannedMarkerReply to keep the ordered script drawn only by its
// own scripted turns when a shared-backend turn (e.g. a mention-driven steer or
// deliver) would otherwise race the counter.
//
// It is exactly the one-element case of newCannedMarkerScript: a single text
// turn whose terminal element repeats, i.e. EVERY matching request settles on
// reply — the pre-script behaviour, unchanged.
func newCannedMarker(marker, reply string) cannedMarker {
return cannedMarker{marker: marker, reply: reply}
return cannedMarker{marker: marker, turns: []CannedTurn{CannedText(reply)}}
}

// newCannedMarkerScript builds an off-script body-marker route that serves an
// ORDERED SEQUENCE of turns rather than one text reply (RIG-3528 T1): matching
// request N of this marker draws turns[N], and once the sequence is exhausted
// the TERMINAL element repeats for every later match. Like newCannedMarker it
// never advances the positional `served` counter, so a marker script and an
// ordered positional script coexist without racing.
//
// The per-marker counter and the repeating terminal element are BOTH
// load-bearing, and this is the design's sharpest hazard. A tool-call turn needs
// TWO model round-trips to settle — the tool-call turn, then the follow-up that
// settles on text — while a marker route matches a substring of the WHOLE
// request body and returns unconditionally. So a naive one-CannedTurn-per-marker
// route DOES NOT TERMINATE: every POST re-matches the marker and re-serves the
// tool call forever. Advancing a per-marker counter is what lets POST 2 draw the
// text settle; repeating the terminal element (rather than 500ing on exhaustion,
// as the positional path does) is what keeps a later re-match — a retry, a
// re-steer, an extra loop iteration — settled instead of erroring the agent out.
//
// Use it via WithCannedMarkerScript. Registering a marker with no turns is a
// construction error (startCannedModelServer): a route that can never settle.
func newCannedMarkerScript(marker string, turns ...CannedTurn) cannedMarker {
return cannedMarker{marker: marker, turns: turns}
}

// cannedModelServer is a running canned model backend. It owns its listener and
Expand All @@ -158,10 +192,10 @@ type cannedModelServer struct {
ln net.Listener
script []CannedTurn
// markers are the caller-supplied off-script body-marker routes (see
// cannedMarker / newCannedMarker): a request whose body contains a
// marker settles on its reply WITHOUT advancing the positional counter,
// generalizing setupTurnMarker. Checked after the built-in Setup marker,
// before the positional claim.
// cannedMarker / newCannedMarker / newCannedMarkerScript): a request whose
// body contains a marker draws that route's next turn WITHOUT advancing the
// positional counter, generalizing setupTurnMarker. Checked after the
// built-in Setup marker, before the positional claim.
markers []cannedMarker
port int
closeErr error
Expand All @@ -173,6 +207,14 @@ type cannedModelServer struct {
// but the stub guards the counter anyway.
servedMu sync.Mutex
served int
// markerServed is the PER-MARKER turn counter, parallel to markers by index:
// markerServed[i] is how many requests marker i has already served, so it
// draws markers[i].turns[markerServed[i]] and clamps to the terminal element
// once exhausted (newCannedMarkerScript). It is deliberately SEPARATE from
// served — a marker route must never consume a positional slot — and guarded
// by its own mutex for the same -race reason served is.
markerMu sync.Mutex
markerServed []int
}

// setupTurnMarker is a stable substring of the server's root-supervisor Setup
Expand Down Expand Up @@ -208,12 +250,19 @@ const setupReply = "canned setup turn settled OK"
// host-gateway); in the hermetic unit test it is loopback. It returns an error
// rather than panicking (rule://go-no-panic-in-lib) so the caller — a test —
// decides fatality. markers are optional off-script body-marker routes
// (newCannedMarker): a request whose body carries one settles on its reply
// without consuming a positional slot, additive to the built-in Setup marker.
// (newCannedMarker / newCannedMarkerScript): a request whose body carries one
// draws that route's next turn without consuming a positional slot, additive to
// the built-in Setup marker. A marker with no turns is a construction error, for
// the same reason an empty script is — a route that can never settle a turn.
func startCannedModelServer(bindAddr string, script []CannedTurn, markers ...cannedMarker) (*cannedModelServer, error) {
if len(script) == 0 {
return nil, errors.New("canned model server requires a non-empty script")
}
for _, m := range markers {
if len(m.turns) == 0 {
return nil, fmt.Errorf("canned model server marker %q requires at least one turn", m.marker)
}
}
ln, err := net.Listen("tcp", bindAddr)
if err != nil {
return nil, fmt.Errorf("canned model server listen on %q: %w", bindAddr, err)
Expand All @@ -223,7 +272,15 @@ func startCannedModelServer(bindAddr string, script []CannedTurn, markers ...can
_ = ln.Close() // failed construction; release the listener we just opened
return nil, fmt.Errorf("canned model server listener has unexpected addr type %T", ln.Addr())
}
c := &cannedModelServer{ln: ln, script: script, markers: markers, port: tcpAddr.Port}
c := &cannedModelServer{
ln: ln,
script: script,
markers: markers,
// One counter per marker, allocated up front so the handler indexes it
// without ever growing the slice under the lock.
markerServed: make([]int, len(markers)),
port: tcpAddr.Port,
}
mux := http.NewServeMux()
mux.HandleFunc(cannedChatPath, c.handleChatCompletions)
// ReadHeaderTimeout bounds a slow-header client (gosec G112); the canned
Expand Down Expand Up @@ -327,17 +384,40 @@ type chatToolCallFunction struct {
Arguments string `json:"arguments"`
}

// handleChatCompletions serves one scripted turn per request. It rejects a
// non-POST with 405 (the provider only ever POSTs) and writes an HTTP error —
// never panics — on any failure (rule://go-no-panic-in-lib). It claims the next
// script index under servedMu; a request past the end of the script is a test
// bug answered with a loud 500 naming exhaustion (never a hang or a default
// turn). The turn's body is either a text turn (a content chunk + a terminal
// finish_reason "stop") or a single-tool-call turn (a chunk whose
// delta.tool_calls carries one entry + a terminal finish_reason "tool_calls"),
// then the literal `data: [DONE]` sentinel. Each event is flushed immediately so
// the client's first-event watchdog sees bytes without waiting on the handler to
// return.
// claimMarkerTurn claims the next turn of marker route i and returns it plus the
// 0-based sequence number of this match (which only disambiguates an emitted
// tool-call id). Once the route's turns are exhausted the TERMINAL element is
// returned for every later match, and the counter keeps climbing so successive
// matches still get distinct call ids. The per-marker counter is guarded by
// markerMu — never servedMu, and never the positional `served` counter, since a
// marker route must not consume a positional slot. Callers hold no lock; the
// index is in range by construction (markerServed is allocated one-per-marker,
// and startCannedModelServer rejects a marker with no turns).
func (c *cannedModelServer) claimMarkerTurn(i int) (CannedTurn, int) {
c.markerMu.Lock()
seq := c.markerServed[i]
c.markerServed[i]++
c.markerMu.Unlock()
turns := c.markers[i].turns
if seq >= len(turns) {
return turns[len(turns)-1], seq
}
return turns[seq], seq
}

// handleChatCompletions serves one turn per request. It rejects a non-POST with
// 405 (the provider only ever POSTs) and writes an HTTP error — never panics —
// on any failure (rule://go-no-panic-in-lib). Routing is: the built-in Setup
// marker first, then the caller-supplied marker routes (each advancing its OWN
// counter and clamping to its terminal turn — claimMarkerTurn), then the
// positional script, whose next index is claimed under servedMu; a positional
// request past the end of the script is a test bug answered with a loud 500
// naming exhaustion (never a hang or a default turn). The turn's body is either
// a text turn (a content chunk + a terminal finish_reason "stop") or a
// single-tool-call turn (a chunk whose delta.tool_calls carries one entry + a
// terminal finish_reason "tool_calls"), then the literal `data: [DONE]`
// sentinel. Each event is flushed immediately so the client's first-event
// watchdog sees bytes without waiting on the handler to return.
func (c *cannedModelServer) handleChatCompletions(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodPost {
http.Error(w, "canned model backend only serves POST", http.StatusMethodNotAllowed)
Expand Down Expand Up @@ -368,14 +448,24 @@ func (c *cannedModelServer) handleChatCompletions(w http.ResponseWriter, r *http
return
}

// Caller-supplied off-script markers (newCannedMarker), checked after the
// built-in Setup marker and BEFORE the positional claim: a request whose body
// carries one settles on its reply without advancing the script counter, so a
// Caller-supplied off-script markers (newCannedMarker /
// newCannedMarkerScript), checked after the built-in Setup marker and BEFORE
// the positional claim: a request whose body carries one draws that route's
// next turn without advancing the positional script counter, so a
// shared-backend turn a leg does not want drawn off its ordered script (a
// mention-driven steer or deliver) is routed the same way the Setup turn is.
for _, m := range c.markers {
//
// Each marker advances its OWN counter and CLAMPS to its terminal turn once
// exhausted, which is what makes a multi-turn marker route terminate: a
// tool-call turn needs a second round-trip to settle, and that follow-up POST
// re-matches the same marker, so a route that always served turns[0] would
// re-serve the tool call forever (see newCannedMarkerScript). Clamping rather
// than 500ing keeps a later re-match settled — unlike the positional path, a
// marker route has no bounded request count to exhaust against.
for i, m := range c.markers {
if strings.Contains(string(body), m.marker) {
c.writeTextTurn(w, flusher, m.reply)
turn, seq := c.claimMarkerTurn(i)
c.writeCannedTurn(w, flusher, turn, seq)
return
}
}
Expand All @@ -393,36 +483,44 @@ func (c *cannedModelServer) handleChatCompletions(w http.ResponseWriter, r *http
return
}
turn := c.script[idx]
c.writeCannedTurn(w, flusher, turn, idx)
}

// writeCannedTurn serves one CannedTurn on the wire: a pure-text turn (a content
// chunk + a terminal finish_reason "stop") or a single-tool-call turn (a chunk
// whose delta.tool_calls carries one entry + a terminal finish_reason
// "tool_calls", which maps to the stopReason toolUse the agent loop gates tool
// execution on). callSeq only disambiguates the emitted tool-call id, so a
// transcript can tell two served calls apart; it carries no routing meaning.
// Shared by the positional-script path and the marker routes so both settle
// identically on the wire.
func (c *cannedModelServer) writeCannedTurn(w http.ResponseWriter, flusher http.Flusher, turn CannedTurn, callSeq int) {
if !turn.isToolCall {
c.writeTextTurn(w, flusher, turn.text)
return
}
const id = "canned-completion"
if turn.isToolCall {
// A single tool-call turn: one delta.tool_calls entry with a unique
// deterministic call id, then a terminal finish_reason "tool_calls" (maps
// to stopReason toolUse the agent loop gates tool execution on).
finish := finishToolCall
callID := fmt.Sprintf("call_%d", idx)
c.writeTurn(w, flusher, []chatChunk{
{ID: id, Object: chunkObject, Choices: []chatChoice{{
finish := finishToolCall
callID := fmt.Sprintf("call_%d", callSeq)
c.writeTurn(w, flusher, []chatChunk{
{ID: id, Object: chunkObject, Choices: []chatChoice{{
Index: 0,
Delta: chatDelta{Role: "assistant", ToolCalls: []chatToolCall{{
Index: 0,
Delta: chatDelta{Role: "assistant", ToolCalls: []chatToolCall{{
Index: 0,
ID: callID,
Type: "function",
Function: chatToolCallFunction{
Name: turn.toolName,
Arguments: turn.toolArgs,
},
}}},
ID: callID,
Type: "function",
Function: chatToolCallFunction{
Name: turn.toolName,
Arguments: turn.toolArgs,
},
}}},
{ID: id, Object: chunkObject, Choices: []chatChoice{{
Index: 0,
Delta: chatDelta{},
FinishReason: &finish,
}}},
})
return
}
c.writeTextTurn(w, flusher, turn.text)
}}},
{ID: id, Object: chunkObject, Choices: []chatChoice{{
Index: 0,
Delta: chatDelta{},
FinishReason: &finish,
}}},
})
}

// writeTextTurn serves a pure-text turn: a content chunk carrying reply then a
Expand Down
Loading
Loading