From 66b1cd6cef7d7d8808697f18a99751e006ccf85d Mon Sep 17 00:00:00 2001 From: DNT Date: Tue, 24 Mar 2026 23:03:19 +0700 Subject: [PATCH 01/15] chore(docker): make host ports customizable via environment variables --- docker-compose.otel.yml | 6 +++--- docker-compose.redis.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docker-compose.otel.yml b/docker-compose.otel.yml index 03c960ab..86c4e2e8 100644 --- a/docker-compose.otel.yml +++ b/docker-compose.otel.yml @@ -9,9 +9,9 @@ services: jaeger: image: jaegertracing/all-in-one:1.68.0 ports: - - "16686:16686" # Jaeger UI - - "4317:4317" # OTLP gRPC - - "4318:4318" # OTLP HTTP + - "${JAEGER_UI_PORT:-16686}:16686" # Jaeger UI + - "${OTLP_GRPC_PORT:-4317}:4317" # OTLP gRPC + - "${OTLP_HTTP_PORT:-4318}:4318" # OTLP HTTP environment: - COLLECTOR_OTLP_ENABLED=true restart: unless-stopped diff --git a/docker-compose.redis.yml b/docker-compose.redis.yml index 3c0660c3..408f39ec 100644 --- a/docker-compose.redis.yml +++ b/docker-compose.redis.yml @@ -8,7 +8,7 @@ services: redis: image: redis:7-alpine ports: - - "6379:6379" + - "${REDIS_PORT:-6379}:6379" volumes: - redis-data:/data command: redis-server --appendonly yes From 6c8ad12858d9a3c754c1624ea0b876814ed2db19 Mon Sep 17 00:00:00 2001 From: nguyenha935 Date: Sun, 21 Jun 2026 06:03:08 +0700 Subject: [PATCH 02/15] fix: unify NO_REPLY detection (#1233) Co-authored-by: GoClaw Operator --- cmd/gateway_cron.go | 25 +-------- cmd/gateway_cron_test.go | 2 + internal/agent/sanitize.go | 57 +++++++++++--------- internal/agent/sanitize_silent_reply_test.go | 8 ++- internal/pipeline/stages_test.go | 29 ++++++++++ 5 files changed, 71 insertions(+), 50 deletions(-) diff --git a/cmd/gateway_cron.go b/cmd/gateway_cron.go index 0ee72308..3b2b10d9 100644 --- a/cmd/gateway_cron.go +++ b/cmd/gateway_cron.go @@ -172,30 +172,7 @@ func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg } func cronOutputContainsNoReplySentinel(content string) bool { - text := strings.TrimSpace(content) - if text == "" { - return false - } - - const token = "NO_REPLY" - for i := 0; i+len(token) <= len(text); i++ { - if !strings.EqualFold(text[i:i+len(token)], token) { - continue - } - beforeOK := i == 0 || !cronNoReplyAlphaNumByte(text[i-1]) - after := i + len(token) - afterOK := after == len(text) || !cronNoReplyAlphaNumByte(text[after]) - if beforeOK && afterOK { - return true - } - } - return false -} - -func cronNoReplyAlphaNumByte(b byte) bool { - return (b >= 'a' && b <= 'z') || - (b >= 'A' && b <= 'Z') || - (b >= '0' && b <= '9') + return agent.IsSilentReply(content) } // resolveCronPeerKind infers peer kind from the cron job's user ID. diff --git a/cmd/gateway_cron_test.go b/cmd/gateway_cron_test.go index cd3f4735..a9d7e10b 100644 --- a/cmd/gateway_cron_test.go +++ b/cmd/gateway_cron_test.go @@ -76,6 +76,8 @@ func TestCronOutputContainsNoReplySentinel(t *testing.T) { {name: "exact", in: "NO_REPLY", want: true}, {name: "prefix explanation", in: "NO_REPLY - nothing to report", want: true}, {name: "suffix", in: "No relevant update. NO_REPLY", want: true}, + {name: "terminal glued punctuation", in: "This is directed at Bảo Ly Content, not me. I should stay silent.NO_REPLY", want: true}, + {name: "standalone suffix after space", in: "This is not for me. NO_REPLY", want: true}, {name: "mid sentence", in: "No changes found. NO_REPLY for this run.", want: true}, {name: "lowercase", in: "no_reply", want: true}, {name: "decorative underscore", in: "NO_REPLY_", want: true}, diff --git a/internal/agent/sanitize.go b/internal/agent/sanitize.go index a5e77841..2d2e9233 100644 --- a/internal/agent/sanitize.go +++ b/internal/agent/sanitize.go @@ -12,15 +12,14 @@ // - collapseConsecutiveDuplicateBlocks() // // Additional Go-specific: -// 5. stripEchoedSystemMessages() → strip hallucinated [System Message] blocks -// 6. stripGarbledToolXML() → strip garbled XML from models like DeepSeek +// 5. stripEchoedSystemMessages() → strip hallucinated [System Message] blocks +// 6. stripGarbledToolXML() → strip garbled XML from models like DeepSeek package agent import ( "log/slog" "regexp" "strings" - "unicode/utf8" "github.com/nextlevelbuilder/goclaw/internal/store" ) @@ -170,8 +169,10 @@ func stripDowngradedToolCallText(content string) string { // Matches TS stripThinkingTagsFromText() with strict mode. // Strips: ..., ..., -// ..., ..., -// ... +// +// ..., ..., +// ... +// // Go regexp doesn't support backreferences, so we use separate patterns. var thinkingTagPatterns = []*regexp.Regexp{ regexp.MustCompile(`(?is)]*>.*?`), @@ -378,17 +379,12 @@ func StripConfigLeak(content, agentType string) string { // --- NO_REPLY detection --- -// IsSilentReply checks if the text begins with a NO_REPLY token. +// IsSilentReply checks if the text contains a standalone NO_REPLY token. // -// Divergent from TS isSilentReplyText() (exact-match only) — we match broadly: -// decorative wrappers (`NO_REPLY_`, `"NO_REPLY"`, `**NO_REPLY**`) AND trailing -// explanations (`NO_REPLY because offline`, `NO_REPLY: note`) suppress delivery. -// Only requirement: the token is not glued to another word (`NO_REPLYING` is NOT silent). -// Case-insensitive. -// -// Trade-off vs upstream #19537: upstream guards against suppressing substantive -// replies that end in NO_REPLY. We accept that risk because observed model output -// leans toward "NO_REPLY + reason" rather than "real reply ending in NO_REPLY". +// Divergent from TS isSilentReplyText() (exact-match only) — we match broadly so +// both prefix forms (`NO_REPLY because offline`) and terminal sentinel forms +// (`not for me. NO_REPLY`) suppress delivery. The token must not be glued to +// another word (`NO_REPLYING` and `XNO_REPLY` are not silent). Case-insensitive. func IsSilentReply(text string) bool { trimmed := strings.TrimSpace(text) if trimmed == "" { @@ -397,18 +393,31 @@ func IsSilentReply(text string) bool { // Strip decorative wrappers from both ends (quotes, markdown emphasis, punctuation). stripped := strings.Trim(trimmed, "_ \t\n\r.,:;!?\"'`*~#>-()[]{}") const token = "NO_REPLY" - if len(stripped) < len(token) { + return containsStandaloneNoReplyToken(stripped, token) +} + +func containsStandaloneNoReplyToken(text, token string) bool { + if len(text) < len(token) { return false } - if !strings.EqualFold(stripped[:len(token)], token) { - return false + for i := 0; i+len(token) <= len(text); i++ { + if !strings.EqualFold(text[i:i+len(token)], token) { + continue + } + beforeOK := i == 0 || !isAlphaNumByte(text[i-1]) + after := i + len(token) + afterOK := after == len(text) || !isAlphaNumByte(text[after]) + if beforeOK && afterOK { + return true + } } - if len(stripped) == len(token) { - return true - } - // Token must not be glued to another word — next rune must be non-alphanumeric. - next, _ := utf8.DecodeRuneInString(stripped[len(token):]) - return !isAlphaNum(next) + return false +} + +func isAlphaNumByte(b byte) bool { + return (b >= 'a' && b <= 'z') || + (b >= 'A' && b <= 'Z') || + (b >= '0' && b <= '9') } func isAlphaNum(r rune) bool { diff --git a/internal/agent/sanitize_silent_reply_test.go b/internal/agent/sanitize_silent_reply_test.go index 05694026..900815e9 100644 --- a/internal/agent/sanitize_silent_reply_test.go +++ b/internal/agent/sanitize_silent_reply_test.go @@ -31,10 +31,14 @@ func TestIsSilentReply(t *testing.T) { {"prefix + space + content", "NO_REPLY hello", true}, {"prefix + colon + content", "NO_REPLY: offline", true}, {"prefix + because", "NO_REPLY because user is away", true}, + {"reason then terminal sentinel", "This is directed at Bảo Ly Content, not me. I should stay silent.NO_REPLY", true}, + {"suffix with whitespace", "This is not for me. NO_REPLY", true}, + {"suffix with newline", "This is not for me.\nNO_REPLY", true}, // NOT silent — token glued to another word, or not at start. {"embedded word", "NO_REPLYING", false}, - {"trailing after content", "Here you go. NO_REPLY", false}, - {"token mid-sentence", "Hello NO_REPLY world", false}, + {"glued suffix stays non-silent", "This is not for me.NO_REPLYING", false}, + {"trailing after content", "Here you go. NO_REPLY", true}, + {"token mid-sentence", "Hello NO_REPLY world", true}, {"empty", "", false}, {"whitespace only", " ", false}, {"unrelated text", "no reply needed", false}, diff --git a/internal/pipeline/stages_test.go b/internal/pipeline/stages_test.go index 75b90e1c..eb7087f3 100644 --- a/internal/pipeline/stages_test.go +++ b/internal/pipeline/stages_test.go @@ -2440,6 +2440,35 @@ func TestFinalizeStage_SanitizesContent(t *testing.T) { } } +func TestFinalizeStage_SuppressesSilentReplyAfterFlush(t *testing.T) { + t.Parallel() + finalContent := "This is directed at Bảo Ly Content, not me. I should stay silent.NO_REPLY" + var flushed []providers.Message + deps := &PipelineDeps{ + IsSilentReply: func(content string) bool { + return content == finalContent + }, + FlushMessages: func(_ context.Context, _ string, messages []providers.Message) error { + flushed = append(flushed, messages...) + return nil + }, + } + stage := NewFinalizeStage(deps) + state := defaultState() + state.Observe.FinalContent = finalContent + + err := stage.Execute(context.Background(), state) + if err != nil { + t.Fatalf("Execute() error: %v", err) + } + if state.Observe.FinalContent != "" { + t.Fatalf("FinalContent = %q, want empty after silent reply suppression", state.Observe.FinalContent) + } + if len(flushed) == 0 || flushed[len(flushed)-1].Content != finalContent { + t.Fatalf("flushed final content = %#v, want original silent reply persisted before suppression", flushed) + } +} + func TestFinalizeStage_DeduplicatesMediaByPath(t *testing.T) { t.Parallel() deps := &PipelineDeps{} From 05d790662aee2efc82cdee46f5ed77ce72b4e45e Mon Sep 17 00:00:00 2001 From: codebit0 <34156842+codebit0@users.noreply.github.com> Date: Sun, 21 Jun 2026 08:33:15 +0900 Subject: [PATCH 03/15] feat(acp): surface session/update tool_call notifications at Info level (#1141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit session/update notifications carrying a ToolCall (or inline tool_call / tool_call_update kind) were only dumped at Debug level inside the params blob. Operators could see `security.tool_granted` (permission granted) but had no way to tell whether the tool actually executed successfully — both "permission granted then failed silently" and "permission granted and succeeded" looked identical in journalctl. Add a structured Info log emitting toolCallId, name/title, status, and a content preview (truncated at 400 chars) whenever the notification contains tool-call state. This is what made it possible to diagnose the recent .goclaw/-path-deny regression — `status=failed` immediately after `security.tool_granted` revealed the gap that the granted-only log hid. No behavior change beyond logging volume; preview is bounded. Co-authored-by: Claude Opus 4.7 (1M context) --- internal/providers/acp/process.go | 32 +++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/internal/providers/acp/process.go b/internal/providers/acp/process.go index 7e34de51..9a404eed 100644 --- a/internal/providers/acp/process.go +++ b/internal/providers/acp/process.go @@ -216,6 +216,38 @@ func (pp *ProcessPool) spawn(ctx context.Context, poolKey string) (*ACPProcess, slog.Warn("acp: session/update parse failed", "error", err) return } + // Surface tool-call notifications at Info level so operators can + // distinguish "permission granted" (security.tool_granted) from + // "tool actually executed / produced output". Without this the + // status transitions are buried in Debug-level params dumps. + if update.ToolCall != nil { + preview := "" + for _, c := range update.ToolCall.Content { + if c.Type == "text" && c.Text != "" { + preview += c.Text + if len(preview) > 400 { + break + } + } + } + if len(preview) > 400 { + preview = preview[:400] + "...(truncated)" + } + slog.Info("acp: tool_call_update", + "sid", update.SessionID, + "id", update.ToolCall.ID, + "name", update.ToolCall.Name, + "status", update.ToolCall.Status, + "content_preview", preview) + } + if update.Update.SessionUpdate == "tool_call" || update.Update.SessionUpdate == "tool_call_update" { + slog.Info("acp: tool_call_update (inline)", + "sid", update.SessionID, + "id", update.Update.ToolCallID, + "title", update.Update.Title, + "kind", update.Update.Kind, + "status", update.Update.Status) + } proc.dispatchUpdate(update) } } From d5033b4aaf66beb6c7ffafe6a15b6920f1acfe13 Mon Sep 17 00:00:00 2001 From: codebit0 <34156842+codebit0@users.noreply.github.com> Date: Sun, 21 Jun 2026 09:03:48 +0900 Subject: [PATCH 04/15] fix(exec): exempt venv python interpreter from .goclaw/ path deny (#1140) The ExecTool path-deny rule blocks any token containing `.goclaw/` unless it matches one of the AllowPathExemptions prefixes (skills-store, tenants). This silently rejected legitimate commands invoking the goclaw-managed Python interpreter via its absolute path: /home/user/.goclaw/venv/bin/python3 .../script.py The first token `/home/user/.goclaw/venv/bin/python3` matched the deny pattern but no exemption, so the entire command was denied. Naive exemption (".goclaw/venv/bin/") does not work: matchesAnyPathExemption resolves both tokens and exemption candidates via EvalSymlinks, and the venv's python3 is a symlink into the host's python cellar (e.g. linuxbrew). The token canonicalizes to /home/linuxbrew/.../python3.14 while a literal ".goclaw/venv/bin/" prefix never gets touched. Fix: resolve venv/bin/python3 once at startup and exempt the dirname of the resolved target. Failure to resolve (no venv present) silently falls through. Without this, ACP-driven agents either fail outright or work only via fragile heuristics (cwd-local symlinks generated on the fly by the LLM). Co-authored-by: Claude Opus 4.7 (1M context) --- cmd/gateway_setup.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/gateway_setup.go b/cmd/gateway_setup.go index 3f6ac2a4..1e212e45 100644 --- a/cmd/gateway_setup.go +++ b/cmd/gateway_setup.go @@ -222,6 +222,14 @@ func setupToolRegistry( filepath.Join(dataDir, "skills-store")+"/", filepath.Join(dataDir, "tenants")+"/", ) + // Allow the goclaw-managed Python venv interpreter to be invoked with its + // absolute path. venv/bin/python3 is a symlink to the real interpreter + // (e.g. linuxbrew cellar), and matchesAnyPathExemption resolves symlinks + // before comparing — so we must exempt the *resolved* target dir. + // Resolved at startup; falls back silently if venv not present. + if real, err := filepath.EvalSymlinks(filepath.Join(filepath.Dir(dataDir), "venv", "bin", "python3")); err == nil { + et.AllowPathExemptions(filepath.Dir(real) + "/") + } // Harden: block access to internal workspace files via shell commands. // Prevents `cat ../config.json`, `cat memory.db` etc. from user workspaces. et.DenyPaths( From fabe86b3de71f82d1548fba54f8f6d7122518cd5 Mon Sep 17 00:00:00 2001 From: codebit0 <34156842+codebit0@users.noreply.github.com> Date: Sun, 21 Jun 2026 09:36:28 +0900 Subject: [PATCH 05/15] fix(cron): invert stateless gate to match UI label (#1139) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cron job handler reset the session when stateless=false and skipped reset when stateless=true — the opposite of what every UI locale labels the field (en/ko/zh/vi all describe stateless as "each run starts fresh without loading previous messages"). The buggy gate caused stateless=true crons to silently accumulate session history across every execution, leading to context bloat and increasing the chance of LLMs short-circuiting tool calls in favor of replaying prior assistant turns. One affected daily ETL cron grew to 38 messages over 18 days before the regression was noticed. Fix: gate the Reset on `if job.Stateless` so the runtime matches the UI contract. No DB migration is required — existing values were set by users based on the UI label, so they already encode the intended behavior. Co-authored-by: Claude Opus 4.7 (1M context) --- cmd/gateway_cron.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/gateway_cron.go b/cmd/gateway_cron.go index 3b2b10d9..0b64444b 100644 --- a/cmd/gateway_cron.go +++ b/cmd/gateway_cron.go @@ -88,11 +88,14 @@ func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg cronCtx = store.WithCredentialUserID(cronCtx, job.Payload.CredentialUserID) } - // Reset session before each cron run to prevent tool errors from previous - // runs from polluting the context and blocking future executions (#294). - // Save() persists the empty session to DB so stale data won't reload after restart. - // Stateless jobs skip this — they intentionally carry no session history. - if !job.Stateless { + // Stateless jobs explicitly reset their session before each run — they + // carry no history between executions, matching the "Stateless" UI label + // and statelessHelp ("each run starts fresh without loading previous + // messages"). Stateful (non-stateless) jobs keep prior turns to enable + // multi-run dialog. + // Save() persists the empty session to DB so stale data won't reload + // after restart (#294). + if job.Stateless { sessionMgr.Reset(cronCtx, sessionKey) sessionMgr.Save(cronCtx, sessionKey) } From 360fe30a8ee8665090b2b2913cddbd8387bdbcd6 Mon Sep 17 00:00:00 2001 From: Zezae Oh Date: Fri, 3 Jul 2026 09:54:40 +0900 Subject: [PATCH 06/15] fix(providers): retry transient Codex response failures (#1332) * fix(providers): retry transient Codex response failures * test(cron): tolerate nil session store in handler tests --- cmd/gateway_cron.go | 2 +- internal/providers/codex.go | 69 +++++++++++++++++++++++++---- internal/providers/codex_test.go | 75 ++++++++++++++++++++++++++++++++ internal/providers/retry.go | 12 ++++- internal/providers/retry_test.go | 2 + 5 files changed, 148 insertions(+), 12 deletions(-) diff --git a/cmd/gateway_cron.go b/cmd/gateway_cron.go index 0b64444b..36c1309e 100644 --- a/cmd/gateway_cron.go +++ b/cmd/gateway_cron.go @@ -95,7 +95,7 @@ func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg // multi-run dialog. // Save() persists the empty session to DB so stale data won't reload // after restart (#294). - if job.Stateless { + if job.Stateless && sessionMgr != nil { sessionMgr.Reset(cronCtx, sessionKey) sessionMgr.Save(cronCtx, sessionKey) } diff --git a/internal/providers/codex.go b/internal/providers/codex.go index 8687cbb5..3acdc0c9 100644 --- a/internal/providers/codex.go +++ b/internal/providers/codex.go @@ -5,9 +5,9 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" "strings" + "time" ) type CodexRoutingDefaults struct { @@ -131,6 +131,42 @@ func (p *CodexProvider) middlewareConfig(req ChatRequest) MiddlewareConfig { } func (p *CodexProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk func(StreamChunk)) (*ChatResponse, error) { + cfg := p.retryConfig + if cfg.Attempts <= 0 { + cfg.Attempts = 1 + } + + var lastErr error + for attempt := 1; attempt <= cfg.Attempts; attempt++ { + result, emitted, err := p.chatStreamOnce(ctx, req, onChunk) + if err == nil { + return result, nil + } + lastErr = err + + // If any user-visible chunk already escaped, do not replay the run: retrying + // could duplicate streamed text/tool calls/images. Pre-output Codex backend + // failures (e.g. response.failed: "processing your request…retry") are safe + // to retry here and are exactly the flaky 429-ish case this guard targets. + if emitted || !IsRetryableError(err) || attempt == cfg.Attempts { + return result, err + } + + delay := computeDelay(cfg, attempt, err) + if hook := retryHookFromContext(ctx); hook != nil { + hook(attempt, cfg.Attempts, err) + } + select { + case <-ctx.Done(): + return result, ctx.Err() + case <-time.After(delay): + } + } + + return nil, lastErr +} + +func (p *CodexProvider) chatStreamOnce(ctx context.Context, req ChatRequest, onChunk func(StreamChunk)) (*ChatResponse, bool, error) { // stripThinking: drop reasoning summaries from ChatResponse.Thinking and // onChunk callbacks. Usage.ThinkingTokens is still populated from the // final response.usage payload (Phase 1 billing accuracy). @@ -138,11 +174,9 @@ func (p *CodexProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk body := p.buildRequestBody(req, true) body = ApplyMiddlewares(body, p.middlewares, p.middlewareConfig(req)) - respBody, err := RetryDo(ctx, p.retryConfig, func() (io.ReadCloser, error) { - return p.doRequest(ctx, body) - }) + respBody, err := p.doRequest(ctx, body) if err != nil { - return nil, err + return nil, false, err } // Wrap respBody so ctx cancellation closes the socket, unblocking bufio.Scanner. cb := NewCtxBody(ctx, respBody) @@ -152,6 +186,16 @@ func (p *CodexProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk toolCalls := make(map[string]*codexToolCallAcc) // keyed by item_id streamState := newCodexMessageStreamState() imageState := newCodexImageState() + emitted := false + wrappedOnChunk := onChunk + if onChunk != nil { + wrappedOnChunk = func(chunk StreamChunk) { + if chunk.Content != "" || chunk.Thinking != "" || len(chunk.Images) > 0 { + emitted = true + } + onChunk(chunk) + } + } sse := NewSSEScanner(cb) for sse.Next() { @@ -162,13 +206,13 @@ func (p *CodexProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk continue } - if err := p.processSSEEvent(&event, result, toolCalls, streamState, imageState, onChunk, stripThinking); err != nil { - return nil, err + if err := p.processSSEEvent(&event, result, toolCalls, streamState, imageState, wrappedOnChunk, stripThinking); err != nil { + return result, emitted || codexResultHasVisibleOutput(result), err } } if err := sse.Err(); err != nil { - return result, fmt.Errorf("%s: stream read error: %w", p.name, err) + return result, emitted || codexResultHasVisibleOutput(result), fmt.Errorf("%s: stream read error: %w", p.name, err) } // Assemble generated images from image accumulator into ChatResponse. @@ -202,7 +246,14 @@ func (p *CodexProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk onChunk(StreamChunk{Done: true}) } - return result, nil + return result, emitted || codexResultHasVisibleOutput(result), nil +} + +func codexResultHasVisibleOutput(result *ChatResponse) bool { + if result == nil { + return false + } + return result.Content != "" || result.Thinking != "" || len(result.ToolCalls) > 0 || len(result.Images) > 0 } // processSSEEvent handles a single SSE event during streaming. diff --git a/internal/providers/codex_test.go b/internal/providers/codex_test.go index b59a5ad9..fe9fa843 100644 --- a/internal/providers/codex_test.go +++ b/internal/providers/codex_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" ) // staticTokenSource implements TokenSource for testing. @@ -1083,3 +1084,77 @@ func TestCodexProviderBuildRequestBodyWithImages(t *testing.T) { t.Errorf("content[1] type = %v, want input_text", content[1]["type"]) } } + +func TestCodexProviderRetriesPreOutputResponseFailed(t *testing.T) { + var calls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + w.Header().Set("Content-Type", "text/event-stream") + flusher := w.(http.Flusher) + if calls == 1 { + fmt.Fprintf(w, "data: %s\n\n", mustJSON(codexSSEEvent{ + Type: "response.failed", + Response: &codexAPIResponse{Error: &codexErrorDetail{Message: "An error occurred while processing your request. You can retry your request."}}, + })) + flusher.Flush() + return + } + fmt.Fprintf(w, "data: %s\n\n", mustJSON(codexSSEEvent{Type: "response.output_text.delta", Delta: "ok"})) + fmt.Fprintf(w, "data: %s\n\n", mustJSON(codexSSEEvent{Type: "response.completed", Response: &codexAPIResponse{Status: "completed"}})) + fmt.Fprint(w, "data: [DONE]\n\n") + flusher.Flush() + })) + defer server.Close() + + p := NewCodexProvider("test", &staticTokenSource{token: "tok"}, server.URL, "gpt-4o") + p.retryConfig = RetryConfig{Attempts: 2, MinDelay: time.Millisecond, MaxDelay: time.Millisecond} + + result, err := p.ChatStream(context.Background(), ChatRequest{Messages: []Message{{Role: "user", Content: "hi"}}}, nil) + if err != nil { + t.Fatalf("ChatStream: %v", err) + } + if calls != 2 { + t.Fatalf("calls = %d, want 2", calls) + } + if result.Content != "ok" { + t.Fatalf("content = %q, want ok", result.Content) + } +} + +func TestCodexProviderDoesNotRetryAfterVisibleOutput(t *testing.T) { + var calls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + w.Header().Set("Content-Type", "text/event-stream") + flusher := w.(http.Flusher) + fmt.Fprintf(w, "data: %s\n\n", mustJSON(codexSSEEvent{Type: "response.output_text.delta", Delta: "partial"})) + fmt.Fprintf(w, "data: %s\n\n", mustJSON(codexSSEEvent{ + Type: "response.failed", + Response: &codexAPIResponse{Error: &codexErrorDetail{Message: "An error occurred while processing your request. You can retry your request."}}, + })) + flusher.Flush() + })) + defer server.Close() + + p := NewCodexProvider("test", &staticTokenSource{token: "tok"}, server.URL, "gpt-4o") + p.retryConfig = RetryConfig{Attempts: 2, MinDelay: time.Millisecond, MaxDelay: time.Millisecond} + + var chunks []string + result, err := p.ChatStream(context.Background(), ChatRequest{Messages: []Message{{Role: "user", Content: "hi"}}}, func(chunk StreamChunk) { + if chunk.Content != "" { + chunks = append(chunks, chunk.Content) + } + }) + if err == nil { + t.Fatal("expected error") + } + if calls != 1 { + t.Fatalf("calls = %d, want 1", calls) + } + if result == nil || result.Content != "partial" { + t.Fatalf("result content = %#v, want partial", result) + } + if len(chunks) != 1 || chunks[0] != "partial" { + t.Fatalf("chunks = %#v, want [partial]", chunks) + } +} diff --git a/internal/providers/retry.go b/internal/providers/retry.go index 9935c82f..de06e045 100644 --- a/internal/providers/retry.go +++ b/internal/providers/retry.go @@ -83,12 +83,20 @@ func IsRetryableError(err error) bool { return true // includes timeouts } - // Check for connection reset / broken pipe / EOF in error string + // Check for connection reset / broken pipe / EOF in error string. + // Some streaming providers (notably Codex/Responses) surface transient + // backend failures as SSE `response.failed` events instead of HTTP 429/5xx. + // Treat explicit retry guidance/rate-limit wording as retryable too. errStr := err.Error() + lowerErr := strings.ToLower(errStr) if strings.Contains(errStr, "connection reset") || strings.Contains(errStr, "broken pipe") || strings.Contains(errStr, "EOF") || - strings.Contains(errStr, "timeout") { + strings.Contains(lowerErr, "timeout") || + strings.Contains(lowerErr, "rate limit") || + strings.Contains(lowerErr, "429") || + (strings.Contains(lowerErr, "response failed") && strings.Contains(lowerErr, "retry")) || + strings.Contains(lowerErr, "while processing your request") { return true } diff --git a/internal/providers/retry_test.go b/internal/providers/retry_test.go index bd361d29..806a7ae2 100644 --- a/internal/providers/retry_test.go +++ b/internal/providers/retry_test.go @@ -31,6 +31,8 @@ func TestIsRetryableError(t *testing.T) { {"broken_pipe", errors.New("write: broken pipe"), true}, {"eof", errors.New("unexpected EOF"), true}, {"timeout_string", errors.New("i/o timeout"), true}, + {"codex_retry_guidance", errors.New("codex: response failed: An error occurred while processing your request. You can retry your request, or contact us through our help center."), true}, + {"codex_rate_limit_wording", errors.New("codex: response failed: rate limit exceeded"), true}, {"generic_error", errors.New("something went wrong"), false}, {"wrapped_retryable", fmt.Errorf("provider: %w", &HTTPError{Status: 429}), true}, {"wrapped_non_retryable", fmt.Errorf("provider: %w", &HTTPError{Status: 400}), false}, From 099ff853a206bb9823284e7702c862ef3a36e1b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=8A=E3=83=A0?= Date: Sun, 5 Jul 2026 19:59:52 +0800 Subject: [PATCH 07/15] fix(background): clear stale provider alerts (#1362) Co-authored-by: Collective Developer --- internal/consolidation/dreaming_worker.go | 1 + internal/consolidation/episodic_worker.go | 1 + internal/consolidation/semantic_worker.go | 1 + internal/consolidation/workers_test.go | 216 +++++++++++++++++----- internal/vault/enrich_worker.go | 1 + 5 files changed, 174 insertions(+), 46 deletions(-) diff --git a/internal/consolidation/dreaming_worker.go b/internal/consolidation/dreaming_worker.go index 4a981793..980413dd 100644 --- a/internal/consolidation/dreaming_worker.go +++ b/internal/consolidation/dreaming_worker.go @@ -230,6 +230,7 @@ func (w *dreamingWorker) synthesize(ctx context.Context, provider providers.Prov if err != nil { return "", fmt.Errorf("dreaming chat: %w", err) } + bgalert.ClearProviderError(ctx, w.alertDeps.SystemConfigs) return resp.Content, nil } diff --git a/internal/consolidation/episodic_worker.go b/internal/consolidation/episodic_worker.go index b93a5383..87cb2238 100644 --- a/internal/consolidation/episodic_worker.go +++ b/internal/consolidation/episodic_worker.go @@ -187,5 +187,6 @@ func (w *episodicWorker) summarizeFromMessages(ctx context.Context, provider pro if err != nil { return "", err } + bgalert.ClearProviderError(ctx, w.alertDeps.SystemConfigs) return resp.Content, nil } diff --git a/internal/consolidation/semantic_worker.go b/internal/consolidation/semantic_worker.go index fe243082..b8d073fd 100644 --- a/internal/consolidation/semantic_worker.go +++ b/internal/consolidation/semantic_worker.go @@ -45,6 +45,7 @@ func (w *semanticWorker) Handle(ctx context.Context, event eventbus.DomainEvent) slog.Warn("semantic: extraction failed", "episodic_id", payload.EpisodicID, "err", err) return nil // non-fatal: extraction failure doesn't block pipeline } + bgalert.ClearProviderError(ctx, w.alertDeps.SystemConfigs) if len(result.Entities) == 0 && len(result.Relations) == 0 { return nil } diff --git a/internal/consolidation/workers_test.go b/internal/consolidation/workers_test.go index 983a6efc..42b7d835 100644 --- a/internal/consolidation/workers_test.go +++ b/internal/consolidation/workers_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/bgalert" "github.com/nextlevelbuilder/goclaw/internal/eventbus" "github.com/nextlevelbuilder/goclaw/internal/knowledgegraph" "github.com/nextlevelbuilder/goclaw/internal/providers" @@ -153,31 +154,57 @@ func (m *mockKGStore) DedupAfterExtraction(_ context.Context, _, _ string, _ []s // Implement remaining store.KnowledgeGraphStore methods func (m *mockKGStore) UpsertEntity(context.Context, *store.Entity) error { return nil } -func (m *mockKGStore) GetEntity(context.Context, string, string, string) (*store.Entity, error) { return nil, nil } +func (m *mockKGStore) GetEntity(context.Context, string, string, string) (*store.Entity, error) { + return nil, nil +} func (m *mockKGStore) DeleteEntity(context.Context, string, string, string) error { return nil } -func (m *mockKGStore) ListEntities(context.Context, string, string, store.EntityListOptions) ([]store.Entity, error) { return nil, nil } -func (m *mockKGStore) SearchEntities(context.Context, string, string, string, int) ([]store.Entity, error) { return nil, nil } -func (m *mockKGStore) UpsertRelation(context.Context, *store.Relation) error { return nil } +func (m *mockKGStore) ListEntities(context.Context, string, string, store.EntityListOptions) ([]store.Entity, error) { + return nil, nil +} +func (m *mockKGStore) SearchEntities(context.Context, string, string, string, int) ([]store.Entity, error) { + return nil, nil +} +func (m *mockKGStore) UpsertRelation(context.Context, *store.Relation) error { return nil } func (m *mockKGStore) DeleteRelation(context.Context, string, string, string) error { return nil } -func (m *mockKGStore) ListRelations(context.Context, string, string, string) ([]store.Relation, error) { return nil, nil } -func (m *mockKGStore) ListAllRelations(context.Context, string, string, int) ([]store.Relation, error) { return nil, nil } -func (m *mockKGStore) Traverse(context.Context, string, string, string, int) ([]store.TraversalResult, error) { return nil, nil } -func (m *mockKGStore) PruneByConfidence(context.Context, string, string, float64) (int, error) { return 0, nil } -func (m *mockKGStore) ScanDuplicates(context.Context, string, string, float64, int) (int, error) { return 0, nil } -func (m *mockKGStore) ListDedupCandidates(context.Context, string, string, int) ([]store.DedupCandidate, error) { return nil, nil } -func (m *mockKGStore) MergeEntities(context.Context, string, string, string, string) error { return nil } +func (m *mockKGStore) ListRelations(context.Context, string, string, string) ([]store.Relation, error) { + return nil, nil +} +func (m *mockKGStore) ListAllRelations(context.Context, string, string, int) ([]store.Relation, error) { + return nil, nil +} +func (m *mockKGStore) Traverse(context.Context, string, string, string, int) ([]store.TraversalResult, error) { + return nil, nil +} +func (m *mockKGStore) PruneByConfidence(context.Context, string, string, float64) (int, error) { + return 0, nil +} +func (m *mockKGStore) ScanDuplicates(context.Context, string, string, float64, int) (int, error) { + return 0, nil +} +func (m *mockKGStore) ListDedupCandidates(context.Context, string, string, int) ([]store.DedupCandidate, error) { + return nil, nil +} +func (m *mockKGStore) MergeEntities(context.Context, string, string, string, string) error { + return nil +} func (m *mockKGStore) DismissCandidate(context.Context, string, string) error { return nil } -func (m *mockKGStore) Stats(context.Context, string, string) (*store.GraphStats, error) { return nil, nil } -func (m *mockKGStore) ListEntitiesTemporal(context.Context, string, string, store.EntityListOptions, store.TemporalQueryOptions) ([]store.Entity, error) { return nil, nil } -func (m *mockKGStore) SupersedeEntity(context.Context, *store.Entity, *store.Entity) error { return nil } +func (m *mockKGStore) Stats(context.Context, string, string) (*store.GraphStats, error) { + return nil, nil +} +func (m *mockKGStore) ListEntitiesTemporal(context.Context, string, string, store.EntityListOptions, store.TemporalQueryOptions) ([]store.Entity, error) { + return nil, nil +} +func (m *mockKGStore) SupersedeEntity(context.Context, *store.Entity, *store.Entity) error { + return nil +} func (m *mockKGStore) SetEmbeddingProvider(store.EmbeddingProvider) {} -func (m *mockKGStore) Close() error { return nil } +func (m *mockKGStore) Close() error { return nil } // mockMemoryStore implements store.MemoryStore for testing. type mockMemoryStore struct { - docs map[string]string - indexed map[string]bool - mu sync.Mutex + docs map[string]string + indexed map[string]bool + mu sync.Mutex } func newMockMemoryStore() *mockMemoryStore { @@ -202,17 +229,31 @@ func (m *mockMemoryStore) IndexDocument(_ context.Context, _, _, path string) er } // Implement remaining store.MemoryStore methods -func (m *mockMemoryStore) GetDocument(context.Context, string, string, string) (string, error) { return "", nil } +func (m *mockMemoryStore) GetDocument(context.Context, string, string, string) (string, error) { + return "", nil +} func (m *mockMemoryStore) DeleteDocument(context.Context, string, string, string) error { return nil } -func (m *mockMemoryStore) ListDocuments(context.Context, string, string) ([]store.DocumentInfo, error) { return nil, nil } -func (m *mockMemoryStore) ListAllDocumentsGlobal(context.Context) ([]store.DocumentInfo, error) { return nil, nil } -func (m *mockMemoryStore) ListAllDocuments(context.Context, string) ([]store.DocumentInfo, error) { return nil, nil } -func (m *mockMemoryStore) GetDocumentDetail(context.Context, string, string, string) (*store.DocumentDetail, error) { return nil, nil } -func (m *mockMemoryStore) ListChunks(context.Context, string, string, string) ([]store.ChunkInfo, error) { return nil, nil } -func (m *mockMemoryStore) Search(context.Context, string, string, string, store.MemorySearchOptions) ([]store.MemorySearchResult, error) { return nil, nil } +func (m *mockMemoryStore) ListDocuments(context.Context, string, string) ([]store.DocumentInfo, error) { + return nil, nil +} +func (m *mockMemoryStore) ListAllDocumentsGlobal(context.Context) ([]store.DocumentInfo, error) { + return nil, nil +} +func (m *mockMemoryStore) ListAllDocuments(context.Context, string) ([]store.DocumentInfo, error) { + return nil, nil +} +func (m *mockMemoryStore) GetDocumentDetail(context.Context, string, string, string) (*store.DocumentDetail, error) { + return nil, nil +} +func (m *mockMemoryStore) ListChunks(context.Context, string, string, string) ([]store.ChunkInfo, error) { + return nil, nil +} +func (m *mockMemoryStore) Search(context.Context, string, string, string, store.MemorySearchOptions) ([]store.MemorySearchResult, error) { + return nil, nil +} func (m *mockMemoryStore) IndexAll(context.Context, string, string) error { return nil } -func (m *mockMemoryStore) SetEmbeddingProvider(store.EmbeddingProvider) {} -func (m *mockMemoryStore) Close() error { return nil } +func (m *mockMemoryStore) SetEmbeddingProvider(store.EmbeddingProvider) {} +func (m *mockMemoryStore) Close() error { return nil } // mockSessionStore implements store.SessionCoreStore for testing. type mockSessionStore struct { @@ -221,8 +262,8 @@ type mockSessionStore struct { } func (m *mockSessionStore) GetOrCreate(context.Context, string) *store.SessionData { return nil } -func (m *mockSessionStore) Get(context.Context, string) *store.SessionData { return nil } -func (m *mockSessionStore) AddMessage(context.Context, string, providers.Message) {} +func (m *mockSessionStore) Get(context.Context, string) *store.SessionData { return nil } +func (m *mockSessionStore) AddMessage(context.Context, string, providers.Message) {} func (m *mockSessionStore) GetHistory(_ context.Context, _ string) []providers.Message { return m.history @@ -232,15 +273,58 @@ func (m *mockSessionStore) GetSummary(_ context.Context, _ string) string { return m.summary } -func (m *mockSessionStore) SetSummary(context.Context, string, string) {} -func (m *mockSessionStore) GetLabel(context.Context, string) string { return "" } -func (m *mockSessionStore) SetLabel(context.Context, string, string) {} +func (m *mockSessionStore) SetSummary(context.Context, string, string) {} +func (m *mockSessionStore) GetLabel(context.Context, string) string { return "" } +func (m *mockSessionStore) SetLabel(context.Context, string, string) {} func (m *mockSessionStore) SetAgentInfo(context.Context, string, uuid.UUID, string) {} -func (m *mockSessionStore) TruncateHistory(context.Context, string, int) {} +func (m *mockSessionStore) TruncateHistory(context.Context, string, int) {} func (m *mockSessionStore) SetHistory(context.Context, string, []providers.Message) {} -func (m *mockSessionStore) Reset(context.Context, string) {} -func (m *mockSessionStore) Delete(context.Context, string) error { return nil } -func (m *mockSessionStore) Save(context.Context, string) error { return nil } +func (m *mockSessionStore) Reset(context.Context, string) {} +func (m *mockSessionStore) Delete(context.Context, string) error { return nil } +func (m *mockSessionStore) Save(context.Context, string) error { return nil } + +type mockSystemConfigStore struct { + data map[string]string + deleteKeys []string + mu sync.Mutex +} + +func (m *mockSystemConfigStore) Get(_ context.Context, key string) (string, error) { + m.mu.Lock() + defer m.mu.Unlock() + if val, ok := m.data[key]; ok { + return val, nil + } + return "", errors.New("not found") +} + +func (m *mockSystemConfigStore) Set(_ context.Context, key, value string) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.data == nil { + m.data = make(map[string]string) + } + m.data[key] = value + return nil +} + +func (m *mockSystemConfigStore) Delete(_ context.Context, key string) error { + m.mu.Lock() + defer m.mu.Unlock() + delete(m.data, key) + m.deleteKeys = append(m.deleteKeys, key) + return nil +} + +func (m *mockSystemConfigStore) List(context.Context) (map[string]string, error) { + m.mu.Lock() + defer m.mu.Unlock() + out := make(map[string]string, len(m.data)) + for k, v := range m.data { + out[k] = v + } + return out, nil +} // mockDomainEventBus implements eventbus.DomainEventBus for testing. type mockDomainEventBus struct { @@ -312,11 +396,11 @@ func TestEpisodicWorkerHandle_WithSummary(t *testing.T) { AgentID: uuid.New().String(), UserID: "test-user", Payload: &eventbus.SessionCompletedPayload{ - SessionKey: "session-123", + SessionKey: "session-123", CompactionCount: 1, - Summary: "Pre-computed summary", - MessageCount: 10, - TokensUsed: 1000, + Summary: "Pre-computed summary", + MessageCount: 10, + TokensUsed: 1000, }, } @@ -500,6 +584,46 @@ func TestSemanticWorkerHandle_WithValidExtraction(t *testing.T) { } } +func TestSemanticWorkerHandle_SuccessClearsBackgroundProviderAlert(t *testing.T) { + mockKG := &mockKGStore{} + mockExtractor := &mockExtractor{ + result: &knowledgegraph.ExtractionResult{ + Entities: []store.Entity{{Name: "Entity1", EntityType: "Person"}}, + }, + } + sc := &mockSystemConfigStore{ + data: map[string]string{ + bgalert.AlertKeyProviderError: `{"reason":"auth_permanent","worker":"kg_extraction"}`, + }, + } + worker := &semanticWorker{ + kgStore: mockKG, + extractor: mockExtractor, + eventBus: newMockDomainEventBus(), + alertDeps: bgalert.AlertDeps{SystemConfigs: sc}, + } + + err := worker.Handle(context.Background(), eventbus.DomainEvent{ + Type: eventbus.EventEpisodicCreated, + TenantID: uuid.New().String(), + AgentID: uuid.New().String(), + UserID: "test-user", + Payload: &eventbus.EpisodicCreatedPayload{ + EpisodicID: "ep-123", + Summary: "Alice works at TechCorp", + }, + }) + if err != nil { + t.Fatalf("Handle failed: %v", err) + } + if _, ok := sc.data[bgalert.AlertKeyProviderError]; ok { + t.Fatal("expected successful extraction to clear stale background provider alert") + } + if len(sc.deleteKeys) != 1 || sc.deleteKeys[0] != bgalert.AlertKeyProviderError { + t.Fatalf("deleteKeys = %#v, want [%q]", sc.deleteKeys, bgalert.AlertKeyProviderError) + } +} + func TestSemanticWorkerHandle_NilExtractor(t *testing.T) { worker := &semanticWorker{ extractor: nil, @@ -571,9 +695,9 @@ func TestDedupWorkerHandle_ValidPayload(t *testing.T) { ctx := context.Background() event := eventbus.DomainEvent{ - Type: eventbus.EventEntityUpserted, - AgentID: "agent-123", - UserID: "user-123", + Type: eventbus.EventEntityUpserted, + AgentID: "agent-123", + UserID: "user-123", Payload: &eventbus.EntityUpsertedPayload{ EntityIDs: []string{"e1", "e2", "e3"}, }, @@ -653,9 +777,9 @@ func TestDreamingWorkerHandle_BelowThreshold(t *testing.T) { ctx := context.Background() event := eventbus.DomainEvent{ - Type: eventbus.EventEpisodicCreated, - AgentID: "agent-123", - UserID: "user-123", + Type: eventbus.EventEpisodicCreated, + AgentID: "agent-123", + UserID: "user-123", Payload: &eventbus.EpisodicCreatedPayload{}, } diff --git a/internal/vault/enrich_worker.go b/internal/vault/enrich_worker.go index 2fd4cb1c..97e3fdc0 100644 --- a/internal/vault/enrich_worker.go +++ b/internal/vault/enrich_worker.go @@ -541,6 +541,7 @@ func (w *EnrichWorker) chatWithRetry(ctx context.Context, provider providers.Pro if resp.FinishReason == "length" { slog.Warn(logPrefix+": truncated", "finish_reason", "length", "model", req.Model, "content_len", len(resp.Content), "max_tokens", req.Options["max_tokens"]) } + bgalert.ClearProviderError(ctx, w.alertDeps.SystemConfigs) return strings.TrimSpace(resp.Content), nil } bgalert.ReportProviderError(ctx, w.alertDeps, "vault_enrich", lastErr) From dc4cfddd318c669ff511e6572c728f275f986546 Mon Sep 17 00:00:00 2001 From: daotri Date: Tue, 7 Jul 2026 12:23:11 +0700 Subject: [PATCH 08/15] fix(mcp): refresh per-user credentials without restart (#1375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Trí Đào Nguyễn Minh --- cmd/gateway_http_wiring.go | 3 ++ internal/agent/loop_pipeline_callbacks.go | 16 +++++++++ internal/http/mcp_user_credentials.go | 36 +++++++++++++++++-- internal/mcp/pool.go | 36 +++++++++++++++++-- internal/store/mcp_store.go | 6 ++-- .../pages/mcp/mcp-user-credentials-dialog.tsx | 7 ++-- ui/web/src/types/mcp.ts | 1 + 7 files changed, 95 insertions(+), 10 deletions(-) diff --git a/cmd/gateway_http_wiring.go b/cmd/gateway_http_wiring.go index 35eb9dfd..e55a955f 100644 --- a/cmd/gateway_http_wiring.go +++ b/cmd/gateway_http_wiring.go @@ -64,6 +64,9 @@ func (d *gatewayDeps) wireHTTPHandlersOnServer( d.server.SetMCPHandler(h.mcp) } if h.mcpUserCreds != nil { + if mcpPool != nil { + h.mcpUserCreds.SetPoolEvictor(mcpPool) + } d.server.SetMCPUserCredentialsHandler(h.mcpUserCreds) } if h.channelInstances != nil { diff --git a/internal/agent/loop_pipeline_callbacks.go b/internal/agent/loop_pipeline_callbacks.go index 1a0c991e..39b84398 100644 --- a/internal/agent/loop_pipeline_callbacks.go +++ b/internal/agent/loop_pipeline_callbacks.go @@ -229,6 +229,22 @@ func (l *Loop) makeBuildFilteredTools(req *RunRequest) func(state *pipeline.RunS allMsgs := state.Messages.All() toolDefs, _, returnedMsgs := l.buildFilteredTools(req, state.Context.HadBootstrap, state.Iteration, maxIter, allMsgs) + if state.Iteration != maxIter && len(userTools) > 0 { + existing := make(map[string]struct{}, len(toolDefs)+len(userTools)) + for _, td := range toolDefs { + if td.Function != nil { + existing[td.Function.Name] = struct{}{} + } + } + for _, t := range userTools { + name := t.Name() + if _, ok := existing[name]; ok { + continue + } + toolDefs = append(toolDefs, tools.ToProviderDef(t)) + existing[name] = struct{}{} + } + } // buildFilteredTools returns the full messages slice; only messages appended // beyond the original length are injections (e.g. final-iteration hint). // Appending the entire slice would duplicate system+history into pending. diff --git a/internal/http/mcp_user_credentials.go b/internal/http/mcp_user_credentials.go index 71c23edf..c67e3f95 100644 --- a/internal/http/mcp_user_credentials.go +++ b/internal/http/mcp_user_credentials.go @@ -1,6 +1,7 @@ package http import ( + "context" "encoding/json" "log/slog" "net/http" @@ -15,6 +16,12 @@ import ( type MCPUserCredentialsHandler struct { store store.MCPServerStore tenantStore store.TenantStore + poolEvictor MCPUserPoolEvictor +} + +// MCPUserPoolEvictor evicts pooled user connections after credential rotation. +type MCPUserPoolEvictor interface { + EvictUser(tenantID uuid.UUID, serverName, userID string) } // NewMCPUserCredentialsHandler creates a handler for MCP user credential endpoints. @@ -22,6 +29,9 @@ func NewMCPUserCredentialsHandler(s store.MCPServerStore, ts store.TenantStore) return &MCPUserCredentialsHandler{store: s, tenantStore: ts} } +// SetPoolEvictor wires the MCP connection pool used by runtime execution. +func (h *MCPUserCredentialsHandler) SetPoolEvictor(e MCPUserPoolEvictor) { h.poolEvictor = e } + // RegisterRoutes registers MCP user credential routes. func (h *MCPUserCredentialsHandler) RegisterRoutes(mux *http.ServeMux) { mux.HandleFunc("PUT /v1/mcp/servers/{id}/user-credentials", h.auth(h.handleSet)) @@ -104,6 +114,7 @@ func (h *MCPUserCredentialsHandler) handleSet(w http.ResponseWriter, r *http.Req return } + h.evictUserConnection(r.Context(), serverID, userID) writeJSON(w, http.StatusOK, map[string]string{"status": "updated"}) } @@ -127,12 +138,19 @@ func (h *MCPUserCredentialsHandler) handleGet(w http.ResponseWriter, r *http.Req } creds, err := h.store.GetUserCredentials(r.Context(), serverID, userID) - if err != nil { - writeJSON(w, http.StatusOK, map[string]any{"has_credentials": false}) + if err != nil || creds == nil { + writeJSON(w, http.StatusOK, map[string]any{ + "user_id": userID, + "has_credentials": false, + "has_api_key": false, + "has_headers": false, + "has_env": false, + }) return } writeJSON(w, http.StatusOK, map[string]any{ + "user_id": userID, "has_credentials": true, "has_api_key": creds.APIKey != "", "has_headers": len(creds.Headers) > 0, @@ -164,9 +182,23 @@ func (h *MCPUserCredentialsHandler) handleDelete(w http.ResponseWriter, r *http. return } + h.evictUserConnection(r.Context(), serverID, userID) writeJSON(w, http.StatusOK, map[string]string{"status": "deleted"}) } +func (h *MCPUserCredentialsHandler) evictUserConnection(ctx context.Context, serverID uuid.UUID, userID string) { + if h.poolEvictor == nil || userID == "" { + return + } + srv, err := h.store.GetServer(ctx, serverID) + if err != nil || srv == nil { + slog.Warn("mcp.user_credentials.evict_lookup_failed", "server_id", serverID, "user", userID, "error", err) + return + } + tid := store.TenantIDFromContext(ctx) + h.poolEvictor.EvictUser(tid, srv.Name, userID) +} + // httpStatusText returns a short error message for common HTTP status codes. func httpStatusText(code int) string { switch code { diff --git a/internal/mcp/pool.go b/internal/mcp/pool.go index a6414bc9..614a77bf 100644 --- a/internal/mcp/pool.go +++ b/internal/mcp/pool.go @@ -51,9 +51,9 @@ type poolEntry struct { // Per-user connections are keyed by tenantID/serverName/user:userID. type Pool struct { mu sync.Mutex - servers map[string]*poolEntry // shared connections: tenantID/serverName - userServers map[string]*poolEntry // user connections: tenantID/serverName/user:userID - userSlots map[string]chan struct{} // per-server semaphores: tenantID/serverName → capacity MaxUserConns + servers map[string]*poolEntry // shared connections: tenantID/serverName + userServers map[string]*poolEntry // user connections: tenantID/serverName/user:userID + userSlots map[string]chan struct{} // per-server semaphores: tenantID/serverName → capacity MaxUserConns cfg PoolConfig slot chan struct{} // semaphore for MaxSize stopCh chan struct{} @@ -434,6 +434,36 @@ func (p *Pool) Evict(tenantID uuid.UUID, serverName string) { slog.Info("mcp.pool.evicted_on_rotation", "key", key) } +// EvictUser closes a specific user-scoped pooled connection by tenant, server, +// and user. Called when per-user credentials are rotated so the next request +// reconnects with fresh headers/env. +func (p *Pool) EvictUser(tenantID uuid.UUID, serverName, userID string) { + key := UserPoolKey(tenantID, serverName, userID) + slotKey := userSlotKey(tenantID, serverName) + p.mu.Lock() + defer p.mu.Unlock() + + entry, ok := p.userServers[key] + if !ok { + return + } + entry.state.connected.Store(false) + if entry.state.cancel != nil { + entry.state.cancel() + } + if client := entry.state.clientPtr.Load(); client != nil { + _ = client.Close() + } + delete(p.userServers, key) + if sem, ok := p.userSlots[slotKey]; ok { + select { + case <-sem: + default: + } + } + slog.Info("mcp.pool.user.evicted_on_rotation", "key", key) +} + // evictLoop runs periodically to close idle connections over MaxIdle count. func (p *Pool) evictLoop() { ticker := time.NewTicker(60 * time.Second) diff --git a/internal/store/mcp_store.go b/internal/store/mcp_store.go index 34ceace7..fc7334db 100644 --- a/internal/store/mcp_store.go +++ b/internal/store/mcp_store.go @@ -78,9 +78,9 @@ type MCPAccessInfo struct { // MCPUserCredentials holds per-user credential overrides for an MCP server. type MCPUserCredentials struct { - APIKey string `json:"api_key,omitempty" db:"-"` // decrypted - Headers map[string]string `json:"headers,omitempty" db:"-"` // decrypted - Env map[string]string `json:"env,omitempty" db:"-"` // decrypted + APIKey string `json:"api_key,omitempty" db:"-"` // decrypted + Headers map[string]string `json:"headers,omitempty" db:"-"` // decrypted + Env map[string]string `json:"env,omitempty" db:"-"` // decrypted } // MCPServerStore manages MCP server configs and access grants. diff --git a/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx b/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx index c40a46ad..c61a2f80 100644 --- a/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx +++ b/ui/web/src/pages/mcp/mcp-user-credentials-dialog.tsx @@ -111,8 +111,10 @@ export function MCPUserCredentialsDialog({ if (Object.keys(data.env).length > 0) creds.env = data.env as Record; const targetUser = canManageUsers ? selectedUserId : undefined; await onSetCredentials(server.id, creds, targetUser); + const nextStatus = await onGetCredentials(server.id, targetUser); + setStatus(nextStatus); + reset({ apiKey: "", headers: {}, env: {} }); toast.success(i18next.t("mcp:userCredentials.saved")); - onOpenChange(false); } catch (err) { toast.error(i18next.t("mcp:userCredentials.saveFailed"), err instanceof Error ? err.message : ""); } finally { @@ -125,8 +127,9 @@ export function MCPUserCredentialsDialog({ try { const targetUser = canManageUsers ? selectedUserId : undefined; await onDeleteCredentials(server.id, targetUser); + const nextStatus = await onGetCredentials(server.id, targetUser); + setStatus(nextStatus); toast.success(i18next.t("mcp:userCredentials.deleted")); - onOpenChange(false); } catch (err) { toast.error(i18next.t("mcp:userCredentials.deleteFailed"), err instanceof Error ? err.message : ""); } finally { diff --git a/ui/web/src/types/mcp.ts b/ui/web/src/types/mcp.ts index 10f7a5ce..8e471bfd 100644 --- a/ui/web/src/types/mcp.ts +++ b/ui/web/src/types/mcp.ts @@ -62,6 +62,7 @@ export interface MCPAgentGrant { } export interface MCPUserCredentialStatus { + user_id?: string; has_credentials: boolean; has_api_key: boolean; has_headers: boolean; From 519df3dd17e06e146c45161ab910e0e69ca6ffe0 Mon Sep 17 00:00:00 2001 From: Du Hoang Dinh Date: Thu, 9 Jul 2026 22:54:37 +0700 Subject: [PATCH 09/15] feat(store): add Upsert() to BuiltinToolStore for additive tool seeding (#554) Approved by github-maintain automation. Clean implementation of additive Upsert() for fork-specific tool seeding. Solves #336. --- internal/store/builtin_tool_store.go | 3 ++ internal/store/pg/builtin_tools.go | 51 +++++++++++++++++++++ internal/store/sqlitestore/builtin-tools.go | 51 +++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/internal/store/builtin_tool_store.go b/internal/store/builtin_tool_store.go index fd08d19c..9e4ad7fa 100644 --- a/internal/store/builtin_tool_store.go +++ b/internal/store/builtin_tool_store.go @@ -29,6 +29,9 @@ type BuiltinToolStore interface { Get(ctx context.Context, name string) (*BuiltinToolDef, error) Update(ctx context.Context, name string, updates map[string]any) error Seed(ctx context.Context, tools []BuiltinToolDef) error + // Upsert inserts/updates tools without reconcile — safe for additive fork-specific tools. + // Does NOT delete rows not in the provided list. + Upsert(ctx context.Context, tools []BuiltinToolDef) error ListEnabled(ctx context.Context) ([]BuiltinToolDef, error) GetSettings(ctx context.Context, name string) (json.RawMessage, error) } diff --git a/internal/store/pg/builtin_tools.go b/internal/store/pg/builtin_tools.go index 8e6a7aef..553beb18 100644 --- a/internal/store/pg/builtin_tools.go +++ b/internal/store/pg/builtin_tools.go @@ -159,6 +159,57 @@ func (s *PGBuiltinToolStore) Seed(ctx context.Context, tools []store.BuiltinTool return tx.Commit() } +// Upsert inserts or updates builtin tool definitions without reconcile DELETE. +// Safe for additive fork-specific tools -- does NOT delete rows not in the list. +func (s *PGBuiltinToolStore) Upsert(ctx context.Context, tools []store.BuiltinToolDef) error { + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + stmt, err := tx.PrepareContext(ctx, + `INSERT INTO builtin_tools (name, display_name, description, category, enabled, settings, requires, metadata, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $9) + ON CONFLICT (name) DO UPDATE SET + display_name = EXCLUDED.display_name, + description = EXCLUDED.description, + category = EXCLUDED.category, + requires = EXCLUDED.requires, + metadata = EXCLUDED.metadata, + settings = CASE + WHEN builtin_tools.settings IS NULL OR builtin_tools.settings::text IN ('{}', 'null') + THEN EXCLUDED.settings + ELSE builtin_tools.settings + END, + updated_at = EXCLUDED.updated_at`) + if err != nil { + return fmt.Errorf("prepare upsert stmt: %w", err) + } + defer stmt.Close() + + now := time.Now() + for _, t := range tools { + settings := t.Settings + if settings == nil { + settings = json.RawMessage("{}") + } + metadata := t.Metadata + if metadata == nil { + metadata = json.RawMessage("{}") + } + _, err := stmt.ExecContext(ctx, + t.Name, t.DisplayName, t.Description, t.Category, + t.Enabled, []byte(settings), pqStringArray(t.Requires), []byte(metadata), now, + ) + if err != nil { + return fmt.Errorf("upsert tool %s: %w", t.Name, err) + } + } + + return tx.Commit() +} + func (s *PGBuiltinToolStore) scanTool(row *sql.Row) (*store.BuiltinToolDef, error) { var def store.BuiltinToolDef var settings []byte diff --git a/internal/store/sqlitestore/builtin-tools.go b/internal/store/sqlitestore/builtin-tools.go index 33e603e0..3cd541df 100644 --- a/internal/store/sqlitestore/builtin-tools.go +++ b/internal/store/sqlitestore/builtin-tools.go @@ -168,6 +168,57 @@ func (s *SQLiteBuiltinToolStore) Seed(ctx context.Context, tools []store.Builtin return tx.Commit() } +// Upsert inserts or updates builtin tool definitions without reconcile DELETE. +// Safe for additive fork-specific tools -- does NOT delete rows not in the list. +func (s *SQLiteBuiltinToolStore) Upsert(ctx context.Context, tools []store.BuiltinToolDef) error { + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + + stmt, err := tx.PrepareContext(ctx, + `INSERT INTO builtin_tools (name, display_name, description, category, enabled, settings, requires, metadata, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + ON CONFLICT (name) DO UPDATE SET + display_name = excluded.display_name, + description = excluded.description, + category = excluded.category, + requires = excluded.requires, + metadata = excluded.metadata, + settings = CASE + WHEN builtin_tools.settings IS NULL OR builtin_tools.settings IN ('{}', 'null') + THEN excluded.settings + ELSE builtin_tools.settings + END, + updated_at = excluded.updated_at`) + if err != nil { + return fmt.Errorf("prepare upsert stmt: %w", err) + } + defer stmt.Close() + + now := time.Now() + for _, t := range tools { + settings := t.Settings + if settings == nil { + settings = json.RawMessage("{}") + } + metadata := t.Metadata + if metadata == nil { + metadata = json.RawMessage("{}") + } + _, err := stmt.ExecContext(ctx, + t.Name, t.DisplayName, t.Description, t.Category, + t.Enabled, []byte(settings), jsonStringArray(t.Requires), []byte(metadata), now, now, + ) + if err != nil { + return fmt.Errorf("upsert tool %s: %w", t.Name, err) + } + } + + return tx.Commit() +} + func (s *SQLiteBuiltinToolStore) scanTool(row *sql.Row) (*store.BuiltinToolDef, error) { var def store.BuiltinToolDef var settings []byte From 3ed1d5f9d89f1bf0017727f0d13371550233ccc2 Mon Sep 17 00:00:00 2001 From: badgerbees <93577481+badgerbees@users.noreply.github.com> Date: Wed, 5 Aug 2026 23:02:41 +0700 Subject: [PATCH 10/15] fix(feishu): harden API client with defensive HTTP status checks (#407) - Check resp.StatusCode range (2xx) before JSON decoding - Provide clear actionable errors with HTTP status codes for 5xx failures - Prevent cryptic JSON parse errors from HTML error pages --- internal/channels/feishu/larkclient.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/channels/feishu/larkclient.go b/internal/channels/feishu/larkclient.go index 0e58bfb3..1628b39f 100644 --- a/internal/channels/feishu/larkclient.go +++ b/internal/channels/feishu/larkclient.go @@ -68,6 +68,10 @@ func (c *LarkClient) getToken(ctx context.Context) (string, error) { } defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return "", fmt.Errorf("lark token request failed with HTTP %d", resp.StatusCode) + } + var result struct { Code int `json:"code"` Msg string `json:"msg"` @@ -150,6 +154,10 @@ func (c *LarkClient) doJSONOnce(ctx context.Context, method, path string, body a } defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("lark api %s %s: failed with HTTP %d", method, path, resp.StatusCode) + } + var result apiResponse if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { return nil, fmt.Errorf("lark api decode: %w", err) @@ -176,6 +184,10 @@ func (c *LarkClient) doDownload(ctx context.Context, path string) ([]byte, strin } defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, "", fmt.Errorf("lark download %s: failed with HTTP %d", path, resp.StatusCode) + } + // Check for JSON error response ct := resp.Header.Get("Content-Type") if ct != "" { @@ -244,6 +256,10 @@ func (c *LarkClient) doMultipart(ctx context.Context, path string, fields map[st } defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("lark upload %s: failed with HTTP %d", path, resp.StatusCode) + } + var result apiResponse if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { return nil, fmt.Errorf("lark upload decode: %w", err) From 94d2799788e4194ca511008c817cc7e4b2ace3a0 Mon Sep 17 00:00:00 2001 From: mds Date: Thu, 6 Aug 2026 22:32:00 +0700 Subject: [PATCH 11/15] feat(http): support persistent sessions via X-GoClaw-User-Id header (#713) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When X-GoClaw-User-Id header is provided, build a stable session key using BuildSessionKey() — same canonical format as Telegram, Discord, Slack channels. This allows HTTP API clients to maintain conversation history across multiple requests. Without the header, behavior is unchanged (random session per request). Session key format: With userId: agent:{id}:http:direct:{userId} (persistent) Without: agent:{id}:http-{random} (stateless) Co-authored-by: Claude Code --- internal/http/chat_completions.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/http/chat_completions.go b/internal/http/chat_completions.go index 1891ade8..a226de4b 100644 --- a/internal/http/chat_completions.go +++ b/internal/http/chat_completions.go @@ -156,12 +156,14 @@ func (h *ChatCompletionsHandler) ServeHTTP(w http.ResponseWriter, r *http.Reques } runID := uuid.NewString() - // Include userID in session key for multi-tenant isolation - sessionSuffix := "http-" + runID[:8] + // Build stable session key per user — same user+agent always reuses the same session + var sessionKey string if userID != "" { - sessionSuffix = "http-" + userID + "-" + runID[:8] + sessionKey = sessions.BuildSessionKey(agentID, "http", sessions.PeerDirect, userID) + } else { + // No user ID — fallback to random session (stateless) + sessionKey = sessions.SessionKey(agentID, "http-"+runID[:8]) } - sessionKey := sessions.SessionKey(agentID, sessionSuffix) slog.Info("chat completions request", "agent", agentID, "stream", req.Stream, "user", userID) From cc510d924eb17c00cb7edd60ba997aac573c05ab Mon Sep 17 00:00:00 2001 From: Stanley Kao Date: Fri, 7 Aug 2026 02:31:02 +0800 Subject: [PATCH 12/15] fix(cmd): add --user flag to agent chat for user_id authentication (#714) The CLI agent chat command connects via WebSocket but never sends user_id in the connect params. The gateway requires user_id for chat.send, making the CLI unusable for any agent interaction. All other clients (browser, Telegram, LINE, etc.) send user_id during connect. The CLI was the only channel missing it. Adds --user / -u flag that passes user_id in the connect frame. Co-authored-by: Claude Opus 4.6 (1M context) --- cmd/agent_chat.go | 8 +++++--- cmd/agent_chat_client.go | 9 ++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/cmd/agent_chat.go b/cmd/agent_chat.go index d9b73f87..ff995a2a 100644 --- a/cmd/agent_chat.go +++ b/cmd/agent_chat.go @@ -15,6 +15,7 @@ import ( func agentChatCmd() *cobra.Command { var ( agentName string + userID string message string sessionKey string ) @@ -30,18 +31,19 @@ Examples: goclaw agent chat -m "What time is it?" # One-shot message goclaw agent chat -s my-session # Continue a session`, Run: func(cmd *cobra.Command, args []string) { - runAgentChat(agentName, message, sessionKey) + runAgentChat(agentName, message, sessionKey, userID) }, } cmd.Flags().StringVarP(&agentName, "name", "n", "default", "agent name") cmd.Flags().StringVarP(&message, "message", "m", "", "one-shot message (omit for interactive mode)") cmd.Flags().StringVarP(&sessionKey, "session", "s", "", "session key (default: auto-generated)") + cmd.Flags().StringVarP(&userID, "user", "u", "", "user ID for authentication") return cmd } -func runAgentChat(agentName, message, sessionKey string) { +func runAgentChat(agentName, message, sessionKey, userID string) { cfgPath := resolveConfigPath() cfg, err := config.Load(cfgPath) if err != nil { @@ -68,7 +70,7 @@ func runAgentChat(agentName, message, sessionKey string) { } fmt.Fprintf(os.Stderr, "Connected to gateway at %s\n", addr) - runClientMode(cfg, addr, agentName, message, sessionKey) + runClientMode(cfg, addr, agentName, message, sessionKey, userID) } // --- Gateway detection --- diff --git a/cmd/agent_chat_client.go b/cmd/agent_chat_client.go index 6a6d4c59..a95dd1cd 100644 --- a/cmd/agent_chat_client.go +++ b/cmd/agent_chat_client.go @@ -15,7 +15,7 @@ import ( "github.com/nextlevelbuilder/goclaw/pkg/protocol" ) -func runClientMode(cfg *config.Config, addr, agentName, message, sessionKey string) { +func runClientMode(cfg *config.Config, addr, agentName, message, sessionKey, userID string) { wsURL := fmt.Sprintf("ws://%s/ws", addr) conn, _, err := websocket.DefaultDialer.Dial(wsURL, nil) @@ -26,7 +26,7 @@ func runClientMode(cfg *config.Config, addr, agentName, message, sessionKey stri defer conn.Close() // Authenticate - if err := wsConnect(conn, cfg.Gateway.Token); err != nil { + if err := wsConnect(conn, cfg.Gateway.Token, userID); err != nil { fmt.Fprintf(os.Stderr, "Gateway auth failed: %v\n", err) os.Exit(1) } @@ -79,7 +79,7 @@ func runClientMode(cfg *config.Config, addr, agentName, message, sessionKey stri } // wsConnect sends the connect RPC and waits for auth response. -func wsConnect(conn *websocket.Conn, token string) error { +func wsConnect(conn *websocket.Conn, token, userID string) error { params := map[string]string{} userId := os.Getenv("GOCLAW_USER_ID") if userId == "" { userId = "system" } @@ -87,6 +87,9 @@ func wsConnect(conn *websocket.Conn, token string) error { if token != "" { params["token"] = token } + if userID != "" { + params["user_id"] = userID + } paramsJSON, _ := json.Marshal(params) reqFrame := protocol.RequestFrame{ From 713faf2ab02052fe2dac78ebd01d8f0acee6770f Mon Sep 17 00:00:00 2001 From: TJF Date: Sun, 9 Aug 2026 19:15:03 -0400 Subject: [PATCH 13/15] feat(tracing): sign exported spans as verifiable receipts (#1505) Implements #688. Adds a SpanExporter that signs exported spans as Ed25519 receipts, chains them, and anchors the sequence with a signed checkpoint, so a third party can verify what a run did without trusting the collector, the database, or the operator. Standard library crypto only. No new dependencies, and no existing file is modified: it attaches through SetExporter the same way OTLP does. Design notes worth reviewing: - Coverage counts spans this exporter received, never spans the runtime executed. Collector.EmitSpan drops spans when its buffer fills, which is correct for observability and means an exporter can only sign what reaches it. Every checkpoint carries a note saying so, so a receipt set reads as a floor on what happened rather than a complete record. - The checkpoint is not optional. A hash chain cannot detect truncation: removing the newest receipts leaves every remaining link valid. Verification reports Valid=false when no checkpoint is supplied, because without one it can only say nothing was altered, never that nothing is missing. - A checkpoint stored beside its receipts is deletable by whoever deletes the receipts, so Sink keeps the two destinations separate to make publishing the anchor elsewhere straightforward. - Content is committed by digest rather than copied, since the package redacts previews and a second store of that content would widen a leak. - Ordering comes from an exporter sequence rather than a clock. - Revocation is not retroactive; RevokedAt covers the compromise case. - Signature and chain failures are reported separately, and a failed sink write does not advance the chain head. 26 tests, most adversarial: tampering, forged signatures under a claimed kid, cross-domain signature replay, validity windows, revocation ordering, middle deletion, reordering, end truncation, empty bundles, sink failure, and concurrent export. go vet and go test -race clean. Co-authored-by: tommylauren --- internal/tracing/receiptexport/README.md | 124 ++++ internal/tracing/receiptexport/exporter.go | 235 ++++++++ .../tracing/receiptexport/exporter_test.go | 339 +++++++++++ internal/tracing/receiptexport/filesink.go | 135 +++++ internal/tracing/receiptexport/receipt.go | 570 ++++++++++++++++++ .../tracing/receiptexport/receipt_test.go | 371 ++++++++++++ internal/tracing/receiptexport/verify.go | 164 +++++ 7 files changed, 1938 insertions(+) create mode 100644 internal/tracing/receiptexport/README.md create mode 100644 internal/tracing/receiptexport/exporter.go create mode 100644 internal/tracing/receiptexport/exporter_test.go create mode 100644 internal/tracing/receiptexport/filesink.go create mode 100644 internal/tracing/receiptexport/receipt.go create mode 100644 internal/tracing/receiptexport/receipt_test.go create mode 100644 internal/tracing/receiptexport/verify.go diff --git a/internal/tracing/receiptexport/README.md b/internal/tracing/receiptexport/README.md new file mode 100644 index 00000000..54a270b2 --- /dev/null +++ b/internal/tracing/receiptexport/README.md @@ -0,0 +1,124 @@ +# receiptexport + +Signs exported spans as Ed25519 receipts so a third party can verify what a run +did without trusting the collector, the database, or the operator. + +Implements `tracing.SpanExporter`, so it attaches the same way OTLP does and +changes nothing in the collector. + +## Why this is not just a trace + +A trace says what the runtime emitted. A receipt says what a named key attested +to, in what order, and whether that record has been altered since. The +difference matters the moment a run crosses an organisational boundary, or +someone has to explain a quiet failure after the fact. + +## Usage + +```go +key, err := receiptexport.GenerateSigningKey() +if err != nil { + return err +} + +sink, err := receiptexport.NewFileSink( + "/var/lib/goclaw/receipts.jsonl", + "/var/lib/goclaw/checkpoint.json", +) +if err != nil { + return err +} + +exporter, err := receiptexport.New(receiptexport.Config{ + Key: key, + Sink: sink, + SpanTypes: []string{"tool_call"}, // omit to sign everything + CheckpointEvery: 100, +}) +if err != nil { + return err +} + +collector.SetExporter(exporter) +``` + +Publish only the public half for verifiers: + +```go +keyring := receiptexport.NewKeyring(key.PublicEntry()) +``` + +Verify with nothing but the evidence and that keyring: + +```go +receipts, checkpoint, err := receiptexport.LoadBundle(receiptPath, checkpointPath) +report, err := receiptexport.VerifyReceipts(receipts, checkpoint, keyring) +``` + +## The checkpoint is not optional + +A hash chain detects modification, reordering, and deletion from the middle. It +cannot detect truncation: remove the newest receipts and every remaining link +still verifies clean. + +The checkpoint closes that by committing to a receipt count and a terminal +hash. `VerifyReceipts` reports `Valid: false` when no checkpoint is supplied, +because without one it can only say "nothing I was given has been altered", +never "nothing is missing". + +**A checkpoint stored beside the receipts it anchors is deletable by whoever +deletes the receipts.** To be worth anything it has to reach somewhere the +operator cannot quietly rewrite: a build log, an object store with retention, or +the commit the run produced. `Sink` keeps receipt and checkpoint destinations +separate so that publishing step is easy to add. + +## Coverage, and what it does not claim + +`Collector.EmitSpan` drops spans when its buffer is full. That is correct for +observability, since tracing must never block a run, and it means an exporter +can only sign what it receives. + +So `Coverage` counts spans **this exporter saw**, never spans the runtime +executed, and it carries a note saying so into every checkpoint. A signed +receipt set is a floor on what happened, not a complete record. + +Quantifying the gap would need a drop counter in the collector, which this +package deliberately does not add. Happy to follow up with one if it is wanted. + +## What a verified receipt establishes + +> This span, with exactly these fields, was exported by the holder of this key +> at this time, and the verified sequence has not been altered since. + +## What it does not establish + +Returned in every verification report rather than left to a reader to infer: + +- Not that these are all the spans the runtime produced. +- Not that a tool call was correct, authorised, or safe. +- Not that receipts signed by a compromised key were truthful. +- Without a checkpoint, not that receipts were not removed from the end. + +## Design notes + +**Content is committed by digest, not copied.** The tracing package redacts +previews for a reason. Duplicating that content into a second, separately +stored artifact would widen the blast radius of a leak while adding nothing a +digest does not already prove. + +**Ordering comes from an exporter sequence, not a clock.** Ordering evidence by +a timestamp invites an older stamp recorded later to make honest history look +altered. + +**Revocation is not retroactive.** A receipt signed while the key was trusted +stays valid, because the alternative silently voids honest history every time +an operator rotates. `RevokedAt` exists for the compromise case. + +**Signature and chain failures are reported separately.** They are different +failures calling for different responses, and a deleted receipt leaves every +remaining signature genuine. + +**A failed write does not advance the chain head**, so a transient sink outage +cannot produce a permanent break that looks identical to tampering. + +**Standard library only**, plus `google/uuid` which the repository already uses. diff --git a/internal/tracing/receiptexport/exporter.go b/internal/tracing/receiptexport/exporter.go new file mode 100644 index 00000000..b03b22fe --- /dev/null +++ b/internal/tracing/receiptexport/exporter.go @@ -0,0 +1,235 @@ +package receiptexport + +import ( + "context" + "fmt" + "log/slog" + "sync" + "time" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// Sink receives signed receipts and checkpoints. Implementations decide where +// evidence lands: a file, an object store, or a commit trailer. +// +// The sink is separated from the exporter because where the checkpoint goes is +// what determines whether any of this is worth having. A checkpoint written +// beside the database is deletable by the same person who deletes the +// receipts, so the anchor only means something once it reaches somewhere the +// operator cannot quietly rewrite. +type Sink interface { + WriteReceipt(ctx context.Context, receipt SpanReceipt) error + WriteCheckpoint(ctx context.Context, checkpoint Checkpoint) error +} + +// Config configures the receipt exporter. +type Config struct { + // Key signs receipts and checkpoints. Required. + Key *SigningKey + // Sink receives the output. Required. + Sink Sink + // CheckpointEvery emits an interim checkpoint after this many receipts. + // Zero means only emit one at Shutdown, which leaves everything after the + // last checkpoint truncatable if the process is killed. + CheckpointEvery uint64 + // SpanTypes filters which spans are signed. Empty signs everything. + // Signing only "tool_call" is the common case and keeps the evidence set + // small enough to review by hand. + SpanTypes []string + // Logger defaults to slog.Default(). + Logger *slog.Logger +} + +// Exporter signs exported spans and implements tracing.SpanExporter. +// +// It never blocks the collector and never panics into it. The SpanExporter +// contract has ExportSpans return nothing, so an exporter cannot report a +// failure upward; the only honest options are to log, count, and keep the +// counts in the checkpoint so a verifier sees that something went wrong. +type Exporter struct { + key *SigningKey + sink Sink + checkpointEvery uint64 + spanTypes map[string]struct{} + log *slog.Logger + + mu sync.Mutex + sequence uint64 + previousHash string + coverage Coverage + sinceLast uint64 + closed bool +} + +// New builds a receipt exporter. +func New(cfg Config) (*Exporter, error) { + if cfg.Key == nil { + return nil, fmt.Errorf("signing key is required") + } + if cfg.Sink == nil { + return nil, fmt.Errorf("sink is required") + } + logger := cfg.Logger + if logger == nil { + logger = slog.Default() + } + var types map[string]struct{} + if len(cfg.SpanTypes) > 0 { + types = make(map[string]struct{}, len(cfg.SpanTypes)) + for _, t := range cfg.SpanTypes { + types[t] = struct{}{} + } + } + return &Exporter{ + key: cfg.Key, + sink: cfg.Sink, + checkpointEvery: cfg.CheckpointEvery, + spanTypes: types, + log: logger, + coverage: Coverage{ + Note: "Counts spans this exporter received. Spans dropped by the collector under buffer pressure never reach an exporter and are not counted here.", + }, + }, nil +} + +// ExportSpans signs and emits receipts. It implements tracing.SpanExporter. +// +// Returns nothing by interface contract, so every failure is logged and +// counted rather than propagated. A signing failure must never take down a +// run that was otherwise fine. +func (e *Exporter) ExportSpans(ctx context.Context, spans []store.SpanData) { + if e == nil || len(spans) == 0 { + return + } + + e.mu.Lock() + defer e.mu.Unlock() + + if e.closed { + e.log.Warn("receiptexport: spans arrived after shutdown, not signed", "count", len(spans)) + return + } + + for _, span := range spans { + if !e.wants(span) { + continue + } + e.coverage.SpansReceived++ + + receipt, hash, err := e.signLocked(span) + if err != nil { + e.coverage.SpansFailed++ + e.log.Error("receiptexport: could not sign span", + "span_id", span.ID, "span_type", span.SpanType, "error", err) + continue + } + if err := e.sink.WriteReceipt(ctx, receipt); err != nil { + e.coverage.SpansFailed++ + e.log.Error("receiptexport: could not write receipt", + "span_id", span.ID, "error", err) + continue + } + + // Advance only once the receipt has actually landed. Advancing before + // the write would leave the next receipt chaining to a hash nobody + // holds, turning a transient sink error into a permanent break that + // looks identical to tampering. + e.previousHash = hash + e.sequence++ + e.coverage.SpansSigned++ + e.sinceLast++ + } + + if e.checkpointEvery > 0 && e.sinceLast >= e.checkpointEvery { + if err := e.checkpointLocked(ctx); err != nil { + e.log.Error("receiptexport: could not write interim checkpoint", "error", err) + } + } +} + +// signLocked builds and signs a receipt against the current chain head and +// returns it with its hash. Caller must hold the mutex. +// +// Deliberately free of side effects: it does not advance the chain head or the +// sequence. The caller does that only after the receipt has been durably +// written, so a failed write leaves the next receipt chaining to the last one +// that actually landed. +func (e *Exporter) signLocked(span store.SpanData) (SpanReceipt, string, error) { + payload, err := PayloadFromSpan(span, e.sequence, e.previousHash, time.Now().UTC()) + if err != nil { + return SpanReceipt{}, "", err + } + receipt, err := SignReceipt(payload, e.key) + if err != nil { + return SpanReceipt{}, "", err + } + hash, err := ReceiptHash(receipt) + if err != nil { + return SpanReceipt{}, "", err + } + return receipt, hash, nil +} + +func (e *Exporter) wants(span store.SpanData) bool { + if e.spanTypes == nil { + return true + } + _, ok := e.spanTypes[span.SpanType] + return ok +} + +// Checkpoint emits a completeness anchor immediately. +// +// Publish the result somewhere the operator cannot rewrite. A checkpoint +// sitting beside the receipts it anchors proves very little. +func (e *Exporter) Checkpoint(ctx context.Context) error { + e.mu.Lock() + defer e.mu.Unlock() + return e.checkpointLocked(ctx) +} + +func (e *Exporter) checkpointLocked(ctx context.Context) error { + payload := CheckpointPayload{ + Type: CheckpointType, + Version: 1, + Sequence: e.sequence, + ReceiptCount: e.coverage.SpansSigned, + TerminalReceiptHash: e.previousHash, + Coverage: e.coverage, + SignedAt: time.Now().UTC().Format(time.RFC3339Nano), + } + checkpoint, err := SignCheckpoint(payload, e.key) + if err != nil { + return fmt.Errorf("sign checkpoint: %w", err) + } + if err := e.sink.WriteCheckpoint(ctx, checkpoint); err != nil { + return fmt.Errorf("write checkpoint: %w", err) + } + e.sinceLast = 0 + return nil +} + +// Shutdown emits a final checkpoint. It implements tracing.SpanExporter. +// +// This is the anchor that makes truncation detectable for the whole run, so a +// failure here is returned rather than swallowed. +func (e *Exporter) Shutdown(ctx context.Context) error { + e.mu.Lock() + defer e.mu.Unlock() + if e.closed { + return nil + } + e.closed = true + if err := e.checkpointLocked(ctx); err != nil { + return fmt.Errorf("final checkpoint: %w", err) + } + return nil +} + +// Coverage reports what this exporter has seen so far. +func (e *Exporter) Coverage() Coverage { + e.mu.Lock() + defer e.mu.Unlock() + return e.coverage +} diff --git a/internal/tracing/receiptexport/exporter_test.go b/internal/tracing/receiptexport/exporter_test.go new file mode 100644 index 00000000..4bcecd93 --- /dev/null +++ b/internal/tracing/receiptexport/exporter_test.go @@ -0,0 +1,339 @@ +package receiptexport + +import ( + "context" + "fmt" + "io" + "log/slog" + "sync" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// memSink captures output and can be told to fail, so tests can prove the +// exporter survives a sink that is down without taking the run with it. +type memSink struct { + mu sync.Mutex + receipts []SpanReceipt + checkpoints []Checkpoint + failWrites bool +} + +func (s *memSink) WriteReceipt(_ context.Context, r SpanReceipt) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.failWrites { + return fmt.Errorf("sink unavailable") + } + s.receipts = append(s.receipts, r) + return nil +} + +func (s *memSink) WriteCheckpoint(_ context.Context, c Checkpoint) error { + s.mu.Lock() + defer s.mu.Unlock() + s.checkpoints = append(s.checkpoints, c) + return nil +} + +func (s *memSink) snapshot() ([]SpanReceipt, []Checkpoint) { + s.mu.Lock() + defer s.mu.Unlock() + return append([]SpanReceipt{}, s.receipts...), append([]Checkpoint{}, s.checkpoints...) +} + +func quietLogger() *slog.Logger { + return slog.New(slog.NewTextHandler(io.Discard, nil)) +} + +func newTestExporter(t *testing.T, cfg Config) (*Exporter, *memSink, *SigningKey) { + t.Helper() + key, err := GenerateSigningKey() + if err != nil { + t.Fatal(err) + } + sink := &memSink{} + cfg.Key = key + cfg.Sink = sink + if cfg.Logger == nil { + cfg.Logger = quietLogger() + } + exp, err := New(cfg) + if err != nil { + t.Fatal(err) + } + return exp, sink, key +} + +func TestExporterSignsAndChainsSpans(t *testing.T) { + exp, sink, key := newTestExporter(t, Config{}) + ctx := context.Background() + + exp.ExportSpans(ctx, []store.SpanData{testSpan("a"), testSpan("b"), testSpan("c")}) + if err := exp.Shutdown(ctx); err != nil { + t.Fatal(err) + } + + receipts, checkpoints := sink.snapshot() + if len(receipts) != 3 { + t.Fatalf("expected 3 receipts, got %d", len(receipts)) + } + if len(checkpoints) != 1 { + t.Fatalf("expected a final checkpoint, got %d", len(checkpoints)) + } + + report, err := VerifyReceipts(receipts, &checkpoints[0], testKeyring(t, key)) + if err != nil { + t.Fatal(err) + } + if !report.Valid { + t.Fatalf("exported evidence should verify: %+v", report) + } +} + +func TestExporterFiltersBySpanType(t *testing.T) { + exp, sink, _ := newTestExporter(t, Config{SpanTypes: []string{"tool_call"}}) + ctx := context.Background() + + llm := testSpan("llm") + llm.SpanType = "llm_call" + + exp.ExportSpans(ctx, []store.SpanData{testSpan("tool"), llm}) + _ = exp.Shutdown(ctx) + + receipts, _ := sink.snapshot() + if len(receipts) != 1 { + t.Fatalf("expected only the tool_call span, got %d receipts", len(receipts)) + } + if receipts[0].Payload.SpanType != "tool_call" { + t.Fatalf("wrong span signed: %s", receipts[0].Payload.SpanType) + } +} + +// ExportSpans returns nothing by interface contract, so a broken sink must be +// counted and logged rather than propagated. A signing problem must never take +// down a run that was otherwise fine. +func TestSinkFailureIsCountedNotPropagated(t *testing.T) { + exp, sink, key := newTestExporter(t, Config{}) + ctx := context.Background() + + sink.failWrites = true + exp.ExportSpans(ctx, []store.SpanData{testSpan("a"), testSpan("b")}) + sink.failWrites = false + exp.ExportSpans(ctx, []store.SpanData{testSpan("c")}) + + if err := exp.Shutdown(ctx); err != nil { + t.Fatal(err) + } + + coverage := exp.Coverage() + if coverage.SpansReceived != 3 { + t.Fatalf("expected 3 received, got %d", coverage.SpansReceived) + } + if coverage.SpansFailed != 2 { + t.Fatalf("expected 2 failures, got %d", coverage.SpansFailed) + } + if coverage.SpansSigned != 1 { + t.Fatalf("expected 1 signed, got %d", coverage.SpansSigned) + } + + // The surviving receipt must still verify, and the checkpoint must carry + // the failure count so a reader sees the gap rather than a clean run. + receipts, checkpoints := sink.snapshot() + report, _ := VerifyReceipts(receipts, &checkpoints[0], testKeyring(t, key)) + if !report.Valid { + t.Fatalf("the receipts that did land should verify: %+v", report) + } + if report.Coverage == nil || report.Coverage.SpansFailed != 2 { + t.Fatal("the checkpoint must carry the failure count") + } +} + +// A failed write must not advance the chain head, or a transient sink error +// would leave a permanent unexplained break. +func TestFailedWriteDoesNotBreakTheChain(t *testing.T) { + exp, sink, key := newTestExporter(t, Config{}) + ctx := context.Background() + + exp.ExportSpans(ctx, []store.SpanData{testSpan("a")}) + sink.failWrites = true + exp.ExportSpans(ctx, []store.SpanData{testSpan("lost")}) + sink.failWrites = false + exp.ExportSpans(ctx, []store.SpanData{testSpan("c")}) + _ = exp.Shutdown(ctx) + + receipts, checkpoints := sink.snapshot() + report, _ := VerifyReceipts(receipts, &checkpoints[0], testKeyring(t, key)) + if !report.ChainIntact { + t.Fatalf("chain should remain intact across a failed write: %+v", report.Results) + } +} + +func TestCoverageNeverClaimsRuntimeCompleteness(t *testing.T) { + exp, _, _ := newTestExporter(t, Config{}) + coverage := exp.Coverage() + if coverage.Note == "" { + t.Fatal("coverage must carry a note distinguishing exporter reach from runtime activity") + } +} + +func TestInterimCheckpoints(t *testing.T) { + exp, sink, _ := newTestExporter(t, Config{CheckpointEvery: 2}) + ctx := context.Background() + + exp.ExportSpans(ctx, []store.SpanData{testSpan("a"), testSpan("b")}) + _, checkpoints := sink.snapshot() + if len(checkpoints) != 1 { + t.Fatalf("expected an interim checkpoint after 2 receipts, got %d", len(checkpoints)) + } + + exp.ExportSpans(ctx, []store.SpanData{testSpan("c"), testSpan("d")}) + _ = exp.Shutdown(ctx) + + _, checkpoints = sink.snapshot() + if len(checkpoints) != 3 { + t.Fatalf("expected two interim checkpoints plus a final one, got %d", len(checkpoints)) + } +} + +func TestSpansAfterShutdownAreNotSigned(t *testing.T) { + exp, sink, _ := newTestExporter(t, Config{}) + ctx := context.Background() + + if err := exp.Shutdown(ctx); err != nil { + t.Fatal(err) + } + exp.ExportSpans(ctx, []store.SpanData{testSpan("late")}) + + receipts, _ := sink.snapshot() + if len(receipts) != 0 { + t.Fatal("a span arriving after the final checkpoint must not be silently appended") + } +} + +func TestShutdownIsIdempotent(t *testing.T) { + exp, sink, _ := newTestExporter(t, Config{}) + ctx := context.Background() + + if err := exp.Shutdown(ctx); err != nil { + t.Fatal(err) + } + if err := exp.Shutdown(ctx); err != nil { + t.Fatalf("second shutdown should be a no-op, got %v", err) + } + _, checkpoints := sink.snapshot() + if len(checkpoints) != 1 { + t.Fatalf("expected exactly one final checkpoint, got %d", len(checkpoints)) + } +} + +func TestNewRequiresKeyAndSink(t *testing.T) { + if _, err := New(Config{}); err == nil { + t.Fatal("a key is required") + } + key, _ := GenerateSigningKey() + if _, err := New(Config{Key: key}); err == nil { + t.Fatal("a sink is required") + } +} + +func TestConcurrentExportKeepsTheChainConsistent(t *testing.T) { + exp, sink, key := newTestExporter(t, Config{}) + ctx := context.Background() + + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + exp.ExportSpans(ctx, []store.SpanData{testSpan(fmt.Sprintf("s%d", n))}) + }(i) + } + wg.Wait() + if err := exp.Shutdown(ctx); err != nil { + t.Fatal(err) + } + + receipts, checkpoints := sink.snapshot() + if len(receipts) != 8 { + t.Fatalf("expected 8 receipts, got %d", len(receipts)) + } + report, _ := VerifyReceipts(receipts, &checkpoints[0], testKeyring(t, key)) + if !report.Valid { + t.Fatalf("concurrent export must still produce a verifiable chain: %+v", report) + } +} + +func TestFileSinkRoundTrip(t *testing.T) { + dir := t.TempDir() + receiptPath := dir + "/receipts.jsonl" + checkpointPath := dir + "/checkpoint.json" + + sink, err := NewFileSink(receiptPath, checkpointPath) + if err != nil { + t.Fatal(err) + } + key, err := GenerateSigningKey() + if err != nil { + t.Fatal(err) + } + exp, err := New(Config{Key: key, Sink: sink, Logger: quietLogger()}) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + exp.ExportSpans(ctx, []store.SpanData{testSpan("a"), testSpan("b")}) + if err := exp.Shutdown(ctx); err != nil { + t.Fatal(err) + } + + // Verify from disk with a keyring holding only public material, which is + // what an auditor would actually have. + receipts, checkpoint, err := LoadBundle(receiptPath, checkpointPath) + if err != nil { + t.Fatal(err) + } + independent := NewKeyring(KeyringEntry{Kid: key.Kid, Alg: key.Alg, PublicKey: key.PublicKey}) + + report, err := VerifyReceipts(receipts, checkpoint, independent) + if err != nil { + t.Fatal(err) + } + if !report.Valid { + t.Fatalf("evidence read back from disk should verify: %+v", report) + } + if report.ReceiptsChecked != 2 { + t.Fatalf("expected 2 receipts, got %d", report.ReceiptsChecked) + } +} + +func TestFileSinkDetectsTruncatedEvidenceFile(t *testing.T) { + dir := t.TempDir() + receiptPath := dir + "/receipts.jsonl" + checkpointPath := dir + "/checkpoint.json" + + sink, _ := NewFileSink(receiptPath, checkpointPath) + key, _ := GenerateSigningKey() + exp, _ := New(Config{Key: key, Sink: sink, Logger: quietLogger()}) + + ctx := context.Background() + exp.ExportSpans(ctx, []store.SpanData{testSpan("a"), testSpan("b"), testSpan("c")}) + _ = exp.Shutdown(ctx) + + // Someone removes the last line of the evidence file. Every remaining + // signature and internal link stays valid; only the anchor catches it. + receipts, checkpoint, _ := LoadBundle(receiptPath, checkpointPath) + truncated := receipts[:len(receipts)-1] + + independent := NewKeyring(KeyringEntry{Kid: key.Kid, Alg: key.Alg, PublicKey: key.PublicKey}) + report, _ := VerifyReceipts(truncated, checkpoint, independent) + + if report.SignaturesInvalid != 0 || !report.ChainIntact { + t.Fatal("truncation leaves signatures and links intact, which is why the anchor is needed") + } + if report.Valid || report.CheckpointValid { + t.Fatal("the checkpoint must reject a truncated evidence file") + } +} diff --git a/internal/tracing/receiptexport/filesink.go b/internal/tracing/receiptexport/filesink.go new file mode 100644 index 00000000..8ef217e6 --- /dev/null +++ b/internal/tracing/receiptexport/filesink.go @@ -0,0 +1,135 @@ +package receiptexport + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "sync" +) + +// FileSink writes receipts as JSON Lines and keeps the latest checkpoint in its +// own file, so evidence can be collected with no infrastructure at all. +// +// It is deliberately the simplest thing that works, and it is not sufficient on +// its own. A checkpoint written next to the receipts it anchors is deletable by +// whoever deletes the receipts. To be worth something the checkpoint has to +// reach somewhere the operator cannot quietly rewrite: a build log, an object +// store with retention, or the commit that the run produced. CheckpointPath is +// separate from ReceiptPath so that publishing step is easy to bolt on. +type FileSink struct { + mu sync.Mutex + receiptPath string + checkpointPath string +} + +// NewFileSink creates the parent directories and returns a sink. +func NewFileSink(receiptPath, checkpointPath string) (*FileSink, error) { + if receiptPath == "" || checkpointPath == "" { + return nil, fmt.Errorf("receipt and checkpoint paths are required") + } + for _, path := range []string{receiptPath, checkpointPath} { + if err := os.MkdirAll(filepath.Dir(path), 0o750); err != nil { + return nil, fmt.Errorf("create directory for %s: %w", path, err) + } + } + return &FileSink{receiptPath: receiptPath, checkpointPath: checkpointPath}, nil +} + +// WriteReceipt appends one receipt as a JSON line. +func (s *FileSink) WriteReceipt(_ context.Context, receipt SpanReceipt) error { + line, err := json.Marshal(receipt) + if err != nil { + return fmt.Errorf("marshal receipt: %w", err) + } + + s.mu.Lock() + defer s.mu.Unlock() + + file, err := os.OpenFile(s.receiptPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) + if err != nil { + return fmt.Errorf("open receipt file: %w", err) + } + defer file.Close() + + if _, err := file.Write(append(line, '\n')); err != nil { + return fmt.Errorf("append receipt: %w", err) + } + // Durability matters more than throughput here: a receipt lost to the page + // cache on a crash is indistinguishable from one that was never signed. + return file.Sync() +} + +// WriteCheckpoint replaces the checkpoint file. Only the latest matters, since +// each one commits to the full count and terminal hash. +func (s *FileSink) WriteCheckpoint(_ context.Context, checkpoint Checkpoint) error { + encoded, err := json.MarshalIndent(checkpoint, "", " ") + if err != nil { + return fmt.Errorf("marshal checkpoint: %w", err) + } + + s.mu.Lock() + defer s.mu.Unlock() + + // Write and rename, so a crash mid-write cannot leave a truncated anchor + // that would fail verification of an otherwise honest run. + temp := s.checkpointPath + ".tmp" + if err := os.WriteFile(temp, append(encoded, '\n'), 0o600); err != nil { + return fmt.Errorf("write checkpoint: %w", err) + } + if err := os.Rename(temp, s.checkpointPath); err != nil { + return fmt.Errorf("replace checkpoint: %w", err) + } + return nil +} + +// LoadBundle reads receipts and the checkpoint back for verification. +func LoadBundle(receiptPath, checkpointPath string) ([]SpanReceipt, *Checkpoint, error) { + raw, err := os.ReadFile(receiptPath) // #nosec G304 -- operator-supplied evidence path + if err != nil { + return nil, nil, fmt.Errorf("read receipts: %w", err) + } + + var receipts []SpanReceipt + for _, line := range splitLines(raw) { + var receipt SpanReceipt + if err := json.Unmarshal(line, &receipt); err != nil { + return nil, nil, fmt.Errorf("parse receipt: %w", err) + } + receipts = append(receipts, receipt) + } + + var checkpoint *Checkpoint + if checkpointPath != "" { + rawCheckpoint, err := os.ReadFile(checkpointPath) // #nosec G304 -- operator-supplied evidence path + if err != nil { + return nil, nil, fmt.Errorf("read checkpoint: %w", err) + } + var parsed Checkpoint + if err := json.Unmarshal(rawCheckpoint, &parsed); err != nil { + return nil, nil, fmt.Errorf("parse checkpoint: %w", err) + } + checkpoint = &parsed + } + + return receipts, checkpoint, nil +} + +func splitLines(raw []byte) [][]byte { + var lines [][]byte + start := 0 + for i, b := range raw { + if b != '\n' { + continue + } + if i > start { + lines = append(lines, raw[start:i]) + } + start = i + 1 + } + if start < len(raw) { + lines = append(lines, raw[start:]) + } + return lines +} diff --git a/internal/tracing/receiptexport/receipt.go b/internal/tracing/receiptexport/receipt.go new file mode 100644 index 00000000..fc8b9147 --- /dev/null +++ b/internal/tracing/receiptexport/receipt.go @@ -0,0 +1,570 @@ +// Package receiptexport signs exported spans as Ed25519 receipts so a third +// party can verify what a GoClaw run did without trusting the collector, the +// database, or the operator who runs them. +// +// It answers a different question from OTLP. A trace says what the runtime +// emitted. A receipt says what a named key attested to, in what order, and +// whether that record has been altered since. That difference matters the +// moment a run crosses an organisational boundary or someone has to explain a +// quiet failure after the fact. +// +// # What a verified receipt establishes +// +// This span, with exactly these fields, was exported by the holder of this key +// at this time, and the verified sequence has not been altered since. +// +// # What it does not establish +// +// That the span is a complete record of what happened. The collector drops +// spans when its buffers fill (see Collector.EmitSpan), so an exporter can only +// sign what it receives. Coverage is therefore reported as spans this exporter +// saw, never as spans the runtime executed. See the Coverage type. +// +// It also does not establish that a tool call was correct or authorised, that +// the operator is honest, or that a key compromised at export time produced +// truthful receipts. Signing binds authorship and order, not judgement. +// +// # Dependencies +// +// Standard library only, plus google/uuid which the repository already uses. +package receiptexport + +import ( + "bytes" + "crypto/ed25519" + "crypto/sha256" + "encoding/base64" + "encoding/hex" + "encoding/json" + "fmt" + "sort" + "strings" + "time" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +const ( + // ReceiptType identifies the signed payload schema. + ReceiptType = "goclaw.span_receipt.v1" + // KeyringType identifies the trust document schema. + KeyringType = "goclaw.receipt_keyring.v1" + // CheckpointType identifies the completeness anchor schema. + CheckpointType = "goclaw.receipt_checkpoint.v1" + // SigningAlg is the only algorithm this package accepts. + SigningAlg = "Ed25519" + + // signingDomain is mixed into every signature so a signature over one kind + // of object can never verify as another. + signingDomain = "goclaw/span-receipt/v1" + // checkpointDomain separates checkpoint signatures from receipt signatures. + checkpointDomain = "goclaw/receipt-checkpoint/v1" +) + +// SpanReceiptPayload is the signed record of one exported span. +// +// Previews and metadata are committed to by digest rather than copied. The +// tracing package redacts previews for a reason, and duplicating that content +// into a second, separately stored artifact would widen the blast radius of a +// leak while adding nothing a digest does not already prove. +type SpanReceiptPayload struct { + Type string `json:"type"` + Version int `json:"version"` + + // Sequence is the exporter's own monotonic counter, not a timestamp. + // Ordering evidence by a clock invites an older stamp recorded later to + // make honest history look altered. + Sequence uint64 `json:"sequence"` + + SpanID string `json:"span_id"` + TraceID string `json:"trace_id"` + ParentSpanID string `json:"parent_span_id,omitempty"` + TenantID string `json:"tenant_id"` + TeamID string `json:"team_id,omitempty"` + AgentID string `json:"agent_id,omitempty"` + + SpanType string `json:"span_type"` + Name string `json:"name,omitempty"` + Status string `json:"status"` + Error string `json:"error,omitempty"` + + // Tool identity is the reason this feature exists, so it is signed + // explicitly rather than left inside an opaque content digest. + ToolName string `json:"tool_name,omitempty"` + ToolCallID string `json:"tool_call_id,omitempty"` + + Model string `json:"model,omitempty"` + Provider string `json:"provider,omitempty"` + FinishReason string `json:"finish_reason,omitempty"` + InputTokens int `json:"input_tokens,omitempty"` + OutputTokens int `json:"output_tokens,omitempty"` + + StartTime string `json:"start_time"` + EndTime string `json:"end_time,omitempty"` + DurationMS int `json:"duration_ms,omitempty"` + + // ContentDigest commits to the previews, metadata and model params without + // carrying them. + ContentDigest string `json:"content_digest"` + + ExportedAt string `json:"exported_at"` + PrevReceiptHash string `json:"prev_receipt_hash,omitempty"` +} + +// Signature is the detached signature over a payload. +type Signature struct { + Alg string `json:"alg"` + Kid string `json:"kid"` + Sig string `json:"sig"` +} + +// SpanReceipt is a payload with its signature. Keeping them separate lets a +// verifier recompute the exact signed bytes without stripping fields out of a +// merged object. +type SpanReceipt struct { + Payload SpanReceiptPayload `json:"payload"` + Signature Signature `json:"signature"` +} + +// SigningKey is an Ed25519 identity. Kid is derived from the public key so it +// cannot be reassigned to a different key. +type SigningKey struct { + Kid string `json:"kid"` + Alg string `json:"alg"` + PublicKey string `json:"public_key"` + privateKey ed25519.PrivateKey `json:"-"` +} + +// KeyringEntry is one trusted key with its validity window. +type KeyringEntry struct { + Kid string `json:"kid"` + Alg string `json:"alg"` + PublicKey string `json:"public_key"` + // NotBefore and NotAfter are RFC3339 timestamps bounding when the key may + // sign. Empty means unbounded. + NotBefore string `json:"not_before,omitempty"` + NotAfter string `json:"not_after,omitempty"` + Status string `json:"status,omitempty"` + RevokedAt string `json:"revoked_at,omitempty"` +} + +// Keyring is the trust input a verifier supplies from its own side. It is +// published separately from receipts on purpose: an artifact that carried its +// own trust anchor would let a producer install trust in its own output. +type Keyring struct { + Type string `json:"type"` + Version int `json:"version"` + Keys []KeyringEntry `json:"keys"` +} + +// Coverage records what this exporter saw. It never claims to describe what the +// runtime executed, because spans dropped by the collector under buffer +// pressure never reach an exporter and leave no machine-readable trace. +type Coverage struct { + SpansReceived uint64 `json:"spans_received"` + SpansSigned uint64 `json:"spans_signed"` + SpansFailed uint64 `json:"spans_failed"` + // Note is carried in the artifact so a reader cannot mistake exporter + // coverage for runtime coverage. + Note string `json:"note"` +} + +// CheckpointPayload is the completeness anchor. +// +// A hash chain detects modification and reordering, and detects deletion from +// the middle. It cannot detect truncation: remove the newest receipts and every +// remaining link still verifies. The checkpoint closes that by committing to a +// count and a terminal hash, and it is only worth anything if it is published +// somewhere the operator cannot quietly rewrite. +type CheckpointPayload struct { + Type string `json:"type"` + Version int `json:"version"` + Sequence uint64 `json:"sequence"` + + ReceiptCount uint64 `json:"receipt_count"` + TerminalReceiptHash string `json:"terminal_receipt_hash,omitempty"` + Coverage Coverage `json:"coverage"` + SignedAt string `json:"signed_at"` +} + +// Checkpoint is a signed completeness anchor. +type Checkpoint struct { + Payload CheckpointPayload `json:"payload"` + Signature Signature `json:"signature"` +} + +// Bundle is a self-contained export for offline verification. +type Bundle struct { + Type string `json:"type"` + Version int `json:"version"` + Receipts []SpanReceipt `json:"receipts"` + Checkpoint *Checkpoint `json:"checkpoint,omitempty"` + // KeyringHint is a convenience. Verifying an artifact with a key the + // artifact supplied proves only internal consistency. + KeyringHint *Keyring `json:"keyring_hint,omitempty"` + VerificationNote string `json:"verification_note"` +} + +// GenerateSigningKey creates a fresh Ed25519 identity. +// +// This is a convenience for getting started, not a recommendation for +// production. Where the private key lives is what determines whether a +// signature is worth anything. +func GenerateSigningKey() (*SigningKey, error) { + pub, priv, err := ed25519.GenerateKey(nil) + if err != nil { + return nil, fmt.Errorf("generate ed25519 key: %w", err) + } + pubB64 := base64.StdEncoding.EncodeToString(pub) + return &SigningKey{ + Kid: KeyIDFromPublicKey(pubB64), + Alg: SigningAlg, + PublicKey: pubB64, + privateKey: priv, + }, nil +} + +// NewSigningKey adopts an existing Ed25519 private key, for deployments that +// hold key material elsewhere. +func NewSigningKey(priv ed25519.PrivateKey) (*SigningKey, error) { + if len(priv) != ed25519.PrivateKeySize { + return nil, fmt.Errorf("expected %d byte ed25519 private key, got %d", ed25519.PrivateKeySize, len(priv)) + } + pub, ok := priv.Public().(ed25519.PublicKey) + if !ok { + return nil, fmt.Errorf("private key does not yield an ed25519 public key") + } + pubB64 := base64.StdEncoding.EncodeToString(pub) + return &SigningKey{ + Kid: KeyIDFromPublicKey(pubB64), + Alg: SigningAlg, + PublicKey: pubB64, + privateKey: priv, + }, nil +} + +// KeyIDFromPublicKey derives a deterministic key id, so a kid always checks +// against the key it claims to identify. +func KeyIDFromPublicKey(publicKeyB64 string) string { + raw, err := base64.StdEncoding.DecodeString(publicKeyB64) + if err != nil { + return "gck_invalid" + } + sum := sha256.Sum256(raw) + return "gck_" + hex.EncodeToString(sum[:])[:16] +} + +// PublicEntry returns the public half of a key for publication in a keyring. +func (k *SigningKey) PublicEntry() KeyringEntry { + return KeyringEntry{Kid: k.Kid, Alg: k.Alg, PublicKey: k.PublicKey, Status: "active"} +} + +// NewKeyring builds a trust document from public entries. +func NewKeyring(entries ...KeyringEntry) *Keyring { + keys := make([]KeyringEntry, 0, len(entries)) + for _, e := range entries { + if e.Alg == "" { + e.Alg = SigningAlg + } + if e.Status == "" { + e.Status = "active" + } + keys = append(keys, e) + } + return &Keyring{Type: KeyringType, Version: 1, Keys: keys} +} + +// canonicalJSON produces deterministic bytes: object keys sorted by code unit, +// arrays left in order, no insignificant whitespace. +// +// Both signer and verifier must derive identical bytes from the same value or +// every signature is a coin flip. Go marshals struct fields in declaration +// order, which is deterministic within this package but not something a +// verifier written in another language can rely on, so keys are sorted. +func canonicalJSON(v any) ([]byte, error) { + raw, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("marshal: %w", err) + } + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var generic any + if err := dec.Decode(&generic); err != nil { + return nil, fmt.Errorf("decode for canonicalization: %w", err) + } + var buf bytes.Buffer + if err := writeCanonical(&buf, generic); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +func writeCanonical(buf *bytes.Buffer, v any) error { + switch value := v.(type) { + case nil: + buf.WriteString("null") + case bool: + if value { + buf.WriteString("true") + } else { + buf.WriteString("false") + } + case json.Number: + buf.WriteString(value.String()) + case string: + encoded, err := json.Marshal(value) + if err != nil { + return fmt.Errorf("marshal string: %w", err) + } + buf.Write(encoded) + case []any: + buf.WriteByte('[') + for i, item := range value { + if i > 0 { + buf.WriteByte(',') + } + if err := writeCanonical(buf, item); err != nil { + return err + } + } + buf.WriteByte(']') + case map[string]any: + keys := make([]string, 0, len(value)) + for k := range value { + keys = append(keys, k) + } + sort.Strings(keys) + buf.WriteByte('{') + for i, k := range keys { + if i > 0 { + buf.WriteByte(',') + } + encoded, err := json.Marshal(k) + if err != nil { + return fmt.Errorf("marshal key: %w", err) + } + buf.Write(encoded) + buf.WriteByte(':') + if err := writeCanonical(buf, value[k]); err != nil { + return err + } + } + buf.WriteByte('}') + default: + return fmt.Errorf("canonicalize: unsupported value of type %T", v) + } + return nil +} + +// DigestOf returns lowercase hex SHA-256 over canonical bytes. +func DigestOf(v any) (string, error) { + canonical, err := canonicalJSON(v) + if err != nil { + return "", err + } + sum := sha256.Sum256(canonical) + return hex.EncodeToString(sum[:]), nil +} + +func signingInput(domain string, payload any) ([]byte, error) { + canonical, err := canonicalJSON(payload) + if err != nil { + return nil, err + } + out := make([]byte, 0, len(domain)+1+len(canonical)) + out = append(out, domain...) + out = append(out, '\n') + out = append(out, canonical...) + return out, nil +} + +// ReceiptHash is the digest the next receipt chains to. It covers the whole +// envelope including the signature, so replacing a signature breaks the chain. +func ReceiptHash(r SpanReceipt) (string, error) { + return DigestOf(r) +} + +// PayloadFromSpan builds the signed fields from a span. +func PayloadFromSpan(span store.SpanData, sequence uint64, prevReceiptHash string, exportedAt time.Time) (SpanReceiptPayload, error) { + contentDigest, err := DigestOf(map[string]any{ + "input_preview": span.InputPreview, + "output_preview": span.OutputPreview, + "metadata": string(span.Metadata), + "model_params": string(span.ModelParams), + "total_cost": formatCost(span.TotalCost), + }) + if err != nil { + return SpanReceiptPayload{}, fmt.Errorf("content digest: %w", err) + } + + payload := SpanReceiptPayload{ + Type: ReceiptType, + Version: 1, + Sequence: sequence, + SpanID: span.ID.String(), + TraceID: span.TraceID.String(), + TenantID: span.TenantID.String(), + SpanType: span.SpanType, + Name: span.Name, + Status: span.Status, + Error: span.Error, + ToolName: span.ToolName, + ToolCallID: span.ToolCallID, + Model: span.Model, + Provider: span.Provider, + FinishReason: span.FinishReason, + InputTokens: span.InputTokens, + OutputTokens: span.OutputTokens, + StartTime: span.StartTime.UTC().Format(time.RFC3339Nano), + DurationMS: span.DurationMS, + ContentDigest: contentDigest, + ExportedAt: exportedAt.UTC().Format(time.RFC3339Nano), + PrevReceiptHash: prevReceiptHash, + } + if span.ParentSpanID != nil { + payload.ParentSpanID = span.ParentSpanID.String() + } + if span.TeamID != nil { + payload.TeamID = span.TeamID.String() + } + if span.AgentID != nil { + payload.AgentID = span.AgentID.String() + } + if span.EndTime != nil { + payload.EndTime = span.EndTime.UTC().Format(time.RFC3339Nano) + } + return payload, nil +} + +// formatCost renders a nullable float deterministically. Floats are not signed +// directly because a signer and verifier can disagree on representation. +func formatCost(cost *float64) string { + if cost == nil { + return "" + } + return fmt.Sprintf("%.10f", *cost) +} + +// SignReceipt signs a payload. +func SignReceipt(payload SpanReceiptPayload, key *SigningKey) (SpanReceipt, error) { + if key == nil || key.privateKey == nil { + return SpanReceipt{}, fmt.Errorf("signing key is required") + } + input, err := signingInput(signingDomain, payload) + if err != nil { + return SpanReceipt{}, err + } + sig := ed25519.Sign(key.privateKey, input) + return SpanReceipt{ + Payload: payload, + Signature: Signature{Alg: SigningAlg, Kid: key.Kid, Sig: base64.StdEncoding.EncodeToString(sig)}, + }, nil +} + +// SignCheckpoint signs a completeness anchor. +func SignCheckpoint(payload CheckpointPayload, key *SigningKey) (Checkpoint, error) { + if key == nil || key.privateKey == nil { + return Checkpoint{}, fmt.Errorf("signing key is required") + } + input, err := signingInput(checkpointDomain, payload) + if err != nil { + return Checkpoint{}, err + } + sig := ed25519.Sign(key.privateKey, input) + return Checkpoint{ + Payload: payload, + Signature: Signature{Alg: SigningAlg, Kid: key.Kid, Sig: base64.StdEncoding.EncodeToString(sig)}, + }, nil +} + +// resolveKey finds the key a receipt names and decides whether it was trusted +// at the moment the receipt claims to have been signed. +// +// Revocation is deliberately not retroactive: a receipt signed while the key +// was trusted stays valid, because the alternative silently voids honest +// history every time an operator rotates. RevokedAt exists for the compromise +// case, where everything from that moment on should fail. +func resolveKey(keyring *Keyring, kid string, at time.Time) (KeyringEntry, error) { + if keyring == nil { + return KeyringEntry{}, fmt.Errorf("keyring is required") + } + if keyring.Type != KeyringType { + return KeyringEntry{}, fmt.Errorf("unsupported keyring type %q", keyring.Type) + } + for _, key := range keyring.Keys { + if key.Kid != kid { + continue + } + if key.Alg != SigningAlg { + return KeyringEntry{}, fmt.Errorf("key %s uses unsupported algorithm %q", kid, key.Alg) + } + if KeyIDFromPublicKey(key.PublicKey) != key.Kid { + return KeyringEntry{}, fmt.Errorf("key %s does not match the public key it names", kid) + } + if err := checkWindow(key, at, kid); err != nil { + return KeyringEntry{}, err + } + return key, nil + } + return KeyringEntry{}, fmt.Errorf("no key in keyring for kid %s", kid) +} + +func checkWindow(key KeyringEntry, at time.Time, kid string) error { + if key.NotBefore != "" { + nb, err := time.Parse(time.RFC3339Nano, key.NotBefore) + if err != nil { + return fmt.Errorf("key %s has an unparseable not_before: %w", kid, err) + } + if at.Before(nb) { + return fmt.Errorf("receipt predates the validity window of key %s", kid) + } + } + if key.NotAfter != "" { + na, err := time.Parse(time.RFC3339Nano, key.NotAfter) + if err != nil { + return fmt.Errorf("key %s has an unparseable not_after: %w", kid, err) + } + if at.After(na) { + return fmt.Errorf("receipt postdates the validity window of key %s", kid) + } + } + if strings.EqualFold(key.Status, "revoked") && key.RevokedAt != "" { + ra, err := time.Parse(time.RFC3339Nano, key.RevokedAt) + if err != nil { + return fmt.Errorf("key %s has an unparseable revoked_at: %w", kid, err) + } + if !at.Before(ra) { + return fmt.Errorf("key %s was revoked before this receipt was signed", kid) + } + } + return nil +} + +func verifySignature(domain string, payload any, sig Signature, keyring *Keyring, at time.Time) error { + if sig.Alg != SigningAlg { + return fmt.Errorf("unsupported signature algorithm %q", sig.Alg) + } + key, err := resolveKey(keyring, sig.Kid, at) + if err != nil { + return err + } + pubRaw, err := base64.StdEncoding.DecodeString(key.PublicKey) + if err != nil { + return fmt.Errorf("public key for %s could not be decoded: %w", sig.Kid, err) + } + if len(pubRaw) != ed25519.PublicKeySize { + return fmt.Errorf("public key for %s is not an ed25519 key", sig.Kid) + } + sigRaw, err := base64.StdEncoding.DecodeString(sig.Sig) + if err != nil { + return fmt.Errorf("signature could not be decoded: %w", err) + } + input, err := signingInput(domain, payload) + if err != nil { + return err + } + if !ed25519.Verify(ed25519.PublicKey(pubRaw), input, sigRaw) { + return fmt.Errorf("signature does not verify over the payload") + } + return nil +} diff --git a/internal/tracing/receiptexport/receipt_test.go b/internal/tracing/receiptexport/receipt_test.go new file mode 100644 index 00000000..6a1c266a --- /dev/null +++ b/internal/tracing/receiptexport/receipt_test.go @@ -0,0 +1,371 @@ +package receiptexport + +import ( + "crypto/ed25519" + "encoding/base64" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +var testTime = time.Date(2026, 8, 5, 12, 0, 0, 0, time.UTC) + +func testKeyring(t *testing.T, key *SigningKey) *Keyring { + t.Helper() + return NewKeyring(key.PublicEntry()) +} + +func testSpan(name string) store.SpanData { + return store.SpanData{ + ID: uuid.New(), + TraceID: uuid.New(), + TenantID: uuid.New(), + SpanType: "tool_call", + Name: name, + Status: "ok", + ToolName: "shell", + ToolCallID: "call_" + name, + StartTime: testTime, + InputPreview: "ls -la", + OutputPreview: "total 0", + } +} + +// signChain builds n chained receipts, so tests can attack a realistic sequence +// rather than a single artifact. +func signChain(t *testing.T, key *SigningKey, n int) ([]SpanReceipt, string) { + t.Helper() + receipts := make([]SpanReceipt, 0, n) + prev := "" + for i := 0; i < n; i++ { + payload, err := PayloadFromSpan(testSpan(string(rune('a'+i))), uint64(i), prev, testTime.Add(time.Duration(i)*time.Second)) + if err != nil { + t.Fatalf("payload %d: %v", i, err) + } + receipt, err := SignReceipt(payload, key) + if err != nil { + t.Fatalf("sign %d: %v", i, err) + } + hash, err := ReceiptHash(receipt) + if err != nil { + t.Fatalf("hash %d: %v", i, err) + } + receipts = append(receipts, receipt) + prev = hash + } + return receipts, prev +} + +func checkpointFor(t *testing.T, key *SigningKey, receipts []SpanReceipt, terminal string) *Checkpoint { + t.Helper() + cp, err := SignCheckpoint(CheckpointPayload{ + Type: CheckpointType, + Version: 1, + Sequence: uint64(len(receipts)), + ReceiptCount: uint64(len(receipts)), + TerminalReceiptHash: terminal, + Coverage: Coverage{SpansReceived: uint64(len(receipts)), SpansSigned: uint64(len(receipts))}, + SignedAt: testTime.Add(time.Hour).Format(time.RFC3339Nano), + }, key) + if err != nil { + t.Fatalf("sign checkpoint: %v", err) + } + return &cp +} + +func TestCanonicalJSONIsOrderIndependent(t *testing.T) { + a, err := canonicalJSON(map[string]any{"b": 1, "a": 2}) + if err != nil { + t.Fatal(err) + } + b, err := canonicalJSON(map[string]any{"a": 2, "b": 1}) + if err != nil { + t.Fatal(err) + } + if string(a) != string(b) { + t.Fatalf("canonical form depends on insertion order: %q vs %q", a, b) + } + if string(a) != `{"a":2,"b":1}` { + t.Fatalf("unexpected canonical form %q", a) + } +} + +func TestCanonicalJSONPreservesArrayOrder(t *testing.T) { + a, _ := canonicalJSON([]int{1, 2}) + b, _ := canonicalJSON([]int{2, 1}) + if string(a) == string(b) { + t.Fatal("array order must be significant") + } +} + +func TestKeyIDDerivesFromPublicKey(t *testing.T) { + key, err := GenerateSigningKey() + if err != nil { + t.Fatal(err) + } + if KeyIDFromPublicKey(key.PublicKey) != key.Kid { + t.Fatal("kid must be derivable from the public key it names") + } +} + +func TestKeyringEntryLyingAboutItsKeyIsRejected(t *testing.T) { + key, _ := GenerateSigningKey() + other, _ := GenerateSigningKey() + + lying := NewKeyring(KeyringEntry{Kid: key.Kid, Alg: SigningAlg, PublicKey: other.PublicKey}) + receipts, terminal := signChain(t, key, 1) + + report, err := VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), lying) + if err != nil { + t.Fatal(err) + } + if report.Results[0].SignatureValid { + t.Fatal("a kid that does not match its public key must not verify") + } + if !strings.Contains(report.Results[0].SignatureError, "does not match the public key") { + t.Fatalf("unexpected error: %s", report.Results[0].SignatureError) + } +} + +func TestValidChainWithCheckpointVerifies(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 4) + + report, err := VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), testKeyring(t, key)) + if err != nil { + t.Fatal(err) + } + if !report.Valid { + t.Fatalf("expected valid, got %+v", report) + } + if !report.CheckpointValid || report.SignaturesInvalid != 0 || !report.ChainIntact { + t.Fatalf("unexpected report: %+v", report) + } +} + +func TestTamperedPayloadFailsSignature(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 2) + receipts[1].Payload.ToolName = "rm" + + report, _ := VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), testKeyring(t, key)) + if report.Results[1].SignatureValid { + t.Fatal("editing a signed field must break the signature") + } + if report.Valid { + t.Fatal("report must not be valid") + } +} + +func TestForgedSignatureUnderTrustedKidFails(t *testing.T) { + key, _ := GenerateSigningKey() + attacker, _ := GenerateSigningKey() + + payload, _ := PayloadFromSpan(testSpan("x"), 0, "", testTime) + forged, _ := SignReceipt(payload, attacker) + forged.Signature.Kid = key.Kid // claim a trusted key + + report, _ := VerifyReceipts([]SpanReceipt{forged}, nil, testKeyring(t, key)) + if report.Results[0].SignatureValid { + t.Fatal("a signature by another key must not verify under a claimed kid") + } +} + +// A receipt signature must not be replayable as a checkpoint signature, which +// is what the separate signing domains are for. +func TestSigningDomainsAreSeparated(t *testing.T) { + key, _ := GenerateSigningKey() + payload := CheckpointPayload{ + Type: CheckpointType, Version: 1, ReceiptCount: 0, + SignedAt: testTime.Format(time.RFC3339Nano), + } + + input, err := signingInput(signingDomain, payload) // deliberately the wrong domain + if err != nil { + t.Fatal(err) + } + wrongDomain := Checkpoint{ + Payload: payload, + Signature: Signature{ + Alg: SigningAlg, Kid: key.Kid, + Sig: base64.StdEncoding.EncodeToString(ed25519.Sign(key.privateKey, input)), + }, + } + + report, _ := VerifyReceipts(nil, &wrongDomain, testKeyring(t, key)) + if report.CheckpointValid { + t.Fatal("a signature made in the receipt domain must not verify as a checkpoint") + } +} + +func TestKeyValidityWindow(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 1) + + tooEarly := NewKeyring(KeyringEntry{ + Kid: key.Kid, Alg: SigningAlg, PublicKey: key.PublicKey, + NotBefore: testTime.Add(time.Hour).Format(time.RFC3339Nano), + }) + report, _ := VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), tooEarly) + if report.Results[0].SignatureValid { + t.Fatal("a receipt signed before the key was valid must fail") + } + + tooLate := NewKeyring(KeyringEntry{ + Kid: key.Kid, Alg: SigningAlg, PublicKey: key.PublicKey, + NotAfter: testTime.Add(-time.Hour).Format(time.RFC3339Nano), + }) + report, _ = VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), tooLate) + if report.Results[0].SignatureValid { + t.Fatal("a receipt signed after the key expired must fail") + } +} + +// Rotation must not silently void honest history, or nobody will ever rotate. +func TestRevocationIsNotRetroactive(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 1) + + revokedLater := NewKeyring(KeyringEntry{ + Kid: key.Kid, Alg: SigningAlg, PublicKey: key.PublicKey, + Status: "revoked", RevokedAt: testTime.Add(time.Hour).Format(time.RFC3339Nano), + }) + report, _ := VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), revokedLater) + if !report.Results[0].SignatureValid { + t.Fatalf("a receipt signed before revocation must stay valid: %s", report.Results[0].SignatureError) + } + + revokedBefore := NewKeyring(KeyringEntry{ + Kid: key.Kid, Alg: SigningAlg, PublicKey: key.PublicKey, + Status: "revoked", RevokedAt: testTime.Add(-time.Hour).Format(time.RFC3339Nano), + }) + report, _ = VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), revokedBefore) + if report.Results[0].SignatureValid { + t.Fatal("a receipt signed after revocation must fail") + } +} + +func TestDeletionFromTheMiddleIsDetected(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, _ := signChain(t, key, 4) + + cut := append([]SpanReceipt{}, receipts[:2]...) + cut = append(cut, receipts[3]) + + report, _ := VerifyReceipts(cut, nil, testKeyring(t, key)) + if report.SignaturesInvalid != 0 { + t.Fatal("remaining signatures should still be genuine") + } + if report.ChainIntact { + t.Fatal("a missing link must be detected") + } +} + +func TestOneBreakDoesNotCascade(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, _ := signChain(t, key, 5) + + cut := append([]SpanReceipt{receipts[0]}, receipts[2:]...) + report, _ := VerifyReceipts(cut, nil, testKeyring(t, key)) + + broken := 0 + for _, r := range report.Results { + if !r.ChainValid { + broken++ + } + } + if broken != 1 { + t.Fatalf("expected exactly one broken link, got %d", broken) + } +} + +// The failure a hash chain alone cannot catch. Removing the newest receipts +// leaves every remaining link and signature perfectly valid, so only an anchor +// committing to a count and terminal hash detects it. +func TestTruncationFromTheEndIsDetectedOnlyByTheCheckpoint(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 4) + checkpoint := checkpointFor(t, key, receipts, terminal) + + truncated := receipts[:2] + + withoutAnchor, _ := VerifyReceipts(truncated, nil, testKeyring(t, key)) + if withoutAnchor.SignaturesInvalid != 0 || !withoutAnchor.ChainIntact { + t.Fatal("truncation leaves signatures and internal links intact, which is the point") + } + if withoutAnchor.Valid { + t.Fatal("a report without a checkpoint must never be valid, since completeness is unproven") + } + + withAnchor, _ := VerifyReceipts(truncated, checkpoint, testKeyring(t, key)) + if withAnchor.CheckpointValid { + t.Fatal("the checkpoint must reject a truncated bundle") + } + if !strings.Contains(withAnchor.CheckpointError, "expects 4 receipts") { + t.Fatalf("unexpected checkpoint error: %s", withAnchor.CheckpointError) + } +} + +func TestEmptyBundleDoesNotVerify(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 3) + checkpoint := checkpointFor(t, key, receipts, terminal) + + // Handing someone zero receipts must not read as success. + report, _ := VerifyReceipts(nil, checkpoint, testKeyring(t, key)) + if report.CheckpointValid || report.Valid { + t.Fatal("an empty bundle must fail against a checkpoint naming receipts") + } + + bare, _ := VerifyReceipts(nil, nil, testKeyring(t, key)) + if bare.Valid { + t.Fatal("an empty bundle with no checkpoint must not be valid either") + } +} + +func TestGenuinelyEmptyRunVerifies(t *testing.T) { + key, _ := GenerateSigningKey() + empty := checkpointFor(t, key, nil, "") + + report, _ := VerifyReceipts(nil, empty, testKeyring(t, key)) + if !report.Valid { + t.Fatalf("a run that produced nothing, anchored as such, should verify: %+v", report) + } +} + +func TestReportStatesWhatItDoesNotEstablish(t *testing.T) { + key, _ := GenerateSigningKey() + receipts, terminal := signChain(t, key, 1) + report, _ := VerifyReceipts(receipts, checkpointFor(t, key, receipts, terminal), testKeyring(t, key)) + + if len(report.DoesNotEstablish) == 0 { + t.Fatal("verification must state its limits") + } + joined := strings.Join(report.DoesNotEstablish, " ") + if !strings.Contains(joined, "drops spans") { + t.Fatal("the collector drop caveat must be carried in the report") + } +} + +func TestContentIsCommittedByDigestNotCopied(t *testing.T) { + span := testSpan("secret") + span.InputPreview = "sensitive-argument-value" + payload, err := PayloadFromSpan(span, 0, "", testTime) + if err != nil { + t.Fatal(err) + } + canonical, err := canonicalJSON(payload) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(canonical), "sensitive-argument-value") { + t.Fatal("previews must be committed to by digest, not copied into the receipt") + } + if payload.ContentDigest == "" { + t.Fatal("a content digest is required") + } +} diff --git a/internal/tracing/receiptexport/verify.go b/internal/tracing/receiptexport/verify.go new file mode 100644 index 00000000..369b5c0c --- /dev/null +++ b/internal/tracing/receiptexport/verify.go @@ -0,0 +1,164 @@ +package receiptexport + +import ( + "fmt" + "time" +) + +// ReceiptResult is the per-receipt outcome of verification. +// +// Signature validity and chain integrity are reported separately on purpose. +// A validly signed receipt pointing at the wrong predecessor means something +// was removed or reordered, which is a different failure from a forged +// signature and calls for a different response. Collapsing them into one +// boolean would hide the distinction that makes the chain worth having. +type ReceiptResult struct { + Index int `json:"index"` + SpanID string `json:"span_id"` + Sequence uint64 `json:"sequence"` + SignatureValid bool `json:"signature_valid"` + SignatureError string `json:"signature_error,omitempty"` + ChainValid bool `json:"chain_valid"` + ChainError string `json:"chain_error,omitempty"` +} + +// VerificationReport is the result of checking a set of receipts. +type VerificationReport struct { + Type string `json:"type"` + ReceiptsChecked int `json:"receipts_checked"` + SignaturesValid int `json:"signatures_valid"` + SignaturesInvalid int `json:"signatures_invalid"` + ChainIntact bool `json:"chain_intact"` + + // CheckpointChecked is false when no anchor was supplied. Without one, + // truncation is undetectable: remove the newest receipts and every + // remaining link still verifies. + CheckpointChecked bool `json:"checkpoint_checked"` + CheckpointValid bool `json:"checkpoint_valid"` + CheckpointError string `json:"checkpoint_error,omitempty"` + + Valid bool `json:"valid"` + Coverage *Coverage `json:"coverage,omitempty"` + Results []ReceiptResult `json:"results"` + Establishes string `json:"establishes"` + DoesNotEstablish []string `json:"does_not_establish"` +} + +// VerifyReceipts checks signatures and chain links over an ordered sequence. +// +// A checkpoint is optional but strongly recommended. Without one the report +// says so via CheckpointChecked, and Valid can only mean "nothing I was given +// has been altered", never "nothing is missing". +func VerifyReceipts(receipts []SpanReceipt, checkpoint *Checkpoint, keyring *Keyring) (VerificationReport, error) { + if keyring == nil { + return VerificationReport{}, fmt.Errorf("keyring is required") + } + + report := VerificationReport{ + Type: "goclaw.receipt_verification.v1", + ReceiptsChecked: len(receipts), + ChainIntact: true, + Establishes: "Each verified span was exported and signed by the named key while that key was trusted, and the verified sequence has not been altered.", + DoesNotEstablish: []string{ + "It does not establish that these are all the spans the runtime produced. The collector drops spans under buffer pressure and an exporter can only sign what it receives.", + "It does not establish that a tool call was correct, authorised, or safe.", + "It does not establish that receipts signed by a compromised key were truthful.", + "Without a checkpoint it does not establish that no receipts were removed from the end of the sequence.", + }, + } + + previousHash := "" + for index, receipt := range receipts { + result := ReceiptResult{ + Index: index, + SpanID: receipt.Payload.SpanID, + Sequence: receipt.Payload.Sequence, + } + + if receipt.Payload.Type != ReceiptType { + result.SignatureError = fmt.Sprintf("unsupported receipt type %q", receipt.Payload.Type) + } else { + exportedAt, err := time.Parse(time.RFC3339Nano, receipt.Payload.ExportedAt) + if err != nil { + result.SignatureError = fmt.Sprintf("unparseable exported_at: %v", err) + } else if err := verifySignature(signingDomain, receipt.Payload, receipt.Signature, keyring, exportedAt); err != nil { + result.SignatureError = err.Error() + } else { + result.SignatureValid = true + report.SignaturesValid++ + } + } + + expected := "" + if index > 0 { + expected = previousHash + } + if receipt.Payload.PrevReceiptHash == expected { + result.ChainValid = true + } else { + report.ChainIntact = false + result.ChainError = fmt.Sprintf("expected prev_receipt_hash %q but found %q", expected, receipt.Payload.PrevReceiptHash) + } + + // Chain from what is actually present, so one break does not cascade + // into every later receipt reporting a failure it did not cause. + hash, err := ReceiptHash(receipt) + if err != nil { + return VerificationReport{}, fmt.Errorf("hash receipt %d: %w", index, err) + } + previousHash = hash + + report.Results = append(report.Results, result) + } + + report.SignaturesInvalid = report.ReceiptsChecked - report.SignaturesValid + + if checkpoint != nil { + report.CheckpointChecked = true + if err := verifyCheckpoint(*checkpoint, receipts, previousHash, keyring); err != nil { + report.CheckpointError = err.Error() + } else { + report.CheckpointValid = true + cov := checkpoint.Payload.Coverage + report.Coverage = &cov + } + } + + report.Valid = report.SignaturesInvalid == 0 && + report.ChainIntact && + report.CheckpointChecked && + report.CheckpointValid + + return report, nil +} + +// verifyCheckpoint closes the truncation hole a chain alone cannot. +func verifyCheckpoint(checkpoint Checkpoint, receipts []SpanReceipt, terminalHash string, keyring *Keyring) error { + if checkpoint.Payload.Type != CheckpointType { + return fmt.Errorf("unsupported checkpoint type %q", checkpoint.Payload.Type) + } + signedAt, err := time.Parse(time.RFC3339Nano, checkpoint.Payload.SignedAt) + if err != nil { + return fmt.Errorf("unparseable signed_at: %w", err) + } + if err := verifySignature(checkpointDomain, checkpoint.Payload, checkpoint.Signature, keyring, signedAt); err != nil { + return fmt.Errorf("checkpoint signature: %w", err) + } + if uint64(len(receipts)) != checkpoint.Payload.ReceiptCount { + return fmt.Errorf("checkpoint expects %d receipts but the bundle holds %d", + checkpoint.Payload.ReceiptCount, len(receipts)) + } + // An empty bundle is only acceptable when the anchor says the run produced + // nothing. Otherwise handing someone zero receipts would verify clean. + if len(receipts) == 0 { + if checkpoint.Payload.TerminalReceiptHash != "" { + return fmt.Errorf("checkpoint names a terminal receipt but the bundle is empty") + } + return nil + } + if checkpoint.Payload.TerminalReceiptHash != terminalHash { + return fmt.Errorf("checkpoint terminal hash %q does not match the last receipt %q", + checkpoint.Payload.TerminalReceiptHash, terminalHash) + } + return nil +} From 8ecd3c029f1da7e643e62004f840d57eb8f898d0 Mon Sep 17 00:00:00 2001 From: daotri Date: Tue, 1 Sep 2026 03:54:08 +0700 Subject: [PATCH 14/15] fix(agent): repair transcript pairing after compaction and truncation (#1470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mid-loop compaction output now runs through sanitizeHistory so orphaned tool messages never reach the provider (OpenAI rejects role:tool without a preceding assistant tool_calls). Background summarization truncates on tool-chain boundaries via the shared toolChainSplitIndex helper so the kept tail never starts mid tool-chain. Co-authored-by: Trí Đào Nguyễn Minh --- internal/agent/loop_compact.go | 34 +-- internal/agent/loop_compact_boundary_test.go | 205 +++++++++++++++++++ internal/agent/loop_history_sanitize.go | 14 +- internal/agent/loop_pipeline_callbacks.go | 7 + 4 files changed, 246 insertions(+), 14 deletions(-) create mode 100644 internal/agent/loop_compact_boundary_test.go diff --git a/internal/agent/loop_compact.go b/internal/agent/loop_compact.go index 7adb6382..8ee15293 100644 --- a/internal/agent/loop_compact.go +++ b/internal/agent/loop_compact.go @@ -56,18 +56,7 @@ func (l *Loop) compactMessagesInPlace(ctx context.Context, messages []providers. keepCount = minKeep } - splitIdx := len(messages) - keepCount - - // Walk backward from splitIdx to find a clean boundary — - // avoid splitting tool_use → tool_result pairs. - for splitIdx > 0 { - m := messages[splitIdx] - if m.Role == "tool" || (m.Role == "assistant" && len(m.ToolCalls) > 0) { - splitIdx-- - continue - } - break - } + splitIdx := toolChainSplitIndex(messages, len(messages)-keepCount) if splitIdx <= 1 { return nil } @@ -133,6 +122,27 @@ func (l *Loop) compactMessagesInPlace(ctx context.Context, messages []providers. return result } +// toolChainSplitIndex walks backward from splitIdx to the nearest clean +// boundary so the kept tail (messages[splitIdx:]) never starts mid tool-chain. +// A tail starting with a tool result — or splitting an assistant(tool_calls) → +// tool sequence — produces an orphaned role:"tool" message, which OpenAI +// rejects with a 400 on the next request. Used by both mid-loop compaction +// and the background summarizer's truncation. +func toolChainSplitIndex(messages []providers.Message, splitIdx int) int { + if splitIdx > len(messages) { + splitIdx = len(messages) + } + for splitIdx > 0 && splitIdx < len(messages) { + m := messages[splitIdx] + if m.Role == "tool" || (m.Role == "assistant" && len(m.ToolCalls) > 0) { + splitIdx-- + continue + } + break + } + return splitIdx +} + // dynamicSummaryMax returns the output-token budget for a compaction or // summarization call, scaled to input size. Formula: in/25 (~4% compression), // clamped to [1024, 8192]. Floor keeps short summaries coherent; cap prevents diff --git a/internal/agent/loop_compact_boundary_test.go b/internal/agent/loop_compact_boundary_test.go new file mode 100644 index 00000000..98b49ef2 --- /dev/null +++ b/internal/agent/loop_compact_boundary_test.go @@ -0,0 +1,205 @@ +package agent + +import ( + "context" + "testing" + "time" + + "github.com/nextlevelbuilder/goclaw/internal/providers" +) + +// TestToolChainSplitIndex verifies the shared boundary walk used by mid-loop +// compaction and the background summarizer: the kept tail (messages[idx:]) +// must never start with a tool result or split an assistant(tool_calls) → +// tool chain, because providers reject orphaned role:"tool" messages. +func TestToolChainSplitIndex(t *testing.T) { + user := providers.Message{Role: "user", Content: "u"} + assistant := providers.Message{Role: "assistant", Content: "a"} + assistantTC := providers.Message{ + Role: "assistant", + Content: "calling tool", + ToolCalls: []providers.ToolCall{{ID: "tc-1"}}, + } + tool := providers.Message{Role: "tool", Content: "result", ToolCallID: "tc-1"} + + tests := []struct { + name string + messages []providers.Message + splitIdx int + want int + }{ + { + name: "clean boundary on user message stays put", + messages: []providers.Message{user, assistant, user, assistant}, + splitIdx: 2, + want: 2, + }, + { + name: "split on tool result walks back before assistant tool_calls", + messages: []providers.Message{user, assistant, assistantTC, tool, user}, + splitIdx: 3, + want: 1, + }, + { + name: "split on assistant with tool_calls walks back", + messages: []providers.Message{user, assistant, assistantTC, tool, user}, + splitIdx: 2, + want: 1, + }, + { + name: "consecutive tool results walk back through whole chain", + messages: []providers.Message{user, assistantTC, tool, tool, user}, + splitIdx: 3, + want: 0, + }, + { + name: "split at zero unchanged", + messages: []providers.Message{user, assistant}, + splitIdx: 0, + want: 0, + }, + { + name: "split at len keeps empty tail unchanged", + messages: []providers.Message{user, assistantTC, tool}, + splitIdx: 3, + want: 3, + }, + { + name: "split beyond len clamps to len", + messages: []providers.Message{user, assistant}, + splitIdx: 5, + want: 2, + }, + { + name: "chain reaching history start returns zero", + messages: []providers.Message{assistantTC, tool, user}, + splitIdx: 1, + want: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := toolChainSplitIndex(tt.messages, tt.splitIdx) + if got != tt.want { + t.Fatalf("toolChainSplitIndex() = %d, want %d", got, tt.want) + } + // Invariant: the kept tail never starts mid tool-chain. + if got > 0 && got < len(tt.messages) { + m := tt.messages[got] + if m.Role == "tool" { + t.Errorf("kept tail starts with role=tool at index %d", got) + } + if m.Role == "assistant" && len(m.ToolCalls) > 0 { + t.Errorf("kept tail starts on assistant(tool_calls) at index %d", got) + } + } + }) + } +} + +// TestMakeCompactMessages_SanitizesOrphanTool verifies the compaction callback +// repairs pairing on the compacted slice: an orphaned tool message surviving +// inside the kept tail must be dropped before the slice replaces run history. +func TestMakeCompactMessages_SanitizesOrphanTool(t *testing.T) { + cap := &capturingProvider{response: "Summary of conversation."} + loop := &Loop{ + provider: cap, + model: "claude-3-5-sonnet", + } + + // 10 messages → keepCount=4, splitIdx=6. messages[6] is a plain user + // message (clean boundary), so the orphan tool at index 7 lands in the + // kept tail and must be repaired by the sanitize pass. + msgs := []providers.Message{ + {Role: "user", Content: "u0"}, + {Role: "assistant", Content: "a1"}, + {Role: "user", Content: "u2"}, + {Role: "assistant", Content: "a3"}, + {Role: "user", Content: "u4"}, + {Role: "assistant", Content: "a5"}, + {Role: "user", Content: "u6"}, + {Role: "tool", Content: "orphaned result", ToolCallID: "orphan-1"}, + {Role: "user", Content: "u8"}, + {Role: "assistant", Content: "a9"}, + } + + compact := loop.makeCompactMessages(nil) + result, err := compact(context.Background(), msgs, loop.model) + if err != nil { + t.Fatalf("compact callback error: %v", err) + } + if len(result) == 0 { + t.Fatal("compact callback returned empty messages") + } + if len(cap.captured) != 1 { + t.Fatalf("provider.Chat called %d time(s), want 1 (compaction must have run)", len(cap.captured)) + } + for i, m := range result { + if m.Role == "tool" { + t.Errorf("orphaned tool message survived at index %d (tool_call_id=%s)", i, m.ToolCallID) + } + } +} + +// truncCapturingStore records the keepLast argument of TruncateHistory calls. +type truncCapturingStore struct { + nopSessionStore + truncated chan int +} + +func (s *truncCapturingStore) TruncateHistory(_ context.Context, _ string, keepLast int) { + select { + case s.truncated <- keepLast: + default: + } +} + +// TestMaybeSummarize_ToolChainBoundary verifies the background summarizer +// widens the kept tail when the raw split (len-keepLast) lands mid tool-chain, +// so truncation never persists a history starting with an orphaned tool message. +func TestMaybeSummarize_ToolChainBoundary(t *testing.T) { + const contextWindow = 10000 + + // threshold = 10000 * 0.85 = 8500 tokens; long content pushes the + // estimate over it so summarization triggers. + longContent := makeLongString(9000) + history := []providers.Message{ + {Role: "user", Content: longContent}, // 0 + {Role: "assistant", Content: longContent}, // 1 + {Role: "user", Content: longContent}, // 2 + {Role: "assistant", Content: longContent}, // 3 + {Role: "assistant", Content: longContent, // 4 + ToolCalls: []providers.ToolCall{{ID: "tc-a"}, {ID: "tc-b"}}}, + {Role: "tool", Content: "result a", ToolCallID: "tc-a"}, // 5 + {Role: "tool", Content: "result b", ToolCallID: "tc-b"}, // 6 + {Role: "user", Content: longContent}, // 7 + {Role: "assistant", Content: longContent}, // 8 + {Role: "user", Content: longContent}, // 9 + } + // keepLast=4 → raw split at index 6 (a tool result). The boundary walk + // must retreat to index 3, keeping the last 7 messages. + const wantKeep = 7 + + store := &truncCapturingStore{ + nopSessionStore: nopSessionStore{history: history}, + truncated: make(chan int, 1), + } + loop := &Loop{ + provider: &capturingProvider{response: "summary"}, + model: "claude-3-5-sonnet", + contextWindow: contextWindow, + sessions: store, + } + + loop.maybeSummarize(context.Background(), "test-session-key") + + select { + case got := <-store.truncated: + if got != wantKeep { + t.Fatalf("TruncateHistory keepLast = %d, want %d (kept tail must not start mid tool-chain)", got, wantKeep) + } + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for maybeSummarize to truncate history") + } +} diff --git a/internal/agent/loop_history_sanitize.go b/internal/agent/loop_history_sanitize.go index 706f0dc9..514a1153 100644 --- a/internal/agent/loop_history_sanitize.go +++ b/internal/agent/loop_history_sanitize.go @@ -242,8 +242,18 @@ func (l *Loop) maybeSummarize(ctx context.Context, sessionKey string) { return } + // Adjust the split so the kept tail never starts mid tool-chain — + // a tail beginning with a tool result (or splitting assistant(tool_calls) + // from its results) persists an orphaned role:"tool" message that + // OpenAI rejects with a 400 on the next request. + splitIdx := toolChainSplitIndex(history, len(history)-keepLast) + if splitIdx <= 0 { + return + } + effectiveKeep := len(history) - splitIdx + summary := l.sessions.GetSummary(sctx, sessionKey) - toSummarize := history[:len(history)-keepLast] + toSummarize := history[:splitIdx] var sb strings.Builder var mediaKinds []string @@ -308,7 +318,7 @@ func (l *Loop) maybeSummarize(ctx context.Context, sessionKey string) { } l.sessions.SetSummary(sctx, sessionKey, SanitizeAssistantContent(resp.Content)) - l.sessions.TruncateHistory(sctx, sessionKey, keepLast) + l.sessions.TruncateHistory(sctx, sessionKey, effectiveKeep) // Inject preserved MediaRefs into the first kept message so they survive truncation. if len(preservedRefs) > 0 { diff --git a/internal/agent/loop_pipeline_callbacks.go b/internal/agent/loop_pipeline_callbacks.go index 39b84398..84a8f3eb 100644 --- a/internal/agent/loop_pipeline_callbacks.go +++ b/internal/agent/loop_pipeline_callbacks.go @@ -560,6 +560,13 @@ func (l *Loop) makeCompactMessages(req *RunRequest) func(ctx context.Context, ms if compacted == nil { return msgs, nil // compaction failed, return original } + // The compacted slice replaces run history and is persisted verbatim — + // repair tool_use/tool_result pairing now so an orphaned role:"tool" + // message never reaches the provider (OpenAI rejects it with a 400). + compacted, repaired := sanitizeHistory(compacted) + if repaired > 0 { + slog.Warn("post_compaction_sanitize", "agent", l.id, "repaired", repaired) + } // Stamp session metadata with the compaction timestamp so operators // can diagnose compaction cadence without a dedicated column. Stored // as RFC3339 string in sessions.metadata JSONB (flushed on next save). From d67e1b09091f5b4790956d4bc1db9704b5dfe081 Mon Sep 17 00:00:00 2001 From: DennyHo0917 Date: Sun, 6 Sep 2026 19:05:35 +0800 Subject: [PATCH 15/15] feat(provider): add API Route OpenAI-compatible provider --- cmd/doctor.go | 2 + cmd/gateway_providers.go | 19 +++++++ cmd/gateway_providers_api_route_test.go | 53 +++++++++++++++++++ cmd/onboard_managed.go | 1 + cmd/providers_cmd.go | 6 ++- cmd/setup_provider.go | 9 +++- docs/02-providers.md | 22 ++++++++ internal/config/config_channels.go | 4 ++ internal/config/config_load.go | 2 + internal/config/config_load_test.go | 27 ++++++++++ internal/config/config_secrets.go | 3 ++ internal/http/openapi_spec.json | 2 +- internal/http/provider_models.go | 2 + internal/http/providers.go | 5 ++ internal/store/provider_store.go | 6 +++ .../frontend/src/constants/providers.ts | 1 + ui/web/src/constants/providers.ts | 1 + .../config/sections/providers-section.tsx | 1 + 18 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 cmd/gateway_providers_api_route_test.go diff --git a/cmd/doctor.go b/cmd/doctor.go index 8edeaa1d..387ad759 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -98,10 +98,12 @@ func runDoctor() { checkDBProviders(db) checkProvider("Anthropic (env)", cfg.Providers.Anthropic.APIKey) checkProvider("OpenAI (env)", cfg.Providers.OpenAI.APIKey) + checkProvider("API Route (env)", cfg.Providers.APIRoute.APIKey) checkProvider("OpenRouter (env)", cfg.Providers.OpenRouter.APIKey) } else { checkProvider("Anthropic", cfg.Providers.Anthropic.APIKey) checkProvider("OpenAI", cfg.Providers.OpenAI.APIKey) + checkProvider("API Route", cfg.Providers.APIRoute.APIKey) checkProvider("OpenRouter", cfg.Providers.OpenRouter.APIKey) checkProvider("Gemini", cfg.Providers.Gemini.APIKey) checkProvider("Groq", cfg.Providers.Groq.APIKey) diff --git a/cmd/gateway_providers.go b/cmd/gateway_providers.go index 26a34099..3b3db769 100644 --- a/cmd/gateway_providers.go +++ b/cmd/gateway_providers.go @@ -43,6 +43,17 @@ func registerProviders(registry *providers.Registry, cfg *config.Config, modelRe slog.Info("registered provider", "name", "openai") } + if cfg.Providers.APIRoute.APIKey != "" { + base := cfg.Providers.APIRoute.APIBase + if base == "" { + base = store.APIRouteDefaultAPIBase + } + prov := providers.NewOpenAIProvider("api_route", cfg.Providers.APIRoute.APIKey, base, store.APIRouteDefaultModel) + prov.WithProviderType(store.ProviderAPIRoute) + registry.Register(prov) + slog.Info("registered provider", "name", "api_route") + } + if cfg.Providers.OpenRouter.APIKey != "" { orProv := providers.NewOpenAIProvider("openrouter", cfg.Providers.OpenRouter.APIKey, "https://openrouter.ai/api/v1", "anthropic/claude-sonnet-4-5-20250929") orProv.WithSiteInfo("https://goclaw.sh", "GoClaw") @@ -410,6 +421,14 @@ func registerProvidersFromDB(registry *providers.Registry, provStore store.Provi "User-Agent": store.KimiCodingRequiredUserAgent, }) registry.RegisterForTenant(p.TenantID, prov) + case store.ProviderAPIRoute: + base := p.APIBase + if base == "" { + base = store.APIRouteDefaultAPIBase + } + prov := providers.NewOpenAIProvider(p.Name, p.APIKey, base, store.APIRouteDefaultModel) + prov.WithProviderType(p.ProviderType) + registry.RegisterForTenant(p.TenantID, prov) default: prov := providers.NewOpenAIProvider(p.Name, p.APIKey, p.APIBase, "") prov.WithProviderType(p.ProviderType) diff --git a/cmd/gateway_providers_api_route_test.go b/cmd/gateway_providers_api_route_test.go new file mode 100644 index 00000000..6a163d64 --- /dev/null +++ b/cmd/gateway_providers_api_route_test.go @@ -0,0 +1,53 @@ +package cmd + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/nextlevelbuilder/goclaw/internal/config" + "github.com/nextlevelbuilder/goclaw/internal/providers" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +func TestRegisterProvidersAPIRouteDefaultsAndAuth(t *testing.T) { + var gotPath, gotAuth string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotAuth = r.Header.Get("Authorization") + _ = json.NewEncoder(w).Encode(map[string]any{ + "choices": []map[string]any{{ + "message": map[string]string{"content": "ok"}, + "finish_reason": "stop", + }}, + }) + })) + t.Cleanup(server.Close) + + cfg := &config.Config{} + cfg.Providers.APIRoute.APIKey = "api-route-key" + cfg.Providers.APIRoute.APIBase = server.URL + registry := providers.NewRegistry(nil) + registerProviders(registry, cfg, providers.NewInMemoryRegistry()) + + p, err := registry.GetForTenant(providers.MasterTenantID, "api_route") + if err != nil { + t.Fatalf("GetForTenant() error = %v", err) + } + if p.DefaultModel() != store.APIRouteDefaultModel { + t.Fatalf("DefaultModel() = %q, want %q", p.DefaultModel(), store.APIRouteDefaultModel) + } + if _, err := p.Chat(context.Background(), providers.ChatRequest{ + Messages: []providers.Message{{Role: "user", Content: "hi"}}, + }); err != nil { + t.Fatalf("Chat() error = %v", err) + } + if gotPath != "/chat/completions" { + t.Fatalf("request path = %q, want /chat/completions", gotPath) + } + if gotAuth != "Bearer api-route-key" { + t.Fatalf("Authorization = %q, want Bearer api-route-key", gotAuth) + } +} diff --git a/cmd/onboard_managed.go b/cmd/onboard_managed.go index 88ab8fd4..7a333dd6 100644 --- a/cmd/onboard_managed.go +++ b/cmd/onboard_managed.go @@ -27,6 +27,7 @@ func testPostgresConnection(dsn string) error { // defaultPlaceholderProviders defines disabled placeholder providers seeded for // UI discoverability. Users can later enable and configure them via the dashboard. var defaultPlaceholderProviders = []store.LLMProviderData{ + {Name: "api_route", DisplayName: "API Route", ProviderType: store.ProviderAPIRoute, APIBase: store.APIRouteDefaultAPIBase, Enabled: false}, {Name: "openrouter", DisplayName: "OpenRouter", ProviderType: store.ProviderOpenRouter, APIBase: "https://openrouter.ai/api/v1", Enabled: false}, {Name: "synthetic", DisplayName: "Synthetic", ProviderType: store.ProviderOpenAICompat, APIBase: "https://api.synthetic.new/openai/v1", Enabled: false}, {Name: "alicloud-api", DisplayName: "AliCloud API", ProviderType: store.ProviderDashScope, APIBase: "https://dashscope-intl.aliyuncs.com/compatible-mode/v1", Enabled: false}, diff --git a/cmd/providers_cmd.go b/cmd/providers_cmd.go index 4ad8581e..b7e4bb53 100644 --- a/cmd/providers_cmd.go +++ b/cmd/providers_cmd.go @@ -7,6 +7,7 @@ import ( "os" "text/tabwriter" + "github.com/nextlevelbuilder/goclaw/internal/store" "github.com/spf13/cobra" ) @@ -123,6 +124,7 @@ func runProvidersAdd() { typeOptions := []SelectOption[string]{ {"Anthropic", "anthropic"}, {"OpenAI", "openai"}, + {"API Route", "api_route"}, {"OpenRouter", "openrouter"}, {"DashScope (Alibaba)", "dashscope"}, {"OpenAI-compatible", "openai-compat"}, @@ -150,7 +152,7 @@ func runProvidersAdd() { // Step 4: Base URL (pre-fill per type, editable) defaultURL := defaultBaseURL(providerType) baseURL := "" - if providerType == "openai-compat" { + if providerType == "openai-compat" || providerType == "api_route" { baseURL, err = promptString("Base URL", "e.g. https://api.example.com/v1", defaultURL) if err != nil { fmt.Println("Cancelled.") @@ -310,6 +312,8 @@ func defaultBaseURL(providerType string) string { return "https://api.anthropic.com" case "openai": return "https://api.openai.com/v1" + case "api_route": + return store.APIRouteDefaultAPIBase case "openrouter": return "https://openrouter.ai/api/v1" case "dashscope": diff --git a/cmd/setup_provider.go b/cmd/setup_provider.go index 0e9ffbee..054efc50 100644 --- a/cmd/setup_provider.go +++ b/cmd/setup_provider.go @@ -47,6 +47,7 @@ func addProvider() { typeOptions := []SelectOption[string]{ {"Anthropic", "anthropic"}, {"OpenAI", "openai"}, + {"API Route", "api_route"}, {"OpenRouter", "openrouter"}, {"DashScope (Alibaba)", "dashscope"}, {"OpenAI-compatible", "openai-compat"}, @@ -68,8 +69,12 @@ func addProvider() { } baseURL := "" - if providerType == "openai-compat" { - baseURL, err = promptString("Base URL", "e.g. https://api.example.com/v1", "") + if providerType == "openai-compat" || providerType == "api_route" { + defaultURL := "" + if providerType == "api_route" { + defaultURL = "https://global.api-route.com/v1" + } + baseURL, err = promptString("Base URL", "e.g. https://api.example.com/v1", defaultURL) if err != nil { return } diff --git a/docs/02-providers.md b/docs/02-providers.md index 1a1885a9..dfad91fa 100644 --- a/docs/02-providers.md +++ b/docs/02-providers.md @@ -95,6 +95,7 @@ Supported price units: input, output, cache read, cache write, reasoning, reques | Provider | API Base | Default Model | Notes | |----------|----------|---------------|-------| | openai | `https://api.openai.com/v1` | `gpt-4o` | | +| api_route | `https://global.api-route.com/v1` | `gpt-5.4-mini` | API Route branded OpenAI-compatible endpoint | | openrouter | `https://openrouter.ai/api/v1` | `anthropic/claude-sonnet-4-5-20250929` | Model must contain `/` | | groq | `https://api.groq.com/openai/v1` | `llama-3.3-70b-versatile` | | | deepseek | `https://api.deepseek.com/v1` | `deepseek-chat` | | @@ -111,6 +112,27 @@ Supported price units: input, output, cache read, cache write, reasoning, reques | byteplus | `https://ark.ap-southeast.bytepluses.com/api/v3` | `seed-2-0-lite-260228` | Seed 2.0 models | | byteplus_coding | `https://ark.ap-southeast.bytepluses.com/api/coding/v3` | `seed-2-0-lite-260228` | Seed 2.0 Coding Plan | +### API Route setup + +Set `GOCLAW_API_ROUTE_API_KEY` in the environment, or add the key through the setup wizard. The base URL and model below are defaults and can be overridden: + +```json5 +{ + providers: { + api_route: { + api_key: "your-api-route-key", + api_base: "https://global.api-route.com/v1" + } + }, + agents: { + defaults: { + provider: "api_route", + model: "gpt-5.4-mini" + } + } +} +``` + --- ## 3. Call Flow diff --git a/internal/config/config_channels.go b/internal/config/config_channels.go index 52585c63..81f5856c 100644 --- a/internal/config/config_channels.go +++ b/internal/config/config_channels.go @@ -247,6 +247,7 @@ type FeishuConfig struct { type ProvidersConfig struct { Anthropic ProviderConfig `json:"anthropic"` OpenAI ProviderConfig `json:"openai"` + APIRoute ProviderConfig `json:"api_route"` // API Route (OpenAI-compatible endpoint) OpenRouter ProviderConfig `json:"openrouter"` Groq ProviderConfig `json:"groq"` Gemini ProviderConfig `json:"gemini"` @@ -319,6 +320,8 @@ func (p *ProvidersConfig) APIBaseForType(providerType string) string { return p.Anthropic.APIBase case "openai", "openai_compat": return p.OpenAI.APIBase + case "api_route": + return p.APIRoute.APIBase case "openrouter": return p.OpenRouter.APIBase case "groq": @@ -366,6 +369,7 @@ func (c *Config) HasAnyProvider() bool { p := c.Providers return p.Anthropic.APIKey != "" || p.OpenAI.APIKey != "" || + p.APIRoute.APIKey != "" || p.OpenRouter.APIKey != "" || p.Groq.APIKey != "" || p.Gemini.APIKey != "" || diff --git a/internal/config/config_load.go b/internal/config/config_load.go index e215bad3..e05e9cd2 100644 --- a/internal/config/config_load.go +++ b/internal/config/config_load.go @@ -156,6 +156,8 @@ func (c *Config) applyEnvOverrides() { envStr("GOCLAW_ANTHROPIC_BASE_URL", &c.Providers.Anthropic.APIBase) envStr("GOCLAW_OPENAI_API_KEY", &c.Providers.OpenAI.APIKey) envStr("GOCLAW_OPENAI_BASE_URL", &c.Providers.OpenAI.APIBase) + envStr("GOCLAW_API_ROUTE_API_KEY", &c.Providers.APIRoute.APIKey) + envStr("GOCLAW_API_ROUTE_BASE_URL", &c.Providers.APIRoute.APIBase) envStr("GOCLAW_OPENROUTER_API_KEY", &c.Providers.OpenRouter.APIKey) envStr("GOCLAW_GROQ_API_KEY", &c.Providers.Groq.APIKey) envStr("GOCLAW_DEEPSEEK_API_KEY", &c.Providers.DeepSeek.APIKey) diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index 912c04bc..5568d28b 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -296,6 +296,33 @@ func TestLoad_EnvVarAPIKeys(t *testing.T) { } } +func TestLoad_APIRouteProviderFromFileAndEnv(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.json5") + if err := os.WriteFile(cfgPath, []byte(`{ + "providers": { + "api_route": { + "api_key": "file-key", + "api_base": "https://example.com/v1" + } + } + }`), 0644); err != nil { + t.Fatalf("write config: %v", err) + } + + t.Setenv("GOCLAW_API_ROUTE_API_KEY", "env-key") + cfg, err := Load(cfgPath) + if err != nil { + t.Fatalf("load error: %v", err) + } + if cfg.Providers.APIRoute.APIKey != "env-key" { + t.Fatalf("API key = %q, want GOCLAW_API_ROUTE_API_KEY override", cfg.Providers.APIRoute.APIKey) + } + if cfg.Providers.APIRoute.APIBase != "https://example.com/v1" { + t.Fatalf("API base = %q, want file value", cfg.Providers.APIRoute.APIBase) + } +} + // --- Allowed origins from JSON5 --- func TestLoad_AllowedOrigins_JSON5(t *testing.T) { diff --git a/internal/config/config_secrets.go b/internal/config/config_secrets.go index 99e6a8a0..307ef23b 100644 --- a/internal/config/config_secrets.go +++ b/internal/config/config_secrets.go @@ -23,6 +23,7 @@ func (c *Config) MaskedCopy() *Config { // Mask provider API keys maskNonEmpty(&cp.Providers.Anthropic.APIKey) maskNonEmpty(&cp.Providers.OpenAI.APIKey) + maskNonEmpty(&cp.Providers.APIRoute.APIKey) maskNonEmpty(&cp.Providers.OpenRouter.APIKey) maskNonEmpty(&cp.Providers.Groq.APIKey) maskNonEmpty(&cp.Providers.DeepSeek.APIKey) @@ -71,6 +72,7 @@ func (c *Config) StripSecrets() { // Provider API keys c.Providers.Anthropic.APIKey = "" c.Providers.OpenAI.APIKey = "" + c.Providers.APIRoute.APIKey = "" c.Providers.OpenRouter.APIKey = "" c.Providers.Groq.APIKey = "" c.Providers.DeepSeek.APIKey = "" @@ -124,6 +126,7 @@ func (c *Config) StripMaskedSecrets() { // Provider API keys stripIfMasked(&c.Providers.Anthropic.APIKey) stripIfMasked(&c.Providers.OpenAI.APIKey) + stripIfMasked(&c.Providers.APIRoute.APIKey) stripIfMasked(&c.Providers.OpenRouter.APIKey) stripIfMasked(&c.Providers.Groq.APIKey) stripIfMasked(&c.Providers.DeepSeek.APIKey) diff --git a/internal/http/openapi_spec.json b/internal/http/openapi_spec.json index 784119e1..e315fd5c 100644 --- a/internal/http/openapi_spec.json +++ b/internal/http/openapi_spec.json @@ -1209,7 +1209,7 @@ "properties": { "name": { "type": "string", "description": "Unique provider slug" }, "display_name": { "type": "string" }, - "provider_type": { "type": "string", "enum": ["anthropic_native", "openai_compat", "gemini_native", "openrouter", "groq", "deepseek", "mistral", "xai", "minimax_native", "cohere", "perplexity", "dashscope", "bailian", "chatgpt_oauth", "claude_cli", "yescale", "zai", "zai_coding", "ollama", "ollama_cloud", "acp"] }, + "provider_type": { "type": "string", "enum": ["api_route", "anthropic_native", "openai_compat", "gemini_native", "openrouter", "groq", "deepseek", "mistral", "xai", "minimax_native", "cohere", "perplexity", "dashscope", "bailian", "chatgpt_oauth", "claude_cli", "yescale", "zai", "zai_coding", "ollama", "ollama_cloud", "acp"] }, "api_base": { "type": "string" }, "api_key": { "type": "string" }, "enabled": { "type": "boolean", "default": true }, diff --git a/internal/http/provider_models.go b/internal/http/provider_models.go index 2ab10072..85a0adf6 100644 --- a/internal/http/provider_models.go +++ b/internal/http/provider_models.go @@ -129,6 +129,8 @@ func openAIModelsAPIBase(providerType, apiBase string) string { return base } switch providerType { + case store.ProviderAPIRoute: + return store.APIRouteDefaultAPIBase case store.ProviderKimiCoding: return store.KimiCodingDefaultAPIBase default: diff --git a/internal/http/providers.go b/internal/http/providers.go index f4a76b17..f7772a5c 100644 --- a/internal/http/providers.go +++ b/internal/http/providers.go @@ -284,6 +284,11 @@ func (h *ProvidersHandler) registerInMemory(p *store.LLMProviderData) providerRu } apiBase := h.resolveAPIBase(p) switch p.ProviderType { + case store.ProviderAPIRoute: + if apiBase == "" { + apiBase = store.APIRouteDefaultAPIBase + } + h.providerReg.RegisterForTenant(p.TenantID, providers.NewOpenAIProvider(p.Name, p.APIKey, apiBase, store.APIRouteDefaultModel).WithProviderType(p.ProviderType)) case store.ProviderChatGPTOAuth: ts := oauth.NewDBTokenSource(h.store, h.secretStore, p.Name).WithTenantID(p.TenantID) codex := providers.NewCodexProvider(p.Name, ts, apiBase, "") diff --git a/internal/store/provider_store.go b/internal/store/provider_store.go index 1a0d08a5..9e81824b 100644 --- a/internal/store/provider_store.go +++ b/internal/store/provider_store.go @@ -35,6 +35,7 @@ const ( ProviderBytePlusCoding = "byteplus_coding" // BytePlus ModelArk Coding Plan ProviderVertex = "vertex" // Google Cloud Vertex AI (OAuth2 service account + ADC) ProviderKimiCoding = "kimi_coding" // Moonshot Kimi Coding (OpenAI-compat, requires fixed User-Agent) + ProviderAPIRoute = "api_route" // API Route (OpenAI-compatible endpoint) // Novita AI defaults. NovitaDefaultAPIBase = "https://api.novita.ai/openai" @@ -51,6 +52,10 @@ const ( KimiCodingDefaultAPIBase = "https://api.kimi.com/coding/v1" KimiCodingDefaultModel = "kimi-k2-turbo-preview" KimiCodingRequiredUserAgent = "claude-code/0.1.0" + + // API Route defaults. + APIRouteDefaultAPIBase = "https://global.api-route.com/v1" + APIRouteDefaultModel = "gpt-5.4-mini" ) // Vertex AI constants live in internal/providers/vertex.go to avoid a store→providers import cycle @@ -85,6 +90,7 @@ var ValidProviderTypes = map[string]bool{ ProviderBytePlusCoding: true, ProviderVertex: true, ProviderKimiCoding: true, + ProviderAPIRoute: true, } // VertexProviderSettings holds Vertex-specific config stored in llm_providers.settings JSONB. diff --git a/ui/desktop/frontend/src/constants/providers.ts b/ui/desktop/frontend/src/constants/providers.ts index 8cc17084..474bdc1a 100644 --- a/ui/desktop/frontend/src/constants/providers.ts +++ b/ui/desktop/frontend/src/constants/providers.ts @@ -6,6 +6,7 @@ export interface ProviderTypeInfo { } export const PROVIDER_TYPES: ProviderTypeInfo[] = [ + { value: 'api_route', label: 'API Route', apiBase: 'https://global.api-route.com/v1', needsKey: true }, { value: 'anthropic_native', label: 'Anthropic (Native)', apiBase: '', needsKey: true }, { value: 'openai_compat', label: 'OpenAI Compatible', apiBase: '', needsKey: true }, { value: 'gemini_native', label: 'Google Gemini', apiBase: 'https://generativelanguage.googleapis.com/v1beta/openai', needsKey: true }, diff --git a/ui/web/src/constants/providers.ts b/ui/web/src/constants/providers.ts index d82aba46..9d85c7e6 100644 --- a/ui/web/src/constants/providers.ts +++ b/ui/web/src/constants/providers.ts @@ -12,6 +12,7 @@ type ProviderAliasSource = string | { name?: string | null }; export const DEFAULT_CODEX_OAUTH_ALIAS = "openai-codex"; export const PROVIDER_TYPES: ProviderTypeInfo[] = [ + { value: "api_route", label: "API Route", apiBase: "https://global.api-route.com/v1", placeholder: "" }, { value: "chatgpt_oauth", label: "ChatGPT Subscription (OAuth)", apiBase: "", placeholder: "" }, { value: "anthropic_native", label: "Anthropic (Native)", apiBase: "", placeholder: "https://api.anthropic.com" }, { value: "openai_compat", label: "OpenAI Compatible", apiBase: "", placeholder: "https://api.openai.com/v1" }, diff --git a/ui/web/src/pages/config/sections/providers-section.tsx b/ui/web/src/pages/config/sections/providers-section.tsx index 372ccc43..1e9fc07b 100644 --- a/ui/web/src/pages/config/sections/providers-section.tsx +++ b/ui/web/src/pages/config/sections/providers-section.tsx @@ -17,6 +17,7 @@ type ProvidersData = Record; const KNOWN_PROVIDERS = [ { key: "anthropic", label: "Anthropic", envKey: "GOCLAW_ANTHROPIC_API_KEY" }, { key: "openai", label: "OpenAI", envKey: "GOCLAW_OPENAI_API_KEY" }, + { key: "api_route", label: "API Route", envKey: "GOCLAW_API_ROUTE_API_KEY" }, { key: "openrouter", label: "OpenRouter", envKey: "GOCLAW_OPENROUTER_API_KEY" }, { key: "groq", label: "Groq", envKey: "GOCLAW_GROQ_API_KEY" }, { key: "gemini", label: "Gemini", envKey: "GOCLAW_GEMINI_API_KEY" },