From 9423aa2689dfb4a0e2e5a56f585aa486800ba6a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 19:42:51 +0000 Subject: [PATCH 1/5] Implement extension activation lifecycle (Task 1.12) Adds host/activation.ts: createExtensionHost activates an extension exactly once per activationEvents match (onStartup, onLanguage:) or per lazy command execution, builds ExtensionContext, disposes subscriptions in reverse order plus deactivate() on shutdown, and quarantines a throwing or rejecting activate() as "failed" without affecting other extensions. Wires the command registry's execute() to await an injected activateExtension hook before re-dispatching an unresolved lazy command, falling back to the existing "not activated yet" error path unchanged when no hook is present. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- packages/core/src/commands/registry.test.ts | 132 +++++ packages/core/src/commands/registry.ts | 57 ++- packages/core/src/host/activation.test.ts | 504 ++++++++++++++++++++ packages/core/src/host/activation.ts | 418 ++++++++++++++++ packages/core/src/host/index.ts | 22 +- packages/core/src/index.ts | 13 + 6 files changed, 1130 insertions(+), 16 deletions(-) create mode 100644 packages/core/src/host/activation.test.ts create mode 100644 packages/core/src/host/activation.ts diff --git a/packages/core/src/commands/registry.test.ts b/packages/core/src/commands/registry.test.ts index ff25629..7d76484 100644 --- a/packages/core/src/commands/registry.test.ts +++ b/packages/core/src/commands/registry.test.ts @@ -367,6 +367,138 @@ test("registerLazy's Disposable removes the command, matching register()'s dispo expect(errors.at(-1)?.message).toBe("Command not found: demo.run"); }); +// --- activateExtension hook (design.md §4.2, Task 1.12) -------------------- + +test("execute() awaits the activateExtension hook and re-dispatches once it resolves the handler", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const activateCalls: string[] = []; + const registry = createCommandRegistry({ + log, + sink, + activateExtension: async (extensionId) => { + activateCalls.push(extensionId); + // Simulate the extension host's activate(ctx) replacing the lazy + // entry with a real handler via ctx.api.commands.register. + registry.register("demo.run", () => "activated!"); + }, + }); + registry.registerLazy("demo.run", { extensionId: "demo.ext" }); + + const result = await registry.execute("demo.run"); + + expect(result).toBe("activated!"); + expect(activateCalls).toEqual(["demo.ext"]); +}); + +test("execute() calls activateExtension only once across repeated executes once the handler resolves", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const activateCalls: string[] = []; + const registry = createCommandRegistry({ + log, + sink, + activateExtension: async (extensionId) => { + activateCalls.push(extensionId); + registry.register("demo.run", () => "activated!"); + }, + }); + registry.registerLazy("demo.run", { extensionId: "demo.ext" }); + + await registry.execute("demo.run"); + await registry.execute("demo.run"); + + expect(activateCalls).toEqual(["demo.ext"]); +}); + +test("execute() falls back to the not-activated-yet error when activateExtension does not resolve a handler", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + const activateCalls: string[] = []; + const registry = createCommandRegistry({ + log, + sink, + activateExtension: async (extensionId) => { + // The extension "activates" but never registers a real handler for + // this command (e.g. it failed, or the manifest was wrong). + activateCalls.push(extensionId); + }, + }); + registry.registerLazy("demo.run", { extensionId: "demo.ext" }); + + const result = await registry.execute("demo.run"); + + expect(result).toBeUndefined(); + expect(activateCalls).toEqual(["demo.ext"]); + expect(errors.at(-1)?.message.toLowerCase()).toContain("not activated yet"); + expect(errors.at(-1)?.extensionId).toBe("demo.ext"); +}); + +test("execute() never calls activateExtension for a command with no extensionId (plain register)", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + let activateCalls = 0; + const registry = createCommandRegistry({ + log, + sink, + activateExtension: async () => { + activateCalls += 1; + }, + }); + registry.register("editor.action.foo", () => "ok"); + + expect(await registry.execute("editor.action.foo")).toBe("ok"); + expect(activateCalls).toBe(0); +}); + +test("execute() never calls activateExtension for an unknown command ID", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + let activateCalls = 0; + const registry = createCommandRegistry({ + log, + sink, + activateExtension: async () => { + activateCalls += 1; + }, + }); + + expect(await registry.execute("no.such.command")).toBeUndefined(); + expect(activateCalls).toBe(0); +}); + +test("without an activateExtension hook, execute() keeps Task 1.11's not-activated-yet behavior unchanged", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + const registry = createCommandRegistry({ log, sink }); + registry.registerLazy("demo.run", { extensionId: "demo.ext" }); + + const result = await registry.execute("demo.run"); + + expect(result).toBeUndefined(); + expect(errors.at(-1)?.message.toLowerCase()).toContain("not activated yet"); +}); + +test("execute() keeps its never-throwing contract when activateExtension itself throws", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + const registry = createCommandRegistry({ + log, + sink, + activateExtension: async () => { + throw new Error("activation exploded"); + }, + }); + registry.registerLazy("demo.run", { extensionId: "demo.ext" }); + + const result = await registry.execute("demo.run"); + + expect(result).toBeUndefined(); + expect(errors.at(-1)?.extensionId).toBe("demo.ext"); + const logged = log.entries().filter((e) => e.level === "error"); + expect(logged.some((e) => e.error.message.includes("activation exploded"))).toBe(true); +}); + test("registerLazy twice for the same ID logs a re-registration warning (last-wins)", async () => { const log = createHostLog(); const { sink } = createRecordingSink(); diff --git a/packages/core/src/commands/registry.ts b/packages/core/src/commands/registry.ts index 5a76569..1210c61 100644 --- a/packages/core/src/commands/registry.ts +++ b/packages/core/src/commands/registry.ts @@ -7,11 +7,13 @@ * Lazy (manifest-declared, not-yet-activated) commands (design.md §4.1's * "lazy commands", §5's `CommandEntry = { handler?, meta, extensionId?, * lazy }`) are registered via {@link CommandRegistry.registerLazy} by the - * extension host (`host/registration.ts`) with no `handler` yet — real - * activation (calling the owning extension's `activate(ctx)` on first - * execution) is Task 1.12; until then, executing a lazy command reports a - * "not activated yet" `HostError` through `log`/`sink` rather than - * throwing or silently no-op'ing. + * extension host (`host/registration.ts`) with no `handler` yet. Real + * activation (Task 1.12, `host/activation.ts`) is wired in via + * {@link CommandRegistryDeps.activateExtension}: `execute()` on an + * unresolved lazy command awaits that hook (activating the owning + * extension) and re-dispatches before falling back to the "not activated + * yet" `HostError` reported through `log`/`sink` — never throwing or + * silently no-op'ing either way. */ import type { @@ -49,6 +51,21 @@ export interface CommandRegistryDeps { log: HostLog; /** Where user-facing command errors are surfaced (Req 3.4, 3.5). */ sink: StatusSink; + /** + * Activate the extension owning an unresolved lazy command before + * re-dispatching (Req 2.5, design.md §4.2's "executing a lazy command + * activates the extension first, then re-dispatches"). Supplied by + * `host/activation.ts`'s `createExtensionHost(...).activateExtension` at + * the assembly layer — see that module's TSDoc for the construction + * order. Optional so `registry.ts` has no hard dependency on the + * extension host: omitted (as in every registry.test.ts case with no + * host in the picture), `execute()` falls straight to the existing "not + * activated yet" error path, unchanged from Task 1.11's behavior. + * Documented to never throw/reject (matching `activateExtension`'s own + * contract); `execute()` guards the call anyway so a misbehaving + * implementation can't break its own never-throwing contract. + */ + activateExtension?: (extensionId: string) => Promise; } /** The public shape of the command registry — the implementation behind @@ -99,7 +116,7 @@ export function isValidCommandId(id: string): boolean { * the registry owning them. */ export function createCommandRegistry(deps: CommandRegistryDeps): CommandRegistry { - const { log, sink } = deps; + const { log, sink, activateExtension } = deps; const commands = new Map(); /** Guarded `log.append` — an injected log must not be able to break the @@ -177,15 +194,37 @@ export function createCommandRegistry(deps: CommandRegistryDeps): CommandRegistr } async function execute(id: string, ...args: unknown[]): Promise { - const entry = commands.get(id); + let entry = commands.get(id); + + if (entry && !entry.handler && entry.extensionId && activateExtension) { + // Lazy, not-yet-activated command (design.md §4.1, §4.2) — activate + // its owning extension, then re-look-up: activation is expected to + // replace this entry with a real handler via register() (Task 1.12). + try { + await activateExtension(entry.extensionId); + } catch (cause) { + // activateExtension is documented to never throw/reject; guard + // anyway so a misbehaving host implementation can't break + // execute()'s own never-throwing contract. + logSafely("error", { + extensionId: entry.extensionId, + message: `activateExtension("${entry.extensionId}") threw: ${describeError(cause)}`, + }); + } + entry = commands.get(id); + } + if (!entry) { const err: HostError = { message: `Command not found: ${id}` }; notifySafely(err); return undefined; } if (!entry.handler) { - // Lazy, not-yet-activated command (design.md §4.1) — real activation - // is Task 1.12; for now report and stop, never throw. + // Still lazy after the activation attempt above (no hook wired, the + // entry carried no extensionId, or activation ran but the extension + // never registered a real handler for this ID — including a + // "failed" activation, design.md §4.2) — report and stop, never + // throw. const err: HostError = { message: `Command "${id}" belongs to extension "${entry.extensionId ?? "unknown"}", ` + diff --git a/packages/core/src/host/activation.test.ts b/packages/core/src/host/activation.test.ts new file mode 100644 index 0000000..a7f38b9 --- /dev/null +++ b/packages/core/src/host/activation.test.ts @@ -0,0 +1,504 @@ +import { expect, test } from "bun:test"; +import type { ExtensionContext, Manifest, Tecode } from "@tecode/api"; +import { createCommandRegistry } from "../commands/registry"; +import { createExtensionHost, type ExtensionRecord } from "./activation"; +import { createHostLog, type HostError } from "./errors"; + +/** A `StatusSink` stub that records every error it receives (matches + * `commands/registry.test.ts`'s/`host/registration.test.ts`'s + * `createRecordingSink`). */ +function createRecordingSink() { + const errors: HostError[] = []; + return { + errors, + sink: { + error(err: HostError) { + errors.push(err); + }, + }, + }; +} + +function fixtureManifest(overrides: Partial = {}): Manifest { + return { + id: "demo.ext", + version: "1.0.0", + apiVersion: "1.0", + activationEvents: ["onStartup"], + contributes: {}, + ...overrides, + }; +} + +/** Builds an `ExtensionRecord` whose `loadModule()` resolves to `mod` + * (an in-memory fixture — no real files, no dynamic imports). */ +function fixtureRecord( + id: string, + manifest: Partial, + mod: unknown, + loadModule?: () => Promise, +): ExtensionRecord { + return { + id, + manifest: fixtureManifest({ id, ...manifest }), + extensionUri: `/extensions/${id}`, + storagePath: `/storage/${id}`, + loadModule: loadModule ?? (() => Promise.resolve(mod)), + }; +} + +/** A minimal `Tecode` whose `commands` namespace is the real registry under + * test (so a fixture's `activate(ctx)` can call `ctx.api.commands.register` + * and actually replace a lazy command) — every other namespace is unused by + * these tests and stubbed out via a type assertion. */ +function fixtureApi(commands: ReturnType): Tecode { + return { commands } as unknown as Tecode; +} + +// --- activateExtension: exactly-once, per event ----------------------------- + +test("onStartup activates a matching extension exactly once, even across repeated calls", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let activateCalls = 0; + const record = fixtureRecord("demo.ext", { activationEvents: ["onStartup"] }, { + activate() { + activateCalls += 1; + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateStartupExtensions(); + await host.activateStartupExtensions(); + + expect(activateCalls).toBe(1); + expect(host.getState("demo.ext")).toBe("active"); +}); + +test("activateExtension is a no-op for an unknown extension ID", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const host = createExtensionHost({ extensions: [], api: fixtureApi(commands), log, sink }); + + await expect(host.activateExtension("no.such.ext")).resolves.toBeUndefined(); + expect(host.getState("no.such.ext")).toBeUndefined(); +}); + +test("a missing activate export still marks the extension active", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const record = fixtureRecord("demo.ext", {}, {}); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + + expect(host.getState("demo.ext")).toBe("active"); +}); + +test("concurrent activateExtension calls for the same extension share one in-flight activation", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let activateCalls = 0; + const record = fixtureRecord("demo.ext", {}, { + async activate() { + activateCalls += 1; + await Promise.resolve(); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await Promise.all([host.activateExtension("demo.ext"), host.activateExtension("demo.ext")]); + + expect(activateCalls).toBe(1); +}); + +// --- onCommand, via registry.execute() re-dispatch -------------------------- + +test("registry.execute() on a lazy command activates its owning extension, then runs the real handler", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + let activateCalls = 0; + const commands = createCommandRegistry({ + log, + sink, + activateExtension: (id) => host.activateExtension(id), + }); + const record = fixtureRecord("demo.ext", { activationEvents: ["onCommand:demo.run"] }, { + activate(ctx: ExtensionContext) { + activateCalls += 1; + ctx.api.commands.register("demo.run", () => "activated!"); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + commands.registerLazy("demo.run", { extensionId: "demo.ext" }); + + const first = await commands.execute("demo.run"); + const second = await commands.execute("demo.run"); + + expect(first).toBe("activated!"); + expect(second).toBe("activated!"); + expect(activateCalls).toBe(1); + expect(host.getState("demo.ext")).toBe("active"); +}); + +test("onLanguage activates every extension declaring that language, exactly once", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let tsActivations = 0; + let jsActivations = 0; + const ts = fixtureRecord("lang.ts", { activationEvents: ["onLanguage:typescript"] }, { + activate() { + tsActivations += 1; + }, + }); + const js = fixtureRecord("lang.js", { activationEvents: ["onLanguage:javascript"] }, { + activate() { + jsActivations += 1; + }, + }); + const host = createExtensionHost({ extensions: [ts, js], api: fixtureApi(commands), log, sink }); + + host.onLanguage("typescript"); + // onLanguage is fire-and-forget (synchronous, matches DocumentManagerDeps' + // onLanguageActivation shape) — give the in-flight activation a tick to settle. + await Promise.resolve(); + await Promise.resolve(); + + expect(tsActivations).toBe(1); + expect(jsActivations).toBe(0); + expect(host.getState("lang.ts")).toBe("active"); + expect(host.getState("lang.js")).toBe("registered"); + + host.onLanguage("typescript"); + await Promise.resolve(); + + expect(tsActivations).toBe(1); +}); + +test("onLanguage is a plain synchronous void function", () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const host = createExtensionHost({ extensions: [], api: fixtureApi(commands), log, sink }); + + // No await, no .then — matches DocumentManagerDeps.onLanguageActivation's + // `(languageId: string) => void` shape exactly. + const result: void = host.onLanguage("typescript"); + expect(result).toBeUndefined(); +}); + +// --- subscriptions and deactivate -------------------------------------------- + +test("deactivateExtension disposes subscriptions in reverse push order, then calls deactivate()", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const order: string[] = []; + const record = fixtureRecord("demo.ext", {}, { + activate(ctx: ExtensionContext) { + ctx.subscriptions.push({ dispose: () => order.push("sub-1") }); + ctx.subscriptions.push({ dispose: () => order.push("sub-2") }); + ctx.subscriptions.push({ dispose: () => order.push("sub-3") }); + }, + deactivate() { + order.push("deactivate"); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + await host.deactivateExtension("demo.ext"); + + expect(order).toEqual(["sub-3", "sub-2", "sub-1", "deactivate"]); + expect(host.getState("demo.ext")).toBe("registered"); +}); + +test("deactivateExtension is a no-op for an extension that is not active", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const record = fixtureRecord("demo.ext", {}, {}); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await expect(host.deactivateExtension("demo.ext")).resolves.toBeUndefined(); + expect(host.getState("demo.ext")).toBe("registered"); + + await expect(host.deactivateExtension("no.such.ext")).resolves.toBeUndefined(); +}); + +test("a subscription that throws on dispose is logged, and the remaining subscriptions still dispose", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const order: string[] = []; + const record = fixtureRecord("demo.ext", {}, { + activate(ctx: ExtensionContext) { + ctx.subscriptions.push({ dispose: () => order.push("sub-1") }); + ctx.subscriptions.push({ + dispose: () => { + throw new Error("dispose boom"); + }, + }); + ctx.subscriptions.push({ dispose: () => order.push("sub-3") }); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + await host.deactivateExtension("demo.ext"); + + expect(order).toEqual(["sub-3", "sub-1"]); + const errors = log.entries().filter((e) => e.level === "error"); + expect(errors.some((e) => e.error.message.includes("dispose boom"))).toBe(true); +}); + +test("disposeAll deactivates every active extension and is idempotent", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const order: string[] = []; + const a = fixtureRecord("ext.a", {}, { + deactivate() { + order.push("a"); + }, + }); + const b = fixtureRecord("ext.b", {}, { + deactivate() { + order.push("b"); + }, + }); + const host = createExtensionHost({ extensions: [a, b], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("ext.a"); + await host.activateExtension("ext.b"); + + await host.disposeAll(); + expect(order.sort()).toEqual(["a", "b"]); + expect(host.getState("ext.a")).toBe("registered"); + expect(host.getState("ext.b")).toBe("registered"); + + // Idempotent: nothing left active, so a second call deactivates nothing. + await host.disposeAll(); + expect(order.sort()).toEqual(["a", "b"]); +}); + +// --- failure isolation -------------------------------------------------------- + +test("a throwing activate marks only that extension failed, reports a HostError, and leaves others unaffected", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let goodActivated = false; + const bad = fixtureRecord("bad.ext", { activationEvents: ["onStartup"] }, { + activate() { + throw new Error("boom"); + }, + }); + const good = fixtureRecord("good.ext", { activationEvents: ["onStartup"] }, { + activate() { + goodActivated = true; + }, + }); + const host = createExtensionHost({ + extensions: [bad, good], + api: fixtureApi(commands), + log, + sink, + }); + + await host.activateStartupExtensions(); + + expect(host.getState("bad.ext")).toBe("failed"); + expect(host.getState("good.ext")).toBe("active"); + expect(goodActivated).toBe(true); + expect(errors.some((e) => e.extensionId === "bad.ext" && e.message.includes("boom"))).toBe( + true, + ); + const logged = log.entries().filter((e) => e.level === "error"); + expect(logged.some((e) => e.error.extensionId === "bad.ext")).toBe(true); +}); + +test("a rejecting async activate likewise marks the extension failed and reports a HostError", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const record = fixtureRecord("demo.ext", {}, { + async activate() { + await Promise.resolve(); + throw new Error("async boom"); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + + expect(host.getState("demo.ext")).toBe("failed"); + expect(errors.some((e) => e.message.includes("async boom"))).toBe(true); +}); + +test("re-activating a failed extension is a no-op (failed is terminal until an explicit deactivate)", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let calls = 0; + const record = fixtureRecord("demo.ext", {}, { + activate() { + calls += 1; + throw new Error("boom"); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + await host.activateExtension("demo.ext"); + + expect(calls).toBe(1); + expect(host.getState("demo.ext")).toBe("failed"); +}); + +test("a rejected loadModule() marks the extension failed without throwing out of activateExtension", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const record = fixtureRecord( + "demo.ext", + {}, + undefined, + () => Promise.reject(new Error("load failed")), + ); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await expect(host.activateExtension("demo.ext")).resolves.toBeUndefined(); + + expect(host.getState("demo.ext")).toBe("failed"); + expect(errors.some((e) => e.message.includes("load failed"))).toBe(true); +}); + +test("subscriptions pushed before a throwing activate are still disposed", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let disposed = false; + const record = fixtureRecord("demo.ext", {}, { + activate(ctx: ExtensionContext) { + ctx.subscriptions.push({ + dispose: () => { + disposed = true; + }, + }); + throw new Error("boom after partial setup"); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + + expect(host.getState("demo.ext")).toBe("failed"); + expect(disposed).toBe(true); +}); + +// --- never-throw guarantees --------------------------------------------------- + +test("a throwing log does not break activateExtension's never-throwing contract", async () => { + const throwingLog = { + append() { + throw new Error("log is broken"); + }, + entries: () => [], + }; + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log: throwingLog, sink }); + const record = fixtureRecord("demo.ext", {}, { + activate() { + throw new Error("boom"); + }, + }); + const host = createExtensionHost({ + extensions: [record], + api: fixtureApi(commands), + log: throwingLog, + sink, + }); + + await expect(host.activateExtension("demo.ext")).resolves.toBeUndefined(); + expect(host.getState("demo.ext")).toBe("failed"); +}); + +test("a throwing sink does not break activateExtension's never-throwing contract", async () => { + const log = createHostLog(); + const throwingSink = { + error() { + throw new Error("sink is broken"); + }, + }; + const commands = createCommandRegistry({ log, sink: throwingSink }); + const record = fixtureRecord("demo.ext", {}, { + activate() { + throw new Error("boom"); + }, + }); + const host = createExtensionHost({ + extensions: [record], + api: fixtureApi(commands), + log, + sink: throwingSink, + }); + + await expect(host.activateExtension("demo.ext")).resolves.toBeUndefined(); + expect(host.getState("demo.ext")).toBe("failed"); +}); + +test("a throwing deactivate() is logged and does not stop deactivateExtension from completing", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const record = fixtureRecord("demo.ext", {}, { + deactivate() { + throw new Error("deactivate boom"); + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + await expect(host.deactivateExtension("demo.ext")).resolves.toBeUndefined(); + + expect(host.getState("demo.ext")).toBe("registered"); + const errs = log.entries().filter((e) => e.level === "error"); + expect(errs.some((e) => e.error.message.includes("deactivate boom"))).toBe(true); +}); + +// --- re-activation after deactivate ------------------------------------------- + +test("an extension can be activated again after an explicit deactivate", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let activateCalls = 0; + const record = fixtureRecord("demo.ext", {}, { + activate() { + activateCalls += 1; + }, + }); + const host = createExtensionHost({ extensions: [record], api: fixtureApi(commands), log, sink }); + + await host.activateExtension("demo.ext"); + await host.deactivateExtension("demo.ext"); + await host.activateExtension("demo.ext"); + + expect(activateCalls).toBe(2); + expect(host.getState("demo.ext")).toBe("active"); +}); + +test("getState returns undefined for an extension the host was never given", () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + const host = createExtensionHost({ extensions: [], api: fixtureApi(commands), log, sink }); + + expect(host.getState("mystery.ext")).toBeUndefined(); +}); diff --git a/packages/core/src/host/activation.ts b/packages/core/src/host/activation.ts new file mode 100644 index 0000000..b0e5ff2 --- /dev/null +++ b/packages/core/src/host/activation.ts @@ -0,0 +1,418 @@ +/** + * Extension activation and the extension context (Req 2.5, 2.6, design.md + * §4.2). Discovery/validation/registration (Task 1.11, `discovery.ts`/ + * `validate.ts`/`registration.ts`) get a manifest's declared contributions + * into the various registries without ever running `index.ts`; this module + * is what finally runs it, exactly once, when one of an extension's + * `activationEvents` fires: + * + * - `onStartup` — {@link ExtensionHost.activateStartupExtensions}, called by + * the CLI after the first frame (Task 1.15). + * - `onCommand:` — handled entirely on the command-registry side + * (`commands/registry.ts`'s `execute`): a lazy `CommandEntry` already + * carries the owning `extensionId` (set by `registerExtension` at + * registration time), so `execute` calls {@link ExtensionHost.activateExtension} + * directly with that ID and re-dispatches — this module does not need to + * inspect `activationEvents` strings for that path at all. + * - `onLanguage:` — {@link ExtensionHost.onLanguage}, shaped as a plain + * synchronous `(languageId: string) => void` so it can be handed straight + * to `DocumentManagerDeps.onLanguageActivation` (`buffer/documentManager.ts`, + * unchanged by this task) at the assembly layer. + * + * **No dynamic `import()` here.** `discovery.ts`'s `importManifestModule` is + * the one sanctioned dynamic-import call site in `core`, and it only ever + * loads a `manifest.ts`/`.js` — never an extension's `index.ts`. Loading an + * extension's actual implementation module is this module's job, but the + * *how* (a real `import()` of a resolved file path, wrapped for the + * compiled-binary case per design.md §4.4) is injected via + * {@link ExtensionRecord.loadModule} rather than performed here — production + * wiring of that closure is the later API-assembly task (1.13), which is + * also where {@link ExtensionRecord}s are actually built from + * `LoadedExtension`s (`registration.ts`). This keeps activation.ts testable + * with plain in-memory fixtures and keeps the dynamic-import surface of + * `core` exactly as small as `discovery.ts` already documents. + */ + +import type { Disposable, ExtensionContext, Manifest, Tecode } from "@tecode/api"; +import type { HostError, HostLog, StatusSink } from "./errors"; + +/** + * One extension the runtime can activate — the shape {@link createExtensionHost} + * consumes, built at the assembly layer (Task 1.13) from a `LoadedExtension` + * (`registration.ts`) plus a real module loader. Defined here (rather than + * in `registration.ts`/Task 1.11) because activation is the first task that + * needs a *loadable* extension, not just a *registered* one. + */ +export interface ExtensionRecord { + /** The validated `manifest.id` (Req 2.3) — matches `LoadedExtension.extensionId`. */ + id: string; + manifest: Manifest; + /** The extension's own directory, as an `ExtensionContext.extensionUri`. */ + extensionUri: string; + /** A per-extension directory for `ExtensionContext.storagePath`. */ + storagePath: string; + /** + * Loads the extension's implementation module. Production callers close + * over a real `import()` of the resolved `index.ts`/`.js` (design.md + * §4.4: built-ins as static imports wrapped in a closure, external + * extensions via `import(pathToFileURL(file).href)`); tests close over an + * in-memory fixture object instead. Called at most once per extension — + * {@link createExtensionHost}'s activation-exactly-once guarantee means a + * second `activateExtension` call for the same ID never re-invokes this. + */ + loadModule(): Promise; +} + +/** + * The shape an extension's implementation module is expected to have (Req + * 2.6). `@tecode/api` declares no runtime type for this — extension authors + * write plain functions, not something importable as a type — so it is + * defined here instead, structurally compatible with a module namespace + * object (`export function activate(ctx) {...}`). + */ +export interface ExtensionModule { + activate?(ctx: ExtensionContext): void | Promise; + deactivate?(): void | Promise; +} + +/** + * One extension's activation state (Req 2.5, 2.6): every extension starts + * `"registered"` (contributions are live, `index.ts` has not run); + * `"active"` once `activate(ctx)` has run without throwing (or the module + * exports no `activate` at all — see {@link ExtensionHost.activateExtension}'s + * TSDoc); `"failed"` if loading the module or running `activate(ctx)` threw + * or rejected. Both `"active"` and `"failed"` are terminal until an explicit + * {@link ExtensionHost.deactivateExtension} returns the extension to + * `"registered"` — activation events are is-a-no-op once past `"registered"`, + * which is what makes "each event activates exactly once" (Req 2.5) hold + * regardless of how many activation events subsequently fire for the same + * extension. + */ +export type ActivationState = "registered" | "active" | "failed"; + +/** Dependencies {@link createExtensionHost} needs. */ +export interface ExtensionHostDeps { + /** Every extension the host may be asked to activate — built once at + * startup from Task 1.11's `LoadExtensionsResult.loaded` (a later task's + * wiring; this module only consumes the array). */ + extensions: ExtensionRecord[]; + /** The live `tecode` API object handed to every extension's `activate(ctx)` + * (Req 1.4, 2.6) — identical for every extension, built once by the + * API-assembly task (1.13). */ + api: Tecode; + log: HostLog; + sink: StatusSink; +} + +/** + * The extension host's public surface (design.md §4.2). Deliberately not + * named `ExtensionHost` in a way that implies it owns discovery/registration + * too — those stay `discovery.ts`/`registration.ts`'s job; this is purely + * the activation lifecycle layered on top of an already-registered set of + * extensions. + */ +export interface ExtensionHost { + /** + * Activate one extension by ID, exactly once (Req 2.5). A no-op — resolves + * immediately, does nothing — when `id` is unknown or the extension is + * already `"active"`/`"failed"`. Concurrent calls for the same + * not-yet-activated extension (e.g. two documents of the same language + * opening back to back before the first activation settles) share one + * in-flight activation rather than running `activate(ctx)` twice. + * + * Never throws or rejects (Req 2.4-style never-throwing boundary, matching + * `registry.ts`/`registration.ts`): a failure loading the module or + * running `activate(ctx)` is caught, reported through `log`/`sink` as a + * {@link HostError}, and leaves the extension `"failed"` — its + * already-registered contributions (commands, views, ...) stay registered + * (Req 2.4's "continue starting up" spirit applied to one extension), and + * no other extension is affected. + */ + activateExtension(id: string): Promise; + /** + * Deactivate one active extension (Req 2.6): disposes its + * `ExtensionContext.subscriptions` in reverse push order (each `dispose()` + * individually guarded — one throwing disposable does not stop the rest), + * then calls its module's `deactivate()` if exported (also guarded). + * A no-op for an extension that is not currently `"active"` + * (idempotent — calling this twice in a row only disposes once). + * + * After deactivation the extension's state returns to `"registered"` + * rather than some fourth "deactivated" state — deliberately, so that a + * subsequent activation event (e.g. after `extensions.reload`-style + * re-registration in a future task) can activate it again. Never throws. + */ + deactivateExtension(id: string): Promise; + /** {@link deactivateExtension} every currently-`"active"` extension. + * Idempotent — a second call finds nothing left to deactivate. Never + * throws. */ + disposeAll(): Promise; + /** Activate every extension whose `manifest.activationEvents` includes + * `"onStartup"` (Req 2.5). Owns no render loop or timing of its own — the + * CLI decides *when* to call this (after the first frame, Task 1.15). + * Never throws. */ + activateStartupExtensions(): Promise; + /** + * Activate every extension whose `manifest.activationEvents` includes + * `` `onLanguage:${languageId}` `` (Req 2.5). Synchronous and + * void-returning by design: `DocumentManagerDeps.onLanguageActivation` + * (`buffer/documentManager.ts`, unchanged by this task) is exactly this + * shape, guards the call in its own try/catch, and does not await it — + * this function starts activation and returns immediately, relying on + * {@link activateExtension}'s own never-rejecting contract so nothing here + * produces an unhandled rejection. + */ + onLanguage(languageId: string): void; + /** Read-only lookup of one extension's current {@link ActivationState}; + * `undefined` for an unknown ID. Exists mainly for tests — the host's own + * decisions never need a caller to branch on this first. */ + getState(id: string): ActivationState | undefined; +} + +/** Per-extension runtime bookkeeping — kept separate from {@link ExtensionRecord} + * (the caller-supplied, immutable description) since this is what activation + * actually mutates. */ +interface ExtensionRuntime { + state: ActivationState; + ctx?: ExtensionContext; + module?: ExtensionModule; +} + +/** Render a caught `unknown` value as a message string without risking a + * second throw (matches `discovery.ts`'s/`registration.ts`'s/`registry.ts`'s + * `describeError`). */ +function describeError(err: unknown): string { + try { + if (err instanceof Error) return err.message; + return String(err); + } catch { + return "Unknown error"; + } +} + +/** + * Pull `activate`/`deactivate` out of a loaded extension module. Anything + * else on the module (default export, other named exports) is ignored — + * extensions are expected to export these two functions by name, the + * VS-Code-familiar convention (unlike `manifest.ts`'s `export default` + * convention, which is documented in `discovery.ts`). + */ +function asExtensionModule(mod: unknown): ExtensionModule { + if (!mod || typeof mod !== "object") return {}; + const record = mod as Record; + const activate = typeof record.activate === "function" ? (record.activate as ExtensionModule["activate"]) : undefined; + const deactivate = + typeof record.deactivate === "function" ? (record.deactivate as ExtensionModule["deactivate"]) : undefined; + return { activate, deactivate }; +} + +/** + * Build the extension activation host (Req 2.5, 2.6, design.md §4.2). + * + * **Wiring `onCommand:` into the command registry**: this host does + * *not* take a `CommandRegistry` dependency. The data flows the other way — + * `commands/registry.ts`'s `execute()` needs *this host's* + * {@link ExtensionHost.activateExtension}, so the simplest ordering (no + * setter, no mutable closure box) is to build the host first and pass its + * `activateExtension` straight into `createCommandRegistry`'s + * `activateExtension` dependency afterward: + * + * ```ts + * const host = createExtensionHost({ extensions, api, log, sink }); + * const commands = createCommandRegistry({ log, sink, activateExtension: host.activateExtension }); + * ``` + * + * (A registry built *before* the host — e.g. because registration.ts needs + * it earlier at startup — still works with this same host unchanged: build + * the registry without `activateExtension` first, build the host, then + * assign `commands` a way to reach `host.activateExtension` — a setter on + * `CommandRegistry` or a mutable closure box the deps function reads from + * would both work equally well here; this codebase's actual startup order + * (a later task) hasn't been settled, so that variant is deliberately left + * for whichever assembly task needs it.) + */ +export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { + const { api, log, sink } = deps; + + const records = new Map(); + const runtimes = new Map(); + for (const record of deps.extensions) { + records.set(record.id, record); + runtimes.set(record.id, { state: "registered" }); + } + + /** In-flight activation promises, keyed by extension ID — collapses + * concurrent {@link activateExtension} calls for the same not-yet-active + * extension into one activation (see {@link ExtensionHost.activateExtension}'s + * TSDoc). */ + const inFlight = new Map>(); + + function logSafely(level: "error" | "warning", err: HostError): void { + try { + log.append(level, err); + } catch { + // Swallowed: reporting a reporting failure has nowhere left to go. + } + } + + function notifySafely(err: HostError): void { + try { + sink.error(err); + } catch { + // Swallowed — see logSafely. + } + } + + /** Dispose `ctx.subscriptions` in reverse push order, one guarded + * `dispose()` at a time (Req 2.6) — a throwing disposable is logged and + * does not stop the rest. Clears the array afterward so a second call + * against the same `ctx` (defensive; callers are expected to gate on + * state) disposes nothing again. */ + function disposeSubscriptions(id: string, ctx: ExtensionContext): void { + const subscriptions = ctx.subscriptions; + for (let i = subscriptions.length - 1; i >= 0; i--) { + const disposable: Disposable | undefined = subscriptions[i]; + try { + disposable?.dispose(); + } catch (cause) { + logSafely("error", { + extensionId: id, + message: `Extension "${id}" subscription dispose() threw: ${describeError(cause)}`, + }); + } + } + subscriptions.length = 0; + } + + function markFailed(id: string, runtime: ExtensionRuntime, cause: unknown): void { + // A partially-set-up extension may have pushed subscriptions before its + // activate() threw/rejected — dispose those now rather than leaking them + // forever (deactivateExtension only ever acts on "active" extensions). + if (runtime.ctx) disposeSubscriptions(id, runtime.ctx); + runtime.state = "failed"; + runtime.ctx = undefined; + runtime.module = undefined; + const err: HostError = { + extensionId: id, + message: `Extension "${id}" failed to activate: ${describeError(cause)}`, + }; + logSafely("error", err); + notifySafely(err); + } + + async function performActivation( + id: string, + record: ExtensionRecord, + runtime: ExtensionRuntime, + ): Promise { + let loaded: unknown; + try { + loaded = await record.loadModule(); + } catch (cause) { + markFailed(id, runtime, cause); + return; + } + + const extensionModule = asExtensionModule(loaded); + const ctx: ExtensionContext = { + api, + extensionUri: record.extensionUri, + subscriptions: [], + storagePath: record.storagePath, + }; + // Visible to markFailed immediately, so subscriptions pushed before a + // throw/rejection below are still reachable for disposal. + runtime.ctx = ctx; + + try { + // A missing `activate` export is not an error (Req 2.6 says "call its + // exported activate(ctx)" — an extension that exports none has simply + // finished its (empty) activation work): the extension becomes + // "active" with nothing to run, which also makes its (possibly + // exported) `deactivate()` reachable on shutdown. + if (extensionModule.activate) { + await extensionModule.activate(ctx); + } + runtime.state = "active"; + runtime.module = extensionModule; + } catch (cause) { + markFailed(id, runtime, cause); + } + } + + function activateExtension(id: string): Promise { + const record = records.get(id); + const runtime = runtimes.get(id); + if (!record || !runtime || runtime.state !== "registered") { + return Promise.resolve(); + } + + const existing = inFlight.get(id); + if (existing) return existing; + + const promise = performActivation(id, record, runtime).finally(() => { + inFlight.delete(id); + }); + inFlight.set(id, promise); + return promise; + } + + async function deactivateExtension(id: string): Promise { + const runtime = runtimes.get(id); + if (!runtime || runtime.state !== "active") return; + + const { ctx, module } = runtime; + if (ctx) disposeSubscriptions(id, ctx); + if (module?.deactivate) { + try { + await module.deactivate(); + } catch (cause) { + logSafely("error", { + extensionId: id, + message: `Extension "${id}" deactivate() threw: ${describeError(cause)}`, + }); + } + } + runtime.state = "registered"; + runtime.ctx = undefined; + runtime.module = undefined; + } + + async function disposeAll(): Promise { + for (const id of records.keys()) { + await deactivateExtension(id); + } + } + + async function activateStartupExtensions(): Promise { + const startupIds = Array.from(records.values()) + .filter((record) => record.manifest.activationEvents.includes("onStartup")) + .map((record) => record.id); + await Promise.all(startupIds.map((id) => activateExtension(id))); + } + + function onLanguage(languageId: string): void { + const event = `onLanguage:${languageId}` as const; + for (const record of records.values()) { + if (record.manifest.activationEvents.includes(event)) { + // Fire-and-forget: activateExtension never rejects (every failure + // path inside performActivation is caught and reported), so this + // cannot produce an unhandled rejection. + void activateExtension(record.id); + } + } + } + + function getState(id: string): ActivationState | undefined { + return runtimes.get(id)?.state; + } + + return { + activateExtension, + deactivateExtension, + disposeAll, + activateStartupExtensions, + onLanguage, + getState, + }; +} diff --git a/packages/core/src/host/index.ts b/packages/core/src/host/index.ts index 7efbea0..f30ead2 100644 --- a/packages/core/src/host/index.ts +++ b/packages/core/src/host/index.ts @@ -1,8 +1,8 @@ -// Extension host (discovery, manifest validation, registration) — activation -// (design.md §4.2) lands in Task 1.12. This module exposes the shared +// Extension host: discovery, manifest validation, registration, and +// activation (design.md §4.1, §4.2). This module exposes the shared // error/log infrastructure (§4.1) that both host loading and the command -// registry (§5) depend on, plus discovery/validation/registration -// themselves. +// registry (§5) depend on, plus discovery/validation/registration/ +// activation themselves. export { createHostLog, createNoopStatusSink, @@ -52,8 +52,16 @@ export { type SkippedExtension, } from "./registration"; +export { + createExtensionHost, + type ActivationState, + type ExtensionHost, + type ExtensionHostDeps, + type ExtensionModule, + type ExtensionRecord, +} from "./activation"; + /** Kept for source compatibility with `core/index.test.ts`'s existing - * placeholder assertion; discovery/validation/registration above are the - * real Task 1.11 surface. Activation (design.md §4.2) is still Task - * 1.12 — remove this once that task's own exports make it redundant. */ + * placeholder assertion; discovery/validation/registration/activation above + * are the real Task 1.11/1.12 surface. */ export const HOST_PLACEHOLDER = true; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9d15e38..76b813a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,6 +1,9 @@ // Placeholder entry point for @tecode/core. Real wiring lands in later tasks. export { checkApiVersionCompatibility, + createExtensionHost, + createHostLog, + createNoopStatusSink, discover, getUserExtensionsDir, getWorkspaceExtensionsDir, @@ -8,12 +11,21 @@ export { loadExtensions, registerExtension, validateManifest, + type ActivationState, type ApiVersionCompatibility, type ConfigRegistrar, type DiscoveredExtension, type DiscoveryDeps, type DiscoveryFs, + type ExtensionHost, + type ExtensionHostDeps, + type ExtensionModule, + type ExtensionRecord, type ExtensionSource, + type HostError, + type HostLog, + type HostLogEntry, + type HostLogLevel, type LoadedExtension, type LoadExtensionsDeps, type LoadExtensionsResult, @@ -24,6 +36,7 @@ export { type RegisterExtensionDeps, type RegisterExtensionResult, type SkippedExtension, + type StatusSink, } from "./host/index"; export { createCommandRegistry, From b143accf31afd2f00bd1cb313f113facb668fe2d Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:15:46 +0000 Subject: [PATCH 2/5] Address review: settle in-flight activations, guard lazy re-entrancy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - disposeAll now awaits all in-flight activations before deactivating, so an extension mid-activation at shutdown (from a fire-and-forget trigger like onLanguage) still gets its subscriptions disposed. - CommandEntry gains an 'activating' marker: an extension whose activate(ctx) executes its own still-lazy command no longer deadlocks on its own in-flight activation promise — the recursive call falls through to the existing not-activated error path, and the outer call re-dispatches to the real handler once activation settles. - The onLanguage test joins the shared in-flight activation via activateExtension instead of counting microtask ticks. - Regression tests for the shutdown race and the self-re-entrant command. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- packages/core/src/commands/registry.ts | 17 ++++- packages/core/src/host/activation.test.ts | 83 +++++++++++++++++++++-- packages/core/src/host/activation.ts | 6 ++ 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/packages/core/src/commands/registry.ts b/packages/core/src/commands/registry.ts index 1210c61..d8f7230 100644 --- a/packages/core/src/commands/registry.ts +++ b/packages/core/src/commands/registry.ts @@ -33,6 +33,12 @@ interface CommandEntry { meta: CommandMeta; extensionId?: string; lazy: boolean; + /** True while `execute()` is awaiting this entry's owning extension's + * activation. Guards re-entrancy: an extension whose `activate(ctx)` + * executes its own still-lazy command would otherwise `await` its own + * in-flight activation promise and deadlock — with the marker set, the + * recursive call falls through to the not-activated error path instead. */ + activating?: boolean; } /** Options for {@link CommandRegistry.registerLazy}. */ @@ -196,10 +202,13 @@ export function createCommandRegistry(deps: CommandRegistryDeps): CommandRegistr async function execute(id: string, ...args: unknown[]): Promise { let entry = commands.get(id); - if (entry && !entry.handler && entry.extensionId && activateExtension) { + if (entry && !entry.handler && entry.extensionId && activateExtension && !entry.activating) { // Lazy, not-yet-activated command (design.md §4.1, §4.2) — activate // its owning extension, then re-look-up: activation is expected to // replace this entry with a real handler via register() (Task 1.12). + // `activating` (see CommandEntry) keeps a recursive execute() from + // inside that same activation from deadlocking on its own promise. + entry.activating = true; try { await activateExtension(entry.extensionId); } catch (cause) { @@ -210,6 +219,12 @@ export function createCommandRegistry(deps: CommandRegistryDeps): CommandRegistr extensionId: entry.extensionId, message: `activateExtension("${entry.extensionId}") threw: ${describeError(cause)}`, }); + } finally { + // Clear on the ORIGINAL entry (activation may have replaced it in + // the map): once activation has settled, future execute() calls may + // legitimately retry the hook (it no-ops fast for active/failed + // extensions). + entry.activating = false; } entry = commands.get(id); } diff --git a/packages/core/src/host/activation.test.ts b/packages/core/src/host/activation.test.ts index a7f38b9..6ef013c 100644 --- a/packages/core/src/host/activation.test.ts +++ b/packages/core/src/host/activation.test.ts @@ -165,9 +165,10 @@ test("onLanguage activates every extension declaring that language, exactly once host.onLanguage("typescript"); // onLanguage is fire-and-forget (synchronous, matches DocumentManagerDeps' - // onLanguageActivation shape) — give the in-flight activation a tick to settle. - await Promise.resolve(); - await Promise.resolve(); + // onLanguageActivation shape) — join the same in-flight activation via + // activateExtension instead of guessing a microtask count, so the wait + // stays deterministic however many awaits performActivation grows. + await host.activateExtension("lang.ts"); expect(tsActivations).toBe(1); expect(jsActivations).toBe(0); @@ -175,7 +176,7 @@ test("onLanguage activates every extension declaring that language, exactly once expect(host.getState("lang.js")).toBe("registered"); host.onLanguage("typescript"); - await Promise.resolve(); + await host.activateExtension("lang.ts"); expect(tsActivations).toBe(1); }); @@ -287,6 +288,80 @@ test("disposeAll deactivates every active extension and is idempotent", async () expect(order.sort()).toEqual(["a", "b"]); }); +test("disposeAll settles an in-flight activation first, so its subscriptions are still disposed", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let releaseActivate: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseActivate = resolve; + }); + const disposed: string[] = []; + const slow = fixtureRecord( + "lang.slow", + { activationEvents: ["onLanguage:slow"] }, + { + async activate(ctx: ExtensionContext) { + ctx.subscriptions.push({ + dispose() { + disposed.push("slow-sub"); + }, + }); + await gate; + }, + }, + ); + const host = createExtensionHost({ extensions: [slow], api: fixtureApi(commands), log, sink }); + + // Fire-and-forget start, then shut down while activation is still pending. + host.onLanguage("slow"); + const disposal = host.disposeAll(); + releaseActivate(); + await disposal; + + expect(disposed).toEqual(["slow-sub"]); + expect(host.getState("lang.slow")).toBe("registered"); +}); + +test("an extension executing its own lazy command during activate() does not deadlock", async () => { + const log = createHostLog(); + const { errors, sink } = createRecordingSink(); + // The hook closes over `host`, declared below — safe because the hook only + // runs from execute(), long after the const initializes (the standard + // registry-before-host wiring order documented in registry.ts). + const commands = createCommandRegistry({ + log, + sink, + activateExtension: (extensionId) => host.activateExtension(extensionId), + }); + let innerResult: unknown = "not-run"; + const selfCalling = fixtureRecord("self.ext", { activationEvents: ["onCommand:self.run"] }, { + async activate(ctx: ExtensionContext) { + // Executes its own still-lazy command BEFORE registering the real + // handler — without registry.ts's `activating` re-entrancy guard this + // would await its own in-flight activation promise forever. + innerResult = await ctx.api.commands.execute("self.run"); + ctx.api.commands.register("self.run", () => "real"); + }, + }); + const host = createExtensionHost({ + extensions: [selfCalling], + api: fixtureApi(commands), + log, + sink, + }); + commands.registerLazy("self.run", { extensionId: "self.ext" }); + + const result = await commands.execute("self.run"); + + // The outer call re-dispatches to the real handler after activation; the + // recursive inner call fell through to the not-activated error path. + expect(result).toBe("real"); + expect(innerResult).toBeUndefined(); + expect(errors.some((e) => e.message.includes("self.run"))).toBe(true); + expect(host.getState("self.ext")).toBe("active"); +}); + // --- failure isolation -------------------------------------------------------- test("a throwing activate marks only that extension failed, reports a HostError, and leaves others unaffected", async () => { diff --git a/packages/core/src/host/activation.ts b/packages/core/src/host/activation.ts index b0e5ff2..9db5ae3 100644 --- a/packages/core/src/host/activation.ts +++ b/packages/core/src/host/activation.ts @@ -379,6 +379,12 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { } async function disposeAll(): Promise { + // Settle in-flight activations first: a fire-and-forget trigger (e.g. + // onLanguage) may still be mid-activation, and deactivateExtension + // skips anything not yet "active" — without this, such an extension + // would finish activating after shutdown with its subscriptions never + // disposed. + await Promise.all(Array.from(inFlight.values())); for (const id of records.keys()) { await deactivateExtension(id); } From 1064fe49fdabd0dd316efa6a8c7c77aa4e932dee Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 20:36:11 +0000 Subject: [PATCH 3/5] Harden shutdown and concurrent lazy-command activation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - disposeAll is now a one-way shutdown: a latch blocks any activation starting after disposal begins (a late onLanguage fire no longer leaves an extension active on a disposed host), and in-flight activations are settled before deactivation as before. - Re-entrancy detection moves from the command registry into the host via an AsyncLocalStorage activation context: only a call from inside the extension's own activate(ctx) resolves immediately (avoiding the self-deadlock), while independent concurrent execute() callers of the same lazy command now correctly await the shared in-flight activation and re-dispatch — the previous per-entry 'activating' marker wrongly failed those callers. Registry marker removed. - Regression tests: post-shutdown activation refusal, and two racing execute() calls of one lazy command both succeeding with exactly one activation. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- packages/core/src/commands/registry.ts | 23 +++---- packages/core/src/host/activation.test.ts | 77 +++++++++++++++++++++++ packages/core/src/host/activation.ts | 36 +++++++++-- 3 files changed, 116 insertions(+), 20 deletions(-) diff --git a/packages/core/src/commands/registry.ts b/packages/core/src/commands/registry.ts index d8f7230..9387174 100644 --- a/packages/core/src/commands/registry.ts +++ b/packages/core/src/commands/registry.ts @@ -33,12 +33,6 @@ interface CommandEntry { meta: CommandMeta; extensionId?: string; lazy: boolean; - /** True while `execute()` is awaiting this entry's owning extension's - * activation. Guards re-entrancy: an extension whose `activate(ctx)` - * executes its own still-lazy command would otherwise `await` its own - * in-flight activation promise and deadlock — with the marker set, the - * recursive call falls through to the not-activated error path instead. */ - activating?: boolean; } /** Options for {@link CommandRegistry.registerLazy}. */ @@ -202,13 +196,16 @@ export function createCommandRegistry(deps: CommandRegistryDeps): CommandRegistr async function execute(id: string, ...args: unknown[]): Promise { let entry = commands.get(id); - if (entry && !entry.handler && entry.extensionId && activateExtension && !entry.activating) { + if (entry && !entry.handler && entry.extensionId && activateExtension) { // Lazy, not-yet-activated command (design.md §4.1, §4.2) — activate // its owning extension, then re-look-up: activation is expected to // replace this entry with a real handler via register() (Task 1.12). - // `activating` (see CommandEntry) keeps a recursive execute() from - // inside that same activation from deadlocking on its own promise. - entry.activating = true; + // Concurrent execute() calls all await here and share the host's + // in-flight activation; the one case that must NOT wait — the + // extension executing its own still-lazy command from inside its own + // activate(ctx), which would deadlock on its own activation promise — + // is detected host-side (host/activation.ts's activation context) and + // resolves immediately, landing on the not-activated error path below. try { await activateExtension(entry.extensionId); } catch (cause) { @@ -219,12 +216,6 @@ export function createCommandRegistry(deps: CommandRegistryDeps): CommandRegistr extensionId: entry.extensionId, message: `activateExtension("${entry.extensionId}") threw: ${describeError(cause)}`, }); - } finally { - // Clear on the ORIGINAL entry (activation may have replaced it in - // the map): once activation has settled, future execute() calls may - // legitimately retry the hook (it no-ops fast for active/failed - // extensions). - entry.activating = false; } entry = commands.get(id); } diff --git a/packages/core/src/host/activation.test.ts b/packages/core/src/host/activation.test.ts index 6ef013c..8a1729a 100644 --- a/packages/core/src/host/activation.test.ts +++ b/packages/core/src/host/activation.test.ts @@ -323,6 +323,83 @@ test("disposeAll settles an in-flight activation first, so its subscriptions are expect(host.getState("lang.slow")).toBe("registered"); }); +test("activateExtension after disposeAll started is a no-op, so nothing activates on a shut-down host", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let releaseActivate: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseActivate = resolve; + }); + let lateActivations = 0; + const slow = fixtureRecord( + "lang.slow", + { activationEvents: ["onLanguage:slow"] }, + { + async activate() { + await gate; + }, + }, + ); + const late = fixtureRecord("lang.late", { activationEvents: ["onLanguage:late"] }, { + activate() { + lateActivations += 1; + }, + }); + const host = createExtensionHost({ + extensions: [slow, late], + api: fixtureApi(commands), + log, + sink, + }); + + host.onLanguage("slow"); + const disposal = host.disposeAll(); + // Fired AFTER shutdown began: must not start a fresh activation that + // would finish on the disposed host. + host.onLanguage("late"); + releaseActivate(); + await disposal; + await host.activateExtension("lang.late"); + + expect(lateActivations).toBe(0); + expect(host.getState("lang.late")).toBe("registered"); +}); + +test("a concurrent external execute() of the same lazy command awaits the shared activation and succeeds", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ + log, + sink, + activateExtension: (extensionId) => host.activateExtension(extensionId), + }); + let releaseActivate: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseActivate = resolve; + }); + let activations = 0; + const slowExt = fixtureRecord("slow.ext", { activationEvents: ["onCommand:slow.run"] }, { + async activate(ctx: ExtensionContext) { + activations += 1; + await gate; + ctx.api.commands.register("slow.run", () => "ready"); + }, + }); + const host = createExtensionHost({ extensions: [slowExt], api: fixtureApi(commands), log, sink }); + commands.registerLazy("slow.run", { extensionId: "slow.ext" }); + + // Two independent callers race the same lazy command while its extension + // is still activating — both must join the one activation and succeed. + const first = commands.execute("slow.run"); + const second = commands.execute("slow.run"); + releaseActivate(); + + expect(await first).toBe("ready"); + expect(await second).toBe("ready"); + expect(activations).toBe(1); +}); + test("an extension executing its own lazy command during activate() does not deadlock", async () => { const log = createHostLog(); const { errors, sink } = createRecordingSink(); diff --git a/packages/core/src/host/activation.ts b/packages/core/src/host/activation.ts index 9db5ae3..a78c645 100644 --- a/packages/core/src/host/activation.ts +++ b/packages/core/src/host/activation.ts @@ -33,6 +33,7 @@ * `core` exactly as small as `discovery.ts` already documents. */ +import { AsyncLocalStorage } from "node:async_hooks"; import type { Disposable, ExtensionContext, Manifest, Tecode } from "@tecode/api"; import type { HostError, HostLog, StatusSink } from "./errors"; @@ -246,6 +247,17 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { * extension into one activation (see {@link ExtensionHost.activateExtension}'s * TSDoc). */ const inFlight = new Map>(); + // The extension ID whose activate(ctx) is executing on the current async + // path. Lets activateExtension tell SELF-re-entrancy (an extension + // triggering its own activation from inside activate — awaiting the + // shared in-flight promise there would deadlock, since that promise + // cannot settle until activate returns) apart from an unrelated + // concurrent caller, who safely awaits the shared promise. + const activatingContext = new AsyncLocalStorage(); + // One-way shutdown latch (see disposeAll): once disposal has begun, new + // activations must not start, or they would finish after shutdown and + // leave an "active" extension with never-disposed subscriptions. + let shutDown = false; function logSafely(level: "error" | "warning", err: HostError): void { try { @@ -331,7 +343,12 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { // "active" with nothing to run, which also makes its (possibly // exported) `deactivate()` reachable on shutdown. if (extensionModule.activate) { - await extensionModule.activate(ctx); + // Run inside this extension's activation context so a re-entrant + // activateExtension(id) call from within activate(ctx) — e.g. the + // extension executing its own still-lazy command — resolves + // immediately instead of deadlocking on its own in-flight promise. + const activate = extensionModule.activate; + await activatingContext.run(id, () => Promise.resolve(activate(ctx))); } runtime.state = "active"; runtime.module = extensionModule; @@ -341,6 +358,15 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { } function activateExtension(id: string): Promise { + // After shutdown has begun, starting (or joining) an activation would + // let it complete on a disposed host — refuse quietly. + if (shutDown) return Promise.resolve(); + // Self-re-entrancy: this call originates from inside this very + // extension's activate(ctx). Returning the shared in-flight promise + // would deadlock (it can't settle until activate returns), so resolve + // immediately — the caller (e.g. a lazy command execute) then proceeds + // down its documented not-yet-activated path. + if (activatingContext.getStore() === id) return Promise.resolve(); const record = records.get(id); const runtime = runtimes.get(id); if (!record || !runtime || runtime.state !== "registered") { @@ -379,11 +405,13 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { } async function disposeAll(): Promise { - // Settle in-flight activations first: a fire-and-forget trigger (e.g. + // One-way shutdown: block NEW activations first (see activateExtension), + // then settle the in-flight ones — a fire-and-forget trigger (e.g. // onLanguage) may still be mid-activation, and deactivateExtension - // skips anything not yet "active" — without this, such an extension - // would finish activating after shutdown with its subscriptions never + // skips anything not yet "active". Without both steps, an extension + // could finish activating after shutdown with its subscriptions never // disposed. + shutDown = true; await Promise.all(Array.from(inFlight.values())); for (const id of records.keys()) { await deactivateExtension(id); From 4662ee521beed5801e2a4fe43918494529355ddd Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:15:21 +0000 Subject: [PATCH 4/5] Detect activation cycles, document the re-entrancy contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The activation context now carries the full set of extension IDs activating on the current async path instead of only the innermost one, so a mutual activation cycle (A's activate executing B's lazy command while B's activate executes A's) short-circuits like direct self-re-entry does instead of deadlocking both activations — and with them disposeAll. Regression test covers the A-to-B-to-A cycle. The activateExtension dep's TSDoc in the command registry now states this re-entrancy contract explicitly, since execute() keeps no re-entrancy state of its own. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- packages/core/src/commands/registry.ts | 8 +++++ packages/core/src/host/activation.test.ts | 39 +++++++++++++++++++++++ packages/core/src/host/activation.ts | 33 ++++++++++++------- 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/packages/core/src/commands/registry.ts b/packages/core/src/commands/registry.ts index 9387174..5b61721 100644 --- a/packages/core/src/commands/registry.ts +++ b/packages/core/src/commands/registry.ts @@ -64,6 +64,14 @@ export interface CommandRegistryDeps { * Documented to never throw/reject (matching `activateExtension`'s own * contract); `execute()` guards the call anyway so a misbehaving * implementation can't break its own never-throwing contract. + * + * Re-entrancy contract: the implementation must resolve immediately for + * a call re-entering an activation already in progress on the current + * async path — an extension executing its own still-lazy command from + * inside `activate(ctx)`, or a mutual activation cycle. `execute()` + * keeps no re-entrancy state of its own, so an implementation that + * hands back its own in-flight activation promise here deadlocks + * (`createExtensionHost` satisfies this via its activation context). */ activateExtension?: (extensionId: string) => Promise; } diff --git a/packages/core/src/host/activation.test.ts b/packages/core/src/host/activation.test.ts index 8a1729a..d0683cb 100644 --- a/packages/core/src/host/activation.test.ts +++ b/packages/core/src/host/activation.test.ts @@ -400,6 +400,45 @@ test("a concurrent external execute() of the same lazy command awaits the shared expect(activations).toBe(1); }); +test("a mutual activation cycle (A's activate runs B's command, B's activate runs A's) does not deadlock", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ + log, + sink, + activateExtension: (extensionId) => host.activateExtension(extensionId), + }); + const extA = fixtureRecord("cycle.a", { activationEvents: ["onCommand:a.run"] }, { + async activate(ctx: ExtensionContext) { + // Triggers B's activation mid-way through A's own. + await ctx.api.commands.execute("b.run"); + ctx.api.commands.register("a.run", () => "a-ready"); + }, + }); + const extB = fixtureRecord("cycle.b", { activationEvents: ["onCommand:b.run"] }, { + async activate(ctx: ExtensionContext) { + // Closes the cycle back to A, whose activation is still in flight + // above us — must short-circuit, not await it. + await ctx.api.commands.execute("a.run"); + ctx.api.commands.register("b.run", () => "b-ready"); + }, + }); + const host = createExtensionHost({ + extensions: [extA, extB], + api: fixtureApi(commands), + log, + sink, + }); + commands.registerLazy("a.run", { extensionId: "cycle.a" }); + commands.registerLazy("b.run", { extensionId: "cycle.b" }); + + const result = await commands.execute("a.run"); + + expect(result).toBe("a-ready"); + expect(host.getState("cycle.a")).toBe("active"); + expect(host.getState("cycle.b")).toBe("active"); +}); + test("an extension executing its own lazy command during activate() does not deadlock", async () => { const log = createHostLog(); const { errors, sink } = createRecordingSink(); diff --git a/packages/core/src/host/activation.ts b/packages/core/src/host/activation.ts index a78c645..171c524 100644 --- a/packages/core/src/host/activation.ts +++ b/packages/core/src/host/activation.ts @@ -247,13 +247,15 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { * extension into one activation (see {@link ExtensionHost.activateExtension}'s * TSDoc). */ const inFlight = new Map>(); - // The extension ID whose activate(ctx) is executing on the current async - // path. Lets activateExtension tell SELF-re-entrancy (an extension - // triggering its own activation from inside activate — awaiting the - // shared in-flight promise there would deadlock, since that promise - // cannot settle until activate returns) apart from an unrelated - // concurrent caller, who safely awaits the shared promise. - const activatingContext = new AsyncLocalStorage(); + // The SET of extension IDs whose activate(ctx) calls are executing on the + // current async path (a set, not a single ID: nested activations — A's + // activate triggering B's — must keep A visible, or an A→B→A cycle would + // evade detection and deadlock). Lets activateExtension tell re-entrancy + // into an activation already on this path (awaiting its shared in-flight + // promise would deadlock, since that promise cannot settle until the + // activate above us returns) apart from an unrelated concurrent caller, + // who safely awaits the shared promise. + const activatingContext = new AsyncLocalStorage>(); // One-way shutdown latch (see disposeAll): once disposal has begun, new // activations must not start, or they would finish after shutdown and // leave an "active" extension with never-disposed subscriptions. @@ -348,7 +350,12 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { // extension executing its own still-lazy command — resolves // immediately instead of deadlocking on its own in-flight promise. const activate = extensionModule.activate; - await activatingContext.run(id, () => Promise.resolve(activate(ctx))); + // Union with any outer activation chain so a nested activation + // (A's activate triggering B's) keeps every in-progress ID visible + // for cycle detection above. + const chain = new Set(activatingContext.getStore() ?? []); + chain.add(id); + await activatingContext.run(chain, () => Promise.resolve(activate(ctx))); } runtime.state = "active"; runtime.module = extensionModule; @@ -361,12 +368,14 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { // After shutdown has begun, starting (or joining) an activation would // let it complete on a disposed host — refuse quietly. if (shutDown) return Promise.resolve(); - // Self-re-entrancy: this call originates from inside this very - // extension's activate(ctx). Returning the shared in-flight promise - // would deadlock (it can't settle until activate returns), so resolve + // Re-entrancy into an activation already running on this async path — + // directly (an extension executing its own lazy command from inside + // activate) or through a cycle (A's activate triggers B, whose activate + // triggers A). Returning the shared in-flight promise would deadlock + // (it can't settle until the activate above us returns), so resolve // immediately — the caller (e.g. a lazy command execute) then proceeds // down its documented not-yet-activated path. - if (activatingContext.getStore() === id) return Promise.resolve(); + if (activatingContext.getStore()?.has(id)) return Promise.resolve(); const record = records.get(id); const runtime = runtimes.get(id); if (!record || !runtime || runtime.state !== "registered") { From a39f3e51edaa01608a82b76309e169be87d0aad6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:37:18 +0000 Subject: [PATCH 5/5] Serialize per-extension deactivation, chain reactivation after teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrent deactivateExtension calls now share one teardown promise per extension (state stayed 'active' until deactivate() settled, so a second caller would run module.deactivate() again), and activateExtension chains after a pending teardown instead of racing it — previously a straggling teardown could wipe the fresh activation's ctx and module, losing its subscriptions. Regression tests: two concurrent deactivations run deactivate() once; a reactivation during a slow teardown waits, ends active with a fresh context, and its subscriptions dispose cleanly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WELSsojQQL1cTAR5iUUsTK --- packages/core/src/host/activation.test.ts | 71 +++++++++++++++++++++++ packages/core/src/host/activation.ts | 32 ++++++++-- 2 files changed, 99 insertions(+), 4 deletions(-) diff --git a/packages/core/src/host/activation.test.ts b/packages/core/src/host/activation.test.ts index d0683cb..1d80d7c 100644 --- a/packages/core/src/host/activation.test.ts +++ b/packages/core/src/host/activation.test.ts @@ -323,6 +323,77 @@ test("disposeAll settles an in-flight activation first, so its subscriptions are expect(host.getState("lang.slow")).toBe("registered"); }); +test("concurrent deactivations share one teardown — deactivate() runs exactly once", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let releaseDeactivate: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseDeactivate = resolve; + }); + let deactivations = 0; + const ext = fixtureRecord("slow.teardown", {}, { + activate() {}, + async deactivate() { + deactivations += 1; + await gate; + }, + }); + const host = createExtensionHost({ extensions: [ext], api: fixtureApi(commands), log, sink }); + await host.activateExtension("slow.teardown"); + + const first = host.deactivateExtension("slow.teardown"); + const second = host.deactivateExtension("slow.teardown"); + releaseDeactivate(); + await Promise.all([first, second]); + + expect(deactivations).toBe(1); + expect(host.getState("slow.teardown")).toBe("registered"); +}); + +test("a reactivation racing a slow teardown waits for it, ending active with a fresh context", async () => { + const log = createHostLog(); + const { sink } = createRecordingSink(); + const commands = createCommandRegistry({ log, sink }); + let releaseDeactivate: () => void = () => {}; + const gate = new Promise((resolve) => { + releaseDeactivate = resolve; + }); + let activations = 0; + const subscriptionLog: string[] = []; + const ext = fixtureRecord("rebounce.ext", {}, { + activate(ctx: ExtensionContext) { + activations += 1; + const generation = activations; + ctx.subscriptions.push({ + dispose() { + subscriptionLog.push(`disposed-${generation}`); + }, + }); + }, + async deactivate() { + await gate; + }, + }); + const host = createExtensionHost({ extensions: [ext], api: fixtureApi(commands), log, sink }); + await host.activateExtension("rebounce.ext"); + + const teardown = host.deactivateExtension("rebounce.ext"); + // Fired while teardown is still awaiting deactivate(): must chain after + // it, not race it — a straggling teardown must never wipe the fresh ctx. + const reactivation = host.activateExtension("rebounce.ext"); + releaseDeactivate(); + await Promise.all([teardown, reactivation]); + + expect(activations).toBe(2); + expect(host.getState("rebounce.ext")).toBe("active"); + // Generation 1's subscription was disposed by the teardown; generation + // 2's is still live and gets disposed by a final clean deactivation. + expect(subscriptionLog).toEqual(["disposed-1"]); + await host.deactivateExtension("rebounce.ext"); + expect(subscriptionLog).toEqual(["disposed-1", "disposed-2"]); +}); + test("activateExtension after disposeAll started is a no-op, so nothing activates on a shut-down host", async () => { const log = createHostLog(); const { sink } = createRecordingSink(); diff --git a/packages/core/src/host/activation.ts b/packages/core/src/host/activation.ts index 171c524..9e34c68 100644 --- a/packages/core/src/host/activation.ts +++ b/packages/core/src/host/activation.ts @@ -247,6 +247,10 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { * extension into one activation (see {@link ExtensionHost.activateExtension}'s * TSDoc). */ const inFlight = new Map>(); + // In-progress deactivations, keyed like inFlight — see deactivateExtension + // for why teardown must be serialized per extension, and activateExtension + // for why a reactivation waits for a pending teardown to finish first. + const tearingDown = new Map>(); // The SET of extension IDs whose activate(ctx) calls are executing on the // current async path (a set, not a single ID: nested activations — A's // activate triggering B's — must keep A visible, or an A→B→A cycle would @@ -376,6 +380,11 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { // immediately — the caller (e.g. a lazy command execute) then proceeds // down its documented not-yet-activated path. if (activatingContext.getStore()?.has(id)) return Promise.resolve(); + // A teardown of this extension is still in progress: reactivation must + // start from a fully torn-down state, or the finishing teardown would + // wipe the new activation's ctx/module out from under it. + const teardown = tearingDown.get(id); + if (teardown) return teardown.then(() => activateExtension(id)); const record = records.get(id); const runtime = runtimes.get(id); if (!record || !runtime || runtime.state !== "registered") { @@ -392,10 +401,7 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { return promise; } - async function deactivateExtension(id: string): Promise { - const runtime = runtimes.get(id); - if (!runtime || runtime.state !== "active") return; - + async function performDeactivation(id: string, runtime: ExtensionRuntime): Promise { const { ctx, module } = runtime; if (ctx) disposeSubscriptions(id, ctx); if (module?.deactivate) { @@ -413,6 +419,24 @@ export function createExtensionHost(deps: ExtensionHostDeps): ExtensionHost { runtime.module = undefined; } + function deactivateExtension(id: string): Promise { + // Serialize per extension: state stays "active" until deactivate() + // settles, so without this a second concurrent call would run + // module.deactivate() a second time — and, if a reactivation slipped in + // after the first teardown finished, the straggler would then wipe the + // NEW activation's ctx/module. Concurrent callers share one teardown. + const existing = tearingDown.get(id); + if (existing) return existing; + const runtime = runtimes.get(id); + if (!runtime || runtime.state !== "active") return Promise.resolve(); + + const promise = performDeactivation(id, runtime).finally(() => { + tearingDown.delete(id); + }); + tearingDown.set(id, promise); + return promise; + } + async function disposeAll(): Promise { // One-way shutdown: block NEW activations first (see activateExtension), // then settle the in-flight ones — a fire-and-forget trigger (e.g.