Skip to content

fix(mcp): address review round 1 on the stateless facade - #2

Merged
octobroker[bot] merged 1 commit into
fix/facade-statelessfrom
fix/facade-stateless-review1
Oct 26, 2026
Merged

octobroker[bot] merged 1 commit into
fix/facade-statelessfrom
fix/facade-stateless-review1

Conversation

@octobroker

@octobroker octobroker Bot commented Oct 26, 2026

Copy link
Copy Markdown

Staging PR for the owner's review. Base is fix/facade-stateless (the head of openabdev#1578); once approved, fix/facade-stateless is fast-forwarded to this commit, which updates openabdev#1578. Do not merge here.

Addresses round 1 of the openabdev#1578 review (F1–F4) plus two of the earlier non-blocking notes:

  • F1 — native/gmail.rs: the serve_http doc no longer claims to mirror the facade transport; it says gmail is still stateful (same idle/restart 404 exposure) and that the reqwest::Client and refresh lock must be shared outside the per-session factory before it can go stateless. GmailNative doc: one instance per stdio serve process, or per MCP session over HTTP.
  • F2 — the 2026-07-28 claim is scoped to the removal of protocol-level sessions (the facade still speaks the 2025 initialize handshake), in both the build_router doc and docs/oab-mcp-facade.md.
  • F3 — ADR §6.2 gains a "Stateless transport" bullet (gmail remains stateful pending unification); the operator guide states that a stale Mcp-Session-Id is ignored rather than rejected; the Trust model section now says "agent session" for the bearer-backed session, with a one-line note that it is unrelated to the MCP protocol session; the e2e test doc says "session-token (bearer) mechanism".
  • F4 — the regression test checks the initialize body (result + protocolVersion, no error) and asserts DELETE /mcp → 405 alongside GET, both with Allow: POST.
  • Earlier non-blocking notes: the constraint that future facade tools must not rely on server→client requests or the GET SSE stream is documented (doc comment + operator guide), and a comment explains why NeverSessionManager is intentionally redundant with with_stateful_mode(false).

Not in this round (reviewer marked them follow-ups / non-blocking): gmail stateless switch, idempotency keys, cancellation-token propagation, json_response, bearer-miss debug logging, cargo audit, tense alignment in the two "is being removed" docs.

🤖 Generated with Claude Code

- F1: gmail adapter doc no longer claims to mirror the facade transport; it
  states that gmail is still stateful (same idle/restart 404 exposure) and what
  must be shared before it can go stateless. Also correct "one instance per
  serve process" for HTTP mode.
- F2: scope the 2026-07-28 claim to the removal of protocol-level sessions (the
  facade still speaks the 2025 initialize handshake), in the build_router doc
  and docs/oab-mcp-facade.md.
- F3: ADR 6.2 gains a "Stateless transport" bullet (gmail remains stateful);
  the operator guide states that a stale Mcp-Session-Id is ignored rather than
  rejected, and the Trust model section now says "agent session" for the
  bearer-backed session so it is not confused with the MCP protocol session;
  the e2e test doc says "session-token (bearer) mechanism".
- F4: the regression test checks the initialize body (result + protocolVersion,
  no error) and asserts DELETE /mcp -> 405 alongside GET, both with Allow: POST.
- From the earlier non-blocking notes: document that future facade tools must
  not rely on server->client requests or the GET SSE stream, and note why
  NeverSessionManager is intentionally redundant with stateful_mode = false.

Agent-Bot: alpha
Co-authored-by: rockexe0000 <rockexe0000@gmail.com>
@octobroker
octobroker Bot merged commit 174e566 into fix/facade-stateless Oct 26, 2026
38 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants