Uh oh!
There was an error while loading. Please reload this page.
feat(plugins): add Cordis-scoped dynamic tool contributions - #4443
Conversation
likun666661
commented
Sep 1, 2026
I think the existing Maka already has a mechanism for changing the provider-visible Tool surface on the next logical model step:
This was an explicit part of the agreement in #3752:
#4098 then simplified this further: the final executable binding is the only availability authority, every non-direct bound Tool is deferred by default, and groups are only search metadata. Against that existing contract, this PR currently does something broader: That duplicates part of the existing availability mechanism and, more importantly, changes the capability model from “the Run binding is the ceiling” to “the ceiling may expand or contract during the Run.” I think that is a separate architectural decision from allowing plugins to contribute Tools. There is also a production-semantics gap in the current tests. The main dynamic plugin tests construct the backend without Suggested minimal integrationI would keep the valuable Plugin Platform work in this PR:
But I would connect it to Runtime through an immutable binding snapshot rather than a live interfacePluginToolSnapshot{revision: stringtools: readonlyMakaTool[]groups: readonlyToolGroup[]release(): void}The Interactive Run Composer would take one plugin snapshot while constructing the backend/Run, merge its Tools and group metadata into the final executable binding, and let the existing Plugin package/entry identity can naturally provide search-source metadata, for example: On install/enable/uninstall, the Plugin Platform updates canonical state and invalidates idle backends. The active Turn keeps its pinned snapshot; the next Turn receives the new binding. Uninstall can remove the entry from future snapshots immediately while reporting cleanup pending until snapshot references and active calls drain. This fits the existing This path would avoid:
If same-Turn install-and-use is a hard requirementThat is a valid but stronger feature. It should be stated as an explicit change to the #3752 capability contract. The ceiling would no longer be a fixed executable Tool set; it would become a fixed set of trusted Tool sources whose contents may change. Even in that design, I do not think the best seam is “re-resolve every Tool before every step.” A more coherent extension would make the Plugin Tool registry a dynamic source behind
This preserves the existing lazy-loading model and makes the additional authority explicit instead of introducing a parallel dynamic-composition path. My recommendation is therefore to start with the snapshot/binding integration and next-Turn mutation semantics. If the required product behavior is specifically “the model installs or authors a plugin and invokes it in the same Turn,” that should be separated and reviewed as a dynamic Tool-source/capability-ceiling change. Without that requirement, most of the per-step composition and persistence work in this PR appears unnecessary. The key decision to settle before continuing is: must plugin installation or removal affect the Turn that is currently running? |
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed 271fa3e3. Thanks for this — the scoped registry is the right shape, and I like that Profile/Session layering resolves through one registry instead of two lookups. The Fiber-owned lifetime and the Plugin Platform transaction hook are also the parts I'd have worried about most, and they hold up: I traced a dispose during an in-flight call and the retired entry throws rather than silently swapping an implementation, so a Turn can't end up executing a Tool it never advertised.
I also want to say up front that I'm not questioning the per-step epoch design. recordRequestComposition's own comment says it's matching DSH request/header epochs, and re-sampling the surface between logical steps is exactly what makes live registration meaningful. Everything below takes that as given.
Four things, one of which I think should block merge — but it's the one that's easiest to fix, because it isn't part of this feature at all.
P0 — the RunComposition v1→v2 change will stop the Host from starting on any existing install. This is separable from the plugin work, and I think dropping it from this PR is cheaper than adding a migration. Details inline on run-composition.ts.
P2 — Host-owned core Tools cannot be shadowed isn't true on the toolProfile path. The summary states this as a property of the change, and it holds for boundTools, but the two guards in interactive-run-composer.ts are checking different things on adjacent lines. Inline.
P2 — the best-effort store latch now sits in front of provider dispatch. You followed the file's existing pattern here and I don't think that was the wrong instinct; the difference is what's downstream of it. Inline.
P2 — epoch equality only compares against the previous epoch, so an oscillating surface re-appends in full. Inline, with the numbers.
A couple of smaller notes I don't think are worth inline threads:
toolNamesis capped at 256 andtoolSchemasat 512, but both are derived from the sameactiveToolsForRequest, so the real cap is 256 and nothing upstream enforces it. Since the summary pitches adapting large catalogues (the 59-Tool dsh-quant example), it's worth aligning those two and deciding whether hitting the cap should truncate the record or fail the step. Same question for MCP tool descriptions —boundedString(schema.description, 16_384)in the snapshot, no length bound at all inmcp-tools.tswhere the description comes straight from the server. I have no evidence a real server exceeds 16 KB, so this is a "which side should give" question rather than a reported bug.toolAvailabilityHashin the per-step epoch readsthis.input.toolAvailability, frozen at backend construction, while the catalogue is now re-sampled each step throughresolveTools(). The real change is already covered bytoolCatalogHash/toolNames/toolSchemas, so nothing is wrong — the field just can't do what its name promises in a per-step record.
One thing worth knowing about plugin-tool-service.test.ts: the conflict case ('desktop-ui and Host-owned Tool conflicts fail closed') calls tools.resolve('alpha', [tool('Read', 'host')]) with an explicit core list, but production calls pluginTools.resolve(sessionId, []) at execution-composition.ts:700. So the guard that test exercises never runs in production, which is why the shadowing path below is green. None of the three test files go through createInteractiveRunComposer; one test that does would cover the second finding directly.
Evidence boundary: I read pr4443 against origin/main and ran the PR's own run-composition.ts + record-schema.ts in isolation to check the decode both directions. The startup consequence in the P0 is traced through the call chain and through Desktop's startDesktopRuntimeHostWithRecovery, not reproduced end to end — I did not stand up an old database and watch a Host fail to start. The 44 KB/epoch figure is measured from 40 real Tools in a main build, so it excludes MCP and plugin contributions and is a lower bound. I did not run the test suites.
AI-assisted review: drafted with Maka; I verified the decode failure, the two guards, the latch's callers, and the shadowing path against the branch source myself.
| import { defineObjectShape, hasExactShape, isRecord } from './record-schema.js'; | ||
| export const RUN_COMPOSITION_SCHEMA_VERSION = 1 as const; | ||
| export const RUN_COMPOSITION_SCHEMA_VERSION = 2 as const; |
There was a problem hiding this comment.
P0 — every existing database has v1 rows, and v2 rejects them on a read path that runs during Host startup.
Two independent reasons a v1 row fails now: the version literal moved to 2, and RUN_COMPOSITION_SHAPE dropped sourceRevisions, baseSystemPromptHash, toolCatalogHash, toolAvailabilityHash and toolNames with an empty optional list, which hasExactShape treats as excess keys. I pulled this file and record-schema.ts out and ran them:
v1 record REJECTED Invalid Run Composition snapshot schema
v1 shape @ version 2 REJECTED Invalid Run Composition snapshot schema
v2 record decoded OK
main decoding v2 REJECTED Invalid Run Composition snapshot schema
So it breaks in both directions, and bumping only the version isn't enough to make an old row readable.
Every real Run has one of these: commitRunComposition is on beforeRunProviderDispatch unconditionally (execution-model-composition.ts:331), and the commit that introduced v1 is an ancestor of v0.2.0-incubating-rc1.
The consequence isn't a silent downgrade. decodeRunCompositionSnapshot throws, isRunCompositionSnapshot returns false, and agent-run.ts:707 throws Invalid AgentRun header schema. That surfaces during recovery with no per-row catch anywhere on the way:
agent-run-store.ts:538 rows.map(row => decodePersistedAgentRunHeader(...)) // bare map
-> listSessionRunsForRecovery -> hosted-execution-recovery -> prepareRecovery
-> execution-composition.ts:1776 -> host-kernel.ts:387 await this.#composition.recover()
host-kernel.ts:387 has a finally and no catch, so the rejection crosses Promise.race and #state never reaches 'ready'. On the Desktop side, startDesktopRuntimeHostWithRecovery rethrows anything canRepairManagedRuntimeHostStartup doesn't recognise, and that predicate only accepts RuntimeHostStartupError with one of seven deployment reasons — a raw schema error isn't among them, so the repair prompt isn't even offered. One old row, no start, no in-product way out. listSessionRunsPage, listSessionRunsBounded and readRun take the same path, and readSqliteAgentRunEvents:979 reads the header first, so events go with it.
The cheapest fix is probably to take this out of this PR. Dynamic plugin Tools don't need those five fields removed from RunCompositionSnapshot — as far as I can tell nothing else in the diff depends on the narrower shape. Landing the feature without the schema change means no migration to write and no P0.
If you'd rather keep it, the repo already has the seam: decodePersistedAgentRunHeader (agent-run.ts:620) is defined by its own comment and tests as the persistence boundary where retired values get folded — automationId and waiting_permission both go through it — while decodeAgentRunHeader stays strict about the current shape. A v1→v2 fold there (drop the removed fields, set schemaVersion: 2) is a few lines, and AGENT_RUN_CONTINUATION_SOURCE_V1_SHAPE/V2_SHAPE in the same file is the existing precedent for discriminated decoding.
Worth flagging either way: the only test that guarded this now asserts the new behaviour. sqlite-core-execution-store.test.ts:733's fixture was updated from schemaVersion: 1 to 2, so no test in the tree constructs a v1 record any more, and run-composition.test.ts asserts that v1-shaped input must throw.
There was a problem hiding this comment.
Fixed in 714bfc2. RunComposition is restored to the exact persisted v1 shape and semantics; the SQLite fixture is back on v1 and the decoder regression now explicitly accepts v1 and rejects v2. Dynamic request surfaces remain separate RequestComposition records, so existing databases require no migration.
| const tools = [...selectedTools]; | ||
| assertUniqueToolNames(tools); | ||
| const resolveTools = (): readonly MakaTool[] => { | ||
| const additionalTools = input.boundTools ? [] : (input.resolveAdditionalTools?.() ?? []); |
There was a problem hiding this comment.
P2 — this guard and the one at :153 are answering different questions, which is how plugin Tools reach toolProfile sessions.
136consthasToolCeiling=input.boundTools!==undefined||input.toolProfile!==undefined;139constadditionalTools=input.boundTools ? [] : (input.resolveAdditionalTools?.()??[]);153constclientCapabilityTools=hasToolCeiling ? [] : (input.clientCapabilities?.tools??[]);Client capability Tools are excluded from toolProfile sessions; plugin Tools aren't, because :139 only looks at boundTools. I think :139 wants hasToolCeiling too, and that's the whole fix.
What follows from it is why I'm raising it rather than leaving it as a nit. The summary says Host-owned core Tools cannot be shadowed, and PluginToolService does guard that — but only when it's handed the core list, and production calls pluginTools.resolve(sessionId, []) at execution-composition.ts:700, so that guard never fires. Downstream:
additionalToolslands inbuildDefaultHostTools(..., [...hostTools, ...additionalTools], ...)at:146, after the builtins.projectHostedExecutionToolsbuildsnew Map(tools.map(tool => [tool.name, tool]))— last wins — so a pluginBashdisplaces the Host one.selected = toolNames.map(name => byName.get(name))picks the plugin entry.- Then:
tool.name==='Bash'
? { ...tool,description: HEADLESS_CODING_V1_BASH_DESCRIPTION,parameters: HEADLESS_CODING_V1_BASH_PARAMETERS}
: tool...tool keeps the plugin's impl while description and parameters are overwritten with the Host contract. The model is shown the Host's Bash schema and calls the plugin's implementation against it.
assertUniqueToolNames(resolved)runs after the Map has already collapsed the duplicate, so it can't see the collision.
I graded this P2 rather than higher because Host plugins are trusted and already run arbitrary code in the Host process, so shadowing Read grants no capability they didn't have; uninstalling restores the original, and nothing persisted or externally visible changes. What makes it worth fixing before merge is that it's silent — a plugin author who picks a colliding name gets a schema/implementation mismatch with no diagnostic, and the comment above assertUniqueToolNames describes exactly the invariant that's being missed here.
There was a problem hiding this comment.
Fixed in 714bfc2. resolveAdditionalTools now uses the same hasToolCeiling guard as Client Capability Tools, covering both boundTools and toolProfile. Added an interactive composer regression proving an explicit profile excludes plugin additions and preserves exactly one Host Read binding.
| if (!this.input.runStore) { | ||
| throw new Error('AgentRun store is not configured'); | ||
| } | ||
| if (!this.runStoreAvailable) throw new Error('AgentRun store is unavailable'); |
There was a problem hiding this comment.
P2 — this is the same latch check as :554 and :578, but it's the first one in front of provider dispatch.
You followed the file's existing pattern and I don't think that was wrong — recordModelProjectionTransition and recordHistoryCompactCheckpoint both open with this line on main. The difference is what happens when it trips. Those two are recorders whose failure the caller can absorb; recordRequestComposition is awaited before every provider request, so a false latch fails the step itself.
runStoreAvailable is best-effort state: enqueueRunStore:1643 clears it on any trace-append failure, and a busy SQLite is enough. Because the throw happens before enqueueRequiredRunStoreWrite runs, the probe that would lift the latch back never executes, so a single transient hiccup turns into "every remaining step of this Run fails before dispatch" with no self-repair.
recordRunComposition right above shows the shape that avoids this: it goes straight to enqueueRequiredRunStoreWrite, whose comment spells out the reasoning — a successful required write proves the store is available again. Dropping :476 and letting the required-write path do its own probing gets the same durability with a recoverable failure mode.
There was a problem hiding this comment.
Fixed in 714bfc2. The best-effort runStoreAvailable precheck is removed; Request Composition goes through enqueueRequiredRunStoreWrite directly, so a successful required write lifts the latch while dispatch remains fail-closed on a real write failure.
| input, | ||
| this.requestComposition ? 'change' : 'initial', | ||
| ); | ||
| if ( |
There was a problem hiding this comment.
P2 — comparing only against the previous epoch means an oscillating surface appends a full replacement every step.
sameRequestCompositionSurface(this.requestComposition, snapshot) looks at the most recent epoch only, so an A→B→A→B sequence writes four full records. activeToolsForRequest does move around within a Turn — repair plans, sandbox boundary finalization and the final child summary step all rewrite it (ai-sdk-backend.ts:2101-2105).
Serializing 40 real Tools from a main build the way the snapshot does gives 44,049 characters per epoch (maka_computer alone is 14 KB, Bash 3.1 KB), before any MCP or plugin Tools. Against EXECUTION_INSPECT_EVIDENCE_MAX_BYTES = 512 * 1024 — shared between AgentRun and runtime events, accumulated by stored_bytes in bounded-evidence.ts:58 — that's roughly 11 epochs before the Run returns limit_exceeded and InspectQueryTooLargeError tells the operator to stop the Host and inspect offline. These events are append-only with no compaction and no cleanup path, and every whole-ledger loader (history-compact-ledger.ts:62, canonical-turn-snapshot.ts:56, conversation-copy.ts:281, …) parses and discards them.
Also worth noting that this.requestComposition is memory-only and never rehydrated from the ledger, so each resume writes a fresh initial epoch even when the surface is identical to what the previous instance recorded. That makes reason less reliable as a ledger fact than it reads — the model-call-attempt.ts:158-159 comment currently suggests every step carries an id, but compaction and memory sub-calls build their own tracker at ai-sdk-backend.ts:3303 and leave requestCompositionId undefined even on a fully upgraded Run.
Deduplicating against every epoch already in this Run — or storing toolSchemas once per (runId, surfaceHash) and having epochs reference it — would keep the DSH-style per-step record without the growth. Since nothing in the tree reads request_composition_resolved or dereferences requestCompositionId outside tests yet, there's also room to store just the hash for now and add the full schemas when a reader needs them.
There was a problem hiding this comment.
Fixed in 714bfc2. Request Composition snapshots are now indexed by a canonical full-surface hash across the entire Run, with existing epochs rehydrated from the ledger. A→B→A and reopening the same Run both reuse the original composition id; ModelCallAttempt references retain the step timeline without repeating full schemas.
xxhZs
commented
Sep 2, 2026
Same-Turn install/enable/use is a hard requirement for this PR, so the per-step dynamic source remains. The compatibility follow-up in 714bfc2 keeps RunComposition v1 unchanged, moves all dynamic evidence to immutable logical-step RequestComposition snapshots, preserves retry freezing and fail-closed dispatch, prevents same-name replacement from inheriting an old tool_search activation, and keeps explicit bound/profile ceilings exact. Full workspace lint/typecheck and 348 focused regressions pass. |
xxhZs
commented
Sep 2, 2026
Followed up on the two non-blocking composition-bound notes in fadbd38: RequestComposition now applies the same fail-closed 256-entry bound to both toolNames and toolSchemas (and the same 128-character name bound), so exact evidence is never truncated. MCP descriptions are normalized to the persisted 16 KiB bound before becoming provider-visible, while trusted Plugin Tool registration rejects empty or oversized descriptions atomically. Added focused boundary regressions; full lint, all-workspace typecheck, and 253 related tests pass. |
likun666661
commented
Sep 2, 2026
There is a blocking regression in the latest head: the per-step resampling now breaks existing The new replacement guard in for(const[name,activatedTool]ofactiveTools){if(this.toolsByName.get(name)!==activatedTool)activeTools.delete(name)}That is not compatible with the current composer lifecycle.
The resulting sequence is deterministic: So The plugin weather scenario does not disprove this. Object reference is therefore not a valid general contribution identity. The minimal coherent fix is to keep the base Host binding stable and resample only dynamic plugin contributions, while carrying an explicit activation identity: A same-name plugin replacement should invalidate activation because its contribution identity changed. An unchanged static Tool must retain activation even if an implementation currently rebuilds its wrapper. Please add a production-path regression through
The compatibility fixes in |
Summary
This PR makes Host plugin Tools a first-class, Context-scoped runtime contribution and allows the effective model Tool surface to change safely between logical model steps in the same Turn.
ctx.tools.register()for trusted Host plugins, with registrations owned by the registering Fiber and the Plugin Platform transactiontools/changenotifications and roll publication back atomically if a listener rejects the changeRuntime semantics
Tool changes take effect at model-step boundaries, never in the middle of an in-flight provider request:
This produces deterministic semantics:
tool_searchactivationComposition and persistence
RunCompositionkeeps its existing persisted v1 schema and semantics. It is written once before the first provider dispatch and remains the immutable initial Run baseline, including composer identity/revision, Prompt and Tool hashes, source revisions, provider options, Tool names, and context-window facts. Existing databases decode without migration.Dynamic Prompt and Tool state is recorded separately. Before each logical model-step dispatch, Runtime durably resolves a
request_composition_resolvedsnapshot containing:The current request and all of its physical retries bind one immutable
requestCompositionId. Surface changes affect only the next logical step. Snapshots are indexed by their complete canonical surface across the whole Run, including after reopening the Run, so A → B → A reuses A instead of appending another full schema record.ModelCallAttemptreferences preserve the per-step timeline even when a prior snapshot is reused.Both commits remain fail-closed before provider dispatch. A required Request Composition write bypasses the best-effort store latch, so a transient trace-write failure can self-heal instead of permanently blocking the rest of the Run.
This preserves the original Runtime Host durability and audit guarantees while generalizing the invariant from “one immutable surface per Run” to “one immutable initial baseline plus immutable logical-step surfaces.”
What this enables
The PR itself provides the runtime and Plugin Platform primitives, not a built-in Tool generator. On top of these primitives, plugins can implement:
The last item requires an out-of-tree authoring/sandbox plugin; this PR deliberately does not add model-authored code execution to Maka core.
Verification
Automated regression
npm run typechecknpm run lintReal Maka + real model
The live scenarios were exercised with DeepSeek V4 Flash on the same dynamic Tool implementation before the compatibility follow-up:
committed + convergedcommitted + converged + cleanup complete; the catalog returned to zero plugin ToolsRun Composition changed after resolutionfailure no longer occursOut-of-tree compatibility and capability probes
These probes are not included in this PR; they use only the public behavior introduced here:
dsh-quantTools without modifying Maka, installed them through the real Plugin Platform, invoked a deterministic calculation, then uninstalled them with 0 Tools remainingTogether these checks demonstrate that the PR supports both ordinary plugin Tool packages and dynamically changing same-Turn Tool surfaces, while keeping request snapshots, retries, execution, cleanup, and durable audit facts coherent.
Boundaries