feat(ambient-ui): status bar with connection context switching#1626
Merged
Conversation
✅ Deploy Preview for cheerful-kitten-f556a0 canceled.
|
Contributor
|
Warning Review limit reached
More reviews will be available in 6 minutes and 28 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds a persistent bottom StatusBar with editable API server context, a runtime-config module and /api/config route, a health-check hook plus tests, Radix popover wrappers, and updates the ambient proxy to use the runtime-configured upstream URL and optional custom token. ChangesRuntime-configurable API server with connection status display
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
specs/ambient-ui/ambient-ui.spec.md (1)
746-749: ⚡ Quick winAdd explicit confidentiality requirement for custom token handling.
This section should explicitly state that custom tokens are never returned by config read APIs, never rendered back after save, and must be redacted in logs/telemetry. The feature introduces sensitive credential input, so this needs to be normative in the spec.
Also applies to: 793-798
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specs/ambient-ui/ambient-ui.spec.md` around lines 746 - 749, Add a normative confidentiality clause to the "status bar" custom-connection text stating that any user-provided bearer token is treated as sensitive: the token MUST NOT be returned by any config read API, MUST NOT be rendered back to the UI after save (only an obfuscated indicator may be shown), and MUST be redacted in all logs and telemetry; apply the same explicit requirements to the related section referenced (lines 793-798) so both the custom connection and any config storage/inspection behavior include these three mandates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/ambient-ui/src/app/api/config/route.ts`:
- Around line 15-23: The PUT handler currently calls request.json() directly (in
function PUT) which will throw on empty/invalid JSON and cause a 500; wrap the
request.json() call in a try/catch inside PUT, and on parse failure return a
Response.json({ error: 'Invalid JSON' }, { status: 400 }) (or similar
bad-request message) so malformed payloads produce a 400 instead of an
exception; keep the existing object/null type check after successful parsing.
In `@components/ambient-ui/src/lib/runtime-config.ts`:
- Around line 3-29: The module currently stores apiServerUrl and customToken in
process-global variables (apiServerUrl, customToken) and exposes setters
(setCustomContext, resetContext) which makes overrides global; change the API to
avoid module-level state by making runtime config per-request or per-session:
remove or stop using the mutable top-level variables and instead accept
overrides via parameters or a request/context object (e.g., add an optional
overrides argument to getRuntimeConfig or read from a per-request
store/session), update callers to pass the per-user url/token instead of calling
setCustomContext/resetContext, and keep defaultApiServerUrl derived from
env.API_SERVER_URL while computing isCustomContext from the passed-in overrides.
Ensure functions referenced (getRuntimeConfig, setCustomContext, resetContext,
apiServerUrl, customToken) are adjusted accordingly so no process-global
override is used.
- Around line 22-25: setCustomContext currently only updates
apiServerUrl/customToken when passed truthy strings, so an empty string can't
clear a previously set customToken; change setCustomContext to accept explicit
clears and assign overrides whenever a value is provided (not only truthy):
update the signature to allow token: string | null | undefined and url: string |
null | undefined, and replace the truthy checks with explicit undefined checks
(e.g. if (url !== undefined) apiServerUrl = url; if (token !== undefined)
customToken = token;) so passing null or '' will clear the override while
leaving undefined unchanged.
In `@specs/ambient-ui/ambient-ui.spec.md`:
- Around line 761-763: The spec step "WHEN the user double-clicks the URL" is
inconsistent with the PR intent for a single-click interaction; update the spec
to require a single click (replace "double-clicks" with "clicks" in the step)
and keep the rest of the acceptance criteria (status bar expands to show
editable URL and Token with URL pre-populated). Also add a brief accessibility
note to the same scenario indicating the interaction must work via keyboard
(Enter/Space focus) and touch so tests and discoverability align with the
implementation.
---
Nitpick comments:
In `@specs/ambient-ui/ambient-ui.spec.md`:
- Around line 746-749: Add a normative confidentiality clause to the "status
bar" custom-connection text stating that any user-provided bearer token is
treated as sensitive: the token MUST NOT be returned by any config read API,
MUST NOT be rendered back to the UI after save (only an obfuscated indicator may
be shown), and MUST be redacted in all logs and telemetry; apply the same
explicit requirements to the related section referenced (lines 793-798) so both
the custom connection and any config storage/inspection behavior include these
three mandates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a95c0196-25f6-4180-914f-6e83eda10423
⛔ Files ignored due to path filters (1)
components/ambient-ui/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (11)
components/ambient-ui/package.jsoncomponents/ambient-ui/src/app/(dashboard)/layout.tsxcomponents/ambient-ui/src/app/api/ambient/v1/[...path]/route.tscomponents/ambient-ui/src/app/api/config/route.tscomponents/ambient-ui/src/components/nav-header.tsxcomponents/ambient-ui/src/components/status-bar.tsxcomponents/ambient-ui/src/components/ui/popover.tsxcomponents/ambient-ui/src/hooks/__tests__/use-connection-status.test.tscomponents/ambient-ui/src/hooks/use-connection-status.tscomponents/ambient-ui/src/lib/runtime-config.tsspecs/ambient-ui/ambient-ui.spec.md
jsell-rh
force-pushed
the
jsell/ambient-ui-statusbar
branch
2 times, most recently
from
b110c07 to
efb6410
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specs/ambient-ui/ambient-ui.spec.md`:
- Around line 761-767: Update the spec wording to match the implemented popover
interaction: replace "the status bar expands to show two editable fields" with
"a popover opens anchored to the status bar showing two editable fields (URL and
Token)" and explicitly note that the URL field is pre-populated, the Token field
is empty with placeholder text (e.g., "Bearer token"), pressing Enter on either
field confirms changes, pressing Escape or clicking outside the popover cancels
and closes it, and the popover is rendered via the `@radix-ui/react-popover`
pattern described in the PR summary ("opens a popover with connection fields").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9c9068c2-4a2a-453f-91c4-ec072914cdb4
⛔ Files ignored due to path filters (1)
components/ambient-ui/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (11)
components/ambient-ui/package.jsoncomponents/ambient-ui/src/app/(dashboard)/layout.tsxcomponents/ambient-ui/src/app/api/ambient/v1/[...path]/route.tscomponents/ambient-ui/src/app/api/config/route.tscomponents/ambient-ui/src/components/nav-header.tsxcomponents/ambient-ui/src/components/status-bar.tsxcomponents/ambient-ui/src/components/ui/popover.tsxcomponents/ambient-ui/src/hooks/__tests__/use-connection-status.test.tscomponents/ambient-ui/src/hooks/use-connection-status.tscomponents/ambient-ui/src/lib/runtime-config.tsspecs/ambient-ui/ambient-ui.spec.md
🚧 Files skipped from review as they are similar to previous changes (9)
- components/ambient-ui/src/app/(dashboard)/layout.tsx
- components/ambient-ui/src/hooks/use-connection-status.ts
- components/ambient-ui/src/app/api/ambient/v1/[...path]/route.ts
- components/ambient-ui/src/components/status-bar.tsx
- components/ambient-ui/src/hooks/tests/use-connection-status.test.ts
- components/ambient-ui/src/components/ui/popover.tsx
- components/ambient-ui/src/app/api/config/route.ts
- components/ambient-ui/src/components/nav-header.tsx
- components/ambient-ui/src/lib/runtime-config.ts
Comment on lines
+761
to
+767
| - WHEN the user clicks the URL | ||
| - THEN the status bar expands to show two editable fields: URL and Token | ||
| - AND the URL field is pre-populated with the current URL | ||
| - AND the Token field is empty with placeholder text (e.g., "Bearer token") | ||
| - AND pressing Enter on either field confirms the change | ||
| - AND pressing Escape cancels and collapses back to the default view | ||
|
|
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Spec/implementation mismatch: popover vs inline expansion.
Line 762 states "the status bar expands to show two editable fields," but the PR summary and implementation use a popover overlay (per @radix-ui/react-popover dependency and PR description: "opens a popover with connection fields"). Update the spec to describe the popover interaction pattern for consistency with the implementation.
📝 Suggested spec correction
#### Scenario: Enter custom context
- GIVEN the status bar displays the default API server URL
- WHEN the user clicks the URL
-- THEN the status bar expands to show two editable fields: URL and Token
+- THEN a popover opens above the status bar with two editable fields: URL and Token
- AND the URL field is pre-populated with the current URL
- AND the Token field is empty with placeholder text (e.g., "Bearer token")
- AND pressing Enter on either field confirms the change
-- AND pressing Escape cancels and collapses back to the default view
+- AND pressing Escape cancels and closes the popover🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@specs/ambient-ui/ambient-ui.spec.md` around lines 761 - 767, Update the spec
wording to match the implemented popover interaction: replace "the status bar
expands to show two editable fields" with "a popover opens anchored to the
status bar showing two editable fields (URL and Token)" and explicitly note that
the URL field is pre-populated, the Token field is empty with placeholder text
(e.g., "Bearer token"), pressing Enter on either field confirms changes,
pressing Escape or clicking outside the popover cancels and closes it, and the
popover is rendered via the `@radix-ui/react-popover` pattern described in the PR
summary ("opens a popover with connection fields").
Add a persistent bottom-of-page status bar showing API server connection status and URL. Supports switching between default SSO context and custom remote connections with user-provided bearer tokens. Status bar: - Connection status indicator (green/red dot + label) polling /api/healthz - API server URL in monospace, double-click to edit - Custom context: URL + token fields, amber override indicator, Reset button - Context switches invalidate all cached data (projects, sessions) BFF proxy: - Auth priority: custom token > SSO session JWT > dev-mode token - Runtime-mutable API server URL via /api/config (GET/PUT/DELETE) - Custom token stored server-side, never exposed in API responses Spec updated with Status Bar and Connection Context Switching requirements. 171 tests, 0 type errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jsell-rh
force-pushed
the
jsell/ambient-ui-statusbar
branch
from
efb6410 to
f19070e
Compare
Contributor
Merge Queue Status
This pull request spent 33 seconds in the queue, including 5 seconds running CI. Required conditions to merge |
This was referenced Jun 1, 2026
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
1 participant
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
removeQueries+refetchQueriesensures fresh data after context switch/on context change to avoid stale entity referencesSpec updated with "Status Bar" and "Connection Context Switching" requirements.
Architecture
src/lib/runtime-config.ts— shared module for runtime-mutable API server URL + custom tokensrc/app/api/config/route.ts— GET/PUT/DELETE for connection context (token stored server-side, never exposed)src/hooks/use-connection-status.ts— polls/api/healthzevery 10sTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
Chores