diff --git a/src/host/attribution.ts b/src/host/attribution.ts index 3a396a8..7f938d5 100644 --- a/src/host/attribution.ts +++ b/src/host/attribution.ts @@ -16,7 +16,10 @@ * wraps that instance's `register` (once — an earlier wrapper of a previous * hook incarnation is peeled back to the original, so a plugin reload * re-wraps without stacking) to capture the reader at registration time - * into a live map. + * into a live map. Every wrapper is undone when the plugin unloads: the + * original `register` goes back on the instance, unless a newer hook + * incarnation re-wrapped it first (that incarnation's own cleanup then + * owns the restore). * - When the reader slot is missing, root-named, or this plugin's own (e.g. * LOCAL-LINK plugins — dev installs via `dsh plugin add ` or * npm/pnpm link — whose anonymous entrypoints make cordis fall back to the @@ -155,8 +158,8 @@ interface ToolServiceLike { /** * Install the runtime-attribution hook on a cordis app context. The hook - * rides the calling fiber's lifetime (`ctx.on` / `ctx.get`), so it is - * disposed with the plugin. + * rides the calling fiber's lifetime (`ctx.on`, and an effect that restores + * every patched `register`), so it is disposed with the plugin. * @param ctx - the context the dsh-context plugin runs in; its fiber name is * excluded from attributions. */ @@ -165,6 +168,9 @@ export function createToolAttribution(ctx: Context): ToolAttribution { const wrapped = new WeakSet() const self = ctx.fiber.name let lastReader: Context | undefined + // Restore closures for every instance this incarnation patched, run by the + // unload effect below. + const patched: (() => void)[] = [] const wrapInstance = (tools: unknown) => { if (!tools || typeof tools !== 'object' || wrapped.has(tools)) return @@ -200,6 +206,9 @@ export function createToolAttribution(ctx: Context): ToolAttribution { } ;(wrappedRegister as { attributedOriginal?: unknown }).attributedOriginal = original instance.register = wrappedRegister + patched.push(() => { + if (instance.register === wrappedRegister) instance.register = original as typeof instance.register + }) } ctx.on('internal/get', (reader, name, _error, next) => { @@ -210,6 +219,15 @@ export function createToolAttribution(ctx: Context): ToolAttribution { return tools }) + // Scope symmetry for the register patch: unloading peels every wrapper this + // incarnation installed back to the true original, so other plugins are + // never left running through a dead hook. A closure that finds a newer + // incarnation's wrapper installed does nothing — that incarnation's own + // effect then owns the restore. + ctx.effect(() => () => { + for (const restore of patched.splice(0)) restore() + }, 'tools.register attribution') + // An instance provided before this plugin started is still wrapped so later // registrations on it are captured. The tools it already holds were // registered before this hook could observe them (boot-time third-party diff --git a/tests/host/attribution.spec.ts b/tests/host/attribution.spec.ts index c5ee062..0402fb8 100644 --- a/tests/host/attribution.spec.ts +++ b/tests/host/attribution.spec.ts @@ -206,6 +206,76 @@ describe('createToolAttribution', () => { assert.equal(second.ownerOf('reload_tool'), 'provider-b', 'the fresh hook records the registration') }) + test('unloading the hook plugin restores the original register', async () => { + const app = new Context() + const fake = { register(_definition?: unknown) { return () => {} } } + app.provide('tools', fake) + const original = fake.register + let attribution!: ToolAttribution + const fiber = app.plugin({ + name: 'dsh-context', + apply(sub) { + attribution = createToolAttribution(sub) + }, + }) + await fiber + assert.notEqual(fake.register, original, 'the hook wrapped the instance at apply') + await fiber.dispose() + assert.equal(fake.register, original, 'unload put the original register back on the instance') + fake.register({ name: 'orphan_tool' }) + assert.equal(attribution.ownerOf('orphan_tool'), undefined, 'no dead-hook recording after unload') + }) + + test('an unloaded older incarnation leaves a newer wrapper installed', async () => { + const app = new Context() + const fake = { register() { return () => {} } } + app.provide('tools', fake) + const original = fake.register + let second!: ToolAttribution + const older = app.plugin({ name: 'older-hook', apply(sub) { createToolAttribution(sub) } }) + await older + assert.notEqual(fake.register, original, 'the first hook wrapped the instance') + const newer = app.plugin({ name: 'newer-hook', apply(sub) { second = createToolAttribution(sub) } }) + await newer + assert.notEqual(fake.register, original, 'the newer incarnation re-wrapped the instance') + await older.dispose() + assert.notEqual(fake.register, original, 'the older disposer did not strip the newer wrapper') + await app.plugin({ + name: 'provider-c', + apply(sub) { + ;((sub as any).tools as { register(def?: unknown): () => void }).register({ name: 'late_tool' }) + }, + }) + assert.equal(second.ownerOf('late_tool'), 'provider-c', 'the surviving hook still records') + await newer.dispose() + assert.equal(fake.register, original, 'the newer disposer restores the true original') + }) + + test('unload then re-apply re-wraps the restored original without stacking', async () => { + const app = new Context() + let calls = 0 + app.provide('tools', { register() { calls++; return () => {} } }) + let attribution!: ToolAttribution + const first = app.plugin({ name: 'dsh-context', apply(sub) { createToolAttribution(sub) } }) + await first + await first.dispose() + const second = app.plugin({ + name: 'dsh-context', + apply(sub) { + attribution = createToolAttribution(sub) + }, + }) + await second + await app.plugin({ + name: 'provider-d', + apply(sub) { + ;((sub as any).tools as { register(def?: unknown): () => void }).register({ name: 'reloaded_tool' }) + }, + }) + assert.equal(calls, 1, 'the original register ran once, through one wrapper only') + assert.equal(attribution.ownerOf('reloaded_tool'), 'provider-d', 'the re-applied hook records the registration') + }) + test('tags boot-time tools whose providers predate the hook as unknown', () => { const app = new Context() const fake = {