From 0e1ad9e9d14630a0e3746253f8507ba3eec27165 Mon Sep 17 00:00:00 2001 From: claudi47 Date: Sun, 9 Aug 2026 11:09:09 +0200 Subject: [PATCH 1/2] feat(openai): stabilize ChatGPT subscription and add dynamic model discovery --- AGENTS.md | 2 +- README.md | 16 +- ROADMAP.md | 28 ++- docs/OVERVIEW.md | 72 +++++-- internal/cli/cli.go | 2 +- internal/cli/cli_test.go | 78 +++++++ internal/cli/effort.go | 102 ++++++++++ internal/cli/model.go | 113 ++++++++--- internal/cli/run.go | 38 +++- internal/config/config_test.go | 41 ++++ internal/config/openai_oauth.go | 44 +++- internal/provider/models.go | 2 +- internal/provider/openai.go | 254 +++++++++++++++-------- internal/provider/openai_models.go | 259 ++++++++++++++++++++++++ internal/provider/openai_models_test.go | 168 +++++++++++++++ internal/provider/provider.go | 7 +- internal/provider/provider_test.go | 19 ++ internal/provider/stream_test.go | 121 +++++++++++ 18 files changed, 1216 insertions(+), 150 deletions(-) create mode 100644 internal/cli/effort.go create mode 100644 internal/provider/openai_models.go create mode 100644 internal/provider/openai_models_test.go diff --git a/AGENTS.md b/AGENTS.md index 0507405..b062d76 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,7 @@ OVERVIEW for the design. ## Command surface (deliberately tiny) `scay` opens the interactive session; **everything else lives inside it** as -slash commands (`/auth`, `/model`, `/mcp`, `/compact`, `/clear`, `/exit`). +slash commands (`/auth`, `/model`, `/effort`, `/mcp`, `/compact`, `/clear`, `/exit`). The only subcommands are `scay yolo` (session with the approval gate bypassed) and `scay update` (self-update from GitHub releases), plus `version`/`help`. Don't add new top-level subcommands — extend the slash commands instead. diff --git a/README.md b/README.md index 012ccab..599b721 100644 --- a/README.md +++ b/README.md @@ -42,9 +42,12 @@ First time, scay walks you through setup right there: 1. **`/auth`** — pick a provider and authenticate. OpenAI accepts an API key or an experimental ChatGPT subscription login; Ollama needs neither. Credentials are stored with `0600` permissions under `~/.config/scay`. -2. **`/model`** — pick the provider, model, and thinking effort. With Ollama, - the menu lists whatever models you actually have installed. -3. **Type what you want.** Anything that isn't a `/command` goes to the model. +2. **`/model`** — pick the provider and model. OpenAI lists + the models available to the current account; Ollama lists whatever is + installed locally. Both fall back to built-in suggestions when offline. +3. **`/effort`** — optionally choose the thinking effort. Use `/effort high` + to set one directly. +4. **Type what you want.** Anything that isn't a `/command` goes to the model.

First run: scay guides you through /model, listing installed Ollama models @@ -67,7 +70,8 @@ Slash commands, inside the session: | Command | What it does | |---|---| -| `/model` | Pick provider, model, and thinking effort (applies live) | +| `/model` | Pick provider and model (applies live) | +| `/effort [level]` | Pick thinking effort, or set `off`, `low`, `medium`, `high`, or `max` directly | | `/auth [provider]` | Authenticate; OpenAI offers API key or experimental ChatGPT login | | `/mcp` | Manage MCP servers — `add`, `list`, `remove`, `get`, `tools` | | `/compact` | Squash a long history into a summary, freeing context | @@ -101,6 +105,10 @@ without touching your saved setup. ChatGPT login relies on the Codex protocol and is explicitly experimental. Responses stream on all four backends, and transient API failures retry automatically with backoff. +For protocol troubleshooting, set `SCAY_OPENAI_DEBUG=1`. The opt-in diagnostic +output includes HTTP status, request ID, model, refresh attempts, and SSE event +types, but never credentials, prompts, or encrypted reasoning content. + ## MCP servers (optional) scay is an [MCP](https://modelcontextprotocol.io) client: plug in external diff --git a/ROADMAP.md b/ROADMAP.md index 57cb0ad..68bd917 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,6 +1,14 @@ # scay roadmap -> **Current state: v0.2.0 pre-release audit complete (2026-07-29).** A full review +> **Current state: OpenAI Subscription stabilization + dynamic model discovery +> implemented and mock-tested (2026-08-09).** The Responses adapter now has +> secret-free opt-in diagnostics, classified errors, complete partial-stream/item +> replay, hardened refresh recovery, account-aware `/model` discovery, a GPT-5.6 +> fallback catalog, and capability-aware reasoning. `go test ./...`, vet, and build +> are clean. **Still required before calling the milestone fully live-validated:** +> five consecutive real Subscription sessions, real refresh-token rotation and +> restart, `/compact` on both auth modes, and the versionless `client_version` +> catalog spike. Earlier release history follows. v0.2.0's full review > pass (three parallel reviewers over provider/cli+config/agent+tools+mcp) found and > fixed ~20 defects before going public — see M9 below for the list. gofmt/vet/ > build/test clean, windows cross-build clean, release.sh dry run clean; live @@ -12,10 +20,8 @@ > banner is now the four-row block-art octopus from the website > (`internal/cli/mascot.go`, tested) with version, provider · model, and effort > beside it — red on a failed startup build; docs screenshots updated to match. -> **Next:** live -> checks vs Anthropic/OpenAI/Gemini when real keys are available — including the -> M8 items and the new Responses fixes (incomplete/refusal handling, -> `strict:false`, Codex `include: reasoning.encrypted_content`). +> **Next:** run the credential-backed M10 matrix, then live checks vs +> Anthropic/Gemini when real keys are available. > > Update this header every session so work resumes instantly. @@ -89,9 +95,19 @@ Legend: `[x]` done · `[~]` in progress · `[ ]` todo - [x] **CLI/auth hardening:** Ctrl-C during hidden key entry restores terminal echo before exiting; the OAuth callback validates `state` in the handler so a stray localhost request can't dead-end the login; `scay update` verifies the release's published sha256 checksums before swapping the binary; browser-launcher zombie reaped; plaintext-credential warning when `/mcp add --project` stores headers. - [x] Known accepted limitation: Ctrl-C during a y/N approval prompt takes effect only after Enter (the blocking stdin read is shared with the REPL); the default answer is deny, so nothing runs. +## M10 — OpenAI Subscription stabilization + model discovery (2026-08-09) +- [x] **Safe diagnostics and errors:** `SCAY_OPENAI_DEBUG=1` reports status, request ID, model, refresh state, and SSE event types without tokens, prompts, tool arguments, or encrypted reasoning. HTTP/model/auth/rate-limit/protocol/stream failures are distinguishable; cross-origin request redirects and OAuth token redirects are blocked. +- [x] **OAuth hardening:** proactive expiry and forced-401 refresh share one serialized path; missing refresh state and refresh failures direct the user to `/auth openai`; rotated access/refresh/account/expiry state is atomically persisted and reload-tested. The documented cross-process lock limitation remains. +- [x] **Streaming/replay:** completed message, reasoning, and multiple function-call items are merged across `response.output_item.done` and empty terminal output; partial text, `response.incomplete`, refusal deltas/items, malformed SSE, and premature stream close are regression-tested. +- [x] **Dynamic `/model`:** Platform uses `GET /v1/models` with conservative GPT/Responses filtering; Subscription uses `GET /backend-api/codex/models` with account header, picker visibility, priority, subscription-only model support, and capability parsing. Both use a three-second bound, one OAuth refresh after 401, validate remote identifiers before display/use, preserve the configured model, retain free-form input, and fall back cleanly. +- [x] **GPT-5.6 + capability-aware effort:** fallback includes `gpt-5.6`, Sol, Terra, Luna, 5.5, and 5.2. Subscription catalog efforts drive request mapping; GPT-5.6 statically maps `off`→`none` and `max`→`max`, while earlier Platform fallback remains `minimal`→`xhigh`. +- [x] **Separate model/effort UX:** `/model` now changes only provider/model; `/effort` shows the neutral ladder with the effective OpenAI mapping, and `/effort high` applies a level directly. Both reload live without clearing same-provider history. +- [x] **No runtime expansion:** standard library only, no app-server process, no persistent model cache, and no fabricated Codex `client_version`. +- [ ] **Live acceptance (credentials required):** five consecutive Subscription text/tool sessions; preventive expiry plus forced 401 and persisted token rotation across restart; `/compact`→turn on Platform and Subscription; incomplete/interrupted recovery; effort `none`/`max` on visible GPT-5.6 models; versionless Subscription catalog request and fallback behavior. + ## M7 — Debloat: one binary, everything in-session (2026-07-04) - [x] Removed `cmd/scay-tui` (the nested Charm Go module) and all references (CI, release script, gitignore, docs); the block-art octopus moved into the CLI banner -- [x] Collapsed the command surface: `auth`/`model`/`mcp` subcommands and `-p` one-shot removed; everything runs inside the interactive session as `/auth`, `/model`, `/mcp`, `/compact`, `/clear`, `/help`, `/exit` +- [x] Collapsed the command surface: `auth`/`model`/`mcp` subcommands and `-p` one-shot removed; everything runs inside the interactive session as `/auth`, `/model`, `/effort`, `/mcp`, `/compact`, `/clear`, `/help`, `/exit` - [x] Session works unconfigured (guides through /auth → /model) and rebuilds the agent live on config changes (MCP sessions closed + reconnected; history survives model/effort changes, cleared on provider switch since reasoning blobs are provider-specific) - [x] `scay yolo`: the session with the approval gate bypassed (replaces `-p --yes`) - [x] `scay update`: stdlib self-update — latest GitHub release tag → download `scay__.tar.gz` → atomic rename over the running executable (darwin/linux) diff --git a/docs/OVERVIEW.md b/docs/OVERVIEW.md index 3a73d98..3e60cb6 100644 --- a/docs/OVERVIEW.md +++ b/docs/OVERVIEW.md @@ -25,8 +25,10 @@ Every dependency and abstraction must earn its weight. Concretely: API key or an experimental browser OAuth flow compatible with Codex. Credentials sit behind a renewable `Credential` interface; the ChatGPT route is isolated because OpenAI does not yet publish third-party OAuth client registration. -3. **Model selection → curated list + free-form.** A short built-in list of current - models per provider, plus "type any model id". Effort from a unified ladder. +3. **Model selection → account-aware + fallback + free-form.** OpenAI and Ollama + discover usable models with a three-second bound; a short built-in list covers + offline/error cases, and users can always type any model id. Other providers + use curated suggestions. Effort stays on a unified ladder. 4. **Key storage → `0600` file + env.** Stored under `os.UserConfigDir()` (`~/.config/scay/`) with `0600` perms; env vars override the file. @@ -100,9 +102,11 @@ replays next request — is identical on both paths. That matters most for: `input_json_delta` fragments concatenate into tool-call args; `redacted_thinking` arrives whole in `content_block_start` and is kept raw. - **OpenAI** (`stream: true`, Responses SSE): `response.output_text.delta` and - reasoning deltas render immediately; `response.completed` carries the complete - output-item list and usage. Function calls and reasoning items are replayed - verbatim on the next request. + reasoning deltas render immediately. Completed message, reasoning, and function + items are retained from `response.output_item.done` and merged with the terminal + `response.completed` or `response.incomplete` payload. This covers Codex streams + whose terminal `output` is empty, partial text, refusals, and multiple tool calls; + opaque items are replayed verbatim on the next request. - **Gemini** (`:streamGenerateContent?alt=sse`): each frame is a chunk of parts. Parts carrying a `thoughtSignature` (and all thought/functionCall parts) are kept raw for replay; unsigned plain-text fragments are merged into one block. @@ -121,7 +125,31 @@ Ctrl-C aborts a turn. | `low` | `low` | effort `low` | `low` | `"low"` | | `medium` | `medium` | effort `medium`| `medium`/dynamic | `"medium"` | | `high` | `high` | effort `high` | `high` | `"high"` | -| `max` | `xhigh` | effort `max` | `high` + budget -1 | `"max"` | +| `max` | `xhigh`/`max` | effort `max` | `high` + budget -1 | `"max"` | + +For OpenAI, the adapter maps the neutral endpoints to the actual model +capabilities when the Codex catalog provides them: `off` chooses `none` or +`minimal`, while `max` chooses `max`, `xhigh`, or the highest advertised level. +GPT-5.6 has a static `none`…`max` fallback; earlier Platform models retain the +`minimal`…`xhigh` fallback because `/v1/models` does not expose capabilities. + +### OpenAI model discovery + +`/model` selects only provider/model and calls `GET /v1/models` for Platform +credentials and `GET /backend-api/codex/models` for ChatGPT credentials, using +the same bearer, account header, and one-refresh-after-401 path as Responses. +Platform results are conservatively filtered to GPT text models suitable for +Responses; Codex results respect picker `visibility`, `priority`, and advertised +reasoning levels. Remote model IDs and effort values are validated before display +or use so catalog data cannot inject terminal controls or arbitrary effort values. +`supported_in_api` is intentionally not used to hide subscription-only models; +the official client applies that flag only outside ChatGPT auth. The configured +model is always retained even if temporarily absent. + +Discovery has no persistent cache and falls back to the curated GPT-5.6/5.5/5.2 +list on timeout, auth failure, empty data, or malformed JSON. scay deliberately +omits the Codex-specific `client_version` rather than claiming to be an official +Codex release; compatibility of the versionless request remains a live-test item. ## The agent loop @@ -150,6 +178,15 @@ attempt's response is handed back untouched so each adapter keeps its own error formatting. Streams retry only up to the first byte; once output is flowing, a drop surfaces as an error rather than a silent re-request. +OpenAI errors add stable categories (`authentication`, `model_unavailable`, +`rate_limit`, `protocol`, or `stream_interrupted`) and a request ID when one is +returned. Setting `SCAY_OPENAI_DEBUG=1` adds secret-free protocol diagnostics: +status, request ID, model, refresh state, and SSE event type only. Tokens, +authorization codes, prompts, tool arguments, and encrypted reasoning are never +logged. OpenAI requests follow redirects only within the original origin, while +OAuth token POSTs do not follow redirects, preventing sensitive bodies or account +headers from being replayed to another host. + Related hardening: the Anthropic adapter merges adjacent same-role wire messages, because a legal scay history can produce them (tool results followed by fresh user input after an interrupted turn, or a compaction summary) while @@ -175,9 +212,10 @@ the loop by holding the output pipe. file. OpenAI supports `api_key` and experimental `chatgpt` credentials; the latter stores access token, rotating refresh token, account ID, and expiry in that protected file. `Credential` resolves a current token at request time, refreshes once after a -401, and persists the rotated token atomically. The browser login uses Authorization -Code + PKCE on a localhost callback; it has no third-party client-registration -guarantee from OpenAI and is deliberately marked experimental. +401, and persists the rotated token atomically. Missing/revoked/incomplete refresh +state reports a direct `/auth openai` recovery path. The browser login uses +Authorization Code + PKCE on a localhost callback; it has no third-party +client-registration guarantee from OpenAI and is deliberately marked experimental. ## MCP @@ -208,14 +246,14 @@ servers are actually configured. ## The interactive session `scay` drops straight into the session; there is nothing else to launch or -compile. The session works even before any model is configured — `/auth` and -`/model` run inside it and the agent is rebuilt live (MCP sessions closed and -reconnected) when they change the config. History survives a model or effort -change but is cleared on a provider switch, because replayed reasoning blocks -are provider-specific opaque blobs another backend would reject. - -Slash commands: `/help`, `/auth`, `/model`, `/mcp …`, `/compact`, `/clear`, -`/exit`. Lines are tokenized with quote support so things like +compile. The session works even before any model is configured — `/auth`, +`/model`, and `/effort` run inside it and the agent is rebuilt live (MCP sessions +closed and reconnected) when they change the config. History survives a model or +effort change but is cleared on a provider switch, because replayed reasoning +blocks are provider-specific opaque blobs another backend would reject. + +Slash commands: `/help`, `/auth`, `/model`, `/effort [level]`, `/mcp …`, +`/compact`, `/clear`, `/exit`. Lines are tokenized with quote support so things like `/mcp add --header 'Authorization: Bearer x' srv https://…` work. Rendering is deliberately plain: assistant text streams verbatim, tool calls print as dim `· tool args` lines (a red `✗` on failure), reasoning pauses show a single dim diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 7c318f1..ed2322e 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -62,7 +62,7 @@ Usage: scay version Print the version Everything else happens inside the session — type /help there: - /model /auth /mcp /compact /clear /exit + /model /effort /auth /mcp /compact /clear /exit API keys and the experimental OpenAI ChatGPT login are stored via /auth (0600 under your OS config dir, e.g. ~/.config/scay). Environment variables diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 7a508be..e8b29bb 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -1,9 +1,16 @@ package cli import ( + "bufio" + "io" + "net/http" + "net/http/httptest" "slices" "strings" "testing" + + "github.com/robitec97/scay/internal/config" + "github.com/robitec97/scay/internal/provider" ) func TestBanner(t *testing.T) { @@ -64,3 +71,74 @@ func TestResolveModel(t *testing.T) { } } } + +func TestEnsureModelPreservesCurrent(t *testing.T) { + if got := ensureModel([]string{"new-a", "new-b"}, "configured"); !slices.Equal(got, []string{"configured", "new-a", "new-b"}) { + t.Fatalf("models = %v", got) + } +} + +func TestSafeDisplayRemovesTerminalControls(t *testing.T) { + got := safeDisplay("gpt-safe\x1b[31m\nforged\u202e") + if strings.ContainsAny(got, "\x1b\n\r") || strings.ContainsRune(got, '\u202e') { + t.Fatalf("unsafe display value: %q", got) + } +} + +func TestProviderModelsUsesOpenAIDiscoveryAndFallback(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + io.WriteString(w, `{"data":[{"id":"gpt-new"}]}`) + })) + cfg := &config.File{Provider: "openai", Model: "configured", Providers: map[string]config.ProviderConfig{ + "openai": {APIKey: "key", Host: srv.URL}, + }} + if got := providerModels(cfg, "openai"); !slices.Equal(got, []string{"configured", "gpt-new"}) { + t.Fatalf("discovered models = %v", got) + } + srv.Close() + got := providerModels(cfg, "openai") + if got[0] != "configured" || !slices.Contains(got, "gpt-5.6") { + t.Fatalf("fallback models = %v", got) + } +} + +func TestResolveEffort(t *testing.T) { + efforts := provider.AllEfforts() + for input, want := range map[string]provider.Effort{"1": provider.EffortOff, "3": provider.EffortMedium, "high": provider.EffortHigh} { + got, ok := resolveEffort(input, efforts) + if !ok || got != want { + t.Errorf("resolveEffort(%q) = %q, %t; want %q", input, got, ok, want) + } + } + if _, ok := resolveEffort("ultra", efforts); ok { + t.Error("unknown effort accepted") + } +} + +func TestModelAndEffortCommandsAreSeparate(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv("AppData", tmp) + + if err := modelCommand(bufio.NewReader(strings.NewReader("1\n1\n"))); err != nil { + t.Fatal(err) + } + cfg, err := config.Load() + if err != nil { + t.Fatal(err) + } + if cfg.Provider != "anthropic" || cfg.Model != "claude-opus-5" || cfg.Effort != "medium" { + t.Fatalf("model command saved %+v", cfg) + } + if err := effortCommand(bufio.NewReader(strings.NewReader("")), []string{"high"}); err != nil { + t.Fatal(err) + } + cfg, err = config.Load() + if err != nil { + t.Fatal(err) + } + if cfg.Provider != "anthropic" || cfg.Model != "claude-opus-5" || cfg.Effort != "high" { + t.Fatalf("effort command changed the wrong settings: %+v", cfg) + } +} diff --git a/internal/cli/effort.go b/internal/cli/effort.go new file mode 100644 index 0000000..b0fbc00 --- /dev/null +++ b/internal/cli/effort.go @@ -0,0 +1,102 @@ +package cli + +import ( + "bufio" + "context" + "fmt" + "strconv" + "time" + + "github.com/robitec97/scay/internal/config" + "github.com/robitec97/scay/internal/provider" +) + +// effortCommand handles /effort and /effort . The saved value remains +// provider-neutral; OpenAI's native value is shown in the menu when it differs. +func effortCommand(in *bufio.Reader, args []string) error { + cfg, err := config.Load() + if err != nil { + return err + } + if cfg.Provider == "" || cfg.Model == "" { + return fmt.Errorf("no model selected — use /model first") + } + if len(args) > 1 { + return fmt.Errorf("usage: /effort [off|low|medium|high|max]") + } + + efforts := provider.AllEfforts() + if len(args) == 1 { + effort, ok := resolveEffort(args[0], efforts) + if !ok { + return fmt.Errorf("invalid effort %q — choose off, low, medium, high, or max", args[0]) + } + cfg.Effort = string(effort) + return config.Save(cfg) + } + + current := provider.EffortMedium + if provider.ValidEffort(cfg.Effort) { + current = provider.Effort(cfg.Effort) + } + openAIEfforts := openAIModelEfforts(cfg) + fmt.Printf("Current effort: %s — Enter keeps it.\n", current) + fmt.Println("Select thinking effort, or type a level:") + for i, effort := range efforts { + marker := " " + if effort == current { + marker = "*" + } + label := string(effort) + if cfg.Provider == "openai" { + native := provider.OpenAIEffort(cfg.Model, effort, openAIEfforts) + if native != label { + label += " (OpenAI: " + native + ")" + } + } + fmt.Printf(" %s %d) %s\n", marker, i+1, label) + } + fmt.Print("> ") + sel := readLine(in) + if sel == "" { + if !provider.ValidEffort(cfg.Effort) { + cfg.Effort = string(current) + return config.Save(cfg) + } + return nil + } + effort, ok := resolveEffort(sel, efforts) + if !ok { + return fmt.Errorf("invalid effort %q", sel) + } + cfg.Effort = string(effort) + return config.Save(cfg) +} + +func openAIModelEfforts(cfg *config.File) []string { + if cfg.Provider != "openai" || cfg.Resolve("openai").Cred.Kind() != "chatgpt" { + return nil + } + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + models, err := discoverOpenAIModels(ctx, cfg) + if err != nil { + return nil + } + for _, model := range models { + if model.ID == cfg.Model { + return model.ReasoningEfforts + } + } + return nil +} + +func resolveEffort(sel string, efforts []provider.Effort) (provider.Effort, bool) { + if n, err := strconv.Atoi(sel); err == nil && n >= 1 && n <= len(efforts) { + return efforts[n-1], true + } + if provider.ValidEffort(sel) { + return provider.Effort(sel), true + } + return "", false +} diff --git a/internal/cli/model.go b/internal/cli/model.go index 29716d0..99d9162 100644 --- a/internal/cli/model.go +++ b/internal/cli/model.go @@ -6,24 +6,25 @@ import ( "fmt" "os" "strconv" + "strings" "time" + "unicode" "github.com/robitec97/scay/internal/config" "github.com/robitec97/scay/internal/provider" ) -// modelCommand handles /model: select the active provider, model, and thinking -// effort using simple numbered prompts (curated list + free-form model id). At -// each step Enter keeps the current setting, so re-runs only change what you -// type. +// modelCommand handles /model: select only the active provider and model using +// simple numbered prompts (curated/discovered list + free-form model id). +// Thinking effort is intentionally separate under /effort. func modelCommand(in *bufio.Reader) error { cfg, err := config.Load() if err != nil { return err } if cfg.Provider != "" && cfg.Model != "" { - fmt.Printf("Current: %s / %s (effort %s) — Enter keeps a setting.\n", - cfg.Provider, cfg.Model, cfg.Effort) + fmt.Printf("Current model: %s / %s — Enter keeps a setting.\n", + cfg.Provider, safeDisplay(cfg.Model)) } // 1) provider (current one marked with *; Enter keeps it) @@ -57,7 +58,7 @@ func modelCommand(in *bufio.Reader) error { if pid == cfg.Provider && m == cfg.Model { marker = "*" } - fmt.Printf(" %s %d) %s\n", marker, i+1, m) + fmt.Printf(" %s %d) %s\n", marker, i+1, safeDisplay(m)) } fmt.Print("> ") model := resolveModel(readLine(in), models) @@ -68,27 +69,11 @@ func modelCommand(in *bufio.Reader) error { return fmt.Errorf("no model selected") } - // 3) thinking effort (Enter keeps the current level, else medium) - fmt.Println("Select thinking effort:") - efforts := provider.AllEfforts() - for i, e := range efforts { - marker := " " - if string(e) == cfg.Effort { - marker = "*" - } - fmt.Printf(" %s %d) %s\n", marker, i+1, e) - } - effort := provider.EffortMedium - if provider.ValidEffort(cfg.Effort) { - effort = provider.Effort(cfg.Effort) - } - if eIdx := readIndex(in, "> ", len(efforts)); eIdx >= 0 { - effort = efforts[eIdx] - } - cfg.Provider = pid cfg.Model = model - cfg.Effort = string(effort) + if !provider.ValidEffort(cfg.Effort) { + cfg.Effort = string(provider.EffortMedium) + } return config.Save(cfg) } @@ -97,6 +82,20 @@ func modelCommand(in *bufio.Reader) error { // that would fail on first use; anything else (and an unreachable server) // gets the curated list. func providerModels(cfg *config.File, pid string) []string { + if pid == "openai" { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + models, err := discoverOpenAIModels(ctx, cfg) + if err != nil || len(models) == 0 { + fmt.Fprintln(os.Stderr, "scay: OpenAI models unavailable, showing suggestions") + return ensureModel(provider.CuratedModels[pid], currentModel(cfg, pid)) + } + ids := make([]string, 0, len(models)) + for _, m := range models { + ids = append(ids, m.ID) + } + return ensureModel(ids, currentModel(cfg, pid)) + } if pid != "ollama" { return provider.CuratedModels[pid] } @@ -110,6 +109,68 @@ func providerModels(cfg *config.File, pid string) []string { return models } +func discoverOpenAIModels(ctx context.Context, cfg *config.File) ([]provider.OpenAIModel, error) { + res := cfg.Resolve("openai") + token, err := res.Cred.Token(ctx, false) + if err != nil { + return nil, err + } + opts := provider.Options{APIKey: token, BaseURL: res.Host} + if openAIDiagnosticsEnabled() { + opts.Diagnostic = os.Stderr + } + if res.Cred.Kind() == "chatgpt" { + opts.ChatGPT = true + opts.OpenAIAuth = func(ctx context.Context, retry bool) (provider.OpenAIAuth, error) { + token, err := res.Cred.Token(ctx, retry) + if err != nil { + return provider.OpenAIAuth{}, err + } + return provider.OpenAIAuth{Token: token, AccountID: res.Cred.AccountID()}, nil + } + } + return provider.OpenAIModels(ctx, opts) +} + +func ensureModel(models []string, current string) []string { + if current == "" { + return append([]string(nil), models...) + } + for _, model := range models { + if model == current { + return append([]string(nil), models...) + } + } + return append([]string{current}, models...) +} + +func currentModel(cfg *config.File, pid string) string { + if cfg.Provider == pid { + return cfg.Model + } + return "" +} + +func openAIDiagnosticsEnabled() bool { + switch os.Getenv("SCAY_OPENAI_DEBUG") { + case "1", "true", "TRUE", "yes", "YES": + return true + default: + return false + } +} + +// safeDisplay keeps model identifiers from injecting terminal control +// sequences when they came from a remote catalog or an edited config file. +func safeDisplay(s string) string { + return strings.Map(func(r rune) rune { + if unicode.IsPrint(r) { + return r + } + return '�' + }, s) +} + // resolveModel interprets sel as a 1-based list index or a literal model id. func resolveModel(sel string, models []string) string { if sel == "" { diff --git a/internal/cli/run.go b/internal/cli/run.go index 3e9aab3..b367c4c 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strings" + "time" "github.com/robitec97/scay/internal/agent" "github.com/robitec97/scay/internal/config" @@ -48,7 +49,7 @@ func runREPL(yolo bool) int { if r.identity == "openai:chatgpt" { name += " / ChatGPT experimental" } - meta = append(meta, name+" · "+r.ag.Model, fmt.Sprintf("effort %s", r.ag.Effort)) + meta = append(meta, name+" · "+safeDisplay(r.ag.Model), fmt.Sprintf("effort %s", r.ag.Effort)) case !errors.Is(err, errNotConfigured): state = MascotFailed } @@ -61,7 +62,8 @@ func runREPL(yolo bool) int { case errors.Is(err, errNotConfigured): fmt.Println(" No model configured yet. Get started:") fmt.Println(" /auth log in to a provider (skip for Ollama)") - fmt.Println(" /model pick provider, model, and thinking effort") + fmt.Println(" /model pick provider and model") + fmt.Println(" /effort pick thinking effort") case err != nil: fmt.Fprintln(os.Stderr, "scay:", err) default: @@ -117,6 +119,12 @@ func (r *repl) command(line string) bool { } else { r.reload() } + case "/effort": + if err := effortCommand(r.in, args); err != nil { + fmt.Fprintln(os.Stderr, "scay:", err) + } else { + r.reload() + } case "/mcp": if err := mcpCommand(args); err != nil { fmt.Fprintln(os.Stderr, "scay:", err) @@ -148,7 +156,8 @@ func (r *repl) command(line string) bool { func printHelp() { fmt.Print(`Commands: - /model pick provider, model, and thinking effort + /model pick provider and model + /effort [level] pick thinking effort, or set off|low|medium|high|max /auth [provider] log in (OpenAI: API key or experimental ChatGPT subscription) /mcp manage MCP servers (run /mcp for subcommands) /compact replace history with a model-written summary (frees context) @@ -158,7 +167,7 @@ Anything else is sent to the model. Ctrl-C interrupts the current turn. `) } -// reload rebuilds the agent after a config change (/auth, /model, /mcp add…), +// reload rebuilds the agent after a config change (/auth, /model, /effort, /mcp add…), // so changes take effect without restarting the session. func (r *repl) reload() { switch err := r.rebuild(); { @@ -209,7 +218,7 @@ func (r *repl) printStatus() { if r.identity == "openai:chatgpt" { name += " / ChatGPT experimental" } - fmt.Printf(" %s / %s (effort %s) in %s\n", name, r.ag.Model, r.ag.Effort, wd) + fmt.Printf(" %s / %s (effort %s) in %s\n", name, safeDisplay(r.ag.Model), r.ag.Effort, wd) } func configuredProviderIdentity() string { @@ -244,6 +253,9 @@ func buildAgent(progress agent.Progress, approve tools.Approver) (*agent.Agent, return nil, nil, nop, tokErr // e.g. a failed ChatGPT token refresh } opts := provider.Options{APIKey: token, BaseURL: res.Host} + if cfg.Provider == "openai" && openAIDiagnosticsEnabled() { + opts.Diagnostic = os.Stderr + } if cfg.Provider == "openai" && res.Cred.Kind() == "chatgpt" { opts.ChatGPT = true opts.OpenAIAuth = func(ctx context.Context, retry bool) (provider.OpenAIAuth, error) { @@ -254,6 +266,22 @@ func buildAgent(progress agent.Progress, approve tools.Approver) (*agent.Agent, return provider.OpenAIAuth{Token: token, AccountID: res.Cred.AccountID()}, nil } } + // Platform's /v1/models does not expose reasoning capabilities. The Codex + // catalog does, so refresh just that metadata at startup without persisting + // a model cache. + if cfg.Provider == "openai" && res.Cred.Kind() == "chatgpt" { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + models, discoverErr := provider.OpenAIModels(ctx, opts) + cancel() + if discoverErr == nil { + for _, model := range models { + if model.ID == cfg.Model { + opts.OpenAIEfforts = model.ReasoningEfforts + break + } + } + } + } p, err := provider.New(cfg.Provider, opts) if err != nil { return nil, nil, nop, err diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f826ab2..c736780 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -6,7 +6,9 @@ import ( "encoding/json" "os" "path/filepath" + "strings" "testing" + "time" ) func TestResolveOpenAIChatGPTCredential(t *testing.T) { @@ -88,6 +90,45 @@ func TestSaveReplacesConfigWithPrivatePermissions(t *testing.T) { } } +func TestChatGPTRefreshRotationPersists(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("XDG_CONFIG_HOME", tmp) + t.Setenv("AppData", tmp) + cfg := &File{Provider: "openai", Providers: map[string]ProviderConfig{ + "openai": {AuthType: "chatgpt", OAuth: &OAuthCredential{ + AccessToken: "old-access", RefreshToken: "old-refresh", AccountID: "workspace_old", ExpiresAt: time.Now().Add(30 * time.Second), + }}, + }} + cred := &chatGPTCredential{file: cfg, providerID: "openai", refresh: func(_ context.Context, token string) (OAuthCredential, error) { + if token != "old-refresh" { + t.Fatalf("refresh token = %q", token) + } + return OAuthCredential{AccessToken: "new-access", RefreshToken: "new-refresh", AccountID: "workspace_new", ExpiresAt: time.Now().Add(time.Hour)}, nil + }} + if token, err := cred.Token(context.Background(), false); err != nil || token != "new-access" { + t.Fatalf("Token() = %q, %v", token, err) + } + reloaded, err := Load() + if err != nil { + t.Fatal(err) + } + got := reloaded.Providers["openai"].OAuth + if got.AccessToken != "new-access" || got.RefreshToken != "new-refresh" || got.AccountID != "workspace_new" { + t.Fatalf("rotated credential was not persisted: %+v", got) + } +} + +func TestChatGPTMissingRefreshTokenRequestsLogin(t *testing.T) { + cfg := &File{Providers: map[string]ProviderConfig{"openai": {AuthType: "chatgpt", OAuth: &OAuthCredential{ + AccessToken: "expired", ExpiresAt: time.Now().Add(-time.Minute), + }}}} + cred := &chatGPTCredential{file: cfg, providerID: "openai"} + if _, err := cred.Token(context.Background(), false); err == nil || !strings.Contains(err.Error(), "/auth openai") { + t.Fatalf("error = %v", err) + } +} + func TestMergedMCPServers(t *testing.T) { dir := t.TempDir() // Project file defines "fs" and overrides "search". diff --git a/internal/config/openai_oauth.go b/internal/config/openai_oauth.go index e378378..89a4beb 100644 --- a/internal/config/openai_oauth.go +++ b/internal/config/openai_oauth.go @@ -7,9 +7,11 @@ import ( "encoding/base64" "encoding/json" "fmt" + "io" "net" "net/http" "net/url" + "os" "strings" "sync" "time" @@ -52,6 +54,7 @@ type chatGPTCredential struct { file *File providerID string mu sync.Mutex + refresh func(context.Context, string) (OAuthCredential, error) } func (c *chatGPTCredential) Kind() string { return "chatgpt" } @@ -83,18 +86,24 @@ func (c *chatGPTCredential) Token(ctx context.Context, force bool) (string, erro c.mu.Lock() defer c.mu.Unlock() pc := c.file.Providers[c.providerID] - if pc.OAuth == nil || pc.OAuth.AccessToken == "" { + if pc.OAuth == nil { return "", ErrNoCredential } - if !force && (pc.OAuth.ExpiresAt.IsZero() || time.Until(pc.OAuth.ExpiresAt) > time.Minute) { + if pc.OAuth.AccessToken != "" && !force && (pc.OAuth.ExpiresAt.IsZero() || time.Until(pc.OAuth.ExpiresAt) > time.Minute) { return pc.OAuth.AccessToken, nil } if pc.OAuth.RefreshToken == "" { return "", fmt.Errorf("ChatGPT login expired — run /auth openai again") } - next, err := refreshChatGPTToken(ctx, pc.OAuth.RefreshToken) + debugOpenAIOAuth("refresh start forced=%t", force) + refresh := c.refresh + if refresh == nil { + refresh = refreshChatGPTToken + } + next, err := refresh(ctx, pc.OAuth.RefreshToken) if err != nil { - return "", err + debugOpenAIOAuth("refresh failed") + return "", fmt.Errorf("ChatGPT login refresh failed: %w — run /auth openai again", err) } if next.AccessToken != "" { pc.OAuth.AccessToken = next.AccessToken @@ -110,9 +119,17 @@ func (c *chatGPTCredential) Token(ctx context.Context, force bool) (string, erro if err := Save(c.file); err != nil { return "", err } + debugOpenAIOAuth("refresh complete rotated=%t", next.RefreshToken != "") return pc.OAuth.AccessToken, nil } +func debugOpenAIOAuth(format string, args ...any) { + switch os.Getenv("SCAY_OPENAI_DEBUG") { + case "1", "true", "TRUE", "yes", "YES": + fmt.Fprintf(os.Stderr, "scay: openai: oauth "+format+"\n", args...) + } +} + type oauthTokenResponse struct { AccessToken string `json:"access_token"` RefreshToken string `json:"refresh_token"` @@ -144,7 +161,7 @@ func refreshChatGPTToken(ctx context.Context, refreshToken string) (OAuthCredent return OAuthCredential{}, err } req.Header.Set("content-type", "application/x-www-form-urlencoded") - resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req) + resp, err := chatGPTTokenHTTPClient().Do(req) if err != nil { return OAuthCredential{}, err } @@ -153,7 +170,7 @@ func refreshChatGPTToken(ctx context.Context, refreshToken string) (OAuthCredent return OAuthCredential{}, fmt.Errorf("refresh ChatGPT login: %s", resp.Status) } var tokens oauthTokenResponse - if err := json.NewDecoder(resp.Body).Decode(&tokens); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&tokens); err != nil { return OAuthCredential{}, err } if tokens.AccessToken == "" { @@ -247,7 +264,7 @@ func exchangeChatGPTCode(ctx context.Context, code, verifier string) (OAuthCrede return OAuthCredential{}, err } req.Header.Set("content-type", "application/x-www-form-urlencoded") - resp, err := (&http.Client{Timeout: 30 * time.Second}).Do(req) + resp, err := chatGPTTokenHTTPClient().Do(req) if err != nil { return OAuthCredential{}, err } @@ -256,7 +273,7 @@ func exchangeChatGPTCode(ctx context.Context, code, verifier string) (OAuthCrede return OAuthCredential{}, fmt.Errorf("exchange ChatGPT login: %s", resp.Status) } var tokens oauthTokenResponse - if err := json.NewDecoder(resp.Body).Decode(&tokens); err != nil { + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&tokens); err != nil { return OAuthCredential{}, err } if tokens.AccessToken == "" || tokens.RefreshToken == "" { @@ -273,6 +290,17 @@ func randomURLValue(n int) (string, error) { return base64.RawURLEncoding.EncodeToString(b), nil } +func chatGPTTokenHTTPClient() *http.Client { + return &http.Client{ + Timeout: 30 * time.Second, + // Token POST bodies contain authorization codes or rotating refresh + // tokens. The fixed issuer endpoint should never redirect them. + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + } +} + func jwtClaims(token string) map[string]any { parts := strings.Split(token, ".") if len(parts) < 2 { diff --git a/internal/provider/models.go b/internal/provider/models.go index a5cf98c..df42443 100644 --- a/internal/provider/models.go +++ b/internal/provider/models.go @@ -9,7 +9,7 @@ var Providers = []string{"anthropic", "openai", "google", "ollama"} // is unreachable; a reachable server reports what's installed (OllamaModels). var CuratedModels = map[string][]string{ "anthropic": {"claude-opus-5", "claude-sonnet-5", "claude-haiku-4-5"}, - "openai": {"gpt-5.2", "gpt-5.5"}, + "openai": {"gpt-5.6", "gpt-5.6-sol", "gpt-5.6-terra", "gpt-5.6-luna", "gpt-5.5", "gpt-5.2"}, "google": {"gemini-3-pro", "gemini-2.5-pro", "gemini-2.5-flash"}, "ollama": {"llama3.2", "qwen3", "gpt-oss"}, } diff --git a/internal/provider/openai.go b/internal/provider/openai.go index 84db9bf..343f380 100644 --- a/internal/provider/openai.go +++ b/internal/provider/openai.go @@ -1,7 +1,6 @@ package provider import ( - "bytes" "context" "encoding/json" "errors" @@ -11,22 +10,27 @@ import ( "net/http" "strings" "time" + "unicode" ) // OpenAI adapter over Responses API. API keys use the public Platform endpoint; // ChatGPT login uses the Codex backend and adds the account header. Keeping the // two routes here lets the agent loop stay entirely provider-agnostic. const ( - openaiBase = "https://api.openai.com/v1" - chatGPTCodexBase = "https://chatgpt.com/backend-api/codex" + openaiBase = "https://api.openai.com/v1" + chatGPTCodexBase = "https://chatgpt.com/backend-api/codex" + openAIMaxErrorBody = 1 << 20 + openAIMaxResponseBody = 32 << 20 ) type openai struct { - apiKey string - auth func(context.Context, bool) (OpenAIAuth, error) - chatGPT bool - endpoint string - http *http.Client + apiKey string + auth func(context.Context, bool) (OpenAIAuth, error) + chatGPT bool + endpoint string + http *http.Client + efforts []string + diagnostic io.Writer } func newOpenAI(opts Options) *openai { @@ -41,7 +45,7 @@ func newOpenAI(opts Options) *openai { if hc == nil { hc = &http.Client{Timeout: 10 * time.Minute} } - return &openai{apiKey: opts.APIKey, auth: opts.OpenAIAuth, chatGPT: opts.ChatGPT, endpoint: base + "/responses", http: hc} + return &openai{apiKey: opts.APIKey, auth: opts.OpenAIAuth, chatGPT: opts.ChatGPT, endpoint: base + "/responses", http: hc, efforts: opts.OpenAIEfforts, diagnostic: opts.Diagnostic} } func (o *openai) Name() string { return "openai" } @@ -73,24 +77,13 @@ type oaiReasoning struct { Effort string `json:"effort"` } -func (o *openai) newHTTPRequest(ctx context.Context, r Request, stream, refresh bool) (*http.Request, error) { - auth := OpenAIAuth{Token: o.apiKey} - if o.auth != nil { - var err error - auth, err = o.auth(ctx, refresh) - if err != nil { - return nil, err - } - } - if auth.Token == "" { - return nil, errors.New("openai: no credential") - } +func (o *openai) requestPayload(r Request, stream bool) ([]byte, error) { body := oaiResponsesRequest{ Model: r.Model, Instructions: r.System, Input: oaiResponseInput(r.Messages), Tools: oaiResponseTools(r.Tools), - Reasoning: &oaiReasoning{Effort: oaiEffort(r.Effort)}, + Reasoning: &oaiReasoning{Effort: oaiEffort(r.Model, r.Effort, o.efforts)}, MaxOutputTokens: r.MaxTokens, Stream: stream, } @@ -108,38 +101,17 @@ func (o *openai) newHTTPRequest(ctx context.Context, r Request, stream, refresh if err != nil { return nil, err } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, o.endpoint, bytes.NewReader(payload)) - if err != nil { - return nil, err - } - req.Header.Set("content-type", "application/json") - req.Header.Set("authorization", "Bearer "+auth.Token) - if o.chatGPT && auth.AccountID != "" { - req.Header.Set("ChatGPT-Account-ID", auth.AccountID) - } - return req, nil + return payload, nil } // do performs an OpenAI request and lets an OAuth credential refresh exactly // once after a 401. API-key callers never take the refresh path. func (o *openai) do(ctx context.Context, r Request, stream bool) (*http.Response, error) { - for refreshed := false; ; { - req, err := o.newHTTPRequest(ctx, r, stream, refreshed) - if err != nil { - return nil, err - } - hc := o.http - if stream { - hc = streamClient(hc) - } - resp, err := doRetry(hc, req) - if err != nil || resp == nil || resp.StatusCode != http.StatusUnauthorized || refreshed || o.auth == nil { - return resp, err - } - io.Copy(io.Discard, io.LimitReader(resp.Body, 4<<10)) - resp.Body.Close() - refreshed = true + payload, err := o.requestPayload(r, stream) + if err != nil { + return nil, err } + return o.doAuthorized(ctx, http.MethodPost, o.endpoint, payload, r.Model, stream) } func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) { @@ -149,14 +121,16 @@ func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) } if resp.StatusCode != http.StatusOK { defer resp.Body.Close() - b, _ := io.ReadAll(resp.Body) - return nil, fmt.Errorf("openai: %s: %s", resp.Status, clip(b, 500)) + b, _ := io.ReadAll(io.LimitReader(resp.Body, openAIMaxErrorBody)) + return nil, openAIStatusError(resp, b) } return func(yield func(Event) bool) { defer resp.Body.Close() done := false var streamedItems []Block + var streamedText strings.Builder + var streamedThinking strings.Builder emit := func(e Event) error { if !yield(e) { return errStopped @@ -178,19 +152,27 @@ func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) } `json:"error"` } if err := json.Unmarshal([]byte(data), &frame); err != nil { - return nil // tolerate newly-added frame types + return fmt.Errorf("openai: protocol: malformed SSE event %s", safeDiagnostic(event)) } kind := frame.Type if kind == "" { kind = event } + o.diagnose("sse event=%s model=%s", safeDiagnostic(kind), safeDiagnostic(r.Model)) switch kind { case "response.output_text.delta": if frame.Delta != "" { + streamedText.WriteString(frame.Delta) + return emit(Event{Kind: "text", Text: frame.Delta}) + } + case "response.refusal.delta": + if frame.Delta != "" { + streamedText.WriteString(frame.Delta) return emit(Event{Kind: "text", Text: frame.Delta}) } case "response.reasoning_summary_text.delta", "response.reasoning_text.delta": if frame.Delta != "" { + streamedThinking.WriteString(frame.Delta) return emit(Event{Kind: "thinking", Text: frame.Delta}) } case "response.output_item.done": @@ -207,19 +189,35 @@ func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) if err != nil { return err } - if len(result.Blocks) == 0 { - result.Blocks = streamedItems - } else if len(result.ToolCalls()) == 0 { - // The public endpoint normally repeats calls in the final - // response; fall back to the streamed ones if it didn't. - for _, b := range streamedItems { - if b.Kind == BlockToolCall { - result.Blocks = append(result.Blocks, b) - } + result.Blocks = mergeOAIBlocks(result.Blocks, streamedItems) + if streamedText.Len() > 0 && !hasBlockKind(result.Blocks, BlockText) { + result.Blocks = append(result.Blocks, Block{Kind: BlockText, Text: streamedText.String()}) + } + if streamedThinking.Len() > 0 && !hasBlockKind(result.Blocks, BlockThinking) { + result.Blocks = append(result.Blocks, Block{Kind: BlockThinking, Text: streamedThinking.String()}) + } + // Some compatible servers put refusal or output text only in the + // terminal response. Emit what was not already streamed so the + // interactive UI cannot finish with a silent turn. + finalText := result.Text() + streamed := streamedText.String() + missing := "" + switch { + case streamed == "": + missing = finalText + case strings.HasPrefix(finalText, streamed): + missing = strings.TrimPrefix(finalText, streamed) + } + if missing != "" { + streamedText.WriteString(missing) + if err := emit(Event{Kind: "text", Text: missing}); err != nil { + return err } } if len(result.ToolCalls()) > 0 { result.StopReason = "tool_calls" + } else if kind == "response.incomplete" { + result.StopReason = "incomplete" } for _, call := range result.ToolCalls() { c := call @@ -233,10 +231,10 @@ func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) // The `error` event carries message at top level; // response.failed nests it under response.error. if frame.Error != nil && frame.Error.Message != "" { - return fmt.Errorf("openai: %s", frame.Error.Message) + return fmt.Errorf("openai: %s", safeOpenAIMessage(frame.Error.Message)) } if frame.Message != "" { - return fmt.Errorf("openai: %s", frame.Message) + return fmt.Errorf("openai: %s", safeOpenAIMessage(frame.Message)) } var failed struct { Error struct { @@ -244,7 +242,7 @@ func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) } `json:"error"` } if len(frame.Response) > 0 && json.Unmarshal(frame.Response, &failed) == nil && failed.Error.Message != "" { - return fmt.Errorf("openai: %s", failed.Error.Message) + return fmt.Errorf("openai: %s", safeOpenAIMessage(failed.Error.Message)) } return errors.New("openai: response failed") } @@ -254,11 +252,11 @@ func (o *openai) Stream(ctx context.Context, r Request) (iter.Seq[Event], error) return } if err != nil { - yield(Event{Kind: "error", Err: err}) + yield(Event{Kind: "error", Err: fmt.Errorf("openai: stream_interrupted: %w", err)}) return } if !done { - yield(Event{Kind: "error", Err: errors.New("openai: stream ended before response.completed")}) + yield(Event{Kind: "error", Err: errors.New("openai: stream_interrupted: stream ended before a terminal response event")}) } }, nil } @@ -269,12 +267,16 @@ func (o *openai) Chat(ctx context.Context, r Request) (*Response, error) { return nil, err } defer resp.Body.Close() - raw, err := io.ReadAll(resp.Body) + limit := int64(openAIMaxResponseBody) + if resp.StatusCode != http.StatusOK { + limit = openAIMaxErrorBody + } + raw, err := io.ReadAll(io.LimitReader(resp.Body, limit)) if err != nil { return nil, err } if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("openai: %s: %s", resp.Status, clip(raw, 500)) + return nil, openAIStatusError(resp, raw) } return parseOAIResponse(raw) } @@ -421,17 +423,111 @@ func oaiResponseTools(tools []Tool) []oaiResponseTool { return out } -func oaiEffort(e Effort) string { - switch e { - case EffortOff: - return "minimal" - case EffortLow: - return "low" - case EffortHigh: - return "high" - case EffortMax: - return "xhigh" - default: - return "medium" +func oaiEffort(model string, e Effort, supported []string) string { + if len(supported) == 0 { + if strings.HasPrefix(model, "gpt-5.6") { + supported = []string{"none", "low", "medium", "high", "xhigh", "max"} + } else { + supported = []string{"minimal", "low", "medium", "high", "xhigh"} + } + } + target := map[Effort]string{EffortOff: "none", EffortLow: "low", EffortMedium: "medium", EffortHigh: "high", EffortMax: "max"}[e] + if target == "" { + target = "medium" + } + return closestOAIEffort(target, supported) +} + +// OpenAIEffort exposes the capability-aware wire mapping for CLI display while +// keeping the actual request construction inside the OpenAI adapter. +func OpenAIEffort(model string, e Effort, supported []string) string { + return oaiEffort(model, e, supported) +} + +func (o *openai) diagnose(format string, args ...any) { + if o.diagnostic != nil { + fmt.Fprintf(o.diagnostic, "scay: openai: "+format+"\n", args...) + } +} + +func safeDiagnostic(s string) string { + if s == "" { + return "-" + } + for _, r := range s { + if r < 0x21 || r > 0x7e { + return "invalid" + } + } + if len(s) > 128 { + return s[:128] + } + return s +} + +func safeOpenAIMessage(s string) string { + const max = 500 + var b strings.Builder + for _, r := range s { + if !unicode.IsPrint(r) { + r = ' ' + } + if b.Len()+len(string(r)) > max { + break + } + b.WriteRune(r) + } + message := strings.TrimSpace(b.String()) + if message == "" && s != "" { + return "request failed" + } + return message +} + +func hasBlockKind(blocks []Block, kind BlockKind) bool { + for _, b := range blocks { + if b.Kind == kind { + return true + } + } + return false +} + +func mergeOAIBlocks(primary, secondary []Block) []Block { + out := append([]Block(nil), primary...) + seen := make(map[string]bool, len(out)) + for _, b := range out { + seen[oaiBlockKey(b)] = true + } + for _, b := range secondary { + key := oaiBlockKey(b) + if !seen[key] { + out = append(out, b) + seen[key] = true + } + } + return out +} + +func oaiBlockKey(b Block) string { + if len(b.Reasoning) > 0 { + var item struct { + ID string `json:"id"` + Type string `json:"type"` + CallID string `json:"call_id"` + } + if json.Unmarshal(b.Reasoning, &item) == nil { + if item.ID != "" { + return item.Type + ":id:" + item.ID + } + if item.CallID != "" { + return item.Type + ":call:" + item.CallID + } + } + return "raw:" + string(b.Reasoning) + } + if b.ToolCall != nil { + return "call:" + b.ToolCall.ID + ":" + b.ToolCall.Name } + return string(b.Kind) + ":" + b.Text } diff --git a/internal/provider/openai_models.go b/internal/provider/openai_models.go new file mode 100644 index 0000000..d2b5162 --- /dev/null +++ b/internal/provider/openai_models.go @@ -0,0 +1,259 @@ +package provider + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "sort" + "strings" +) + +// OpenAIModel is the small, provider-owned subset of model discovery metadata +// that scay actually uses. Platform currently supplies only ID; the Codex +// backend also supplies picker ordering and reasoning capabilities. +type OpenAIModel struct { + ID string + ReasoningEfforts []string + DefaultEffort string + Priority int +} + +// OpenAIModels fetches the account-aware OpenAI catalog. It deliberately does +// not send client_version: that field identifies the Codex client version, and +// scay must not impersonate a release of the official client. +func OpenAIModels(ctx context.Context, opts Options) ([]OpenAIModel, error) { + o := newOpenAI(opts) + base := strings.TrimSuffix(o.endpoint, "/responses") + resp, err := o.doAuthorized(ctx, http.MethodGet, base+"/models", nil, "", false) + if err != nil { + return nil, err + } + defer resp.Body.Close() + raw, err := io.ReadAll(io.LimitReader(resp.Body, 2<<20)) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, openAIStatusError(resp, raw) + } + if opts.ChatGPT { + return parseCodexModels(raw) + } + return parsePlatformModels(raw) +} + +func (o *openai) doAuthorized(ctx context.Context, method, url string, body []byte, model string, stream bool) (*http.Response, error) { + for refreshed := false; ; { + auth := OpenAIAuth{Token: o.apiKey} + var err error + if o.auth != nil { + auth, err = o.auth(ctx, refreshed) + if err != nil { + return nil, err + } + } + if auth.Token == "" { + return nil, fmt.Errorf("openai: authentication: no credential — run /auth openai") + } + if o.chatGPT && auth.AccountID == "" { + return nil, fmt.Errorf("openai: authentication: ChatGPT login is missing an account ID — run /auth openai") + } + req, err := http.NewRequestWithContext(ctx, method, url, bytes.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Set("authorization", "Bearer "+auth.Token) + if len(body) > 0 { + req.Header.Set("content-type", "application/json") + } + if o.chatGPT && auth.AccountID != "" { + req.Header.Set("ChatGPT-Account-ID", auth.AccountID) + } + hc := openAISameOriginClient(o.http) + if stream { + hc = streamClient(hc) + } + resp, err := doRetry(hc, req) + if resp != nil { + o.diagnose("request status=%d request_id=%s model=%s refresh=%t", resp.StatusCode, safeDiagnostic(resp.Header.Get("x-request-id")), safeDiagnostic(model), refreshed) + } + if err != nil || resp == nil || resp.StatusCode != http.StatusUnauthorized || refreshed || o.auth == nil { + return resp, err + } + io.Copy(io.Discard, io.LimitReader(resp.Body, 4<<10)) + resp.Body.Close() + o.diagnose("auth retry model=%s", safeDiagnostic(model)) + refreshed = true + } +} + +// openAISameOriginClient prevents 307/308 redirects from replaying a request +// body (prompts, tools, or OAuth-backed requests) and the account header to a +// different origin. Same-origin redirects retain the caller's redirect policy. +func openAISameOriginClient(hc *http.Client) *http.Client { + copy := *hc + previous := copy.CheckRedirect + copy.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) > 0 && (req.URL.Scheme != via[0].URL.Scheme || req.URL.Host != via[0].URL.Host) { + return http.ErrUseLastResponse + } + if previous != nil { + return previous(req, via) + } + if len(via) >= 10 { + return fmt.Errorf("openai: stopped after 10 redirects") + } + return nil + } + return © +} + +func parsePlatformModels(raw []byte) ([]OpenAIModel, error) { + var wire struct { + Data []struct { + ID string `json:"id"` + } `json:"data"` + } + if err := json.Unmarshal(raw, &wire); err != nil { + return nil, fmt.Errorf("openai: protocol: decode models: %w", err) + } + models := make([]OpenAIModel, 0, len(wire.Data)) + for _, m := range wire.Data { + if validOpenAIModelID(m.ID) && platformResponsesModel(m.ID) { + models = append(models, OpenAIModel{ID: m.ID}) + } + } + sort.SliceStable(models, func(i, j int) bool { return models[i].ID > models[j].ID }) + return models, nil +} + +func parseCodexModels(raw []byte) ([]OpenAIModel, error) { + var wire struct { + Models []struct { + Slug string `json:"slug"` + Visibility string `json:"visibility"` + Priority int `json:"priority"` + DefaultEffort string `json:"default_reasoning_level"` + Efforts []struct { + Effort string `json:"effort"` + } `json:"supported_reasoning_levels"` + } `json:"models"` + } + if err := json.Unmarshal(raw, &wire); err != nil { + return nil, fmt.Errorf("openai: protocol: decode Codex models: %w", err) + } + models := make([]OpenAIModel, 0, len(wire.Models)) + for _, m := range wire.Models { + // supported_in_api controls API-key auth in the official client. This + // endpoint is used for ChatGPT auth, where subscription-only models are + // valid; picker visibility is the relevant filter. + if !validOpenAIModelID(m.Slug) || (m.Visibility != "" && m.Visibility != "list") { + continue + } + model := OpenAIModel{ID: m.Slug, Priority: m.Priority} + if validOpenAIEffort(m.DefaultEffort) { + model.DefaultEffort = m.DefaultEffort + } + for _, effort := range m.Efforts { + if validOpenAIEffort(effort.Effort) { + model.ReasoningEfforts = append(model.ReasoningEfforts, effort.Effort) + } + } + models = append(models, model) + } + sort.SliceStable(models, func(i, j int) bool { return models[i].Priority < models[j].Priority }) + return models, nil +} + +func validOpenAIModelID(id string) bool { + return id != "" && safeDiagnostic(id) == id +} + +func validOpenAIEffort(effort string) bool { + switch effort { + case "none", "minimal", "low", "medium", "high", "xhigh", "max": + return true + default: + return false + } +} + +func platformResponsesModel(id string) bool { + if !strings.HasPrefix(id, "gpt-") { + return false + } + for _, excluded := range []string{"audio", "realtime", "transcribe", "tts", "image", "embedding", "moderation", "instruct"} { + if strings.Contains(id, excluded) { + return false + } + } + // Date-pinned snapshots make the picker noisy; users can still type one. + parts := strings.Split(id, "-") + if len(parts) >= 4 { + last := parts[len(parts)-3:] + if len(last[0]) == 4 && len(last[1]) == 2 && len(last[2]) == 2 { + return false + } + } + return true +} + +func closestOAIEffort(target string, supported []string) string { + rank := map[string]int{"none": 0, "minimal": 0, "low": 1, "medium": 2, "high": 3, "xhigh": 4, "max": 5} + targetRank := rank[target] + best, bestDistance := "", 100 + for _, effort := range supported { + r, ok := rank[effort] + if !ok { + continue + } + distance := r - targetRank + if distance < 0 { + distance = -distance + } + if distance < bestDistance || (distance == bestDistance && r < rank[best]) { + best, bestDistance = effort, distance + } + } + if best != "" { + return best + } + if len(supported) > 0 { + return supported[0] + } + return target +} + +func openAIStatusError(resp *http.Response, raw []byte) error { + kind := "request" + switch resp.StatusCode { + case http.StatusUnauthorized, http.StatusForbidden: + kind = "authentication" + case http.StatusTooManyRequests: + kind = "rate_limit" + } + var wire struct { + Error struct { + Message string `json:"message"` + Code string `json:"code"` + Type string `json:"type"` + } `json:"error"` + } + _ = json.Unmarshal(raw, &wire) + marker := strings.ToLower(wire.Error.Code + " " + wire.Error.Type + " " + wire.Error.Message) + if strings.Contains(marker, "model") && (strings.Contains(marker, "not found") || strings.Contains(marker, "not_found") || strings.Contains(marker, "access")) { + kind = "model_unavailable" + } + message := wire.Error.Message + if message == "" { + message = string(raw) + } + message = safeOpenAIMessage(message) + if message == "" { + message = "request failed" + } + return fmt.Errorf("openai: %s: %s: %s (request_id=%s)", kind, resp.Status, message, safeDiagnostic(resp.Header.Get("x-request-id"))) +} diff --git a/internal/provider/openai_models_test.go b/internal/provider/openai_models_test.go new file mode 100644 index 0000000..ad64171 --- /dev/null +++ b/internal/provider/openai_models_test.go @@ -0,0 +1,168 @@ +package provider + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" +) + +func TestOpenAIPlatformModelsFiltersCatalog(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/models" || r.Header.Get("Authorization") != "Bearer key" { + t.Errorf("request = %s %s auth=%q", r.Method, r.URL, r.Header.Get("Authorization")) + } + io.WriteString(w, `{"data":[ + {"id":"gpt-5.6"},{"id":"gpt-5.6-2026-08-01"},{"id":"gpt-realtime-2"}, + {"id":"text-embedding-4"},{"id":"gpt-4.1"},{"id":"o4-mini"}, + {"id":"gpt-forged\nmenu"} + ]}`) + })) + defer srv.Close() + + models, err := OpenAIModels(context.Background(), Options{APIKey: "key", BaseURL: srv.URL}) + if err != nil { + t.Fatal(err) + } + got := []string{} + for _, model := range models { + got = append(got, model.ID) + } + if want := []string{"gpt-5.6", "gpt-4.1"}; !slices.Equal(got, want) { + t.Fatalf("models = %v, want %v", got, want) + } +} + +func TestOpenAIChatGPTModelsRefreshAndCapabilities(t *testing.T) { + requests, refreshes := 0, 0 + var diagnostic bytes.Buffer + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + if r.URL.RawQuery != "" { + t.Errorf("scay must not send a fake Codex client_version: %s", r.URL.RawQuery) + } + if r.Header.Get("ChatGPT-Account-ID") != "workspace_1" { + t.Errorf("account header = %q", r.Header.Get("ChatGPT-Account-ID")) + } + w.Header().Set("x-request-id", "req_models_1") + if requests == 1 { + w.WriteHeader(http.StatusUnauthorized) + return + } + io.WriteString(w, `{"models":[ + {"slug":"hidden","visibility":"hide","supported_in_api":true,"priority":0}, + {"slug":"forged\u001b[31m","visibility":"list","priority":1}, + {"slug":"subscription-only","visibility":"list","supported_in_api":false,"priority":30}, + {"slug":"gpt-5.6-terra","visibility":"list","supported_in_api":true,"priority":20, + "default_reasoning_level":"low","supported_reasoning_levels":[{"effort":"none"},{"effort":"low"},{"effort":"evil\nvalue"},{"effort":"max"}]}, + {"slug":"gpt-5.6-sol","visibility":"list","supported_in_api":true,"priority":10} + ]}`) + })) + defer srv.Close() + + models, err := OpenAIModels(context.Background(), Options{ + BaseURL: srv.URL, ChatGPT: true, Diagnostic: &diagnostic, + OpenAIAuth: func(_ context.Context, retry bool) (OpenAIAuth, error) { + if retry { + refreshes++ + return OpenAIAuth{Token: "fresh-secret", AccountID: "workspace_1"}, nil + } + return OpenAIAuth{Token: "stale-secret", AccountID: "workspace_1"}, nil + }, + }) + if err != nil { + t.Fatal(err) + } + if requests != 2 || refreshes != 1 || len(models) != 3 || models[0].ID != "gpt-5.6-sol" || models[2].ID != "subscription-only" { + t.Fatalf("requests=%d refreshes=%d models=%+v", requests, refreshes, models) + } + if !slices.Equal(models[1].ReasoningEfforts, []string{"none", "low", "max"}) || models[1].DefaultEffort != "low" { + t.Fatalf("capabilities lost: %+v", models[1]) + } + log := diagnostic.String() + if !strings.Contains(log, "status=401") || !strings.Contains(log, "request_id=req_models_1") || + strings.Contains(log, "stale-secret") || strings.Contains(log, "fresh-secret") { + t.Fatalf("unsafe or incomplete diagnostic: %q", log) + } +} + +func TestOpenAIEffortUsesModelCapabilities(t *testing.T) { + cases := []struct { + model string + effort Effort + supported []string + want string + }{ + {"gpt-5.6-sol", EffortOff, nil, "none"}, + {"gpt-5.6-sol", EffortMax, nil, "max"}, + {"gpt-5.5", EffortOff, nil, "minimal"}, + {"gpt-5.5", EffortMax, nil, "xhigh"}, + {"future", EffortMax, []string{"none", "low", "high"}, "high"}, + {"future", EffortOff, []string{"minimal", "medium"}, "minimal"}, + } + for _, tc := range cases { + if got := oaiEffort(tc.model, tc.effort, tc.supported); got != tc.want { + t.Errorf("oaiEffort(%q, %q, %v) = %q, want %q", tc.model, tc.effort, tc.supported, got, tc.want) + } + } +} + +func TestOpenAIStatusErrorsAreClassified(t *testing.T) { + resp := &http.Response{StatusCode: 429, Status: "429 Too Many Requests", Header: http.Header{"X-Request-Id": {"req_1"}}} + err := openAIStatusError(resp, []byte(`{"error":{"message":"slow down"}}`)) + if !strings.Contains(err.Error(), "rate_limit") || !strings.Contains(err.Error(), "req_1") { + t.Fatal(err) + } + resp.StatusCode, resp.Status = 404, "404 Not Found" + err = openAIStatusError(resp, []byte(`{"error":{"code":"model_not_found","message":"model not found"}}`)) + if !strings.Contains(err.Error(), "model_unavailable") { + t.Fatal(err) + } + resp.StatusCode, resp.Status = 500, "500 Internal Server Error" + err = openAIStatusError(resp, []byte("remote\nmessage\x1b[31m")) + if strings.ContainsAny(err.Error(), "\n\r\x1b") || !strings.Contains(err.Error(), "remote message [31m") { + t.Fatalf("unsafe remote error: %q", err) + } +} + +func TestOpenAIDiagnosticsRejectControlCharacters(t *testing.T) { + var diagnostic bytes.Buffer + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("content-type", "application/json") + io.WriteString(w, `{"output":[]}`) + })) + defer srv.Close() + + p, err := New("openai", Options{APIKey: "secret-token", BaseURL: srv.URL, Diagnostic: &diagnostic}) + if err != nil { + t.Fatal(err) + } + _, err = p.Chat(context.Background(), Request{Model: "gpt-safe\nforged\x1b[31m"}) + if err != nil { + t.Fatal(err) + } + log := diagnostic.String() + if !strings.Contains(log, "model=invalid") || strings.Contains(log, "forged") || strings.Contains(log, "secret-token") || strings.Contains(log, "\x1b") { + t.Fatalf("unsafe diagnostic: %q", log) + } +} + +func TestOpenAIModelsHonorsCancellationAndRequiresChatGPTAccount(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := OpenAIModels(ctx, Options{APIKey: "key", BaseURL: "http://127.0.0.1:1"}); err == nil || !strings.Contains(err.Error(), "canceled") { + t.Fatalf("cancellation error = %v", err) + } + if _, err := OpenAIModels(context.Background(), Options{ + ChatGPT: true, BaseURL: "http://127.0.0.1:1", + OpenAIAuth: func(context.Context, bool) (OpenAIAuth, error) { + return OpenAIAuth{Token: "token"}, nil + }, + }); err == nil || !strings.Contains(err.Error(), "/auth openai") { + t.Fatalf("missing account error = %v", err) + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index aff39b9..d37a7a1 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -13,6 +13,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "iter" "net/http" "strings" @@ -165,8 +166,10 @@ type Options struct { // OpenAIAuth supplies a current bearer token. retry is true only after an // OpenAI 401, allowing OAuth implementations to refresh a rotating token. - OpenAIAuth func(ctx context.Context, retry bool) (OpenAIAuth, error) - ChatGPT bool // use the experimental ChatGPT/Codex backend instead of Platform + OpenAIAuth func(ctx context.Context, retry bool) (OpenAIAuth, error) + ChatGPT bool // use the experimental ChatGPT/Codex backend instead of Platform + OpenAIEfforts []string // model capabilities returned by OpenAI discovery, when available + Diagnostic io.Writer // opt-in, secret-free OpenAI protocol diagnostics } // OpenAIAuth is the resolved authorization for one OpenAI request. diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go index 4353028..33d5a16 100644 --- a/internal/provider/provider_test.go +++ b/internal/provider/provider_test.go @@ -170,6 +170,25 @@ func TestOpenAIChatGPTRefreshesAfterUnauthorized(t *testing.T) { } } +func TestOpenAIRejectsCrossOriginRedirect(t *testing.T) { + redirected := 0 + target := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + redirected++ + })) + defer target.Close() + source := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("location", target.URL+"/stolen") + w.WriteHeader(http.StatusTemporaryRedirect) + })) + defer source.Close() + + p, _ := New("openai", Options{APIKey: "secret", BaseURL: source.URL}) + _, err := p.Chat(context.Background(), Request{Model: "gpt-5.6", Messages: []Message{{Role: RoleUser, Text: "private prompt"}}}) + if err == nil || redirected != 0 { + t.Fatalf("cross-origin redirect followed: requests=%d error=%v", redirected, err) + } +} + func TestGeminiAdapter(t *testing.T) { var body string resp := `{"candidates":[{"content":{"parts":[{"functionCall":{"name":"bash","args":{"command":"pwd"}}}]},"finishReason":"STOP"}],"usageMetadata":{"promptTokenCount":5,"candidatesTokenCount":6}}` diff --git a/internal/provider/stream_test.go b/internal/provider/stream_test.go index c6b129b..44a56c0 100644 --- a/internal/provider/stream_test.go +++ b/internal/provider/stream_test.go @@ -127,6 +127,127 @@ func TestOpenAIStream(t *testing.T) { } } +func TestOpenAIIncompleteStreamKeepsEveryCompletedItem(t *testing.T) { + var body string + srv := sseServer(&body, + [2]string{"response.output_text.delta", `{"type":"response.output_text.delta","delta":"partial"}`}, + [2]string{"response.output_item.done", `{"type":"response.output_item.done","item":{"id":"rs_1","type":"reasoning","summary":[],"encrypted_content":"opaque"}}`}, + [2]string{"response.output_item.done", `{"type":"response.output_item.done","item":{"type":"function_call","call_id":"call_1","name":"bash","arguments":"{\"command\":\"pwd\"}"}}`}, + [2]string{"response.output_item.done", `{"type":"response.output_item.done","item":{"type":"function_call","call_id":"call_2","name":"read_file","arguments":"{\"path\":\"x\"}"}}`}, + [2]string{"response.incomplete", `{"type":"response.incomplete","response":{"status":"incomplete","output":[]}}`}, + ) + defer srv.Close() + + p, _ := New("openai", Options{APIKey: "k", BaseURL: srv.URL}) + events, err := p.Stream(context.Background(), historyReq("gpt-5.6")) + if err != nil { + t.Fatal(err) + } + text, _, resp := collect(t, events) + if text != "partial" || resp.Text() != "partial" || resp.StopReason != "tool_calls" { + t.Fatalf("partial response lost: text=%q response=%+v", text, resp) + } + if calls := resp.ToolCalls(); len(calls) != 2 || calls[0].ID != "call_1" || calls[1].ID != "call_2" { + t.Fatalf("multiple calls lost: %+v", calls) + } + input := oaiResponseInput([]Message{{Role: RoleAssistant, Blocks: resp.Blocks}}) + joined := "" + for _, item := range input { + joined += string(item) + } + mustContain(t, joined, `"encrypted_content":"opaque"`, `"call_id":"call_1"`, `"call_id":"call_2"`, `"type":"output_text"`) +} + +func TestOpenAIInterruptedStreamReturnsClassifiedError(t *testing.T) { + var body string + srv := sseServer(&body, [2]string{"response.output_text.delta", `{"type":"response.output_text.delta","delta":"orphan"}`}) + defer srv.Close() + p, _ := New("openai", Options{APIKey: "k", BaseURL: srv.URL}) + events, err := p.Stream(context.Background(), historyReq("gpt-5.6")) + if err != nil { + t.Fatal(err) + } + var streamErr error + for event := range events { + if event.Kind == "error" { + streamErr = event.Err + } + } + if streamErr == nil || !strings.Contains(streamErr.Error(), "stream_interrupted") { + t.Fatalf("stream error = %v", streamErr) + } +} + +func TestOpenAIStreamRefusalAndMalformedFrame(t *testing.T) { + var body string + refusal := sseServer(&body, + [2]string{"response.refusal.delta", `{"type":"response.refusal.delta","delta":"cannot comply"}`}, + [2]string{"response.completed", `{"type":"response.completed","response":{"output":[{"type":"message","content":[{"type":"refusal","refusal":"cannot comply"}]}]}}`}, + ) + defer refusal.Close() + p, _ := New("openai", Options{APIKey: "k", BaseURL: refusal.URL}) + events, err := p.Stream(context.Background(), historyReq("gpt-5.6")) + if err != nil { + t.Fatal(err) + } + text, _, resp := collect(t, events) + if text != "cannot comply" || resp.Text() != "cannot comply" { + t.Fatalf("refusal was lost: delta=%q response=%+v", text, resp) + } + + malformed := sseServer(&body, [2]string{"response.output_text.delta", `{not-json}`}) + defer malformed.Close() + p, _ = New("openai", Options{APIKey: "k", BaseURL: malformed.URL}) + events, err = p.Stream(context.Background(), historyReq("gpt-5.6")) + if err != nil { + t.Fatal(err) + } + var protocolErr error + for event := range events { + if event.Kind == "error" { + protocolErr = event.Err + } + } + if protocolErr == nil || !strings.Contains(protocolErr.Error(), "protocol") { + t.Fatalf("malformed frame error = %v", protocolErr) + } +} + +func TestOpenAIStreamEmitsTerminalOnlyText(t *testing.T) { + var body string + srv := sseServer(&body, + [2]string{"response.completed", `{"type":"response.completed","response":{"output":[{"type":"message","content":[{"type":"refusal","refusal":"terminal refusal"}]}]}}`}, + ) + defer srv.Close() + p, _ := New("openai", Options{APIKey: "k", BaseURL: srv.URL}) + events, err := p.Stream(context.Background(), historyReq("gpt-5.6")) + if err != nil { + t.Fatal(err) + } + text, _, resp := collect(t, events) + if text != "terminal refusal" || resp.Text() != "terminal refusal" { + t.Fatalf("terminal-only text was lost: delta=%q response=%+v", text, resp) + } +} + +func TestOpenAIStreamEmitsTerminalTextSuffix(t *testing.T) { + var body string + srv := sseServer(&body, + [2]string{"response.output_text.delta", `{"type":"response.output_text.delta","delta":"partial"}`}, + [2]string{"response.completed", `{"type":"response.completed","response":{"output":[{"type":"message","content":[{"type":"output_text","text":"partial response"}]}]}}`}, + ) + defer srv.Close() + p, _ := New("openai", Options{APIKey: "k", BaseURL: srv.URL}) + events, err := p.Stream(context.Background(), historyReq("gpt-5.6")) + if err != nil { + t.Fatal(err) + } + text, _, resp := collect(t, events) + if text != "partial response" || resp.Text() != "partial response" { + t.Fatalf("terminal suffix was lost or duplicated: delta=%q response=%+v", text, resp) + } +} + func TestGeminiStream(t *testing.T) { var body string srv := sseServer(&body, From 06a984d3bdf7192a0c2a549cd3aa5403c555be15 Mon Sep 17 00:00:00 2001 From: claudi47 Date: Sun, 9 Aug 2026 13:37:52 +0200 Subject: [PATCH 2/2] fix: preserve cancelled tool results and bound overwrite previews --- internal/agent/agent.go | 22 +++++++++- internal/agent/agent_test.go | 56 ++++++++++++++++++++++++ internal/tools/tools_test.go | 33 ++++++++++++++ internal/tools/write.go | 83 ++++++++++++++++++++++++++++++++++-- 4 files changed, 189 insertions(+), 5 deletions(-) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index e6d014f..0deb4a5 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -78,10 +78,12 @@ func (a *Agent) Run(ctx context.Context, s *Session, input string) error { return nil // the model is done } - for _, call := range calls { + for callIndex, call := range calls { // Stop dispatching once the user interrupts — mutating tools must - // not keep running (or keep prompting) after a Ctrl-C. + // not keep running (or keep prompting) after a Ctrl-C. Every queued + // call still needs a result so the provider history remains valid. if err := ctx.Err(); err != nil { + appendSkippedToolResults(s, calls[callIndex:], err) return err } p.ToolUse(call.Name, briefArgs(call.Args)) @@ -101,11 +103,27 @@ func (a *Agent) Run(ctx context.Context, s *Session, input string) error { Text: text, IsError: isErr, }) + if err := ctx.Err(); err != nil { + appendSkippedToolResults(s, calls[callIndex+1:], err) + return err + } } } return ErrMaxIterations } +func appendSkippedToolResults(s *Session, calls []provider.ToolCall, cause error) { + for _, call := range calls { + s.Append(provider.Message{ + Role: provider.RoleTool, + ToolCallID: call.ID, + ToolName: call.Name, + Text: "Error: tool call skipped after cancellation: " + cause.Error(), + IsError: true, + }) + } +} + // callModel performs one model call, forwarding assistant text to p as it // arrives. Providers without streaming fall back to Chat transparently; either // way the returned Response is the complete turn and the emitted text ends diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index b2b1780..6658a49 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -2,6 +2,8 @@ package agent import ( "context" + "encoding/json" + "errors" "fmt" "io" "iter" @@ -101,6 +103,7 @@ type chatOnly struct { chats int last provider.Request reply string + resp *provider.Response } func (p *chatOnly) Name() string { return "stub" } @@ -108,6 +111,9 @@ func (p *chatOnly) Name() string { return "stub" } func (p *chatOnly) Chat(_ context.Context, r provider.Request) (*provider.Response, error) { p.chats++ p.last = r + if p.resp != nil { + return p.resp, nil + } reply := p.reply if reply == "" { reply = "plain answer" @@ -117,6 +123,56 @@ func (p *chatOnly) Chat(_ context.Context, r provider.Request) (*provider.Respon }}, nil } +type testTool struct { + name string + run func(context.Context) (string, error) +} + +func (t testTool) Spec() tools.Spec { return tools.Spec{Name: t.name} } +func (t testTool) Run(ctx context.Context, _ json.RawMessage) (string, error) { + return t.run(ctx) +} + +func TestAgentCancellationRecordsSkippedToolResults(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + secondRuns := 0 + reg := tools.NewRegistry( + testTool{name: "first", run: func(context.Context) (string, error) { + cancel() + return "", context.Canceled + }}, + testTool{name: "second", run: func(context.Context) (string, error) { + secondRuns++ + return "unexpected", nil + }}, + ) + p := &chatOnly{resp: &provider.Response{Blocks: []provider.Block{ + {Kind: provider.BlockToolCall, ToolCall: &provider.ToolCall{ID: "call_1", Name: "first", Args: json.RawMessage(`{}`)}}, + {Kind: provider.BlockToolCall, ToolCall: &provider.ToolCall{ID: "call_2", Name: "second", Args: json.RawMessage(`{}`)}}, + }}} + sess := &Session{} + ag := &Agent{Provider: p, Tools: reg, Model: "m", MaxIterations: 3} + + if err := ag.Run(ctx, sess, "run both"); !errors.Is(err, context.Canceled) { + t.Fatalf("Run error = %v, want context.Canceled", err) + } + if secondRuns != 0 { + t.Fatalf("second tool ran %d times after cancellation", secondRuns) + } + if len(sess.Messages) != 4 { + t.Fatalf("messages = %+v", sess.Messages) + } + for i, id := range []string{"call_1", "call_2"} { + result := sess.Messages[i+2] + if result.Role != provider.RoleTool || result.ToolCallID != id || !result.IsError { + t.Fatalf("result %d = %+v", i, result) + } + } + if !strings.Contains(sess.Messages[3].Text, "skipped after cancellation") { + t.Fatalf("skipped result = %q", sess.Messages[3].Text) + } +} + func (p *chatOnly) Stream(context.Context, provider.Request) (iter.Seq[provider.Event], error) { return nil, provider.ErrStreamingUnsupported } diff --git a/internal/tools/tools_test.go b/internal/tools/tools_test.go index e10ee99..fe99ea3 100644 --- a/internal/tools/tools_test.go +++ b/internal/tools/tools_test.go @@ -92,6 +92,39 @@ func TestWriteCreatesDirsAndGates(t *testing.T) { } } +func TestWriteOverwritePreviewIsBounded(t *testing.T) { + dir := t.TempDir() + path := writeFile(t, dir, "large.txt", strings.Repeat("old line\n", 1024)) + const oldSize = int64(64 << 20) + if err := os.Truncate(path, oldSize); err != nil { + t.Fatal(err) + } + var approval string + approve := ApproverFunc(func(_, detail string) bool { + approval = detail + return false + }) + w := &writeTool{root: dir, approve: approve} + if _, err := run(t, w, `{"path":"large.txt","content":"replacement"}`); err == nil || !strings.Contains(err.Error(), "rejected") { + t.Fatalf("write error = %v", err) + } + if len(approval) > writeDiffDisplayLimit+300 { + t.Fatalf("approval preview is not bounded: %d bytes", len(approval)) + } + for _, want := range []string{"overwrite large.txt", "+ replacement", "diff preview truncated", "old=67108864 bytes"} { + if !strings.Contains(approval, want) { + t.Fatalf("approval missing %q: %q", want, approval) + } + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Size() != oldSize { + t.Fatalf("denied overwrite changed file: size=%d", info.Size()) + } +} + func TestReadOffsetLimit(t *testing.T) { dir := t.TempDir() writeFile(t, dir, "f.txt", "one\ntwo\nthree\nfour\n") diff --git a/internal/tools/write.go b/internal/tools/write.go index 6bb62ac..692d630 100644 --- a/internal/tools/write.go +++ b/internal/tools/write.go @@ -4,10 +4,14 @@ import ( "context" "encoding/json" "fmt" + "io" "os" "path/filepath" + "strings" ) +const writeDiffDisplayLimit = 4000 + type writeTool struct { root string approve Approver @@ -47,10 +51,12 @@ func (t *writeTool) Run(_ context.Context, raw json.RawMessage) (string, error) // overwrite — approving a byte count alone is approving blind. target := describeTarget(t.root, a.Path, full) action := "create " + target - detail := truncate(a.Content, 4000) - if old, err := os.ReadFile(full); err == nil { + detail := truncate(a.Content, writeDiffDisplayLimit) + if old, oldSize, err := readWritePreview(full, writeDiffDisplayLimit); err == nil { action = "overwrite " + target - detail = truncate(renderDiff(a.Path, string(old), a.Content), 4000) + detail = renderWriteDiffPreview(a.Path, string(old), oldSize, a.Content, writeDiffDisplayLimit) + } else if !os.IsNotExist(err) { + return "", err } if !t.approve.Approve(action, fmt.Sprintf("%s (%d bytes)\n%s", action, len(a.Content), detail)) { return "", fmt.Errorf("write rejected by user") @@ -68,3 +74,74 @@ func (t *writeTool) Run(_ context.Context, raw json.RawMessage) (string, error) } return fmt.Sprintf("Wrote %s (%d bytes).", a.Path, len(a.Content)), nil } + +func readWritePreview(path string, limit int) ([]byte, int64, error) { + f, err := os.Open(path) + if err != nil { + return nil, 0, err + } + defer f.Close() + info, err := f.Stat() + if err != nil { + return nil, 0, err + } + preview, err := io.ReadAll(io.LimitReader(f, int64(limit))) + return preview, info.Size(), err +} + +func renderWriteDiffPreview(path, old string, oldSize int64, new string, limit int) string { + header := fmt.Sprintf("--- %s\n+++ %s\n", path, path) + available := limit - len(header) + if available < 0 { + available = 0 + } + oldBudget := available / 2 + newBudget := available - oldBudget + oldPart, oldCut := renderDiffSection("- ", old, oldBudget) + newPart, newCut := renderDiffSection("+ ", new, newBudget) + truncated := oldCut || int64(len(old)) < oldSize || newCut + preview := header + oldPart + newPart + if truncated { + preview += fmt.Sprintf("\n… [diff preview truncated; old=%d bytes, new=%d bytes]", oldSize, len(new)) + } + return preview +} + +func renderDiffSection(prefix, content string, limit int) (string, bool) { + var b strings.Builder + b.Grow(limit) + for len(content) > 0 { + remaining := limit - b.Len() + if remaining <= len(prefix) { + return b.String(), true + } + // Search only the bytes that could fit. Indexing an unbounded line in + // a huge replacement would avoid allocations but could still stall. + lineBudget := remaining - len(prefix) - 1 + window := content + if len(window) > lineBudget+1 { + window = window[:lineBudget+1] + } + newline := strings.IndexByte(window, '\n') + if newline < 0 { + if len(content) > lineBudget { + b.WriteString(prefix) + b.WriteString(content[:remaining-len(prefix)]) + return b.String(), true + } + b.WriteString(prefix) + b.WriteString(content) + b.WriteByte('\n') + return b.String(), false + } + line, rest := content[:newline], content[newline+1:] + b.WriteString(prefix) + b.WriteString(line) + b.WriteByte('\n') + if rest == "" { + return b.String(), false + } + content = rest + } + return b.String(), false +}