fix(server): bound slow-client event buffers - #9715

💭 I'm reviewing the changed files—LiveStreamBudget.ts, ThreadLiveEventCoalescer.ts, and ws.ts—checking that their import conventions match the codebase's standards for referencing shared contract packages. I'm verifying that using the existing OrchestrationGetSnapshotError contract error is appropriate, and confirming the diagnostics-ignore comment for Stream.fromPull is properly justified. I'm also checking that Effect.catchTags is used consistently across the files rather than catchTag. Now I'm considering whether LiveStreamBudget.ts follows the module convention correctly—it exports a makeLiveStreamBudget factory returning a plain object rather than a Context.Service tag, so I'm comparing it against the sibling ThreadLiveEventCoalescer.ts module to see if this pattern is consistent across the codebase. I notice the new import in ws.ts was inserted out of alphabetical order relative to the existing imports, but since ordering isn't called out in the conventions and lint presumably passes, I'll treat this as a minor style issue rather than a real finding. I want to look at the actual files directly to confirm my judgment, particularly the OrchestrationGetSnapshotError definition and whether its message is derived from attributes.

1. browse_code — Show the definition of OrchestrationGetSnapshotError in @t3tools/contracts (full class definition, fields, Schema.TaggedError usage).

2. browse_code — apps/server/src/orchestration/ThreadLiveEventCoalescer.ts

💭 I'm reviewing the LiveStreamBudget.ts file for convention adherence — checking how it constructs errors, reusing the contract error type for buffer overflow, and confirming the Effect.fn naming pattern matches expectations. The eslint-disable-next-line directive has an adjacent explanation comment about Stream.fromPull needing the pull effect as its result, which satisfies the rule requiring justification for suppressed diagnostics. I'm also checking whether yielding a tagged error instance works correctly as a YieldableError in Effect 4, and flagging a module-level WeakMap used for memoization — it's not really a hidden service dependency, but it is shared mutable state across calls that's worth noting against the "no module globals" convention. I'm now moving to review ws.ts, checking the newly added import placement, and examining patterns like catchTag usage, Queue.orDie, and closure/finalizer handling for any convention violations.

3. browse_code — apps/server/src/orchestration/LiveStreamBudget.ts

4. browse_code — apps/server/src/ws.ts

💭 I'm checking conventions one by one: the raceFirst usage in ws.ts looks fine, and the LiveStreamBudget error reuses an existing contract error rather than introducing a new one, so that's not a violation either. Now I'm looking at the import statements to see if anything else stands out. I keep scanning for module-global dependency hiding and other structural issues, but nothing new turns up — the diagnostic-disable directive has a proper explanation, and there don't seem to be other new suppressions in the diff. So far it looks like there are no real findings, but I want to verify the imports in ws.ts once more before concluding.

5. browse_code — apps/server/src/ws.ts

💭 I'm checking whether LiveStreamBudget fits the namespace-import convention meant for service boundaries, but since it's a factory module like ThreadLiveEventCoalescer with named exports rather than a Foo-style service, it seems more consistent to keep using named imports here too. Now I'm scanning for genuine convention violations that might be planted: the module-level serializedSizes WeakMap is shared mutable state but appears to be pure identity-keyed memoization rather than a hidden service dependency, so it's borderline at best. Other candidates like the missing as const or the deliver mutation of source.pull seem like stylistic or behavioral concerns rather than actual convention breaches, and there's no layer since this isn't a service module. Checking further, the reused RPC error class with just message+cause can't carry structured attributes without breaking contract, and the warning log already carries the structured detail separately, so that's not a real finding either. Log annotations look bounded, the Effect.logWarning placement after Deferred.fail seems fine, and both the diagnostics-off directive and the test additions look properly justified — nothing stands out as a clear violation yet, so I'm looking at the Effect.catchTags usage next. Since that stream error channel is fully handled by catchTags, it fits the convention allowing broad error handling there. I'm also checking the queue-clear orDie call and the closure-scoped boolean flag in the RPC handler, but neither seems to violate anything, and since lint presumably passed already, import ordering isn't an issue either — so I'm leaning toward everything being clear, though I want to verify one more requirement before concluding.