Replies: 4 comments
|
I think the bigger invariant here is that plugin invocation kind and returned-effect semantics should be explicit, not inferred independently at execution time. Would it be worth normalizing every registered plugin into {invoke_as: class|apply, effect_contract} once at load/registration, then making Fiber state transition to ACTIVE only after that declared effect has actually settled? |
|
That is the better fix, and I think it is a strictly stronger statement of the invariant than the one I proposed. My patch decides The second half of your proposal is the part I would push hardest for, because it is what actually broke: ACTIVE only after the declared effect has settled. The construction bug was merely how the effect got dropped; the damage was that the state machine advertised a state the fiber had not reached. Any future path that drops or defers an effect reintroduces the same lie unless the transition is gated on settlement rather than on "apply returned". Two implementation notes from carrying the narrow version in production:
Happy to be superseded here — a declared contract is the correct shape, and my fix should be read as a stopgap for trees that need the crash to stop before that lands. |
|
Thanks — agreed on treating the syntax predicate as a compatibility adapter rather than the runtime invariant. A staged path might be: 1 Normalize every plugin at registration into an internal descriptor such as { callback, invoke_as, effect_contract, source }. That would allow the immediate class-syntax fix to land independently, while providing a migration path toward an invocation model that no longer depends on function shape. I’d be happy to help turn this into focused regression cases or a concrete patch if the maintainers want to pursue it. |
|
I added an English plugin-authoring note based on this report: https://sandbaseai.github.io/deepseek-harness-handbook/first-plugin.html The guidance keeps the claim scoped: a function-shaped apply may be treated as constructible in the reported runner path, dropping a disposer or pending Promise. Test the exact composition loader, keep cleanup in ctx.effect(), and fail closed until async activation is observable. |
Uh oh!
There was an error while loading. Please reload this page.
Found while chasing a consumer's silent boot hang; reproducible in ten lines against the vendored cordis in this repo.
The defect
Fiber's runner decidesnewversus plain call withisConstructor(runtime.callback)— a CAN-construct heuristic that any plainfunctiondeclaration satisfies by owning a prototype. An object plugin whoseapplyis declared withfunction— which is what every ESM module row in a composition YAML resolves to, and the dominant idiom across this repo's own packages — is therefore constructed:new apply(ctx, config)runs the body, but a returned effect becomes thenewresult, and theinstance[init]()lookup discards it. Two consequences:export function apply(ctx) { return () => cleanup() }leaks its cleanup, silently, in every tree.fiber.await(), Loader settlement, and every boot audit built on fiber state all report a healthy row.Repro
Mount it as an object/namespace plugin (or as a composition row through the loader): the fiber reaches
state === ACTIVEandfiber.await()resolves while the apply is pending forever. Declare the same body asapply: () => new Promise(() => {})(arrow — no prototype) and the fiber correctly holds LOADING with a pending await. The plugin's behavior depends on which function syntax the author happened to use.Downstream this is a silent-hang class: a composition row whose async apply sticks (a plugin bug, or a filesystem call an endpoint agent holds) passes every audit, the session composes half-applied, and the only observer left is a supervisor's process-kill timer.
The fix we adopted
Replace constructibility with authored intent: construct only
class-syntax callbacks.and at the runner,
if (isClassSyntax(runtime.callback)).toStringsees through proxies (the trap defaults to the target), and bound functions reportfunction, which is right — a bound class cannot carry plugin statics anyway.One migration note from our own tree: shape-based tracking ("
{apply}never constructs", recorded at registration) looks cleaner but breaks a real pattern — our test harness wraps every plugin, classes included, as{apply: callback}and relied on the old heuristic rescuing them. The syntax rule preserves that while still fixing the drop. Second migration note: previously-discarded returns from syncfunctionapplies become honored (awaited when thenable, collected when disposable), which is the documented contract for the other plugin shapes — our full suite (14k unit tests, e2e trees) passed unchanged, but a codebase with an apply returning a non-effect value would surface it asInvalid effectat mount.Our commit, on 0.1.1-rc.2 vintage:
7e7287d170(private fork; happy to share the patch). On top of it we gave preset/composition mounts a deadline that reports rows still applying by id, since truthful fiber states make that audit possible at all.All reactions