Skip to content

Model requests as first-class Invocations; merge the dual Session classes - #17

Merged
sergiobayona merged 5 commits into
mainfrom
refactor/invocation-core-model
Jul 19, 2026
Merged

sergiobayona merged 5 commits into
mainfrom
refactor/invocation-core-model

Conversation

@sergiobayona

Copy link
Copy Markdown
Owner

Summary

This branch fixes the core modeling flaw in the session/request layer: request-scoped data (headers, params, resolved auth) lived as mutable state on the long-lived Session, and the session itself was split across two structurally confusable classes. Five commits, each independently green:

  1. Phase 0 — Characterization. A deterministic spec proving the request-identity race (a POST on a shared session could be authenticated with a concurrent request's headers), contract specs pinning exactly what tool/resource handlers observe, and removal of the dormant session-fixation path in BaseSessionManager.
  2. Phase 1 — One normalization boundary. RequestContext.coerce replaces five duplicated request-normalization layers (security middleware, ApiKey rack-env sniffing, JwtToken dual-key fallbacks, twin normalize_headers/normalize_params). Auth strategies consume RequestContext natively; custom auth handlers keep working via hash-style access ([], dig, key?).
  3. Phase 2 — The race fix. New VectorMCP::Invocation: a per-request view of a session carrying its own RequestContext and resolved SessionContext, delegating session-lived state. The transport builds one per POST and never mutates the session with request data again. The Phase 0 race spec flips red → green with the handler contract untouched.
  4. Phase 3 — One Session class. Both transport wrapper Structs are deleted; VectorMCP::Session carries lifecycle state directly, streaming-connection state moves to a metadata-backed SessionStreaming module, and all .context unwrapping, respond_to? guards, and the "never pass the wrapper struct" contract comment are gone.
  5. Phase 4 (deprecation half, targets 0.6.0). Resource handlers now receive the invocation (compatibly — it forwards the full SessionContext auth API); the six request-scoped Session methods emit deprecation warnings via VectorMCP.deprecation_warning; handle_message's session_id argument is documented deprecated. Removals land in 0.7 after a release cycle.

Security

  • Fixes a request-identity race: concurrent POSTs on one session could authenticate against each other's headers. Request state is now structurally per-request, not guarded-per-callsite.
  • Hardens BaseSessionManager against session fixation (never creates a session from a client-supplied ID), matching existing HTTP-stream behavior.

Compatibility

  • Tool handlers, on_request/on_notification hooks: unchanged (invocation quacks like Session).
  • Resource handlers: second argument is now the invocation; existing handlers calling user/authenticated?/can? etc. keep working (to_h is the one non-forwarded method, noted in CHANGELOG).
  • Custom auth blocks: unchanged (request[:headers] works on RequestContext).
  • Auth requests no longer carry the undocumented :session_id key.

Testing

  • rake green: 1429 examples, 0 failures (unit + integration + examples), RuboCop clean.
  • New: race characterization spec, handler contract specs, Invocation unit specs, deprecation-warning specs.

…ign base session manager

- Add characterization spec (pending) proving a POST can be authenticated
  with a concurrent request's headers (shared mutable request_context on
  the session). Drives the Invocation refactor; flips to green in Phase 2.
- Pin the handler-facing session contract (tool and resource handler
  arguments) so the refactor can prove user-visible behavior is unchanged.
- Align BaseSessionManager#get_or_create_session with the HttpStream
  override: never create a session from a client-supplied ID.
Authentication strategies now consume VectorMCP::RequestContext directly.
Legacy request shapes (raw Rack env, {headers:, params:} hashes with
symbol or string keys) are coerced once, via RequestContext.coerce —
the only remaining converter — at each auth entry point.

Deleted the duplicated normalization layers:
- Security::Middleware#normalize_request / #extract_request_data /
  #extract_from_rack_env (duplicated RequestContext.from_rack_env)
- ApiKey's private rack-env sniffing and Util-delegating wrappers
- JwtToken's request[:headers] || request["headers"] fallbacks
- RequestContext's byte-identical normalize_headers/normalize_params
  (collapsed to one normalize_string_map)

RequestContext gains hash-style read access ([], dig, key?/has_key?)
so existing :custom auth handlers written against the legacy request
hash keep working unchanged. The transport OAuth gate and the in-handler
auth path now construct the same type, removing the hand-synchronized
contract between them. The auth request no longer carries :session_id
(undocumented; use the session in handlers instead).
Fixes the request-identity race: request-scoped state (headers, params,
resolved auth) previously lived as mutable state on the shared session,
so concurrent POSTs on one session could authenticate against each
other's headers. The Phase 0 characterization spec now passes.

- Add VectorMCP::Invocation: a per-request view of a Session that
  carries its own RequestContext and Security::SessionContext while
  delegating session-lived state (id, data, initialize!, sample, ...).
  It quacks like a Session, so tool handlers and on_request hooks are
  unaffected; invocations of one session compare equal for subscriber
  dedup.
- HttpStream builds one Invocation per POST at dispatch time and no
  longer writes request data onto the session: the mutation sites in
  resolve_session_for_post and SessionManager#get_or_create_session are
  gone. Sessions keep their creation-time request_context.
- RequestContext#merge extracted from Session#update_request_context
  and shared with Invocation.
…CP::Session

The transport previously wrapped every VectorMCP::Session in a second
Struct also named Session (defined once in BaseSessionManager and
re-declared verbatim in HttpStream::SessionManager), reachable only via
a field literally named .context. Handlers needed respond_to? guards and
a contract comment to police which of the two confusable types they got.

- VectorMCP::Session now carries its transport lifecycle directly:
  created_at, last_accessed_at, touch!, expired?(timeout), age, and a
  transport-owned metadata hash.
- HTTP streaming connection state lives in SessionManager::SessionStreaming,
  a metadata-backed module the manager extends onto each new session.
- Both wrapper Structs are deleted; session managers store and return
  VectorMCP::Session directly. All .context unwrapping is gone.
- Server#handle_message's session_id parameter is now optional (defaults
  to session.id; it was only ever a log prefix).
- Specs that constructed wrapper structs now build real sessions.
…n request API

Ships the 0.6.0 side of the public-API convergence; removals follow in 0.7.

- Resource handlers' second argument is now the per-request Invocation.
  Existing handlers keep working: Invocation forwards the full
  Security::SessionContext auth API (user, authenticated?, can?,
  user_identifier, ...) to the request's resolved security context, and
  handlers additionally gain session identity and request data.
- The six request-scoped Session methods (request_context=,
  update_request_context, request_headers?, request_params?,
  request_header, request_param) now emit deprecation warnings via the
  new VectorMCP.deprecation_warning helper and are tagged @deprecated
  for removal in 0.7.
- Server#handle_message's session_id argument is documented deprecated;
  the HTTP transport no longer passes it.
- Dropped the now-unconditional respond_to?(:security_context=) guard.
- CHANGELOG: added Unreleased section covering the whole refactor arc.
@sergiobayona
sergiobayona merged commit 16ae85f into main Jul 19, 2026
8 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.

1 participant