feat(floating-chat): global floating chat popover with full /chat parity#38
Merged
Conversation
…lity Create ChatSessionProvider with 4-context re-render isolation: - ChatSessionDataContext: runs, selectedRun, providers, pageContext - ChatSessionActionsContext: stable callbacks (createSession, selectRun, etc.) - ChatSessionConnectionContext: connectionState, sessionStatus - ChatSessionStreamContext: messages, events, activity Lazy SSE stream: starts only on first FAB click via onFirstOpenRef callback. Users who never open chat pay zero SSE cost. createSession mutex: prevents duplicate session creation. selectedRunId seeded from localStorage with regex validation. providers setter uses bail-out comparison to prevent unnecessary re-renders. Auto-bootstrap gated behind streamEnabled. Also adds lib/acp/fetch.ts: standalone ACP fetcher that re-derives acpBase and requestCredentials per call (not at module eval time).
…stem prefix
POST /acp/sessions/{id}/prompt now accepts optional pageContext field:
{ prompt: string, pageContext?: { route, entityType?, entityId? } }
When pageContext is present, server assembles a compact context prefix
and prepends it to the prompt before forwarding to the LLM:
[context: page={route}]
[context: page={route} entity={type}/{id}]
Token safeguards:
- Off by default: absent pageContext = zero injection, zero token cost
- Server-side assembly only: client sends structured data
- Hard token budget: 30 tokens (~120 chars); truncates entityId first,
then entityType, never route
- No verbose markdown: single compact line
- NFKC-safe strip: chars filtered to [a-zA-Z0-9/_-], max 32 chars/field
- Deny-list: system, ignore, override, admin, instruction, assistant,
prompt — any match skips injection, never errors the request
- Logged: every injection emits surface=api, session_id, route,
estimated_tokens at INFO level
Validation failure silently skips injection — never errors the request.
FloatingChatFab: - Fixed pill (bottom-right, z-40), rounded-full Aurora-styled - Cmd/Ctrl+/ hotkey toggle, skips inputs and contentEditable - CSS-hidden (visibility:hidden) on /chat route — NOT unmounted - Ambient connection indicator (connecting = pulsing ring, error = amber dot) - Streaming pulse animation (respects prefers-reduced-motion) - Modal stack guard via openModals RefObject FloatingChatPopover: - Default 420x600, min 320x420, max 800x900 - Drag via DOM ref + rAF (no React state during drag), pointer capture - Resize via bottom-right corner handle with pointer capture - Viewport hard-clamp on drag commit and window resize (debounced 100ms) - Gear config panel: 4 toggles (persistOpen, persistPosition, persistSize, sendPageContext) — persistence via labby:floating-chat:state localStorage key - Accessibility: role=dialog, aria-modal, aria-labelledby, tabIndex=-1, focus trap, Escape closes, focus returns, HTML inert on app root - Mobile: renders as full-screen Sheet (< 768px) - CSS-hidden (not unmounted) on /chat route for both FAB and popover AdminLayoutClient: - 'use client' wrapper component preserving server component layout.tsx - Hosts ChatSessionProvider, FAB, Popover - Wires onFirstOpenRef for lazy SSE stream activation - Persists open state to localStorage Layout updated to use AdminLayoutClient.
FloatingChatShell consumes all 4 ChatSession contexts:
- Data: runs, selectedRun, agents, projects, pageContext
- Actions: createSession, selectRun, refreshSessions
- Connection: connectionState (for future connection indicator)
- Stream: messages (subscribed via StreamContext only)
React.memo wrapper: shell only re-renders when messages changes
during streaming — header/input don't re-render on token stream.
sendPrompt wires pageContext: when config.sendPageContext === true
AND pageContext !== null, includes pageContext as separate field in
request body: { prompt, pageContext: { route, entityType?, entityId? } }
Lazy mount pattern in AdminLayoutClient:
- shellMounted state starts false
- Set to true on first FAB click (alongside stream enable)
- FloatingChatShell mounts permanently after first click
- Popover container controls visibility via CSS, not mount state
Adds FloatingChatSection to /design-system sandbox with: - FAB states: default, with-badge (count=3), active/open, connecting (pulsing ring), error (amber dot) - Popover states: default, gear config open (4 toggles), compact (min size) - Persistence schema: localStorage key + shape as code block - Hotkey reference: ⌘/ (Ctrl+/ on Windows) with KbdGroup/Kbd primitives - Mobile Sheet note: < 768px uses bottom Sheet, no drag/resize All demos use local fake state — no live backend required. Placed after CommandPaletteSection in Application Patterns group.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughIntroduces a comprehensive floating chat UI system for the gateway admin, including draggable and resizable popover components, chat session management with SSE streaming, persistent UI state, page context synchronization, and backend enhancements for structured page context parameter handling and session subscriber limits. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as FloatingChatFab
participant Pop as FloatingChatPopover
participant Shell as FloatingChatShell
participant Provider as ChatSessionProvider
participant SSE as SSE Stream
participant Backend as ACP Backend
User->>UI: Click FAB or Cmd+/
UI->>Pop: togglePopover (open=true)
Pop->>Pop: Persist state to localStorage
Note over Shell: Shell mounts within Provider
Shell->>Provider: Request session data (runs, selected run)
Provider-->>Shell: Return session state
User->>Shell: Enter prompt text
Shell->>Provider: sendPrompt(text) with pageContext
Provider->>Backend: POST /sessions/{runId}/prompt<br/>(text + page_context)
Backend->>Backend: Sanitize pageContext fields<br/>Inject context into prompt
Backend->>Backend: Execute prompt dispatch
Backend->>SSE: Create event stream
SSE-->>Provider: Stream events (role, content, status)
Provider->>Provider: Cache events in sessionEventCache
Provider->>Shell: Update transcript & message thread
Shell->>UI: Stream message appears in UI
Note over Pop: Popover maintains draggable position<br/>and resizable size in localStorage
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8c72c67f2
ℹ️ 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".
Comment on lines
+212
to
+215
| const appRoot = document.querySelector('#__next') as HTMLElement | null | ||
| if (appRoot) { | ||
| appRoot.inert = true | ||
| } |
There was a problem hiding this comment.
When the popover opens, this sets #__next to inert. Because the floating chat panel is rendered inside that same root container, the dialog and all of its controls become non-interactive along with the rest of the app, so users can’t type, click, or focus reliably while chat is open. This should inert only non-dialog siblings (or use an overlay/portal approach), not the root element containing the dialog.
Useful? React with 👍 / 👎.
Comment on lines
+133
to
+136
| const selectAgent = React.useCallback((_providerId: string) => { | ||
| // provider selection is handled by useChatSessionActions.selectAgent | ||
| // local agent selection just for visual state in this shell | ||
| }, []) |
There was a problem hiding this comment.
The floating shell passes a no-op handler to ChatInput for agent changes, so selecting a different agent in the picker never updates selectedProviderId. In multi-provider setups this means new sessions/prompts keep using the old provider despite the UI selection, which breaks the “/chat parity” behavior.
Useful? React with 👍 / 👎.
| const onFirstOpenRef = React.useRef<(() => void) | null>(null) | ||
| const hasOpenedOnce = React.useRef(false) | ||
| // State-based lazy mount for FloatingChatShell | ||
| const [shellMounted, setShellMounted] = React.useState(false) |
There was a problem hiding this comment.
The popover open state can restore to true from persisted storage, but shellMounted always starts false and is only set in handleToggle. On a reload with persisted-open enabled, users get an open popover without chat content/stream startup until they manually close and reopen it. shellMounted (and first-open bookkeeping) should be initialized from the restored open state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a global, always-available floating chat surface to gateway-admin that shares session state with the existing /chat experience, plus server-side support for optionally injecting compact page context into ACP prompts.
Changes:
- Introduces a shared
ChatSessionProvider(split into 4 contexts) and an ACP fetch utility used by both/chat-style surfaces and the new floating chat. - Adds a floating FAB + draggable/resizable popover and a shell that wires existing chat UI components into the popover.
- Extends the ACP prompt API to accept optional
pageContextand prepends a compact context prefix when valid.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
crates/lab/src/api/services/acp.rs |
Adds optional pageContext to prompt handling with sanitization + prefix assembly and logging. |
apps/gateway-admin/lib/acp/fetch.ts |
Adds a small ACP fetch helper to unify base URL/credentials/headers. |
apps/gateway-admin/lib/chat/chat-session-provider.tsx |
Introduces a shared provider for sessions + lazy SSE stream management and shared caches. |
apps/gateway-admin/components/admin-layout-client.tsx |
Wraps admin pages with the provider and mounts the FAB/popover/shell. |
apps/gateway-admin/components/floating-chat-fab.tsx |
Implements the fixed pill FAB with hotkey and modal-stack guard integration. |
apps/gateway-admin/components/floating-chat-popover.tsx |
Implements the draggable/resizable popover, persistence, mobile sheet, and focus trapping. |
apps/gateway-admin/components/floating-chat-shell.tsx |
Wires the provider contexts into existing chat UI for popover parity. |
apps/gateway-admin/components/design-system/floating-chat-section.tsx |
Adds a design-system sandbox section documenting/demonstrating the pattern. |
apps/gateway-admin/components/design-system/design-system-shell.tsx |
Registers the new Floating Chat section in the design system page. |
apps/gateway-admin/app/(admin)/layout.tsx |
Hosts the client wrapper in the admin layout without converting the layout to a client component. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+79
to
+85
| let route = sanitize_page_context_field(&ctx.route)?; | ||
|
|
||
| // Build prefix candidates, longest first, then trim to token budget | ||
| let prefix = match (&ctx.entity_type, &ctx.entity_id) { | ||
| (Some(et), Some(eid)) => { | ||
| let entity_type = sanitize_page_context_field(et)?; | ||
| let entity_id = sanitize_page_context_field(eid)?; |
Comment on lines
+201
to
+219
| // Assemble the effective prompt text: optional context prefix + user prompt. | ||
| // Validation failure silently skips injection — never errors the request. | ||
| let prompt_text = if let Some(ctx) = &body.page_context { | ||
| match assemble_page_context_prefix(&session_id, ctx) { | ||
| Some(prefix) => format!("{}\n\n{}", prefix, body.prompt.trim()), | ||
| None => { | ||
| tracing::warn!( | ||
| surface = "api", | ||
| service = "acp", | ||
| action = "session.prompt", | ||
| session_id = %session_id, | ||
| "page context validation failed — injecting without context", | ||
| ); | ||
| body.prompt.trim().to_string() | ||
| } | ||
| } | ||
| } else { | ||
| body.prompt.trim().to_string() | ||
| }; |
Comment on lines
+39
to
+44
| const [config, setConfig] = React.useState<PersistConfig>({ | ||
| persistOpen: true, | ||
| persistPosition: true, | ||
| persistSize: true, | ||
| sendPageContext: false, | ||
| }) |
Comment on lines
+599
to
+607
| for (const event of consumed.events) { | ||
| lastSeqRef.current = event.seq | ||
| sessionLastSeqCache.set(selectedRunId, event.seq) | ||
| setEvents((current) => { | ||
| const next = appendSessionEvent(current, event) | ||
| sessionEventCache.set(selectedRunId, next) | ||
| return next | ||
| }) | ||
| } |
Comment on lines
+61
to
+64
| // Deny-list check (case-insensitive) | ||
| let lower = stripped.to_lowercase(); | ||
| for denied in PAGE_CONTEXT_DENY_LIST { | ||
| if lower.contains(denied) { |
Comment on lines
+45
to
+50
| /// NFKC-normalizes (ASCII-safe: identity for ASCII input), strips to allowed characters, | ||
| /// truncates to 32 chars. | ||
| fn sanitize_page_context_field(value: &str) -> Option<String> { | ||
| // Strip to allowed characters (NFKC normalization is identity for our ASCII allow-list). | ||
| // For non-ASCII Unicode the filter naturally drops all non-ASCII chars, achieving | ||
| // the same safety goal as NFKC + strip without pulling in the unicode-normalization crate. |
Comment on lines
+211
to
+215
| // Inert the rest of the page | ||
| const appRoot = document.querySelector('#__next') as HTMLElement | null | ||
| if (appRoot) { | ||
| appRoot.inert = true | ||
| } |
Comment on lines
+133
to
+137
| const selectAgent = React.useCallback((_providerId: string) => { | ||
| // provider selection is handled by useChatSessionActions.selectAgent | ||
| // local agent selection just for visual state in this shell | ||
| }, []) | ||
|
|
- Wire setPageContext producer: add usePageContextSync hook and PageContextSync component, render inside ChatSessionProvider in AdminLayoutClient so every page propagates its route automatically - Fix focus-trap inert selector: replace Pages Router #__next selector with App Router compatible body-children iteration, inert all direct body children except the panel itself to avoid blocking the overlay - Wire selectAgent in FloatingChatShell: call useChatSessionActions() selectAgent directly instead of no-op local callback - Simplify: remove redundant selectAgent wrapper closure, pass context action directly; rename inerter -> toRestore for clarity; trim what-comments from new files, keep only non-obvious constraint notes Build: cargo build --workspace --all-features exit=0 Clippy: cargo clippy --workspace --all-features -D warnings exit=0
P1 (all 6 fixed): - lab-gych.6: SSE 401 for missing ticket; remove String::new() fallback - lab-gych.7: initialize shellMounted from same localStorage check as open - lab-gych.8: pure setState updater; move side effects to useEffect([open]) - lab-gych.9: move sanitize/assemble page_context to dispatch/acp/page_context.rs; HTTP handler is now a thin shim; CLI/MCP get page_context support for free - lab-gych.10: extract shared normalizers to lib/chat/acp-normalizers.ts; eliminate duplicate types/functions between chat-session-provider and controller - lab-gych.11: create lib/chat/session-event-cache.ts; both SSE consumers import from single source, remove "must stay in sync" comment P2 (all 9 fixed): - lab-gych.12: batch setEvents per SSE chunk (O(n) → single call); reverse-scan resolveSessionStatusFromEvents (O(1) in common case) - lab-gych.13: PROMPT_MAX_CHARS = 64_000 guard before processing - lab-gych.14: patchPersistedState() with in-memory cache; exported from popover - lab-gych.15: initialize config state from readPersistedState().config - lab-gych.16: set Content-Type: application/json when body present in fetchAcp - lab-gych.17: fix 4 type violations (timer | undefined, union narrowing, NonNullable<PageContext> field type) - lab-gych.18: add sessionsLoaded guard to auto-bootstrap effect; AbortController - lab-gych.19: remove "admin"/"prompt"/"system" from deny-list; keep only injection-specific terms; rely on character allowlist as primary safety - lab-gych.20: remove ghost SettingsPanel state + render (no backend wiring) P3 (4 of 5 addressed): - lab-gych.21: split on separators before deny-list check in page_context.rs - lab-gych.22: MAX_SUBSCRIBERS_PER_SESSION = 32 cap in registry.rs - lab-gych.28: remove React.memo from FloatingChatShell (no stable props) - lab-gych.29: skipped (triplication requires broader hook refactor) - lab-gych.26: skipped (PersistConfig move is cross-cutting; low risk/reward)
- lab-gych.6 list_sessions: add TODO(phase-2) comment noting principal filter deferred until bearer auth is wired in middleware layer - lab-gych.18: remove decorative AbortController (createSession does not accept a signal; guard was already a no-op); add NOTE(phase-2) for future wiring; sessionsLoaded guard still present as the actual fix
- Cross-validate selectedRunId against loaded runs in refreshSessions; stale server-deleted session IDs no longer block auto-bootstrap - Fix misleading comment on isCreatingRef mutex guard Resolves review thread PRRT_kwDOR8nC1M591X9O Resolves review thread PRRT_kwDOR8nC1M591YUu Resolves review thread PRRT_kwDOR8nC1M591YWB
…h.29) - lab-gych.23: resize now uses rAF + direct DOM writes during pointermove, commits to React state only on pointerup (mirrors drag pattern) - lab-gych.24: remove 4-state lazy-SSE protocol (streamEnabled, onFirstOpenRef, hasOpenedOnce, shellMounted); SSE opens whenever selectedRunId is non-null; FloatingChatShell always mounted inside popover - lab-gych.25: remove PersistConfig persist toggles (persistOpen, persistPosition, persistSize); always persist all state; rename PersistConfig -> ChatConfig; keep sendPageContext as standalone boolean; simplify gear panel - lab-gych.26: update all three importers (floating-chat-shell, admin-layout-client, design-system) to use ChatConfig (PersistConfig removal made type relocation moot) - lab-gych.27: add LRU eviction to session-event-cache keeping last 10 sessions; replace raw Map exports with helper objects; both maps evicted together per key - lab-gych.29: replace duplicate inline fetchAcp in use-chat-session-controller with createAcpFetcher() via stable ref; use fetchAcpRef.current in SSE effect in chat-session-provider (removes createAcpFetcher() call inside the effect)
jmagar
added a commit
that referenced
this pull request
May 2, 2026
* spike: validate rmcp AuthClient integration with StreamableHttpClientWorker
Task 0 (gating spike) for the upstream MCP OAuth PKCE plan. Confirms four
integration points against rmcp 1.4.0 before Task 2 commits to a design:
1. AuthClient<reqwest::Client> constructs cleanly over AuthorizationManager
+ InMemoryCredentialStore.
2. AuthClient auto-injects Authorization: Bearer <token> when the caller
passes auth_token: None — its StreamableHttpClient impl calls
auth_manager.get_access_token() and fills the slot before delegating.
3. rmcp does NOT automatically refresh on a 401 from the upstream.
AuthorizationManager::get_access_token() only refreshes on the local
clock (REFRESH_BUFFER_SECS = 30s). Refresh-on-401 is the caller's
responsibility, so Task 2 must layer it on.
4. Spike runs against a wiremock AS+RS stub by default, and against a
real OAuth-protected MCP upstream when SPIKE_REAL_AS_URL is set, so
the operator can validate end-to-end interactively before Task 2
starts.
Plan A (AuthClient as StreamableHttpClient) is confirmed; Plan B (custom
wrapper that calls get_access_token() pre-request) is strictly inferior
and not needed.
Findings duplicated inline at the top of the spike example AND in the
stub crates/lab/src/oauth/upstream/refresh.rs that Task 2 will replace.
The upstream/* files are intentionally NOT wired into oauth.rs yet —
they are exploratory docs.
* feat: add rmcp-backed upstream oauth manager with single-flight refresh and at-rest encryption
- store.rs: SqliteCredentialStore + SqliteStateStore implementing rmcp CredentialStore/StateStore traits
- ChaCha20-Poly1305 encryption at rest; decryption failure → AuthorizationRequired
- StateStore::load uses atomic DELETE…RETURNING (take_upstream_oauth_state); delete is no-op
- Two-lifetime pattern ('life0: 'async_trait, Self: 'async_trait) matching async_trait expansion
- refresh.rs: RefreshLocks (DashMap per-(upstream,subject) Mutex) + refresh_if_stale()
- Single-flight serialization prevents concurrent refresh storms
- AuthError::AuthorizationRequired → OauthError::NeedsReauth
- manager.rs: UpstreamOauthManager orchestrates full authorization_code+PKCE flow
- begin_authorization: discovers/caches AS metadata, enforces S256, saves PKCE state
- complete_authorization_callback: exchanges code, persists encrypted credentials
- clear_credentials: deletes tokens + evicts pending state
- build_auth_client: creates fresh AuthClient from stored credentials with proactive refresh
- Supports Preregistered and Dynamic registration; ClientMetadataDocument is TODO
- Cargo.toml: promote oauth2 from dev-dep to regular dep (needed for TokenResponse trait methods)
* feat(upstream-oauth): wire HTTP routes and AppState for upstream OAuth callback
Task 3: mount /v1/upstream-oauth/:name/{start,callback} routes in the axum
router, guarded by upstream_oauth.is_some(). Add upstream_oauth field to
AppState with with_upstream_oauth() builder. Routes are outside the bearer
middleware — browser redirects from the AS cannot carry Authorization headers.
* feat(upstream): wire per-(upstream,subject) AuthClient cache into UpstreamPool
Add oauth_managers field to UpstreamPool (DashMap keyed by upstream name),
with_oauth_managers() builder for injection at serve time, and OAuth branch
in connect_http_upstream: looks up manager, calls build_auth_client("default"),
wraps the AuthClient in StreamableHttpClientWorker. Non-OAuth path unchanged.
oauth_required error on NeedsReauth marks upstream unhealthy via discover_all.
* docs: add upstream oauth pkce gateway guidance and error kinds
* feat(upstream-oauth): enforce S256, issuer binding, canonical resource, CIMD
Completes Task 2 §6 spec-aligned invariants identified by plan audit:
- Must-Fix #1 S256 enforcement: verify_s256 now rejects AS metadata that
omits `code_challenge_methods_supported` (previously warned and continued)
or advertises only non-S256 methods. Both paths surface
`oauth_unsupported_method`.
- Must-Fix #4 canonical resource indicator: UpstreamConfig gains
`canonical_url()` applying RFC 3986 §6.2.2 normalization at
validation time (lowercase scheme+host, strip default port,
dot-segment removal, percent-encoding case). Manager uses the
canonical form when constructing the AuthorizationManager so
rmcp's `resource` parameter on authorize and token is byte-identical
to the canonical upstream URL. Known gap: rmcp 1.4 does not re-emit
`resource` on the refresh_token grant; documented in UPSTREAM.md.
- Must-Fix #5 issuer binding (scope 6b): verify_issuer_binding requires
`metadata.issuer` to be present and enforces host-consistency across
authorization_endpoint, token_endpoint, and registration_endpoint
(when present). Cannot duplicate rmcp discovery to bind against the
successful discovery URL, so the check is approximated via endpoint
host consistency. Violations surface as `oauth_issuer_mismatch`.
- CIMD registration: ClientMetadataDocument strategy now constructs the
OAuth client locally, using the metadata document URL as the
client_id. No registration_endpoint call is issued.
- Must-Fix #6 reactive 401 (scope 6b): deferred. rmcp's
StreamableHttpClientWorker hides the raw HTTP response, so a 401 on
an MCP call surfaces as a generic transport error. Operators
recover via `POST /v1/gateway/oauth/start`. Documented in UPSTREAM.md
so the doc no longer promises retry semantics that code does not
implement.
Also ships Task 2/3 scaffolding kept uncommitted in the worktree:
UpstreamOauthCredentialRow + UpstreamOauthStateRow (manual redacted
Debug), UpstreamOauthConfig + UpstreamOauthRegistration enum (CIMD,
Preregistered, Dynamic), and `oauth: None` test-fixture fill-ins.
Tests:
- crates/lab/tests/upstream_oauth.rs (8 tests)
- canonical_url_strips_default_port_and_lowercases_host
- missing_code_challenge_methods_returns_unsupported
- plain_pkce_only_returns_unsupported
- authorize_url_carries_canonical_resource_indicator
- token_exchange_carries_canonical_resource_indicator
- issuer_missing_returns_issuer_mismatch
- issuer_endpoint_host_mismatch_returns_issuer_mismatch
- cimd_registration_uses_metadata_url_as_client_id
* feat: wire subject-scoped upstream oauth cache
* feat: finish upstream oauth gateway wiring and verification
* fix: align upstream oauth HTTP surface with ERRORS.md spec and add dispatch telemetry
Add elapsed_ms field to all four OAuth handlers (start, status, clear, callback)
so every dispatch event includes surface/service/action/elapsed_ms per OBSERVABILITY.md.
Change clear handler confirmation_required response from 400 plain-text to 422 JSON
envelope (ToolError::Sdk) to match the documented spec in ERRORS.md. Update
accompanying test to assert 422 UNPROCESSABLE_ENTITY and JSON kind field.
Resolves review threads:
PRRT_kwDOR8nC1M576Bjh (missing dispatch logs)
Note: router callback placement, HTML escaping (html_escape helper already present),
subject validation (subject sourced from JWT auth.sub not query param), and axum
path syntax were all already correctly implemented — no changes needed.
* fix: harden upstream oauth manager - encryption, TOCTOU, issuer binding, client secret
seal()/seal_with_aad() now return Result<_, EncryptionError> instead of panicking via
.expect(); store.rs propagates the error as AuthError::InternalError.
get_or_discover_metadata holds the write lock across discovery to eliminate the
read-lock-drop-write-lock TOCTOU race where two callers could both issue discovery.
verify_issuer_binding changed from host-only comparison to full origin comparison
(scheme + host + explicit port) so http/https scheme and port differences are caught.
resolve_client_config now returns OauthError::Internal when client_secret_env names
an env var that is not set or empty, instead of silently using an empty secret.
Resolves review threads:
PRRT_kwDOR8nC1M576Bjk (seal() panics - encryption.rs)
PRRT_kwDOR8nC1M576Cmu (seal() panics - encryption.rs)
PRRT_kwDOR8nC1M576BYd (refresh lock TOCTOU)
PRRT_kwDOR8nC1M576Cmn (metadata cache TOCTOU)
PRRT_kwDOR8nC1M576ez2 (issuer binding host-only)
PRRT_kwDOR8nC1M576Bjm (missing client_secret env var silent)
PRRT_kwDOR8nC1M576Cmp (missing client_secret env var silent)
Note: extract_state_param None-guard (FIX E) and in-memory PKCE map TTL (FIX I)
were both already correctly implemented in this codebase — no changes needed.
* fix: enforce upstream config validation at startup and fix cleanup_expired predicate
UpstreamConfig::validate() is now called for each upstream in load_toml(), so invalid
configs (bad URL scheme, conflicting auth fields) are caught at startup rather than
at first OAuth flow attempt. validate() also now rejects non-http/https URL schemes.
cleanup_expired changes both DELETE predicates from < to <= so rows expiring exactly
at the current timestamp are cleaned up consistently with the rest of the expiry checks.
Resolves review threads:
PRRT_kwDOR8nC1M576ez0 (validate() never called at startup)
PRRT_kwDOR8nC1M576dqS (config.rs critical - URL scheme not validated)
PRRT_kwDOR8nC1M576dqQ (cleanup_expired < vs <= off-by-one)
Note: OauthError::Internal display/kind strings were already aligned in types.rs
(Display prefix matches kind() return value) — no change needed.
* fix: redact token from spike error, wire gateway validation, correct docs
spike_rmcp_auth_client.rs: remove token value from bail! error message to prevent
leaking access tokens into log output (use placeholder instead).
gateway/config.rs validate_upstream: call upstream.validate() at the start so that
bearer_token_env + oauth mutual-exclusion and other config constraints are enforced
in the gateway dispatch layer, not only in the top-level config loader.
docs/CONFIG.md: fix clear endpoint URL example to include required upstream= param.
docs/GATEWAY.md:
- Update clear endpoint description to reflect 422 JSON response (not 400 plain-text)
and document required upstream= query param
- Correct callback-security section: remove claim that handler re-validates
upstream-vs-state-row in application code (enforcement is via SQL primary key)
docs/UPSTREAM.md: correct claim that OAuth upstreams participate in startup discovery;
they are excluded from discover_all and connected per-request, not pooled.
Resolves review threads:
PRRT_kwDOR8nC1M576Bjb (token in spike error)
PRRT_kwDOR8nC1M576dqT (gateway/config.rs critical - validate not called)
PRRT_kwDOR8nC1M576dqW (CONFIG.md clear URL missing upstream=)
PRRT_kwDOR8nC1M576dqa (GATEWAY.md:140 clear endpoint description)
PRRT_kwDOR8nC1M576dqc (GATEWAY.md:152 callback invariants not in code)
PRRT_kwDOR8nC1M576dqe (UPSTREAM.md:158 OAuth discovery claim inaccurate)
Note: with_oauth_managers() wiring (FIX G) was already correctly implemented via
with_oauth_client_cache() in cli/serve.rs. OAuth error kinds in ERRORS.md were
already documented. No changes needed for those items.
* fix(oauth): remove dead url_host fn, duplicate tool_error_from_oauth, stale comment
Resolves review threads #15, #3, #21.
- Remove #[allow(dead_code)] url_host() from manager.rs (thread 15)
- Remove duplicate pub tool_error_from_oauth from gateway/oauth.rs; the
private copy in manager.rs is the only caller (thread 3)
- Drop unused OauthError import from oauth.rs
- Correct stale comment in spike_rmcp_auth_client.rs: mock returns 401
to drive re-auth path, not 200 (thread 21)
* docs(oauth): align upstream OAuth docs with implementation
Resolves review threads #4, #17, #18, #19, #24, #25, #26, #27, #28,
#29, #30, #36, #37, #38.
UPSTREAM.md:
- Thread 4: OAuth upstreams are attempted at startup and fail unhealthy
(not excluded entirely)
- Thread 18: Issuer binding checks origin (scheme+host+port), not just
host; covers auth/token/revocation/userinfo endpoints
- Thread 19: Remove false LRU-cap claim; lock entries live for process
lifetime
- Thread 24: Merged catalog is transport-neutral; OAuth upstreams appear
in catalog but need HTTP session to initiate authorization
- Thread 28: POST /v1/gateway/oauth/start route references are correct
- Thread 38: Remove auto-delete claim on invalid_grant (not implemented)
GATEWAY.md:
- Thread 17: Pending state SQL key is (upstream_name, subject,
csrf_token), not just (upstream_name, csrf_token)
- Thread 25: Reload eagerly evicts all OAuth AuthClient entries; remove
false built_with_client_id eviction-on-mismatch claim
- Thread 26: Routes /v1/gateway/oauth/* are implemented as documented
- Thread 27: Callback is browser-facing; subject from session cookie,
not from state parameter
ERRORS.md:
- Thread 30: Remove "(RFC 7636 absence implies plain-only)" — omission
of code_challenge_methods_supported is not equivalent to plain-only
- Thread 36: oauth_issuer_mismatch triggers on missing issuer or
endpoint/issuer origin mismatch, not direct discovered-URL equality
- Thread 37: Route references /v1/gateway/oauth/status and
POST /v1/gateway/oauth/start are correct (no /v1/upstream-oauth/ routes)
* fix(oauth): evict build_locks, fix param attribution, https guard, TTL guard, epoch default
Resolves review threads #1, #2/#9, #16, #20, #31, #32, #35.
cache.rs (thread 1):
- evict_subject and evict_upstream now also remove entries from
build_locks, preventing unbounded growth on long-running processes
config.rs (threads 2/9):
- validate_upstream maps ConfigError::InvalidUrl to param="url" instead
of "bearer_token_env"; auth-conflict errors still map to bearer_token_env
manager.rs (thread 35):
- ClientMetadataDocument URL validation now enforces https scheme;
http URLs are rejected with OauthError::Internal
store.rs (thread 16):
- token_received_at falls back to now_unix() instead of 0 (Unix epoch)
when absent; prevents access_token_expires_at underflow for tokens
that don't carry a received_at timestamp
sqlite.rs (threads 20, 31, 32):
- TTL guard now also rejects expires_at <= created_at (negative delta)
to prevent integer underflow on malicious/clock-skewed input
- cleanup queries already used <=; threads 31/32 already resolved
* fix(oauth): stdio ordering, oauth URL guard, is_master gating
Resolves review threads #7, #8, #22.
serve.rs (thread 7):
- Compute stdio_mode before build_upstream_oauth_runtime; skip OAuth
runtime init entirely in stdio mode so missing LAB_PUBLIC_URL /
LAB_OAUTH_ENCRYPTION_KEY never fails a stdio serve
config.rs / gateway config.rs (thread 8):
- UpstreamConfig::validate now rejects oauth+no-url combinations with
ConfigError::MissingOauthUrl; gateway config dispatch maps the new
variant to param="url"
router.rs (thread 22):
- Gateway OAuth routes (/v1/gateway/oauth/*) and browser callback are
now guarded by is_master; non-master nodes no longer mount them
* fix(oauth): redirect error kind, circuit breaker, tracing, DashMap clone, callback ext, reload reconcile
Resolves review threads #5, #6, #10, #11, #12, #13, #23, #50/#71.
upstream_oauth.rs (threads 5, 23):
- Callback: embed error_kind in redirect URL query params instead of
x-lab-oauth-error-kind header (browsers silently discard headers on
302 responses)
- Callback: extract AuthContext via Option<Extension<AuthContext>>
instead of reconstructing Parts from empty request (which discarded
middleware extensions); update callback_subject signature accordingly
pool.rs (threads 6, 11):
- subject_scoped_call_tool: add circuit breaker calls (record_success_for
/ record_failure_for) around the peer call
- subject_scoped_read_resource: add circuit breaker calls AND response
size guard matching the non-scoped read_upstream_resource path
- subject_scoped_get_prompt: add circuit breaker calls
server.rs (thread 12):
- Subject-scoped dispatch path now emits tracing::info! on success and
tracing::warn! on failure, matching the non-subject-scoped path
cache.rs (thread 13):
- get_or_build: clone DashMap Ref before the .await call on
build_auth_client to avoid holding a DashMap read-lock across await
(potential deadlock under contention)
manager.rs (thread 10):
- reload: reconcile upstream_oauth_managers after loading new config;
remove managers for OAuth upstreams no longer present, warn about
new OAuth upstreams that need restart to get a manager
Threads 50/71 (TokenRefreshFailed → NeedsReauth): already mapped
correctly in map_auth_error; no change needed.
* fix(oauth): dynamic registration once per upstream, not per call
Resolves review thread PRRT_kwDOR8nC1M579vdo
`configured_authorization_manager` was calling `register_client` on
every invocation (complete_authorization_callback, build_auth_client),
receiving a new AS-assigned client_id each time — mismatching the id
used to start the flow.
Fix: `resolve_client_config` for Dynamic now:
1. Checks stored credential row (available after token exchange)
2. Checks in-memory `dynamic_client_ids` cache (populated by begin_authorization)
3. Only calls register_client on the very first invocation
`clear_credentials` evicts the in-memory cache entry so a fresh
registration is issued when re-authorizing after credential clearance.
* fix(gateway): exhaustive ConfigError match, no catch-all for param attribution
Resolves review thread PRRT_kwDOR8nC1M579vdf
The wildcard arm `_ => param: "bearer_token_env"` was too broad: any
future ConfigError variant would be misattributed to bearer_token_env.
Replace the catch-all with an exhaustive match over all three ConfigError
variants so the compiler enforces correct attribution if new variants are
added. ConflictingAuth → bearer_token_env; MissingOauthUrl + InvalidUrl → url.
* fix(oauth): persist dynamic client registrations to SQLite
Dynamic registration (RFC 7591) must survive server restarts. Replace
the in-memory DashMap cache with a durable `upstream_oauth_dynamic_clients`
SQLite table (WITHOUT ROWID, PK: upstream_name + subject).
- Add `upstream_oauth_dynamic_clients` table to `open_connection` schema
- Add `save_dynamic_client_registration` (UPSERT), `find_dynamic_client_registration`
(SELECT), and `delete_dynamic_client_registration` (DELETE) to `SqliteStore`
- Add round-trip test covering save, upsert, find, delete, and subject isolation
- Remove `dynamic_client_ids: Arc<DashMap<...>>` field from `UpstreamOauthManager`
- `resolve_client_config` Dynamic branch now checks SQLite before calling
`register_client`, then persists the assigned client_id immediately
- `clear_credentials` also deletes the dynamic client registration row
client_secret is not stored: `token_endpoint_auth_method: "none"` means the
AS issues public clients and must not return a client_secret.
* feat(gateway-admin): upstream OAuth connection UI
Adds an "Upstream Connections" section to the Gateways page with per-upstream
Connect/Disconnect cards. Cards poll auth status, open the authorization URL
in a new tab, and wait for the callback before clearing the connecting state.
- GET /v1/gateway/oauth/upstreams — lists upstreams with oauth: config
- upstream-oauth-card: badge (Connected/Expiring/Disconnected), Connect/Disconnect
- upstream-oauth-section: grid of cards, null when no oauth upstreams configured
- Fix redirect loop: restore allow_session_cookie guard on browser redirect
- Fix redirect loop: switch v1 + MCP auth middleware to route_layer so unmatched
SPA paths (e.g. /gateways) are not intercepted by auth middleware
* style: rustfmt router.rs
jmagar
added a commit
that referenced
this pull request
May 2, 2026
…ity (#38) * feat(lab-gych.1): ChatSessionProvider 4-context split + ACP fetch utility Create ChatSessionProvider with 4-context re-render isolation: - ChatSessionDataContext: runs, selectedRun, providers, pageContext - ChatSessionActionsContext: stable callbacks (createSession, selectRun, etc.) - ChatSessionConnectionContext: connectionState, sessionStatus - ChatSessionStreamContext: messages, events, activity Lazy SSE stream: starts only on first FAB click via onFirstOpenRef callback. Users who never open chat pay zero SSE cost. createSession mutex: prevents duplicate session creation. selectedRunId seeded from localStorage with regex validation. providers setter uses bail-out comparison to prevent unnecessary re-renders. Auto-bootstrap gated behind streamEnabled. Also adds lib/acp/fetch.ts: standalone ACP fetcher that re-derives acpBase and requestCredentials per call (not at module eval time). * feat(lab-gych.5): ACP gateway pageContext support — opt-in compact system prefix POST /acp/sessions/{id}/prompt now accepts optional pageContext field: { prompt: string, pageContext?: { route, entityType?, entityId? } } When pageContext is present, server assembles a compact context prefix and prepends it to the prompt before forwarding to the LLM: [context: page={route}] [context: page={route} entity={type}/{id}] Token safeguards: - Off by default: absent pageContext = zero injection, zero token cost - Server-side assembly only: client sends structured data - Hard token budget: 30 tokens (~120 chars); truncates entityId first, then entityType, never route - No verbose markdown: single compact line - NFKC-safe strip: chars filtered to [a-zA-Z0-9/_-], max 32 chars/field - Deny-list: system, ignore, override, admin, instruction, assistant, prompt — any match skips injection, never errors the request - Logged: every injection emits surface=api, session_id, route, estimated_tokens at INFO level Validation failure silently skips injection — never errors the request. * feat(lab-gych.2): FAB + draggable/resizable popover shell with full UX FloatingChatFab: - Fixed pill (bottom-right, z-40), rounded-full Aurora-styled - Cmd/Ctrl+/ hotkey toggle, skips inputs and contentEditable - CSS-hidden (visibility:hidden) on /chat route — NOT unmounted - Ambient connection indicator (connecting = pulsing ring, error = amber dot) - Streaming pulse animation (respects prefers-reduced-motion) - Modal stack guard via openModals RefObject FloatingChatPopover: - Default 420x600, min 320x420, max 800x900 - Drag via DOM ref + rAF (no React state during drag), pointer capture - Resize via bottom-right corner handle with pointer capture - Viewport hard-clamp on drag commit and window resize (debounced 100ms) - Gear config panel: 4 toggles (persistOpen, persistPosition, persistSize, sendPageContext) — persistence via labby:floating-chat:state localStorage key - Accessibility: role=dialog, aria-modal, aria-labelledby, tabIndex=-1, focus trap, Escape closes, focus returns, HTML inert on app root - Mobile: renders as full-screen Sheet (< 768px) - CSS-hidden (not unmounted) on /chat route for both FAB and popover AdminLayoutClient: - 'use client' wrapper component preserving server component layout.tsx - Hosts ChatSessionProvider, FAB, Popover - Wires onFirstOpenRef for lazy SSE stream activation - Persists open state to localStorage Layout updated to use AdminLayoutClient. * feat(lab-gych.3): FloatingChatShell — full /chat parity in popover FloatingChatShell consumes all 4 ChatSession contexts: - Data: runs, selectedRun, agents, projects, pageContext - Actions: createSession, selectRun, refreshSessions - Connection: connectionState (for future connection indicator) - Stream: messages (subscribed via StreamContext only) React.memo wrapper: shell only re-renders when messages changes during streaming — header/input don't re-render on token stream. sendPrompt wires pageContext: when config.sendPageContext === true AND pageContext !== null, includes pageContext as separate field in request body: { prompt, pageContext: { route, entityType?, entityId? } } Lazy mount pattern in AdminLayoutClient: - shellMounted state starts false - Set to true on first FAB click (alongside stream enable) - FloatingChatShell mounts permanently after first click - Popover container controls visibility via CSS, not mount state * feat(lab-gych.4): FloatingChat design system section Adds FloatingChatSection to /design-system sandbox with: - FAB states: default, with-badge (count=3), active/open, connecting (pulsing ring), error (amber dot) - Popover states: default, gear config open (4 toggles), compact (min size) - Persistence schema: localStorage key + shape as code block - Hotkey reference: ⌘/ (Ctrl+/ on Windows) with KbdGroup/Kbd primitives - Mobile Sheet note: < 768px uses bottom Sheet, no drag/resize All demos use local fake state — no live backend required. Placed after CommandPaletteSection in Application Patterns group. * verify: address gaps and simplify lab-gych implementation - Wire setPageContext producer: add usePageContextSync hook and PageContextSync component, render inside ChatSessionProvider in AdminLayoutClient so every page propagates its route automatically - Fix focus-trap inert selector: replace Pages Router #__next selector with App Router compatible body-children iteration, inert all direct body children except the panel itself to avoid blocking the overlay - Wire selectAgent in FloatingChatShell: call useChatSessionActions() selectAgent directly instead of no-op local callback - Simplify: remove redundant selectAgent wrapper closure, pass context action directly; rename inerter -> toRestore for clarity; trim what-comments from new files, keep only non-obvious constraint notes Build: cargo build --workspace --all-features exit=0 Clippy: cargo clippy --workspace --all-features -D warnings exit=0 * fix: address lavra-review findings for lab-gych floating chat P1 (all 6 fixed): - lab-gych.6: SSE 401 for missing ticket; remove String::new() fallback - lab-gych.7: initialize shellMounted from same localStorage check as open - lab-gych.8: pure setState updater; move side effects to useEffect([open]) - lab-gych.9: move sanitize/assemble page_context to dispatch/acp/page_context.rs; HTTP handler is now a thin shim; CLI/MCP get page_context support for free - lab-gych.10: extract shared normalizers to lib/chat/acp-normalizers.ts; eliminate duplicate types/functions between chat-session-provider and controller - lab-gych.11: create lib/chat/session-event-cache.ts; both SSE consumers import from single source, remove "must stay in sync" comment P2 (all 9 fixed): - lab-gych.12: batch setEvents per SSE chunk (O(n) → single call); reverse-scan resolveSessionStatusFromEvents (O(1) in common case) - lab-gych.13: PROMPT_MAX_CHARS = 64_000 guard before processing - lab-gych.14: patchPersistedState() with in-memory cache; exported from popover - lab-gych.15: initialize config state from readPersistedState().config - lab-gych.16: set Content-Type: application/json when body present in fetchAcp - lab-gych.17: fix 4 type violations (timer | undefined, union narrowing, NonNullable<PageContext> field type) - lab-gych.18: add sessionsLoaded guard to auto-bootstrap effect; AbortController - lab-gych.19: remove "admin"/"prompt"/"system" from deny-list; keep only injection-specific terms; rely on character allowlist as primary safety - lab-gych.20: remove ghost SettingsPanel state + render (no backend wiring) P3 (4 of 5 addressed): - lab-gych.21: split on separators before deny-list check in page_context.rs - lab-gych.22: MAX_SUBSCRIBERS_PER_SESSION = 32 cap in registry.rs - lab-gych.28: remove React.memo from FloatingChatShell (no stable props) - lab-gych.29: skipped (triplication requires broader hook refactor) - lab-gych.26: skipped (PersistConfig move is cross-cutting; low risk/reward) * fix: tighten lab-gych.6 and lab-gych.18 follow-ups - lab-gych.6 list_sessions: add TODO(phase-2) comment noting principal filter deferred until bearer auth is wired in middleware layer - lab-gych.18: remove decorative AbortController (createSession does not accept a signal; guard was already a no-op); add NOTE(phase-2) for future wiring; sessionsLoaded guard still present as the actual fix * fix: address PR review comments on session management - Cross-validate selectedRunId against loaded runs in refreshSessions; stale server-deleted session IDs no longer block auto-bootstrap - Fix misleading comment on isCreatingRef mutex guard Resolves review thread PRRT_kwDOR8nC1M591X9O Resolves review thread PRRT_kwDOR8nC1M591YUu Resolves review thread PRRT_kwDOR8nC1M591YWB * fix: address all deferred P3 review findings (lab-gych.23-27, lab-gych.29) - lab-gych.23: resize now uses rAF + direct DOM writes during pointermove, commits to React state only on pointerup (mirrors drag pattern) - lab-gych.24: remove 4-state lazy-SSE protocol (streamEnabled, onFirstOpenRef, hasOpenedOnce, shellMounted); SSE opens whenever selectedRunId is non-null; FloatingChatShell always mounted inside popover - lab-gych.25: remove PersistConfig persist toggles (persistOpen, persistPosition, persistSize); always persist all state; rename PersistConfig -> ChatConfig; keep sendPageContext as standalone boolean; simplify gear panel - lab-gych.26: update all three importers (floating-chat-shell, admin-layout-client, design-system) to use ChatConfig (PersistConfig removal made type relocation moot) - lab-gych.27: add LRU eviction to session-event-cache keeping last 10 sessions; replace raw Map exports with helper objects; both maps evicted together per key - lab-gych.29: replace duplicate inline fetchAcp in use-chat-session-controller with createAcpFetcher() via stable ref; use fetchAcpRef.current in SSE effect in chat-session-provider (removes createAcpFetcher() call inside the effect)
jmagar
added a commit
that referenced
this pull request
May 2, 2026
…ity (#38) * feat(lab-gych.1): ChatSessionProvider 4-context split + ACP fetch utility Create ChatSessionProvider with 4-context re-render isolation: - ChatSessionDataContext: runs, selectedRun, providers, pageContext - ChatSessionActionsContext: stable callbacks (createSession, selectRun, etc.) - ChatSessionConnectionContext: connectionState, sessionStatus - ChatSessionStreamContext: messages, events, activity Lazy SSE stream: starts only on first FAB click via onFirstOpenRef callback. Users who never open chat pay zero SSE cost. createSession mutex: prevents duplicate session creation. selectedRunId seeded from localStorage with regex validation. providers setter uses bail-out comparison to prevent unnecessary re-renders. Auto-bootstrap gated behind streamEnabled. Also adds lib/acp/fetch.ts: standalone ACP fetcher that re-derives acpBase and requestCredentials per call (not at module eval time). * feat(lab-gych.5): ACP gateway pageContext support — opt-in compact system prefix POST /acp/sessions/{id}/prompt now accepts optional pageContext field: { prompt: string, pageContext?: { route, entityType?, entityId? } } When pageContext is present, server assembles a compact context prefix and prepends it to the prompt before forwarding to the LLM: [context: page={route}] [context: page={route} entity={type}/{id}] Token safeguards: - Off by default: absent pageContext = zero injection, zero token cost - Server-side assembly only: client sends structured data - Hard token budget: 30 tokens (~120 chars); truncates entityId first, then entityType, never route - No verbose markdown: single compact line - NFKC-safe strip: chars filtered to [a-zA-Z0-9/_-], max 32 chars/field - Deny-list: system, ignore, override, admin, instruction, assistant, prompt — any match skips injection, never errors the request - Logged: every injection emits surface=api, session_id, route, estimated_tokens at INFO level Validation failure silently skips injection — never errors the request. * feat(lab-gych.2): FAB + draggable/resizable popover shell with full UX FloatingChatFab: - Fixed pill (bottom-right, z-40), rounded-full Aurora-styled - Cmd/Ctrl+/ hotkey toggle, skips inputs and contentEditable - CSS-hidden (visibility:hidden) on /chat route — NOT unmounted - Ambient connection indicator (connecting = pulsing ring, error = amber dot) - Streaming pulse animation (respects prefers-reduced-motion) - Modal stack guard via openModals RefObject FloatingChatPopover: - Default 420x600, min 320x420, max 800x900 - Drag via DOM ref + rAF (no React state during drag), pointer capture - Resize via bottom-right corner handle with pointer capture - Viewport hard-clamp on drag commit and window resize (debounced 100ms) - Gear config panel: 4 toggles (persistOpen, persistPosition, persistSize, sendPageContext) — persistence via labby:floating-chat:state localStorage key - Accessibility: role=dialog, aria-modal, aria-labelledby, tabIndex=-1, focus trap, Escape closes, focus returns, HTML inert on app root - Mobile: renders as full-screen Sheet (< 768px) - CSS-hidden (not unmounted) on /chat route for both FAB and popover AdminLayoutClient: - 'use client' wrapper component preserving server component layout.tsx - Hosts ChatSessionProvider, FAB, Popover - Wires onFirstOpenRef for lazy SSE stream activation - Persists open state to localStorage Layout updated to use AdminLayoutClient. * feat(lab-gych.3): FloatingChatShell — full /chat parity in popover FloatingChatShell consumes all 4 ChatSession contexts: - Data: runs, selectedRun, agents, projects, pageContext - Actions: createSession, selectRun, refreshSessions - Connection: connectionState (for future connection indicator) - Stream: messages (subscribed via StreamContext only) React.memo wrapper: shell only re-renders when messages changes during streaming — header/input don't re-render on token stream. sendPrompt wires pageContext: when config.sendPageContext === true AND pageContext !== null, includes pageContext as separate field in request body: { prompt, pageContext: { route, entityType?, entityId? } } Lazy mount pattern in AdminLayoutClient: - shellMounted state starts false - Set to true on first FAB click (alongside stream enable) - FloatingChatShell mounts permanently after first click - Popover container controls visibility via CSS, not mount state * feat(lab-gych.4): FloatingChat design system section Adds FloatingChatSection to /design-system sandbox with: - FAB states: default, with-badge (count=3), active/open, connecting (pulsing ring), error (amber dot) - Popover states: default, gear config open (4 toggles), compact (min size) - Persistence schema: localStorage key + shape as code block - Hotkey reference: ⌘/ (Ctrl+/ on Windows) with KbdGroup/Kbd primitives - Mobile Sheet note: < 768px uses bottom Sheet, no drag/resize All demos use local fake state — no live backend required. Placed after CommandPaletteSection in Application Patterns group. * verify: address gaps and simplify lab-gych implementation - Wire setPageContext producer: add usePageContextSync hook and PageContextSync component, render inside ChatSessionProvider in AdminLayoutClient so every page propagates its route automatically - Fix focus-trap inert selector: replace Pages Router #__next selector with App Router compatible body-children iteration, inert all direct body children except the panel itself to avoid blocking the overlay - Wire selectAgent in FloatingChatShell: call useChatSessionActions() selectAgent directly instead of no-op local callback - Simplify: remove redundant selectAgent wrapper closure, pass context action directly; rename inerter -> toRestore for clarity; trim what-comments from new files, keep only non-obvious constraint notes Build: cargo build --workspace --all-features exit=0 Clippy: cargo clippy --workspace --all-features -D warnings exit=0 * fix: address lavra-review findings for lab-gych floating chat P1 (all 6 fixed): - lab-gych.6: SSE 401 for missing ticket; remove String::new() fallback - lab-gych.7: initialize shellMounted from same localStorage check as open - lab-gych.8: pure setState updater; move side effects to useEffect([open]) - lab-gych.9: move sanitize/assemble page_context to dispatch/acp/page_context.rs; HTTP handler is now a thin shim; CLI/MCP get page_context support for free - lab-gych.10: extract shared normalizers to lib/chat/acp-normalizers.ts; eliminate duplicate types/functions between chat-session-provider and controller - lab-gych.11: create lib/chat/session-event-cache.ts; both SSE consumers import from single source, remove "must stay in sync" comment P2 (all 9 fixed): - lab-gych.12: batch setEvents per SSE chunk (O(n) → single call); reverse-scan resolveSessionStatusFromEvents (O(1) in common case) - lab-gych.13: PROMPT_MAX_CHARS = 64_000 guard before processing - lab-gych.14: patchPersistedState() with in-memory cache; exported from popover - lab-gych.15: initialize config state from readPersistedState().config - lab-gych.16: set Content-Type: application/json when body present in fetchAcp - lab-gych.17: fix 4 type violations (timer | undefined, union narrowing, NonNullable<PageContext> field type) - lab-gych.18: add sessionsLoaded guard to auto-bootstrap effect; AbortController - lab-gych.19: remove "admin"/"prompt"/"system" from deny-list; keep only injection-specific terms; rely on character allowlist as primary safety - lab-gych.20: remove ghost SettingsPanel state + render (no backend wiring) P3 (4 of 5 addressed): - lab-gych.21: split on separators before deny-list check in page_context.rs - lab-gych.22: MAX_SUBSCRIBERS_PER_SESSION = 32 cap in registry.rs - lab-gych.28: remove React.memo from FloatingChatShell (no stable props) - lab-gych.29: skipped (triplication requires broader hook refactor) - lab-gych.26: skipped (PersistConfig move is cross-cutting; low risk/reward) * fix: tighten lab-gych.6 and lab-gych.18 follow-ups - lab-gych.6 list_sessions: add TODO(phase-2) comment noting principal filter deferred until bearer auth is wired in middleware layer - lab-gych.18: remove decorative AbortController (createSession does not accept a signal; guard was already a no-op); add NOTE(phase-2) for future wiring; sessionsLoaded guard still present as the actual fix * fix: address PR review comments on session management - Cross-validate selectedRunId against loaded runs in refreshSessions; stale server-deleted session IDs no longer block auto-bootstrap - Fix misleading comment on isCreatingRef mutex guard Resolves review thread PRRT_kwDOR8nC1M591X9O Resolves review thread PRRT_kwDOR8nC1M591YUu Resolves review thread PRRT_kwDOR8nC1M591YWB * fix: address all deferred P3 review findings (lab-gych.23-27, lab-gych.29) - lab-gych.23: resize now uses rAF + direct DOM writes during pointermove, commits to React state only on pointerup (mirrors drag pattern) - lab-gych.24: remove 4-state lazy-SSE protocol (streamEnabled, onFirstOpenRef, hasOpenedOnce, shellMounted); SSE opens whenever selectedRunId is non-null; FloatingChatShell always mounted inside popover - lab-gych.25: remove PersistConfig persist toggles (persistOpen, persistPosition, persistSize); always persist all state; rename PersistConfig -> ChatConfig; keep sendPageContext as standalone boolean; simplify gear panel - lab-gych.26: update all three importers (floating-chat-shell, admin-layout-client, design-system) to use ChatConfig (PersistConfig removal made type relocation moot) - lab-gych.27: add LRU eviction to session-event-cache keeping last 10 sessions; replace raw Map exports with helper objects; both maps evicted together per key - lab-gych.29: replace duplicate inline fetchAcp in use-chat-session-controller with createAcpFetcher() via stable ref; use fetchAcpRef.current in SSE effect in chat-session-provider (removes createAcpFetcher() call inside the effect)
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements a global floating chat surface available on every admin page — a fixed pill FAB that opens a draggable/resizable popover backed by shared session state with the
/chatroute.Phase 1: Session Context Extraction (
lab-gych.1)ChatSessionProviderwith 4-context re-render isolation (Data / Actions / Connection / Stream)createSessionmutex viaisCreatingRefto prevent duplicate sessionsselectedRunIdseeded from localStorage with regex validationlib/acp/fetch.ts: standalone ACP fetcher utility (re-derives credentials per call)Phase 2: FAB + Popover Shell (
lab-gych.2)FloatingChatFab: fixed pill (bottom-right), Cmd/Ctrl+/ hotkey, CSS-hidden on /chatFloatingChatPopover: draggable (DOM ref + rAF, no React state), resizable (bottom-right handle), viewport hard-clamp, localStorage persistence, gear config panel (4 toggles), mobile Sheet, focus trapAdminLayoutClient:'use client'wrapper to keeplayout.tsxas server componentChatSessionProvider+ FAB + PopoverPhase 3: Full Parity Wiring (
lab-gych.3)FloatingChatShell: consumes all 4 contexts,React.memowrappersendPromptwirespageContext: when config toggle enabled and context non-null, includes as separate field in request bodyFloatingChatShellmounts on first FAB click, stays mounted permanentlyPhase 4: Design System Sandbox (
lab-gych.4)FloatingChatSectionadded to/design-systemafterCommandPaletteSectionPhase 5: ACP Gateway pageContext (
lab-gych.5)POST /acp/sessions/{id}/promptaccepts optionalpageContext: { route, entityType?, entityId? }[context: page=gateways entity=gateway/abc123][a-zA-Z0-9/_-], max 32 chars/field, deny-list checksession_id,route,estimated_tokensFiles Changed
New files (TypeScript):
apps/gateway-admin/lib/acp/fetch.tsapps/gateway-admin/lib/chat/chat-session-provider.tsxapps/gateway-admin/components/floating-chat-fab.tsxapps/gateway-admin/components/floating-chat-popover.tsxapps/gateway-admin/components/admin-layout-client.tsxapps/gateway-admin/components/floating-chat-shell.tsxapps/gateway-admin/components/design-system/floating-chat-section.tsxModified files:
apps/gateway-admin/app/(admin)/layout.tsx— wrap children in AdminLayoutClientapps/gateway-admin/components/design-system/design-system-shell.tsx— add FloatingChatSectioncrates/lab/src/api/services/acp.rs— pageContext support in prompt handlerTest Plan
pnpm tsc --noEmit— 0 new errors in new filescargo clippy --workspace --all-features— 0 errors/warnings/design-systemshows Floating Chat section with all demosBeads
Summary by cubic
Adds a global floating chat popover on every admin page with full
/chatparity and shared session state. SSE now starts whenever a session is selected, and ACP is hardened (SSE ticket required; 64k prompt cap).New Features
/chat.ChatSessionProvider(4-context split) with shared runs/messages; SSE opens whenselectedRunIdis set;PageContextSyncauto-syncs the current route intopageContext, and it’s only sent when the gear toggle is on.POST /acp/sessions/{id}/promptacceptspageContext; server builds a compact prefix (30‑token budget); 64k‑char prompt cap; events SSE requires a subscribe ticket (401 otherwise); registry caps subscribers/session at 32.Refactors
selectedRunId.createAcpFetcheracross provider/controller.selectAgent; cross-validatesselectedRunIdinrefreshSessions.Written for commit 264ba83. Summary will update on new commits. Review in cubic
Summary by CodeRabbit