346 Commits

Author	SHA1	Message	Date
Michal	527983d87e	fix(db-tests): seed a SecretBackend for Secret model tests ... The four Secret tests in models.test.ts created rows via raw prisma.secret.create without a backendId, so the schema default '' violated Secret_backendId_fkey (that default only exists so 'db push' can add the column to pre-existing rows; real writes always carry a valid FK). Seed a plaintext backend per test, the same shape bootstrapSecretBackends creates, and clear SecretBackend rows in clearAllTables so nothing leaks between tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-10-09 15:40:11 +01:00
Michal	24f202cb79	Merge origin/main into feat/agentic-teams ... Conflict in src/mcpd/src/main.ts: both sides touched the import block. Kept this branch's mapUrlToPermission plus main's bootstrapUngatedGlobal; dropped main's SET_OWN_PASSWORD_OPERATION import — this branch moved that usage into middleware/url-permission.ts, which imports it itself. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-10-09 15:36:32 +01:00
Michal	fe00f42bce	fix(ungated-global): create settings secret with a valid backend FK ... bootstrapUngatedGlobal wrote its seed marker into the mcpctl-system-settings Secret, creating the row with the default backendId '' when it did not exist. The Secret_backendId_fkey constraint is enforced at insert time, so on a DB where that secret was absent, mcpd crash-looped on startup (P2003). Fix: run the seed after bootstrapSecretBackends (so the default plaintext backend exists) and set backendId to the resolved default on create. If no backend exists at all, skip writing the marker rather than crash. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-10-03 01:57:37 +01:00
Michal	7b70c91c41	ungated-global: system project whose servers ride every project, pre-gate ... New seed-once system project `ungated-global`: every server attached to it is merged into every project's MCP endpoint in front of the gate (listed and callable before begin_session), and its project prompt is prepended to every project's instructions. - mcpd: bootstrapUngatedGlobal seeds the project once-ever (marker key `ungatedGlobalSeeded` in mcpctl-system-settings), with default server `calcmcp` (patched CalcMCP fork, mysources.co.uk/michal/calcmcp:1.1.0) and a "never do math yourself, always use calcmcp" prompt. Deletions are durable: `mcpctl delete server calcmcp -p ungated-global` stays deleted. - mcpd: GET /projects/:id/servers appends global servers flagged `ungatedGlobal: true` (direct attachment wins, no self-merge); GET /projects/:id/instructions prepends the global prompt and lists the global servers. - mcplocal: discovery surfaces the flagged names; the gate plugin exempts them — gated tools/list returns begin_session + exempt tools, calls to them pass through without ungating, and initialize instructions list them under "Available immediately (no begin_session required)". - templates/calcmcp.yaml so a deleted default can be re-created by hand. - docs/ungated-global.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-10-03 01:45:44 +01:00
Michal	979e9720c0	ambient context: give every project's model a clock ... Models consuming projects have no idea what day it is. Three always-on layers from mcplocal plus one from mcpd: - initialize instructions and the begin_session briefing carry '[context] now: <date/time> — project: <name>' (frozen at session start by design); - a get_current_time virtual tool (ungated catalogs only, same pattern as read_prompts); - every tools/call result gets a trailing '[context] now: …' content item at the endpoint send site — the only chokepoint that also covers virtual tools, gate intercepts and failure results, and the layer that keeps a days-old chat session current; - mcpd agent/project chat system blocks append the same line each turn. Time renders as UTC plus an optional local timezone from MCPLOCAL_AMBIENT_TZ (IANA name; both services read it; invalid or unset means UTC only). ~15 tokens per stamp, minute precision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-10-02 00:33:20 +01:00
Michal	6fbd9c19fc	fix(mcplocal): rebind upstream auth token on every project refresh ... A cached project router outlives the caller's bearer token. syncUpstreams only created a McpdUpstream (which binds the token at construction) for server names not already registered, so after a token rotation every pre-rotation upstream kept posting the old bearer: six attached, healthy servers 401'd on tools/list forever — no error anywhere the client could see — while the two servers attached after the rotation worked. - discovery.ts: rebuild every upstream on every sync, so it always carries the current caller token and the current server id (also fixes a stale serverId after delete+recreate under the same name). - router.ts: record per-server discovery failures with their messages and expose them (getToolDiscoveryFailures). - gate.ts: a partial catalog is now loud — initialize instructions and the begin_session response name every attached server that contributed no tools and the error that silenced it. - tests: regression tests for token rebinding and serverId refresh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-09-22 23:44:52 +01:00
Michal	859150770f	feat(servers): per-server memory ceiling, because 512Mi OOMKills silently ... Server pods have always had a hardcoded 512Mi limit. That is right for a server that proxies an API and fatal for one that drives a browser: the `docs` server (docs-mcp-server, which scrapes with headless Chromium) idles at ~228Mi and crosses 512Mi within seconds of its first scrape. An OOMKill is the quietest failure we have. The kernel kills it, the pod restarts, the readiness probe passes, and the instance reads `healthy` again — while the six scrape jobs whose queue lived in memory are gone and the index has 17 pages in it. Nothing is logged, because the process never got to say anything. memoryLimitMb is declared per server, in MiB, and converted to bytes in the container spec. NULL keeps DEFAULT_MEMORY_LIMIT, so every existing server is bit-for-bit unchanged — a bigger default would have cost real memory on every node for servers that do not need it. mcpctl create server docs --memory-limit-mb 2048 --force Tests assert the two places a new column dies quietly: the repository's field-by-field mapping (a column not mapped there returns "patched" and changes nothing) and the spec built in instance.service. docs/reliability gains the OOMKill section — the `lastState.terminated.reason` check is what turns "it restarted again" into an answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JPjtnE6Gd343oRNtMU9Bcd	2026-09-17 00:24:18 +01:00
Michal	70bc7f1598	Merge origin/main into feat/agentic-teams ... 124 commits of main against 33 of agentic-teams. 22 files overlapped, 7 conflicted. Three needed real reconciliation rather than a side-pick: **The `volumes` collision.** Both branches independently added a `volumes` field to ContainerSpec meaning different things: main's is persistent storage keyed by a server-derived `claimName` that deliberately outlives an instance; mine is emptyDir scratch shared between an init container and the main container. Worse, each emitted the pod-level `volumes` list from its own helper, so spreading both would have silently kept only the last — a server with a PVC would have lost it the moment a session scratch volume appeared. Resolved by renaming the unreleased side: mine is now `scratchVolumes`, main's keeps `volumes` since it is already shipped and in use. `buildPodVolumes` now owns the whole pod-level list and emits both kinds; `buildPodExtras` is init containers only. fsGroup/securityContext stays tied to the PVC case, because an emptyDir is created writable by the container's own user and chowning it is work with no purpose. Five tests pin the coexistence so a later change cannot drop one kind again. **The extracted permission mapper.** I moved `mapUrlToPermission` out of main.ts into middleware/url-permission.ts so it could be tested; main kept it inline and kept editing it. Kept the extraction and ported main's one change across — the `/api/v1/audit/events/purge` mapping onto the existing `audit-purge` operation. Verified rule-by-rule that the extracted version is a superset: all 15 of main's rules plus the 5 agentic-teams ones. **The NUL byte in create.ts.** Both branches fixed the same defect independently, mine as `''` and main's as `'\0'`. Took main's. The rest were one-liners: .gitignore comment wording, and an add/add on proxy-long-running.test.ts where main's copy carries a progressive-delivery test mine lacked — took main's in full. Not changed, but worth flagging: main's generateDeploymentSpec hardcodes `automountServiceAccountToken: false` while generatePodSpec honours `spec.automountServiceAccountToken`. That asymmetry is main's own and predates this merge, so it is left exactly as-is rather than altered inside a merge commit. Verified: 3138 tests pass (237 files), build clean, prisma schema valid. Lint 809/80 vs 794/77 pre-merge — the delta is main's own debt; every file touched during conflict resolution was checked and introduces no new errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p	2026-09-06 23:40:04 +01:00
Michal	22d8e13390	fix(smoke): match the _resultId format the pipeline actually emits ... The drill-down checks matched /_resultId:\s*(\S+)/ — the legacy paginator's JSON spelling. The content pipeline writes _resultId="pm-abc", so the regex found nothing: "Has _resultId for drill-down" reported a false failure on every run, and the drill-down tests that used the same regex to extract an id skipped themselves as "not large enough for section-split". Both were invisible in a green suite. One shared extractResultId() in the smoke harness now accepts either spelling and returns the bare id, so the ad-hoc character strip on the captured group goes away too. proxy-pipeline's check block reports 21/21 instead of 20 with a phantom failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JQr5Z9gYrqBQXTGBBuemZ2	2026-09-05 18:03:56 +01:00
Michal	0e12637271	fix(mcplocal): stop upstream required params from blocking a drill-down ... A paginated tool result tells the model to call the same tool again with only _resultId/_section. The advertised schema still carried the upstream's own `required` list, so a client that validates arguments rejected exactly that call — "Received tool input did not match expected schema" — and the model concluded pagination was broken and gave up (websearch/fetch_content requires `url`; verified live on llm-model-tester). `required` is ANDed with everything else in a schema, so no property declaration could rescue it. It now becomes an alternation: either the upstream's requirements (a fresh call) or `_resultId` (a re-read, which is answered from cache and never reaches the upstream). Validators that understand anyOf enforce that; naive ones that only read top-level `required` now find none and accept both. Schemas that already carry a combinator get the alternation appended to `allOf`, and a schema with no required params is left untouched. Covered by unit tests on a required-bearing upstream schema plus a smoke assertion that no advertised tool keeps a drill-down-blocking `required`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JQr5Z9gYrqBQXTGBBuemZ2	2026-09-05 17:58:57 +01:00
Michal	bc7eb5a0ad	fix(mcplocal): learn which models reject sampling params, don't hardcode them ... Auto-following to the newest Opus moved the failure rather than removing it. The deploy's own smoke output showed it: 404 "model not found" became HTTP 400: `temperature` is deprecated for this model. Anthropic removed temperature/top_p/top_k on the current generation (Opus 5, Sonnet 5, Opus 4.7/4.8, Fable 5), and the adapter sends temperature: 0 unconditionally. So the gate's prompt-selection was still degrading on every call -- just with a different status code. A list of which models accept sampling would rot exactly the way the pinned model ids did, which is the whole thing this branch is trying to stop. So the provider learns it instead: the first 400 naming a sampling parameter drops it and retries, and remembers the model so every later call omits it up front. One wasted call, once, rather than a hardcoded table to maintain. The match is deliberately narrow -- a 400 must actually name temperature/top_p/ top_k. An unrelated 400 (missing max_tokens, bad schema) propagates untouched; there is a test for that, because a broad match here would silently swallow real request errors and retry them pointlessly. Verified live: claude-opus-latest -> claude-opus-5, first attempt 400 on temperature, retry succeeds past it. The retry then hit HTTP 429 -- the personal OAuth token's rate limit, which is the pre-existing credential-tiering issue, not this path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-26 00:11:50 +01:00
Michal	47d7c779d0	feat: per-server tool-call timeout, on the server resource ... Completes the plan's last item. The deadline shipped with a global default; this makes it overridable per server, where the knowledge actually lives -- a server with genuinely slow tools declares its own budget instead of forcing the global up for everyone. Source of truth is the server resource, following healthCheck exactly: Prisma column + migration (NULL keeps today's behaviour, so no existing server changes), zod validation on create and update, the repository, an `--tool-call-timeout` flag mirroring `--health-check-timeout`, and the apply schema so `apply -f` accepts what `get server -o yaml` emits. That round-trip needed care: get emits `toolCallTimeoutSeconds: null` for every server without an override, so the apply schema is nullable, not merely optional -- otherwise the very first `get -o yaml | apply -f` on an untouched server would have failed validation. Bounded at one hour. A deadline exists so a wedged call answers instead of hanging; a value beyond an hour is indistinguishable from having none. It reaches mcplocal through server discovery rather than the project-scoped serverOverrides map, and is applied on EVERY sync rather than only at first registration -- raising a server's timeout should take effect at the next refresh, not require the upstream to be dropped and rebuilt. The endpoint sees the wire name (`docmost_search`), so it decodes to the canonical `server/tool` before resolving. The override is keyed by server, so every tool on that server inherits it without being enumerated, and the failure message quotes the deadline actually applied rather than the global. Smoke tests (tests/smoke/bounded-failures.smoke.test.ts) cover the two production faults against the live proxy: a stale session recovers instead of stranding the client on an uncorrelatable 404, every request is answered, and `mcpctl trace` responds for an unknown code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-26 00:03:06 +01:00
Michal	08b451b7aa	feat(mcplocal): Anthropic models auto-follow the newest in their family ... `claude-opus-4-20250514` was pinned as the heavy provider and had been returning 404 on every gate ranking and every pagination title. The only visible symptom was a fallback that looked like an ordinary one -- it surfaced here because the degradation notice added earlier in this branch finally printed the reason. Checked against GET /v1/models: BOTH pins were dead. The fast tier's claude-haiku-3-5-20241022 is gone too, so that tier had been silently 404ing as well. The provider's listModels() asserted "Anthropic doesn't have a models listing endpoint" and returned four hardcoded dated ids. That endpoint does exist and answers fine with the OAuth token this deployment uses; the hardcoded list was simply stale, and a test pinned it in place. So: `claude-<family>-latest` (or a bare `opus` / `sonnet` / `haiku` / `fable`) now resolves against the live list. Exact ids pass through untouched, so pinning still works when someone wants it. Newest is decided by `created_at`, never by parsing the version out of the id. That is not incidental: `claude-opus-4-5` sorts ABOVE `claude-opus-5` as a string, and "4-5" parses as a larger minor than "5". There is a test for exactly that trap. Resolution is cached (12h, MCPCTL_ANTHROPIC_MODEL_TTL_MS) so it is not a per-call network hop, and shared across instances since the model list is account-wide. When the endpoint is unreachable it falls back to a pinned known-good id per family and says so on stderr -- the map going stale can then only cost availability, never correctness. The constructor default was `claude-sonnet-4-20250514`, also retired; it now tracks the family too. Local config: heavy -> claude-opus-latest, fast -> claude-haiku-latest. Verified live: opus-latest -> claude-opus-5, haiku-latest -> claude-haiku-4-5-20251001, sonnet-latest -> claude-sonnet-5, and an exact id passes through. Also found while there: both anthropic entries were named "anthropic", and the registry keys by name, so the second silently OVERWROTE the first and one tier's model was discarded entirely. Renamed to anthropic-fast / anthropic-heavy so NamedProvider keeps them distinct and the tier split is real. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:55:44 +01:00
Michal	14eb2623bc	fix(cli): trace's 'slowest' names a step, not the aggregate ... tool_call_trace and pipeline_execution are aggregates OF the stages, so letting them compete always named the total and told you nothing. Verified live: now reports 'slowest: paginate (252ms)' rather than 'tool_call_trace (509ms)'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:48:07 +01:00
Michal	89db09a12d	feat(cli): mcpctl trace <code>, and bound the table it reads ... The trace codes are now emitted in every deadline message and degradation notice, so they need somewhere to go. `mcpctl trace <code>` queries the audit events mcpd already stores and renders the request as a waterfall: each step with its offset, duration, byte delta, and any ⚠ degradation or ✗ error, then a summary naming the slowest step. No new endpoint was needed -- GET /api/v1/audit/events?correlationId= already existed and correlationId is already an indexed column; it just had nothing writing meaningful values into it until this branch, and nothing reading it. --strict exits non-zero when the trace contains an error or a degraded step, so it composes into scripts. An empty result explains itself rather than printing nothing: batches flush up to 5s late, old requests predate trace codes, and the codes exclude I/L/O/U so a mistyped one is worth calling out. Retention: AuditEvent had no prune at all. That was defensible while the table was write-only; it is not now that a command reads it. Mirrors the AuditLog convention exactly -- POST /api/v1/audit/events/purge, triggered rather than scheduled, guarded by the existing audit-purge RBAC operation so it cannot be granted by accident separately from the log purge. 30 days by default rather than AuditLog's 90: these are several rows per MCP call, not a record of administrative mutations. Note for the plan's sake: I had assumed AuditLog retention was a scheduled job registered in main.ts. It is not -- it is a manual endpoint. Mirroring what the codebase actually does beat inventing a scheduler that exists for neither table. completions are generated, so `trace` is registered in PROJECT_SCOPED_COMMANDS and both shells regenerated; the freshness test passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:44:36 +01:00
Michal	b6e270ee48	feat(mcplocal): every request answers, even when the pipeline wedges ... The LLM budgets bound the failure we actually hit. This is the backstop for the ones they cannot see: a wedged plugin hook, a virtual-tool handler, an upstream path without its own timeout. transport.onmessage IS the whole request pipeline, and it writes to a socket that has already been hijacked from Fastify. The SDK does not await it, so a throw became an unhandled rejection and a hang became silence -- in both cases the client got no response at all and waited out its own timeout. onmessage is now structured so that reaching transport.send() is unconditional: the route call is raced against a deadline, and the notification flush and the send each carry their own catch, so a failure while flushing can no longer cost the client its response. transport.onerror was never assigned, so SDK-level transport errors were swallowed entirely. It is now. Shape of the answer is deliberate. A tools/call comes back as a SUCCESSFUL result with isError and readable text naming the trace code -- the same shape router.ts already uses for an expired _resultId, because a transport error tends to surface as a hard failure while a tool error is something the model reads and acts on. It also says the upstream may still be running, which is true and matters. Other methods get a JSON-RPC error (-32001). MCPLOCAL_TOOLCALL_DEADLINE_MS defaults to TOOLCALL_TIMEOUT_MS + 30s rather than 120s. The watchdog arms earlier in the request than mcpd's fetch does, so at equal values it would always fire first, masking mcpd's specific UpstreamTimeoutError with a generic message and cutting off pagination and the rest of the post-processing. There is a test asserting the two stay ordered. The pause queue is exempt, but not bypassed. It blocks until a human operator releases, edits or drops a response, so the deadline SUSPENDS -- clock stopped, then re-armed with whatever was left. A test asserts the re-arm, because an exemption that forgot to re-arm would silently make every paused request immortal. End-to-end: an upstream that never settles now yields a response in under a second carrying ⚠, the deadline, and `mcpctl trace <code>`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:41:09 +01:00
Michal	0eca4148e3	fix(mcplocal): recreate a stale session instead of 404ing outside the envelope ... Second of the two faults that both presented as a 1800s hang. mcplocal keeps MCP sessions in memory only, so any restart -- a deploy, an RPM install, scripts/release.sh -- invalidates every connected client's session id. The response was a bare 404 {"error":"Session not found"}: 3.5ms server-side, but it lands OUTSIDE the JSON-RPC envelope with no id, so the client cannot correlate it to its request and simply waits out its own timeout. Every mcpctl tool call in that session became a 30-minute stall while the server was perfectly healthy and answering other traffic in milliseconds. Now the session is recreated, keeping the id the client already holds -- a re-handshake would change it and invalidate everything the client has cached. The SDK assigns sessionId/_initialized only while handling an `initialize` (webStandardStreamableHttp.js:419-420), so adoptSession() replays exactly those two assignments. It reaches into SDK internals, so it fails LOUDLY: the guard throws with a pointer to itself, and session-adopt.test.ts is the canary that goes red at `pnpm test` rather than in production on the next SDK bump. Recreation is not silent. Doing it quietly would hand an agent an ungated tool list with no signal that its gate state had vanished, so the first tools/call result carries the established "⚠ Session state unavailable (...)" notice telling it to call begin_session again. onsessioninitialized does NOT fire on the adopt path, so its body is factored into registerSession() and called explicitly. That is the subtle part: skipping it would have produced recreated sessions with null userName on every audit event, silently. Also fixed, and it is the CAUSE rather than the symptom: MCP clients close a session with DELETE + Content-Type: application/json and an empty body, which Fastify's default parser rejected with FST_ERR_CTP_EMPTY_JSON_BODY before the route handler ran. sessions.delete() therefore never happened and every "closed" session leaked -- manufacturing the stale-session condition this commit handles. Fixing recreation without fixing teardown would have shipped the workaround and kept the cause. project-mcp-endpoint.test.ts's "returns 404 for unknown session ID" inverts to "recreates an unknown session instead of 404ing it", and asserts the id comes back unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:36:27 +01:00
Michal	c06c34f0a2	feat(mcplocal): a trace code that actually reaches the audit trail ... The plumbing for request tracing was all present and almost entirely inert. correlationId was generated at the endpoint and threaded into traffic events, but: - emitTrace() dropped it, so every durable tool_call_trace row had correlationId = null and could not be joined to anything; - ExecuteOptions.correlationId existed but its only production caller never passed it, making every stage_execution and pipeline_execution row unjoinable too -- dead code that looked like a feature; - plugin events (all three gate emits) had no access to it at all; - and the value itself was `<session-uuid>:<jsonrpc-id>`, which nobody can retype from a screenshot. Also fixed: onToolCallBefore intercepts returned without emitTrace, so every tool call made while a session was gated produced no trace whatsoever. The trace code now IS the correlationId. Nothing parses the old format -- checked across mcpd's audit repository and route, mcplocal's router and traffic capture, and the CLI console -- sessionId is its own audit column and the JSON-RPC id is in the traffic body, so nothing is lost. AuditEvent.correlationId is already an indexed Postgres column, so lookup works with no migration. 8 chars of Crockford base32 without I/L/O/U: readable aloud, typeable from a screenshot, 40 bits (a collision shows two traces, it corrupts nothing). Getting it to the plugins and the executor needed AsyncLocalStorage, not a parameter. Both are constructed per SESSION, not per request -- processContent is a closure with no RouteContext -- so threading an argument would have touched PluginSessionContext, every plugin, ExecuteOptions and StageContext. ALS also stays correct with two requests in flight on one session, which a mutable field on the session-scoped context would not; there is a test for exactly that. PluginContextImpl.emitAuditEvent auto-fills it the way it already auto-fills sessionId, so this covers every gate event and every plugin written later without a per-callsite edit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:32:10 +01:00
Michal	c2a419b4f7	fix(mcplocal): one LLM budget per stage, so a loop cannot multiply it ... A per-call timeout bounds one call. It does not bound a stage that calls the LLM in a loop -- and summarize-tree does worse than loop: buildTree recurses to maxDepth (3 by default) and iterates per section at every level, then groupSections iterates again. Hundreds of sequential calls are reachable, so a 10s per-call cap is a multiplier, not a bound. StageBudget is one wall-clock budget shared across everything a single stage invocation does. Once it is gone the stage takes its deterministic path -- first-line excerpts, numbered pages -- and says so. The executor builds one per stage from config.budgetMs ?? MCPCTL_STAGE_LLM_BUDGET_MS (30s) and disposes it in a finally, so a long-lived mcplocal never accumulates timers. Two details that matter more than they look: - A warm cache is never budget-gated. cachedSummarize and generatePageTitles used ctx.cache.getOrCompute, which makes the budget check impossible to place correctly; split into get/set so a cached summary -- which costs nothing -- is still returned when the budget is spent. - "No LLM configured" is NOT a degradation. It is a deliberate choice, and numbered pages are the expected output there, so it gets no warning. Only failures, timeouts and exhausted budgets do. Crying wolf on a working configuration would train people to ignore the notice. summarize-tree's single-block path previously had no try/catch at all, so a failure escaped the stage entirely and executor.ts discarded the whole stage's work. It now degrades like the others. Degradation is reported three ways, all from util/degrade.ts: the reason in the content behind the established "⚠ <feature> unavailable (<reason>)" prefix, the count of affected sections (the recursion can hit one exhausted budget dozens of times, so the user needs the reason once plus what it cost), and degraded/degradedReason on the stage_execution audit event -- previously a degradation was invisible in the trace. docs/reliability.md now names ONE helper rather than a list of compliant call sites. That list is exactly how this drifted: the doc named the gate and llm/pagination.ts, and the newer stages never joined it. Tests: the 20-section recursion completes in ~300ms against a 300ms budget; a warm cache still serves real titles with the budget exhausted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:29:45 +01:00
Michal	2c90f5971d	fix(mcplocal): no LLM call can hang a tool call forever ... docs/reliability.md has stated the rule since it was written: "LLM-optional operations must be time-bounded, fall back deterministically, and report the degradation". The gate and llm/pagination.ts obey it. The newer proxymodel stages never adopted it -- stages/paginate.ts:72 awaited ctx.llm.complete() with no timeout and no signal, and its try/catch caught errors, not hangs. So when a provider hung rather than erroring, the promise never settled, the tool call never returned, mcplocal never wrote to the already-hijacked socket, and the client waited out its own 1800s timeout. In the journal that is three POST /projects/docmost/mcp requests accepted and never answered -- the only three such requests in 9,600 across every project, each followed immediately by [llm-adapter] "trying next" and [paginate] "Smart page titles failed". It read as a Docmost transport fault for two sessions. It was neither: Docmost answers /search in 203ms, and through mcplocal on a fresh session in 461ms. docmost_search returns ~14KB and crosses the pagination threshold; update_page returns a small ack and does not. One tool paginated, one did not, and only the paginating one could hang. Bound it centrally in the adapter rather than at each call site, so a stage that passes no options is still bounded and a stage written next year inherits the guarantee: - LLMCompleteOptions gains optional budgetMs/perCallTimeoutMs/signal. Optional matters -- seven inline stubs implement LLMProvider structurally and a required member would break every one. - LLMProviderAdapter.complete() runs one budget with a deadline across the whole failover chain, each attempt capped at min(perCall, remaining). A per-provider timeout would have made the worst case N x timeout; wrapping the method would have killed failover mid-chain. - Every attempt now gets a real AbortSignal. anthropic and openai (which back vllm) honour it; deepseek/ollama/gemini ignore it, and withTimeout's race unblocks the caller anyway -- exactly what its doc comment anticipated. - A caller abort stops failover: if nobody is waiting, don't burn the chain. util/degrade.ts generalises the gate's bounded -> fallback -> loud log -> degradedReason pattern into bounded(), which cannot throw, so the deterministic fallback is unconditional rather than something a catch block must remember. gate.ts is refactored onto it; plugin-gate and router-gate pass UNMODIFIED (61 tests), which is the proof the generalisation is faithful. anySignal() rather than AbortSignal.any: that landed in Node 20.3 and package.json declares >=20.0.0. proxymodel-llm-adapter.test.ts now asserts signal: expect.any(AbortSignal) -- every provider attempt being cancellable is the guarantee, so it is pinned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqMidYEGUJG5fxeoTELBu2	2026-08-25 23:23:16 +01:00
Michal	b4ad95ca66	test(mcplocal): smoke asserts the wire-form tool-name contract ... The smoke suite talks to the live mcplocal through the HTTP boundary, which serves wire names since #124 — assertions expecting `favourite/`, `all/` and `smoke-aws-docs/` prefixes now check the underscore wire form (config pins stay canonical). These two files were the release-gate failures after the bbd3188 rollout; 166/166 green against the live fleet after the update. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-25 22:06:18 +01:00
Michal	c014fcdd82	fix(mcplocal): name tools in wire form in gate and favourite-index prose ... Follow-up to #124: the boundary WireNameCodec serves underscore-joined names, but the gate plugin's tool inventories (initialize instructions and begin_session response) still listed canonical `server/tool`, and the favourite-index instruction told the model to prefer `favourite/<tool>` — prose naming functions the model cannot call. Inventories now run through sanitizeWireName and the instruction describes the favourite_/all_ prefixes. Cosmetic for routing (slash names still pass through the codec) but load-bearing for tool selection: models copy names out of prose. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-25 21:58:52 +01:00
michal	7fbb827aa5	Merge pull request 'fix(mcplocal): serve OpenAI-safe tool names on the wire' (#124) from fix/wire-safe-tool-names into main	2026-08-25 19:23:19 +00:00
Michal	eb0e97e76e	test(mcplocal): end-to-end wire-name coverage on the project endpoint ... Drives /projects/:name/mcp over the real Streamable HTTP transport with a fake websearch upstream: tools/list must serve `websearch_fetch_content` (every name matching the OpenAI function-name charset), calling that wire name must reach the upstream as bare `fetch_content`, and a legacy client echoing the canonical `websearch/fetch_content` must still route. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-25 20:23:09 +01:00
Michal	065ce02a60	fix(mcplocal): serve OpenAI-safe tool names on the wire ... The proxy namespaces tools as `server/tool` (and favourite-index presents `favourite/<tool>` / `all/<server>/<tool>`). A `/` is not a valid character in an OpenAI-style function name, so hosts that forward MCP tool names verbatim as LLM function names depend on the model faithfully echoing an illegal name. LibreChat did exactly that: deepseek-v4-flash intermittently dropped the `websearch/` prefix, LibreChat's registry lookup failed, and it reported "This tool's MCP server is temporarily unavailable" while nothing was down — the calls never reached mcplocal at all (confirmed against the AuditEvent table, 2026-08-25). Claude Code and the pi extension only dodge this because they sanitize names client-side. Fix at the HTTP boundary only: a WireNameCodec rewrites tools/list responses to wire-safe names (`/` -> `_`, exact-match reverse map, deterministic suffix on collision) and maps tools/call names back before routing. Wired into both /mcp and /projects/:name/mcp. Everything inside the proxy — routing maps, plugins, favourites config, audit events — keeps canonical names, and unknown inbound names (legacy clients echoing slash names, virtual tools) pass through unchanged, so existing clients keep working. Codecs are keyed per project and outlive the router cache TTL so a client can call a tool it listed minutes earlier; after a restart the client's initialize-time tools/list repopulates the map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-25 20:17:33 +01:00
Michal	913c0fbdc6	fix(secrets): wrap an image server's own command, not just entrypoint ... Migrating docmost and my-home-assistant, both rendered their secret and neither picked it up. The pod spec showed why: command: (empty) args: ["node","build/index.js"] server.entrypoint: (unset) wrapCommand consulted only `server.entrypoint` for non-package servers, so with `entrypoint` unset it returned undefined, the wrapper was skipped entirely, and the container ran its normal command — which never sourced /vault/secrets/<name>. The failure is silent by construction: the agent init container succeeds, the file is there, and the server simply starts with empty credentials. An explicit `command` on an image server is already a complete command line — mcpd's exec mode would run exactly it — so it should be wrapped verbatim. `entrypoint` is only needed when there is no command at all and the image's own ENTRYPOINT would take over. Now branches on the three real shapes: package server (prepend the runner entrypoint mcpd owns), image + command (use verbatim), image only (require the declared entrypoint). Seven tests, one per shape plus the two undefined cases. Reintroducing the old logic fails two of them — checked before keeping. Both servers were rolled back to secretDelivery: env and are healthy; they can migrate once this ships. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-21 11:14:39 +01:00
Michal	0a83f71648	fix(secrets): injector-delivered servers must attach, not exec ... With injected delivery the credentials exist ONLY in PID 1's environment: the container command is a shell that sources /vault/secrets/<name> and execs the real server, so the values live in that process and nowhere else — not in the pod spec, which is the whole point. mcpd picks its STDIO mode from the server's shape: an explicit command or a packageName selects `exec`, a bare dockerImage selects `attach`. `exec` spawns a NEW process inside the container, which never sourced the file and therefore starts with empty credentials. The server comes up, answers tools/list, and fails every authenticated call — precisely the silent empty-token failure this feature exists to prevent. Seen as `Readiness check (list_datasources) failed: process exited 1` on a pod whose PID 1 demonstrably held the token (verified via /proc/1/environ: GRAFANA_URL set, token 46 chars, nothing in the pod spec). So secretDelivery: injector now forces attach regardless of server shape. Safe because wrapCommandForInjector execs rather than forks, so PID 1 IS the server. It also means no bouncer or HTTP shim is needed — mcpd's PersistentStdio already holds one long-lived connection; it was simply pointed at the wrong process. Note this inverts an assumption I had earlier: image-entrypoint servers were already on the attach path and would have worked; it is the package-based ones that were broken. gitea remains the sole exception, and for the unrelated reason that it is distroless and has no shell to source the file. Extracts the decision as a pure `chooseStdioMode()` so it can be tested — it is subtle and fails silently. Six cases pinned, including that env delivery still execs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-21 02:05:51 +01:00
Michal	ec35e1cc36	fix(secrets): the injector wrapper must replace the entrypoint, not extend it ... Caught migrating a real server: the pod crashlooped with `.: cannot open /vault/secrets/grafana-creds`, and the reason was in the generated spec: args: ["/bin/sh","-c",". /vault/secrets/grafana-creds; exec \"$0\" \"$@\"", "@leval/mcp-grafana"] mcpd deliberately maps ContainerSpec.command -> k8s `args` so a package server keeps its runner image's ENTRYPOINT (`npx -y`, `uvx`). Putting the sourcing wrapper there meant the pod actually ran `npx -y /bin/sh -c '...'` — npx trying to resolve a package called /bin/sh. The agent had rendered the file correctly; nothing ever sourced it. Adds `ContainerSpec.entrypoint`, which maps to k8s `command` and so REPLACES the image entrypoint, and has wrapCommand fold that entrypoint into the argv it returns (`npx -y` / `uvx` for package servers, the server's own `entrypoint` field for dockerImage servers — already required at validation for exactly this reason). Two tests pin it: a wrapped server emits `command` and no `args`; an unwrapped one still emits `args` and no `command`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-21 01:28:21 +01:00
Michal	ef9ba6fb8d	fix(servers): persist secretDelivery and entrypoint ... `mcpctl patch server my-grafana secretDelivery=injector` printed "patched server 'my-grafana'" and changed nothing. The repository maps update/create fields explicitly, one by one, so a new column silently does nothing until it is added there — and the silence is total: the API returns 200, the CLI reports success, and `get -o yaml` still shows the old value. Caught by trying to migrate a real server, not by any test. Adds the two fields to both create and update, plus tests that assert the mapping directly. Those tests fail against the unfixed repository (3 of 4) — verified before keeping them. This class of bug will recur: the mapping is manual and nothing links a schema column to it. The tests at least make the next omission loud for these two fields. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 23:40:59 +01:00
Michal	370fd0a034	feat(secrets): opt-in injected secret delivery, scoped per server ... Completes the path that stops mcpd writing secret VALUES into MCP server pod specs. With `secretDelivery: injector`, the pod fetches its own secrets from OpenBao through the agent injector, under a ServiceAccount and role scoped to just that server's secrets — so the value never enters etcd, and gitea-mcp cannot read the Grafana token. Opt-in per server, defaulting to `env`. Every existing server is bit-for-bit unchanged, and migrating is one reversible decision at a time rather than a flag day. The two invariants under most risk, both tested: - **Opted-out servers produce an identical manifest.** No annotations, no serviceAccountName, automountServiceAccountToken still false. - **Opted-in servers still fail LOUDLY on a bad ref.** Once mcpd stops reading a server's secrets, the check e6cd735 added no longer fires for it, and a typo'd secretRef would degrade into a vault-agent-init crashloop that mcpd reports as a generic pod failure — the same class of bug that had gitea-mcp running for weeks on an empty token while reporting healthy. `validateServerEnvRefs` resolves every ref and throws the value away, purely to keep that error. After the value cache it is a cache hit and costs nothing. Shell quoting is the other silent-failure trap and is treated as part of the contract: the agent renders `export NAME='value'` and the container command sources it, so a value containing a space, `$`, a quote or a newline would truncate and yield an empty token. `shellSingleQuote` is tested by executing a real /bin/sh over nine adversarial values including `'; export PWNED=1; '` — and those tests fail against naive quoting, confirmed before keeping them. `sh -c <script> arg0 arg1 …` preserves argv via $0/$@, and `exec` keeps PID 1 as the real process, which matters because mcpd attaches to PID 1's stdin/stdout for STDIO servers. Docker/Podman declare `capabilities.secretRefs: false` and fall back to inline resolution, so local development is untouched. Deleting a server revokes its identity, after its pods are gone and best-effort — a role no pod can authenticate as grants nothing, and failing the delete over it would strand the row. Per the CLI rules, `secretDelivery`/`entrypoint` are `create` flags, round-trip through apply -f, and show in `describe server` — which now also flags servers still inlining secrets into their pod spec. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 23:33:20 +01:00
Michal	f8427959e5	test(secrets): cover per-server identity containment ... ServerIdentityService shipped without tests. Containment is the whole point of the design, so it needs assertions rather than trust: a shared role would let third-party MCP images (gitea-mcp, ha-mcp) read every secret under secret/mcpctl/*, which is worse than the pod-spec exposure it replaces. Eight cases, all about what a server must NOT get: the grant covers only the secrets that server declares; a secret referenced twice is one grant, not two; inline env values never widen it; another server's secrets never appear. Plus the ordering invariant (ServiceAccount before the role that binds it — the reverse lets a pod start, fail to log in and crashloop while the role is still being written), and that a backend which cannot scope identities REFUSES rather than silently succeeding, which would leave a pod believing it held access it never got. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 23:13:49 +01:00
Michal	7b5136491f	fix(secrets): list via GET ?list=true — the LIST verb dies at the proxy ... Found by the readiness probe added in the previous commit, on its first run against production: `mcpctl status` reported Secrets: bao-k8s* ✗ auth failed: OpenBao list: HTTP 400 Bad Request That is a real bug, not a probe artifact. `bao-k8s` is configured with the public ingress URL, and Cilium's ingress Envoy rejects the non-standard LIST HTTP method outright. Verified live from the mcpd pod: LIST https://bao.ad.itaz.eu/v1/secret/metadata/mcpctl/ -> 400 Bad Request GET https://bao.ad.itaz.eu/v1/secret/metadata/mcpctl/?list=true -> 403 permission denied (bogus token, i.e. reached bao) LIST http://openbao.openbao.svc:8200/... (ClusterIP, no Envoy) -> 403 permission denied So the verb was fine against bao and fatal through the ingress. OpenBao accepts both forms and documents the GET form for exactly this reason. This was never noticed because nothing called list() in production — it backs `mcpctl migrate secrets`, which would have failed with an opaque HTTP 400 against any ingress-fronted backend. Also adds the per-server scoping primitives Phase 2 needs, with tests: buildServerSecretPolicyHcl (no wildcards, read-only, stable under reordering), buildServerProvisioningPolicyHcl (prefix-confined so mcpd cannot grant itself more than it holds), ensureKubernetesAuthRole / delete helpers, the driver-level ensureServerIdentity/removeServerIdentity capability, and ServerIdentityService. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 22:54:42 +01:00
michal	eb3e558a44	Merge pull request 'feat(secrets): survive OpenBao outages and report real backend health' (#115) from feat/openbao-resilience into main	2026-08-20 21:37:57 +00:00
Michal	545e7745da	test(secrets): cover the rotator loop's boot-time dead-token detection ... SecretBackendRotatorLoop had zero tests, despite being the detector added in e51b924 specifically so a re-initialised OpenBao surfaces the moment mcpd boots rather than 24h later when the scheduled rotation finally fires. The class already injects setTimeout/clearTimeout and a logger, so this needed no production change. Nine cases, weighted to what actually breaks: the boot health check runs per rotatable backend; a dead token emits kind BACKEND_TOKEN_DEAD through the injected logger (the point of the earlier console.error removal — a bare console call never reaches ErrorLogBuffer, so `mcpctl errors` could not see it); a throwing health check does not abort start(); overdue backends rotate immediately and still get scheduled; the 60s floor holds across 50 adversarial-jitter draws; and stop() both clears timers and trips the `stopped` guard against rescheduling, which had never been exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 22:17:24 +01:00
Michal	fe9987cefd	test(secrets): smoke-cover backend health honesty + cache correctness ... Two of these four assertions currently FAIL against the live cluster, which is the point: mcpd there still renders `Secrets: bao-k8s* ✓` from the rotation field, and its JSON status carries no live/ready pair. Per the project rule the fix is to deploy, not to relax the assertion. The status check specifically rejects a bare "name ✓" with no qualifier — that string is the old rendering and proves the probe was never consulted. Deliberately does not take the real OpenBao down. Simulating an outage against shared infrastructure to satisfy a test would be worse than the bug it covers; the outage paths are unit-tested with an injected clock. Docs: adds a Reliability section (request hardening, the stale-while-error table, the cold-cache gap and why persisting values is not the answer, live-vs-ready) and replaces the stale "Kubernetes ServiceAccount auth is not shipped yet" note — it shipped in 5152066, and it is what the live backend has used since June. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 22:16:04 +01:00
Michal	0fbfc72d68	feat(secrets): report real backend health instead of a hard-coded tick ... `mcpctl status` derived its Secrets verdict entirely from `tokenMeta.lastRotationError`. The rotator writes that field only for `auth: 'token'` backends (SecretBackendRotator.isRotatable), so a `kubernetes`-auth backend never wrote it and the line rendered a green tick unconditionally — including with OpenBao sealed, unreachable, or answering 403 to every read. The one signal we had was structurally incapable of going red for the backend we actually run. New `GET /api/v1/secretbackends/:id/health` reports two signals, kept separate on purpose: live — reachable at all? (unauthenticated sys/health) ready — can we read through it? (uses our credentials) live-but-not-ready is the exact shape of a re-initialised OpenBao handing back valid-looking tokens that grant nothing; collapsing both into one boolean is what hid that for four days. Needs no RBAC mapping — it falls through to the generic `secretbackends` resource, so a GET is `view:secretbackends`. `mcpctl status` now renders four states — reachable / degraded (serving N cached secrets) / unreachable / auth failed — with rotation error demoted to a trailing clause rather than the verdict. A failed probe renders "? unknown", never green: not knowing is not health. JSON output carries the same probe, so scripts stop being told every k8s-auth backend is fine. Also adds a boot-time cache warm. The stale-while-error cache can only absorb an outage for secrets it has already seen, so a cold mcpd during a backend outage still fails; resolving each running server's refs once at startup closes that for the common case. Best-effort and deliberately partial — if the backend is also down at boot this is a no-op and instances fail loudly, which is correct. Persisting last-known-good to Postgres or disk would just be plaintext-at-rest again. Tests: 15 new. The five status assertions were confirmed to fail against the old rotation-only logic before being kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 22:14:12 +01:00
Michal	bd3e1134c9	feat(secrets): survive OpenBao outages instead of cascading them ... mcpd re-read the secret backend on every use — server env resolution, LLM api keys, chat, git providers, code repos, webhooks — with no value cache, no request timeout, and a retry that only fired on HTTP 403. A few seconds of OpenBao unavailability therefore turned into minutes of degraded service: instances that restarted during the blip failed env resolution, got marked ERROR, and entered the 30s x5 then 5min backoff. Three changes, in dependency order: 1. Typed errors. `SecretNotFoundError` (definitive) vs `SecretBackendUnavailableError` (transport). The distinction has to be typed rather than string-matched — a mis-classified "not found" would resurrect deleted secrets, and a mis-classified auth failure would paper over revoked grants, which is how an upstream re-init once broke every secret write for four days (e51b924). 2. Driver resilience. Every request now carries an AbortSignal timeout (there was none, so an unreachable backend hung its caller) and retries 5xx/429/network with full-jitter backoff — 503 is what a sealed OpenBao returns and used to be an immediate hard failure. The 403 purge-and-retry stays single-shot and outside the retry budget: it is a credential refresh, not a backend-unavailable condition, and looping on it would hide a genuinely revoked grant. `healthCheck()` no longer routes through the authenticated path, so an expired role stops reporting as "OpenBao is down"; it maps OpenBao's status codes (sealed/standby/uninitialised) instead. New `authCheck()` covers the readiness half via list(), which exercises the capability we actually depend on — unlike lookup-self, which only proves the token exists. 3. CachingSecretBackendDriver. Fresh reads inside the TTL never touch the network; past it we always try the backend, and on a transport failure serve the last known-good value instead of throwing. That is what stops the ERROR storm. Deleted secrets evict and rethrow — serving those stale would resurrect a revoked credential, strictly worse than an outage — and non-transport errors rethrow untouched. plaintext is not wrapped: its read() is an identity function over the row handed in. A cold cache during an outage still fails, loudly and by design (e6cd735). Also routes BACKEND_TOKEN_DEAD / BACKEND_ROTATION_FAILED through pino rather than bare console.error. They bypassed the multistream feeding ErrorLogBuffer, so the one failure `mcpctl errors` exists to surface was the one it never showed. Tests: 20 new. The two load-bearing guards (never serve a deleted secret stale; never serve stale for a non-transport error) were confirmed to fail against deliberately broken code before being kept. The 403 purge-retry path had no coverage at all until now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki	2026-08-20 22:08:10 +01:00
Michal	6d38953a96	docs(agentic-teams): document the new resources, and make the webhook reachable ... Five resources shipped with no documentation and one documented lie: docs/inference-tasks.md still told people `mcpctl get tasks` lists the inference queue. Work items own that name now. Adds docs/epics.md (the Epic -> EpicTask -> Task hierarchy, the state machine, risk-scaled review gates, the accept-merges gate, and reconciliation), docs/gitproviders.md (forge registration, discovery/import, merge-on-accept, post-merge reopen) and docs/agent-authority.md (the three run modes, and plainly which two are not implemented). Two things found while writing them, both fixed rather than filed: - `webhookSecretKey` was read by the webhook route but writable from NOWHERE -- absent from the create schema, update schema, service, CLI and apply spec. Since the endpoint fails closed on a null key, post-merge CI failures could never reopen their task; the board would keep claiming `merged` for work that broke the build. Adds --webhook-secret-key, the schema fields and the apply key, with tests covering persist / omit / explicit-null / reject-empty. Omission deliberately does not clear the column, so an unrelated edit cannot silently disarm the webhook. - src/cli/src/commands/create.ts contained a literal NUL byte inside a string literal (a binary-file check written with the raw character rather than an escape). Identical behaviour, but it made the whole file read as binary, so `grep -I` skipped it entirely and 1120 lines of CLI were invisible to every text search. Replaced with the escape. The Epic write path (criteria/status/tradeoffs) is API-only -- documented as such rather than papered over, and filed as task 180. Full suite 2612 green; lint at baseline 794/77. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p	2026-08-16 00:14:58 +01:00
Michal	2a85447fac	feat(rbac): bound what an agent may DO by whoever invoked it ... Closes 177. An agent carries a project's prompts and its MCP tools, and the only check on a chat turn was run:agents:<name> - so anyone who could talk to an agent inherited tool access to every project that agent had joined. Membership was the capability. AgentProject widened it from one pinned project to many. The rule now: knowledge travels with the agent, action is bounded by the authority of the run. Prompts and skills still load regardless of who is asking - a QA reviewer that forgets the project's conventions when a different person asks is not a reviewer. Every MCP tool call is gated. Authority is resolved once per run (services/run-authority.ts) and is per-RUN, not per-agent: the same agent answers a human under that human's authority and, once a scheduler exists, runs unattended under its own. `caller` intersects the agent's memberships with the caller's run:projects - reusing the exact grant the project-chat route already demands, rather than inventing a parallel permission. `agent` takes every membership without consulting a user. Both are implemented; `service-account` throws 501 rather than honouring a forgeable x-service-account header, and task 179 records what it needs first. Gated in BOTH listTools and callTool, deliberately: withholding a tool from the list is presentation, not enforcement - a model can invent a tool name it was never shown, and callTool is what would run it. The accepted consequence is stated plainly rather than papered over: a user can ask about a project they cannot reach and the agent will answer from its knowledge. The system block tells the model when it is holding such context so a persona can apply it without reciting it. That is guidance to a model, not a boundary - the boundary is that the tools are absent. Two sibling holes found while exploring and closed here: - Thread hijack. resolveThreadId on the agent path had no owner check, so anyone with run:agents:<name> could append to another user's thread by id and read its history back. The project path already did this correctly; mirrored, 404 not 403 so ids cannot be probed. - McpToken project escape. `project` travels in the request BODY, so the URL-based token scope check in main.ts never saw it - a token bound to project A could drive an agent into B's tools. The binding is now a ceiling. Tests: 19 authority tests (intersection, fail-closed with no user and with no RBAC wired, token ceiling, agent authority bypassing RBAC entirely, the 501) and 4 through the real chat path asserting prompts load while tools do not. Three existing assertions updated for the new dispatcher signature - a deliberate interface change, not tests bent to fit. Local: build clean, workspace 2608 passed (+16), lint unchanged at 794 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p	2026-08-15 23:54:53 +01:00
Michal	5fb1154190	Merge remote-tracking branch 'origin/main' into fix/stdio-restart-recovery	2026-08-15 23:32:46 +01:00
Michal	b022f322f0	fix(mcpd): recover k8s STDIO instances after in-place container restarts ... A container restart (OOMKill, crash, transient npx failure) used to strand the instance in ERROR forever while its pod sat 1/1 Running, because five gaps lined up (#114): - the exec/attach websocket died without ending the stdout PassThrough (client-node installs no onclose), so PersistentStdioClient — whose only death signal was stdout 'end' — kept believing it was connected and every request rode the 120s timeout into a dead pipe; - the stdioClients cache is keyed by pod name, which survives a restart, so nothing ever evicted the corpse; - syncStatus never re-inspected ERROR rows and never read restartCount, so neither the recovery nor the in-place restart was visible; - its ERROR writes clobbered retry metadata, making the row instantly dueForRetry, and the retry recreated the pod under the SAME name — an uncaught 409 that looped ERROR against a healthy pod; - the stuck row consumed the whole replica budget, blocking a fresh-id replacement. The fix, layer by layer: - ws 'close'/'error' now end stdout in both k8s interactive paths (and the docker interactive path mirrors its own one-shot handlers), funneling into an identity-guarded teardown in PersistentStdioClient that also listens for stream 'close'/'error' — a late event from a previous session cannot clobber a reconnected one; - syncStatus re-inspects ERROR rows (pod running again → back to RUNNING with retry metadata cleared), tracks restartCount in instance metadata to catch restarts between polls, MERGES retry metadata instead of clobbering it, and evicts the cached stdio client through a setter-injected hook whenever the pipe is known-dead; - createContainer adopts an alive pod on 409 instead of throwing (and only replaces a genuinely dead one, waiting out the deletion grace period); - attach mode retries once through a fresh client before surfacing an error, so the first call after a detected death succeeds; - the health probe evicts the cached client when failures cross the threshold, and shutdown finally calls closeAll(). Closes #114 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-15 23:32:23 +01:00
Michal	f0ba6e8096	feat(git): reopen a task when CI fails on the commit its merge produced ... Closes 151 — the last loop in the lifecycle. A task reaches `merged`, CI then fails on the merge commit, and the same task comes back as `reopened` instead of the breakage going unnoticed because the board already said done. Two properties make this safe rather than a liability. **It fails closed.** The endpoint is unauthenticated by nature — a forge calls it — so it accepts nothing unless the provider has a configured webhook signing secret AND the HMAC verifies. No secret, no effect. The signature is computed over the RAW body: re-serializing a parsed object changes the bytes, which is the classic way this check silently stops working, and there is a test for it. A 404 for an unknown provider is deliberately vague, because confirming which provider names exist is free reconnaissance on a public endpoint. **It matches precisely.** Accept now records `merge_commit_sha` — the commit the merge actually produced, which is what CI runs against — so a failing status finds exactly the task that caused it. The alternative, reopening "whatever merged most recently", would reopen innocent work. A failure on a commit mcpctl did not merge is somebody else's business and is ignored, as is a task that has already moved on from `merged`. Capturing the merge commit is best-effort by design: the merge already succeeded, so failing to re-read the sha must not fail the accept — it only costs the webhook its precise match later. The route is skipped by the RBAC hook, since the forge has no mcpctl identity. That is only safe because of the fail-closed rule above, and both the skip and the reason are asserted in tests. Tests: 12 covering every refusal path (no secret, unsigned, wrong key, signature over different bytes), the vague 404, the RBAC skip, and the reopen matrix. Local: build clean, workspace 2592 passed (+12), db 128 passed (same 4 pre-existing), lint unchanged at 794 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p	2026-08-15 23:10:49 +01:00
Michal	c66502e590	fix(mcplocal): fetch project instructions with the caller's token (#113) ... The instructions fetch was the one downstream call in getOrCreateRouter still using mcpdClient — whose token is an empty string in HTTP mode — so mcpd answered 401, the catch swallowed it, and every session initialized with no instructions while nothing looked broken. requestClient exists precisely for this; use it. Closes #113 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-15 21:43:00 +01:00
Michal	740ce31469	fix(mcplocal): give proxied tools/call a 120s budget, not the 30s default ... McpdUpstream.send routed every non-list method through the default-timeout client, so any tool that legitimately runs past 30s — web_url_read through a browser solver, a large PDF extraction, a slow retail site — died as "mcpd proxy error: mcpd did not respond within 30000ms" while mcpd was still working on it. LibreChat agents hit this constantly on real pages. New TOOLCALL_TIMEOUT_MS (default 120s, env MCPLOCAL_TOOLCALL_TIMEOUT_MS) sits between DISCOVERY_TIMEOUT_MS (list calls must stay short so a dead upstream can't stall session init) and LONG_RUNNING_TIMEOUT_MS (chat and inference, minutes). withTimeout preserves the caller token and headers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JaFvfHrQyUKCGv6o3N2Wir	2026-08-15 21:43:00 +01:00
Michal	ac9423e9c8	feat(git): merge the PR when a task is accepted ... Closes 148/149 — the accept gate now lands code instead of just moving a row. Driver gains createPullRequest / getPullRequest / mergePullRequest, verified against the real Gitea API shapes rather than guessed: MergePullRequestOption requires a capitalised `Do`, and supports `head_commit_id`. That `head_commit_id` is the point. A human approved a SPECIFIC commit; if someone pushed after that, merging anyway lands code nobody reviewed. So the reviewed sha travels with the request and is checked twice — once by us for a clear MergeStaleError naming both commits, and again by the forge, which closes the race between our read and the merge. When it trips, the message says what happened and what to do: re-review the current head, then accept again. Accept merges BEFORE it transitions. If the forge refuses — conflicts, failing checks, a moved branch — the task stays in awaiting_review and no event is written. Reporting `merged` while main does not have the code is the single worst thing this gate could do, and there is a test asserting the transition never happens on a failed merge. Failure modes are answers, not stack traces: no PR attached is approval-only and succeeds; a repo with no forge connection explains that mcpctl cannot merge it; an already-merged PR is treated as success so a retry after a timeout is safe. owner/repo comes from `sourceFullName` when discovery recorded it, falling back to parsing https, ssh:// and scp-style clone URLs, and returning null rather than guessing when it cannot tell. The merger is injected and optional, so an mcpd without a git provider still runs and accept degrades to approval-only — which is also what the help text now says, restored after the earlier commit that removed a claim we could not back up. Tests: 7 driver PR tests (Do field, head pinning, stale refusal, idempotent re-merge, 405 → refusal), 10 merger tests incl. URL parsing, 7 accept-gate tests. Local: build clean, workspace 2580 passed (+23), lint unchanged at 794 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N4wNHWf7xSwnZCWpJcyv9p	2026-08-15 00:05:31 +01:00
Michal	821c45b2ec	fix(cli): stop claiming that accept merges anything ... `mcpctl tasks accept --help` said "fires the merge". Nothing merges. There is no createPullRequest or mergePullRequest anywhere in the codebase — the git-provider client is task 148 and is not built. Accept records human approval and moves the task to `merged`. Until 148/149 land, `merged` means "a human approved this", not "this is in main", and the help text and service comments now say so. A user who accepted a task expecting a PR to land would have been silently wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-14 23:57:46 +01:00
Michal	03350856ea	fix(mcplocal): make the paginated-result contract usable by any MCP client ... UniFi tools were unusable from non-Claude agents. A tool result over 2000 chars is replaced with a table of contents and re-read by calling the tool again with _resultId/_section, but those params were never declared on the tool's inputSchema — and unifi-network and my-grafana ship `additionalProperties: false`, so for a client that validates arguments the drill-down call was illegal and the data unreachable. The stub's own wording made it worse: "Use section parameter" names a param that does not exist; sending it fell through to the upstream, re-paginated, and minted a fresh _resultId. An unbounded loop. UniFi took the blame because it is the one server whose results always trip the threshold: get_devices was 11,718 chars, get_clients 136,696 across 19 pages with a 92-char page-1. Grafana and Gitea return compact results. - content-pipeline gains onToolsList, declaring _resultId/_section on every tool it can paginate (gate tools excluded — they are intercepted before the pipeline runs). additionalProperties stays false: a property listed in `properties` is already legal under it, so declaring is enough and the upstream keeps its typo protection. - createDefaultPlugin wired only the gate's onToolsList, so the pipeline's would have been dropped on the floor. Both now chain, gate first. - _resultId without _section re-shows the table of contents instead of forwarding an unknown argument to a strict upstream. - The stub names the tool, the live _resultId and a real section id. - Nested MCP envelopes are collapsed. A server fronting another MCP server returns the inner result wrapped in its own content/structuredContent pair, so the payload arrives two or three times over. A layer is peeled only when it carries nothing the inner value lacks, which keeps it lossless. Live against the sre project: get_devices 11,718 -> 5,210 chars and no longer paginates at all; get_clients 136,696 -> 62,358 across 8 pages instead of 19. - deploy/mcplocal.service shipped MCPLOCAL_MCPD_URL=http://10.0.0.194:3100, which is dead — every working machine had a hand-written drop-in. Points at the k8s ingress now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNXFvxanvM6uiFcb4Mp3xU	2026-08-14 23:27:36 +01:00
Michal	8a31121e30	Merge remote-tracking branch 'origin/main' into feat/web-search-templates ... # Conflicts: # completions/mcpctl.bash # completions/mcpctl.fish # src/cli/src/commands/create.ts # src/db/src/seed/index.ts	2026-08-12 23:11:54 +01:00
Michal	ac5dee906e	fix(cli): don't brick the chat REPL when the first turn fails upstream ... Observed live: turn 1 died with an anthropic 429 before the stream's `final` frame, streamOnce resolved '' as the thread id, the REPL stored it, and every later message sent `threadId: ""` — rejected by mcpd's z.string().min(1) with HTTP 400. Permanently stuck: no turn could succeed again, so no `final` frame could ever repair the id. Two independent layers: - streamOnce now resolves `string | undefined` — undefined when no `final` frame arrived — and the REPL keeps its previous thread state on undefined instead of overwriting it. One-shot mode skips the `(thread: ...)` footer when there is none to report. - chatBody refuses to serialize an empty threadId at all, so even a leaked '' can never reach the wire. Regression cover in chat-thread-brick.test.ts (7 tests), including the full REPL chain: failed turn 1 → turn 2 body carries no threadId key. The assertions are the direct inverse of the old behavior, so they fail pre-fix by construction. CLI suite 726 passed, lint clean, tsc clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016dNpnBqyyz9GxfznVcX2sP	2026-08-10 22:54:53 +01:00
Michal	bbd2195c64	test(mcplocal): prove chat SSE streams live through the proxy, not buffered ... `mcpctl chat reviewer` showed nothing until the turn finished, then dumped the whole answer at once. mcpd streams token deltas and the CLI renders them incrementally — the sole buffering point was mcplocal's catch-all /api/v1/* proxy reading the whole SSE body via res.text() before replying. The previous commit (cherry-picked from feat/agentic-teams) pipes the body through instead; this one adds the cover that was missing: - proxy-long-running.test.ts: a progressive-delivery test in which the stand-in mcpd withholds its final frame until the client has observed the first delta through the proxy. A buffering proxy cannot satisfy that ordering — verified: the test fails in 3s (no hang) against a res.text() proxy and passes against the piped one. The existing SSE test used inject(), which collects the whole body and so passes either way. - agent-chat.smoke.test.ts: a live smoke that posts through mcplocal on localhost:3200 (the path `mcpctl chat` actually takes — every other chat smoke uses --direct and bypasses the proxy entirely) and asserts delta frames arrive spread across the generation window, not in one burst at stream end. Uses its own agent: the shared smoke agent pins replies to a single token, too short to tell live streaming from a buffer dump. Also settles the strict-boolean-expressions lint on the auth-header check the streaming split touched. Local: workspace 2546 passed, proxy suite 10/10, smoke file loads + self-skips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016dNpnBqyyz9GxfznVcX2sP	2026-08-10 22:27:21 +01:00