provider: OpenRouter auto-router support + .ss write guard + /security
ober
3c08e485c1ed4d0cbf3eb550077c62a741fab44e
new file mode 100644 --- /dev/null +++ b/jcode-better.md @@ -0,0 +1,985 @@ +# jcode-better — implementation handoff + +**Audience:** a local LLM (or human) implementing changes in this repository +(`jerboa-code`, the `jcode` coding agent). +**Written:** 2026-07-24, against commit `ced05b0`. +**Status:** plan. Nothing below is implemented yet unless a later commit says so. + +This document is the complete brief. It contains: a review digest of what jcode +already does, the features to add (with exact config shapes, code anchors, and +test specs), security and performance recommendations, the mandatory +verification protocol, and a pitfalls catalog. Follow the work packages in +order. **Do not claim any work package is done until the full test suite +passes** (see §9 — "full test suite" means `make test`, not a subset). + +--- + +## 1. Ground rules for the implementer (MANDATORY) + +These come from `AGENTS.md` and are non-negotiable. Several exist because +previous sessions lost hours to mistakes they prevent in seconds. + +1. **NEVER edit `*.ss` or `*.sls` with raw `edit`/`write`/`sed`/`python`.** + Use the jerboa-mcp tools: + - Add a top-level form → `jerboa_balanced_insert` (anchor = one unique + complete form already in the file). + - Replace exact text → `jerboa_balanced_replace` (**dry-run by default**; + pass `dry_run: false` to write). + - New file → `jerboa_write_file` with `verify: true`. + - File already unbalanced → STOP. `git checkout -- <file>` and redo, or + `jerboa_repair_balance` (dry-run first). +2. **After EVERY `.ss` change, run `jerboa_check_balance`** (the balanced tools + do this automatically), then `jerboa_verify` on the file before building. +3. **Keep closer-runs ≤ ~4.** Flatten deep nesting with helper `def`s, + `let*`, or `cond =>`. +4. **No `(def ...)` after an expression in a body.** Internal defines come + first, or use `let`/`let*`. "invalid context for definition" means you + violated this — or a missing paren above glued two top-level forms + together (check balance first). +5. **Jerboa ≠ Gerbil/Racket.** Before using any function you are not 100% + sure of, check it with `jerboa_function_signature` or + `jerboa_module_exports`. Common hallucinations are listed in AGENTS.md + (`symbol<?`, `string-contains?`, `time->seconds`, `thread-sleep!`, 2-arg + `path-expand`, ... do not exist). +6. **Run the full test suite before calling anything done** (§9). If the + build behaves as if your edit didn't happen, delete stale artifacts: + `find lib -name "*.so" -delete && find lib -name "*.wpo" -delete && make build`. +7. **Minimal diffs.** Match the existing code style (see any file in + `src/jcode/`). Do not reformat, do not "improve" unrelated code. +8. **Docs in the same session.** Every user-visible change updates the + relevant file under `docs/` in the same work package. +9. **Do not commit** unless the user asks. When asked: clean `make binary` + (macOS) first. +10. **Never hardcode secrets.** The opencode reference config in §11 contains + redacted/placeholder keys — keep them placeholders. + +--- + +## 2. Repository review digest (what exists today) + +### 2.1 What jcode is + +A terminal coding agent (TUI + headless modes) written in Jerboa Scheme. +Source lives in `src/jcode/`: + +| Directory | Contents | +|---|---| +| `core/` | agent loop, config, sessions (SQLite+FTS5), models registry, compaction, permissions, sandbox, path-policy, guardrails support, verified workflows, hooks, plugins, secrets | +| `provider/` | `provider.ss` (3437 lines — all 14 providers), `sampling.ss` (per-model sampling card) | +| `tool/` | tool registry + 15 tool modules (file, bash, git, web, task, apply-patch, …) | +| `guardrails/` | response validator, rescue/repair for malformed tool calls, nudges, step enforcer, error tracker | +| `mcp/`, `proxy/`, `ui/`, `eval/` | MCP client, local proxy, TUI/serve interfaces, benchmarks | + +### 2.2 Feature history (from ~450 commits) + +Four investment themes dominate: + +1. **Local-model steering (the "smartness" work).** `docs/local-model-smartness-plan.md` + is the canonical plan; dozens of commits implement it: forced + `tool_choice` turns, bounded repair drafts, stale-catalog scrubbing, + retry nudges with concrete verifier output, `reasoning_effort` clamping + for local models, compact tool schemas, anti-pattern/recipe injection + via jerboa-mcp. Lesson encoded there: *models improve when the harness is + more deterministic, not when prompts are longer.* +2. **Security hardening waves.** P0 RCE fixes (hooks, plugin autoload, + config-trust), SSRF blocklists, proxy loopback+token auth, secret env + scrubbing, shell-injection fixes (argv spawn), `@file` credential leaks, + sandbox (Landlock/seccomp/Seatbelt, opt-in), path-policy with + `openat`/`O_NOFOLLOW`, sensitive-path denylist, config trust model + (`config-secure-ref` = fail-closed from trusted global config only). +3. **Provider breadth.** 14 providers on 4 wire formats; per-provider quirks + (OpenRouter credit-cap retry, Qwen `tool_choice`, Kimi empty content); + live model/pricing caches (`/refresh-models`); `{file:}` indirection for + API keys; opencode `auth.json` import. +4. **Sessions & UX.** SQLite session store with FTS5 search, resume-by-index, + checkpoints (`/undo`), repomap, TUI polish. + +### 2.3 Gaps this handoff closes + +| Gap | Evidence | Work package | +|---|---|---| +| No per-model/per-provider request-options pass-through, so OpenRouter routing knobs (auto-router plugins, provider sort/price, fallbacks) are unreachable | `apply-openai-provider-overrides!` only whitelists `reasoning_effort`/`verbosity` (`provider.ss:1173-1181`) | WP1 | +| Duplicated request-body builders (stream vs non-stream) — drift hazard for any new knob | `openai-body` (`provider.ss:1691`) vs `openai-stream-body` (`provider.ss:2429`) | WP0 | +| Native write tools can edit `.ss`/`.sls`, bypassing the mandated jerboa-mcp balanced tools | AGENTS.md rule is prompt-only; `handle-write` (`tool/file.ss:214`) has no scheme guard | WP3 | +| No runtime (on-the-fly) security console: tool ACL, loop kill-switch, MCP tool ACL, secret egress scan | `current-disabled-tools` exists (`tool/registry.ss:40`) but has no user surface; no loop detection; no egress scan | WP4, WP5 | +| Per-turn repeated O(history) walks (token estimation) and per-turn tool-catalog rebuilds | `estimate-message-tokens` (`core/compaction.ss:56`) called up to 4×/turn; `get-tool-schemas` (`tool/registry.ss:149`) rebuilds every turn | WP6 | +| Model capability metadata (`tool_call`, context/output limits) is hardcoded heuristics | `model-rejects-tools?` (`provider.ss:1733`), `model-context-window` (`core/models.ss:543`) | WP2 | + +--- + +## 3. The OpenRouter objective (what the user asked for) + +The user's opencode setup (`~/.config/opencode/opencode.json`) uses: + +- `"model": "openrouter/auto-beta"` — **Auto Router (Beta)**: OpenRouter's + server-side, prompt-aware model router. Per-request it picks a model from + an allow-list, biased by a cost/quality slider. +- `"small_model": "ds4-summary/deepseek-v4-flash"` — a cheap model for + summaries (see WP8, optional). +- The `openrouter/auto-beta` model entry carries **request options**: + +```json +"options": { + "plugins": [ + { "id": "auto-router", + "cost_quality_tradeoff": 10, + "allowed_models": ["deepseek/*", "moonshotai/*", "qwen/*", "minimax/*", + "bytedance-seed/*", "mistralai/*", "cohere/*", "inclusionai/*", + "poolside/*"] } + ] +} +``` + +The user wants jcode to reach the same OpenRouter knobs so the **cheapest +adequate model is used per task**, plus the plain `"openrouter/auto"` ("Auto +Router") entry. Relevant OpenRouter request-body fields (all pass-through — +jcode does not interpret them, it merges them verbatim): + +| Body field | Type | Meaning | +|---|---|---| +| `plugins` | array of objects | e.g. `{id: "auto-router", cost_quality_tradeoff, allowed_models}`; also `web`, `response-healing` | +| `provider` | object | upstream routing prefs: `sort` (`price`/`throughput`/`latency`), `order`, `only`, `ignore`, `allow_fallbacks`, `require_parameters`, `data_collection` (`allow`/`deny`), `quantizations`, `max_price` | +| `models` | array of strings | fallback model list (with `route: "fallback"`) | +| `route` | string | `"fallback"` when using `models` | +| `transforms` | array of strings | e.g. `["middle-out"]` | +| `reasoning` | object | `{effort}` etc. for reasoning-capable models | + +**jcode must treat these as opaque pass-through configuration** and not +validate beyond "is it a hash/list". The names inside belong to OpenRouter +and change over time. + +--- + +## 4. WP0 — Unify the two OpenAI body builders (preparatory) + +**Why first:** WP1 adds a body-rewriting hook. Doing it twice (and keeping it +in sync forever) is how regressions happen. The two builders already have +identical hook sequences (`provider.ss:1691-1706` vs `2429-2449`). + +**Change:** + +1. In `src/jcode/provider/provider.ss`, replace both `openai-body` and + `openai-stream-body` with a single builder: + +```scheme +(def (openai-request-body provider messages tools stream?) + (let ((body (make-hash-table))) + (hash-put! body "model" (provider-model provider)) + (when stream? (hash-put! body "stream" #t)) + (hash-put! body "max_tokens" (openai-max-tokens provider)) + (apply-sampling-to-body! body (provider-model provider) (provider-name provider)) + (apply-openai-provider-overrides! body provider) + (apply-local-verified-overrides! body provider) + (apply-forced-tool-template-overrides! body provider) + (maybe-apply-logprobs! body provider tools) + (apply-prompt-cache-controls! body provider) + (when stream? + (let ((opts (make-hash-table))) + (hash-put! opts "include_usage" #t) + (hash-put! body "stream_options" opts))) + (hash-put! body "messages" (map (lambda (m) (openai-message->json provider m)) messages)) + (when (and tools (not (null? tools)) + (model-supports-tools? provider)) ;; see WP2; until then keep model-rejects-tools? + (hash-put! body "tools" tools) + (hash-put! body "tool_choice" (openai-tool-choice provider))) + body)) +``` + + Keep the old names as 1-line wrappers so no caller changes: + `(def (openai-body p m t) (openai-request-body p m t #f))` and + `(def (openai-stream-body p m t) (openai-request-body p m t #t))`. + +2. Use `jerboa_balanced_insert` anchored on the full + `(def (openai-body provider messages tools) ...)` form to add the new + function; then `jerboa_balanced_replace` the two old bodies with the + wrappers. + +**Tests (add to `test/run.ss`, new section at the end before the summary — +style: `(section "=== openai body unify ===")` with `check!`/`check-pred!`):** + +- `(openai-body ...)` on a stub provider equals `(openai-stream-body ...)` + minus `stream`/`stream_options` keys (compare `hash-keys` sets and the + `model`/`max_tokens` values). Build the stub provider with the existing + test pattern (search `test/run.ss` for `make-provider` usages). + +**Acceptance:** `make test` passes; no behavioral change. + +--- + +## 5. WP1 — Provider/model `options` pass-through + OpenRouter routing knobs + +### 5.1 Config schema (new, additive) + +```json +{ + "provider": "openrouter", + "model": "openrouter/auto-beta", + "providers": { + "openrouter": { + "api_key": "{file:~/.keys/openrouter}", + "options": { + "provider": { "sort": "price", + "allow_fallbacks": true, + "data_collection": "deny" }, + "transforms": ["middle-out"] + }, + "max_price": { "prompt": 1.0, "completion": 4.0 }, + "models": { + "openrouter/auto": { + "name": "Auto Router", "tool_call": true, + "limit": { "context": 2000000, "output": 2000000 } + }, + "openrouter/auto-beta": { + "name": "Auto Router (Beta)", "tool_call": true, + "limit": { "context": 2000000, "output": 2000000 }, + "options": { + "plugins": [ + { "id": "auto-router", + "cost_quality_tradeoff": 10, + "allowed_models": ["deepseek/*", "moonshotai/*", "qwen/*", + "minimax/*", "bytedance-seed/*", "mistralai/*", + "cohere/*", "inclusionai/*", "poolside/*"] } + ] + } + } + } + } + } +} +``` + +### 5.2 Merge semantics (specify exactly; implement exactly) + +1. Start from the body built by `openai-request-body` (WP0). +2. **Gating.** Options are merged only when + `(equal? (provider-kind (provider-name provider)) "openrouter")` **or** + `providers.<name>.options_passthrough` is truthy. Rationale: strict + OpenAI-compatible endpoints (mistral, ollama `/v1`, several MaaS) return + 4xx on unknown body fields — see the comment at `provider.ss:1643-1646`. +3. **Provider level:** deep-merge `providers.<name>.options` into body. +4. **Model level:** deep-merge `providers.<name>.models.<model>.options` + into body (wins on conflicts). +5. **Deep-merge rule** (reuse the shape of `deep-merge!` in + `core/config.ss:64`): hashes merge recursively per key; **arrays and + scalars replace** (never concatenate). So model-level + `provider: {sort: "latency"}` replaces only `sort`, keeping provider-level + `allow_fallbacks`. +6. **Cost guardrail (global-only, always wins):** if + `(config-secure-ref "providers" "openrouter" "max_price")` is a hash, + ensure `body.provider.max_price` is set to it, **overriding** anything + from steps 3–4. It resolves via `config-secure-ref` on purpose: a cloned + repo's `./jcode.json` must not be able to remove the user's price ceiling + (the same trust hole as `pluginAllowWorkspace` — see + `core/config.ss:151-164`). When no routing object exists yet, create it. +7. **Reserved keys** are never treated as pass-through: at model level + `name`, `limit`, `tool_call`, `options`; at provider level everything + *inside* `options` is passed, but sibling keys (`api_key`, `base_url`, + `max_tokens`, `max_price`, `options_passthrough`, `wire`, `keep_alive`, + `models`) are not — only the contents of the `options` hash are merged. + +### 5.3 Code changes (all in `src/jcode/provider/provider.ss`) + +Insert after the `apply-prompt-cache-controls!` definition +(`provider.ss:1660-1666`) using `jerboa_balanced_insert` with that whole +form as anchor: + +```scheme +(def (options-passthrough-allowed? provider) + (let ((name (provider-name provider))) + (or (equal? (provider-kind name) "openrouter") + (config-ref "providers" name "options_passthrough")))) + +(def (deep-merge-options! dst src) + ;; hashes merge recursively; arrays/scalars replace (see jcode-better.md §5.2) + (hash-for-each + (lambda (k v) + (let ((old (hash-get dst k))) + (if (and (hash-table? old) (hash-table? v)) + (deep-merge-options! old v) + (hash-put! dst k v)))) + src) + dst) + +(def (apply-provider-options! body provider) + (when (options-passthrough-allowed? provider) + (let* ((name (provider-name provider)) + (model (provider-model provider)) + (p-opts (config-ref "providers" name "options")) + (m-opts (and model + (config-ref "providers" name "models" model "options")))) + (when (hash-table? p-opts) (deep-merge-options! body p-opts)) + (when (hash-table? m-opts) (deep-merge-options! body m-opts))))) + +(def (apply-openrouter-max-price! body provider) + ;; Global-only price ceiling; a project config cannot remove it. + (when (equal? (provider-kind (provider-name provider)) "openrouter") + (let ((cap (config-secure-ref "providers" "openrouter" "max_price"))) + (when (hash-table? cap) + (let ((routing (hash-get body "provider"))) + (unless (hash-table? routing) + (set! routing (make-hash-table)) + (hash-put! body "provider" routing)) + (hash-put! routing "max_price" cap)))))) +``` + +Then add one call line inside `openai-request-body` (WP0), immediately after +the `(apply-prompt-cache-controls! body provider)` line: + +```scheme + (apply-provider-options! body provider) + (apply-openrouter-max-price! body provider) +``` + +If WP0 was skipped (don't skip it), add the two lines to **both** +`openai-body` and `openai-stream-body`. + +`config-secure-ref` and `config-ref` are already imported in `provider.ss` +(verify with `grep` — if the import list lacks `:jcode/core/config`, add it). + +### 5.4 Registry entries + +In `src/jcode/core/models.ss`, add to the top of `openrouter-models` +(`models.ss:271`): + +```scheme + ("openrouter/auto" . "Auto Router") + ("openrouter/auto-beta" . "Auto Router (Beta)") +``` + +Note: users with an existing `~/.jcode/models-cache.json` see cache entries +first; `/refresh-models` repopulates. Mention in docs. + +### 5.5 Tests (append to `test/run.ss`) + +Use the existing mock-HTTP helpers (`serve-one-captured-json!`, +`serve-one-captured-sse!` — defined near `test/run.ss:301` and `:257`) which +capture the exact request body jcode sends. Existing provider tests show the +pattern; mirror them. Required cases: + +1. **Provider options land:** config with + `providers.openrouter.options = {"provider": {"sort": "price"}}`; drive a + non-streaming `openai-chat` against the mock; assert captured body has + `provider.sort == "price"`. +2. **Model options land:** model `openrouter/auto-beta` with the plugins + block from §5.1; assert captured body `plugins[0].id == "auto-router"`, + `cost_quality_tradeoff == 10`, `allowed_models` length 9. +3. **Model overrides provider (deep):** provider `options.provider = + {"sort":"price","allow_fallbacks":true}`, model `options.provider = + {"sort":"latency"}` → body `provider.sort == "latency"` AND + `provider.allow_fallbacks == true`. +4. **Arrays replace, not concat:** provider `transforms:["a"]`, model + `transforms:["b"]` → body `transforms == ["b"]`. +5. **Gating:** same options under provider `mistral` (no + `options_passthrough`) → captured body has NO `provider`/`plugins` keys. + With `"options_passthrough": true` → they appear. +6. **Cost guardrail:** global-config `max_price` + project-config + `options.provider.max_price` lower/higher → body carries the GLOBAL value. + Also: `max_price` present with no other routing options → body gets + `provider.max_price` and nothing else under `provider`. +7. **Stream path:** repeat case 2 through `openai-stream-chat` against + `serve-one-captured-sse!` (this is the path the TUI actually uses). +8. **No options configured:** captured body equals pre-WP1 shape (guard + against accidental key injection). +9. **402 retry still works:** existing credit-cap tests must pass unchanged + (they re-serialize the same body hash — confirm). + +For config setup in tests, follow the existing `with-temp-home` / +config-writing helpers at the top of `test/run.ss` (lines ~140-210). + +### 5.6 Docs + +Add an **"OpenRouter routing (auto router, fallbacks, price caps)"** section +to `docs/providers.md` after the prompt-caching section: the §5.1 config, +the pass-through field table (§3), the gating rule, `options_passthrough` +for other endpoints, and the global `max_price` guardrail with its trust +rationale. Note that `data_collection: "deny"` is the privacy knob and +`sort: "price"` is the "cheapest adequate model" knob. + +### 5.7 Acceptance + +All §5.5 tests + `make test` green; docs committed in the same change. + +--- + +## 6. WP2 — Routed-model surfacing + per-model capability metadata + +### 6.1 Surface which model the router actually used + +OpenRouter returns the resolved upstream model in the top-level `model` +field of each response/chunk. + +- Non-stream: in `openai-parse-response` (`provider.ss:1737`) — actually do + it in `openai-chat-with-stats` / `openai-extract-stats` where the full JSON + is available: read `(hash-get json "model")` and, when it differs from + `(provider-model provider)`, add `(cons 'routed-model <model>)` to the + usage/stats alist and `log-info` it. +- Stream: in `openai-stream-chat`'s chunk handling — capture the first + chunk's `model` field the same way. +- TUI: (stretch) show `auto → <routed-model>` in the status line. The usage + alist flows there already; a minimal version just logs. + +Tests: mock response JSON containing `"model": "deepseek/deepseek-v3.2"` +while request model was `openrouter/auto-beta`; assert `'routed-model` in +the stats alist. + +### 6.2 Per-model `tool_call` and `limit` metadata + +Mirror opencode's per-model fields so configs translate 1:1. + +1. New predicate in `provider.ss` (insert near `model-rejects-tools?`, + `provider.ss:1733`): + +```scheme +(def (model-supports-tools? provider) + (let ((flag (and (provider-model provider) + (config-ref "providers" (provider-name provider) + "models" (provider-model provider) "tool_call")))) + (and (not (eq? flag #f)) + (not (model-rejects-tools? (provider-model provider)))))) +``` + + Replace the three `(not (model-rejects-tools? ...))` call sites + (`openai-request-body` after WP0, plus `ollama-native-body` at + `provider.ss:2258`) with `(model-supports-tools? provider)`. + +2. Context/output limits: in `core/models.ss`, extend + `model-context-window` (`models.ss:543`) to first consult config: + +```scheme +(def (configured-model-limit model-id key) + ;; scan providers.*.models.<id>.limit.<key>; first hit wins + (let ((providers (config-ref "providers"))) + ...)) +``` + + then fall back to the existing heuristics, plus a new heuristic: + `((string-prefix? mid "openrouter/auto") 200000)` (conservative; routers + advertise 2M but routed models vary). Output-limit config is advisory + only — wire it into `openai-max-tokens` (`provider.ss:1091`) as another + `configured` candidate AFTER the explicit `max_tokens` lookups: + `(config-ref "providers" name "models" model "limit" "output")`. + +Tests: `tool_call: false` → request body has no `tools`/`tool_choice` even +when tools are passed; `limit.context: 12345` → `(model-context-window id)` +is 12345; `limit.output` flows into `max_tokens` when nothing higher +priority is set. Keep the `deepseek-reasoner` behavior intact +(`model-rejects-tools?` still applies). + +### 6.3 Docs + +Document `models.<id>.tool_call` / `.limit` / `.options` in +`docs/providers.md` (models section). + +### 6.4 Acceptance + +`make test` green including new tests; old `reasoner` test still green. + +--- + +## 7. WP3 — `.ss`/`.sls` write-tool guard (force jerboa-mcp balanced tools) + +**Goal:** the AGENTS.md rule ("NEVER edit `.ss`/`.sls` with raw tools; use +the jerboa-mcp balanced tools") becomes enforced by the tool layer, exactly +like opencode's write blocking. A model that tries native +`write`/`edit`/`multi-edit`/`patch`/`edit_block`/`apply_patch` on a `.ss` or +`.sls` path gets a refusal that names the correct MCP tools. + +This is a **workflow/quality guard, not a security boundary** (bash remains +a bypass; that's accepted). Resolve its config with plain `config-ref`, NOT +`config-secure-ref`. + +### 7.1 Behavior spec + +- Trigger: path (workspace-relative or absolute) ends in `.ss` or `.sls` + (case-sensitive; keep scope tight — no `.scm`, `.sld`, etc.). +- Default: **ON**. Disable via `"tools": {"scheme_write_guard": false}` or + env `JCODE_SCHEME_WRITE_GUARD=0`. +- Allow-list: `"tools": {"scheme_write_guard_allow": ["eval/**", + "test/fixtures/**"]}` — workspace-relative globs matched with the same + tiny glob engine as permissions (`glob-style-match`, + `core/permissions.ss:87`). You will need to export `glob-style-match` + from `permissions.ss` (add it to the `export` form) and import it in + `file.ss` / `apply-patch.ss`. +- Refusal message (returned as the tool result string, matching the style + of `sensitive-path-error` at `file.ss:144`): + +``` +Error: write refused — direct writes to Scheme source (.ss/.sls) are disabled in this workspace (scheme_write_guard). Use the jerboa MCP balanced tools instead: jerboa_balanced_insert (add a top-level form), jerboa_balanced_replace (exact text; dry_run:false to write), jerboa_write_file (whole new file; verify:true), jerboa_repair_balance (fix paren imbalance). To allow this path, add it to tools.scheme_write_guard_allow in jcode.json; to disable the guard entirely set tools.scheme_write_guard=false or JCODE_SCHEME_WRITE_GUARD=0. +``` + + (Swap `write` for the actual tool name. Concrete tool names in the error + are deliberate — local models obey concrete instructions, per + `docs/local-model-smartness-plan.md`.) + +### 7.2 Code changes + +`src/jcode/tool/file.ss`: + +1. Add near `generated-artifact-error` (`file.ss:200`): + +```scheme +(def (scheme-source-path? path) + (and (string? path) + (or (string-suffix? ".ss" path) + (string-suffix? ".sls" path)) + #t)) + +(def (scheme-write-guard-enabled?) + (let ((env (getenv "JCODE_SCHEME_WRITE_GUARD"))) + (if (and env (member env '("0" "false" "off"))) + #f + (let ((cfg (config-ref "tools" "scheme_write_guard"))) + (not (eq? cfg #f)))))) ;; default ON + +(def (scheme-write-guard-allowed? path) + (let ((globs (config-ref "tools" "scheme_write_guard_allow")) + (rel (workspace-relative-path path))) + (and (list? globs) + (string? rel) + (let loop ((gs globs)) + (cond ((null? gs) #f) + ((and (string? (car gs)) + (glob-style-match (car gs) rel)) #t) + (else (loop (cdr gs)))))))) + +(def (scheme-write-error tool path) + (and (scheme-source-path? path) + (scheme-write-guard-enabled?) + (not (scheme-write-guard-allowed? path)) + (format "Error: ~a refused — ..." tool path))) ;; §7.1 message +``` + +2. In the `cond` chains of `handle-write` (`file.ss:220`), `handle-edit` + (`file.ss:239`), `handle-multi-edit` (`:455`), `handle-patch` (`:518`), + `handle-edit_block` (`:283`), insert **after** the + `generated-artifact-error` clause and **before** `path-policy-error`: + +```scheme + ((scheme-write-error "<tool>" path) => (lambda (e) e)) +``` + +3. `src/jcode/tool/apply-patch.ss`: same insertion in its handler (its + policy check is at `apply-patch.ss:186`). Note `apply_patch` accepts + multiple file ops — run the guard per target path. +4. Update the `register-tool!` descriptions for `write`, `edit`, + `multi-edit`, `patch`, `edit_block` (`file.ss:27-58`) and `apply_patch` + to append: `"NOTE: .ss/.sls files are guarded — use the jerboa MCP + balanced tools (jerboa_balanced_insert / jerboa_balanced_replace / + jerboa_write_file) for those paths."` This upfront steering saves wasted + turns. +5. `config-ref` import: `file.ss` already imports config? Verify; add + `:jcode/core/config` and `:jcode/core/permissions` to its import form if + missing. `apply-patch.ss` likewise. + +### 7.3 Tests (`test/run.ss`) + +1. `write` on `foo.ss` → result starts with `"Error: write refused"` and + contains `"jerboa_balanced_insert"`. Same for `.sls`. +2. `write` on `foo.py` succeeds (temp dir, existing helpers). +3. Guard disabled via config (`scheme_write_guard: #f`) → `.ss` write + succeeds. +4. Allow glob: `scheme_write_guard_allow: ["eval/**"]`, write to + `eval/tmp/x.ss` → succeeds; write to `src/x.ss` → refused. +5. `multi-edit`, `edit_block`, `patch`, `apply_patch` on `.ss` → refused. +6. `read` on `.ss` still works (guard is write-only). +7. Existing tests that write `.ss` fixtures through the tools must keep + passing — if any trip the guard, they were relying on the loophole; + switch them to the allow-list in test config (do NOT weaken the guard). + (`write-test-output-file` at `test/run.ss:125` bypasses the tools and is + unaffected.) + +### 7.4 Docs + +`docs/tools.md`: new subsection under the file-tools safety model: +behavior, defaults, opt-outs, the MCP tool names, and a note that this +enforces the repo's AGENTS.md policy. Mention the recommended bash-side +hardening is the existing `permissions.deny` (e.g. `"sed -i *"`). + +### 7.5 Acceptance + +`make test` green with the new tests. + +--- + +## 8. WP4 + WP5 — On-the-fly security controls + +The user asked for better runtime control over what a model may do. +Current layers (keep them all): PLAN/BUILD mode, `permissions` bash rules, +sandbox (opt-in), path-policy, write scopes, sensitive denylist, +generated-artifact guard, plugin/hook/config-trust gates, proxy auth, SSRF +blocklist, turn-scoped tool narrowing (guardrails). The additions below are +ordered by value/effort. Each is independently shippable; run `make test` +after each. + +### WP4a — `/security` posture command (TUI) + +New slash command printing the *effective* posture: mode, sandbox +enabled/platform, permissions rule counts (allow/prompt/deny), disabled +tools, active write scope, MCP servers + tool counts, scheme_write_guard +state, secret-redaction state, OpenRouter price cap presence. Follow the +existing `/agents` command pattern in `src/jcode/ui/tui.ss` (see +`docs/AGENTS.md` for the command list; `/agents` implementation is the +template). Pure read, no behavior change. Test: command produces output +containing `sandbox:` and `mode:` lines (TUI command tests exist — mirror +one). + +### WP4b — Runtime tool ACL (`/tools`, `/tool off`, `/tool on`) + +- Add `*user-disabled-tools*` `(make-parameter '())` in + `tool/registry.ss`; extend `tool-disabled?` (`registry.ss:57`) to also + consult it. +- TUI commands: `/tools` (list enabled), `/tool off bash`, `/tool on bash`. + Session-only — **never persisted** (persistence would let a session + plant config; document this decision). +- Interaction note for the code comment: guardrails' per-turn + `current-tool-allowlist` still applies underneath; user disables only + narrow further. +- Tests: after `(user-disable-tool! "bash")`, `tool-execute "bash"` + returns the unavailable message; re-enable restores. Keep functions + (`user-disable-tool!`, `user-enable-tool!`, `user-tools-status`) exported + and pure-ish so they're testable without the TUI. + +### WP4c — MCP tool ACL at connect time + +- Config: `"mcpServers": { "jerboa": { "tool_deny": ["*_eval*"], + "tool_allow": ["jerboa_verify", "jerboa_eval"] } }` (deny checked first; + globs via `glob-style-match`). +- Apply in `src/jcode/mcp/` where tools are registered (find the + `register-tool!` call for MCP tools; `set-tool-origin!` at + `registry.ss:77` shows the area): skip registration for denied tools, or + `set-tool-internal!` them (internal = hidden from the LLM but callable — + choose **skip registration** for deny so even internal callers can't + reach them; document). +- Trust: global-config `tool_deny` **always** applies + (`config-global-ref`); project-config `tool_allow` applies only under + `trust-project-config?` (a cloned repo must not widen a user's deny + list — same rationale as `config.ss:166-175`; use `config-secure-ref` + for the deny list and plain `config-ref` for allow intersected with + trust). +- Tests: stub MCP registration with a fake tool list; assert denied names + are absent from `list-tools`. + +### WP4d — Loop kill-switch (doom-loop detector) + +- In `tool-execute` (`registry.ss:117`), track `(cons name + canonical-args-json)` per turn in a parameter + (`*turn-tool-calls*`). Same pair seen: 3rd time → execute but append + `"⚠ loop-detect: you have made this exact call 3 times; change approach + or report blockage."` to the result; 5th time → do not execute, return + refusal string. +- Turn boundary: reset when a new provider request starts — hook where the + agent loop dispatches tool calls (`core/agent.ss`, tool-result creation + around lines 1501-1749). Export `reset-turn-tool-calls!` from the + registry and call it from the agent loop's per-turn entry (find where + `provider-stream-chat` is invoked per turn). +- Config: `"guardrails": {"loop_detect": true}` (default true), + `JCODE_LOOP_DETECT=0` to disable. Canonicalization: args hash → + `json-object->string` (key order is stable from parsed JSON; good enough + — comment this). +- Tests: register a stub tool, call 5× identical → 5th is the refusal and + the handler ran only 4 times; different args unaffected; reset works. + +### WP5 — Secret egress redaction (best-effort, labeled) + +- New `core/redact.ss`: `(redact-secrets s)` replaces matches of common + token shapes with `[redacted]`: `sk-or-[A-Za-z0-9-]{8,}`, `sk-ant-…`, + `sk-[A-Za-z0-9]{20,}`, `AIza[A-Za-z0-9_-]{10,}`, `ghp_[A-Za-z0-9]{20,}`, + `github_pat_[A-Za-z0-9_]{20,}`, `xox[baprs]-[A-Za-z0-9-]{10,}`, + `-----BEGIN [A-Z ]*PRIVATE KEY-----` blocks, and `<PROVIDER>_API_KEY=<value>` + assignments. Implement with `std text regex` (check + `jerboa_module_exports` for the exact API first). +- Apply in `core/agent.ss` where tool results become messages (around + lines 1501/1627/1731-1749): `(make-tool-result id (maybe-redact result))`. + Default ON; `"guardrails": {"secret_redact": false}` disables; log-warn + once per session when a redaction fires (no content in the log). +- Scope honesty: this is defense-in-depth on top of the sensitive-path + denylist (`.env` reads are allowed in some configs — that's why this + exists). It cannot catch base64/rot13 smuggling; say so in docs. +- Tests: result containing `sk-or-v1-abc123...` → redacted in the stored + message; normal code text untouched; disable flag works. + +### Docs for WP4/WP5 + +`docs/tools.md` (safety model) + `docs/cli.md` (slash commands) + +`docs/threat-model.md` (one paragraph each: what each control does and does +NOT defend against). + +--- + +## 9. WP6 — Performance improvements (measure-first) + +Methodology requirement: **before** changing anything, capture a baseline. +Add `test/bench-turn.ss` (standalone, NOT wired into `make test`): build a +synthetic 600-message session (~2 KB average content, 30 tool calls) and +time (a) `should-compact-for-budget?` + one `make-tiered` pass, (b) +`get-tool-schemas` with ~90 registered tools, 1000 iterations each, report +ms. Run via the same `$(JEXEC)` the Makefile uses. Record numbers in the +commit message; each item below must show a win on this bench (or a real +profile) — no speculative "optimizations". + +1. **Token-estimation memoization.** `estimate-message-tokens` + (`compaction.ss:56`) re-walks the full history every call; the tiered + strategy calls it up to 3 more times per turn + (`compaction-strategy.ss:158-170`). Messages are treated as immutable + (new records are made rather than mutated — verify and comment this + assumption). Add a small `eq?`-keyed hash cache message→byte-size, plus + `invalidate-estimation-cache!` called on session load and after + compaction rewrites history. Acceptance: identical compaction decisions + (existing tests pass), bench (a) ≥ 5× faster on repeat turns. +2. **Tool-catalog build cache.** `get-tool-schemas` (`registry.ss:149`) + rebuilds the full mapped list every turn (with jerboa-mcp attached this + is ~90 tools). Add a catalog version counter bumped in + `register-tool!`, `register-internal-tool!`, `set-tool-internal!`, + `user-disable/enable` (WP4b), and when mode/allowlist parameters change + (wrap the parameters or set the counter at the same call sites — find + all writers of `current-tool-allowlist`/`current-disabled-tools` with + `grep`). Cache `(list version schemas)`; rebuild only on version + change. Acceptance: byte-identical schema output across a no-change + boundary (test), bench (b) ≥ 10× on cache hits. +3. **Model-search haystack cache.** `filter-models` + (`models.ss:156-172`) downcases/formats every entry per keystroke (~300 + entries after `/refresh-models`). Cache the downcased haystack alongside + each entry when the cache is loaded/written (`load-models-cache!`, + `write-models-cache!`) and for the builtin lists (compute once at + startup). Trivial; skip the bench requirement for this one. +4. **(Stretch, profile first) message-JSON prefix reuse.** + `openai-message->json` re-serializes the whole history per request. + Only attempt if a profile shows it matters against local servers; cache + per-message serialized JSON by `eq?` identity. Same immutability + assumption as (1) — share the cache infrastructure if both land. + +**Do not touch** the SSE line parser, the HTTP layer, or the TUI renderer +without a profile showing them hot. + +--- + +## 10. WP7/WP8 — Optional follow-ons (only after WP0–WP6 are green) + +- **WP7 — Split `provider.ss`.** 3437 lines is hostile to local models + (every read costs context; every edit risks the wrong anchor). Mechanical + split into `provider/http.ss` (transport+retry), `provider/openai.ss` + (chat-completions incl. stream), `provider/anthropic.ss`, + `provider/google.ss`, `provider/ollama.ss`, `provider/grok.ss`, + `provider/provider.ss` (struct + dispatch + quirks). One move per commit, + exports unchanged, `make test` after each move. No behavior change. +- **WP8 — `small_model` support (opencode parity).** opencode routes + summaries to a cheap model (`"small_model": + "ds4-summary/deepseek-v4-flash"`). jcode's compaction summarizer + (`ui/tui.ss:2057-2091`) uses the session model. Add config + `"small_model": {"provider": ..., "model": ...}` used by the + compaction/summarization call site when present (fall back to session + model). Investigate first: confirm the summarizer is the only consumer, + keep the change to one call site, test with a mock provider. +- **WP9 — Agent-model routing via Auto Router.** The `agents` block + (docs/AGENTS.md) routes roles to models. With WP1 done, an agent can + point at `openrouter/auto-beta` with a *different* `options` profile + (e.g. delegates → `cost_quality_tradeoff: 10`, implementers → `3`). + This works via `agents.<name>.provider/model` + per-model `options` + without new code — but only if options are looked up per *model*, which + WP1 does. Add a docs example only; if agent-level option overrides are + wanted, add `agents.<name>.options` merged last (small change, separate + commit, tests). + +--- + +## 11. Reference: opencode → jcode config translation + +The user's working opencode config (`~/.config/opencode/opencode.json`) +translates to this `~/.jcode/config.json` once WP1/WP2 land (keys redacted — +use `{file:}` indirection or the encrypted store, `jcode keys import`): + +```json +{ + "provider": "openrouter", + "model": "openrouter/auto-beta", + "providers": { + "openrouter": { + "options": { + "provider": { "sort": "price", + "allow_fallbacks": true, + "data_collection": "deny" } + }, + "max_price": { "prompt": 2.0, "completion": 8.0 }, + "models": { + "openrouter/auto": { + "name": "Auto Router", "tool_call": true, + "limit": { "context": 2000000, "output": 2000000 } + }, + "openrouter/auto-beta": { + "name": "Auto Router (Beta)", "tool_call": true, + "limit": { "context": 2000000, "output": 2000000 }, + "options": { + "plugins": [ + { "id": "auto-router", + "cost_quality_tradeoff": 10, + "allowed_models": ["deepseek/*", "moonshotai/*", "qwen/*", + "minimax/*", "bytedance-seed/*", "mistralai/*", + "cohere/*", "inclusionai/*", "poolside/*"] } + ] + } + } + } + }, + "ds4-summary": { + "base_url": "http://10.0.0.4:8000/v1", + "api_key": "dsv4-local", + "options_passthrough": false, + "models": { + "deepseek-v4-flash": { + "name": "DeepSeek V4 Flash summary text-only", + "tool_call": false, + "limit": { "context": 100000, "output": 8192 } + } + } + } + }, + "small_model": { "provider": "ds4-summary", "model": "deepseek-v4-flash" }, + "tools": { "scheme_write_guard": true }, + "guardrails": { "loop_detect": true, "secret_redact": true }, + "sandbox": { "enabled": true, "allow_network": false } +} +``` + +Notes: +- `ds4-summary` is a custom OpenAI-compatible endpoint; jcode already + supports arbitrary provider names (models.ss `provider-kind` falls back + to the name itself — it dispatches via the OpenAI path only if kind maps + there. CHECK during WP2: unknown provider names currently keep their own + name as kind, which is NOT in the OpenAI dispatch case list at + `provider.ss:2907`. opencode solves this with `npm: + "@ai-sdk/openai-compatible"`. **Add to WP2:** treat + `providers.<name>.wire == "openai"` (or `options_passthrough`-style + `"compat": "openai"`) as mapping `provider-kind` to `"openai"` for + dispatch — small change in `models.ss provider-kind` + tests. Without + this, custom endpoints like `ds4-summary` cannot be used at all.) + Decide the exact key (`"wire"` already exists for ollama at + `provider.ss:2185` — reuse it: `(wire == "openai") → kind "openai"`). +- `max_price` units: dollars per 1M tokens, matching OpenRouter's + `provider.max_price` shape (`{prompt, completion}`). Pass through + verbatim; document that OpenRouter interprets them. + +--- + +## 12. Verification protocol (the definition of "done") + +Per work package, and again at the end: + +1. `jerboa_verify` (MCP) on every `.ss` file you touched → no syntax, + expand, arity, or duplicate-definition errors. +2. `make build` → clean. If edits seem ignored: + `find lib -name "*.so" -delete && find lib -name "*.wpo" -delete && make build`. +3. **Full test suite — `make test`** → green. This runs, in order: + `build`, `test-websearch-worker` (`websearch-worker-failclosed.ss` + + `websearch-search-limits.ss`), `test-tui-native-loader-security` + (shell script), `test/security-regression.sh` → + `test/security-regression.ss`, then the main suite `test/run.ss` + (~14k lines). All must pass. A subset run is NOT sufficient evidence. +4. New tests you added → present in `test/run.ss` output (grep the run + log for your section names). +5. `make fuzz` (harness fuzzer, `test/fuzz.ss`) → no new crashes. + `JCODE_FUZZ_ITERS` controls depth; default run is fine. +6. Behavior check by hand for the headline feature, e.g. after WP1: + `JCODE_TRACE`-style tracing already logs redacted request bodies + (`log-trace "openai-request"` in `openai-post-json`, + `provider.ss:1146`) — run one real (or mock) turn and confirm the + `plugins`/`provider` fields appear exactly once in the body. +7. Docs updated in the same session: `docs/providers.md` (WP1/WP2), + `docs/tools.md` + `docs/cli.md` + `docs/threat-model.md` (WP3/WP4/WP5), + and this file's checkboxes if you keep it. +8. Report format per WP: what changed (files), test evidence (the suite's + final line from `test/run.ss` — it prints `N passed, N failed, N skipped` + and exits 1 on any failure; your report must show `0 failed`), bench + numbers (WP6 only), follow-ups discovered. + +If `make test` fails for a pre-existing reason on a clean checkout, capture +evidence (`git stash && make test`) and report it rather than working +around it silently. + +--- + +## 13. Pitfalls catalog (found during this review — read before coding) + +1. **Two body builders.** WP0 exists precisely because of this. If you add + a knob to one builder only, the TUI (streaming) and headless + (non-streaming) paths diverge. After WP0 there is one builder; keep it + that way. +2. **Name collision:** OpenRouter's routing object is called `provider`, + same word as jcode's `provider-record`. In code, name local things + `routing`/`routing-opts` (as in the §5.3 sketch) so nobody confuses + `(provider-name provider)` with `(hash-get body "provider")`. +3. **Strict endpoints 4xx on unknown fields** (mistral, ollama `/v1`). The + gating rule (§5.2 step 2) is load-bearing. Do not "simplify" it away; + the `prompt_cache_key` precedent exists for exactly this reason + (`provider.ss:1643-1650`). +4. **Config deep-merge replaces non-hash values** (`config.ss:64-74`). + Your runtime options merge (§5.2) is a *different* function from config + load merge — implement `deep-merge-options!` separately; do not call + `deep-merge!` on live config (it mutates the config hash!). +5. **`max_price` trust:** must resolve via `config-secure-ref` + (global-only unless the user opted into project trust). Resolving it + via `config-ref` would let a cloned repo remove your price ceiling. +6. **402 credit retry re-serializes the same body hash** + (`openai-post-json`, `provider.ss:1139-1171`). Options applied during + body construction survive retries automatically — but only if you apply + them in the builder, not by mutating the serialized string later. +7. **`openrouter-qwen-model?` `tool_choice` quirk** + (`provider.ss:1668-1680`): auto-router may route to a Qwen upstream. + Default `tool_choice` is `"auto"`, which is already safe. Do not force + object tool_choice when the model id is a router id. +8. **`model-rejects-tools?` is a substring hack** ("reasoner"). WP2's + `model-supports-tools?` layers config on top — keep the old predicate + as fallback; don't delete it (tests depend on it). +9. **`test/run.ss` is a monolith.** Append a new `(section ...)` block + immediately before the `;; ── Results ──` marker at the end of the file + (the summary prints `N passed, N failed, N skipped` and exits 1 on any + failure); reuse `check!`, `check-pred!`, `skip!`, `with-temp-home`, and + the `serve-one-*` mock servers. Do not reorder or renumber existing + tests. Do not let your new section depend on network. +10. **Models cache wins over the builtin registry** + (`models.ss:148-154`). New builtin registry entries (§5.4) won't + appear for users with an existing cache until `/refresh-models`. + That's expected — document it, don't "fix" it. +11. **WP3 will block your own instincts.** Once the scheme guard is on, + native `write`/`edit` on `.ss` fails — including in eval/benchmark + workflows that write solution files. Ship WP3 with its allow-list and + update any in-repo workflows (check `eval/`, `test/fixtures`) to use + it, or run those flows with the guard disabled via config. +12. **Runtime ACL is session-only by design (WP4b).** Do not persist + disabled tools into config files — a session writing security config + is a self-escalation vector. +13. **Don't scan logs with secret content.** WP5 redaction happens before + messages are stored; make sure `log-trace` of tool results + (`agent.ss:1742`) runs AFTER redaction or not at all on unredacted + text. +14. **Unknown-provider dispatch (WP11 note in §11):** custom provider + names keep their own `provider-kind`, which is not in the + `stream-dispatch` case list (`provider.ss:2906-2926`). The `"wire"` + mapping must be implemented for `ds4-summary`-style endpoints or + `small_model` (WP8) will error with "Unknown provider". +15. **Compaction estimation immutability assumption (WP6.1):** if any code + path mutates a message in place (grep for `message-content-set!`-style + mutators before relying on this), the `eq?` cache breaks. Verify + first; if mutation exists, invalidate at those call sites instead. + +--- + +## 14. Suggested commit plan + +One commit per work package (WP0 … WP6 at minimum), each with `make test` +green at that point: + +1. `refactor: unify openai request body builders (WP0)` +2. `provider: per-provider/model options pass-through + OpenRouter routing knobs (WP1)` +3. `provider: routed-model surfacing + per-model tool_call/limit metadata + wire dispatch (WP2)` +4. `tools: scheme_write_guard for .ss/.sls — enforce jerboa-mcp balanced edits (WP3)` +5. `security: /security posture + runtime tool ACL + MCP tool ACL (WP4a-c)` +6. `security: loop kill-switch + secret egress redaction (WP4d, WP5)` +7. `perf: estimation memo, catalog cache, model-search haystacks (WP6)` +8. `docs: sync providers/tools/cli/threat-model for WP1-WP6` + +(Prompt for the human: review each diff; WP5's redaction and WP4c's MCP +ACL are the two highest-risk items — read those closely.) + +--- + +*End of handoff. When in doubt: smaller diff, run the suite, ask.* --- a/src/jcode/core/models.ss +++ b/src/jcode/core/models.ss @@ -82,9 +82,10 @@ (def (provider-kind p) "Return the built-in provider behavior used by provider name P. Names like mlx2 are treated as MLX aliases so multiple local/remote MLX servers can be - configured independently." + configured independently. Custom OpenAI-compatible endpoints can be declared + with providers.<name>.wire = \"openai\"." (cond - ((equal? p "alibaba") "openai") ;; alibaba MaaS is OpenAI-compatible + ((equal? p "alibaba") "openai") ((member p *builtin-providers*) p)