Take over app dependency and profile updates#109
Merged
therealityreport merged 14 commits intoJun 16, 2026
Conversation
- Remove user-settable displayName from admin authorization; require verified email; honor server-set admin custom claim (F-01) - Fail closed on missing CRON_SECRET in production for episode-progression, matching create-survey-runs (F-02) - Apply the admin host allowlist before trusting the propagated internal-admin JWT; reject non-HS256 alg headers (F-03) - Never honor the dev-admin bypass when VERCEL_ENV=production; refuse to boot if TRR_DEV_ADMIN_BYPASS is set on a production deploy (F-04) - Document src/proxy.ts as host-isolation-only Next 16 middleware - Add regression tests for every gate (cron fail-closed, bypass gating, host-before-JWT ordering, forged-alg rejection)
…publish/trr-app/20260616-takeover-all
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change set updates local admin routing to portless HTTPS hosts, tightens admin auth and internal token checks, adds request-aware admin responses, introduces social shared-source fallback status flows, adds show-core auto-refresh orchestration, and extends social profile admin tooling for comments recovery and telemetry. ChangesAdmin platform updates
Sequence Diagram(s)sequenceDiagram
participant Proxy
participant AdminUrlDefaults
participant PortlessStatusRoute
participant PortlessStatus
participant AdminPage
Proxy->>AdminUrlDefaults: resolve admin origin and host
PortlessStatusRoute->>PortlessStatus: getPortlessStatus()
PortlessStatus-->>AdminPage: status snapshot
AdminPage->>Proxy: navigate to clean admin URL
sequenceDiagram
participant SocialPage
participant LandingRoute
participant LandingRepository
participant SocialBackend
participant LocalDb
SocialPage->>LandingRoute: GET /api/admin/social/landing
LandingRoute->>LandingRepository: getSocialLandingPayloadResult()
LandingRepository->>SocialBackend: fetch /shared/sources
SocialBackend-->>LandingRepository: error or source data
LandingRepository->>LocalDb: loadSharedAccountSourcesFromLocalDb()
LocalDb-->>LandingRepository: fallback source rows
LandingRepository-->>LandingRoute: payload with shared_source_status
LandingRoute-->>SocialPage: landing payload
sequenceDiagram
participant ShowsPage
participant AutoRefreshSettingsRoute
participant ShowRefreshStream
participant ShowDetailPage
ShowsPage->>AutoRefreshSettingsRoute: GET show-core-auto-refresh
AutoRefreshSettingsRoute-->>ShowsPage: paused/unpaused
ShowsPage->>ShowRefreshStream: POST show_core refresh
ShowRefreshStream-->>ShowsPage: operation/progress/complete events
ShowsPage->>ShowDetailPage: navigate with showCoreOperationId
Estimated code review effort🎯 5 (Critical) | ⏱️ ~110 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR aggregates dependency/profile updates and “phase-0” admin hardening, with a strong focus on Portless-based local HTTPS routes, admin/auth security tightening, and expanded social-admin observability + fallback behaviors.
Changes:
- Update runtime/tooling dependencies (Node/Next/React/Firebase/Vitest/etc.) and add node baseline smoke + Portless helper scripts.
- Harden admin/auth flows (email verification requirement, custom-claim support, internal-admin token parsing/host binding, cron route auth fail-closed).
- Expand admin social/show tooling (Portless status surfaces, shared-source fallback + status reporting, backend progress rollup usage, route timing headers, show-core pre-navigation refresh + auto-refresh controls).
Reviewed changes
Copilot reviewed 70 out of 77 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/portless-banner.mjs | New helper to print clean Portless URLs. |
| scripts/node-baseline-smoke.sh | New node/pnpm baseline smoke script. |
| requirements.lock.txt | Python dependency lock refresh. |
| package.json | Root deps/scripts updated (Portless dev flow, smoke script, dep bumps). |
| docs/cross-collab/rhoslc-media-refresh-e2e-checklist.md | Update checklist URLs/commands for Portless. |
| apps/web/tests/social-shared-sources-route.test.ts | New tests for shared-sources API fallback. |
| apps/web/tests/social-landing-repository.test.ts | Expand social landing repo tests (timeouts, rollup, fallback). |
| apps/web/tests/social-admin-proxy.test.ts | Add local-proxy header assertions for loopback backend. |
| apps/web/tests/social-account-profile-read-routes.test.ts | Assert timing headers + local document rendering behavior. |
| apps/web/tests/social-account-profile-page.runtime.test.tsx | Add UI/runtime coverage for new progress + month-window behavior. |
| apps/web/tests/social-account-comments-audit-cursor-retries-route.test.ts | New tests for audit-cursor-retries proxy route. |
| apps/web/tests/server-auth-adapter.test.ts | Add/adjust auth tests (verified emails, bypass restrictions, host binding). |
| apps/web/tests/internal-admin-auth.test.ts | Extend internal-admin token validation tests (segments/alg/host). |
| apps/web/tests/cron-routes-auth.test.ts | New tests for cron auth guards (fail closed in prod). |
| apps/web/tests/client-admin-access.test.ts | Update client-admin capability rules (no displayName-based admin). |
| apps/web/tests/admin-social-page-auth-bypass.test.tsx | Update cache key + request expectations. |
| apps/web/tests/admin-portless-status.test.ts | New tests for parsing/snapshotting portless list. |
| apps/web/tests/admin-host-middleware.test.ts | Update middleware expectations for Portless + legacy fallback behavior. |
| apps/web/tests/admin-backend-proxy-route.test.ts | Add local HTML document rendering test for JSON proxy routes. |
| apps/web/src/proxy.ts | Middleware refactor to shared admin URL helpers; redirect root to /admin on canonical admin host. |
| apps/web/src/lib/server/trr-api/social-profile-route-factory.ts | Add local HTML document response support + optional timing headers. |
| apps/web/src/lib/server/trr-api/social-admin-proxy.ts | Mark loopback backend requests with x-trr-local-admin-proxy. |
| apps/web/src/lib/server/trr-api/local-api-document-response.ts | New helper to render HTML docs for direct local API navigation. |
| apps/web/src/lib/server/trr-api/internal-admin-auth.ts | Strengthen token parsing/alg validation; add host binding claim support. |
| apps/web/src/lib/server/trr-api/admin-backend-proxy-route.ts | Use adminJsonResponse for consistent local-document behavior. |
| apps/web/src/lib/server/postgres.ts | Add queryWithStatementTimeout helper. |
| apps/web/src/lib/server/auth.ts | Harden admin auth rules (verified email, custom claim, bypass disabled on prod deploys, host-bound internal context). |
| apps/web/src/lib/server/admin/social-landing-repository.ts | Social landing: timing, optional timeouts, backend rollup usage, shared-source fallback + status metadata. |
| apps/web/src/lib/server/admin/shell-exec.ts | Allow portless as a safeExec command. |
| apps/web/src/lib/server/admin/shared-account-sources.ts | New local-db loader/normalizers for shared social sources. |
| apps/web/src/lib/server/admin/portless-status.ts | New Portless status collector/parser/snapshot builder. |
| apps/web/src/lib/server/admin/admin-snapshot-route.ts | Allow buildSnapshotResponse to use adminJsonResponse when request is available. |
| apps/web/src/lib/fonts/brand-fonts/generated/normalized-r2-font-catalog.json | Regenerated font catalog timestamp. |
| apps/web/src/lib/fonts/brand-fonts/generated/normalized-brand-font-registry.json | Regenerated font registry timestamp. |
| apps/web/src/lib/fonts/brand-fonts/generated/discovered-brand-font-snapshot.json | Regenerated discovered font snapshot timestamp. |
| apps/web/src/lib/fonts/brand-fonts/generated/brand-font-match-results.json | Regenerated font match results timestamp. |
| apps/web/src/lib/admin/social-landing.ts | Add shared-source load status type + payload field. |
| apps/web/src/lib/admin/social-account-profile.ts | Extend social profile/admin types for new backend fields. |
| apps/web/src/lib/admin/client-access.ts | Remove displayName-based admin gating; require verified email or allowlisted UID. |
| apps/web/src/lib/admin/admin-url-defaults.ts | New centralized helpers/constants for admin/Portless URL behavior. |
| apps/web/src/components/admin/SocialAccountProfilePage.tsx | UI updates: month window backfill, progress metrics, transient error handling, sort metadata display. |
| apps/web/src/components/admin/RedditAdminShell.tsx | Switch to AdminGlobalHeader + AdminBreadcrumbs. |
| apps/web/src/components/admin/AdminGlobalHeader.tsx | Warn on portful Portless admin fallback and suggest clean URL. |
| apps/web/src/app/api/cron/episode-progression/route.ts | Fail-closed cron auth in production when CRON_SECRET missing. |
| apps/web/src/app/api/admin/trr-api/social/shared/sources/route.ts | Add local-db fallback response payload/headers when backend unavailable. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/summary/route.ts | Enable proxy route timing headers. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/snapshot/route.ts | Pass request into snapshot builder for local doc responses. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/posts/route.ts | Simplify timeout + enable timing headers. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/hashtags/route.ts | Enable timing headers. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/comments/route.ts | Enable timing headers. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/comments/audit-cursor-retries/route.ts | New audit-cursor-retries proxy route (GET/POST). |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/collaborators-tags/route.ts | Enable timing headers. |
| apps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/catalog/review-queue/route.ts | Enable timing headers. |
| apps/web/src/app/api/admin/trr-api/social/ingest/live-status/route.ts | Pass request into snapshot builder. |
| apps/web/src/app/api/admin/trr-api/social-growth/cast-comparison/snapshot/route.ts | Pass request into snapshot builder. |
| apps/web/src/app/api/admin/trr-api/shows/settings/show-core-auto-refresh/route.ts | New backend proxy route for show-core auto-refresh settings (GET/PUT). |
| apps/web/src/app/api/admin/trr-api/shows/[showId]/seasons/[seasonNumber]/social/analytics/week/[weekIndex]/snapshot/route.ts | Pass request into snapshot builder. |
| apps/web/src/app/api/admin/trr-api/shows/[showId]/seasons/[seasonNumber]/social/analytics/snapshot/route.ts | Pass request into snapshot builder. |
| apps/web/src/app/api/admin/social/landing/route.ts | Avoid serving stale cache when shared sources are in fallback mode. |
| apps/web/src/app/api/admin/reddit/communities/[communityId]/backfill/snapshot/route.ts | Pass request into snapshot builder. |
| apps/web/src/app/api/admin/portless/status/route.ts | New admin route to expose Portless status snapshot. |
| apps/web/src/app/api/admin/health/instagram-comment-rollups/route.ts | New backend-proxy admin health route. |
| apps/web/src/app/admin/trr-shows/page.tsx | Add pre-navigation show-core refresh + UI state. |
| apps/web/src/app/admin/trr-shows/[showId]/page.tsx | Add show-core auto refresh controls + warning-driven auto refresh behaviors. |
| apps/web/src/app/admin/social/page.tsx | Add shared-source fallback warning + cache key bump + raw-source link. |
| apps/web/src/app/admin/page.tsx | Add Portless URL/status card + diagnostics copy workflow. |
| apps/web/scripts/cast-smoke-preflight.mjs | Default admin origin now prefers Portless URLs. |
| apps/web/scripts/browser-smoke-admin-detail-routes.mjs | Default base URL now prefers Portless URLs. |
| apps/web/README.md | Document Portless-first local workflow and legacy fallback flag. |
| apps/web/package.json | Update Next/React/tooling deps; allow custom dev port via PORT env var. |
| apps/web/.env.example | Update admin origin defaults + add TRR_LEGACY_LOCAL_ADMIN_FALLBACK. |
| apps/vue-wordle/package.json | Vue wordle dependency/tooling bumps. |
Files not reviewed (1)
- apps/vue-wordle/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
40
to
+44
| } catch (error) { | ||
| return socialProxyErrorResponse(error, "[api] Failed to fetch shared social account sources"); | ||
| const sourceScope = normalizeSharedAccountSourceScope( | ||
| request.nextUrl.searchParams.get("source_scope"), | ||
| ); | ||
| try { |
Comment on lines
+254
to
+261
| (error) => { | ||
| if (!timedOut) { | ||
| logSocialLandingTiming(label, startedAt, "error", { | ||
| error: error instanceof Error ? error.name : typeof error, | ||
| }); | ||
| } | ||
| throw error; | ||
| }, |
Comment on lines
+80
to
+93
| const normalizeAdminHost = (value: unknown): string | null => { | ||
| if (typeof value !== "string") { | ||
| return null; | ||
| } | ||
| const trimmed = value.trim().toLowerCase(); | ||
| if (!trimmed) { | ||
| return null; | ||
| } | ||
| if (trimmed.startsWith("[") && trimmed.includes("]")) { | ||
| return trimmed.slice(0, trimmed.indexOf("]") + 1); | ||
| } | ||
| const host = trimmed.split(":")[0]; | ||
| return host || null; | ||
| }; |
Comment on lines
+15
to
+16
| echo "packageManager=$(node -p "require('${APP_ROOT}/package.json').packageManager")" | ||
| echo "engines.node=$(node -p "require('${APP_ROOT}/package.json').engines.node")" |
Contributor
Codex Exhaustive Code ReviewFindings
Validation
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/api/admin/trr-api/social/shared/sources/route.ts (1)
31-74:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFail closed on auth errors before local fallback.
On Line 32,
requireAdminis inside the sametryas backend fetch. If auth fails, execution drops into the fallback block (Lines 41-70) and can return local shared-source rows to unauthorized callers.Suggested fix
export async function GET(request: NextRequest) { const queryString = request.nextUrl.searchParams.toString(); const backendEndpoint = buildSharedSourcesBackendEndpoint(queryString); + try { + await requireAdmin(request); + } catch (error) { + return socialProxyErrorResponse(error, "[api] Failed to fetch shared social account sources"); + } + try { - await requireAdmin(request); const data = await fetchSocialBackendJson("/shared/sources", { queryString, fallbackError: "Failed to fetch shared social account sources", retries: 0, timeoutMs: 30_000,🤖 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 `@apps/web/src/app/api/admin/trr-api/social/shared/sources/route.ts` around lines 31 - 74, The requireAdmin call on line 32 is inside the same try-catch block as fetchSocialBackendJson, which means if authentication fails, the error is caught and execution falls through to the fallback block that returns local shared-source data. This creates a security vulnerability where unauthorized users can access restricted sources. Move the requireAdmin call outside and above the try-catch block so that authentication failures are handled independently and fail immediately, before any fallback mechanism can execute.apps/web/src/components/admin/SocialAccountProfilePage.tsx (1)
6279-6318:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve shard activity when
worker_counters.totalis omitted.The new
worker_countersbranch setstotalto0when the backend does not sendtotal, even ifrunning,queued,failed, orretryingare present. That hidescommentsShardStatusLabelbecause it only renders whentotal > 0, and it also drops row-level issue reasons.🐛 Proposed fix
const commentsShardHealthSummary = useMemo(() => { + const rowIssueReasons = Array.from( + new Set( + activeCommentsProgressRankedRows + .map(({ shard }) => (shard.issueLabel ? formatDiagnosticToken(shard.issueLabel) : null)) + .filter((value): value is string => Boolean(value)), + ), + ).slice(0, 3); const counters = activeCommentsRunProgressData?.worker_counters; if (counters) { + const failed = readFiniteNumber(counters.failed) ?? 0; + const retrying = readFiniteNumber(counters.retrying) ?? 0; + const running = readFiniteNumber(counters.running) ?? 0; + const queued = readFiniteNumber(counters.queued) ?? 0; + const complete = readFiniteNumber(counters.completed) ?? 0; return { - failed: readFiniteNumber(counters.failed) ?? 0, - retrying: readFiniteNumber(counters.retrying) ?? 0, - running: readFiniteNumber(counters.running) ?? 0, - queued: readFiniteNumber(counters.queued) ?? 0, - complete: readFiniteNumber(counters.completed) ?? 0, - total: readFiniteNumber(counters.total) ?? 0, - issueReasons: [] as string[], + failed, + retrying, + running, + queued, + complete, + total: readFiniteNumber(counters.total) ?? failed + retrying + running + queued + complete, + issueReasons: rowIssueReasons, }; } let failed = 0; let retrying = 0; let running = 0; let queued = 0; let complete = 0; - const issueReasons = new Set<string>(); + const issueReasons = new Set<string>(rowIssueReasons);🤖 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 `@apps/web/src/components/admin/SocialAccountProfilePage.tsx` around lines 6279 - 6318, The commentsShardHealthSummary useMemo function sets total to 0 when the backend omits worker_counters.total, which prevents commentsShardStatusLabel from rendering and loses row-level issue reasons. Fix this by calculating total as the sum of the individual counter values (failed + retrying + running + queued + complete) instead of using counters.total directly, and ensure that issueReasons are still collected from activeCommentsProgressRankedRows by iterating through those rows even when worker_counters data is available, so that shard activity information is preserved in both code paths.
🤖 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 `@apps/web/scripts/browser-smoke-admin-detail-routes.mjs`:
- Around line 7-10: The DEFAULT_BASE_URL initialization is missing
process.env.TRR_APP_ORIGIN in its environment variable precedence chain, causing
it to diverge from cast-smoke-preflight. Add TRR_APP_ORIGIN to the conditional
chain of environment variables checked before falling back to the hardcoded
default URL, ensuring the precedence order matches the cast-smoke-preflight
script so both resolve the app origin consistently when TRR_APP_ORIGIN is the
only environment variable set.
In `@apps/web/src/app/admin/page.tsx`:
- Around line 97-100: The `isWrongAdminHost()` function hard-codes the admin
hostname to `admin.trr.localhost`, which incorrectly marks all other hosts
(including valid non-local admin environments) as wrong. Additionally, the
navigation logic around lines 383-401 hard-codes redirects to a local portless
URL. Replace the hard-coded `admin.trr.localhost` string with a configurable
admin host value (either from environment variables, configuration, or derived
from the current URL's domain structure) so that the host validation and
redirect logic work correctly across both local and non-local admin
environments.
In `@apps/web/src/app/admin/social/page.tsx`:
- Around line 2488-2555: The `networkSourceGroups` rendering block is currently
applied to every `network` in the map regardless of whether it matches the
`networkSourceSet`. Add a condition to check if `networkSourceSet.key ===
network.key` before rendering the `NetworkSourceGroupCard` components (around
line 2532 where `networkSourceGroups.length > 0` is checked). Only display the
grouped sources when the current network matches the networkSourceSet;
otherwise, fall through to render the `network.handles` as normal. This prevents
non-matching networks from displaying incorrect grouped sources and suppressing
their own handles.
In `@apps/web/src/app/admin/trr-shows/`[showId]/page.tsx:
- Around line 2820-2822: The autoShowCorePaused state defaults to false before
the actual persisted pause setting loads asynchronously, allowing the
auto-refresh effect to potentially enqueue show_core before the true pause state
is known. Introduce a new loading state (such as autoShowCorePauseLoaded)
initialized to false to track whether the pause setting has been fetched. In the
async loading effect that populates the pause state (around lines 10524-10553),
set both the actual pause value and mark the loading state as complete. In the
auto-refresh effect (lines 10582-10585), add a guard condition to only enqueue
show_core if autoShowCorePauseLoaded is true, ensuring the persisted pause
setting is honored before auto-refresh occurs.
In `@apps/web/src/app/admin/trr-shows/page.tsx`:
- Around line 238-243: Add a request id or abort signal mechanism to guard
against stale pre-navigation attempts in the show navigation flow. The issue
appears at two locations: in the pre-navigation request body (around lines
238-243) and in the subsequent router.push logic (around lines 269-289).
Implement a unique identifier for each navigation attempt and store it before
the async operation. Before calling router.push in the promise chain, verify
that the stored request id matches the current one to ensure the completed
promise is not stale. This prevents fast repeated clicks from enqueueing
duplicate show-core jobs and ensures older promises cannot navigate after the
user has clicked a different show.
- Around line 216-222: The current logic treats a non-OK settingsResponse the
same as a successful read with paused unset, allowing the operation to proceed
despite unknown pause state. Refactor the conditional to fail closed: first
check if settingsResponse.ok is false and return early with a skipped result or
throw an error to prevent further processing when the settings read fails, then
only proceed with the auto-refresh operation after confirming through a
successful response that paused is not true. This ensures the client does not
enqueue an operation while the pause state is unknown.
In
`@apps/web/src/app/api/admin/trr-api/shows/settings/show-core-auto-refresh/route.ts`:
- Around line 41-44: The catch block in the PUT request handler silently
converts JSON parsing errors into an empty object, allowing malformed payloads
to be processed as valid empty settings updates. Instead of catching and
replacing with {}, check if the JSON parsing fails and return a 400 Bad Request
response to the client. Additionally, after successfully parsing the JSON body,
validate that the result is actually an object (using typeof check or
instanceof) before forwarding it to the backend, and return 400 if the parsed
value is not an object. This ensures that only valid PUT payloads with proper
JSON objects are proxied forward.
In `@apps/web/src/components/admin/instagram/InstagramCommentsPanel.tsx`:
- Around line 715-721: The issue is that auditCursorRecovery state retains stale
data when a new show_filter load is initiated or fails, causing a mismatch
between the UI display of recovery targets and the actual filter being used in
the POST request. Clear auditCursorRecovery (set to null) at the beginning of
the refreshAuditCursorRecovery function when starting a new filter load, and
ensure it remains cleared if the load fails, so that recovery targets are always
synchronized with the currently loaded filter. Apply the same fix pattern at
lines 801-827 and 1329-1347 to maintain consistency across all affected code
locations.
- Around line 1559-1566: The search input field for the show-filter in
InstagramCommentsPanel.tsx lacks an accessible name for assistive technologies.
Add an aria-label attribute to the input element (the one with placeholder="Show
filter") that clearly describes the purpose of the field. Use descriptive text
like "Show filter" or "Filter audit cursor" to ensure screen readers can
properly announce the field's purpose to users.
- Around line 1382-1389: The networkSpend and targetProgressRows sections are
rendering regardless of the hideActiveRunProgress flag setting, while existing
progress sections properly respect this flag. Modify the rendering logic for
these sections to check the hideActiveRunProgress flag before displaying them.
This may involve either wrapping the useMemo calls conditionally or checking the
flag in the JSX where networkSpend and targetProgressRows are being rendered, to
ensure they follow the same conditional rendering pattern as the existing
warning, scope, and shard progress sections.
In `@apps/web/src/lib/server/auth.ts`:
- Around line 530-540: The requestAdminHost() function prioritizes the
x-forwarded-host header which can be client-controlled and should not be trusted
for admin authorization decisions. Remove or deprioritize the x-forwarded-host
header check from the requestAdminHost() function, relying instead on more
trustworthy sources like request.nextUrl.hostname. Since this function is used
for host allowlist checks and internal-admin token verification, ensure the
hardened version properly validates the host without trusting external headers
that could bypass authentication. Review all locations where requestAdminHost()
is called to verify the fix is consistent across the codebase.
In `@apps/web/src/lib/server/postgres.ts`:
- Around line 573-580: The current guard using Math.max(1,
Math.trunc(timeoutMs)) on line 573 does not prevent non-finite values like NaN
or Infinity from passing through, which then creates invalid timeout strings
(NaNms, Infinityms) when formatting on line 580. Add a check using
Number.isFinite() to validate that timeoutMs is a finite number before applying
Math.max and Math.trunc, and either reject or default to a reasonable timeout
value if the input is non-finite.
In `@apps/web/src/lib/server/trr-api/local-api-document-response.ts`:
- Around line 39-43: The HTML response in local-api-document-response.ts spreads
options?.headers into a plain object, which loses data when callers pass Headers
instances or tuple-array forms of HeadersInit. The JSON branch correctly
preserves all HeadersInit forms via NextResponse.json(). Fix the HTML branch by
using a NextResponse constructor method that properly handles all HeadersInit
forms (Headers instances, plain objects, and tuple arrays) instead of spreading
headers into a plain object, ensuring consistency with how the JSON branch
handles headers and maintaining compatibility with call sites like
admin-backend-proxy-route.ts that pass resolveHeaders() results typed as
HeadersInit.
In `@apps/web/src/proxy.ts`:
- Around line 188-190: The getForwardedRequestHost function currently returns
the full x-forwarded-host header value without parsing. When multiple hosts are
present in this header (comma-separated), only the first host should be used for
proper normalization and routing. Modify the function to split the
x-forwarded-host header value by commas and use only the first value after
trimming whitespace, then fall back to the host header or hostname as currently
implemented.
In `@apps/web/tests/admin-backend-proxy-route.test.ts`:
- Around line 172-212: The test doesn't properly validate the TRR_LOCAL_DEV gate
because NODE_ENV is already non-production in the test environment, allowing
document rendering regardless of the flag. Save the previous NODE_ENV value
before setting TRR_LOCAL_DEV to "1", set NODE_ENV to "production" to ensure the
test validates that TRR_LOCAL_DEV actually enables document rendering, and
restore NODE_ENV in the finally block alongside the TRR_LOCAL_DEV restoration to
maintain test isolation.
In `@apps/web/tests/social-account-profile-read-routes.test.ts`:
- Around line 189-220: The test "renders local dev browser document responses
for direct API navigation" does not actually validate the TRR_LOCAL_DEV override
behavior because it runs in a non-production test environment where document
mode is already enabled. To properly test the override functionality, save the
current NODE_ENV value at the start (similar to how previousLocalDev is saved),
set NODE_ENV to "production" before calling getPosts, and then restore the
original NODE_ENV value in the finally block alongside the restoration of
TRR_LOCAL_DEV. This ensures the test validates that setting TRR_LOCAL_DEV="1"
correctly enables document mode specifically in production environments.
In `@scripts/node-baseline-smoke.sh`:
- Around line 5-8: The WORKSPACE_ROOT variable is being set relative to
APP_ROOT, which is not defined in the script and causes the path to resolve
outside the repository. Replace the definition of WORKSPACE_ROOT on line 5 to
use the script's own location as a reference point instead. Use the BASH_SOURCE
variable with dirname to determine the script's directory, then navigate up one
level to reach the repository root. This ensures the path resolution works
correctly regardless of how the script is invoked, allowing the subsequent
source command on line 8 to correctly locate and load the node-baseline.sh file
from scripts/lib/.
---
Outside diff comments:
In `@apps/web/src/app/api/admin/trr-api/social/shared/sources/route.ts`:
- Around line 31-74: The requireAdmin call on line 32 is inside the same
try-catch block as fetchSocialBackendJson, which means if authentication fails,
the error is caught and execution falls through to the fallback block that
returns local shared-source data. This creates a security vulnerability where
unauthorized users can access restricted sources. Move the requireAdmin call
outside and above the try-catch block so that authentication failures are
handled independently and fail immediately, before any fallback mechanism can
execute.
In `@apps/web/src/components/admin/SocialAccountProfilePage.tsx`:
- Around line 6279-6318: The commentsShardHealthSummary useMemo function sets
total to 0 when the backend omits worker_counters.total, which prevents
commentsShardStatusLabel from rendering and loses row-level issue reasons. Fix
this by calculating total as the sum of the individual counter values (failed +
retrying + running + queued + complete) instead of using counters.total
directly, and ensure that issueReasons are still collected from
activeCommentsProgressRankedRows by iterating through those rows even when
worker_counters data is available, so that shard activity information is
preserved in both code paths.
🪄 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 Plus
Run ID: 35398fc3-4a1d-44ba-869d-61597dd0e503
⛔ Files ignored due to path filters (8)
apps/vue-wordle/package-lock.jsonis excluded by!**/package-lock.json,!apps/vue-wordle/**apps/vue-wordle/package.jsonis excluded by!apps/vue-wordle/**apps/web/src/lib/admin/api-references/generated/inventory.tsis excluded by!**/generated/**apps/web/src/lib/fonts/brand-fonts/generated/brand-font-match-results.jsonis excluded by!**/generated/**,!apps/web/src/lib/fonts/brand-fonts/generated/**apps/web/src/lib/fonts/brand-fonts/generated/discovered-brand-font-snapshot.jsonis excluded by!**/generated/**,!apps/web/src/lib/fonts/brand-fonts/generated/**apps/web/src/lib/fonts/brand-fonts/generated/normalized-brand-font-registry.jsonis excluded by!**/generated/**,!apps/web/src/lib/fonts/brand-fonts/generated/**apps/web/src/lib/fonts/brand-fonts/generated/normalized-r2-font-catalog.jsonis excluded by!**/generated/**,!apps/web/src/lib/fonts/brand-fonts/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (68)
apps/web/.env.exampleapps/web/README.mdapps/web/package.jsonapps/web/scripts/browser-smoke-admin-detail-routes.mjsapps/web/scripts/cast-smoke-preflight.mjsapps/web/src/app/admin/page.tsxapps/web/src/app/admin/social/page.tsxapps/web/src/app/admin/trr-shows/[showId]/page.tsxapps/web/src/app/admin/trr-shows/page.tsxapps/web/src/app/api/admin/health/instagram-comment-rollups/route.tsapps/web/src/app/api/admin/portless/status/route.tsapps/web/src/app/api/admin/reddit/communities/[communityId]/backfill/snapshot/route.tsapps/web/src/app/api/admin/social/landing/route.tsapps/web/src/app/api/admin/trr-api/shows/[showId]/seasons/[seasonNumber]/social/analytics/snapshot/route.tsapps/web/src/app/api/admin/trr-api/shows/[showId]/seasons/[seasonNumber]/social/analytics/week/[weekIndex]/snapshot/route.tsapps/web/src/app/api/admin/trr-api/shows/settings/show-core-auto-refresh/route.tsapps/web/src/app/api/admin/trr-api/social-growth/cast-comparison/snapshot/route.tsapps/web/src/app/api/admin/trr-api/social/ingest/live-status/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/catalog/review-queue/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/collaborators-tags/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/comments/audit-cursor-retries/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/comments/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/hashtags/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/posts/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/snapshot/route.tsapps/web/src/app/api/admin/trr-api/social/profiles/[platform]/[handle]/summary/route.tsapps/web/src/app/api/admin/trr-api/social/shared/sources/route.tsapps/web/src/app/api/cron/episode-progression/route.tsapps/web/src/components/admin/AdminGlobalHeader.tsxapps/web/src/components/admin/RedditAdminShell.tsxapps/web/src/components/admin/SocialAccountProfilePage.tsxapps/web/src/components/admin/instagram/InstagramCommentsPanel.tsxapps/web/src/lib/admin/admin-url-defaults.tsapps/web/src/lib/admin/client-access.tsapps/web/src/lib/admin/social-account-profile.tsapps/web/src/lib/admin/social-landing.tsapps/web/src/lib/server/admin/admin-snapshot-route.tsapps/web/src/lib/server/admin/portless-status.tsapps/web/src/lib/server/admin/shared-account-sources.tsapps/web/src/lib/server/admin/shell-exec.tsapps/web/src/lib/server/admin/social-landing-repository.tsapps/web/src/lib/server/auth.tsapps/web/src/lib/server/postgres.tsapps/web/src/lib/server/trr-api/admin-backend-proxy-route.tsapps/web/src/lib/server/trr-api/internal-admin-auth.tsapps/web/src/lib/server/trr-api/local-api-document-response.tsapps/web/src/lib/server/trr-api/social-admin-proxy.tsapps/web/src/lib/server/trr-api/social-profile-route-factory.tsapps/web/src/proxy.tsapps/web/tests/admin-backend-proxy-route.test.tsapps/web/tests/admin-host-middleware.test.tsapps/web/tests/admin-portless-status.test.tsapps/web/tests/admin-social-page-auth-bypass.test.tsxapps/web/tests/client-admin-access.test.tsapps/web/tests/cron-routes-auth.test.tsapps/web/tests/internal-admin-auth.test.tsapps/web/tests/server-auth-adapter.test.tsapps/web/tests/social-account-comments-audit-cursor-retries-route.test.tsapps/web/tests/social-account-profile-page.runtime.test.tsxapps/web/tests/social-account-profile-read-routes.test.tsapps/web/tests/social-admin-proxy.test.tsapps/web/tests/social-landing-repository.test.tsapps/web/tests/social-shared-sources-route.test.tsdocs/cross-collab/rhoslc-media-refresh-e2e-checklist.mdpackage.jsonrequirements.lock.txtscripts/node-baseline-smoke.shscripts/portless-banner.mjs
Comment on lines
+7
to
+10
| const DEFAULT_BASE_URL = | ||
| process.env.ADMIN_APP_ORIGIN || | ||
| process.env.PORTLESS_ADMIN_URL || | ||
| "https://admin.trr.localhost"; |
There was a problem hiding this comment.
Include TRR_APP_ORIGIN in smoke default URL precedence.
This script now diverges from cast-smoke-preflight precedence and can resolve a different app origin when only TRR_APP_ORIGIN is set.
Suggested patch
const DEFAULT_BASE_URL =
+ process.env.TRR_APP_ORIGIN ||
process.env.ADMIN_APP_ORIGIN ||
process.env.PORTLESS_ADMIN_URL ||
"https://admin.trr.localhost";📝 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.
Suggested change
| const DEFAULT_BASE_URL = | |
| process.env.ADMIN_APP_ORIGIN || | |
| process.env.PORTLESS_ADMIN_URL || | |
| "https://admin.trr.localhost"; | |
| const DEFAULT_BASE_URL = | |
| process.env.TRR_APP_ORIGIN || | |
| process.env.ADMIN_APP_ORIGIN || | |
| process.env.PORTLESS_ADMIN_URL || | |
| "https://admin.trr.localhost"; |
🤖 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 `@apps/web/scripts/browser-smoke-admin-detail-routes.mjs` around lines 7 - 10,
The DEFAULT_BASE_URL initialization is missing process.env.TRR_APP_ORIGIN in its
environment variable precedence chain, causing it to diverge from
cast-smoke-preflight. Add TRR_APP_ORIGIN to the conditional chain of environment
variables checked before falling back to the hardcoded default URL, ensuring the
precedence order matches the cast-smoke-preflight script so both resolve the app
origin consistently when TRR_APP_ORIGIN is the only environment variable set.
Comment on lines
+97
to
+100
| function isWrongAdminHost() { | ||
| if (typeof window === "undefined") return false; | ||
| return window.location.hostname !== "admin.trr.localhost"; | ||
| } |
There was a problem hiding this comment.
Hard-coded wrong-host detection breaks non-local admin environments.
Line 99 marks every host except admin.trr.localhost as wrong, and Line 401 always navigates to the local portless URL. That will mislabel valid non-local admin hosts and can redirect users to an unreachable local domain.
Proposed fix
function isWrongAdminHost() {
if (typeof window === "undefined") return false;
- return window.location.hostname !== "admin.trr.localhost";
+ const hostname = window.location.hostname.toLowerCase();
+ const isLocalhostFamily = hostname === "localhost" || hostname.endsWith(".localhost");
+ if (!isLocalhostFamily) return false;
+ return hostname !== "admin.trr.localhost";
}
@@
const openCleanAdminWithDiagnostics = async () => {
if (typeof window === "undefined") return;
+ const hostname = window.location.hostname.toLowerCase();
+ const isLocalhostFamily = hostname === "localhost" || hostname.endsWith(".localhost");
+ const targetAdminUrl = isLocalhostFamily
+ ? PORTLESS_ADMIN_DASHBOARD_URL
+ : `${window.location.origin}/admin`;
setDiagnosticsState("copying");
@@
- expected_admin_url: PORTLESS_ADMIN_DASHBOARD_URL,
+ expected_admin_url: targetAdminUrl,
@@
- window.location.assign(PORTLESS_ADMIN_DASHBOARD_URL);
+ window.location.assign(targetAdminUrl);Also applies to: 383-401
🤖 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 `@apps/web/src/app/admin/page.tsx` around lines 97 - 100, The
`isWrongAdminHost()` function hard-codes the admin hostname to
`admin.trr.localhost`, which incorrectly marks all other hosts (including valid
non-local admin environments) as wrong. Additionally, the navigation logic
around lines 383-401 hard-codes redirects to a local portless URL. Replace the
hard-coded `admin.trr.localhost` string with a configurable admin host value
(either from environment variables, configuration, or derived from the current
URL's domain structure) so that the host validation and redirect logic work
correctly across both local and non-local admin environments.
Comment on lines
+2488
to
+2555
| {networkSets.map((network) => { | ||
| const isBravoTvSet = network.key === "bravo-tv"; | ||
| const linkedProfileCount = | ||
| networkSourceSet.key === network.key | ||
| ? networkSourceSet.sources.length | ||
| : network.handles.length; | ||
| const freshnessLabel = isBravoTvSet | ||
| ? formatCompactTimestamp(landingFreshnessAt) | ||
| : null; | ||
| return ( | ||
| <div | ||
| key={network.key} | ||
| className="rounded-2xl border border-zinc-200 bg-zinc-50 p-5" | ||
| > | ||
| <div className="flex flex-col gap-4 lg:flex-row lg:items-start lg:justify-between"> | ||
| <div> | ||
| <p className="text-xs font-semibold uppercase tracking-[0.18em] text-zinc-500"> | ||
| Profile Set | ||
| </p> | ||
| <h3 className="text-lg font-semibold text-zinc-900"> | ||
| {network.title} | ||
| </h3> | ||
| </div> | ||
| <div className="flex flex-wrap items-center gap-2 lg:justify-end"> | ||
| <span className="w-fit rounded-full border border-zinc-200 bg-white px-2.5 py-1 text-xs font-semibold text-zinc-600"> | ||
| {linkedProfileCount.toLocaleString()} linked profile | ||
| {linkedProfileCount === 1 ? "" : "s"} | ||
| </span> | ||
| {freshnessLabel ? ( | ||
| <span className="w-fit rounded-full border border-zinc-200 bg-white px-2.5 py-1 text-xs font-semibold text-zinc-500"> | ||
| Loaded {freshnessLabel} | ||
| </span> | ||
| ) : null} | ||
| {isBravoTvSet ? ( | ||
| <IconActionAnchor | ||
| href={BRAVO_SHARED_SOURCES_RAW_URL} | ||
| label="Open raw Bravo TV shared-source rows" | ||
| > | ||
| <ExternalLink aria-hidden="true" /> | ||
| </IconActionAnchor> | ||
| ) : null} | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| {networkSourceGroups.length > 0 ? ( | ||
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | ||
| {networkSourceGroups.map((group) => ( | ||
| <NetworkSourceGroupCard | ||
| key={`${network.key}:${group.key}`} | ||
| group={group} | ||
| networkKey={network.key} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ) : network.handles.length > 0 ? ( | ||
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | ||
| {network.handles.map((handle) => ( | ||
| <HandleInventoryCard | ||
| key={`${network.key}:${handle.platform}:${handle.handle}`} | ||
| handle={handle} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ) : ( | ||
| <p className="mt-4 text-sm text-zinc-500"> | ||
| No linked profiles are configured for this set. | ||
| </p> | ||
| )} | ||
| </div> | ||
| ))} | ||
| {networkSourceGroups.length > 0 ? ( | ||
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | ||
| {networkSourceGroups.map((group) => ( | ||
| <NetworkSourceGroupCard | ||
| key={`${network.key}:${group.key}`} | ||
| group={group} | ||
| networkKey={network.key} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ) : network.handles.length > 0 ? ( | ||
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | ||
| {network.handles.map((handle) => ( | ||
| <HandleInventoryCard | ||
| key={`${network.key}:${handle.platform}:${handle.handle}`} | ||
| handle={handle} | ||
| /> | ||
| ))} | ||
| </div> | ||
| ) : ( | ||
| <p className="mt-4 text-sm text-zinc-500"> | ||
| No linked profiles are configured for this set. | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Scope shared-source group rendering to the matching network set
Line 2532 currently uses networkSourceGroups for every network card. Because those groups come from a single networkSourceSet, non-matching network cards can render the wrong grouped sources and suppress their own network.handles.
Suggested fix
{networkSets.map((network) => {
const isBravoTvSet = network.key === "bravo-tv";
+ const showSharedGroups =
+ networkSourceSet.key === network.key && networkSourceGroups.length > 0;
const linkedProfileCount =
networkSourceSet.key === network.key
? networkSourceSet.sources.length
: network.handles.length;
@@
- {networkSourceGroups.length > 0 ? (
+ {showSharedGroups ? (
<div className="mt-4 grid gap-3 xl:grid-cols-2">
{networkSourceGroups.map((group) => (
<NetworkSourceGroupCard
key={`${network.key}:${group.key}`}
group={group}
networkKey={network.key}
/>
))}
</div>
) : network.handles.length > 0 ? (📝 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.
Suggested change
| {networkSets.map((network) => { | |
| const isBravoTvSet = network.key === "bravo-tv"; | |
| const linkedProfileCount = | |
| networkSourceSet.key === network.key | |
| ? networkSourceSet.sources.length | |
| : network.handles.length; | |
| const freshnessLabel = isBravoTvSet | |
| ? formatCompactTimestamp(landingFreshnessAt) | |
| : null; | |
| return ( | |
| <div | |
| key={network.key} | |
| className="rounded-2xl border border-zinc-200 bg-zinc-50 p-5" | |
| > | |
| <div className="flex flex-col gap-4 lg:flex-row lg:items-start lg:justify-between"> | |
| <div> | |
| <p className="text-xs font-semibold uppercase tracking-[0.18em] text-zinc-500"> | |
| Profile Set | |
| </p> | |
| <h3 className="text-lg font-semibold text-zinc-900"> | |
| {network.title} | |
| </h3> | |
| </div> | |
| <div className="flex flex-wrap items-center gap-2 lg:justify-end"> | |
| <span className="w-fit rounded-full border border-zinc-200 bg-white px-2.5 py-1 text-xs font-semibold text-zinc-600"> | |
| {linkedProfileCount.toLocaleString()} linked profile | |
| {linkedProfileCount === 1 ? "" : "s"} | |
| </span> | |
| {freshnessLabel ? ( | |
| <span className="w-fit rounded-full border border-zinc-200 bg-white px-2.5 py-1 text-xs font-semibold text-zinc-500"> | |
| Loaded {freshnessLabel} | |
| </span> | |
| ) : null} | |
| {isBravoTvSet ? ( | |
| <IconActionAnchor | |
| href={BRAVO_SHARED_SOURCES_RAW_URL} | |
| label="Open raw Bravo TV shared-source rows" | |
| > | |
| <ExternalLink aria-hidden="true" /> | |
| </IconActionAnchor> | |
| ) : null} | |
| </div> | |
| </div> | |
| </div> | |
| {networkSourceGroups.length > 0 ? ( | |
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | |
| {networkSourceGroups.map((group) => ( | |
| <NetworkSourceGroupCard | |
| key={`${network.key}:${group.key}`} | |
| group={group} | |
| networkKey={network.key} | |
| /> | |
| ))} | |
| </div> | |
| ) : network.handles.length > 0 ? ( | |
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | |
| {network.handles.map((handle) => ( | |
| <HandleInventoryCard | |
| key={`${network.key}:${handle.platform}:${handle.handle}`} | |
| handle={handle} | |
| /> | |
| ))} | |
| </div> | |
| ) : ( | |
| <p className="mt-4 text-sm text-zinc-500"> | |
| No linked profiles are configured for this set. | |
| </p> | |
| )} | |
| </div> | |
| ))} | |
| {networkSourceGroups.length > 0 ? ( | |
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | |
| {networkSourceGroups.map((group) => ( | |
| <NetworkSourceGroupCard | |
| key={`${network.key}:${group.key}`} | |
| group={group} | |
| networkKey={network.key} | |
| /> | |
| ))} | |
| </div> | |
| ) : network.handles.length > 0 ? ( | |
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | |
| {network.handles.map((handle) => ( | |
| <HandleInventoryCard | |
| key={`${network.key}:${handle.platform}:${handle.handle}`} | |
| handle={handle} | |
| /> | |
| ))} | |
| </div> | |
| ) : ( | |
| <p className="mt-4 text-sm text-zinc-500"> | |
| No linked profiles are configured for this set. | |
| </p> | |
| )} | |
| {networkSets.map((network) => { | |
| const isBravoTvSet = network.key === "bravo-tv"; | |
| const showSharedGroups = | |
| networkSourceSet.key === network.key && networkSourceGroups.length > 0; | |
| const linkedProfileCount = | |
| networkSourceSet.key === network.key | |
| ? networkSourceSet.sources.length | |
| : network.handles.length; | |
| const freshnessLabel = isBravoTvSet | |
| ? formatCompactTimestamp(landingFreshnessAt) | |
| : null; | |
| return ( | |
| <div | |
| key={network.key} | |
| className="rounded-2xl border border-zinc-200 bg-zinc-50 p-5" | |
| > | |
| <div className="flex flex-col gap-4 lg:flex-row lg:items-start lg:justify-between"> | |
| <div> | |
| <p className="text-xs font-semibold uppercase tracking-[0.18em] text-zinc-500"> | |
| Profile Set | |
| </p> | |
| <h3 className="text-lg font-semibold text-zinc-900"> | |
| {network.title} | |
| </h3> | |
| </div> | |
| <div className="flex flex-wrap items-center gap-2 lg:justify-end"> | |
| <span className="w-fit rounded-full border border-zinc-200 bg-white px-2.5 py-1 text-xs font-semibold text-zinc-600"> | |
| {linkedProfileCount.toLocaleString()} linked profile | |
| {linkedProfileCount === 1 ? "" : "s"} | |
| </span> | |
| {freshnessLabel ? ( | |
| <span className="w-fit rounded-full border border-zinc-200 bg-white px-2.5 py-1 text-xs font-semibold text-zinc-500"> | |
| Loaded {freshnessLabel} | |
| </span> | |
| ) : null} | |
| {isBravoTvSet ? ( | |
| <IconActionAnchor | |
| href={BRAVO_SHARED_SOURCES_RAW_URL} | |
| label="Open raw Bravo TV shared-source rows" | |
| > | |
| <ExternalLink aria-hidden="true" /> | |
| </IconActionAnchor> | |
| ) : null} | |
| </div> | |
| </div> | |
| {showSharedGroups ? ( | |
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | |
| {networkSourceGroups.map((group) => ( | |
| <NetworkSourceGroupCard | |
| key={`${network.key}:${group.key}`} | |
| group={group} | |
| networkKey={network.key} | |
| /> | |
| ))} | |
| </div> | |
| ) : network.handles.length > 0 ? ( | |
| <div className="mt-4 grid gap-3 xl:grid-cols-2"> | |
| {network.handles.map((handle) => ( | |
| <HandleInventoryCard | |
| key={`${network.key}:${handle.platform}:${handle.handle}`} | |
| handle={handle} | |
| /> | |
| ))} | |
| </div> | |
| ) : ( | |
| <p className="mt-4 text-sm text-zinc-500"> | |
| No linked profiles are configured for this set. | |
| </p> | |
| )} |
🤖 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 `@apps/web/src/app/admin/social/page.tsx` around lines 2488 - 2555, The
`networkSourceGroups` rendering block is currently applied to every `network` in
the map regardless of whether it matches the `networkSourceSet`. Add a condition
to check if `networkSourceSet.key === network.key` before rendering the
`NetworkSourceGroupCard` components (around line 2532 where
`networkSourceGroups.length > 0` is checked). Only display the grouped sources
when the current network matches the networkSourceSet; otherwise, fall through
to render the `network.handles` as normal. This prevents non-matching networks
from displaying incorrect grouped sources and suppressing their own handles.
Comment on lines
+2820
to
+2822
| const [autoShowCorePaused, setAutoShowCorePaused] = useState(false); | ||
| const [autoShowCorePauseSaving, setAutoShowCorePauseSaving] = useState(false); | ||
| const [autoShowCorePauseError, setAutoShowCorePauseError] = useState<string | null>(null); |
There was a problem hiding this comment.
Gate auto-refresh until the pause setting is known.
Line 2820 defaults to “unpaused”, while the real setting loads asynchronously on Lines 10524-10553. The effect on Lines 10582-10585 can therefore enqueue show_core before a persisted paused setting is honored.
🐛 Proposed fix
- const [autoShowCorePaused, setAutoShowCorePaused] = useState(false);
+ const [autoShowCorePaused, setAutoShowCorePaused] = useState<boolean | null>(null);- const nextPaused = !autoShowCorePaused;
+ const nextPaused = autoShowCorePaused !== true;- if (autoShowCorePaused) return;
+ if (autoShowCorePaused !== false) return;- {autoShowCorePaused && episodeIdGapWarning
+ {autoShowCorePaused === true && episodeIdGapWarning
? " Automatic show core refreshes are paused."
: ""}- : autoShowCorePaused
+ : autoShowCorePaused === true
? "Resume Auto"
: "Pause Auto"}Also applies to: 10524-10553, 10582-10585
🤖 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 `@apps/web/src/app/admin/trr-shows/`[showId]/page.tsx around lines 2820 - 2822,
The autoShowCorePaused state defaults to false before the actual persisted pause
setting loads asynchronously, allowing the auto-refresh effect to potentially
enqueue show_core before the true pause state is known. Introduce a new loading
state (such as autoShowCorePauseLoaded) initialized to false to track whether
the pause setting has been fetched. In the async loading effect that populates
the pause state (around lines 10524-10553), set both the actual pause value and
mark the loading state as complete. In the auto-refresh effect (lines
10582-10585), add a guard condition to only enqueue show_core if
autoShowCorePauseLoaded is true, ensuring the persisted pause setting is honored
before auto-refresh occurs.
Comment on lines
+216
to
+222
| const settingsResponse = await fetchWithAuth( | ||
| "/api/admin/trr-api/shows/settings/show-core-auto-refresh", | ||
| ); | ||
| const settings = (await settingsResponse.json().catch(() => ({}))) as { paused?: unknown }; | ||
| if (settingsResponse.ok && settings.paused === true) { | ||
| return { operationId: null, paused: true }; | ||
| } |
There was a problem hiding this comment.
Fail closed when the auto-refresh settings read fails.
A non-OK settings response is currently treated the same as “not paused,” so the client can still POST an auto-refresh operation while the pause state is unknown. Throw or return a skipped result when settings cannot be loaded, and only enqueue after a successful settings read confirms paused !== true.
🐛 Proposed fix
const settingsResponse = await fetchWithAuth(
"/api/admin/trr-api/shows/settings/show-core-auto-refresh",
);
- const settings = (await settingsResponse.json().catch(() => ({}))) as { paused?: unknown };
- if (settingsResponse.ok && settings.paused === true) {
+ const settings = (await settingsResponse.json().catch(() => ({}))) as {
+ error?: unknown;
+ paused?: unknown;
+ };
+ if (!settingsResponse.ok) {
+ const message =
+ typeof settings.error === "string"
+ ? settings.error
+ : "Failed to load show core auto-refresh settings";
+ throw new Error(message);
+ }
+ if (settings.paused === true) {
return { operationId: null, paused: true };
}🤖 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 `@apps/web/src/app/admin/trr-shows/page.tsx` around lines 216 - 222, The
current logic treats a non-OK settingsResponse the same as a successful read
with paused unset, allowing the operation to proceed despite unknown pause
state. Refactor the conditional to fail closed: first check if
settingsResponse.ok is false and return early with a skipped result or throw an
error to prevent further processing when the settings read fails, then only
proceed with the auto-refresh operation after confirming through a successful
response that paused is not true. This ensures the client does not enqueue an
operation while the pause state is unknown.
Comment on lines
+39
to
+43
| headers: { | ||
| ...options?.headers, | ||
| "content-type": "text/html; charset=utf-8", | ||
| "x-trr-local-api-document": "1", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify JS semantics: object spread drops header entries for Headers / tuple forms.
node - <<'NODE'
const cases = [
["plain-object", { "x-test": "1" }],
["headers-instance", new Headers([["x-test", "1"]])],
["tuple-array", [["x-test", "1"]]],
];
for (const [name, v] of cases) {
const spread = { ...v };
console.log(name, "keys=", Object.keys(spread), "x-test=", spread["x-test"]);
}
NODE
# Inspect call sites to confirm real usage of adminJsonResponse headers inputs.
rg -n --type=ts -C3 '\badminJsonResponse\s*\('Repository: therealityreport/trr-app
Length of output: 6455
🏁 Script executed:
#!/bin/bash
# Find the declaration and type of responseHeaders in admin-backend-proxy-route.ts
rg -n 'responseHeaders' apps/web/src/lib/server/trr-api/admin-backend-proxy-route.ts -B2 -A2
# Also check the function signature and return type of adminJsonResponse
rg -n -A15 'export.*adminJsonResponse' apps/web/src/lib/server/trr-api/local-api-document-response.ts | head -40Repository: therealityreport/trr-app
Length of output: 1911
🏁 Script executed:
#!/bin/bash
# Find the resolveHeaders function to understand what type it returns
rg -n 'resolveHeaders' apps/web/src/lib/server/trr-api/admin-backend-proxy-route.ts -B5 -A15 | head -60
# Also search for the resolveHeaders function definition
rg -n -A10 'const resolveHeaders|function resolveHeaders'Repository: therealityreport/trr-app
Length of output: 4870
🏁 Script executed:
#!/bin/bash
# Verify the complete implementation of the HTML branch in local-api-document-response.ts
cat -n apps/web/src/lib/server/trr-api/local-api-document-response.tsRepository: therealityreport/trr-app
Length of output: 2052
Object spread silently drops headers in HTML-document mode.
Lines 39-43 spread options?.headers into a plain object, which loses headers when callers pass Headers instances or tuple-array forms. The JSON branch (line 50) correctly preserves all HeadersInit forms via NextResponse.json(), but the HTML branch creates an inconsistency.
Real call sites in admin-backend-proxy-route.ts (lines 309, 316) pass responseHeaders from resolveHeaders(), which is typed HeadersInit and can return any of its three forms. When developers use the library through the normal API, they can inadvertently lose headers in HTML mode while keeping them in JSON mode.
💡 Proposed fix
if (localDevApiDocumentEnabled() && wantsBrowserDocument(request)) {
const body = JSON.stringify(data, null, 2);
+ const responseHeaders = new Headers(options?.headers);
+ responseHeaders.set("content-type", "text/html; charset=utf-8");
+ responseHeaders.set("x-trr-local-api-document", "1");
return new NextResponse(
`<!doctype html><html><head><meta charset="utf-8"><title>${escapeHtml(options?.title ?? "TRR API")}</title></head><body><pre>${escapeHtml(body)}</pre></body></html>`,
{
status: options?.status,
- headers: {
- ...options?.headers,
- "content-type": "text/html; charset=utf-8",
- "x-trr-local-api-document": "1",
- },
+ headers: responseHeaders,
},
);
}🤖 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 `@apps/web/src/lib/server/trr-api/local-api-document-response.ts` around lines
39 - 43, The HTML response in local-api-document-response.ts spreads
options?.headers into a plain object, which loses data when callers pass Headers
instances or tuple-array forms of HeadersInit. The JSON branch correctly
preserves all HeadersInit forms via NextResponse.json(). Fix the HTML branch by
using a NextResponse constructor method that properly handles all HeadersInit
forms (Headers instances, plain objects, and tuple arrays) instead of spreading
headers into a plain object, ensuring consistency with how the JSON branch
handles headers and maintaining compatibility with call sites like
admin-backend-proxy-route.ts that pass resolveHeaders() results typed as
HeadersInit.
Comment on lines
+188
to
190
| function getForwardedRequestHost(request: NextRequest): string | null { | ||
| return request.headers.get("x-forwarded-host") ?? request.headers.get("host") ?? request.nextUrl.hostname; | ||
| } |
There was a problem hiding this comment.
Parse only the first x-forwarded-host value.
Line 189 currently consumes the full header value. If multiple hosts are present, host normalization/matching can fail and cause incorrect routing decisions.
Proposed fix
function getForwardedRequestHost(request: NextRequest): string | null {
- return request.headers.get("x-forwarded-host") ?? request.headers.get("host") ?? request.nextUrl.hostname;
+ const forwarded = request.headers.get("x-forwarded-host");
+ const forwardedHost = forwarded?.split(",")[0]?.trim();
+ return forwardedHost || request.headers.get("host") || request.nextUrl.hostname;
}📝 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.
Suggested change
| function getForwardedRequestHost(request: NextRequest): string | null { | |
| return request.headers.get("x-forwarded-host") ?? request.headers.get("host") ?? request.nextUrl.hostname; | |
| } | |
| function getForwardedRequestHost(request: NextRequest): string | null { | |
| const forwarded = request.headers.get("x-forwarded-host"); | |
| const forwardedHost = forwarded?.split(",")[0]?.trim(); | |
| return forwardedHost || request.headers.get("host") || request.nextUrl.hostname; | |
| } |
🤖 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 `@apps/web/src/proxy.ts` around lines 188 - 190, The getForwardedRequestHost
function currently returns the full x-forwarded-host header value without
parsing. When multiple hosts are present in this header (comma-separated), only
the first host should be used for proper normalization and routing. Modify the
function to split the x-forwarded-host header value by commas and use only the
first value after trimming whitespace, then fall back to the host header or
hostname as currently implemented.
Comment on lines
+172
to
+212
| it("renders local dev browser document responses for JSON proxy routes", async () => { | ||
| const previousLocalDev = process.env.TRR_LOCAL_DEV; | ||
| process.env.TRR_LOCAL_DEV = "1"; | ||
| vi.stubGlobal( | ||
| "fetch", | ||
| vi.fn().mockResolvedValue( | ||
| new Response(JSON.stringify({ ok: true, source: "backend" }), { | ||
| status: 200, | ||
| headers: { "content-type": "application/json" }, | ||
| }), | ||
| ), | ||
| ); | ||
|
|
||
| try { | ||
| const response = await makeTestRoute()( | ||
| new NextRequest("http://localhost/api/test?page=2", { | ||
| method: "PATCH", | ||
| headers: { | ||
| accept: "text/html,application/xhtml+xml", | ||
| "content-type": "application/json", | ||
| "sec-fetch-dest": "document", | ||
| }, | ||
| body: JSON.stringify({ force: true }), | ||
| }), | ||
| { params: Promise.resolve({ itemId: "item-1" }) }, | ||
| ); | ||
| const body = await response.text(); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.headers.get("content-type")).toContain("text/html"); | ||
| expect(response.headers.get("x-trr-local-api-document")).toBe("1"); | ||
| expect(body).toContain("<pre>"); | ||
| expect(body).toContain(""source": "backend""); | ||
| } finally { | ||
| if (typeof previousLocalDev === "undefined") { | ||
| delete process.env.TRR_LOCAL_DEV; | ||
| } else { | ||
| process.env.TRR_LOCAL_DEV = previousLocalDev; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Set NODE_ENV="production" in this test to actually validate the TRR_LOCAL_DEV gate.
Line 174 sets TRR_LOCAL_DEV, but under test NODE_ENV is already non-production, so document rendering is enabled regardless. This can pass even if the flag handling regresses.
Proposed tightening
it("renders local dev browser document responses for JSON proxy routes", async () => {
+ const previousNodeEnv = process.env.NODE_ENV;
const previousLocalDev = process.env.TRR_LOCAL_DEV;
+ process.env.NODE_ENV = "production";
process.env.TRR_LOCAL_DEV = "1";
vi.stubGlobal(
"fetch",
@@
} finally {
+ if (typeof previousNodeEnv === "undefined") {
+ delete process.env.NODE_ENV;
+ } else {
+ process.env.NODE_ENV = previousNodeEnv;
+ }
if (typeof previousLocalDev === "undefined") {
delete process.env.TRR_LOCAL_DEV;
} else {🤖 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 `@apps/web/tests/admin-backend-proxy-route.test.ts` around lines 172 - 212, The
test doesn't properly validate the TRR_LOCAL_DEV gate because NODE_ENV is
already non-production in the test environment, allowing document rendering
regardless of the flag. Save the previous NODE_ENV value before setting
TRR_LOCAL_DEV to "1", set NODE_ENV to "production" to ensure the test validates
that TRR_LOCAL_DEV actually enables document rendering, and restore NODE_ENV in
the finally block alongside the TRR_LOCAL_DEV restoration to maintain test
isolation.
Comment on lines
+189
to
+220
| it("renders local dev browser document responses for direct API navigation", async () => { | ||
| const previousLocalDev = process.env.TRR_LOCAL_DEV; | ||
| process.env.TRR_LOCAL_DEV = "1"; | ||
| fetchSocialBackendJsonMock.mockResolvedValueOnce({ | ||
| items: [{ source_id: "DZaAVVLiAog" }], | ||
| pagination: { page: 1, page_size: 20, total: 1, total_pages: 1 }, | ||
| }); | ||
|
|
||
| try { | ||
| const response = await getPosts( | ||
| new NextRequest("http://localhost/api/admin/trr-api/social/profiles/instagram/bravotv/posts?limit=20", { | ||
| headers: { | ||
| accept: "text/html,application/xhtml+xml", | ||
| "sec-fetch-dest": "document", | ||
| }, | ||
| }), | ||
| { params: Promise.resolve({ platform: "instagram", handle: "bravotv" }) }, | ||
| ); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(response.headers.get("content-type")).toContain("text/html"); | ||
| expect(response.headers.get("x-trr-local-api-document")).toBe("1"); | ||
| expect(response.headers.get("x-trr-admin-proxy-route-ms")).toMatch(/^\d+$/); | ||
| await expect(response.text()).resolves.toContain(""source_id": "DZaAVVLiAog""); | ||
| } finally { | ||
| if (previousLocalDev === undefined) { | ||
| delete process.env.TRR_LOCAL_DEV; | ||
| } else { | ||
| process.env.TRR_LOCAL_DEV = previousLocalDev; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Test does not actually exercise the TRR_LOCAL_DEV override branch.
At Line 190-192, this case runs under non-production test env, where document mode is already enabled; the assertion can pass even if TRR_LOCAL_DEV handling regresses. Set NODE_ENV="production" inside this test (and restore it) so it validates the intended override path.
Suggested patch
it("renders local dev browser document responses for direct API navigation", async () => {
+ const previousNodeEnv = process.env.NODE_ENV;
const previousLocalDev = process.env.TRR_LOCAL_DEV;
+ process.env.NODE_ENV = "production";
process.env.TRR_LOCAL_DEV = "1";
fetchSocialBackendJsonMock.mockResolvedValueOnce({
items: [{ source_id: "DZaAVVLiAog" }],
pagination: { page: 1, page_size: 20, total: 1, total_pages: 1 },
});
try {
const response = await getPosts(
@@
expect(response.headers.get("x-trr-admin-proxy-route-ms")).toMatch(/^\d+$/);
await expect(response.text()).resolves.toContain(""source_id": "DZaAVVLiAog"");
} finally {
+ if (previousNodeEnv === undefined) {
+ delete process.env.NODE_ENV;
+ } else {
+ process.env.NODE_ENV = previousNodeEnv;
+ }
if (previousLocalDev === undefined) {
delete process.env.TRR_LOCAL_DEV;
} else {
process.env.TRR_LOCAL_DEV = previousLocalDev;
}
}
});🤖 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 `@apps/web/tests/social-account-profile-read-routes.test.ts` around lines 189 -
220, The test "renders local dev browser document responses for direct API
navigation" does not actually validate the TRR_LOCAL_DEV override behavior
because it runs in a non-production test environment where document mode is
already enabled. To properly test the override functionality, save the current
NODE_ENV value at the start (similar to how previousLocalDev is saved), set
NODE_ENV to "production" before calling getPosts, and then restore the original
NODE_ENV value in the finally block alongside the restoration of TRR_LOCAL_DEV.
This ensures the test validates that setting TRR_LOCAL_DEV="1" correctly enables
document mode specifically in production environments.
Comment on lines
+5
to
+8
| WORKSPACE_ROOT="$(cd "${APP_ROOT}/.." && pwd)" | ||
|
|
||
| # shellcheck disable=SC1091 | ||
| source "${WORKSPACE_ROOT}/scripts/lib/node-baseline.sh" |
There was a problem hiding this comment.
Source path resolves outside the repository and breaks script startup.
Line 8 sources scripts/lib/node-baseline.sh from WORKSPACE_ROOT, but Line 5 defines that as ${APP_ROOT}/... For normal bash scripts/node-baseline-smoke.sh execution, this points outside the repo and fails before baseline checks run.
💡 Proposed fix
APP_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
-WORKSPACE_ROOT="$(cd "${APP_ROOT}/.." && pwd)"
# shellcheck disable=SC1091
-source "${WORKSPACE_ROOT}/scripts/lib/node-baseline.sh"
+source "${APP_ROOT}/scripts/lib/node-baseline.sh"🤖 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 `@scripts/node-baseline-smoke.sh` around lines 5 - 8, The WORKSPACE_ROOT
variable is being set relative to APP_ROOT, which is not defined in the script
and causes the path to resolve outside the repository. Replace the definition of
WORKSPACE_ROOT on line 5 to use the script's own location as a reference point
instead. Use the BASH_SOURCE variable with dirname to determine the script's
directory, then navigate up one level to reach the repository root. This ensures
the path resolution works correctly regardless of how the script is invoked,
allowing the subsequent source command on line 8 to correctly locate and load
the node-baseline.sh file from scripts/lib/.
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 `@apps/web/tests/helpers/expected-console.ts`:
- Around line 78-81: The `expectCalled` method does not guarantee that
`release()` is called when the assertion fails. If the
`expect(entry.matchedCalls.length).toBeGreaterThan(0)` assertion throws an
error, the `release()` call will be skipped, causing capture state to leak into
later tests. Wrap the assertion in a try/finally block where the assertion is in
the try block and `release()` is in the finally block to ensure cleanup always
executes regardless of whether the assertion passes or fails.
🪄 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 Plus
Run ID: 72e48552-eb69-46e5-831e-7b64e4fc9eaa
📒 Files selected for processing (1)
apps/web/tests/helpers/expected-console.ts
Comment on lines
78
to
81
| expectCalled: () => { | ||
| expect(matchedCalls.length).toBeGreaterThan(0); | ||
| spy.mockRestore(); | ||
| expect(entry.matchedCalls.length).toBeGreaterThan(0); | ||
| release(); | ||
| }, |
There was a problem hiding this comment.
Ensure cleanup runs even when expectCalled fails.
At Line 78, a failed assertion throws before release() executes, leaking capture state into later tests. Wrap the assertion in try/finally so cleanup is guaranteed.
Suggested patch
return {
expectCalled: () => {
- expect(entry.matchedCalls.length).toBeGreaterThan(0);
- release();
+ try {
+ expect(entry.matchedCalls.length).toBeGreaterThan(0);
+ } finally {
+ release();
+ }
},📝 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.
Suggested change
| expectCalled: () => { | |
| expect(matchedCalls.length).toBeGreaterThan(0); | |
| spy.mockRestore(); | |
| expect(entry.matchedCalls.length).toBeGreaterThan(0); | |
| release(); | |
| }, | |
| expectCalled: () => { | |
| try { | |
| expect(entry.matchedCalls.length).toBeGreaterThan(0); | |
| } finally { | |
| release(); | |
| } | |
| }, |
🤖 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 `@apps/web/tests/helpers/expected-console.ts` around lines 78 - 81, The
`expectCalled` method does not guarantee that `release()` is called when the
assertion fails. If the `expect(entry.matchedCalls.length).toBeGreaterThan(0)`
assertion throws an error, the `release()` call will be skipped, causing capture
state to leak into later tests. Wrap the assertion in a try/finally block where
the assertion is in the try block and `release()` is in the finally block to
ensure cleanup always executes regardless of whether the assertion passes or
fails.
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\n- Captures TRR app dependency/profile updates from the takeover branch set.\n- Aggregates prior admin phase-0 hardening work onto main for review.\n\n## Local validation\n- PASS: make app-validate-quick\n- TRR-APP build: not run; full production build requires explicit current-chat approval.\n\n## Notes\n- Quick validation covered generated checks and focused Vitest coverage.
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Tests