feat: Plan 9 PR B — provider read APIs + provider view - #27
Conversation
CP: - authenticateOidc validates X-Tenant-Id against tenant_memberships (403 on mismatch; falls back to first membership when absent) — Decision 4 - GET /v1/policies/active (tenant-scoped) - GET /v1/decisions/aggregates|samples|agents (tenant-scoped reads; bounded from/to window: default 24h, max 31d) - GET /v1/scores/:principalId/history (current + network_score_history) - GET /v1/graph/summary (counts, top issuers, latest score write time) - GET /v1/edge-nodes (tenant fleet + sync lag fields) - decisions router: /batch stays apiKeyOnly; reads use authMiddleware - integration coverage for all new routes (authz, isolation, windows) Dashboard: - provider view: trust-summary stacked chart (recharts), agent list (score + blacklisted + score_reason; blacklist never inferred from score), sampled decision feed, edge sync status, active policy card - staleness banner when latest score write is older than 1h (Decision 8) - vitest: staleness + aggregate pivot + panel components
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThe control plane adds tenant-scoped provider read APIs for policies, decisions, scores, graph data, and edge nodes. The dashboard consumes these APIs and renders provider panels with charts, tables, synchronization status, policy data, and stale-score indicators. ChangesProvider read APIs
Provider dashboard
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProviderHomePage
participant provider_api
participant ControlPlane
participant Database
ProviderHomePage->>provider_api: fetch provider dashboard data
provider_api->>ControlPlane: request authenticated read APIs
ControlPlane->>Database: query tenant-scoped provider data
Database-->>ControlPlane: aggregates, samples, scores, nodes, policy
ControlPlane-->>provider_api: typed API responses
provider_api-->>ProviderHomePage: dashboard data
ProviderHomePage->>ProviderHomePage: pivot aggregates and compute staleness
ProviderHomePage-->>ProviderHomePage: render provider panels
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoPlan 9: Provider read APIs + dashboard provider view
AI Description
Diagram
High-Level Assessment
Files changed (34)
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@control-plane/src/domains/edgenode/edgeNodeRepository.ts`:
- Around line 23-35: Scope the hw CTE’s MAX(sync_version) calculation to the
caller tenant by filtering sync_events with the query’s tenant parameter before
computing high_water_version. Keep the outer edge-node and cursor joins
unchanged, and add an integration case where another tenant has a later
sync_events row to verify the reported lag and high-water mark remain
tenant-specific.
In `@control-plane/src/lib/timeWindow.ts`:
- Around line 7-13: Update parseOptional to validate non-empty string inputs
against the accepted ISO 8601 timestamp format before calling new Date,
rejecting values such as “January 2, 2024” and “01/02/2024” with the existing
BAD_REQUEST error. Add tests covering these non-ISO inputs while preserving null
handling and valid ISO parsing.
In `@control-plane/src/routes/scores.ts`:
- Around line 21-28: Enforce tenant scoping in both handlers: in scores.ts,
require req.user.tenantId and pass it to the scoreRead methods so principal
authorization occurs before returning score data; in graph.ts, require the
tenant ID and use the tenant-scoped graph-summary query. Add integration tests
using tenant B credentials to verify tenant A’s score and graph data, including
global counts, issuer activity, and score-write metadata, cannot be read.
Affected sites: control-plane/src/routes/scores.ts lines 21-28 and
control-plane/src/routes/graph.ts lines 20-22.
In `@dashboard/src/pages/ProviderHomePage.tsx`:
- Around line 41-49: Update the six useQuery definitions in ProviderHomePage,
including summaryQuery, aggregatesQuery, agentsQuery, samplesQuery, edgesQuery,
and policyQuery, to include the active tenant ID in each queryKey so tenant
changes partition cached data and trigger refetches. Add a tenant-switch test
verifying that results from the previous tenant are not rendered after switching
tenants.
In `@dashboard/src/styles/index.css`:
- Around line 263-266: Update the color declaration in .badge--passthrough so
its text achieves at least 4.5:1 contrast against the existing `#f0ead2`
background, while preserving the badge’s current styling and structure.
🪄 Autofix
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: ASSERTIVE
Plan: Pro
Run ID: e4bf1cc9-a4f9-4b10-b19e-38321410c258
⛔ Files ignored due to path filters (1)
dashboard/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
control-plane/src/__tests__/integration/dashboard-reads.test.tscontrol-plane/src/app.tscontrol-plane/src/domains/decision/decisionReadRepository.tscontrol-plane/src/domains/edgenode/edgeNodeRepository.tscontrol-plane/src/domains/policy/policyRepository.tscontrol-plane/src/domains/score/scoreReadRepository.tscontrol-plane/src/lib/activeMembership.test.tscontrol-plane/src/lib/activeMembership.tscontrol-plane/src/lib/timeWindow.test.tscontrol-plane/src/lib/timeWindow.tscontrol-plane/src/middleware/auth.tscontrol-plane/src/routes/decisions.tscontrol-plane/src/routes/edgeNodes.tscontrol-plane/src/routes/graph.tscontrol-plane/src/routes/policies.tscontrol-plane/src/routes/scores.tscontrol-plane/src/testutil/testDb.tsdashboard/package.jsondashboard/src/api/provider.tsdashboard/src/lib/aggregates.test.tsdashboard/src/lib/aggregates.tsdashboard/src/lib/staleness.test.tsdashboard/src/lib/staleness.tsdashboard/src/pages/ProviderHomePage.tsxdashboard/src/pages/provider/AgentList.tsxdashboard/src/pages/provider/DecisionFeed.tsxdashboard/src/pages/provider/EdgeSyncStatus.tsxdashboard/src/pages/provider/PolicyCard.tsxdashboard/src/pages/provider/StalenessBanner.tsxdashboard/src/pages/provider/TrustSummaryChart.tsxdashboard/src/pages/provider/components.test.tsxdashboard/src/styles/index.cssdocs/superpowers/plans/HANDOVER.md
| `WITH hw AS ( | ||
| SELECT COALESCE(MAX(sync_version), 0) AS high_water_version FROM sync_events | ||
| ) | ||
| SELECT en.id, en.name, en.status, en.last_seen_at, | ||
| en.last_sync_version::text AS last_sync_version, en.created_at, | ||
| sc.last_cursor::text AS last_cursor, sc.last_sync_at, | ||
| hw.high_water_version::text AS high_water_version, | ||
| (hw.high_water_version - COALESCE(sc.last_cursor, 0))::text AS lag | ||
| FROM edge_nodes en | ||
| LEFT JOIN sync_cursors sc | ||
| ON sc.tenant_id = en.tenant_id AND sc.edge_node_id = en.id | ||
| CROSS JOIN hw | ||
| WHERE en.tenant_id = $1 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Scope the sync high-water mark to the caller tenant.
Line 23 calculates high_water_version from every sync_events row. The outer query returns one tenant’s edges and cursors.
For example, a tenant A edge at cursor 5 reports a lag of 95 when tenant A has events only through version 10 and tenant B has an event at version 100. This also exposes cross-tenant synchronization progress.
Proposed fix
`WITH hw AS (
- SELECT COALESCE(MAX(sync_version), 0) AS high_water_version FROM sync_events
+ SELECT COALESCE(MAX(sync_version), 0) AS high_water_version
+ FROM sync_events
+ WHERE tenant_id = $1
)Add an integration case with a later sync_events row for another tenant.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `WITH hw AS ( | |
| SELECT COALESCE(MAX(sync_version), 0) AS high_water_version FROM sync_events | |
| ) | |
| SELECT en.id, en.name, en.status, en.last_seen_at, | |
| en.last_sync_version::text AS last_sync_version, en.created_at, | |
| sc.last_cursor::text AS last_cursor, sc.last_sync_at, | |
| hw.high_water_version::text AS high_water_version, | |
| (hw.high_water_version - COALESCE(sc.last_cursor, 0))::text AS lag | |
| FROM edge_nodes en | |
| LEFT JOIN sync_cursors sc | |
| ON sc.tenant_id = en.tenant_id AND sc.edge_node_id = en.id | |
| CROSS JOIN hw | |
| WHERE en.tenant_id = $1 | |
| `WITH hw AS ( | |
| SELECT COALESCE(MAX(sync_version), 0) AS high_water_version | |
| FROM sync_events | |
| WHERE tenant_id = $1 | |
| ) | |
| SELECT en.id, en.name, en.status, en.last_seen_at, | |
| en.last_sync_version::text AS last_sync_version, en.created_at, | |
| sc.last_cursor::text AS last_cursor, sc.last_sync_at, | |
| hw.high_water_version::text AS high_water_version, | |
| (hw.high_water_version - COALESCE(sc.last_cursor, 0))::text AS lag | |
| FROM edge_nodes en | |
| LEFT JOIN sync_cursors sc | |
| ON sc.tenant_id = en.tenant_id AND sc.edge_node_id = en.id | |
| CROSS JOIN hw | |
| WHERE en.tenant_id = $1 |
🤖 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 `@control-plane/src/domains/edgenode/edgeNodeRepository.ts` around lines 23 -
35, Scope the hw CTE’s MAX(sync_version) calculation to the caller tenant by
filtering sync_events with the query’s tenant parameter before computing
high_water_version. Keep the outer edge-node and cursor joins unchanged, and add
an integration case where another tenant has a later sync_events row to verify
the reported lag and high-water mark remain tenant-specific.
| async handler(req, res) { | ||
| const principalId = req.params.principalId as string; | ||
| const limit = req.query.limit ? parseInt(req.query.limit as string, 10) : 100; | ||
| const [current, items] = await Promise.all([ | ||
| scoreRead.getCurrentScore(principalId), | ||
| scoreRead.getScoreHistory(principalId, limit), | ||
| ]); | ||
| ok(res, { principal_id: principalId, current, items }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Enforce tenant scope for score and graph reads.
The PR objective requires tenant-scoped score and graph reads. These handlers authenticate the caller but do not use req.user.tenantId.
Any authenticated tenant can request a known principalId score history. Any authenticated tenant can also retrieve global principal counts, issuer activity, and score-write metadata.
control-plane/src/routes/scores.ts#L21-L28: require a tenant ID and pass it to repository methods that authorize the principal before returning its score data.control-plane/src/routes/graph.ts#L20-L22: require a tenant ID and pass it to a tenant-scoped graph-summary query.- Add integration tests that use tenant B credentials against tenant A score and graph data.
📍 Affects 2 files
control-plane/src/routes/scores.ts#L21-L28(this comment)control-plane/src/routes/graph.ts#L20-L22
🤖 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 `@control-plane/src/routes/scores.ts` around lines 21 - 28, Enforce tenant
scoping in both handlers: in scores.ts, require req.user.tenantId and pass it to
the scoreRead methods so principal authorization occurs before returning score
data; in graph.ts, require the tenant ID and use the tenant-scoped graph-summary
query. Add integration tests using tenant B credentials to verify tenant A’s
score and graph data, including global counts, issuer activity, and score-write
metadata, cannot be read. Affected sites: control-plane/src/routes/scores.ts
lines 21-28 and control-plane/src/routes/graph.ts lines 20-22.
- timeWindow: reject Date-parseable but non-ISO 8601 forms (e.g. 01/02/2024) to avoid ambiguous from/to; accept date-only + zoned timestamps. (+tests) - ProviderHomePage: embed active tenant id in query keys so a tenant switch partitions the cache and refetches instead of showing stale data. (providerQueryKeys helper + tests) - badge--passthrough: darken text to #6b5d23 (5.4:1 on #f0ead2) for WCAG AA. Skipped (replied inline): - edge-node high-water tenant scoping: sync_version is a global monotonic counter (global advisory-lock allocator); matches merged listSyncLag (#16). - scores/graph tenant scoping: trust graph + network scores are global/shared by Plan 9 Decision 5; any authenticated tenant member may read them.
|
@coderabbitai review Addressed the actionable findings in
Two findings intentionally skipped, with rationale:
|
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Changed files in the follow-up commit =='
git show --stat --oneline 8681675
echo
echo '== Relevant symbols and changed regions =='
for file in \
control-plane/src/lib/timeWindow.ts \
control-plane/src/lib/timeWindow.test.ts \
dashboard/src/pages/ProviderHomePage.tsx \
dashboard/src/styles/index.css \
control-plane/src/domains/edgenode/edgeNodeRepository.ts \
control-plane/src/domains/score/scoreReadRepository.ts
do
if [ -f "$file" ]; then
echo "--- $file ---"
ast-grep outline "$file" --items all 2>/dev/null || true
fi
done
echo
echo '== Follow-up diff =='
git show --format=fuller --find-renames 8681675 -- \
control-plane/src/lib/timeWindow.ts \
control-plane/src/lib/timeWindow.test.ts \
dashboard/src/pages/ProviderHomePage.tsx \
dashboard/src/styles/index.css \
control-plane/src/domains/edgenode/edgeNodeRepository.ts \
control-plane/src/domains/score/scoreReadRepository.ts
echo
echo '== Global sync counter and existing admin lag implementation =='
rg -n -C 4 --glob '*.{ts,sql}' \
'pg_advisory_xact_lock\(8392018\)|sync_version|listSyncLag|global_high_water|high.water' \
control-plane
echo
echo '== Plan 9 Decision 5 references =='
rg -n -C 3 --glob '*.{md,ts}' \
'Decision 5|shared.*trust graph|network_scores|graph/summary|score history' \
docs control-plane 2>/dev/null || true
echo
echo '== Provider query-key tests =='
rg -n -C 5 --glob '*.{test,spec}.{ts,tsx}' \
'providerQueryKeys|ProviderHomePage|tenant' \
dashboardLength of output: 108605 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Query keys and active tenant source =='
for file in dashboard/src/lib/queryKeys.ts dashboard/src/auth/AuthProvider.tsx dashboard/src/api/provider.ts; do
if [ -f "$file" ]; then
echo "--- $file ---"
cat -n "$file"
fi
done
echo
echo '== Tenant header construction and provider endpoint usage =='
rg -n -C 4 --glob '*.{ts,tsx}' \
'X-Tenant-Id|tenantId|fetchGraphSummary|fetchAggregates|fetchAgents|fetchSamples|fetchEdgeNodes|fetchActivePolicy' \
dashboard/src
echo
echo '== Full PR diff against merge base =='
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
git diff --check "$base"...HEAD
git diff --stat "$base"...HEAD
git diff --find-renames "$base"...HEAD -- \
control-plane/src \
dashboard/src \
dashboard/package.json \
control-plane/package.json
echo
echo '== Offset-free date-time regression candidates =='
rg -n -C 4 --glob '*.{ts,tsx}' \
'resolveTimeWindow\(|ISO_8601_RE|T\d{2}:\d{2}(?!.*(?:Z|[+-]\d{2}:?\d{2}))' \
control-plane/srcLength of output: 21521
The provider query keys now partition the cache by active tenant. The passthrough badge color now meets the stated contrast target. I withdraw the edge-node high-water and graph/score scoping findings. The implementation matches the global sync stream and shared-graph design. One issue remains in ✏️ Learnings added
|
Code Review by Qodo
1.
|
| const tenantId = requireTenant(req); | ||
| const { from, to } = resolveTimeWindow(req.query.from, req.query.to); | ||
| const limit = req.query.limit ? parseInt(req.query.limit as string, 10) : 50; | ||
| const items = await decisionRead.getSamples({ tenantId, from, to, limit }); |
There was a problem hiding this comment.
3. Limit parse/validate mismatch 🐞 Bug ≡ Correctness
New routes validate limit using Number() (so inputs like 1e2 or 1.5 pass min/max checks) but then parse with parseInt(), causing the executed limit to differ from the validated value. This produces incorrect paging/limits and can lead to confusing API behavior for the new read endpoints.
Agent Prompt
### Issue description
`defineHandler` validates `type: 'number'` using `Number(str)`, but new handlers parse `limit` via `parseInt()`. Values like `limit=1e2` validate as 100 but execute as 1.
### Issue Context
This pattern appears in the new decision read routes and the new scores history route.
### Fix Focus Areas
- control-plane/src/shared/http/defineHandler.ts[20-39]
- control-plane/src/routes/decisions.ts[76-86]
- control-plane/src/routes/decisions.ts[99-110]
- control-plane/src/routes/scores.ts[18-28]
### Suggested fix
Pick one consistent semantic and apply it end-to-end:
- If `limit` must be an integer (recommended for SQL `LIMIT`):
- In each handler, parse with a strict integer check:
- `const raw = req.query.limit as string | undefined;`
- `if (raw && !/^\d+$/.test(raw)) throw new AppError(CODES.BAD_REQUEST, 'limit must be an integer');`
- `const limit = raw ? parseInt(raw, 10) : <default>;`
- Keep the existing min/max constraints (or re-check after parsing).
- Alternatively, enhance `defineHandler` to support an explicit integer type (e.g., `type: 'int'`) and reuse it for `limit`/`offset` across routes.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| FROM decision_samples | ||
| WHERE tenant_id = $1 | ||
| AND decided_at >= $2 | ||
| AND decided_at <= $3 |
There was a problem hiding this comment.
4. Read apis lack indexes 🐞 Bug ➹ Performance
The new decision read APIs filter decision_samples by tenant_id and decided_at (and group by principal_id), but the schema only defines UNIQUE(edge_node_id, wal_seq); as data grows this creates a high risk of expensive scans/sorts for the Provider dashboard reads. decision_aggregates likewise lacks an index aligned to the new tenant+time+dimension query pattern (its unique index is keyed by (tenant_id, edge_node_id, ...), but the read query does not constrain edge_node_id).
Agent Prompt
### Issue description
New read queries over `decision_samples`/`decision_aggregates` are tenant+time bounded and ordered, but there are no supporting indexes for those access patterns in the current schema.
### Issue Context
- `getSamples` reads by `tenant_id` + `decided_at` range and orders by `decided_at DESC`.
- `listAgents` reads by `tenant_id` + `decided_at` range and groups by `principal_id`.
- `getAggregates` reads by `tenant_id` + `bucket_minute` range + `dimension_kind`, but the existing unique index is keyed by `(tenant_id, edge_node_id, ...)` and the query does not filter by `edge_node_id`.
### Fix Focus Areas
- control-plane/src/domains/decision/decisionReadRepository.ts[14-33]
- control-plane/src/domains/decision/decisionReadRepository.ts[49-68]
- control-plane/src/domains/decision/decisionReadRepository.ts[82-112]
- control-plane/migrations/006_audit/migration.sql[23-53]
### Suggested fix
Add migrations to create indexes aligned with the new reads (validate with `EXPLAIN` on representative data):
- For samples feed:
- `CREATE INDEX ... ON decision_samples (tenant_id, decided_at DESC, id DESC);`
- For agents aggregation (optional depending on plan):
- `CREATE INDEX ... ON decision_samples (tenant_id, decided_at, principal_id);`
- For aggregates rollups:
- `CREATE INDEX ... ON decision_aggregates (tenant_id, dimension_kind, bucket_minute);`
Consider `CONCURRENTLY` if applied to a live DB, and ensure indexes don’t unduly impact ingest write throughput.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Plan 9 PR B — provider data path
@coderabbitai review
Implements the PR B slice of
docs/superpowers/plans/2026-08-01-plan-9-dashboard.md(PR A shell merged in #26).Control plane
X-Tenant-Idvalidation inauthenticateOidc(Decision 4): header validated againsttenant_memberships, sets activetenantId/role; 403 on mismatch; falls back to first membership when absent. Pure helperselectActiveMembership+ unit tests.GET /v1/policies/active— active policy for caller tenant (404 when none).GET /v1/decisions/aggregates?from&to&dimension— per-minute rollups summed across edges; window defaults to last 24h, capped at 31d (risk note).GET /v1/decisions/samples?from&to&limit— sampled feed, newest first (limit <= 500).GET /v1/decisions/agents?from&to&limit— provider agent list: distinct principals in the tenant's sampled decisions joined to currentnetwork_scores(blacklisted/score_reasonsurfaced from the score row only, never inferred — Decision 5).GET /v1/scores/:principalId/history— current score +network_score_historyitems.GET /v1/graph/summary— node/edge counts, top issuers by outgoing volume (read-only),latest_score_computed_atfor the staleness banner (Decision 8).GET /v1/edge-nodes— tenant fleet + sync lag vs high-water mark.decisionsrouter restructured:POST /batchstaysapiKeyOnly; reads useauthMiddleware(OIDC or API key).resetTestDatanow truncates decision/policy/edge tables for suite isolation.Dashboard
/provider): trust-summary stacked bar chart (recharts), agent list, sampled decision feed, edge sync status, read-only active policy card.Verification
tsc --noEmitclean; 129 unit + 30 integration tests pass (7 new integration tests cover authz, tenant isolation, window bounds).vite buildsucceeds.Out of scope (per plan): policy PUT, API-key CRUD, agent-builder view (PR C); billing + admin (PR D). No new migrations (Decision 12).
Summary by CodeRabbit