Skip to content

fix(opencode): normalize colliding fingerprint tool definitions in place - #4972

Closed
mostafa-binesh wants to merge 2 commits into
decolua:masterfrom
mostafa-binesh:fix/opencode-fingerprint-normalize-tool-names
Closed

mostafa-binesh wants to merge 2 commits into
decolua:masterfrom
mostafa-binesh:fix/opencode-fingerprint-normalize-tool-names

Conversation

@mostafa-binesh

Copy link
Copy Markdown
Contributor

Summary

Fixes #4971.

OpenCode builds its shell tool description from process.platform:

// packages/core/src/tool/plugin/shell.ts:28-46
const OS = process.platform === "darwin" ? "macOS"
  : process.platform === "win32" ? "Windows"
  : process.platform === "linux" ? "Linux" : process.platform
const description = (shell) => [
  "Execute a shell command and return its output.",
  ...(shell ? [`Commands run on ${OS} using ${shell}.`] : []),
  "Quote file paths containing spaces or special characters.", ...
].join(" ")

The fingerprint in open-sse/utils/opencodeFingerprint.js is a frozen capture of the Linux CLI (Commands run on Linux using bash.) and isFingerprintDefinition() compares with ===. So a caller shell from any non-Linux client never matches, and the pass treats a platform wording difference as a conflicting tool: it renames shell to shell_client, appends a placeholder, and injects "do not call the `shell` tool…" into the system prompt.

And because the alias is numbered off the caller's own names (taken), a client that already declares shell_client/shell_client2 pushes the alias to shell_client3 — four shell-ish names on the wire for one real tool, and a call to the advertised name can arrive at the client as a tool it does not have.

Change

concealFingerprintToolNames() now tries in-place normalization before falling back to the alias:

  • Compatible arguments (every caller parameter is a fingerprint parameter, and every required fingerprint parameter is declared) → replace description + parameters with the fingerprint's and keep the caller's name.
  • Different arguments → the alias + response-side restore path is untouched.

read still passes through untouched on every platform, and so does shell on Linux: both are byte-identical, so nothing is rewritten.

The guard is deliberately name-level. Overwriting parameters unconditionally would silently drop arguments the caller's handler expects — which is the reason the alias path exists at all:

function canNormalizeInPlace(tool, key) {
  const props = (chatFunctionOf(tool) || tool)?.parameters?.properties;
  if (!props || typeof props !== "object" || Array.isArray(props)) return false;
  const wanted = Object.keys(FINGERPRINT_TOOL_DEFINITIONS[key].parameters.properties || {});
  const declared = Object.keys(props);
  if (declared.some((name) => !wanted.includes(name))) return false;
  return FINGERPRINT_TOOL_DEFINITIONS[key].parameters.required.every((name) => declared.includes(name));
}

Result

For a real OpenCode client the wire is exactly what the gate asks for:

caller tools before after
read (exact), shell (Windows/pwsh) read, shell_client, shell + placeholder note read, shell — no alias, no map, no note
read, shell, shell_client, shell_client2 read, shell_client3, shell_client, shell_client2, shell read, shell, shell_client, shell_client2
caller shell declaring only {cmd} shell_client, read, shell + note unchanged (schema differs)

The placeholder note is not dead code: it still fires whenever a fingerprint name is genuinely absent and has to be appended.

Tests

Adds an opencodeFingerprint — in-place normalization block covering:

  • in-place adoption for both the flat ({name}) and nested ({function:{name}}) shapes
  • a real OpenCode tool set collapsing to exactly read, shell
  • the plugin alias collision (no shell_client3 fabricated)
  • three schema-incompatible shapes that must still conceal and still emit the note

All pre-existing assertions in tests/unit/opencode-fingerprint.test.js and tests/unit/opencode-free-tool-choice.test.js are unaffected — every existing aliasing case uses a tool with no parameters at all, which the compatibility guard rejects by design.

npx vitest run unit/opencode-fingerprint.test.js unit/opencode-free-tool-choice.test.js

Security notes

  • No new data enters the request: the pass only rewrites string fields already on the wire. The parameter guard reduces how much of a caller's schema can be replaced (previously any conflicting tool was renamed wholesale).
  • No auth, credential, header, or URL handling is touched; the alias map stays request-local in the existing WeakMap.
  • The in-place branch reuses the frozen FINGERPRINT_TOOL_DEFINITIONS object references exactly as appendMissingFingerprintTools() already does — no new mutation surface is introduced.
  • Behaviour for genuinely conflicting schemas is unchanged, so this cannot widen what a caller reaches upstream.

Docs

docs/ARCHITECTURE.md gains a Known Architectural Notes entry describing the pass and the frozen-capture platform caveat.

OpenCode composes its `shell` tool description from `process.platform`, so a
Windows or macOS client never matches the captured Linux definition. Every such
request was treated as a conflicting caller tool: `shell` was renamed to
`shell_client`, a placeholder `shell` was appended, and a system note told the
model to avoid it.

Besides leaking an internal rewrite into the prompt, the alias name is derived
from the caller's tool names, so a client that already declares `shell_client`
(a plugin, a harness shim) pushed the alias to `shell_client2`/3, and a call to
the advertised name could reach the client as a tool it does not have.

Adopt OpenCode's definition in place whenever the caller's parameters can carry
it — the platform-wording case, and the common one. Genuinely different schemas
keep the alias + restore path unchanged.

- `read` on every platform, and `shell` on Linux, stay byte-identical and are
  still passed straight through.
- Adds regression tests for platform drift, the plugin alias collision, and the
  schema-incompatible fallback.
yudhaharsanto added a commit to yudhaharsanto/9router that referenced this pull request Oct 17, 2026
Cherry-picked from decolua#4972.

Co-Authored-By: [CL] <noreply@anthropic.com>
leiblitz31 added a commit to leiblitz31/9router-go that referenced this pull request Oct 17, 2026
…t normalization, 403 direct replay

- internal/proxy/opencode.go: TranslateOpenCodeSessionID now generates fresh
  timestamped session IDs instead of hash-derived ones. The free-tier gate
  rejects non-recent timestamps with 403 FreeTierError.
- internal/translator/opencode_v2_fingerprint.go: port decolua/9router#4972 -
  normalize colliding fingerprint tool definitions in-place when arguments
  are compatible, instead of always aliasing to *_client.
- internal/proxy/proxy.go: port decolua/9router#4988 - replay proxied 403
  FreeTierError once on direct egress (unless strictProxy).
- internal/providers/capabilities.go: context window 1M for muse-spark and
  nemotron free models.
andyvandaric added a commit to andyvandaric/9router that referenced this pull request Oct 17, 2026
… strategy in full

Problem: the fork ran an older fingerprint contract than upstream frontier on the keyed lane and lacked both proxy escape hatches from issue decolua#4983. opencode-zen still sent the v1 UA plus a stub bash/glob/grep/read quartet with empty schemas and the v1-only x-opencode-request header, the fingerprint helper could not satisfy the gate when a caller tool reuses read/shell with platform-specific wording, and proxyAwareFetch had no way out of an egress the gate had flagged.

Changes: port upstream PR decolua#4972 (in-place fingerprint normalization with alias fallback), decolua#4883 (UA opencode/latest/2.0.29/cli across both executors, registry and dashboard test; strict v2 channel matcher kept), decolua#4860 fingerprint/strategy for opencode-zen (verbatim read/shell via applyFingerprintTools, cli client, four session mirrors, drop x-opencode-request and x-api-key for free tier; /zen/v1 to /inference/v1 switch NOT taken), decolua#4987 (open-sse/utils/proxyDispatcher.js with opt-in socks5 dns=local, shared by proxyFetch and the dashboard proxy test, fail-closed ERR_PROXY_DNS_MODE) and decolua#4988 (replay a proxied 403 FreeTierError once on the direct egress, strictProxy contract preserved). undici ^7.19.2 to ^7.30.1. Version 0.5.108.

Verification: full suite re-derived on this machine from a git stash -u baseline - failure sets byte-identical (95 = 95, 0 new), passes 3592 to 3617. Targeted opencode/proxy/session groups 214 pass. undici 7.30.1 exposes Socks5ProxyAgent and createSocks5Connection; the local-DNS dispatcher is exercised end to end against a local SOCKS5 plus TLS fixture (11 cases). Root cause of the free-tier 403 is egress-IP load-shedding (decolua#4983), so these ports restore request truth and add escape hatches but cannot make a flagged egress succeed.

@luqman-v1 luqman-v1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM: normalizes colliding OpenCode fingerprint tool definitions in place when arguments match, avoiding unnecessary tool aliasing. Clean unit tests.

@decolua

decolua commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Thanks @mostafa-binesh for the careful write-up — merged into master. 🙏

One thing worth flagging for the record: this had to be merged manually. The PR base is 87a74e1 (v0.5.104) and ce060c79b has since rewritten the same file to add the CLI <env> marker and the Claude /messages shape, so a plain cherry-pick conflicted. I resolved it by keeping all of master's newer work and layering your in-place normalization on top. One assertion in your suite expected body.instructions to be undefined; on current master the env marker always rides along on that field, so I widened it to check both halves.

Also noted in the commit message, as reviewers should know: normalizing in place replaces the caller's own shell description with OpenCode's, so safety instructions a client embeds there no longer reach the model, and a caller that declares a subset of the parameters gets its schema widened on the wire. Both follow from the upstream gate, and a tool declaring an unknown property still takes the alias path.

This supersedes #5157 (closed), which fixed only the powershell wording and left pwsh and macOS renamed.

@decolua decolua closed this Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants