diff --git a/packages/core/agent-loop/tests/scope-lifecycle.spec.ts b/packages/core/agent-loop/tests/scope-lifecycle.spec.ts index 7a45898b8e..b89f06727d 100644 --- a/packages/core/agent-loop/tests/scope-lifecycle.spec.ts +++ b/packages/core/agent-loop/tests/scope-lifecycle.spec.ts @@ -170,6 +170,29 @@ describe('agent scope lifecycle', () => { expect(heard).toEqual(['a1:2']) }) + it('a listener may drive the agent through its declared `this` (the carrier is method-transparent)', async () => { + // ds-review-bot regression: agent/* listeners are typed + // `this: Scoped`, and ReactLoopAgent's send/steer/cancel read the + // native-private #carrier — a proxy-receiver carrier made + // `this.send(...)` throw TypeError. The carrier binds methods to the real + // agent, so driving through the event `this` is a working supported shape. + const adapter = new MockAdapter([textResponse('first'), textResponse('second')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + let followUpSent = false + ctx.on('agent/session-start', function (this: Agent) { + // Deliberately through `this`, not the args subject. + this.send(text('driven through this')) + followUpSent = true + }) + const second = ctx.agentLoop.create(AgentId('a2'), { model: 'mock' }) + expect(followUpSent).toBe(true) + await second.whenIdle() + // The send actually reached the loop: the prompt ran a turn. + expect(second.session.events.some(e => e.type === 'turn/start')).toBe(true) + await agent.whenIdle() + }) + it('owner unload honors the documented teardown order: unregistration AFTER the drain, before detach', async () => { const ctx = await harness() let handle!: ReturnType diff --git a/packages/core/scope/src/index.ts b/packages/core/scope/src/index.ts index 9c4fd0019e..1243b7d715 100644 --- a/packages/core/scope/src/index.ts +++ b/packages/core/scope/src/index.ts @@ -23,7 +23,7 @@ */ import type { Context } from 'cordis' -import { Context as CordisContext, withProps } from 'cordis' +import { Context as CordisContext } from 'cordis' /** * The identity a scope is keyed by. Opaque and compared by object identity — @@ -178,10 +178,14 @@ export function scopeOf(ctx: Context): ScopeKey | undefined { * * Use it as the `thisArg` of the dispatch: * `ctx.waterfall(scopeTarget(this, exec.agent), 'tools/pre-execute', …)`. The - * carrier is a proxy over `base` — listener `this` stays `base`-shaped, but - * identity-comparing `this` against the subject is not supported; the subject - * always travels in the event's arguments. The returned carrier is branded - * {@link Scoped} and runtime-marked ({@link isScopeCarrier} / + * carrier is a TRANSPARENT proxy over `base`: reads delegate with `base` as + * the receiver and retrieved methods are bound to `base`, so a listener may + * call subject methods through its `this` (`this.send(…)` on a + * `Scoped`) even when the subject uses native `#private` fields — a + * bare proxy receiver would throw on those. Identity is still not + * transparent: `this !== subject` and method identity varies per read; the + * subject always travels in the event's arguments. The returned carrier is + * branded {@link Scoped} and runtime-marked ({@link isScopeCarrier} / * {@link carrierKeyOf}) so both the type system and the dev invariants can * tell a carrier from a bare subject. * @param base - the object the event is dispatched on behalf of (the owning @@ -198,15 +202,41 @@ export function scopeTarget(base: T, key: ScopeKey | undefined const tag = scopeOf(ctx) return tag === undefined || tag === key } - // withProps overlays own-property reads; the symbol-keyed props have no - // structural overlap with T. withProps is typed `any` upstream (a generic - // proxy helper); the carrier is structurally the same T it overlays plus - // the compile-time brand, so pin the type via the return annotation. - // eslint-disable-next-line @typescript-eslint/no-unsafe-return - return withProps(base, { + const overlay: Record = { [CordisContext.filter]: filter, [kCarrier]: { key }, - }) + } + // A hand-rolled proxy, NOT cordis withProps: withProps delegates gets with + // the PROXY as receiver, so a getter on `base` runs with proxy `this` and a + // method call through the carrier gets a proxy receiver — either one throws + // on a native `#private` field of the subject (TypeError: private member + // not declared). Cordis hands the carrier to listeners as `this`, and the + // event declarations type it `Scoped` — so subject method calls + // through it are a SUPPORTED shape and must reach the real object: gets + // delegate with `base` as receiver, functions come back bound to `base`, + // and sets land on `base` directly. + return new Proxy(base, { + get(target, prop) { + // hasOwn, not `in`: the overlay literal inherits Object.prototype, so + // `in` would claim `toString`/`constructor` and shadow the subject's. + if (Object.hasOwn(overlay, prop)) return overlay[prop] + const value: unknown = Reflect.get(target, prop, target) + if (typeof value !== 'function') return value + // Proxy invariant guard: a non-configurable, non-writable OWN data + // property must be reported unchanged, so it cannot be bound. Class + // methods live on the prototype (no own descriptor) and bind freely; + // only a frozen own-function prop keeps the raw (unbound) function. + const own = Reflect.getOwnPropertyDescriptor(target, prop) + if (own !== undefined && own.configurable === false && own.writable === false) return value + // `Function.prototype.bind` types as `any`; the value is structurally + // T[prop] and the trap's contract is untyped (`any`), so unknown is the + // honest safe return. + return value.bind(target) as unknown + }, + set(target, prop, value) { + return Reflect.set(target, prop, value, target) + }, + }) as Scoped } /** @@ -219,9 +249,9 @@ export function scopeTarget(base: T, key: ScopeKey | undefined */ export function isScopeCarrier(value: unknown): value is Scoped { if (typeof value !== 'object' || value === null) return false - // A property READ, not an `in` check: withProps overlays props via get/set - // traps only (no `has` trap), so `kCarrier in carrier` would fall through to - // the wrapped base and always answer false. + // A property READ, not an `in` check: the carrier overlays its marks in the + // get trap only (no `has` trap), so `kCarrier in carrier` would fall + // through to the wrapped base and always answer false. return (value as { [kCarrier]?: { key: ScopeKey | undefined } })[kCarrier] !== undefined } diff --git a/packages/core/scope/tests/scope.spec.ts b/packages/core/scope/tests/scope.spec.ts index e932ed80db..53e5bc931b 100644 --- a/packages/core/scope/tests/scope.spec.ts +++ b/packages/core/scope/tests/scope.spec.ts @@ -179,6 +179,49 @@ describe('scopeTarget dispatch filtering', () => { expect(result).toBe('seed+v') expect(seenLabel).toBe('the-base') }) + + it('is transparent for subjects with native #private fields: methods and getters through the carrier reach the real object', () => { + // The ds-review-bot regression: cordis hands the carrier to listeners as + // `this` (typed Scoped), so subject method calls through it are a + // supported shape. A proxy that delegates with the PROXY as receiver + // (cordis withProps) throws TypeError on any native #private the method + // or getter touches; the carrier must delegate with the BASE as receiver + // and bind retrieved methods to it. + class Subject { + #count = 0 + bump(): number { return ++this.#count } + get count(): number { return this.#count } + } + const subject = new Subject() + const carrier = scopeTarget(subject, subject) + expect(carrier.bump()).toBe(1) // method call: bound to the base + expect(subject.count).toBe(1) // ...and it mutated the REAL object + expect(carrier.count).toBe(1) // getter: runs with the base as receiver + // The get trap returns the method already bound to the base; + // detachability IS the assertion. + // eslint-disable-next-line @typescript-eslint/unbound-method + const detached = carrier.bump + expect(detached()).toBe(2) + }) + + it('delegates sets to the base and leaves frozen own function props unbound (proxy invariant)', () => { + const frozenFn = (): string => 'frozen' + const base: { mutable: number; pinned: () => string; toString: () => string } = { + mutable: 0, + pinned: frozenFn, + toString: () => 'base-str', + } + Object.defineProperty(base, 'pinned', { value: frozenFn, writable: false, configurable: false }) + const carrier = scopeTarget(base, undefined) + carrier.mutable = 7 + expect(base.mutable).toBe(7) // sets land on the base, not a detached overlay + // A non-configurable, non-writable own data prop must be reported + // unchanged (binding it would violate the proxy get invariant). + expect(carrier.pinned).toBe(frozenFn) + // The overlay literal inherits Object.prototype; hasOwn (not `in`) keeps + // it from shadowing the subject's own prototype-surface members. + expect(String(carrier)).toBe('base-str') + }) }) describe('carrier marks', () => {