docs(session): define broker SDK contract - #895
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryThis PR defines the forward-looking contract and phased release gates for extracting the session broker into a reusable, authenticated, one-daemon-per-application SDK.
Confidence Score: 4/5The protocol contract should be corrected before implementation because its replay-window range cannot be enforced with the specified duplicate-tracking state. The document requires a 64-bit bitmap to track an inclusive 128-value acceptance range, forcing conforming implementations to disagree about valid requests or lose replay detection. Files Needing Attention: docs/session-broker-sdk.md Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Host[Application host] --> Supervisor[Session broker host and supervisor]
Supervisor --> Daemon[One daemon for immutable appId]
ProducerA[Session producer A] --> Daemon
ProducerB[Session producer B] --> Daemon
Caller[Authenticated caller] --> Daemon
Daemon --> Core[Broker core: identity, routing, limits, state]
Daemon --> Adapter{Runtime adapter}
Adapter --> Node[Node HTTP and WebSocket]
Adapter --> Bun[Bun HTTP and WebSocket]
Daemon -. application-owned commands and schemas .-> Hunk[Hunk review semantics]
Hunk -. separate capability .-> Browser[Browser review protocol]
Prompt To Fix All With AI### Issue 1
docs/session-broker-sdk.md:310-314
**Replay window exceeds bitmap**
When concurrent requests arrive out of order, the contract accepts sequences across the inclusive 128-value range from `highest - 63` through `highest + 64` while specifying only a 64-bit seen bitmap, causing implementations either to reject requests described as valid or lose duplicate-detection state and accept replayed requests.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "docs(session): define broker SDK contrac..." | Re-trigger Greptile |
| Caller sessions start at sequence string `"1"` and use a 64-sequence sliding replay window so up | ||
| to 32 concurrent HTTP requests may arrive out of order. The daemon tracks the highest accepted | ||
| uint64 value plus a 64-bit seen bitmap. It | ||
| accepts an unseen sequence from `highest - 63` through `highest + 64`, advances and shifts the | ||
| window for a new high value, and rejects duplicates, older values, or larger forward jumps. Caller |
There was a problem hiding this comment.
When concurrent requests arrive out of order, the contract accepts sequences across the inclusive 128-value range from highest - 63 through highest + 64 while specifying only a 64-bit seen bitmap, causing implementations either to reject requests described as valid or lose duplicate-detection state and accept replayed requests.
Knowledge Base Used: Session protocols and clients
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/session-broker-sdk.md
Line: 310-314
Comment:
**Replay window exceeds bitmap**
When concurrent requests arrive out of order, the contract accepts sequences across the inclusive 128-value range from `highest - 63` through `highest + 64` while specifying only a 64-bit seen bitmap, causing implementations either to reject requests described as valid or lose duplicate-detection state and accept replayed requests.
**Knowledge Base Used:** [Session protocols and clients](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/session-protocols-and-clients.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Good catch—the original wording was ambiguous. The 64-bit design is sufficient because the bitmap stores only accepted sequences from highest - 63 through highest; values above highest have never been accepted and need no replay bit until they advance and shift the window. I clarified that algorithm in cc24a43 and also specified sequence-zero reservation, initial state, atomic admission, safe uint64 arithmetic, delta-64 clearing, and exhaustion behavior.
Responded by Pi using gpt-5.6-sol.
This comment was generated by Pi using GPT-5.6 Sol
There was a problem hiding this comment.
Agreed. With highest tracking the greatest admitted sequence, the bitmap only needs to record accepted values at or below it; future values cannot be replays until they advance the high-water mark. The clarified delta handling, including clearing rather than shifting by 64, atomic admission, uint64-safe comparisons, sequence-zero reservation, and opening a new caller session before exhaustion, resolves the finding.
Summary
@hunk/session-brokerpackage whose root resolves Node or Bun automatically through conditional exportsWhy
The generic broker workspaces already contain reusable routing and lifecycle machinery, but later extraction phases would otherwise have to independently decide security, compatibility, discovery, delivery, and packaging behavior. This contract gives those phases one reviewed target while keeping Hunk review semantics application-owned.
The useful core/daemon/host/runtime boundaries remain internal modules. Consumers install one package and call one portable API; the package export map selects the Bun or Node server implementation without runtime-specific imports.
Important boundaries
@hunkscope and trusted-publisher verificationValidation
bun test packages/session-broker-core packages/session-broker packages/session-broker-bun packages/session-broker-node— 44 passedbun run typecheckbun run deps:check— 360 modules / 1,454 dependencies, no violationsbun run check:docsoxfmt --checkfor changed Markdown filesgit diff --checkTested on Linux x86_64 with Bun 1.3.10 and Node 24.14.1 installed. The contract selects Bun 1.3.14 as the future public floor; clean-package attestation on that exact version remains a release gate.
This PR description was generated by Pi using gpt-5.6-sol