feat(runtime): add plugin-backed Session executors - #5283
Conversation
Allow Host plugins to register scoped black-box executors through ctx.executors and route their output through the existing Session event stream. Persist executor selection across ordinary Sessions, WorkHub-created work, child agents, Graph operators, and Session revisions without exposing Maka tools to the external runtime. Generated-by: Codex
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for putting this together. I reviewed c6727723a75eb4c450a6f86d45c55f37adde1a83 from the execution boundary outward. Treating a complete external agent as a backend, while keeping Session admission and persistence in the Host, is a sensible direction. Keeping its processes, credentials, and tools inside the plugin also avoids forcing an agent into the model-provider contract.
P1 — Define the external conversation contract before enabling ordinary Session copies. Reachability: a normal branch or revision of a root executor Session. session-revision-coordinator.ts now copies executorId and the existing coordinator still copies the selected transcript into the new Session. On its next turn, however, PluginExecutorBackend.#produce sends only the new Session's conversationKey, the current text, and attachment/reference fields. It drops BackendSendInput.runtimeContext and supplies neither the source conversation nor the exact copy boundary. A provider that correctly maintains its own conversations by the documented key starts a fresh external conversation, even though Maka presents the copied history.
I checked the copy/admission callers and captured the request from the exact backend: with a copied history containing a code word and a new question asking for it, the provider receives only the question and the new Session ID. The copied fact never reaches that request. Plugins can query visible Session messages through ctx.sessionQuery, but the new executor contract neither requires that nor defines how those messages relate to the external conversation at the selected boundary. Leaving each plugin to guess would move the same continuity problem into every integration.
Could you first state which Session operations this initial executor contract actually supports? The smallest safe revision is to keep the existing backend/Host seams, explicitly reject unsupported branch/revision operations at their Host authority, and only enable them when a concrete provider can preserve the selected history boundary. Please apply the same contract check to resume/recovery rather than treating a persisted executor ID as proof that external context has been restored. A fresh one-turn Codex run, including child/Graph routing, does not exercise this boundary.
This does not need a second history model or a generic restoration framework. A bounded first integration with explicit unsupported-operation errors would be easier to validate and maintain. I suggest keeping this work in progress until the continuity contract and its production-path acceptance are clear.
The two inline P2s are separate local defects: class-provider registration loses its method, and retirement cancellation loses its cause. They can be repaired at the existing executor boundary without widening the architecture.
Validation: exact-head source/caller inspection plus executable probes for the backend request, class-provider registration, and reject-on-abort retirement. The latter produced error followed by complete(error) for a service-initiated retirement. The copy flow itself was traced, not exercised end to end against Codex. I did not rerun the repository suites or the author's live Codex experiment. AI-assisted review with Codex and Reviewer Sol.
| } | ||
| const entry: RegisteredExecutor = { | ||
| ...identity, | ||
| provider: Object.freeze({ ...provider }), |
There was a problem hiding this comment.
P2 — Preserve the registered provider's callable method. Reachability: a normal plugin implemented as a class. validateProvider accepts a prototype execute method, but { ...provider } only copies own enumerable properties. Registration succeeds and the first execution then throws TypeError: entry.provider.execute is not a function. I reproduced this through the real Context and service with class Provider { id = 'remote'; async execute() { return { status: 'completed', text: 'ok' }; } }. Could you retain the provider or explicitly bind and store its execute method? A class-instance registration/execution regression would cover this public interface shape.
| ); | ||
| this.#publishResult(turnId, messageId, result, queue); | ||
| } catch (error) { | ||
| if (signal.aborted) { |
There was a problem hiding this comment.
P2 — Carry service-initiated cancellation across the backend boundary. Reachability: hot reload or retirement while an executor is running. The service aborts its internal signal, combined with this backend signal using AbortSignal.any. If the provider follows the normal fetch/SDK pattern and rejects on that abort, this outer signal is still not aborted. The catch therefore emits error and complete(error) instead of cancellation. An exact-code service/backend probe reproduced Executor was retired: remote followed by complete(error); the existing AgentRun consumer records that as a failed terminal outcome. Conversely, a provider returning cancelled loses its reason and is always labelled user_stop.
Could the service pass an explicit cancellation cause through to this adapter, preserving the actual source in the existing terminal mapping? That keeps retirement independent of whether the provider rejects or returns a cancelled result, and avoids guessing from the wrong signal. Please cover reject-on-abort retirement alongside the existing resolve-as-cancelled case.
Sun-GLiang
left a comment
Sun-GLiang
left a comment
There was a problem hiding this comment.
Thanks for the change. I reviewed c672772. The scoped registry and Session propagation are covered well, and 362 focused Core, Storage, Runtime, and Runtime Host tests passed locally. I found four behavioral gaps and two cleanup/reuse issues below. Two focused negative controls reproduce the lost executor provenance and stop-after-abort behavior.
Review assisted by Codex; the findings were checked against the production paths and focused probes.
| ) | ||
| .digest('hex')}` as const; | ||
| return { | ||
| providerStateIdentity, |
There was a problem hiding this comment.
[P1] Persist plugin executor identity in invocation openings
This identity is computed and handed to RuntimeKernel, but AgentRun.buildInvocationOpening() selects provenance: 'unknown' whenever llmConnectionId is absent. Plugin-executor Sessions deliberately omit that field, so every executor Run drops the exact executor generation from its durable opening.
I extended the existing child-executor regression to require runtime provenance and this hash; the actual value was unknown. Please add a trusted executor route shape carrying executorId and providerStateIdentity, and use it in both fresh and continuation opening builders.
| .digest('hex')}` as const; | ||
| return { | ||
| providerStateIdentity, | ||
| build: (factoryContext) => |
There was a problem hiding this comment.
[P1] Keep the cached backend bound to the prepared executor generation
prepare() hashes the currently visible contribution, but the backend stores only executorId; every later send() resolves that id again through PluginExecutorService. RuntimeKernel caches the backend and its prepared identity, while plugin replacement changes the registry entry without invalidating that cache.
After a hot reload, the next Run can therefore be admitted and recorded with the old generation identity while executing the new provider. Please either bind a generation-specific execution handle during preparation or invalidate affected cached backends whenever executor membership changes.
| } | ||
| }, | ||
| }); | ||
| return normalizeResult(result); |
There was a problem hiding this comment.
[P2] Let cancellation win over a late provider result
Please recheck the combined signal after provider.execute() settles. A provider can observe or catch the abort and still return { status: 'completed' }; this path accepts that result, so user stop or executor retirement can be published as a successful end_turn.
Changing the stop fixture to return success after receiving the abort produced text_complete, complete instead of abort, complete. An exception caused by retirement has the related problem that it becomes an ordinary failure because only the backend's outer signal is inspected. Normalize any return or throw after this combined signal aborts to cancelled, and cover both stop and retirement races.
| readonly connectionSlug: string; | ||
| readonly model: string; | ||
| }> { | ||
| if (input.executorId) { |
There was a problem hiding this comment.
[P2] Validate executor availability before committing the Session
This accepts any syntactically valid executor id without checking whether that contribution is visible to the target Session. The durable Session is then created and projected as ready; the first Turn fails only when backend preparation calls pluginExecutors.identity().
Model-backed creation validates its exact route before persistence. Please give executor creation the same fail-closed behavior, or explicitly project the Session as unavailable until the named executor becomes visible.
| this.#now = input.now ?? Date.now; | ||
| } | ||
|
|
||
| providerStateIdentity(): `sha256:${string}` { |
There was a problem hiding this comment.
[P3] Remove the unused duplicate identity implementation
providerStateIdentity() has no callers in the repository, and its hashing algorithm is duplicated in execution-composition.ts. Please remove this method, or extract one shared helper/identity handle and make preparation and execution consume that single implementation.
| } | ||
|
|
||
| function isExecutorId(value: unknown): value is string { | ||
| return typeof value === 'string' && /^[A-Za-z][A-Za-z0-9._:-]{0,127}$/u.test(value); |
There was a problem hiding this comment.
[P3] Reuse one executor-id validator
The same executor-id expression is now repeated across eight production locations in Core, Runtime, Runtime Host, and Storage. That creates multiple acceptance boundaries for one domain identifier and makes later changes prone to partial updates.
Could @maka/core expose the canonical predicate or pattern, with the protocol, persistence, service, and Zod boundaries reusing it?
likun666661
left a comment
likun666661
left a comment
There was a problem hiding this comment.
Reviewed 8bb9854. No blocking issues found in this static follow-up review. The current implementation addresses the earlier concerns with explicit guards for unsupported conversation copying and safe-boundary continuation, generation-bound executor handles, persisted executor provenance, cancellation normalization, bound provider methods, and executor availability validation before Session creation. The plugin/backend boundary is coherent and the scope is appropriate for an initial external-executor extension point.
Validation: inspected the current diff and existing review findings; GitHub audit and test checks are passing. I did not rerun the suites or external-runtime E2E locally. AI-assisted review with Codex.
4 participants
Summary
ctx.executorscontribution so Host plugins can register, inspect, execute, cancel, and drain black-box runtimesexecutorIdacross ordinary Session creation, WorkHub-created work, child agents, Graph operators, catalog projections, and Session revisionsThis PR adds the plugin extension point and Maka routing bridge only. It does not ship a Codex provider or add a UI/default executor selector.
Verification
npm run buildnpm run typechecknpx biome checkon all 46 changed filesnpm run check:asf-headersgit diff --check origin/main...HEADReview focus
The executor id is a Maka routing identity, while any external conversation id remains plugin-owned. Registration follows existing plugin scope and transactional hot-reload semantics; retirement aborts and drains active external calls before release.
AI use
Select exactly one:
Tool(s) and scope: Codex authored the implementation, tests, local Codex app-server verification probe, and PR description. The commit includes a
Generated-by: Codextrailer.Checklist
Does this PR entail a change in behavior?