fix(bus): acquire PubSub subscription eagerly to close /event race
`bus.subscribe(def)` and `bus.subscribeAll()` previously returned a `Stream` whose underlying `PubSub.subscribe` ran lazily on first pull (via `Stream.unwrap`). When a Stream was built in one place and consumed in another — e.g. the `/event` SSE handler, which returns the stream inside `HttpServerResponse.stream` for the body-pump fiber to consume later — any publish in the hand-off window was lost. The `/event` handler's `Stream.concat(server.connected, events)` shape made this concrete: the events stream's PubSub.subscribe was only run *after* `server.connected` was emitted and flushed to the socket, so publishes that landed between the client receiving `server.connected` and the body-pump fiber pulling from `events` were silently dropped. SDK consumers (`client.event.subscribe()`, the Slack bot, `opencode run --attach`) hit this routinely because they typically subscribe and then trigger something that publishes (e.g. `sdk.part.update`). Change the bus interface so subscribe / subscribeAll return `Effect<Stream, never, Scope>`. The subscription is now acquired eagerly when the caller yields the effect, lives in the caller's scope, and is released by the scope's finalizer. Any publish after `yield*` is buffered into the subscription queue regardless of when the consumer activates. Migrated callers: - /event handler (handlers/event.ts) - plugin/index.ts - project/project.ts - project/vcs.ts - share/share-next.ts Regression tests added: - test/bus/bus-effect.test.ts — 3 unit tests for eager subscribe, including the /event-shape Stream.concat pattern. - test/server/httpapi-event-diagnostics.test.ts — 7 diagnostic tests (D1-D7) isolating each variable in the publisher chain. D7 (no-op AppRuntime warmup) is the smallest regression trigger. - test/server/httpapi-sdk.test.ts — end-to-end SDK subscription through /event during a sync.run-driven publish.
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import { afterEach, describe, expect } from "bun:test"
|
||||
import { ConfigProvider, Effect, Layer } from "effect"
|
||||
import { ConfigProvider, Deferred, Effect, Layer } from "effect"
|
||||
import type * as Scope from "effect/Scope"
|
||||
import { HttpRouter } from "effect/unstable/http"
|
||||
import { ChildProcessSpawner } from "effect/unstable/process"
|
||||
@@ -22,7 +22,7 @@ import { TestLLMServer } from "../lib/llm-server"
|
||||
import path from "path"
|
||||
import { resetDatabase } from "../fixture/db"
|
||||
import { disposeAllInstances, TestInstance, tmpdirScoped } from "../fixture/fixture"
|
||||
import { testEffect } from "../lib/effect"
|
||||
import { awaitWithTimeout, testEffect } from "../lib/effect"
|
||||
|
||||
const noopBootstrap = Layer.succeed(InstanceBootstrap.Service, InstanceBootstrap.Service.of({ run: Effect.void }))
|
||||
const it = testEffect(
|
||||
@@ -671,6 +671,70 @@ describe("HttpApi SDK", () => {
|
||||
),
|
||||
)
|
||||
|
||||
// Regression: SyncEvent must publish on the same ProjectBus the /event handler
|
||||
// subscribes to, AND the /event stream must forward handler ALS/context into the
|
||||
// body-pump fiber. Drives the full SDK → /event → Session.updatePart → sync.run →
|
||||
// bus.publish → SDK subscriber path. Goes red if either the publisher uses a
|
||||
// different bus instance (Bug 2 / pre-#27825) or the stream loses context (Bug 1 /
|
||||
// pre-#27425).
|
||||
serverPathParity("streams sync-backed part updates to /event subscribers", (serverPath) =>
|
||||
withStandardProject(serverPath, ({ sdk, directory }) =>
|
||||
Effect.gen(function* () {
|
||||
const session = yield* capture(() => sdk.session.create({ title: "sync-backed part event" }))
|
||||
const sessionID = String(record(session.data).id)
|
||||
const seeded = yield* seedMessage(directory, sessionID)
|
||||
|
||||
const controller = new AbortController()
|
||||
yield* Effect.addFinalizer(() => Effect.sync(() => controller.abort()))
|
||||
const events = yield* call(() => sdk.event.subscribe(undefined, { signal: controller.signal }))
|
||||
yield* Effect.addFinalizer(() =>
|
||||
call(async () => void (await events.stream.return?.(undefined))).pipe(Effect.ignore),
|
||||
)
|
||||
|
||||
const ready = yield* Deferred.make<void>()
|
||||
const received = yield* Deferred.make<unknown>()
|
||||
|
||||
yield* call(async () => {
|
||||
for await (const event of events.stream) {
|
||||
const payload = record(event).payload ?? event
|
||||
const type = record(payload).type
|
||||
if (type === "server.connected") {
|
||||
Deferred.doneUnsafe(ready, Effect.void)
|
||||
continue
|
||||
}
|
||||
if (type === MessageV2.Event.PartUpdated.type) {
|
||||
Deferred.doneUnsafe(received, Effect.succeed(payload))
|
||||
return
|
||||
}
|
||||
}
|
||||
}).pipe(Effect.forkScoped)
|
||||
|
||||
yield* awaitWithTimeout(Deferred.await(ready), "timed out waiting for /event server.connected", "2 seconds")
|
||||
|
||||
const updated = yield* capture(() =>
|
||||
sdk.part.update({
|
||||
sessionID,
|
||||
messageID: seeded.message.id,
|
||||
partID: seeded.part.id,
|
||||
part: { ...seeded.part, text: "updated via sync" } as NonNullable<
|
||||
Parameters<Sdk["part"]["update"]>[0]["part"]
|
||||
>,
|
||||
}),
|
||||
)
|
||||
expect(updated.status).toBe(200)
|
||||
|
||||
const event = yield* awaitWithTimeout(
|
||||
Deferred.await(received),
|
||||
"timed out waiting for message.part.updated bus payload over /event",
|
||||
"5 seconds",
|
||||
)
|
||||
const properties = record(record(event).properties)
|
||||
expect(record(properties.part)).toMatchObject({ id: seeded.part.id, type: "text" })
|
||||
return { type: record(event).type, partType: record(properties.part).type }
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
serverPathParity("matches generated SDK prompt no-reply routes", (serverPath) =>
|
||||
withStandardProject(serverPath, ({ sdk }) =>
|
||||
Effect.gen(function* () {
|
||||
|
||||
Reference in New Issue
Block a user