Add source-backed AI Hub workspace surface - #301
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR introduces a signed, source-backed GET /api/ai-hub/surface endpoint and replaces the AI Hub UI with a data-driven, refreshable frontend that fetches summary, prompt/workflow/agent cards, evaluation metrics, and run events derived from PromptTemplate, LLMProvider, and SecurityAuditEvent rows. Tests cover signed-session auth, header exclusion, role-based visibility, and a Postgres smoke path. ChangesAI Hub Source-Backed Surface
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ 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)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/test_ai_hub_api.py (1)
185-233:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAdd a real PostgreSQL smoke test for
/api/ai-hub/surface.This slice is DB-affecting but currently covered only with
MockSession. Please add one Postgres-backed bootstrap/smoke test before merge evidence is treated as complete.As per coding guidelines: "DB-affecting API slices need both mocked fast tests and a real PostgreSQL bootstrap/smoke path before PR merge evidence is considered complete."
🤖 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 `@backend/tests/test_ai_hub_api.py` around lines 185 - 233, Add a PostgreSQL-backed smoke test for the /api/ai-hub/surface endpoint instead of only using MockSession: create a new test (e.g., test_ai_hub_surface_postgres_smoke) that uses the real get_db dependency (override get_db only to yield a real async DB session connected to the test Postgres instance), run any required bootstrap/seed routines to insert the minimal prompts/providers/audit_events rows (reuse the same helper factories used by MockSession if available), call the endpoint via TestClient and assert the same critical response fields (status 200 and presence/absence checks done in test_ai_hub_surface_uses_signed_source_evidence), and ensure you clear app.dependency_overrides and teardown DB state after the test so it does not leak to other tests.
🧹 Nitpick comments (1)
backend/api/ai_hub.py (1)
270-271: ⚡ Quick winMake ordering deterministic for stable payload rendering.
Sorting only by timestamp can reorder cards/events nondeterministically when timestamps tie. Add a secondary stable sort key.
♻️ Proposed change
- .order_by(desc(PromptTemplate.updated_at)) + .order_by(desc(PromptTemplate.updated_at), desc(PromptTemplate.id)) .limit(8) - .order_by(desc(LLMProvider.updated_at)) + .order_by(desc(LLMProvider.updated_at), desc(LLMProvider.id)) .limit(8) - .order_by(desc(SecurityAuditEvent.observed_at)) + .order_by( + desc(SecurityAuditEvent.observed_at), + desc(SecurityAuditEvent.event_uid), + ) .limit(8)Also applies to: 285-286, 304-305
🤖 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 `@backend/api/ai_hub.py` around lines 270 - 271, The order_by calls that currently use only desc(PromptTemplate.updated_at) (seen in the .order_by(...) before .limit(8)) are non-deterministic when timestamps tie; update each such .order_by call (including the other occurrences around PromptTemplate.updated_at at the mentioned locations) to include a secondary stable key like PromptTemplate.id (e.g., add PromptTemplate.id.asc() as a tie-breaker) so ordering becomes deterministic; ensure you apply the same change to every similar query in this file.
🤖 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 `@backend/api/ai_hub.py`:
- Around line 276-288: The _list_providers function returns provider metadata
for any authenticated user; add an authorization guard that checks AuthContext
for allowed roles (e.g., admin or provider-registry role) before querying
LLMProvider: inspect auth_context (fields like role, is_admin, or has_role) and
if the caller is not in the permitted roles, either return an empty list or
raise an appropriate authorization error, otherwise proceed with the existing
select(...) query and return statement; update the function to perform this role
check early and reference _list_providers, AuthContext, and LLMProvider when
making the change.
- Around line 264-269: The query currently allows PromptTemplate.is_shared to be
true without scoping to the tenant, so shared prompts can leak across tenants;
update the where clause that uses PromptTemplate.is_shared (near the
.where(or_(...)) block) to require the shared row also matches the request's
tenant/workspace (e.g. add PromptTemplate.organization_id ==
auth_context.organization_id or PromptTemplate.workspace_id ==
auth_context.workspace_id as part of the OR condition for shared rows), so only
prompts that are either owned by auth_context.user_id OR are shared within the
same tenant/workspace are returned.
In `@frontend/src/app/ai-hub/page.test.tsx`:
- Around line 147-149: The test's header omission assertion can false-pass due
to case-sensitivity and only checks one header name; update the assertion that
inspects fetchMock.mock.calls[0] (firstFetchCall) headers to normalize header
names (e.g., lower-case keys) or explicitly check for both 'X-User-Id' and
'x-user-id' and any other forbidden identity headers so the test fails if any
case-variant or additional identity header is present; locate the usage of
fetchMock, firstFetchCall and the headers access in page.test.tsx and replace
the single JSON string containment check with a case-insensitive key presence
check or explicit checks for all forbidden header names.
---
Outside diff comments:
In `@backend/tests/test_ai_hub_api.py`:
- Around line 185-233: Add a PostgreSQL-backed smoke test for the
/api/ai-hub/surface endpoint instead of only using MockSession: create a new
test (e.g., test_ai_hub_surface_postgres_smoke) that uses the real get_db
dependency (override get_db only to yield a real async DB session connected to
the test Postgres instance), run any required bootstrap/seed routines to insert
the minimal prompts/providers/audit_events rows (reuse the same helper factories
used by MockSession if available), call the endpoint via TestClient and assert
the same critical response fields (status 200 and presence/absence checks done
in test_ai_hub_surface_uses_signed_source_evidence), and ensure you clear
app.dependency_overrides and teardown DB state after the test so it does not
leak to other tests.
---
Nitpick comments:
In `@backend/api/ai_hub.py`:
- Around line 270-271: The order_by calls that currently use only
desc(PromptTemplate.updated_at) (seen in the .order_by(...) before .limit(8))
are non-deterministic when timestamps tie; update each such .order_by call
(including the other occurrences around PromptTemplate.updated_at at the
mentioned locations) to include a secondary stable key like PromptTemplate.id
(e.g., add PromptTemplate.id.asc() as a tie-breaker) so ordering becomes
deterministic; ensure you apply the same change to every similar query in this
file.
🪄 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: Pro
Run ID: 606b37b9-15b0-46d3-b544-e6851e66741c
📒 Files selected for processing (9)
AGENTS.mdbackend/api/ai_hub.pybackend/main.pybackend/tests/test_ai_hub_api.pydocs/plans/2026-05-29-ai-hub-source-backed-surface.mdfrontend/src/app/ai-hub/page.test.tsxfrontend/src/components/AIHubLayout.tsxfrontend/tests/e2e/ai-hub-source-surface.spec.tsfrontend/tests/e2e/helpers.ts
|
PR governance metadata gate is not ready for
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/tests/test_ai_hub_api.py (1)
168-188:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve existing dependency overrides in this helper.
app.dependency_overrides.clear()wipes unrelated overrides from outer fixtures, so this helper can make adjacent tests order-dependent. Snapshot and restore the original mapping here the same way the Postgres smoke path does.Suggested fix
def _request_with_signed_session( db_session: MockSession, **payload_overrides: object, ): previous_secret = settings.AUTH_SESSION_HMAC_SECRET + original_overrides = dict(app.dependency_overrides) async def scoped_db(): yield db_session settings.AUTH_SESSION_HMAC_SECRET = SecretStr(TEST_SESSION_HMAC_SECRET) app.dependency_overrides[get_db] = scoped_db @@ finally: settings.AUTH_SESSION_HMAC_SECRET = previous_secret app.dependency_overrides.clear() + app.dependency_overrides.update(original_overrides)🤖 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 `@backend/tests/test_ai_hub_api.py` around lines 168 - 188, The helper _request_with_signed_session currently clears app.dependency_overrides at the end which removes unrelated overrides; instead snapshot the original mapping, set app.dependency_overrides[get_db] = scoped_db for the test, and restore the original mapping in the finally block; also preserve and restore settings.AUTH_SESSION_HMAC_SECRET as already done — i.e., save original_overrides = dict(app.dependency_overrides) before mutating, assign the scoped override for get_db, and in finally restore app.dependency_overrides = original_overrides to avoid breaking other tests.
🧹 Nitpick comments (1)
backend/tests/test_ai_hub_api.py (1)
168-180: ⚡ Quick winAdd a regression for unsupported JWT
critheaders.The new signed-session coverage never exercises the
crit-header rejection boundary, and this helper currently makes that hard by hardcoding the JWT header. Please allow header overrides and add a case asserting/api/ai-hub/surfacerejects a bearer token carrying unsupportedcritvalues.As per coding guidelines, "JWT/session verification must reject unsupported critical headers (
crit) before trusting payload claims; do not rely only on library defaults for this boundary."Also applies to: 191-253
🤖 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 `@backend/tests/test_ai_hub_api.py` around lines 168 - 180, The helper _request_with_signed_session currently hardcodes JWT headers and thus cannot exercise rejection of unsupported critical (`crit`) headers; modify _request_with_signed_session to accept an optional headers_override argument (pass-through to where _signed_session_token is called) so tests can inject a JWT with a `crit` claim, ensure _signed_session_token/validation path uses that header when building the token, and add a regression test that sends a bearer token with an unsupported crit value to the /api/ai-hub/surface endpoint and asserts it is rejected (unauthorized/400) to confirm JWT/session verification rejects unsupported critical headers before trusting payload claims.
🤖 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 `@backend/tests/test_ai_hub_api.py`:
- Around line 258-264: The test function
test_ai_hub_surface_postgres_smoke_uses_signed_scope currently calls
create_async_engine(settings.DATABASE_URL, ...) before any guard, which will
hard-fail when DATABASE_URL is unset; modify the test to either short-circuit
with pytest.skip when settings.DATABASE_URL is falsy or move the
create_async_engine call into the try/guard block so engine is only constructed
after verifying the URL, and use pytest.skip(...) with a clear message if
settings.DATABASE_URL is missing.
---
Outside diff comments:
In `@backend/tests/test_ai_hub_api.py`:
- Around line 168-188: The helper _request_with_signed_session currently clears
app.dependency_overrides at the end which removes unrelated overrides; instead
snapshot the original mapping, set app.dependency_overrides[get_db] = scoped_db
for the test, and restore the original mapping in the finally block; also
preserve and restore settings.AUTH_SESSION_HMAC_SECRET as already done — i.e.,
save original_overrides = dict(app.dependency_overrides) before mutating, assign
the scoped override for get_db, and in finally restore app.dependency_overrides
= original_overrides to avoid breaking other tests.
---
Nitpick comments:
In `@backend/tests/test_ai_hub_api.py`:
- Around line 168-180: The helper _request_with_signed_session currently
hardcodes JWT headers and thus cannot exercise rejection of unsupported critical
(`crit`) headers; modify _request_with_signed_session to accept an optional
headers_override argument (pass-through to where _signed_session_token is
called) so tests can inject a JWT with a `crit` claim, ensure
_signed_session_token/validation path uses that header when building the token,
and add a regression test that sends a bearer token with an unsupported crit
value to the /api/ai-hub/surface endpoint and asserts it is rejected
(unauthorized/400) to confirm JWT/session verification rejects unsupported
critical headers before trusting payload claims.
🪄 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: Pro
Run ID: 8ddcc8be-ef3c-4021-9755-dc8743d35111
📒 Files selected for processing (4)
backend/api/ai_hub.pybackend/tests/test_ai_hub_api.pydocs/plans/2026-05-29-ai-hub-source-backed-surface.mdfrontend/src/app/ai-hub/page.test.tsx
✅ Files skipped from review due to trivial changes (1)
- docs/plans/2026-05-29-ai-hub-source-backed-surface.md
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/ai-hub/page.test.tsx
- backend/api/ai_hub.py
Summary
GET /api/ai-hub/surfacebacked by prompt templates, organization LLM provider metadata, and durable provider audit events.Verification
PYTHONDONTWRITEBYTECODE=1 python3 -m pytest -qfrombackend/→ 510 passed, 13 skippedPYTHONDONTWRITEBYTECODE=1 python3 -m bandit -r backend/ -x backend/tests/ -q→ passednpm run lint→ passednpm run typecheck→ passednpm test→ 103 passedenv -u FORCE_COLOR -u NO_COLOR PLAYWRIGHT_PORT=18130 npm run test:e2e -- --project=desktop tests/e2e/ai-hub-source-surface.spec.ts tests/e2e/mobile-hamburger.spec.ts→ 4 passedgit diff --check→ passedBrowser screenshots inspected
frontend/test-results/ai-hub-source-surface-rend-39496--Hub-with-scroll-at-desktop-desktop/ai-hub-desktop-scrolled.pngfrontend/test-results/ai-hub-source-surface-rend-32190-I-Hub-with-scroll-at-tablet-desktop/ai-hub-tablet-scrolled.pngfrontend/test-results/ai-hub-source-surface-rend-73ede-I-Hub-with-scroll-at-mobile-desktop/ai-hub-mobile-scrolled.pngfrontend/test-results/ai-hub-source-surface-rend-73ede-I-Hub-with-scroll-at-mobile-desktop/ai-hub-mobile-hamburger.pngfrontend/test-results/mobile-hamburger-Mobile-Wo-024c1-gles-and-displays-correctly-desktop/mobile-hamburger-open-scrolled.pngSummary by CodeRabbit
New Features
Tests
Documentation