Skip to content

fix(openai-agents): isolate config from generic OPENAI_* env vars - #206

Merged
dylanneve1 merged 6 commits into
mainfrom
fix/openai-agents-config-isolation
May 18, 2026
Merged

fix(openai-agents): isolate config from generic OPENAI_* env vars#206
dylanneve1 merged 6 commits into
mainfrom
fix/openai-agents-config-isolation

Conversation

@dylanneve1

Copy link
Copy Markdown
Owner

Summary

  • The openai-agents backend was silently overriding ~/.talon/config.json with OPENAI_API_KEY / OPENAI_BASE_URL / OPENAI_API_MODE from the shell. Operators who keep an OPENAI_API_KEY exported for unrelated tools were hitting 401s when pointing Talon at OpenRouter (their real OpenAI key was being sent to the wrong endpoint). Switched to a Talon-only env namespace β€” TALON_AGENTS_KEY, TALON_AGENTS_URL, TALON_AGENTS_API_MODE β€” that overrides config but doesn't shadow it via foreign vars.
  • prompts/base.md hard-coded Claude Code tool names (Read, Write, Edit, Bash, Glob, Grep) as if every backend exposed them. With openai-agents selected those tools aren't registered, so models like DeepSeek v4 Flash would dutifully emit Read calls and @openai/agents would reject the run with Tool Read not found in agent Talon. Rewrote the prompt to instruct the model to use only the tools listed for the current run.

Env precedence (after this PR)

knob precedence
key TALON_AGENTS_KEY > config.openaiApiKey
baseURL TALON_AGENTS_URL > config.openaiBaseUrl
apiMode TALON_AGENTS_API_MODE > config.openaiApiMode

The underlying openai SDK still reads OPENAI_API_KEY for its own default client, but Talon always injects an explicit client whenever any key or baseURL is configured, so that fallback can no longer mask config.

Test plan

  • npm run typecheck clean
  • npm run lint (pre-existing warnings only)
  • npx vitest run src/__tests__/openai-agents-*.test.ts β€” 36 tests pass, including new coverage that OPENAI_BASE_URL env is explicitly ignored when set alongside config
  • Manual: OPENAI_API_KEY=sk-proj-… (real OpenAI key) exported in shell + openaiApiKey=sk-or-v1-… in ~/.talon/config.json β†’ log line now reads key=config:openaiApiKey baseURL=config:openaiBaseUrl …, OpenRouter auth succeeds
  • Manual: confirmed bot starts and stays up against deepseek/deepseek-v4-flash:free via OpenRouter

πŸ€– Generated with Claude Code

The OpenAI Agents backend silently picked up `OPENAI_API_KEY`,
`OPENAI_BASE_URL`, and `OPENAI_API_MODE` from the environment and
preferred them over `~/.talon/config.json`. Operators who keep
`OPENAI_API_KEY` exported in their shell for unrelated tools were
hitting 401s when pointing Talon at OpenRouter or another
OpenAI-compatible endpoint via config β€” the shell's real-OpenAI key
was being sent to the wrong service.

Switch the backend to a Talon-only env namespace:

  key:      TALON_AGENTS_KEY      > config.openaiApiKey
  baseURL:  TALON_AGENTS_URL      > config.openaiBaseUrl
  apiMode:  TALON_AGENTS_API_MODE > config.openaiApiMode

The `OPENAI_*` vars are now ignored at this layer (the underlying
`openai` SDK still reads `OPENAI_API_KEY` for its own default client,
but Talon always injects an explicit client when any key/baseURL is
configured, so that fallback can no longer mask config). Tests
updated to cover both the new override and the explicit
non-override of `OPENAI_*`.

Also: rewrite `prompts/base.md` so it no longer hard-codes the
Claude-SDK tool names (`Read`, `Write`, `Edit`, `Bash`, `Glob`,
`Grep`) as if they were universally available. With the openai-agents
backend selected, those tools aren't registered, but the old prompt
still advertised them β€” models (e.g. DeepSeek v4 Flash via OpenRouter)
would dutifully call `Read` and the SDK would reject the run with
`Tool Read not found in agent Talon`. The new wording tells the model
to use only the tools listed for the current run.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@claudiusthebot claudiusthebot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM β€” approving. CI is 31/34 green; 3 Windows jobs still pending (slow infra, not related to the changes).

What's right here

src/backend/openai-agents/init.ts β€” The OPENAI_* β†’ TALON_AGENTS_* namespace switch is the correct design. The problem it solves is real: anyone using Talon alongside other OpenAI tooling (a shell alias, a Claude Code session, a local dev environment) will have OPENAI_API_KEY exported, and pointing Talon at OpenRouter while that key is live would cause silent auth failures at the endpoint. The Talon-specific namespace makes intent explicit and prevents this whole class of confusion.

The source-tracking in the log line (key=config:openaiApiKey baseURL=env:TALON_AGENTS_URL) is a nice touch β€” it makes debugging auth issues much faster.

prompts/base.md β€” The original text hard-coded Read / Write / Edit / Bash / Glob / Grep as "the tools you have" regardless of backend. That was fine when Claude SDK was the only backend; with openai-agents it would cause models to emit Read(path="...") calls that the SDK immediately rejects with Tool Read not found. The rewrite ("use only the tools registered for this run") is backend-agnostic and will remain correct as more backends get added. Good fix that should have shipped with #199.

src/__tests__/openai-agents-models.test.ts β€” The explicit "OPENAI_BASE_URL env is ignored" test case is exactly right. A test that checks the absence of leakage is more valuable than one that just tests the happy path.

Minor note

The package-lock.json changes remove "peer": true from a handful of packages (grammy, hono, zod, express, etc.). These were probably marked peer in an earlier npm version's lockfile format and npm 10 re-normalized them on npm install. Harmless, but worth being aware of if you need to bisect a lockfile-related issue later.

Migration note (for the record)

With this PR, the OpenRouter migration config for June 1 is:

{
  "backend": "openai-agents",
  "model": "meta-llama/llama-3.3-70b-instruct",
  "openaiBaseUrl": "https://openrouter.ai/api/v1",
  "openaiApiKey": "<or-key>"
}

…or via env: TALON_AGENTS_KEY=sk-or-v1-… TALON_AGENTS_URL=https://openrouter.ai/api/v1. Clean and unsurprising.

Ready to land once the 3 Windows jobs clear.

dylanneve1 and others added 5 commits May 18, 2026 12:09
`prompts/base.md` and `prompts/identity.md` are loaded on every
turn for every backend, but they hard-coded Claude-SDK tool names
(`Read`, `Write`, `Edit`, `Bash`, `Glob`, `Grep`) and imperative
instructions ("Read it to know who you are", "Write the answers
using the Write tool", "update your memory file using the Write
tool"). For backends without those tools β€” including openai-agents
talking to a non-OpenAI endpoint via OpenRouter β€” models would
faithfully emit `Read` / `Write` tool calls and `@openai/agents`
would reject the run with `Tool Read not found in agent Talon`,
visible to the user as the generic "Something went wrong. Try
again or /reset" friendly error.

Rewrite both files to:

- Describe capabilities abstractly ("persist when a filesystem-
  capable tool is available") instead of naming specific tools.
- Tell the model to use only the tools the runtime registers for
  this turn, and to fall back to plain conversation when a tool it
  wants doesn't exist β€” never invent tool names.
- Keep workspace paths (`~/.talon/workspace/...`) since those are
  data references, not tool names.

The backend-specific prompt suffixes (Telegram tools, OpenAI Agents
delivery suffix, etc.) continue to enumerate the real tools β€” the
shared prompts now defer to that list rather than asserting their
own.

Existing workspaces won't be re-seeded β€” operators upgrading need
to refresh `~/.talon/prompts/base.md` and `~/.talon/prompts/identity.md`
from the package (or delete the files and let them re-seed).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`initWithBase` was clearing OPENAI_API_KEY and OPENAI_API_MODE before
each call, and `afterEach` was saving/restoring them β€” both leftovers
from when Talon read those env vars. Now that Talon ignores them, the
handling is dead code. Keep OPENAI_BASE_URL save/restore because one
test still mutates it to prove Talon ignores it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Telegram and Discord `/restart` handlers spawned `talon restart`
(then fell back to `node bin/talon.js restart`), which called the
daemon's `daemonRestart()`. That path assumes the bot was launched
via `talon start` so a managed PID file exists; for anything else
(`npm start`, `npx tsx src/index.ts`, systemd, pm2, foreman, a
debugger) the daemon's stop+start logic doesn't apply and the
respawn was unreliable. Visible symptom: bot replies "Restarting..."
and never comes back.

Replace with `respawnSelf()` in `src/util/respawn.ts`, shared by
both frontends. It:

  1. Spawns the current Node binary with `process.execArgv` (so the
     tsx loader is preserved for .ts entrypoints) + the same
     argv[1..] + same cwd + env, detached + stdio:"ignore".
  2. On the child's `spawn` event, raises SIGTERM on self so the
     existing graceful-shutdown handler in `src/index.ts` runs
     (`frontend.stop()`, flush sessions/settings/history, unlink
     PID file). The previous implementation called
     `process.exit(0)` directly, which bypassed all of that and
     left state half-flushed.
  3. On spawn `error`, logs the failure and SIGTERMs anyway so an
     external supervisor can take over.

Telegram polling briefly overlaps between old and new (Telegram only
allows one long-poll per bot), but grammy retries on 409 and the
new child takes over within seconds.

Drop the now-unused `spawn` / `resolve` / `dirname` / `fileURLToPath`
imports from both frontends.

Also remove the dead OPENAI_BASE_URL save/restore from the
openai-agents model tests β€” it was retained for one test that
asserted Talon ignored OPENAI_BASE_URL, but with the env-var rename
that test no longer added coverage (Talon doesn't read the var
anywhere) so the test was removed in the previous commit and the
plumbing is now likewise dead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hment

Talon was built around the Claude SDK backend, which ships filesystem
and shell tools (`Read` / `Write` / `Edit` / `Bash` / `Glob` / `Grep`)
as part of Claude Code. The OpenAI Agents backend had no equivalent,
so models running through it (especially via OpenRouter) saw a
toolset limited to Telegram frontend tools + plugins β€” no way to
read identity.md, persist memory, run scripts, etc. The shared
system prompts (which now correctly avoid hard-coding tool names per
the previous commit) describe filesystem capabilities abstractly,
and this backend should actually provide them.

Adds `src/backend/openai-agents/builtins.ts` defining the six
Claude-Code-equivalent tools as plain `@openai/agents` function
tools with Zod schemas:

  - Read(file_path, offset?, limit?)  β€” cat -n-style output
  - Write(file_path, content)          β€” overwrite with mkdir -p
  - Edit(file_path, old, new, replace_all?)  β€” unique-match by default
  - Bash(command, description?, timeout_ms?)  β€” bash -lc, 30s default
  - Glob(pattern, path?)               β€” node:fs/promises#glob
  - Grep(pattern, path?, include?)     β€” prefers rg, falls back to grep

Wired into the agent constructor alongside the existing MCP servers.
Tool names + parameter shapes match Claude Code exactly so the
shared prompt vocabulary applies uniformly across backends.

Endpoint-advertised model catalog
─────────────────────────────────

/status used to render `Context 11.5k / 0 (0%)` for unknown models
because the openai-agents backend kept a hardcoded catalog of OpenAI
models only β€” anything else fell into a passthrough `UnifiedModelInfo`
with no `contextWindow`. Hard-coding per-provider catalogs doesn't
scale (OpenRouter alone has 350+ models).

Instead, fetch `<baseURL>/models` once at backend init and stash
whatever the endpoint advertises (id, display name, context length,
free-pricing flag) into `state.endpointModels`. `makePassthroughModel`,
`getProviderModels`, and `listModels` then consult that map so:

  - /status renders the real context window (e.g. `Context 11.5k / 128k`)
  - /settings model picker can paginate the full OpenRouter catalog
  - /model free filter actually returns the endpoint's free models

Designed to be tolerant: works against any OpenAI-compatible endpoint
that implements `GET /models` (OpenRouter exposes `context_length` +
`pricing`; vLLM exposes `context_length`; Ollama exposes neither β€” we
just record what's present). 10s timeout, fail-soft on errors (logged
at debug, doesn't block startup).

Decoupling: friendly error wording
──────────────────────────────────

The `auth` and `overloaded` user-facing strings called out Claude by
name ("check your Claude credentials", "Claude is busy right now").
Reworded to be backend-agnostic β€” now matches what the user actually
sees when running against OpenAI Agents / Codex / OpenCode / Kilo as
well.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI Code Quality reported formatting drift in the two new files added
in 263897e (`builtins.ts`, `init.ts`). No behavioral change β€” just
prettier --write output.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dylanneve1
dylanneve1 merged commit fc16cb2 into main May 18, 2026
34 checks passed
@dylanneve1
dylanneve1 deleted the fix/openai-agents-config-isolation branch May 18, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants