Add a silent namespaced extension bus#69
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 867fce790a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tempPath = `${statePath}.tmp.${process.pid}.${randomUUID()}`; | ||
|
|
||
| try { | ||
| writeFileSync(tempPath, JSON.stringify(envelope), { mode: 0o600 }); |
There was a problem hiding this comment.
[DEFERRED] Every state commit performs synchronous write/fsync/rename/fsync on the broker event loop. Normal consumers commit rarely, but the API permits a same-user owner to stall all routing. Choosing async queued durability versus explicit commit throttling is a maintainer-level design decision.
| return state; | ||
| } | ||
|
|
||
| commitState(namespace: string, expectedRevision: number, payload: unknown): StateCommitResult { |
There was a problem hiding this comment.
[DEFERRED] Persisted namespaces and the in-memory state cache have no total cap or reclamation policy, allowing unbounded growth across short-lived namespace names. A global cap, TTL, or explicit deletion policy needs an upstream lifecycle decision.
| } | ||
|
|
||
| // Sort by startedAt, then sessionId | ||
| candidates.sort((a, b) => { |
There was a problem hiding this comment.
[DEFERRED] Full owner recomputation scans every namespace against every session and capability on connect/disconnect/update. The configured maxima permit a large worst case; deciding whether those bounds are acceptable or an incremental namespace index is required affects the broker design broadly.
|
|
||
| const isCapable = recipientSession.extensions.some((ext) => ext.namespace === namespace); | ||
| if (isCapable) { | ||
| writeMessage(recipientSession.socket, { |
There was a problem hiding this comment.
[DEFERRED] A 64 KiB state commit is broadcast to every capable socket without a byte-aware backpressure policy, so slow consumers can accumulate significant buffered output. A bounded/coalesced queue versus disconnect-on-backlog policy is a broader broker transport decision.
| owner?: IntercomExtensionOwner; | ||
| state?: IntercomExtensionState; | ||
| }; | ||
| publish(payload: unknown, options?: { audience?: "owner" | "capable"; ownerOnly?: boolean }): void; |
There was a problem hiding this comment.
[DEFERRED] publish() is fire-and-forget even though the broker may reject stale authority, payload size, or capability, and generic client errors are intentionally hidden from the TUI. Adding request IDs and Promise/result semantics would materially expand the protocol and needs maintainer direction.
Summary
Validation
npm test— 94 tests passnpm pack --dry-runCompatibility
Older clients ignore the additive broker feature list. New clients detect older brokers and do not send extension operations.