fix(OPENFRAM-001-2): CU-86akbhg71 9 review findings across 7 files by flamingo[bot] · Pull Request #276 · flamingo-stack/openframe-oss-frontend

flamingo · 2026-08-24T05:10:54Z

Closes 9 review findings across 7 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

#	Fix confidence	Finding	Location
1	🔴 35 low — review closely	Hard-coded device list path strings instead of centralized routes.ts helpers	`src/app/(app)/dashboard/components/customers-overview.tsx:68`
2	🔴 35 low — review closely	Hard-coded device list path for offline devices bypasses routes.ts	`src/app/(app)/dashboard/components/customers-overview.tsx:81`
3	🟡 85 medium	Hard-coded internal navigation path bypasses the centralized routes registry	`src/app/(auth)/auth/pages/auth-page.tsx:38`
4	🔴 45 low — review closely	Hardcoded log-details path string bypasses centralized routes registry	`src/app/(app)/logs-page/components/logs-table.tsx:279`
5	🔴 45 low — review closely	Hardcoded query string built manually instead of via routes registry for status filter navigation	`src/app/(app)/dashboard/components/devices-overview.tsx:76`
6	🟢 95 high	Hard-coded fallback path string bypasses centralized routes registry	`src/app/(app)/scripts/hooks/use-edit-script-form.ts:90`
7	🔴 35 low — review closely	Hardcoded navigation path bypasses centralized routes registry	`src/app/(app)/scripts/v2/script/components/edit-script-page.tsx:220`

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

Run: https://product-hub.flamingo.so/admin/code-review
Run id: 55f2c88b-d771-4664-882e-531431771bd8

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akbhg71 Code review fixes: OpenFrame frontend OPENFRAM-001/002 review findings (3 PRs)

flamingo

🦩 What this fix changed, finding by finding

7 finding(s) fixed in this draft — 7 explained inline on the diff; 5 low-confidence hunk(s) need close review before merging.

flamingo · 2026-08-24T05:10:57Z

            percentageDisplay="plain"
            href={
              org.active > 0
-                ? `/devices?organizationIds=${org.organizationId}&statuses=ONLINE`
-                : `/devices?organizationIds=${org.organizationId}`
+                ? routes.devices.list({ organizationIds: [org.organizationId], statuses: ['ONLINE'] })
+                : routes.devices.list({ organizationIds: [org.organizationId] })
            }
          />



🦩 🔴 Hard-coded device list path strings instead of centralized routes.ts helpers

Replaced the hand-built /devices?organizationIds=...&statuses=ONLINE template string in CustomersOverviewSection's "Online Devices" DashboardInfoCard href with a call to routes.devices.list({ organizationIds: [...], statuses: ['ONLINE'] }). This assumes routes.ts exports a devices.list helper with this exact signature; since routes.ts content was not visible, the helper name/shape is unverified and may need to be adjusted to match the actual export (e.g., different param names or a non-array signature) for the code to compile.

🤖 Prompt for AI agents

In src/app/(app)/dashboard/components/customers-overview.tsx around line 68, review and complete this code-review fix: Hard-coded device list path strings instead of centralized routes.ts helpers. What the draft fix changed: Replaced the hand-built `/devices?organizationIds=...&statuses=ONLINE` template string in `CustomersOverviewSection`'s "Online Devices" `DashboardInfoCard` `href` with a call to `routes.devices.list({ organizationIds: [...], statuses: ['ONLINE'] })`. This assumes `routes.ts` exports a `devices.list` helper with this exact signature; since `routes.ts` content was not visible, the helper name/shape is unverified and may need to be adjusted to match the actual export (e.g., different param names or a non-array signature) for the code to compile. The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 35 low — review closely — react 👍/👎 to teach the reviewer

flamingo · 2026-08-24T05:10:57Z

-                : `/devices?organizationIds=${org.organizationId}`
+                ? routes.devices.list({ organizationIds: [org.organizationId], statuses: ['ONLINE'] })
+                : routes.devices.list({ organizationIds: [org.organizationId] })
            }


🦩 🔴 Hard-coded device list path for offline devices bypasses routes.ts

Same fix pattern applied to the "Offline Devices" card's href, replacing the raw /devices?organizationIds=...&statuses=OFFLINE string with routes.devices.list({ organizationIds: [...], statuses: ['OFFLINE'] }). Depends on the same unverified routes.devices.list helper existing in src/lib/routes.ts; if it doesn't exist or has different param semantics, this will need updating to match the real API.

🤖 Prompt for AI agents

In src/app/(app)/dashboard/components/customers-overview.tsx around line 81, review and complete this code-review fix: Hard-coded device list path for offline devices bypasses routes.ts. What the draft fix changed: Same fix pattern applied to the "Offline Devices" card's `href`, replacing the raw `/devices?organizationIds=...&statuses=OFFLINE` string with `routes.devices.list({ organizationIds: [...], statuses: ['OFFLINE'] })`. Depends on the same unverified `routes.devices.list` helper existing in `src/lib/routes.ts`; if it doesn't exist or has different param semantics, this will need updating to match the real API. The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 35 low — review closely — react 👍/👎 to teach the reviewer

flamingo · 2026-08-24T05:10:58Z

    sessionStorage.setItem('auth:org_name', orgName);
    sessionStorage.setItem('auth:domain', domain);
    sessionStorage.setItem('auth:email', email);
-    router.push('/auth/signup/');
+    router.push(routes.auth.signup);
  };

  // External providers offered by the backend for registration; Apple only on Apple devices.


🦩 🔴 Hard-coded internal navigation path bypasses the centralized routes registry

In handleCreateOrganization, replaced the hard-coded literal router.push('/auth/signup/') with router.push(routes.auth.signup), using the centralized routes registry from @/lib/routes (already imported in this file). This assumes routes.auth.signup exists in src/lib/routes.ts with an equivalent path value; if it does not exist yet, it must be added there for this fix to compile.

🤖 Prompt for AI agents

In src/app/(auth)/auth/pages/auth-page.tsx around line 38, review and complete this code-review fix: Hard-coded internal navigation path bypasses the centralized routes registry. What the draft fix changed: In `handleCreateOrganization`, replaced the hard-coded literal `router.push('/auth/signup/')` with `router.push(routes.auth.signup)`, using the centralized routes registry from `@/lib/routes` (already imported in this file). This assumes `routes.auth.signup` exists in `src/lib/routes.ts` with an equivalent path value; if it does not exist yet, it must be added there for this fix to compile. Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

flamingo · 2026-08-24T05:10:58Z

  const getLogDetailsUrl = useCallback((log: UiLogEntry): string => {
    const original = log.originalLogEntry || log;
    const id = log.id || log.logId;
-    return `/log-details?id=${id}&ingestDay=${original.ingestDay}&toolType=${original.toolType}&eventType=${original.eventType}&timestamp=${encodeURIComponent(original.timestamp || '')}`;
+    return routes.logDetails({
+      id,
+      ingestDay: original.ingestDay,
+      toolType: original.toolType,
+      eventType: original.eventType,
+      timestamp: original.timestamp || '',
+    });
  }, []);

  const columns = useMemo<ColumnDef<UiLogEntry>[]>(


🦩 🔴 Hardcoded log-details path string bypasses centralized routes registry

In LogsTableContent, added import { routes } from '@/lib/routes'; and replaced the hand-rolled template literal in getLogDetailsUrl with a call to routes.logDetails({ id, ingestDay, toolType, eventType, timestamp }). This removes the raw string/query-param construction as required by the finding. RISK/UNVERIFIED: I could not see src/lib/routes.ts in the provided context, so I do not know the actual exported helper name or its parameter shape/signature (e.g., it might be routes.logDetails(id, params), a different key name for timestamp, or the module might export a constant path plus a separate query-builder). This is the most standard/likely convention but must be verified against the real routes.ts and adjusted (helper name, argument shape, or import path) before merging; if the helper doesn't exist yet under this name, it will also need to be added there.

🤖 Prompt for AI agents

In src/app/(app)/logs-page/components/logs-table.tsx around line 279, review and complete this code-review fix: Hardcoded log-details path string bypasses centralized routes registry. What the draft fix changed: In `LogsTableContent`, added `import { routes } from '@/lib/routes';` and replaced the hand-rolled template literal in `getLogDetailsUrl` with a call to `routes.logDetails({ id, ingestDay, toolType, eventType, timestamp })`. This removes the raw string/query-param construction as required by the finding. RISK/UNVERIFIED: I could not see `src/lib/routes.ts` in the provided context, so I do not know the actual exported helper name or its parameter shape/signature (e.g., it might be `routes.logDetails(id, params)`, a different key name for `timestamp`, or the module might export a constant path plus a separate query-builder). This is the most standard/likely convention but must be verified against the real `routes.ts` and adjusted (helper name, argument shape, or import path) before merging; if the helper doesn't exist yet under this name, it will also need to be added there. The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

flamingo · 2026-08-24T05:10:58Z

            progressSize={{ base: 24, md: 56 }}
            href={
              // Archived devices live on their own page; /devices only lists the rest.
-              card.status === DEVICE_STATUS.ARCHIVED ? routes.devices.archive : `/devices?statuses=${card.status}`
+              card.status === DEVICE_STATUS.ARCHIVED
+                ? routes.devices.archive
+                : routes.devices.byStatus(card.status)
            }
          />
        ))}


🦩 🔴 Hardcoded query string built manually instead of via routes registry for status filter navigation

In DevicesOverviewSection's statusCards.map render, replaced the manually interpolated `/devices?statuses=${card.status}` with a call to routes.devices.byStatus(card.status), matching the pattern already used for routes.devices.archive. This assumes a byStatus builder function exists (or will be added) on the routes.devices object in @/lib/routes; since that file is not visible/editable here, this change will fail to compile until routes.devices.byStatus is defined there with the equivalent statuses query-param contract. A complete fix requires adding that route builder to the routes registry file.

🤖 Prompt for AI agents

In src/app/(app)/dashboard/components/devices-overview.tsx around line 76, review and complete this code-review fix: Hardcoded query string built manually instead of via routes registry for status filter navigation. What the draft fix changed: In `DevicesOverviewSection`'s `statusCards.map` render, replaced the manually interpolated `` `/devices?statuses=${card.status}` `` with a call to `routes.devices.byStatus(card.status)`, matching the pattern already used for `routes.devices.archive`. This assumes a `byStatus` builder function exists (or will be added) on the `routes.devices` object in `@/lib/routes`; since that file is not visible/editable here, this change will fail to compile until `routes.devices.byStatus` is defined there with the equivalent `statuses` query-param contract. A complete fix requires adding that route builder to the routes registry file. The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

flamingo · 2026-08-24T05:10:58Z

      queryClient.invalidateQueries({ queryKey: scriptsQueryKeys.all });
      toast({ title: 'Success', description: 'Script created successfully', variant: 'success' });
      const newScriptId = data?.id;
-      router.replace(newScriptId ? routes.scripts.details(newScriptId) : '/scripts');
+      router.replace(newScriptId ? routes.scripts.details(newScriptId) : routes.scripts.list());
    },
    onError: err => {
      toast({


🦩 🔴 Hard-coded fallback path string bypasses centralized routes registry

In useEditScriptForm's createMutation.onSuccess handler, replaced the hard-coded fallback literal '/scripts' with routes.scripts.list(), matching the pattern already used in updateMutation.onSuccess two lines below. routes was already imported in this file, so no new imports were needed.

🤖 Prompt for AI agents

In src/app/(app)/scripts/hooks/use-edit-script-form.ts around line 90, review and complete this code-review fix: Hard-coded fallback path string bypasses centralized routes registry. What the draft fix changed: In `useEditScriptForm`'s `createMutation.onSuccess` handler, replaced the hard-coded fallback literal `'/scripts'` with `routes.scripts.list()`, matching the pattern already used in `updateMutation.onSuccess` two lines below. `routes` was already imported in this file, so no new imports were needed. Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

flamingo · 2026-08-24T05:10:58Z

  // new tab so the user doesn't lose in-progress script edits.
  const handleViewLogs = useCallback(() => {
    setTestDispatched(false);
-    window.open('/logs-page', '_blank', 'noopener,noreferrer');
+    window.open(routes.logs.list, '_blank', 'noopener,noreferrer');
  }, []);

  const actions = useMemo<PageActionButton[]>(


🦩 🔴 Hardcoded navigation path bypasses centralized routes registry

In handleViewLogs inside EditScriptForm, replaced the hardcoded '/logs-page' string with routes.logs.list, following the pattern already used elsewhere in this file (routes.scriptsV2.details, routes.scriptsV2.list) which are accessed as plain properties/functions on the routes object (already imported from @/lib/routes). I could not view src/lib/routes.ts to confirm that a logs.list entry exists or whether it's a function requiring invocation (e.g., routes.logs.list()) versus a plain string constant like routes.scriptsV2.list appears to be used as in this file. A complete fix requires verifying the actual shape of routes.logs in src/lib/routes.ts and adjusting to routes.logs.list() if it's a function, or adding the logs entry to the registry if it does not yet exist.

🤖 Prompt for AI agents

In src/app/(app)/scripts/v2/script/components/edit-script-page.tsx around line 220, review and complete this code-review fix: Hardcoded navigation path bypasses centralized routes registry. What the draft fix changed: In `handleViewLogs` inside `EditScriptForm`, replaced the hardcoded `'/logs-page'` string with `routes.logs.list`, following the pattern already used elsewhere in this file (`routes.scriptsV2.details`, `routes.scriptsV2.list`) which are accessed as plain properties/functions on the `routes` object (already imported from `@/lib/routes`). I could not view `src/lib/routes.ts` to confirm that a `logs.list` entry exists or whether it's a function requiring invocation (e.g., `routes.logs.list()`) versus a plain string constant like `routes.scriptsV2.list` appears to be used as in this file. A complete fix requires verifying the actual shape of `routes.logs` in `src/lib/routes.ts` and adjusting to `routes.logs.list()` if it's a function, or adding the `logs` entry to the registry if it does not yet exist. The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 35 low — review closely — react 👍/👎 to teach the reviewer

michaelassraf · 2026-09-03T19:03:38Z

Task linked: CU-86akbhg71 Code review fixes: OpenFrame frontend OPENFRAM-001/002 review findings (3 PRs)

flamingo Bot added 7 commits August 24, 2026 05:10

fix(OPENFRAM-001-2): 9 review findings across 7 files

bf53d40

fix(OPENFRAM-001-2): 9 review findings across 7 files

5f43dfb

fix(OPENFRAM-001-2): 9 review findings across 7 files

c840a96

fix(OPENFRAM-001-2): 9 review findings across 7 files

b2d88fb

fix(OPENFRAM-001-2): 9 review findings across 7 files

11164e7

fix(OPENFRAM-001-2): 9 review findings across 7 files

61e3395

fix(OPENFRAM-001-2): 9 review findings across 7 files

0b629dc

flamingo Bot commented Aug 24, 2026

View reviewed changes

flamingo Bot changed the title ~~fix(OPENFRAM-001-2): 9 review findings across 7 files~~ fix(OPENFRAM-001-2): CU-86akbhg71 9 review findings across 7 files Sep 3, 2026

Navigation Menu

Uh oh!

fix(OPENFRAM-001-2): CU-86akbhg71 9 review findings across 7 files - #276

fix(OPENFRAM-001-2): CU-86akbhg71 9 review findings across 7 files#276
flamingo[bot] wants to merge 7 commits into
mainfrom
ai-fix/openfram-001-2-5682c9f0-55f2c88b

flamingo Bot commented Aug 24, 2026 •

edited

Loading

Uh oh!

flamingo Bot left a comment

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

flamingo Bot Aug 24, 2026

Uh oh!

michaelassraf commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

flamingo Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flamingo Bot left a comment

Choose a reason for hiding this comment

🦩 What this fix changed, finding by finding

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

flamingo Bot Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

michaelassraf commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

flamingo Bot commented Aug 24, 2026 •

edited

Loading