Skip to content

[codex] fix(graphql): bind live chat service at runtime#511

Merged
penso merged 2 commits intomainfrom
nosy-hoverfly
Mar 28, 2026
Merged

[codex] fix(graphql): bind live chat service at runtime#511
penso merged 2 commits intomainfrom
nosy-hoverfly

Conversation

@penso
Copy link
Copy Markdown
Collaborator

@penso penso commented Mar 28, 2026

Summary

  • fix GraphQL chat binding by resolving the chat service through GatewayState::chat() at request time instead of using the frozen services.chat slot
  • add a GraphQL-specific schema builder path that injects a late-bound chat proxy while leaving the rest of the service bundle unchanged
  • add an HTTP regression test that builds the schema first, installs chat_override afterwards, and proves GraphQL chat.send and sessions.active hit the live runtime service
  • root cause: GraphQL cloned the static services.chat pointer before LiveChatService was attached, while RPC and WebSocket methods already resolved chat through the late-bound override
  • fixes fix(graphql): use late-bound live chat service instead of static services bundle #494

Validation

Completed

  • cargo +nightly-2025-11-30 test -p moltis-httpd --test graphql_chat_binding
  • just format
  • cargo +nightly-2025-11-30 fmt --all -- --check

Remaining

  • just lint (started, but stopped after roughly 5 minutes to honor the repo command timeout rule; full workspace clippy status is still pending)

Manual QA

  • Start the gateway with GraphQL enabled.
  • Install the live chat override after the GraphQL schema has already been built.
  • Verify GraphQL chat.send and sessions.active match RPC/WebSocket behavior on the same runtime state instead of falling back to the noop chat service.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 28, 2026

Merging this PR will improve performance by 18.75%

⚡ 1 improved benchmark
✅ 38 untouched benchmarks
⏩ 5 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
env_substitution 12.1 µs 10.2 µs +18.75%

Comparing nosy-hoverfly (1a4a415) with main (e77c74d)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@penso penso marked this pull request as ready for review March 28, 2026 17:55
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 28, 2026

Codecov Report

❌ Patch coverage is 40.38462% with 31 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
crates/httpd/src/graphql_routes.rs 33.33% 28 Missing ⚠️
crates/gateway/src/services.rs 66.66% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR fixes a late-binding bug where GraphQL was holding a frozen Arc<NoopChatService> snapshot because the schema was constructed before LiveChatService was installed, while RPC and WebSocket paths already resolved chat through GatewayState::chat() at call time.

Key changes:

  • Adds GraphqlChatServiceProxy — a thin ChatService impl that calls GatewayState::chat() on every method invocation, mirroring the existing pattern used by GatewaySystemInfoService for late-bound state.
  • Extracts build_graphql_schema into graphql_routes.rs so the two identical schema-build blocks in server.rs collapse to a single call.
  • Adds to_services_with_chat to GatewayServices so the proxy can inject a custom chat Arc while leaving all other services unchanged; to_services now delegates to it.
  • Adds a focused HTTP integration test (graphql_chat_binding) that builds the schema first, installs a RecordingChatService override afterwards, and asserts that both chat.send and sessions.active reach the live override.

Minor note: After this refactor to_services has no remaining call sites in the workspace — it is a pub method with no caller. It can be removed or documented for future use (see inline comment).

Confidence Score: 5/5

Safe to merge; the proxy pattern is correct, all 16 ChatService methods are forwarded, and the integration test validates the targeted regression.

The only finding is a P2 style observation about a now-unused pub shim (to_services). No logic errors, no missing methods, no correctness or data-integrity concerns.

crates/gateway/src/services.rs — to_services is now dead code and can be cleaned up.

Important Files Changed

Filename Overview
crates/gateway/src/services.rs Adds to_services_with_chat so callers can inject a custom chat service; to_services now delegates to it. After this refactor to_services has no remaining call sites in the workspace.
crates/httpd/src/graphql_routes.rs Introduces GraphqlChatServiceProxy (resolves GatewayState::chat() at call time) and build_graphql_schema helper; all 16 ChatService methods are forwarded correctly.
crates/httpd/src/server.rs Replaces the two duplicated inline schema-build blocks with a call to build_graphql_schema; no other logic changes.
crates/httpd/tests/graphql_chat_binding.rs New integration test that builds the schema first, installs a RecordingChatService override afterwards, and asserts GraphQL mutations/queries reach the live override; TempDir is returned and dropped naturally at end of test scope.

Sequence Diagram

sequenceDiagram
    participant Client
    participant GraphQL Schema
    participant GraphqlChatServiceProxy
    participant GatewayState
    participant LiveChatService

    Note over GraphQL Schema, GraphqlChatServiceProxy: Schema built once at startup<br/>(proxy holds Arc<GatewayState>)

    Client->>GraphQL Schema: POST /graphql (mutation chat.send)
    GraphQL Schema->>GraphqlChatServiceProxy: send(params)
    GraphqlChatServiceProxy->>GatewayState: chat() — read lock
    GatewayState-->>GraphqlChatServiceProxy: Arc<LiveChatService> (override)
    GraphqlChatServiceProxy->>LiveChatService: send(params)
    LiveChatService-->>GraphqlChatServiceProxy: ServiceResult
    GraphqlChatServiceProxy-->>GraphQL Schema: ServiceResult
    GraphQL Schema-->>Client: { data: { chat: { send: ... } } }

    Note over GatewayState: Before fix: schema held a frozen<br/>Arc<NoopChatService> snapshot
Loading

Reviews (2): Last reviewed commit: "fix(graphql): address review feedback" | Re-trigger Greptile

@penso penso merged commit 050c2ed into main Mar 28, 2026
14 of 24 checks passed
@penso penso deleted the nosy-hoverfly branch March 28, 2026 22:29
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.

fix(graphql): use late-bound live chat service instead of static services bundle

1 participant