OpenAI shipped GPT Image 2.5 on 2026-09-08 as two model IDs rather than one:
flare is tuned for speed, sunburst for editing precision. Both accept text and
image input and run on /images/generations, /images/edits and the Responses
API, which is the path the native image_generation tool already uses.
Flare replaces gpt-image-2 as the default because OpenAI reports higher quality
at roughly half the latency, which suits create_image's one-shot call. Both
gpt-image-2 and gpt-image-1.5 stay on the whitelist, so a saved image_model
keeps working untouched.
Also in this change:
- The rejection message is now built from the whitelist itself instead of a
hand-written string that drifts whenever a model is added.
- codex_build.go uses the DefaultImageModel constant instead of hardcoding
"gpt-image-2", so the inline chat path cannot drift from the shared default.
- Both new models join the isEditModel branch so reference images route to
/images/edits.
- ParamField gains an optional labelKey, letting the dropdown read its label
from i18n instead of showing English inside a translated UI. It passes a
defaultValue, so a missing key degrades to the literal rather than breaking.
- Locale keys added to all five catalogs; ko was missing the whole
mediaChain.imageModel* group.
Surface parity: the desktop UI is unchanged because sortable-provider-card
renders no param form and passes params through as Record<string, unknown>.
The CLI is unchanged because cmd/ exposes no image-model flag. The API contract
is unchanged because image_model lives in the free-form params map; only the
backend whitelist widened.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenAI rejects non-default temperature on gpt-5 / gpt-5-chat with
HTTP 400 ("Unsupported value: 'temperature' does not support 0.7
with this model. Only the default (1) value is supported.").
skipTemp in buildRequestBody only covered gpt-5-mini / gpt-5-nano and
the o-series, so base gpt-5 and gpt-5-chat fell through and received
config.DefaultTemperature (0.7). On channel integrations the inbound
error is suppressed, leaving the agent silently dead.
The gpt-5.X flagship variants (gpt-5.1, gpt-5.4, gpt-5.5) keep their
temperature — only the base gpt-5 and gpt-5-chat aliases are locked
to the default.
Regression cases added for gpt-5, gpt-5-chat, gpt-5-chat-latest and
provider-prefixed forms; gpt-5 moved to the locked list in
TestBuildRequestBody_TemperatureKeptForNonReasoningModels per the
maintainer triage on #1460.
Fixes#1460
ChatStream parsed the tool-call accumulator map with a positional loop
(for i := 0; i < len(accumulators); i++ { acc := accumulators[i] }),
assuming OpenAI-compatible providers always emit contiguous zero-based
tc.Index values in streamed delta.tool_calls.
Some OpenAI-compatible gateways (observed with a self-hosted point-p1
proxy) emit non-contiguous or non-zero-based indices. When that
happens accumulators[i] misses a populated key and returns a nil
*toolCallAccumulator, and the next line dereferences acc.rawArgs on
that nil pointer, causing a SIGSEGV panic. The panic kills the
in-flight chat run's goroutine; the gateway's tracing safety-net logs
'finalizing orphan trace' and the user's message goes unanswered.
Fix: range over the accumulators map directly instead of indexing by
position, and skip nil entries defensively.
Verified: built a full backend image (make build-full equivalent,
CGO_ENABLED=0 go build -tags embedui) from this branch, deployed it
against a real point-p1-backed agent, and confirmed multiple
multi-tool-call turns (use_skill, read_file, exec x3) that previously
triggered the panic now complete via v3.run.completed with no panic
and no safety-net trace.
DeepSeek rejects a request whose assistant history omits reasoning_content
once the conversation is in thinking mode:
HTTP 400: The `reasoning_content` in the thinking mode must be passed
back to the API.
A thinking-mode turn that only emits tool calls produces no reasoning of
its own, so Message.Thinking is empty and the wire field was dropped. The
next request then mixes assistant messages with and without the field and
upstream fails. Only kimi_coding had an empty-string fallback.
Detect thinking mode from the conversation itself: when any assistant
message carries reasoning, every replayed assistant message must carry the
field, empty string included. Narrower than matching on model name, which
would also inject the key for non-thinking models such as deepseek-chat.
Reproduces from a run where iteration 1 returned reasoning plus an edit
call, the edit failed, iteration 2 returned a bare read_file call, and
iteration 3 was rejected.
Gemini drops tool_call_id and pairs functionCall/functionResponse by
function name, so its OpenAI-compat shim requires a non-empty
FunctionResponse.name. The name was only recoverable through a reverse
id->name lookup over assistant tool_calls still present in the request
window, which fails after pruning, truncation or tool_call collapse and
produces HTTP 400 "Name cannot be empty" on any follow-up iteration.
Add Message.ToolName, set it at every tool-result creation site and
preserve it through context pruning, then prefer it when serializing.
Fall back to the existing index for history persisted before the field,
so old sessions keep working without a reset.
When neither source resolves a name, drop the unlabelled tool result
instead of emitting an empty one: an empty name is a guaranteed 400 and
a synthetic name would match no prior functionCall. Gated behind the
existing Gemini detection so other OpenAI-compat hosts, which pair by
tool_call_id, are unaffected.
Field is JSON-optional, so session history needs no migration and older
binaries ignore it on rollback.
A reasoning model emits nothing at all while thinking, so the wait for the
response's FIRST byte — not any streaming timeout — decides whether its answer
is reachable. At 180s a large-prompt reasoning request routinely lost that race:
the gateway reported "http2: timeout awaiting response headers" while the
router's own usage log showed the same request completing normally moments later
(promptTokens=77194 -> completionTokens=2789). The request was healthy; only the
deadline was short.
Raise it to 300s. The value is deliberately modest rather than generous because
it is multiplied twice downstream: RetryDo wraps every provider call with
Attempts: 3, and a workflow step requeues up to MaxTaskDispatches times on top
of that, so 300s is already ~45min of worst-case wall clock per step.
Removing the timeout entirely was considered and rejected. Agent run contexts
come from scheduler queue.go via context.WithCancel, so they carry no deadline —
this transport timeout is the only thing bounding a request that never answers.
Without it a silently dead connection would hold its scheduler lane until the OS
TCP keepalive expired, and workflow requeue could not rescue it because requeue
only fires when a run ends. The doc comment claiming the design could "rely on
ctx deadlines" was wrong and is corrected here.
Tests lock the sizing from both sides (must exceed the latency that failed, must
not exceed the ceiling the retry math was sized against) and assert the retry
multiplier itself has not drifted.
Co-authored-by: nguyenha935 <nguyenha935@users.noreply.github.com>
SetLastPromptTokens received TotalUsage.PromptTokens — the sum of prompt
tokens across every think→act→observe iteration of a run — instead of the
final iteration's own usage. A tool-heavy run inflated the stored value by
the iteration count, so the sessions list showed e.g. 901K "context used"
against a 258K window, and maybeSummarize over-triggered compaction once
the inflation exceeded the 40% overhead clamp (observed: 83 compactions
on a 4-message session).
Track the final iteration's usage separately (ThinkState.LastUsage) and
persist its actual context size: prompt tokens + cached segments (Anthropic
reports input_tokens excluding cache read/creation) + output tokens (the
final reply joins history, so it occupies the next request's prompt). The
run-cumulative total still feeds AccumulateTokens for billing.
A single dispatch goroutine delivered every reply in the process — every
channel, every user — each Send blocking the next. One media upload of a
few seconds froze all outbound traffic for that long, and sustained
throughput was capped at one Send round-trip regardless of lane capacity.
The ordering that loop actually protected is per-conversation: a run emits
block replies, retry notices and a final answer to one ChatID, and those
must arrive in that order. A global order was never required.
Dispatch now shards on channel+ChatID. A conversation always maps to the
same shard and is delivered serially there, so the ordering guarantee is
unchanged, while unrelated conversations proceed in parallel.
GOCLAW_OUTBOUND_SHARDS tunes the worker count (default 8).
Temp media needed a real fix to go with it. Serial dispatch deduped it
implicitly — the first delivery removed the file, and a later message
carrying the same path found os.Stat failing and skipped it. Across
concurrent shards that check alone is a TOCTOU race, so claims are now
taken atomically and released after the send.
Also makes the pool limits that bound this path configurable, defaults
unchanged: GOCLAW_PG_MAX_OPEN_CONNS / GOCLAW_PG_MAX_IDLE_CONNS, and
GOCLAW_HTTP_MAX_IDLE_CONNS / GOCLAW_HTTP_MAX_IDLE_CONNS_PER_HOST. The
per-host limit binds when one provider takes nearly all traffic: every
request past it pays a fresh TCP+TLS handshake, which a deployment
raising LaneMain needs to be able to lift.
The Ollama integration never applied a correct context window, so agents
with real prompts (20k-100k tokens) were rejected with HTTP 400
exceed_context_size_error against a 4096-token default.
Three root causes fixed:
1. FetchOllamaModelContext issued a GET to /api/show, which Ollama answers
with 405 (the endpoint is POST-only). Now POSTs {"model": "<name>"}.
2. The response parser expected a flat model_info.context_length, but a real
Ollama server namespaces the key by architecture (gemma4.context_length,
qwen3.context_length, ...). extractContextLength now matches "context_length"
or any "*.context_length" key.
3. num_ctx was resolved once at startup for a hardcoded "llama3.3" model and
never for the model an agent actually uses. Resolution now happens per
request for the real model inside OllamaProvider.resolveNumCtx, cached under
an RWMutex, with an explicit settings override winning and the fetched value
bounded by OllamaDefaultNumCtx so an enormous advertised window (Qwen3.5
reports 262144) cannot balloon the KV cache beyond VRAM.
Also classify Ollama's "exceed_context_size" 400 as a context-overflow error so
the pipeline's emergency-compaction+retry path (Issue 958) engages gracefully
instead of surfacing a raw error.
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Current Claude CLI releases (>= 2.x) no longer register TodoRead and
NotebookRead. Passing them via --disallowedTools makes the CLI print
'Permission deny rule "..." matches no known tool' on every invocation,
including background episodic summarization, burying real errors.
Keep TodoWrite/NotebookEdit blocked (still registered, no GoClaw
equivalent). Add a regression test guarding both directions.
Fixes#1374
* fix: unify NO_REPLY detection (#1233)
Co-authored-by: GoClaw Operator <operator@goclaw>
* feat(acp): surface session/update tool_call notifications at Info level (#1141)
session/update notifications carrying a ToolCall (or inline tool_call /
tool_call_update kind) were only dumped at Debug level inside the params
blob. Operators could see `security.tool_granted` (permission granted) but
had no way to tell whether the tool actually executed successfully — both
"permission granted then failed silently" and "permission granted and
succeeded" looked identical in journalctl.
Add a structured Info log emitting toolCallId, name/title, status, and a
content preview (truncated at 400 chars) whenever the notification
contains tool-call state. This is what made it possible to diagnose the
recent .goclaw/-path-deny regression — `status=failed` immediately after
`security.tool_granted` revealed the gap that the granted-only log hid.
No behavior change beyond logging volume; preview is bounded.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(exec): exempt venv python interpreter from .goclaw/ path deny (#1140)
The ExecTool path-deny rule blocks any token containing `.goclaw/` unless
it matches one of the AllowPathExemptions prefixes (skills-store, tenants).
This silently rejected legitimate commands invoking the goclaw-managed
Python interpreter via its absolute path:
/home/user/.goclaw/venv/bin/python3 .../script.py
The first token `/home/user/.goclaw/venv/bin/python3` matched the deny
pattern but no exemption, so the entire command was denied.
Naive exemption (".goclaw/venv/bin/") does not work: matchesAnyPathExemption
resolves both tokens and exemption candidates via EvalSymlinks, and the
venv's python3 is a symlink into the host's python cellar (e.g. linuxbrew).
The token canonicalizes to /home/linuxbrew/.../python3.14 while a literal
".goclaw/venv/bin/" prefix never gets touched.
Fix: resolve venv/bin/python3 once at startup and exempt the dirname of
the resolved target. Failure to resolve (no venv present) silently falls
through.
Without this, ACP-driven agents either fail outright or work only via
fragile heuristics (cwd-local symlinks generated on the fly by the LLM).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cron): invert stateless gate to match UI label (#1139)
The cron job handler reset the session when stateless=false and skipped
reset when stateless=true — the opposite of what every UI locale labels
the field (en/ko/zh/vi all describe stateless as "each run starts fresh
without loading previous messages").
The buggy gate caused stateless=true crons to silently accumulate session
history across every execution, leading to context bloat and increasing
the chance of LLMs short-circuiting tool calls in favor of replaying
prior assistant turns. One affected daily ETL cron grew to 38 messages
over 18 days before the regression was noticed.
Fix: gate the Reset on `if job.Stateless` so the runtime matches the UI
contract. No DB migration is required — existing values were set by users
based on the UI label, so they already encode the intended behavior.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(providers): retry transient Codex response failures (#1332)
* fix(providers): retry transient Codex response failures
* test(cron): tolerate nil session store in handler tests
* fix(background): clear stale provider alerts (#1362)
Co-authored-by: Collective Developer <man@collective.dev>
---------
Co-authored-by: Duy /zuey/ <duy@wearetopgroup.com>
Co-authored-by: nguyenha935 <nguyenthanhha935@gmail.com>
Co-authored-by: GoClaw Operator <operator@goclaw>
Co-authored-by: codebit0 <34156842+codebit0@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Zezae Oh <zezaeoh@gmail.com>
Co-authored-by: Collective Developer <man@collective.dev>
Ollama has two separate request-building code paths: OllamaProvider
(native /api/chat, used for num_ctx control) and OpenAIProvider
(OpenAI-compat /v1/chat/completions). An earlier fix disabled thinking
mode by hardcoding think=false, but only in the OpenAI-compat path --
OllamaProvider.buildRequest() never set the think field at all, so
reasoning-capable models (qwq, deepseek-r1) defaulted to visible
chain-of-thought reasoning regardless of that fix. Confirmed live via
a docker-engineer agent streaming full reasoning traces despite the
existing disable.
Replaced the hardcoded always-off behavior with a provider-level
tri-state setting (llm_providers.settings.thinking_enabled: unset =
default off, explicit true/false overrides), configurable via the
provider's Advanced settings dialog. Both OllamaProvider.buildRequest()
and OpenAIProvider.buildRequestBody() now read and respect this same
setting, so the toggle works regardless of which Ollama code path a
given deployment routes through.
Added tests for setting parsing (unset/true/false/malformed) and both
provider request-builders' handling of the override.
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
bufio.Scanner has a fixed max-token size (SSEScanBufMax, 1MB). Codex's
native image_generation streams a full base64 image in a SINGLE SSE
data line (response.image_generation_call.partial_image and the final
image_generation_call item), which regularly exceeds 1MB for anything
larger than a thumbnail. Scanner.Scan() then fails with:
stream read error: bufio.Scanner: token too long
killing the whole turn after the model already generated the image.
Replace bufio.Scanner with bufio.Reader.ReadString, which grows to fit
a line of any length instead of erroring past a fixed cap. SSEScanBufMax
is now unused and removed; SSEScanBufInit is reused as the reader's
initial buffer size.
buildRequestBody emitted prompt_cache_key and prompt_cache_retention for
every Codex request. The ChatGPT subscription OAuth backend
(chatgpt.com/backend-api) rejects prompt_cache_retention with HTTP 400
"Unsupported parameter", so every LLM call from an agent using a ChatGPT
OAuth Codex provider failed at the first think step.
Gate both params on isOpenAINativeEndpoint(apiBase), the same native-only
policy already documented and applied by CacheMiddleware. This covers both
CodexProvider and CodexAdapter, since the adapter delegates to the same
builder. Server-side prefix caching still works on the OAuth backend
without these params.
Update the two prompt-cache tests to assert both cases: native endpoints
include the params, the OAuth backend omits them.
* fix(prompt): remove duplicate Team Members section from system prompt
The TEAM.md context file already provides the Members section with better
formatting. The system prompt section was redundant and inconsistently
formatted.
- Remove buildTeamMembersSection() call from system prompt building
- Remove unused buildTeamMembersSection() function
* fix(mcp): wire store to manager for prompt preview tool visibility
MCP manager needs database access to query configured servers for tool visibility
in prompt preview.
- Add SetStore() method to MCP manager
- Wire pgStores.MCP to manager after initialization
- Add debug logging for MCP initialization flow
* fix(systemprompt): hide Tooling section when agent has no tools
The Tooling section header and boilerplate were displayed even when the
agent had no tools available. Add early return in buildToolingSection()
to skip the section entirely when toolNames is empty.
This reduces prompt noise for agents with no tool access.
* feat(mcp): cache tool descriptions for prompt preview visibility
MCP tools now show descriptions in prompt preview without requiring a live
server connection.
- Add CacheToolDescriptions() method to MCPServerStore (PG + SQLite)
- Cache tool descriptions in settings['tool_cache'] when server connects
- Use cached descriptions in ListToolsForAgent (fallback: hints → cache → global)
- Descriptions are auto-populated from live server manifest on connection
- Admins can still override via tool_hints in server settings
* test(mcp): add CacheToolDescriptions to MCPServerStore test fakes
Commit 5ce410b6 added CacheToolDescriptions() to the MCPServerStore
interface but missed updating test mock implementations, breaking
go vet across internal/mcp, internal/agent, internal/http, and
internal/channels/bitrix24.
Add no-op implementations matching each fake's existing style.
* fix(providers): enforce per-agent tool policy for Claude CLI provider
The Claude CLI provider (stdio+MCP bridge) was not enforcing per-agent
tool policy, unlike other providers where the policy-filtered tool list
already drives the system prompt's Tooling section.
Two gaps closed:
1. --disallowedTools was previously skipped entirely when no MCP config
path was resolved, letting the CLI subprocess run with its full
native toolset (Bash, Edit, Read, Write, Glob, Grep, WebFetch,
WebSearch) regardless of agent policy. It's now unconditional and
derived from the agent's actual allowed-tools list (state.Tool.AllowedTools),
mapped to Claude CLI's native tool names.
2. The MCP bridge server executed any tool call without checking the
calling agent's policy. It now resolves the agent's policy from
context (via the existing HMAC-verified agent lookup) and denies
calls to tools outside that agent's allowed set, logging
security.mcp_bridge_denied on denial.
Both gaps were closed using existing plumbing (PolicyEngine.WouldAllow,
AgentData.ParseToolsConfig, the bridge context middleware) — no new
cross-cutting mechanism was introduced.
* feat(web): show MCP/tool schemas in system prompt preview dialog
The prompt-preview API response includes a separate `tools` field
(the actual JSON schemas sent to the LLM as the tools API parameter)
alongside `prompt` (the system prompt text), but the web UI only
rendered `prompt`, silently dropping the tools list.
Add a collapsible Tools section to both the full-screen System Prompt
dialog and the inline agent-detail preview, showing tool count, name,
description, and expandable parameter schema per tool. i18n keys added
to en/vi/zh locales.
* fix(prompt): render pinned skills on bootstrap turns
Pinned skills are documented (web UI copy) as "always inlined in the
system prompt", but the entire Skills section was gated behind
!cfg.IsBootstrap, so pinned skill XML never appeared on bootstrap
turns (first message of a session) despite the promise.
Separate pinned-skill rendering from bootstrap-suppressed guidance:
- Bootstrap + pinned skills present: render pinned XML only, no
search/manage guidance (which stays suppressed as before)
- Non-bootstrap: unchanged behavior
- Minimal/none modes: pinned skills always render regardless of
bootstrap state
Add regression tests covering all four prompt modes on bootstrap
turns, plus a non-bootstrap guard confirming existing behavior is
preserved.
* fix(skills): resolve managed skills directory per-tenant, not master-only
skills.Loader was wired at startup to scan a single fixed directory
(the master tenant's managed-skills dir), making any skill belonging
to a non-master tenant invisible to both pinned-skills prompt
resolution and skill_search/use_skill, regardless of DB visibility
settings.
- Loader now resolves the calling tenant's managed-skills directory
per-call via context (store.TenantIDFromContext), never enumerating
other tenants' directories
- Skill cache is now tenant-keyed to prevent slug collisions and
cross-tenant cache leaks across tenants using the same skill slug
- gateway_setup.go passes the root data dir instead of a pre-resolved
master-tenant path
Write-side tooling (skill_manage, publish_skill) was already correctly
tenant-scoped per-operation — no changes needed there.
Added TestLoader_ManagedSkills_TenantIsolation proving two tenants
with same-slug/different-content skills never see each other's
content, including after cache population from a different tenant's
lookup.
Known follow-up (not in this commit): skill_search's BM25 index is
still a single process-global index shared across tenants, which is
a related but separate cross-tenant search-result leak requiring its
own scoped fix (per-tenant index maps + threading tenant context
through ensureIndex/rebuildIndex).
* fix(tools): scope skill_search BM25 index per-tenant
SkillSearchTool held a single process-global BM25 index built once
from whichever tenant's context first triggered ensureIndex, then
reused for all subsequent Execute() calls regardless of caller —
leaking one tenant's skill search results into another's, the
search-path counterpart to the managed-directory bug fixed in
7b4668ad.
- index/lastVersion are now keyed per-tenant (map[uuid.UUID]*tenantIndexState)
- ensureIndex resolves the calling tenant from context and only
builds/reads that tenant's index entry, never touching another
tenant's cached state
- Builtin/bundled skills remain visible in every tenant's index
(Loader already merges those tiers correctly per 7b4668ad)
Loader.Version() remains a single global counter — a version bump in
one tenant causes unnecessary rebuilds in others but does not cause
cross-tenant leakage, an acceptable tradeoff to avoid scope creep.
Added TestSkillSearchTool_TenantIsolation proving two tenants with
same-slug/different-content skills never see each other's search
results, including after cache population from a different tenant.
* fix(tools): fix group-spec expansion in tool policy engine
PolicyEngine.registry was only ever set via SetRegistry(), which was
never called in production (only in one test) — so pe.registry was
permanently nil in production. Every group-expansion helper
(applyProfile, intersectWithSpec, unionWithSpec, subtractSpec,
expandSpec, matchDenySpec, filterByCapability) silently dropped any
"group:*" spec entry instead of expanding it when registry was nil.
Concretely: "group:mcp" (auto-injected into agentToolPolicy.AlsoAllow
for any agent with MCP tools) never resolved to real tool names, so
MCP tools connected successfully and appeared in prompt text (which
reads the registry directly, bypassing PolicyEngine) but were never
included in the actual ChatRequest.Tools payload sent to the LLM —
confirmed live via mcp.agent.tools_loaded tools=6 immediately followed
by mcp.filtered_tools mcp_defs_count=0 in the same request. This
affects any agent relying on group-based grants, not just MCP.
PolicyEngine is a shared/global singleton used concurrently across
all agents (constructed once at gateway startup), so mutating a
registry field per-call would be a data race. Fix instead threads the
registry as an explicit parameter from FilterTools down through all
internal group-expansion helpers, and adds IsDenied/WouldAllow
registry parameters, removing the dead SetRegistry() mechanism
entirely.
Also fixes group expansion for the per-user-MCP-tools path: FilterTools
is sometimes called with a userToolOverlay wrapping a *Registry rather
than a *Registry directly; added Unwrap() to userToolOverlay so the
new registry-resolution logic works for both cases.
Added 4 tests proving group expansion works via the threaded parameter
alone (no SetRegistry): plain registry allow, userToolOverlay allow,
deny-side group expansion, and the WouldAllow bridge-server path.
Blast radius note: this restores intended access for every agent
configured with group:* specs (group:mcp, group:vault, group:goclaw,
group:coding, etc.) that were silently inert before. Existing agent
configs relying on group grants will gain the tool access they were
nominally already configured for.
* fix(mcp): cache tool descriptions from pool-connected servers too
5ce410b6 added tool-description caching (for prompt-preview visibility)
only inside connectServer. connectViaPool — the separate connect path
used when MCP connections go through the shared pool — never got the
same caching hook, even though it shares the same underlying
connectAndDiscover wire handshake.
Confirmed live: cloudflare/docker connected via connectViaPool and
received real descriptions over the wire, but prompt preview still
showed blank descriptions because this path never wrote to the cache.
Also removes the temporary mcp.connect.raw_tool debug log added
earlier this session for diagnosing the same issue — no longer needed
now that the root cause is fixed.
* chore(skills): remove temporary pinned-skills diagnostic logging
Confirmed live: pinned skills (caveman, infra-ansible-knowledge) now
resolve correctly end-to-end for tenant-scoped agents. Debug logging
added to trace the resolution chain is no longer needed.
* fix(tools): deny always wins over AlsoAllow group grants
AlsoAllow's unionWithSpec could reintroduce a tool explicitly listed
in Deny, since it added tools back from allTools without re-checking
deny specs. Previously masked because AlsoAllow's group-expansion was
also broken (fixed in f7af95de this session) — group specs silently
expanded to nothing, so this ordering bug never manifested. Now that
group expansion works, an admin-denied tool that's also reachable via
a group:* AlsoAllow entry (e.g. group:mcp) would silently reappear.
Re-apply deny-spec subtraction as a final step after AlsoAllow union,
for both global and per-agent policy, so deny always wins regardless
of which allow mechanism tries to add a tool back.
Added tests proving global and per-agent Deny correctly override an
overlapping AlsoAllow group grant, while sibling non-denied tools in
the same group remain allowed.
* fix(mcp): enumerate cached tools instead of wildcard placeholder in prompt preview
ListToolsForAgent (the prompt-preview path) collapsed any server with
an empty ToolAllow (unrestricted grant — the common case) into a
single "server__*" placeholder entry, even when tool_cache already
had every real tool name and description from connect time (5ce410b6,
8ffd67b9). This meant agents with unrestricted MCP server access never
saw individual tool names or descriptions in prompt preview, forcing
trial-and-error tool usage.
When ToolAllow is empty and tool_cache is populated, enumerate every
cached tool (skipping any explicitly denied) and emit one
MCPToolPreviewInfo per tool, matching the construction logic already
used for the ToolAllow-non-empty case. Falls back to the single
placeholder only when tool_cache is also empty (server never
connected).
The live (non-preview) conversation path, buildMCPToolDescs, does not
have this bug — it resolves tool identity from the live connected
registry, never from ToolAllow, so no placeholder shortcut exists
there.
Added tests covering both the cache-populated enumeration case and
the no-cache placeholder fallback.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(prompt): filter alias re-injection and add MCP schemas to preview ToolDefs
Prompt-preview's tools: schema array (PreviewResult.ToolDefs) had two
bugs, both preview-only — confirmed live conversations already use
correctly-filtered tool payloads via buildToolsPayload/PolicyEngine.FilterTools,
unaffected by either bug:
1. Alias re-injection iterated ALL registry aliases globally with no
check against the deny-filtered toolNames list, letting denied
tools reappear via their alias name (e.g. a denied canonical tool
still showing up as its Claude-Code-compat alias like Bash/Edit/Write/Read).
Now skips any alias whose canonical tool isn't in the filtered set.
2. MCP tool descriptions (from the store-based, connection-free
ListToolsForAgent path) only ever fed the prompt TEXT section,
never got converted into ToolDefinition schema objects — so the
tools: array never showed MCP tools at all, even when the MCP
section of the prompt text correctly listed them. Now appends a
ToolDefinition per MCP tool with name+description populated and a
placeholder {"type":"object"} Parameters schema, documented as
preview-only (real parameter schemas require a live MCP connection,
only available during an actual conversation turn).
Added tests proving denied-tool-alias exclusion and MCP tool inclusion
in preview ToolDefs.
* feat(skills): inline full content for pinned skills instead of pointer-only
Web UI documented "Pinned skills are always inlined in the system
prompt", but BuildPinnedSummary was just BuildSummary with an
allowlist filter — same as the general searchable skill list:
name/description(truncated)/location pointer only, requiring
use_skill+read_file round trips to get actual content. No code path
inlined real SKILL.md content for pinned skills specifically.
BuildPinnedSummary now reads and inlines full SKILL.md content
(frontmatter stripped) per pinned skill inside <skill_instructions>
tags. Per-skill (10000 bytes) and total (30000 bytes) size caps fall
back to the original pointer-only format with a note when a skill is
too large to inline, so oversized skills degrade gracefully instead
of blowing the prompt budget.
Added tests: full-content inlining, size-cap fallback, and tenant
isolation for the new inline path (mirroring the existing managed-
skills tenant isolation test).
* fix(mcp): real parameter schemas and full policy enforcement in prompt preview
Three interconnected fixes to prompt preview, none affecting the live
conversation path (which was already correct):
1. Real MCP parameter schemas instead of a useless empty placeholder.
tool_cache previously stored only name+description; extended to
also capture the real JSON Schema from the MCP server's tools/list
response (CachedToolInfo{Description, Parameters}) at connect time,
for both direct-connect and pool-connect paths. Preview now shows
complete, real input schemas instead of {"type":"object"} with no
properties — verified against actual wire data from a live
cloudflare MCP server showing genuinely rich schemas (zone_id,
type, name, content, ttl, proxied, all typed with descriptions and
correct required arrays) that were previously being discarded.
Backward-compatible: old-shape cache entries degrade gracefully to
description-only rather than crashing, self-healing on next
connect.
2. Global tool deny now enforced in preview. BuildPreviewPrompt
previously hand-rolled a partial policy reimplementation
(per-agent deny only, explicitly skipping the full PolicyEngine
"because runtime state isn't available in preview") — but
PolicyEngine.WouldAllow already handles this per-tool-name without
needing channel context. A tool denied via the global config (not
per-agent) would appear in preview despite being correctly denied
in every real conversation. Preview now calls WouldAllow per
candidate tool, with a graceful per-agent-deny-only fallback when
no PolicyEngine is wired (e.g. in tests).
3. MCP tools now also subject to the same policy check — previously
the MCP tool supplement (store-based, connection-free tool listing)
added MCP tool names unconditionally, bypassing WouldAllow entirely,
so a denied MCP tool could still appear in preview.
Added tests for all three: real-schema presence, global-deny exclusion
for both core and MCP tools, and backward-compat cache handling.
* fix(prompt): remove redundant per-tool MCP enumeration from prompt text
Now that MCP tool schemas in the tools: API parameter are real and
complete (1290d4f1), the ## MCP Tools prompt-text section's per-tool
"- mcp_x__y: description" enumeration is pure duplication with zero
added value — the model already gets each tool's real schema
(including description) via tools:.
buildMCPToolsInlineSection now keeps only the behavioral instructions
that aren't expressible via JSON schema and thus aren't duplicated:
prefer-MCP-over-core-tools guidance, and the optional-parameter
guidance (don't guess/fill optional fields). The per-tool name+
description enumeration loop is removed. Section still only appears
when the agent has MCP tools (len(cfg.MCPToolDescs) > 0, unchanged
gate).
Updated tests to assert the enumeration is gone while the behavioral
instructions remain; ToolDefs assertions are now the authoritative
check for MCP tool allow/deny filtering behavior (prompt text no
longer enumerates names at all).
* test(agent): update TeamContextInjection test for removed Team Members section
TestBuildSystemPrompt_TeamContextInjection asserted the presence of a
'Team Members' prompt-text section that was intentionally removed in
71d33180 (duplicate of the canonical TEAM.md-context-file Members
section, which has better formatting). The test was never updated to
match, causing it to fail on every run since. Moved the assertion
from wantIn to wantNotIn for the 3 affected subtests -- BuildSystemPrompt
correctly no longer renders team-member roster info directly; that
info now comes exclusively from the TEAM.md context-file mechanism,
outside this test's isolated scope.
* fix(prompt): resolve real registry for WouldAllow calls in preview
BuildPreviewPrompt's two WouldAllow calls hardcoded reg=nil, silently
breaking group:* expansion (e.g. group:mcp) needed to resolve the
AlsoAllow grant production actually uses to grant MCP tool access
(resolver_helpers.go's agentToolPolicyWithMCP injects
AlsoAllow: ["group:mcp"]). With reg=nil, WouldAllow could match
literal tool names fine (the 18 core/static tools) but could never
resolve group-based grants, so every MCP tool silently failed
WouldAllow and was excluded from preview -- confirmed live via curl:
18 tools returned, zero mcp_* ones, for an agent with genuinely
working MCP access in real conversations.
Live conversations were never affected -- internal/mcp/bridge_server.go's
WouldAllow call already correctly passes a real registry.
Fix resolves a real *tools.Registry from deps.ToolLister via
tools.ResolveConcreteRegistry (the same helper used at the live call
site), passing it to both WouldAllow calls instead of nil. Falls back
to nil gracefully for test mocks that don't implement the full
ToolExecutor interface, preserving existing test behavior.
Added a test proving an MCP tool granted via the exact production
AlsoAllow: ["group:mcp"] pattern is now correctly included in preview
ToolDefs, where the old reg=nil bug would have silently excluded it.
* fix(prompt): use literal deny check for MCP tools in preview, not group expansion
The MCP-tools policy gate added in 1290d4f1 called WouldAllow with a
real registry (per 9b5fd2eb), which requires group:mcp expansion
against that registry to grant access via the production
AlsoAllow: ["group:mcp"] pattern. But MCP tools are only ever
registered into ephemeral per-agent registry clones at live connection
time (manager_connect.go) -- never into the shared/global registry
preview uses. group:mcp always resolved empty in preview's
connection-free context, so WouldAllow denied every MCP tool --
confirmed live via tool-name diff: live conversations correctly
included all 6 MCP tools, preview included zero.
MCP access-granting is already correctly handled by
ListToolsForAgent's own per-server tool_allow/tool_deny grant logic
(confirmed working correctly earlier this session). The preview gate
only needs to catch the narrower case of a literally-denied tool name
via global/per-agent policy config -- it never needed group
expansion. Replaced WouldAllow with IsDenied(nil, name, agentPolicy),
which forces a pure literal-name match with zero registry dependency,
matching the existing usage pattern already established elsewhere in
policy.go. This class of bug cannot recur: there's no registry-passing
code path left in this check to silently reintroduce group-expansion
dependence.
Added a test proving MCP tool inclusion in preview is independent of
group-expansion outcome (no AlsoAllow: group:mcp needed for a
non-denied tool to appear).
Also reverts the temporary loop.filtered_tool_names/
preview_prompt.filtered_tool_names diagnostic logging used to capture
the live-vs-preview tool-name comparison that diagnosed this bug.
* fix(http): preserve real MCP parameter schemas through HTTP preview adapter
mcpPreviewAdapter.ListToolsForAgent (the HTTP-layer glue converting
mcp.MCPToolPreviewInfo to agent.MCPToolPreviewInfo for BuildPreviewPrompt)
only copied RegisteredName and Description, silently dropping
Parameters -- a bug present since this adapter was introduced
(2499d0be/7e250244), unrelated to today's other MCP preview fixes.
This was masked until d5fc6344 fixed MCP tools being excluded from
preview entirely (a separate bug) -- once MCP tools started appearing
again, this pre-existing adapter gap became visible: tools showed up
correctly, but always with the bare {"type":"object"} placeholder
instead of their real cached schema (confirmed live: update_dns_record
missing its 7 real properties).
One-line fix: copy Parameters through in the adapter's struct literal.
Added a regression test constructing a real *mcp.Manager with
populated tool_cache, asserting the adapter's output preserves
specific real schema properties (not just non-nil Parameters) --
verified this test fails without the fix and passes with it.
---------
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Ollama models like qwq and deepseek-r1 have thinking enabled by default.
Disable it to prevent bloated response generation unless explicitly requested.
- Add isOllamaEndpoint() detection (providerType, name, apiBase)
- Always send think=false in Ollama requests unless OptThinkingLevel is set
- SupportsThinking() and Capabilities().Thinking return false for Ollama
- Add tests covering default-off, explicit-opt-in, and capability flags
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
* feat(ollama): auto-detect context window, allow user override
Query Ollama API (/api/show) to fetch model's actual context_length
on provider startup/hot-registration. Use queried value by default,
but allow user to override via num_ctx field in provider settings.
- backend: query API, store in provider config, use in requests
- frontend: add num_ctx input field to Ollama provider dialog (4 langs)
- fallback: 131072 tokens (128K) if API unreachable
Fixes context-window errors when using large-context models.
* fix(ollama): route to /api/chat endpoint to honor num_ctx
Ollama's /v1/chat/completions (OpenAI-compat shim) ignores options.num_ctx.
Route to /api/chat (native endpoint) which respects num_ctx.
Request body is identical, only endpoint URL changes:
- Before: /v1/chat/completions (ignores options.num_ctx)
- After: /api/chat (honors options.num_ctx)
Fixes context-window errors when using models with large context windows.
* refactor(providers): replace OpenAI-wrapped Ollama with native SDK
Implement dedicated OllamaProvider using github.com/ollama/ollama/api
official client library. Replaces the broken approach of wrapping
Ollama behind OpenAI-compatible parsing.
Benefits:
- Proper response parsing (Ollama native format, not OpenAI)
- Automatic /v1 suffix stripping
- Built-in num_ctx support (injected into every request)
- Tool call support via Ollama's native ToolCall schema
- Token usage mapping from Ollama Metrics
- Clean streaming implementation
Fixes context-window errors by using Ollama native API directly.
Removes dependency on OpenAI response format hacks.
- new: internal/providers/ollama.go (OllamaProvider)
- updated: cmd/gateway_providers.go, internal/http/providers.go
- updated: go.mod (added github.com/ollama/ollama)
* fix(ollama): fetch num_ctx for config-registered providers
Config-file-registered Ollama providers (cfg.Providers.Ollama.Host)
were hardcoding numCtx=nil, so num_ctx was never sent to Ollama.
Add FetchOllamaModelContext call for config providers, same as
DB-registered providers. Now config Ollama providers also auto-detect
and send the model's context window to the Ollama API.
Fixes context-window errors when using config-registered Ollama.
* fix(ollama): always send num_ctx to prevent context window errors
OllamaProvider.buildRequest only injected options.num_ctx when an
explicit override was configured. When resolveOllamaNumCtx saw that
/api/show returned the OllamaDefaultNumCtx value (131072), it returned
nil, causing the provider to omit num_ctx entirely. Ollama then fell
back to the model's baked-in default (4096 tokens), causing requests
with long system prompts to fail with "request exceeds context size".
Fix both layers:
- resolveOllamaNumCtx now always returns &numCtx instead of nil when
/api/show returns the default, so the explicit value is always passed.
- OllamaProvider.buildRequest falls back to OllamaDefaultNumCtx (131072)
when p.numCtx is nil, ensuring every request carries a large context
window even if the startup query was skipped.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat(tools): implement image reference processing and native provider support
- Support OpenAI image edits via both Multipart form-data and JSON payloads
- Automatically append reference image descriptions to prompt under [Reference Image Roles]
- Support downloading image URLs for Gemini native image generation
- Deduplicate reference images to optimize API request size
- Add unit tests for Codex, DashScope, MiniMax, BytePlus and local/remote path resolution
* fix(tools): SSRF-guard reference-image URL downloads in create_image
downloadImageBytes fetched caller-supplied ref_images[].url with a plain
http.Client and unbounded io.ReadAll — no SSRF validation, redirect
policy, or size cap, letting the gateway dial loopback/private/metadata
hosts or read arbitrarily large responses.
- Validate the URL via security.Validate and pin the resolved IP, then
download through security.NewSafeClient (pinned dial, no redirects).
- Cap the response with a bounded read (refImageMaxBytes, 20 MB).
- Reject non-HTTP(S) reference URLs up front (file://, data:, gopher://)
so provider-forwarded URLs stay HTTP(S)-only; document the trust
boundary between gateway-side fetch and provider-forwarded URLs.
- Add regression tests: blocked loopback/private/metadata, unfollowed
redirect, oversized response, and non-http(s) scheme rejection.
* feat(providers): explicit prompt cache for DashScope/Qwen
Extend OpenAI-compat path with Anthropic-style cache_control:ephemeral
inline blocks for Alibaba DashScope endpoints (provider_type=bailian or
URL contains "dashscope"). Verified live: qwen3.6-plus / qwen3.5-plus /
qwen3-coder-plus all return 99.7% cache hit rate on 6K-token prefix,
yielding ~89% token cost reduction on cached prefix (90% Alibaba discount).
- isDashScope(): 3-source detection (URL + providerType + name) handles
reverse-proxied endpoints; covers both "dashscope" and "bailian"
- buildRequestBody wraps system content with SplitSystemPromptForCache
(exported from anthropic_request.go) using <!-- GOCLAW_CACHE_BOUNDARY -->
- Tool prefix cache: cache_control on last tool definition, with 4-marker
budget guard
- Parse cache_creation_input_tokens from prompt_tokens_details into
Usage.CacheCreationTokens; propagates via existing span metadata writers
- Runtime escape hatch: GOCLAW_DISABLE_DASHSCOPE_CACHE=true
- Live integration smoke test (build tag integration, env-gated)
* chore(providers): polish per PR #1127 review
- Use strings.Repeat instead of custom repeat() helper in smoke test
- Clarify BuildRequestBodyForTest is test-only, not public API
* feat(providers): enable thinking for Qwen 3.7/3.6, observe cache in smoke test
Qwen3.7-plus and qwen3.6-plus support deep thinking but were missing from
dashscopeThinkingModels, so enable_thinking/thinking_budget was silently
skipped. Add both to the whitelist and test.
Add qwen3.7-plus to the cache smoke test as cache-optional: Alibaba's
context-cache doc does not yet list 3.7/3.6 for explicit cache, and the
cache wrap is a safe no-op when unsupported, so a no-cache result is logged
rather than failed to avoid a flaky live assertion.
Claude-Session: https://claude.ai/code/session_01X3jkrc7N3ar8ZGzUJWyNS5
* fix(providers): preserve DashScope detection for proxy routes
Upstream returns HTTP 400 `thinking is enabled but reasoning_content
is missing in assistant tool call message at index N` when an
assistant message with tool_calls is replayed in history without a
reasoning_content field. Kimi has server-side thinking enabled by
default for kimi-k2-turbo-preview, so the field is required even when
goclaw doesn't have captured reasoning content to send (e.g. the model
emitted a tool_call without any thinking, or the stream chunk that
carried it was lost).
The existing branch already gates on
openAIWireAssistantReasoningContent(model) (kimi/deepseek/o-series)
and emits the field only when Thinking != "". Extend so kimi_coding
also emits an empty string when Thinking is unset — satisfies Kimi's
"must be present" check without inventing reasoning content. Other
providers in the allowlist keep today's behavior: omit when empty.
Three new tests:
- kimi_coding always carries reasoning_content on assistant
- kimi_coding preserves real Thinking content when set
- non-kimi providers (deepseek) do NOT inject empty reasoning_content
Reference: NousResearch/hermes-agent plugins/model-providers/kimi-coding
documents the same upstream behavior (thinking enabled by default,
reasoning_content roundtrip required).
Moonshot's Kimi Coding endpoint is OpenAI-compatible on the wire but
has two non-standard rules:
1. Every request must carry `User-Agent: claude-code/0.1.0` — without
it the upstream rejects the call outright.
2. `temperature` is locked to the server default; passing any other
value returns HTTP 400 `invalid temperature: only 1 is allowed for
this model`.
Rather than special-case either, this commit generalises both:
- WithExtraHeaders on OpenAIProvider — static headers attached to
every outgoing request. Reusable by any future provider that needs
pinned identity headers; mirrored in adapter_openai.ToRequest so
callers using the adapter path see the same shape.
- The existing skipTemp branch in openai_request.go gets a
provider_type check — kimi_coding joins o1/o3/o4/gpt-5-mini in
omitting `temperature` from the request body.
Provider wiring:
- store.ProviderKimiCoding constant + ValidProviderTypes entry +
KimiCoding{DefaultAPIBase,DefaultModel,RequiredUserAgent}.
- case store.ProviderKimiCoding in both registration switches
(cmd/gateway_providers.go and internal/http/providers.go).
- UI dropdown entry with the API base pre-filled.
5 unit tests cover: real outgoing header injection, adapter-path
header mirroring, empty-map WithExtraHeaders no-op, kimi_coding
strips temperature, and the negative control (other providers still
forward temperature).
Admin flow: Providers → Add → "Kimi Coding (Moonshot)" → paste API
key → save.
Anthropic returns HTTP 400 when temperature is sent to claude-opus-4-6,
claude-sonnet-4-6, and newer 4.x models. Agent summoning and chat both
hardcode temperature; skip the field for affected model IDs.
Co-authored-by: Ojus Save <ojusave@users.noreply.github.com>
* fix(sandbox): avoid shell in FsBridge writes
Replace sh -c with interpolated path by shell-free 'tee -- <path>' argv form,
piping content via stdin. Prevents command injection through filenames
containing shell metacharacters inside the sandbox container.
Co-authored-by: evgyur <evgyur@gmail.com>
* fix(security): fail-closed on pairing DB errors across channels
On IsPaired lookup error, deny instead of granting access. Covers the shared
CheckDMPolicy/CheckGroupPolicy helpers (Slack/Discord/Feishu/WhatsApp/Zalo) and
the four inline Telegram pairing checks.
Co-authored-by: Srini <srinis.k@gmail.com>
* fix(security): harden provider URL validation against SSRF
Enforce scheme check for all provider types; restrict local types (ollama,
claude_cli, acp) to an explicit localhost allowlist instead of skipping checks;
resolve remote hostnames and reject any IP in a private/reserved range via the
shared security.IsBlocked CIDR list (covers loopback, link-local, metadata,
multicast, and unspecified 0.0.0.0/::). Closes the wildcard-DNS bypass and the
local-type escape hatch. Operator opt-in via GOCLAW_ALLOW_PRIVATE_PROVIDER_URLS.
Exports security.IsBlocked as the single source of truth for blocked ranges.
Co-authored-by: Linh Vo Van <linh.vo@e-cq.net>
* feat(pipeline): add fail-closed tool call authorization gate
Gate tool execution against the server-side AllowedTools allowlist built from the
RBAC/tenant-aware filtered tool set. Resolve the tool-call prefix before the
allowlist lookup so prefixed agents are not wrongly blocked, re-check deny on lazy
MCP activation, and expand IsDenied to cover aliased tool names.
Co-authored-by: Huy Doan <tui@pm.me>
* fix(security): expand file-serve deny-list defense-in-depth
Add absolute-path deny prefixes (/home, /Users, /srv, /var/lib, /var/www, /opt)
and an explicit fail-closed log when no file-serving boundary is configured.
Co-authored-by: Linh Vo Van <linh.vo@e-cq.net>
* fix(providers): allow claude cli executable paths
Refs: #1185
---------
Co-authored-by: evgyur <evgyur@gmail.com>
Co-authored-by: Srini <srinis.k@gmail.com>
Co-authored-by: Linh Vo Van <linh.vo@e-cq.net>
Co-authored-by: Huy Doan <tui@pm.me>
Squash merge PR #112 after resolving the project changelog conflict with current dev. Local checks passed: config gateway tests, provider/http/tools deny-pattern tests, go build ./..., and go build -tags sqliteonly ./.... PR CI run 26704832350 passed release-versioning, go, and web.
Resolve the PR #8/#9/#10 stack on current dev, including Bitrix24 install callback hardening, migration renumbering, duplicate-domain fail-closed routing, UI textarea/mobile cleanup, and review hardening.
