* fix(build): embed commit SHA for release provenance (#1571 part 2)
Docker builds exclude .git via .dockerignore, so buildvcs cannot read
VCS metadata and published images carry no commit information. A running
image cannot be lined up with the source commit it was built from.
Embed cmd.CommitSHA at link time (maintainer-endorsed option 2) and
surface it in goclaw version output:
- cmd: add CommitSHA var, print commit in version cmd when injected
- Makefile: pass git rev-parse HEAD via LDFLAGS
- Dockerfile: accept COMMIT_SHA build arg (default unknown)
- docker-compose.yml: pass GOCLAW_COMMIT_SHA through as build arg
- release workflows: inject github.sha into release and dev-beta builds
Backward compatible: binaries built without the flag keep the existing
version output format.
* fix(build): pass COMMIT_SHA to docker image builds, surface it in doctor/upgrade
Review follow-up for PR #1590:
- Add COMMIT_SHA=${{ github.sha }} to the build-args of every
docker/build-push-action step (release.yaml, dev-beta-release.yaml,
release-beta.yaml, fork-image.yaml) so published images — the
artifact issue #1571 is about — carry the commit, not just release
tarball binaries.
- Surface the commit in doctor and upgrade output (App version line)
via a shared commitSuffix() helper, so operators can read provenance
from logs without exec'ing goclaw version (review suggestion #2).
- version cmd refactored onto the same helper; output unchanged.
Runs on models without a registered tokenizer (e.g. 9router brand models)
ended with the generic "Agent couldn't generate a response" fallback even
though the real request used about 55% of the context window.
PruneStage counted history with TokenCounter, which falls back to a
chars/2 heuristic for unregistered models and overcounted about 1.8x.
Once over budget it ran memory flush (~35s, invisible in traces), then
mid-loop compaction, which cannot summarize a history made only of tool
call/result pairs. The callback reported the untouched history as
compacted, PruneStage still saw it over budget and returned AbortRun
before any LLM call, and FinalizeStage replaced the empty reply with the
fallback.
- PruneStage and ContextStage overhead count with the request guard's
BudgetCounter. PruneStage no longer controls loop flow; the final
request guard in ThinkStage decides.
- CompactMessages returns ErrNotCompacted when history is unchanged.
Callers stop counting it as a compaction and do not retry it in the
same run, while post-run summarization still sees the pressure.
- When the guard exhausts every reduction step, ThinkStage stops the run
with a localized chat.context_budget_exceeded notice instead of an
error, so the run's tool results are still persisted. The stop reason
marks the trace and agent span as error; team tasks, cron and
heartbeat treat it as a failure via RunOutcome.Failure().
- Memory flush and mid-loop compaction emit event spans.
- Web and desktop UIs treat an unset context_pruning as enabled (the
backend default since 7639a8c0), keep it unset when untouched, and can
re-enable pruning after it was turned off.
Register Requesty (https://router.requesty.ai/v1) next to OpenRouter on the
existing OpenAI-compatible transport: config and env vars
(GOCLAW_REQUESTY_API_KEY, GOCLAW_REQUESTY_BASE_URL), secret masking, DB and
in-memory registration, CLI setup, doctor, placeholder provider, OpenAPI enum,
Web/Desktop provider lists and docs.
The models list merges Requesty managed policies (GET /models/managed) with
the key's catalog from GET /models.
TestHandleTeammateMessageSchedulesStreamedRun fails CI intermittently under
-race. Two separate races, both in the test rather than in what it exercises:
1. It shared `gotReq` between the scheduler's RunFunc and the assertions. The
RunFunc runs on a scheduler goroutine, and the announce loop schedules a
second run after the teammate one, so the write could land while the test
was reading — and the second run also closed an already-closed channel.
Requests now arrive over a buffered channel: no shared state, and a second
run cannot clobber the first.
2. The deferred sched.Stop() ran while handleTeammateMessage's background
goroutine was still calling Schedule, so Lane.Submit's wg.Add raced
Lane.Stop's wg.Wait. The gateway drains BgWg before stopping the scheduler
(gateway_consumer.go waits on it; sched.Stop is an outer defer in
gateway.go); the test skipped that step. It now drains too, which also makes
the test match the shutdown order it is meant to represent.
Reproduced before the fix with `-race -count=60 -cpu=1,4` (fails within a few
iterations) and clean afterwards over `-count=200 -cpu=1,2,4`, plus the whole
cmd package under -race.
Nothing in production changes; `Stream: true` and the channel-manager assertion
are untouched.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(delegate): add action=list, scoped to the originating chat
Fixes#1545. A delegation result was addressable only by the UUID returned once
in a tool result, which the calling model had to carry forward by hand. One
mistyped character orphaned a completed, durably stored result with no way back:
`get` answers "delegation result not found", and there was nothing else to ask.
Observed in production with a 31B-class caller — one flipped character, and
separately a splice of the previous delegation's tail onto the next one's prefix.
`spawn`, the sibling async mechanism over the same table, has had list/wait/cancel
all along; `delegate` had delegate/get.
Scope is tenant and calling agent, as get already resolves, plus the origin chat.
The chat rather than the session, for three reasons:
- It survives a session reset. Deferring long work, clearing the context and
coming back to ask for status is ordinary use; a SessionKey predicate would
return nothing exactly then — when the handle is most likely already lost.
- It keeps chats apart, which is the enumeration boundary #1525 is about: there
spawn's list filters on the parent agent key alone and ignores the session,
so one chat reads another chat's task text.
- It does not carry a conversation between chats. A delegation raised in a team
chat stays visible in that team chat and does not surface in someone's DM
with the same agent. History stays where it began.
In a direct chat that separates users as well, since the chat ID is per person.
Group chats deliberately show the group what the group started.
get is left as it was, deliberately. #1525 is an enumeration defect — no prior
knowledge needed and task text is disclosed. get is access through an unguessable
handle, and adding a predicate there would break fetching a result by an ID kept
across a reset, which is the very failure this fixes.
No schema change: the origin fields are already persisted by
createDelegateCompletion. ListByParent is filtered in Go behind a cap of 20,
which suits handle recovery; a dedicated predicate would be the next step if this
ever needs to page.
Tests pin the chat boundary, the session-reset case, refusal when there is no
chat to scope to (without querying the store), and the cap. The fake store leaves
ListBySession embedded and nil, so a refactor back to session scoping panics
rather than passing quietly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(delegate): list needs its own store query — ListByParent excludes delegations
The action shipped in 506ecba4 always returned an empty list. It read through
SubagentTaskStore.ListByParent, whose SQL carries
AND COALESCE(metadata->>'completion_kind', 'subagent') <> 'delegate'
ListBySession carries the same clause. Both serve spawn and filter delegations
out on purpose, so no listing in the store could return one — only Get by ID
reaches a delegation. The feature was a no-op in production while its unit tests
were green, because the fake store returned whatever rows the fixture supplied
and never reproduced the predicate that does the damage.
Found by running it against a live cluster: an async delegation was created,
`get` returned it completed with its result, and `list` reported zero.
Adds ListDelegationsByChat to the interface and to both implementations, with
the inverse predicate plus origin_chat_id, and points the tool at it. Chat scope
and delegate-only selection now live in the query rather than in a Go filter over
whatever the store happened to return; an empty chat yields no rows instead of
falling back to everything.
Tests are where the fix matters most:
- internal/store/sqlitestore exercises the real SQL. It pins that
ListDelegationsByChat returns the chat's delegation, that a spawn in the same
chat is not one, that another tenant's identically named chat stays invisible,
and — the part that would have caught this — that ListByParent and
ListBySession still do not return delegations, so the complementarity is
documented rather than assumed.
- the tool's fake now panics if ListByParent or ListBySession is called, so a
regression to either fails loudly instead of quietly listing nothing, and it
applies the chat and kind predicates itself so fixtures behave like the store.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Team tasks are not dispatched inline: the turn's PendingTeamDispatch
collects them and the post-turn drain assigns and dispatches them once
the turn ends. That same tracker also owns the per-(team, chat) create
lock taken by team_tasks list/search.
Async delegations and async spawns keep the caller's context values
(context.WithoutCancel) but run detached — after the caller's turn has
already ended and drained. A team lead reached through delegate therefore
adds every task it creates to a tracker nobody will drain again, and
takes a create lock nobody will release. Two symptoms follow:
- tasks stay pending forever and are never dispatched, so the team never
starts work. The task ticker does not cover this: it never dispatches
pending tasks, it only marks them stale after 2h and asks the lead to
retry by hand.
- the next team_tasks list/search for the same (team, chat) blocks on the
never-released mutex for the remaining lifetime of the process.
Give each detached child run its own tracker through the existing
InjectTeamDispatch helper, drained when that run ends. The synchronous
delegate/spawn paths are deliberately left alone: they execute inside the
caller's turn, where the caller's tracker is the correct owner.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(agent): filter skill slash commands by the agent's grants
The inline <available_skills> block was already filtered by visibility and
agent grants, but slash activation read the loader's full skill list. A
/<slug> command could therefore activate a skill the agent was never granted,
and the not-found suggestions disclosed that such skills existed.
Resolve slash commands against the agent's allow list via FilterSkills. The
list is filtered before matching, so /list-skills, /help and the near-match
suggestions are all covered by the same change.
The allow-list convention is unchanged: nil means every skill (the fallback
when the access store errors), an empty slice means none, and a populated
slice is an explicit set of slugs.
* fix(agent): split slash commands on any whitespace
The tokenizer separated the skill name from the rest of the message with
strings.Cut(after, " ") — a literal space. A user who typed the command and
pressed Enter before the rest of the message sent "/ck:git\nreview the diff",
which parsed to the target "ck:git\nreview" and matched no skill. The reply
was "skill not found" followed by near-matches that included the skill they
had just named, because the similarity fallback searched the mangled string
and still landed beside it. Multi-line messages are ordinary in Slack and
Telegram, so this was reachable in normal use.
Introduce cutFirstField, which splits on the first run of whitespace, and use
it everywhere the literal-space split appeared. The match loop had the same
assumption in strings.HasPrefix(raw, value+" "); hasFieldPrefix replaces it
and decodes the following rune properly, so a multi-byte skill name is not
truncated. That also stops a plain string prefix from matching: a command for
"frontend-design-extra" no longer resolves to "frontend-design".
* fix(tools): inherit the parent agent's tool policy in spawned subagents
buildSubagentToolsRegistry clones the parent registry, then overwrites exec,
read_file, write_file and list_files with freshly constructed tools. A fresh
tool carries none of the hardening the gateway applies to the parent's
instances at startup: exec path denials and their exemptions, the shell
deny-group toggles, the command keyword allowlist, and the read/write/list
deny prefixes covering config.json, the internal databases and delegate/.
Spawning a subagent therefore widened what an agent could reach — the parent
was blocked from the data dir, the subagent was not. Verified by disabling
the new call: the subagent's exec read config.json out of the denied data
dir, read_file did the same, and write_file overwrote it.
Copy the policy from the live parent instances rather than re-deriving it, so
there is one source of truth and a deny path reloaded later through config
pub/sub reaches subagents without a second wiring site that can drift.
Allow-prefixes are inherited alongside the denials: they are what make the
denied roots usable at all, since the skills store sits under the denied data
dir. Copying denials without them would leave a subagent unable to read the
skills it is told to use.
* fix(agent): use one inline-vs-search decision for prompt and preview
The system-prompt preview exists to show the prompt an agent actually gets,
but it decided between inline skills and search mode on its own terms: it
counted tokens with the fallback counter over the fully rendered XML — tags
and <location> paths included, at roughly runes/2 — while the prompt builder
estimated name+description at chars/4. On the same eighteen skills that read
3087 against 944, so the preview reported search mode for an agent that was
running inline.
Extract shouldInlineSkills and call it from both paths. The estimate keeps
the prompt builder's rule, mirroring BuildSummary's 200-rune description
truncation, so the decision still costs no rendering.
The preview's loader interface gains FilterSkills to feed it. Its behaviour
on an access-store error is unchanged: an empty allow list yields no skills
rather than falling back to showing every skill.
* fix(http): keep reference files when importing skills
Export archives the whole skill directory, but import recognised only
metadata.json, SKILL.md and grants.jsonl. The switch had no default, so every
other file was discarded without a log line. A skill whose SKILL.md cites
references/*.md arrived without them, the import reported success, and the
skill failed at the first read.
Collect the remaining entries and write them under the skill directory with
their structure intact. Archive entry names are attacker-controlled, so each
path goes through sanitizeRelPath: a traversal attempt collapses to a
relative path and the write stays inside the skill directory.
* test(agent): make the inline-decision table exercise both gates
The "100 skills, 200-char descriptions" case claimed to cover the token
ceiling, but 100 exceeds the count ceiling so the count check short-circuited
and the token branch never ran — deleting that branch would not have failed
the test.
Split the table so each case isolates one gate: tiny descriptions for the
count rows, a count safely under the ceiling for the token rows. Removing the
token comparison now fails the over-the-ceiling case and nothing else.
* fix(agent): gate slash commands on the managed tier only
The allow list comes from a query over the `skills` table, so filesystem-tier
skills — the workspace, .agents and ~/.agents directories of the five-tier
loader — have no row in it. Filtering every skill against the list made those
four tiers unreachable by slash while skill_search still found them
unfiltered, which turned a security fix into a functional regression.
Apply the list to managed skills only. Builtins are seeded with is_system and
returned unconditionally, so they stay reachable either way.
This closes slash activation of ungranted managed skills. It does not close
skill_search and use_skill, which still read the loader's full list; that is a
wider change and wants its own review.
* fix(http): stop imported skill files from replacing guarded ones
Two ways the auxiliary write could go wrong.
GuardSkillContent scans SKILL.md before anything reaches disk, but the switch
that routes archive entries compares the raw path. An entry named
"./SKILL.md" does not match it, so it fell through to the auxiliary set —
where sanitizeRelPath collapses it back to "SKILL.md" and the write replaced
the file that had just been scanned. Reject any auxiliary path that resolves
to a name the import handles by itself, and log the attempt.
Tar directory entries carry a trailing separator and no content. Written as
files they took the name a real directory needed, so everything beneath was
dropped — and whether that happened depended on map iteration order, making
it intermittent. Skip them; MkdirAll creates what is needed.
Also cap the number of auxiliary files per skill and log when the cap trims
an archive, so a truncated import is visible rather than silent.
---------
Co-authored-by: mor-phongdt <phong.dangtuan@mor.com.vn>
Review on #1533 asked for coverage of the behavioural contract rather than
the field assignment, and that is the right call — the PR text arguing a test
would only restate the assignment was wrong, because the contract has two
halves that a refactor could break independently.
The test drives handleTeammateMessage through a real scheduler and asserts
both: the scheduled run requests streaming, and the run is not registered with
the channel manager, so its chunks cannot be routed to a user. It fails with
Stream reverted to false.
The remaining half — a streamed call still yielding the complete final response
— is already covered in internal/agent/loop_pipeline_callbacks_test.go.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
handleTeammateMessage pinned Stream: false, so every team member run — coder,
reviewer, researcher — made a non-streamed provider call. On a slow reasoning
model that means a silent connection for the whole generation, and
ResponseHeaderTimeout eventually kills it:
iter 0 think: llm call: litellm: request failed:
Post ".../v1/chat/completions": http2: timeout awaiting response headers
Observed on a member asked to write a large single-file page: 15 minutes, zero
output tokens, run failed. The same model over the same provider is fine on an
ordinary channel run, which streams by default.
Streaming here is for connection liveness, not delivery. A teammate run is never
registered with the channel manager, so HandleAgentEvent returns on its first
line and the chunks are dropped; nothing reaches a user incrementally. The task
result is unchanged — it still comes from the final RunResult, since ChatStream
returns the complete response once the stream ends.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Model listing and chat provider construction rewrite localhost to
host.docker.internal via config.DockerLocalhost() when running inside
Docker, but all three memory.NewOpenAIEmbeddingProvider call sites
(verify-embedding handler, runtime memory embedding) passed the raw
api_base through. In Docker the container resolved localhost to itself,
so Ollama Verify Embedding and runtime embedding failed with connection
refused while the model dropdown loaded fine.
Centralize the rewrite inside the constructor so no call site can miss
it, add a SetInDockerForTest hook for environment-independent regression
tests, and pin Docker detection off in httptest-based suites.
Fixes#1519
The CLI agent chat command connects via WebSocket but never sends
user_id in the connect params. The gateway requires user_id for
chat.send, making the CLI unusable for any agent interaction.
All other clients (browser, Telegram, LINE, etc.) send user_id
during connect. The CLI was the only channel missing it.
Adds --user / -u flag that passes user_id in the connect frame.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vault_read is a singleton wired once at boot with the global workspace
root, then joins doc.Path (stored relative to the TENANT root) onto it.
For master tenant both roots coincide, so the bug stayed invisible; every
other tenant reads ENOENT because its files live under tenants/<slug>/.
vault_search is DB-only and keeps returning the doc, so the symptom is
"search finds it, read fails" rather than an obvious missing file.
Resolve the tenant-scoped root per request via TenantLayer, which is a
no-op for master and preserves existing single-tenant behaviour. Passing
the tenant root into resolvePath also narrows the boundary check, so it
now doubles as cross-tenant isolation.
Runs originating from channels or cron may not carry the tenant slug in
context; without it TenantLayer falls back to a UUID-named directory that
does not match the slug-named one the write path creates. Add a nil-safe
TenantStore lookup for that case only.
SSRF protection resolves a hostname and judges the resulting IP. That
model assumes DNS resolution describes where the traffic actually goes,
which stops being true behind a TUN/fake-IP proxy: every query is answered
with a synthetic address out of a reserved range, and the proxy then
routes that address to the real public host. The IP is a handle, not a
destination.
In that environment web_fetch rejects ordinary public sites — observed
with news.sina.cn resolving to 198.18.0.236 — and no configuration can
fix it, because the block list is compiled in. The agent then burns
iterations retrying URLs that can never succeed.
GOCLAW_SSRF_ALLOWED_CIDRS lets an operator name the ranges their proxy
hands out. Empty by default, so nothing changes for deployments that do
not set it, and the accepted and refused entries are both logged at
startup — this widens what LLM- and admin-supplied URLs can reach, so it
should be visible.
Ranges an SSRF actually targets can never be allowlisted: link-local
(including cloud metadata at 169.254.169.254), multicast and unspecified
are refused at parse time, in either direction, so neither an exact entry
nor a wider range that swallows one gets through.
Applied inside isBlocked rather than only in validate() because
NewSafeClient re-checks the pinned IP at dial time through the same
function — relaxing just the pre-flight check would pass validation and
then fail to connect.
internal/tools carries its own private-range list for web_fetch and
web_search, separate from this package and not identical to it. It has to
consult the same setting, or relaxing one gate leaves the other rejecting
the very traffic the operator permitted. Unifying the two lists is left
alone here; it is a wider change than this one.
Co-authored-by: Conner Mo <connermo@ConnerdeMacBook-Pro.local>
Added error handling for input reading in the agent chat client and for database row reading in the doctor commands, ensuring that errors are reported clearly to the user.
The v3 pipeline compacts session history mid-loop (prune_stage +
final-request guard) but only mutates the run's message buffer, never the
session store. Each turn reloads full history and re-compacts from scratch:
message_tokens climb 129k->156k across turns while every turn compacts back
down to ~60k. The lossy compaction differs per run, degrading the agent.
The same missing persistence stalls episodic memory: the cumulative
compaction count never advances, so the episodic worker's idempotency key
(sessionKey:count) is pinned and every cycle after the first is skipped.
Observed on live traffic: 8 run.completed since deploy, 0 new episodic.
Fixes, all reusing existing machinery (no new store methods, no migrations):
- Bug A: emitSessionCompleted reads cumulative GetCompactionCount (matching
the legacy v2 path) instead of the per-run counter that resets to 0.
- Bug B/anti-loop: finalize passes state.Prune.MidLoopCompacted into
maybeSummarize; under pressure it lowers the trigger to a unit-aligned
threshold (compactionInputCap - overhead, same MaxRequestShare the guard
uses) so the compaction is PERSISTED via the existing TruncateHistory +
IncrementCompaction path. Defensive floor prevents over-compaction on
pathological config; tool-result-only bloat still skips (history-only).
- Bug C: SourceID embeds the count (sessionKey:count) so the eventbus dedup
key advances per compaction cycle instead of swallowing rapid same-session
turns within the 5m TTL.
Tests: episodic compaction, maybe_summarize pressure, request budget.
go build (PG + sqliteonly), go vet, go test -race all green.
The dashboard /tts page writes to system_configs[tts.<provider>.voice],
but the LLM-invoked tts tool was only checking args > agent OtherConfig
> builtin_tool_tenant_configs[tts].default_voice_id. Two different
storage locations → the user's chosen voice was ignored, Edge defaulted
to en-US-AriaNeural even when "HoaiMy" (vi-VN-HoaiMyNeural) was set
correctly in the dashboard.
Add system_configs as a 4th-level fallback so the dashboard becomes
the single source of truth.
- TtsTool gains SetSystemConfigStore(s) setter
- resolveVoiceAndModel takes providerName + looks up
tts.<provider>.voice/model when no higher-precedence source set
- effectiveProvider resolved BEFORE voice/model so the right key is hit
- Wired at gateway boot in cmd/gateway.go (after pgStores ready)
- 4 unit tests covering: fallback, arg precedence, no-store, empty provider
Verified against production trace 019e6036-44bb-703b-85fa-dee34f7ab2c0
where the tts tool was called with provider=edge, no voice arg, and
defaulted to English instead of the configured Vietnamese voice.
* feat(audio): add openai_compat TTS/STT provider for self-hosted endpoints
Adds a provider pair speaking the OpenAI audio wire format against any
compatible endpoint (gpu-manager, Speaches, vLLM, LocalAI, llama.cpp,
Ollama), so a self-hosted engine no longer needs a bespoke HTTP shim
implementing goclaw's proprietary /transcribe_audio contract.
Kept distinct from the openai package on purpose. Both speak the same
wire format, but "openai" carries api.openai.com semantics, and
audio.IsVoiceCompatible applies OpenAI's voice allowlist to any provider
by that name — FilterVoiceForProvider then silently substitutes "alloy"
for anything outside it. A self-hosted voice ID such as Piper's
"fr_FR-gilles-low" would be replaced with no error and no log line, and
the caller would hear the wrong language. Providers without validation
rules pass through untouched, which is the correct behaviour for an
engine whose voice namespace is its own.
Other deliberate departures from the openai package:
- api_base is required. There is no public endpoint to fall back to, and
defaulting to api.openai.com would send self-hosted traffic to a vendor.
- An empty api_key omits the Authorization header rather than sending an
empty Bearer token; self-hosted engines commonly have no auth.
- Empty voice/model are omitted from the request instead of sent blank,
since some engines resolve the model from the requested voice.
- The multipart filename is never blank: several engines type the audio
by extension rather than Content-Type and reject what they cannot type.
Endpoint error bodies are preserved in the returned error so a format
mismatch (e.g. an engine that cannot encode mp3) is diagnosable.
* feat(audio): wire openai_compat TTS/STT provider into gateway config
Adds tts.openai_compat config block and registers both providers at
startup. api_base is the enable switch rather than an API key, since
self-hosted endpoints commonly have no auth and there is no vendor
default to fall back to.
The STT chain is now built from the providers that actually registered
instead of being hardcoded to {elevenlabs, proxy}. Transcribe skips
unregistered names with a warning on every call, so a static chain
naming absent providers is log noise on the hot path. openai_compat goes
first when present: it is an explicit operator choice and keeps audio on
the local network. "proxy" stays last, as BridgeLegacySTT registers it
later per channel.
Behaviour is unchanged when tts.openai_compat.api_base is unset: the
chain resolves to {elevenlabs, proxy} exactly as before.
---------
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
* feat(mcp): expand CRUD MCP server to near-complete CLI parity
Closes the CLI-vs-MCP tool coverage gap identified by auditing every
`goclaw` CLI command against the existing goclaw_* MCP tool set. Adds
skill grant/revoke, agent skill pin/unpin, and full CRUD/inspection
surfaces for memory, knowledge graph (including dedup/merge/prune),
tenants, providers, LLM traces, channel contacts, pending messages,
audit activity, system config, tenant storage (list/size/delete/move),
scoped agent config export/import, secure-CLI binary registry, and a
DB-backed health check.
Deliberately out of scope, documented inline where relevant:
- `goclaw credentials`: confirmed CLI-local (~/.goclaw/config.yaml +
keychain), no server resource to wrap. goclaw_secure_cli_binaries_*
covers the closest real, previously-uncovered server resource instead.
- Full tar-archive agent export/import (KG + workspace files): the CLI's
version streams a multi-section archive with progress events, a shape
that doesn't map to a single MCP tool call. Config + context files
(the portable "brain") is covered.
- `kg extract` (LLM-driven text extraction): goclaw_kg_ingest accepts
the same Entity/Relation shapes the extractor produces, so a caller
can run extraction itself and hand off the result.
Wires 8 new store dependencies (Memory, KnowledgeGraph, Tracing,
Contacts, PendingMessages, Activity, SystemConfigs, SecureCLI) through
gateway.Server setters -> cmd/gateway.go -> CRUDDeps, following the
existing Providers/Tenants pattern. Storage and secure-CLI-binary
handlers duplicate internal/http's path-escape/symlink-hiding
validation logic (documented inline) since internal/http already
imports internal/mcp and the reverse would cycle.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(mcp): return sessionKey in goclaw_chat_send response for persistent conversations
The goclaw_chat_send tool creates a new session internally when sessionKey is
empty, but never returned the key to the caller. This prevented using
goclaw_chat_history to fetch previous messages in the session.
Add SessionKey field to ChatSendResult so callers can:
1. Start a new agent chat without providing sessionKey
2. Receive the sessionKey back in the response
3. Use that sessionKey for follow-up messages and history queries
Fixes the training loop pattern: start chat → get sessionKey → call
goclaw_chat_history with that key → iterate skill based on actual failures.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* feat(mcp): add timing diagnostics to goclaw_chat_send for timeout troubleshooting
Log request arrival, processing duration, and errors with millisecond precision.
Helps identify whether timeouts occur at MCP client→goclaw, goclaw→ollama, or
during agent execution. Critical for production debugging.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
* fix(gateway): add http.Server timeouts for defensive timeout handling
Set explicit timeouts in http.Server:
- ReadTimeout: 1h (allow large uploads, long-running agent operations)
- WriteTimeout: 1h (allow streaming responses to slow clients)
- IdleTimeout: 30s (close idle keep-alive connections quickly)
Provides defense-in-depth when Nginx/Traefik timeouts are misconfigured.
Matches Nginx timeout (3600s) to prevent race conditions.
Timeout chain: traefik 3600s = nginx 3600s = goclaw 3600s
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
---------
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat(hooks/observe): add structured output validation hook (ObserveHook)
- hooks/types.go: Add ObserveHook interface + BuiltInHookType enum
- hooks/dispatcher.go: HookDispatcher emits ObserveHook with PhaseResult payload
- pipeline/observe_stage.go: ObserveStage emits hook after ObserveResult built
- pipeline/substates.go: ObserveStageResult carries hook results + validation errors
- hooks/config.go: BuiltInHookTypeObserve added to BuiltInHookType enum
PhaseResult carries structured output, token usage, tool calls + validation errors
emitted post-ObserveStage. ObserveHook implementations can validate structured
output against schemas, detect tool-call loops, enforce token budgets, etc.
Hook fires after ObserveStage produces ObserveResult, before results propagate
to next stage. ValidationError returned by hook halts pipeline and propagates
error to caller without further stage execution.
Co-Authored-By: Claude <noreply@anthropic.com>
* ui(hooks): add post_model_response event to web UI
- Add event to Zod schema, filter dropdown, and form dialog
- Implement conditional test panel UI for model response payload
- Add translations (en/zh/vi) for new test panel fields
- Updated beta description to reference the new event
Co-Authored-By: Claude <noreply@anthropic.com>
* feat(mcp): add MCP CRUD server exposing goclaw resources at /api/mcp/ with Bearer token auth
and X-GoClaw-Tenant-Id header, default to master tenant
* feat(mcp): add goclaw_skills_write_file tool to edit skill files on disk
The CRUD MCP server's goclaw_skills_update only touched skill DB metadata,
with no way to edit a skill's SKILL.md/file content on the filesystem. Extract
the versioned write logic from the web UI's skill file editor
(SkillsHandler.handleWriteFile) into skills.WriteVersionedFile so both
surfaces share identical validation and versioning, and expose it as a new
MCP tool.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
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>
/bitrix24/* only mounted when a bitrix24 Channel loaded successfully via
WebhookHandlers(), which requires portal + bot_code + bot_name already
set. But completing portal OAuth requires /bitrix24/install to be
reachable, and the UI won't let you pick an uninstalled portal when
creating a bot -- deadlock on any fresh deployment: no bot can load
until OAuth completes, OAuth can't complete without the route mounted.
Claim and mount the shared bitrix24.WebhookRouter() singleton directly
at boot, independent of whether any Channel loaded. ClaimWebhookRoute
is idempotent (first-claim-wins via CompareAndSwap), so this is a
no-op once a real Channel has already claimed the route.
Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>
- Reparent tool_call spans under their producing llm_call span via
RunState.CurrentLLMSpanID so a model turn's trace visibly contains the
tool calls it triggered.
- Wire internal/hooks EmitHookSpan into the dispatcher writeExec so every
hook execution (pre/post tool use, etc.) produces a trace span with input,
console output, decision, error, and duration for agent troubleshooting.
- Fix usage_events_span_id_fkey violations: usage events were inserted
synchronously referencing a span flushed ~5s later, silently dropping
token/cost data. Route them through the collector so they flush after
spans in the same cycle. No schema migration; FK preserved.
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Adds a "react" action to the message tool so an agent can set an emoji
reaction on an existing message (e.g. mark its own status post 👍 once
fully paid). Mirrors the ChannelEditor wiring: ReactionSetter capability
-> Manager.ReactToMessage -> telegram Channel.ReactToMessage (validates
against Telegram's supported reaction set; ✅/❌ are rejected).
Co-authored-by: skensel <skensel@MacBook-Pro-skensel.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Fix image_generation nil-pointer crash on codex agents: the sentinel now
carries a name-only Function so the many sites reading td.Function.Name never
nil-deref; codex_build still branches on Type.
- Agent-declared trigger words in IDENTITY.md wake the bot in groups without an
@mention (whole-word, Cyrillic-aware; text + caption), cached per-agent 60s.
- channel_post support with a synthetic sender + a recover() guard so a
malformed update can't crash the gateway.
- message tool action=edit (editMessageText + editMessageCaption fallback),
targeting the replied-to message via reply_to_message_id.
- message tool topic=<name> posts into a named forum topic; topics learned from
forum_topic_created into channel_contacts and resolved by name.
Co-authored-by: skensel <skensel@MacBook-Pro-skensel.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bitrix.portals.create only persisted the DB row — it never called
Router.RegisterPortal, so a portal created while the gateway was
already running stayed invisible to PortalByDomain/PortalByKey until
the next full restart. The only other RegisterPortal call site is
BootstrapPortals, which runs once at boot. Result: the OAuth install
callback 404'd with "unknown portal" for any portal created through
the self-service UI mid-uptime — hit live on web1trang.bitrix24.com.
Fix: inject registerPortal/unregisterPortal func fields into
BitrixPortalsMethods (mirrors the existing gatewayPublicURL func()
pattern), defaulting to bitrix24.WebhookRouter() + NewPortal/
RegisterPortal (same construction path as BootstrapPortals) and
Router.UnregisterPortal respectively. handleCreate calls
registerPortal after a successful Create; handleDelete calls
unregisterPortal after a successful Delete for symmetry. Both are
best-effort — a registration failure is logged, not returned as an
RPC error, since the DB row is already valid either way.
Injected via func fields (not calling bitrix24.WebhookRouter()
inline) so tests can stub live-router registration without touching
that process-wide singleton, whose test-reset helper isn't exported
outside the bitrix24 package.
Tests: 4 new cases covering register-on-create, best-effort failure
handling, unregister-on-delete, and no-unregister-when-delete-blocked.
All 20 tests in bitrix_portals_test.go pass; go build (both PG and
sqliteonly tags) and go vet clean.
Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>
The provider verify call and the provider models-list call used hardcoded
timeouts (30s and 15s) that were too low for slow models. Introduce a
tenant-scoped, UI-editable setting `providers.request_timeout_sec`
(default 30) stored in system_configs, consumed by both handlers, and
overlaid from config.json like tts.timeout_ms. Adds a field to the System
Settings modal (all 4 locales) so operators can raise it without a
redeploy.
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
Eleven built-in tools were registered in the runtime tool registry (and
thus appeared in agent system prompts and were subject to allow/deny) but
were absent from builtinToolSeedData(), so operators could not see or
toggle them in the agent allow/deny UI. Seed datetime, heartbeat,
memory_expand, list_group_members, zalo_list_groups, vault_search,
vault_read, delegate, workstation_exec, claude_remote, and mcp_tool_search
so the UI catalog reflects the actual registered tool set. telegram_manager
is intentionally excluded per existing test guard.
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
The system prompt's Tooling section previously listed tools even when
they were disabled for a tenant and already stripped from the API
tools parameter, confusing the LLM into thinking it could use them.
Co-authored-by: Bruno Clermont <bruno.clermont@gmail.com>
* fix: resolve Zalo group name to real chat ID, notify origin on failed forward
Two related root causes behind "forward message to group by name" silently
failing while the agent reports success:
1. No tool let the agent resolve a group's display name (e.g. "Ban Dieu
Hanh") to the chat ID the message tool actually requires. sessions_list
only exposes session keys (numeric group IDs), never human-readable
names, so the agent had no reliable way to turn a name into a real
target and ended up passing the display name itself as `target`.
Adds zalo_list_groups, wrapping the already-used (dashboard picker)
protocol.FetchGroups behind the same optional-interface pattern as
list_group_members/GroupMemberProvider (GroupListProvider on
channels.Manager, gated to zalo_personal via RequiredChannelTypes).
2. When the resulting send fails downstream (e.g. Zalo rejects a bad
chat_id), dispatchOutbound only ever retried/notified media failures on
the same (already-broken) destination, and dropped text-only failures
entirely — even though message.go's own postCrossTargetNotice comment
states forwards must never announce a fake delivery. Because the bus
publish is fire-and-forget, the tool had already returned "sent" and
announced success to the origin chat before the real send was even
attempted.
message.go now tags cross-target forwards with origin channel/chat in
OutboundMessage.Metadata; dispatchOutbound uses it to notify the ORIGIN
chat with the real failure instead of silently dropping it or retrying
against the same invalid target.
* test: cover forward-origin metadata tagging and dispatch failure notice
Extracts dispatchOutbound's error branch into handleSendFailure so it can
be unit tested without driving the consumer loop/goroutine, and adds
coverage for: forward failures notifying the origin chat (not the broken
destination), pre-existing non-forward media/text-only behavior staying
unchanged, message.go tagging cross-target group forwards with origin
metadata alongside group_id, and the new zalo_list_groups tool/Manager
delegator.
* feat(bitrix24): replace two MCP text inputs with a filtered dropdown [B24:2794]
Bitrix24 channel creation used to demand two hand-typed strings —
mcp_server_name and mcp_base_url — plus zero indication of which MCP
servers can actually auto-onboard. Typos silently disabled provisioning
and admin had to know which servers implement /api/auto-onboard.
Ship a single dropdown backed by mcp_servers.require_user_credentials,
plus the machinery to make it work end-to-end.
Phase 1 — DB & store
* Promote require_user_credentials from settings JSONB to a top-level
column on mcp_servers (PG migration 000089, SQLite migration v54).
* Backfill from existing settings blobs so no admin needs to re-tick.
* Add MCPServerData.RequireUserCredentials to the Go store layer, plumb
through Create / Get / GetByName / List / Update on both stores,
extend the export DTO, and add require_user_credentials to the HTTP
allowlist.
* Bump RequiredSchemaVersion 87 -> 89 (jumping 88, which was on disk
but not wired) and SchemaVersion 53 -> 54 with an
idempotentColumnMigration guard.
Phase 2 — Bitrix24 channel factory
* Add MCPServerID (UUID string) to bitrix24 InstanceConfig, keep
MCPServerName + MCPBaseURL as legacy fallback with a "deprecated"
doc comment.
* Factory validation accepts either mcp_server_id alone or the legacy
pair; half-config still fails fast.
* initMCPProvisioner prefers GetServer(id) and sources the base URL
from mcp_servers.url when the id path is used. Legacy name path
unchanged so pre-migration configs keep working.
* Log line now carries mcp_server_id + require_user_credentials so
operators can eyeball the wiring.
Phase 3 — Frontend types & MCP form
* Add optional top-level require_user_credentials to MCPServerData /
MCPServerInput in both ui/web and ui/desktop/frontend types.
* mcp-form-dialog reads the top-level flag first and falls back to
settings.require_user_credentials so cached responses from
pre-upgrade backends still render correctly.
* On submit send both the top-level flag AND the legacy settings
entry so mid-rollout backends stay consistent.
Phase 4 — Bitrix24 channel form dropdown
* New mcp-select field type + MCPServerSelect component. Uses the
shared useMCP() react-query cache and filters client-side to
servers whose require_user_credentials is true (OR settings
JSONB during the migration window).
* Explicit "None (disable MCP provisioning)" option so admins can
clear the binding without editing config JSON.
* Legacy mcp_server_name / mcp_base_url text inputs kept in the
Advanced panel, relabelled "(legacy)" with pointer help text.
Phase 5 — channel_instances.config backfill
* PG migration 000090 and SQLite migration v55 rewrite existing
bitrix24 channel_instances.config to add mcp_server_id by
resolving mcp_server_name against mcp_servers, tenant-scoped
via agents.tenant_id (channel_instances doesn't carry tenant_id
directly).
* Idempotent — only touches rows already carrying
mcp_server_name that lack mcp_server_id. Legacy keys are left
in place so provisioner.go can still fall back for unmigrated
or future-created legacy configs.
* down.sql drops the mcp_server_id key. Provisioner immediately
reverts to the legacy fallback path.
Tests
* provisioner_test.go: three new cases exercise the mcp_server_id
path (invalid UUID string, valid UUID with missing row, valid
UUID with a per-user row). fakeMCPStore gains a serversByID
map and a real GetServer implementation.
* Existing legacy-config tests unchanged and still green.
Verification
* go build ./... && go build -tags sqliteonly ./...
* go vet ./internal/mcp/... ./internal/channels/bitrix24/...
./internal/store/... ./internal/http/...
* go test ./internal/mcp/... ./internal/channels/bitrix24/... -> ok
* Live-tested against a local docker image on the goclaw-deploy
postgres. Migrations 89 + 90 applied cleanly. Three existing
bitrix24 channels (bitrix-sales / nguyen-dao-openline / tieu-vi)
had their configs backfilled with the b24-syn-mcp UUID and the
provisioner boots with require_user_credentials=true. UI dropdown
correctly shows only b24-syn-mcp (the only server with the flag
ticked) alongside a "None" clearer option.
Surface parity
* Gateway server: store + factory + provisioner + HTTP allowlist.
* API contract: adds require_user_credentials + mcp_server_id
as optional fields on existing routes. No new endpoints.
* Web UI: MCP form + Bitrix24 channel form + shared types.
* CLI/runtime package: N/A because no CLI subcommand reads the
mcp_server_id field.
* fix(bitrix24): derive auto-onboard base URL from mcp_servers.url origin [B24:2794]
The Phase 2 refactor swapped provisioner base-URL sourcing from the
legacy per-channel MCPBaseURL config field (which historically stored the
MCP server's ORIGIN, e.g. https://mcp.example.com) to mcp_servers.url,
which stores the JSON-RPC ENDPOINT the agent loop dials (e.g.
https://mcp.example.com/mcp). The two are semantically different but
share a single column.
mcp_client.newMCPClient then appends "/api/auto-onboard" to whatever
baseURL it receives, so the id-path started POSTing to
".../mcp/api/auto-onboard" — 404 for every per-user credential mint and
refresh. Existing users kept working only until their cached access
tokens expired.
Fix: derive the origin (scheme://host[:port]) from server.URL before
handing it to the auto-onboard client. The legacy path is untouched
because MCPBaseURL from channel config is already the origin.
https://b24-mcp-dev.synity.so/mcp -> https://b24-mcp-dev.synity.sohttps://mcp.example.com/mcp/ -> https://mcp.example.comhttps://mcp.example.com -> https://mcp.example.com
Table-driven test covers six shapes plus four error cases (empty,
whitespace-only, no scheme, no host). Updated the existing
TestInitMCPProvisioner_MCPServerID fixture to seed a URL with the /mcp
subpath so it regression-guards the same code path.
Verified live: user 614 sent a message that triggered the expired-cred
refresh branch; goclaw logged "self-refreshed user credentials
created=false" and the agent immediately reported
"mcp.user_tools_loaded user=614 tools=2". Before this fix the same event
logged 'auto-onboard failed: mcp auto-onboard: 404 Not Found'.
Surface parity:
- Gateway server: provisioner + one new helper (deriveAutoOnboardBaseURL).
- API contract: N/A because the wire shape hasn't changed.
- Web UI: N/A because the UI still writes mcp_server_id verbatim.
- CLI/runtime: N/A because no CLI reads the derived base URL.
---------
Co-authored-by: DangTinh311 <dangtinh31193@gmail.com>
fchengyan