fix(host): restore the patched tools.register when the plugin unloads
createToolAttribution permanently replaced the global tools service's register method and never undid it: after the plugin was disabled, removed, or reloaded, later registrations kept running through the dead hook's wrapper, which retained the live map and recorded into it. The wrap now registers a fiber effect whose disposer peels every wrapper this incarnation installed back to the true original, so the global instance is left exactly as it was found. A guard keeps the restore order-independent: a closure that finds a newer incarnation's wrapper installed does nothing, letting that incarnation's own effect own the cleanup. ctx.effect exists in every supported dsh version (cordis 4.0.1 since 0.1.0-rc.7). Fixes #36
This commit is contained in:
1 parent
32ae8efe48
commit
88aeccfe30
2 files changed
+91
-3
No files matched your search
+21
-3
@@ -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 <path>` 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
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
Reference in new issue
Block a user