fix(scope): harden lifecycle ownership foundation
Make Cordis construction and teardown ownership reentrancy-safe, then carry caller and provider ownership through reservation, setup, publication, quiescence, and sentinel retirement. Stabilize registry carriers and factory/workflow boundaries, add adversarial lifecycle regressions, and align the rewritten RFC plus generated contracts with the enforced behavior.
This commit is contained in:
@@ -19,9 +19,9 @@ Creates and holds event-sourced `Session` instances. Persistence is intentionall
|
||||
`create()` covers the common case (the session is owned by the calling fiber). When a session must be torn down **in order with another resource** — so a final flush is captured before the store-owned append observer detaches — `create()`'s self-contained effect is wrong, because a fiber unload disposes sibling effects *concurrently*. For that, split the lifecycle and fold it into the owner's single effect:
|
||||
|
||||
- `ctx.sessions.prepare(id?, options?): Session` — read `options.seed`/`options.meta` once, validate and detach the metadata/header, and construct the `Session` WITHOUT entering it into the store. Same options as `create`.
|
||||
- `ctx.sessions.reserve(id): SessionRegistrationReservation` — hold an unpublished id under the calling fiber and construct its one owned Session through `reservation.prepare(options?)`. Until `release()` or owner unload, bare `prepare`/`create`/`enter` calls for that id reject; the factory later presents the exact capability to `enter`, making setup-time publication structurally impossible without leaking an abandoned reservation across HMR disposal.
|
||||
- `ctx.sessions.enter(session, reservation?): () => void` — install the module-private `session/event` observer, capture its scope carrier, and add the session under one accepted id; returns the idempotent DETACH disposer, which clears notification, carrier, and accepted-key state. Does NOT emit `session/created` (the caller installs the disposer first, then calls `announce`, so a throwing listener rolls the attach back). It re-checks the id because public `prepare`/`enter` calls may be interleaved; a stale prepared object must not overwrite a live same-id session. A factory passes the opaque capability from `reserve(id)` so setup cannot enter the reserved session or publish a same-id replacement before the owning transaction.
|
||||
- `ctx.sessions.announce(session): void` — begin the one allowed `session/created` announcement for an entered session; repeat and reentrant calls reject before dispatch. Its detach emits `session/disposed` exactly once, including rollback after a partially delivered creation notification; a never-announced entry emits neither edge.
|
||||
- `ctx.sessions.reserve(id): SessionRegistrationReservation` — hold an unpublished id under the calling fiber and construct its one owned Session through `reservation.prepare(options?)`. `release` is the exact owner effect disposer, so the agent lifecycle can adopt it and keep the ID reserved until scope cleanup quiesces. Until that release, bare `prepare`/`create`/`enter` calls for the id reject; the factory later presents the exact capability to `enter`, making setup-time publication structurally impossible without leaking an abandoned reservation across HMR disposal.
|
||||
- `ctx.sessions.enter(session, reservation?): () => void` — claim the ID across caller-controlled filter/carrier evaluation, then install the module-private `session/event` observer and add the exact session under its accepted key; a reentrant same-ID entry cannot be overwritten. Returns the idempotent, exact-object-guarded DETACH disposer, which clears notification, carrier, and accepted-key state without letting a stale capability delete a replacement. Does NOT emit `session/created` (the caller installs the disposer first, then calls `announce`, so a throwing listener rolls the attach back). It re-checks the id because public `prepare`/`enter` calls may be interleaved. A factory passes the opaque capability from `reserve(id)` so setup cannot enter the reserved session or publish a same-id replacement before the owning transaction.
|
||||
- `ctx.sessions.announce(session): void` — begin the one allowed `session/created` announcement for an entered session; repeat and reentrant calls reject before dispatch. A detach requested synchronously by a creation listener is deferred until that dispatch unwinds, so another creation listener cannot observe `session/disposed` before its own `session/created` callback. Detach emits `session/disposed` exactly once, including rollback after a partially delivered creation notification; a never-announced entry emits neither edge.
|
||||
|
||||
`dsh-agent-loop` is the canonical consumer: after unpublished agent setup it enters both session and agent before announcing either, then nests loop stop, agent removal, session detach, and scope unwind in one ordered lifecycle. The final flush therefore settles before this package detaches the session, whether teardown starts from an `AgentHandle` or owner-fiber unload.
|
||||
|
||||
|
||||
@@ -37,7 +37,9 @@ declare module 'cordis' {
|
||||
* A session was created in the store. A synchronous listener throw vetoes
|
||||
* publication and rollback emits the matching `session/disposed` edge;
|
||||
* returned-promise rejection is observed and logged but cannot retroactively
|
||||
* veto this synchronous boundary.
|
||||
* veto this synchronous boundary. A synchronous listener that requests the
|
||||
* advanced detach does not remove the entry immediately: removal and the
|
||||
* paired `session/disposed` edge wait until the creation dispatch unwinds.
|
||||
* Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): the carrier is the
|
||||
* session's owner scope, captured when the session was ENTERED (an agent's
|
||||
* session is entered through `agent.ctx`, so its events dispatch in that
|
||||
@@ -648,7 +650,9 @@ export interface SessionRegistrationReservation {
|
||||
prepare(options?: CreateSessionOptions): Session
|
||||
/**
|
||||
* Release the unpublished reservation; idempotent. The store also releases
|
||||
* it automatically when the fiber that called `reserve` disposes.
|
||||
* it automatically when the fiber that called `reserve` disposes. This
|
||||
* function is that exact Cordis effect disposer, so an ordered lifecycle may
|
||||
* yield it by identity and place release after quiescence.
|
||||
* @returns nothing.
|
||||
*/
|
||||
release(): void
|
||||
@@ -662,10 +666,16 @@ export interface SessionRegistrationReservation {
|
||||
*/
|
||||
export class SessionStore extends Service {
|
||||
private store = new Map<SessionId, Session>()
|
||||
/** Ids claimed across caller-code boundaries before their exact entry commits. */
|
||||
private enteringIds = new Set<SessionId>()
|
||||
/** The one accepted map key for each live session; never reread caller state. */
|
||||
private acceptedIds = new WeakMap<Session, SessionId>()
|
||||
/** Sessions whose creation announcement began and therefore require a pair. */
|
||||
private announced = new WeakSet<Session>()
|
||||
/** Entries currently dispatching `session/created`; detach waits for dispatch to unwind. */
|
||||
private announcing = new WeakSet<Session>()
|
||||
/** A detach requested reentrantly from `session/created`. */
|
||||
private pendingDetach = new WeakSet<Session>()
|
||||
/** Unpublished identities held across factory load/setup transactions. */
|
||||
private reservations = new Map<SessionId, SessionRegistrationReservation>()
|
||||
/** The exact prepared object owned by each reservation capability. */
|
||||
@@ -696,18 +706,19 @@ export class SessionStore extends Service {
|
||||
*/
|
||||
reserve(id: SessionId): SessionRegistrationReservation {
|
||||
if (typeof id !== 'string') throw new TypeError('session id must be a string')
|
||||
if (this.store.has(id) || this.reservations.has(id)) {
|
||||
if (this.store.has(id) || this.reservations.has(id) || this.enteringIds.has(id)) {
|
||||
throw new Error(`session "${id}" already exists or is reserved`)
|
||||
}
|
||||
let active = true
|
||||
let prepared = false
|
||||
const rawRelease = (): void => {
|
||||
if (!active) return
|
||||
active = false
|
||||
this.reservedSessions.delete(reservation)
|
||||
this.reservations.delete(id)
|
||||
}
|
||||
let disposeEffect!: () => Promise<void> | void
|
||||
// `release` is the exact effect disposer, so an ordered composite can
|
||||
// adopt the automatic owner cleanup instead of racing it as a sibling.
|
||||
const release = this.ctx.effect(() => rawRelease, `sessions.reserve(${id})`)
|
||||
const reservation: SessionRegistrationReservation = Object.freeze({
|
||||
id,
|
||||
prepare: (options?: CreateSessionOptions) => {
|
||||
@@ -720,20 +731,9 @@ export class SessionStore extends Service {
|
||||
this.reservedSessions.set(reservation, session)
|
||||
return session
|
||||
},
|
||||
release: () => {
|
||||
rawRelease()
|
||||
// Remove the now-inert ownership effect on manual transaction settle;
|
||||
// its cleanup is the exact idempotent raw release above.
|
||||
void disposeEffect()
|
||||
},
|
||||
release,
|
||||
})
|
||||
this.reservations.set(id, reservation)
|
||||
try {
|
||||
disposeEffect = this.ctx.effect(() => rawRelease, `sessions.reserve(${id})`)
|
||||
} catch (error: unknown) {
|
||||
rawRelease()
|
||||
throw error
|
||||
}
|
||||
return reservation
|
||||
}
|
||||
|
||||
@@ -845,7 +845,9 @@ export class SessionStore extends Service {
|
||||
* @param session - a {@link prepare}d session not yet in the store.
|
||||
* @param reservation - the exact unpublished-id capability when a factory
|
||||
* reserved this session across setup.
|
||||
* @returns the detach disposer (observer + store removal).
|
||||
* @returns the detach disposer (observer + store removal). When called from
|
||||
* a synchronous `session/created` listener, removal and disposal wait until
|
||||
* that creation dispatch unwinds.
|
||||
* @throws if a session with this id is already in the store.
|
||||
*/
|
||||
enter(session: Session, reservation?: SessionRegistrationReservation): () => void {
|
||||
@@ -858,30 +860,71 @@ export class SessionStore extends Service {
|
||||
|| this.reservedSessions.get(reservation) !== session) {
|
||||
throw new Error(`session "${id}" registration reservation does not own this prepared session`)
|
||||
}
|
||||
if (this.store.has(id)) throw new Error(`session "${id}" already exists`)
|
||||
if (this.store.has(id) || this.enteringIds.has(id)) {
|
||||
throw new Error(`session "${id}" already exists`)
|
||||
}
|
||||
if (appendObservers.has(session)) throw new Error(`session "${id}" is already attached to a store`)
|
||||
this.enteringIds.add(id)
|
||||
// The carrier is decided HERE, once, from the ENTERING context's scope tag
|
||||
// (`this.ctx` is the caller's context — the tracker mechanism): every
|
||||
// session/created|event|flush dispatch for this session uses it, so the
|
||||
// session's whole event feed is scope-filtered consistently. The base is
|
||||
// the session itself (scoped listeners' `this` is the session).
|
||||
const carrier = scopeTarget(session, scopeOf(this.ctx))
|
||||
let carrier: Scoped<Session>
|
||||
try {
|
||||
carrier = scopeTarget(session, scopeOf(this.ctx))
|
||||
} finally {
|
||||
this.enteringIds.delete(id)
|
||||
}
|
||||
const currentReservation = this.reservations.get(id)
|
||||
if (reservation === undefined) {
|
||||
/* v8 ignore next 2 -- reserve() rejects enteringIds, so carrier
|
||||
* construction cannot install a new same-id reservation */
|
||||
if (currentReservation !== undefined) {
|
||||
throw new Error(`session "${id}" is reserved for unpublished creation`)
|
||||
}
|
||||
} else if (currentReservation !== reservation
|
||||
|| this.reservedSessions.get(reservation) !== session) {
|
||||
throw new Error(`session "${id}" registration reservation does not own this prepared session`)
|
||||
}
|
||||
/* v8 ignore next 1 -- enteringIds prevents a same-store commit during carrier construction */
|
||||
if (this.store.has(id)) throw new Error(`session "${id}" already exists`)
|
||||
if (appendObservers.has(session)) throw new Error(`session "${id}" is already attached to a store`)
|
||||
this.carriers.set(session, carrier)
|
||||
const emitCtx = this.ctx
|
||||
appendObservers.set(session, (event) => { emitCtx.emit(carrier, 'session/event', session, event) })
|
||||
this.acceptedIds.set(session, id)
|
||||
this.store.set(id, session)
|
||||
let entered = true
|
||||
return () => {
|
||||
const detach = (): void => {
|
||||
if (!entered) return
|
||||
entered = false
|
||||
const wasAnnounced = this.announced.delete(session)
|
||||
appendObservers.delete(session)
|
||||
this.acceptedIds.delete(session)
|
||||
this.carriers.delete(session)
|
||||
this.store.delete(id)
|
||||
if (wasAnnounced) this.emitDisposed(session, carrier, id)
|
||||
// A creation listener may own the advanced detach capability. Keep the
|
||||
// entry and its event observer live until the synchronous creation
|
||||
// dispatch unwinds, then publish the paired disposal edge.
|
||||
if (this.announcing.has(session)) {
|
||||
this.pendingDetach.add(session)
|
||||
return
|
||||
}
|
||||
this.detachEntered(session, id, carrier)
|
||||
}
|
||||
return detach
|
||||
}
|
||||
|
||||
/** Remove one exact entered session and emit its paired disposal when announced. */
|
||||
private detachEntered(session: Session, id: SessionId, carrier: Scoped<Session>): void {
|
||||
this.pendingDetach.delete(session)
|
||||
// A stale capability cannot remove observers or storage belonging to a
|
||||
// later same-id lifecycle.
|
||||
/* v8 ignore next 1 -- the commit claim makes replacement impossible; this
|
||||
* remains the exact-identity backstop against future mutation paths */
|
||||
if (this.store.get(id) !== session || this.acceptedIds.get(session) !== id) return
|
||||
const wasAnnounced = this.announced.delete(session)
|
||||
appendObservers.delete(session)
|
||||
this.acceptedIds.delete(session)
|
||||
this.carriers.delete(session)
|
||||
this.store.delete(id)
|
||||
if (wasAnnounced) this.emitDisposed(session, carrier, id)
|
||||
}
|
||||
|
||||
/** Emit `session/created` exactly once for an {@link enter}ed session (with
|
||||
@@ -892,25 +935,31 @@ export class SessionStore extends Service {
|
||||
* @throws if the session is not live or its announcement already began,
|
||||
* including a reentrant call from a creation listener. */
|
||||
announce(session: Session): void {
|
||||
const carrier = this.liveCarrierFor(session)
|
||||
const { carrier, id } = this.liveEntryFor(session)
|
||||
if (this.announced.has(session)) {
|
||||
throw new Error(`session "${session.id}" was already announced`)
|
||||
throw new Error(`session "${id}" was already announced`)
|
||||
}
|
||||
// Mark before emit: Cordis emit may deliver to earlier listeners and then
|
||||
// throw. Rollback must still pair that partial creation with disposal, and
|
||||
// a listener cannot recursively create a second lifecycle edge.
|
||||
this.announced.add(session)
|
||||
const args: unknown[] = [carrier, 'session/created', session]
|
||||
for (const callback of this.ctx.events.dispatch('emit', args)) {
|
||||
// Synchronous throws intentionally propagate and veto publication; the
|
||||
// yielded detach then emits the paired disposal edge. An async function
|
||||
// is nevertheless assignable to a void listener, so observe its returned
|
||||
// promise: rejection is too late to roll back and must be logged instead
|
||||
// of becoming unhandled.
|
||||
const returned: unknown = callback(...args)
|
||||
void Promise.resolve(returned).catch((error: unknown) => {
|
||||
this.ctx.logger.warn(`session "${session.id}": session/created listener rejected: ${renderThrown(error)}`)
|
||||
})
|
||||
this.announcing.add(session)
|
||||
try {
|
||||
for (const callback of this.ctx.events.dispatch('emit', args)) {
|
||||
// Synchronous throws intentionally propagate and veto publication; the
|
||||
// yielded detach then emits the paired disposal edge. An async function
|
||||
// is nevertheless assignable to a void listener, so observe its returned
|
||||
// promise: rejection is too late to roll back and must be logged instead
|
||||
// of becoming unhandled.
|
||||
const returned: unknown = callback(...args)
|
||||
void Promise.resolve(returned).catch((error: unknown) => {
|
||||
this.ctx.logger.warn(`session "${id}": session/created listener rejected: ${renderThrown(error)}`)
|
||||
})
|
||||
}
|
||||
} finally {
|
||||
this.announcing.delete(session)
|
||||
if (this.pendingDetach.has(session)) this.detachEntered(session, id, carrier)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -940,11 +989,11 @@ export class SessionStore extends Service {
|
||||
* @returns resolves when every flush listener has settled; rejects if one rejects.
|
||||
*/
|
||||
async flush(session: Session): Promise<void> {
|
||||
await this.ctx.parallel(this.liveCarrierFor(session), 'session/flush', session)
|
||||
await this.ctx.parallel(this.liveEntryFor(session).carrier, 'session/flush', session)
|
||||
}
|
||||
|
||||
/** Return the exact live session's carrier; detached/prepared objects reject. */
|
||||
private liveCarrierFor(session: Session): Scoped<Session> {
|
||||
/** Return the exact live session's accepted id and carrier; detached/prepared objects reject. */
|
||||
private liveEntryFor(session: Session): { id: SessionId; carrier: Scoped<Session> } {
|
||||
const id = this.acceptedIds.get(session)
|
||||
if (id === undefined || this.store.get(id) !== session) {
|
||||
throw new Error(`session "${id ?? session.id}" is not live in this store`)
|
||||
@@ -957,7 +1006,7 @@ export class SessionStore extends Service {
|
||||
if (carrier === undefined) {
|
||||
throw new Error(`session "${id}" has no dispatch carrier`)
|
||||
}
|
||||
return carrier
|
||||
return { id, carrier }
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -740,6 +740,79 @@ describe('SessionStore', () => {
|
||||
expect(ctx.sessions.get(SessionId('racy'))).toBe(live)
|
||||
})
|
||||
|
||||
it('claims an id across Context.filter evaluation before committing the exact session', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const id = SessionId('reentrant-enter')
|
||||
const nested = new Session(id)
|
||||
const outer = new Session(id)
|
||||
let nestedError = ''
|
||||
let attempted = false
|
||||
Object.defineProperty(outer, Context.filter, {
|
||||
configurable: true,
|
||||
get() {
|
||||
if (!attempted) {
|
||||
attempted = true
|
||||
try {
|
||||
ctx.sessions.enter(nested)
|
||||
} catch (error: unknown) {
|
||||
nestedError = String(error)
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
},
|
||||
})
|
||||
|
||||
const detach = ctx.sessions.enter(outer)
|
||||
expect(nestedError).toMatch(/already exists/)
|
||||
expect(ctx.sessions.get(id)).toBe(outer)
|
||||
detach()
|
||||
expect(ctx.sessions.get(id)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('revalidates reservation ownership after carrier construction runs caller code', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const id = SessionId('released-during-enter')
|
||||
const reservation = ctx.sessions.reserve(id)
|
||||
const session = reservation.prepare()
|
||||
Object.defineProperty(session, Context.filter, {
|
||||
configurable: true,
|
||||
get() {
|
||||
reservation.release()
|
||||
return undefined
|
||||
},
|
||||
})
|
||||
|
||||
expect(() => ctx.sessions.enter(session, reservation)).toThrow(/does not own this prepared session/)
|
||||
expect(ctx.sessions.get(id)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('rejects when carrier construction attaches the same session to another store', async () => {
|
||||
const firstCtx = new Context()
|
||||
const secondCtx = new Context()
|
||||
await firstCtx.plugin(SessionStore)
|
||||
await secondCtx.plugin(SessionStore)
|
||||
const session = new Session(SessionId('cross-store-carrier'))
|
||||
let attempted = false
|
||||
let detachSecond = (): void => {}
|
||||
Object.defineProperty(session, Context.filter, {
|
||||
configurable: true,
|
||||
get() {
|
||||
if (!attempted) {
|
||||
attempted = true
|
||||
detachSecond = secondCtx.sessions.enter(session)
|
||||
}
|
||||
return undefined
|
||||
},
|
||||
})
|
||||
|
||||
expect(() => firstCtx.sessions.enter(session)).toThrow(/already attached to a store/)
|
||||
expect(firstCtx.sessions.get(session.id)).toBeUndefined()
|
||||
expect(secondCtx.sessions.get(session.id)).toBe(session)
|
||||
detachSecond()
|
||||
})
|
||||
|
||||
it('prepare() + enter() + announce() register a session and emit session/created', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
@@ -869,6 +942,49 @@ describe('SessionStore', () => {
|
||||
expect({ created, disposed }).toEqual({ created: 1, disposed: 1 })
|
||||
})
|
||||
|
||||
it('defers a reentrant detach until the creation dispatch unwinds', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const order: string[] = []
|
||||
const session = ctx.sessions.prepare(SessionId('reentrant-detach'))
|
||||
const detach = ctx.sessions.enter(session)
|
||||
|
||||
ctx.on('session/created', (created) => {
|
||||
order.push('created:first')
|
||||
detach()
|
||||
expect(ctx.sessions.get(created.id)).toBe(created)
|
||||
})
|
||||
ctx.on('session/created', (created) => {
|
||||
order.push('created:second')
|
||||
expect(ctx.sessions.get(created.id)).toBe(created)
|
||||
})
|
||||
ctx.on('session/disposed', (disposed) => {
|
||||
order.push('disposed')
|
||||
expect(ctx.sessions.get(disposed.id)).toBeUndefined()
|
||||
})
|
||||
|
||||
ctx.sessions.announce(session)
|
||||
|
||||
expect(order).toEqual(['created:first', 'created:second', 'disposed'])
|
||||
expect(ctx.sessions.get(session.id)).toBeUndefined()
|
||||
detach()
|
||||
})
|
||||
|
||||
it('rolls back create when its owner unloads from session/created', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
let ownerCtx!: Context
|
||||
const owner = await ctx.plugin(Object.assign((inner: Context) => { ownerCtx = inner }, { inject: ['sessions'] }))
|
||||
const id = SessionId('create-unload-race')
|
||||
ctx.on('session/created', (session) => {
|
||||
if (session.id === id) void owner.dispose()
|
||||
})
|
||||
|
||||
ownerCtx.sessions.create(id)
|
||||
await owner.dispose()
|
||||
expect(ctx.sessions.get(id)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('synthesizes a minimal current-version header for a bare-created session', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
|
||||
Reference in New Issue
Block a user