feat(runtime): add extension lifecycle kernel - #2979
Conversation
|
Thanks for the kernel — the design is the right shape for the problem (agents need a safe way to define/activate/update/stop/remove runtime capabilities; a dynamic script leaking listeners/timers/UI entries breaks Run persistence and recovery). The immutable-revision + current/candidate + effect-ownership + serialized-mutation model is standard and self-consistent, and I verified it doesn't collide with existing authorities: the kernel is a new lifecycle authority inside Conclusion: PASS with two P2s (both handleable or explicitly deferred). P2-1 — P2-2 — no production callers: 1065 lines of kernel + 1331 lines of tests + package export are all unreachable. grep finds no consumer; the PR and issue #2973 explicitly defer wiring ("Those integrations remain follow-up phases"). Accepted as the declared Phase 1 boundary, but please explicitly record that the module is pure dead code with no behavioral verification loop until the Phase 2 adapter lands (and note the real integration risk is Phase 2's P3 (optional): an update-introduced dependency cycle leaves "status=failed but current still running" (the cycle handler only sets status/diagnostic, doesn't stop current; only the initial-activation cycle is tested); AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on 中文摘要（AI 辅助审查）结论：PASS（2 个 P2 需处理或显式延后）。设计形状正确（agent 需要安全地定义/激活/更新/停止/移除运行时能力；动态脚本泄漏 listeners/timers/UI 条目破坏 Run 持久化与恢复）；不可变 revision+current/candidate+effect 所有权+序列化变更模型标准自洽；与既有 authority 无冲突（@maka/runtime 内新的生命周期 authority 刻意不碰 Tool/permission/sandbox/Run authority，grep 确认 runtime 无既有扩展生命周期路径；与"Runtime Host 唯一执行权威"不冲突——Phase 1 未接线）。测试质量高（真实 TCP/EventEmitter/timer 泄漏检测、菱形依赖图、cleanup 失败恢复、2000 次 seeded soak+不变量断言）无可删；1065 行与 issue 需求 1:1 对应。CI 9/9 绿。P2-1：install 触发全 scope reconcile 静默重跑失败 binding 的扩展代码（prepare/activate）——reconcile 循环对 enabled、无依赖/owner、无 current 的 record 无条件重试 #activateInitial/#updateCurrent，不区分 status='failed'；即使失败 binding 的 prepare 在注册清理 effect 前有副作用（spawn/占端口）则每次 mutation 重复泄漏；ReconcileResult.errors 无人消费调用方无感知；与 kernel 文档（"Installing a revision never executes extension code"）和 prepare JSDoc（"仅当 binding enabled 且依赖满足时调用"）矛盾；retry() 既存为显式重试 API，隐式重试令人意外。修复：reconcile 跳过 status='failed' 恢复 retry() 显式语义，或文档化自动重试行为+补测试（我的建议测试：激活失败一个 binding 后 install 无关 revision 断言其 prepare 计数不变——当前实现会失败）。P2-2：无生产调用点（1065 行 kernel+1331 行测试+package export 全部不可达）——PR 与 issue #2973 已显式延后接线，接受为已声明的 Phase 1 边界，但请显式记录"合入后该模块是纯死代码、无行为验证闭环"，并注意 Phase 2 的 ownEffect 并入既有 Tool registry 时不得产生并行注册路径。P3（可选）：update 引入依赖环留"status=failed 但 current 仍在运行"（环处理只置 status/diagnostic 不停 current，仅初始激活环有测试）；stop/removeBinding/disposeScope 的 candidate?.controller.abort() 是死代码（序列化保证 mutation 时无 in-flight candidate、无 AbortSignal 监听者）；#mutate 在扩展代码（prepare/activate）内调 kernel mutation 会死锁（内层排队在外层 awaited 的 mutation 后）；activate 对既有 binding 不清 record.diagnostic 与 update/start 不一致；retiredOwners 挂起时无关 mutation 把 'active' 翻成 'failed'；#requireRevision 在 try 外；#mutate 无超时/看门狗（prepare 挂起永久阻塞所有后续 mutation，已文档化但无恢复手段）。 |
Astro-Han
left a comment
There was a problem hiding this comment.
The candidate/current split, dependency ordering, reverse effect cleanup, and scope isolation form a strong lifecycle core. Three seams still violate the kernel's own ownership contract: cancellation is queued behind the hook it must cancel, catalog installation can execute unrelated extension code, and retired effects lose their revision identity.
A simpler first-principles model separates three authorities: revision catalog mutations remain effect-free; cancellation is an out-of-band signal that can interrupt the serialized mutation currently awaiting a hook; every live or retired effect owner retains its revision identity until cleanup succeeds. The serialized queue can then remain the state-commit authority without also blocking cancellation or erasing provenance.
Review performed with three Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and current main.
中文评论
candidate/current 分离、依赖顺序、effect 逆序清理和 scope 隔离构成了扎实的 lifecycle 核心。但三个 seam 仍违背 kernel 自己的 ownership contract：cancellation 排在它必须取消的 hook 之后，catalog install 会执行无关 extension code，retired effects 又丢失 revision identity。
更简单的第一性原理模型应分开三个权威：revision catalog mutation 保持 effect-free；cancellation 是可中断当前等待 hook 的队列外信号；每个 live/retired effect owner 在清理成功前保留 revision identity。这样 serialized queue 可以继续作为 state commit 权威，而不会同时阻塞取消或抹掉 provenance。
本次审查使用了三位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具；我已依据最新 head 和当前 main 复核问题。
| } | ||
|
|
||
| stop(bindingId: string): Promise<ExtensionBindingInspection> { | ||
| return this.#mutate(async () => { |
There was a problem hiding this comment.
P1 — stop cannot abort the lifecycle hook that is blocking the mutation queue. prepare/healthCheck/activate are awaited inside #mutate; stop, removeBinding, and disposeScope only reach candidate.controller.abort() after that same mutation finishes. A hook that never resolves therefore makes the exposed AbortSignal unusable and permanently blocks teardown. Keep an out-of-band active-candidate cancellation lane: abort immediately, then enqueue state convergence; add a never-resolving hook regression test.
| ); | ||
| } | ||
| this.#revisions.set(key, installed); | ||
| await this.#reconcileAllScopes(); |
There was a problem hiding this comment.
P1 — install() violates the documented effect-free catalog contract. Installing any unrelated revision runs #reconcileAllScopes(), which can retry a previously failed enabled binding and execute its prepare/activation, allocating resources or publishing effects. Installing a revision cannot make an already-valid binding newly activatable because activate() already requires its exact revision. Remove reconciliation from install() and trigger lifecycle work only through explicit binding operations.
| `Extension revision is not installed: ${key}`, | ||
| ); | ||
| } | ||
| const user = [...this.#bindings.values()].find( |
There was a problem hiding this comment.
P2 — Pending retired effects are invisible to revision-in-use checks. After v1→v2 commits and v1 cleanup fails, the v1 owner remains in retiredOwners but has no revision identity; uninstall(ext, '1') succeeds and that revision can be reinstalled while its old effects remain live. Store the revision with every retired owner and reject uninstall/reinstall until matching cleanup succeeds (or conservatively block all revisions for that extension while retired owners exist).
|
/agentic_review |
Code Review by Qodo
1. Install executes extension code
|
| this.#revisions.set(key, installed); | ||
| await this.#reconcileAllScopes(); | ||
| }); |
There was a problem hiding this comment.
1. Install executes extension code 🐞 Bug ≡ Correctness
install() reconciles every scope, so installing an unrelated revision can rerun and even activate a previously failed enabled binding. This violates the explicit data-only installation contract and makes installation produce extension-owned effects.
Agent Prompt
## Issue description
`install()` invokes reconciliation and can execute `prepare`, health checks, or activation for existing bindings, contrary to the installation contract.
## Issue Context
A failed enabled binding remains eligible for reconciliation. Installing any unrelated revision currently retries that binding and may publish effects; simply adding tests or validation cannot restore the invariant, so the unnecessary reconciliation path should be deleted.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[262-274]
- packages/runtime/src/extension-lifecycle-kernel.ts[515-518]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.test.ts[10-45]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const dependency = <T = unknown>(extensionId: string): T => { | ||
| const activation = this.#dependencyActivation(record.scopeId, extensionId); | ||
| if (!activation) { | ||
| throw new ExtensionLifecycleOperationError( | ||
| 'activation_failed', | ||
| `Required dependency ${extensionId} is no longer active`, | ||
| ); |
There was a problem hiding this comment.
2. Undeclared dependencies evade teardown 🐞 Bug ≡ Correctness
The activation context allows reading any active extension in the scope even when it was not declared as a dependency. Because readiness and dependent teardown only inspect declared dependencies, stopping that provider leaves the undeclared consumer active with a stale dependency value or resource.
Agent Prompt
## Issue description
`dependency()` and `dependencyRevision()` must reject extension IDs absent from the candidate revision's declared dependencies.
## Issue Context
The existing dependency-definition list is already the lifecycle authority used for readiness and teardown, so reuse it rather than adding another dependency registry or dynamic edge state. Add coverage showing an undeclared active provider cannot be read and that declared providers still trigger dependent teardown.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[728-756]
- packages/runtime/src/extension-lifecycle-kernel.ts[759-769]
- packages/runtime/src/extension-lifecycle-kernel.ts[833-839]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.test.ts[156-198]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const record = this.#requireBinding(bindingId); | ||
| record.enabled = false; | ||
| record.candidate?.controller.abort(); | ||
| const failures = await this.#retryRetiredOwners(record); | ||
| failures.push(...(await this.#deactivateCascade(record, new Set()))); |
There was a problem hiding this comment.
3. Stop cannot abort preparation 🐞 Bug ☼ Reliability
stop() is queued behind the activation mutation and therefore cannot reach controller.abort() while prepare() or healthCheck() is awaiting the exposed signal. A cooperative candidate waiting for abort consequently blocks both stop and the global mutation queue indefinitely.
Agent Prompt
## Issue description
Stop, removal, and scope disposal need to signal cancellation while candidate work is still awaiting, without allowing concurrent committed-state mutation.
## Issue Context
Moving all state mutation outside serialization would weaken the kernel invariant, while the current queued abort is unreachable during an in-flight await. Use the smallest cancellation-request mechanism that can abort the active candidate immediately and any candidate started by an already queued earlier mutation; this introduces per-binding cancellation intent and associated race tests because no existing external cancellation seam can satisfy the invariant.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[387-451]
- packages/runtime/src/extension-lifecycle-kernel.ts[506-513]
- packages/runtime/src/extension-lifecycle-kernel.ts[622-650]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.test.ts[392-430]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| binding.extensionId === extensionId && | ||
| (binding.desiredRevision === revision || | ||
| binding.current?.definition.revision === revision || | ||
| binding.candidate?.definition.revision === revision), |
There was a problem hiding this comment.
4. Uninstall ignores retained activations 🐞 Bug ≡ Correctness
After a new revision commits and old-current cleanup fails, uninstall() can delete the old revision because it does not associate retired effect owners with their originating revision. The binding still owns and must retry old-revision cleanup, contradicting the revision-in-use invariant.
Agent Prompt
## Issue description
Prevent uninstallation while cleanup ownership from that revision remains retained.
## Issue Context
Desired/current/candidate references are already checked, but failed old-current cleanup is reduced to an unversioned `EffectOwner`. Consolidate retired cleanup into a small retired-activation record carrying the existing definition and owner rather than introducing a separate reference authority; this adds revision metadata to retained cleanup and requires an uninstall-after-failed-cutover regression test.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[229-240]
- packages/runtime/src/extension-lifecycle-kernel.ts[277-301]
- packages/runtime/src/extension-lifecycle-kernel.ts[613-619]
- packages/runtime/src/extension-lifecycle-kernel.ts[849-870]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.test.ts[490-524]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const dependencies = [...(definition.dependencies ?? [])].map((dependency) => { | ||
| validateId('dependency.extensionId', dependency.extensionId); | ||
| if (dependency.extensionId === definition.extensionId) { |
There was a problem hiding this comment.
5. Nested definitions throw typeerror 🐞 Bug ≡ Correctness
normalizeDefinition() dereferences dependency and contribution entries without validating that each entry is an object. Inputs such as dependencies: [null] escape the documented lifecycle error contract as native TypeErrors instead of invalid_definition.
Agent Prompt
## Issue description
Malformed dependency and contribution entries must produce `ExtensionLifecycleOperationError` with `invalid_definition`, not native property-access errors.
## Issue Context
Reuse `invalidDefinition()` before reading fields from each entry. This is a local validation correction and needs no new public surface or state.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[939-965]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.system.test.ts[280-303]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (!prepared || typeof prepared !== 'object' || typeof prepared.activate !== 'function') { | ||
| throw new ExtensionLifecycleOperationError( | ||
| 'invalid_definition', | ||
| `Extension ${definition.extensionId}@${definition.revision} returned an invalid candidate`, | ||
| ); |
There was a problem hiding this comment.
6. Malformed disposer becomes permanent 🐞 Bug ☼ Reliability
Candidate validation checks only activate, so a truthy non-function dispose is committed through a wrapper that fails on every cleanup attempt. This can leave an otherwise successful binding permanently stuck with cleanup_failed, while malformed healthCheck values are also misclassified rather than rejected as invalid definitions.
Agent Prompt
## Issue description
Validate `healthCheck` and `dispose` as functions when present before registering or invoking them.
## Issue Context
The candidate-shape validation is the closest existing seam and already rejects a missing `activate`. Extending that local validation is sufficient; no new state, authority, or public surface is needed, but malformed optional-member regression tests should be added.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[635-650]
- packages/runtime/src/extension-lifecycle-kernel.ts[198-210]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.system.test.ts[362-404]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| record = existing; | ||
| const cleanupFailures = await this.#retryRetiredOwners(record); | ||
| if (cleanupFailures.length > 0) throw cleanupOperationError(cleanupFailures); | ||
| record.desiredRevision = input.revision; | ||
| record.enabled = true; |
There was a problem hiding this comment.
7. Waiting retains stale diagnostic 🐞 Bug ◔ Observability
Calling activate() on an existing failed binding changes its desired revision and status but does not clear the previous diagnostic. If the new revision waits for a dependency, inspection reports waiting alongside an unrelated old failure diagnostic.
Agent Prompt
## Issue description
Clear a prior diagnostic when existing-binding `activate()` starts a new desired lifecycle attempt.
## Issue Context
Reuse the behavior already present in `update()` and `start()` rather than adding diagnostic state or new APIs. Add a regression covering failed revision followed by activation of a dependency-waiting revision.
## Fix Focus Areas
- packages/runtime/src/extension-lifecycle-kernel.ts[305-349]
- packages/runtime/src/extension-lifecycle-kernel.ts[352-379]
- packages/runtime/src/extension-lifecycle-kernel.ts[546-551]
- packages/runtime/src/__tests__/extension-lifecycle-kernel.test.ts[461-488]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Closing in favor of the newer extension composition/kernel PRs. |
Summary
Add the product-independent Phase 1 lifecycle kernel from #2973, without wiring Tool, UI, or dynamic-script contribution adapters yet.
The implementation is intentionally isolated from Maka's existing Tool, permission, sandbox, and Run authorities. Those integrations remain follow-up phases in the issue roadmap.
Refs #2973
Verification
npm --workspace @maka/runtime run test:dist: 2819 pass / 0 fail / 12 skipnpm run typecheck: passed for all workspacesnpx biome check packages/runtime/src/__tests__/extension-lifecycle-kernel.system.test.ts docs/architecture/extension-lifecycle-kernel.md: passedgit diff --check: passedThe system suite drives the exported kernel rather than mocking lifecycle behavior. It uses a real TCP server/client, actual port release, EventEmitter listeners, timers, a transitive dependency graph, cleanup-failure recovery, and a 2,000-operation seeded lifecycle soak across multiple scopes and revisions.
Review focus
Checklist
Does this PR entail a change in behavior?