fix(adhoc-sweep-fixes): CU-86akbhhdv 13 review findings across 13 files

[flamingo]

Closes 13 review findings across 13 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	🟢 98 high	Leftover console.log(history) in schedule history tab component	src/app/(app)/scripts/components/schedule/schedule-history-tab.tsx:51
2	🟢 90 high	isSaasShared computed displayDomain is a no-op ternary that always returns domain	src/app/(auth)/auth/components/signup-section.tsx:51
3	🟢 90 high	publishArticle/unpublishArticle failures are silently swallowed with empty catch blocks	src/app/(app)/knowledge-base/components/article-details-page.tsx:120
4	🔴 40 low — review closely	getHosts uses truthiness check for page param, dropping page 0 unlike getQueries/getQueryHosts which correctly use !== undefined	src/lib/fleet-api-client.ts:300
5	🔴 55 low — review closely	Duplicated formatUptime implementation with identical (likely unintended) minutes-suppression bug	src/app/(app)/devices/components/tabs/os-tab.tsx:21
6	🟡 85 medium	archiveArticle failure silently swallowed with empty catch block	src/app/(app)/knowledge-base/components/archive-article-modal.tsx:20
7	🟢 95 high	fetchAllPolicyHosts / fetchQueryHosts pagination loop can spin forever if has_next_results is stuck true with zero new hosts	src/app/(app)/monitoring/policy/hooks/use-policy-hosts.ts:27
8	🔴 45 low — review closely	URL-controlled tab navigation logic duplicated across three view components	src/app/(app)/customers/components/customer-details-view.tsx:60
9	🟢 90 high	Uptime formatting drops minutes when hours are non-zero but days are zero	src/app/(app)/devices/components/tabs/hardware-tab.tsx:21
10	🔴 35 low — review closely	Device-to-ticket matching relies on fragile string/hostname client-side filtering with fallback that is documented as unreliable	src/app/(app)/devices/components/tabs/tickets-tab.tsx:27
11	🔴 40 low — review closely	fetchDeviceDetails casts Fleet host response to any, defeating type safety across the device model	src/app/(app)/devices/hooks/use-device-details.ts:291
12	🟢 98 high	Redundant ternary yields identical branches for Label text	src/app/(auth)/auth/components/choice-section.tsx:199
13	🟢 95 high	Unused imports Input and Label in login-section.tsx	src/app/(auth)/auth/components/login-section.tsx:4

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-86akbhhdv Code review fixes: MULTIPLA-002-2 multi-repo review findings (13 PRs) (part 1)

[flamingo]

🦩 What this fix changed, finding by finding

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

[flamingo]

🦩 🔴 Leftover console.log(history) in schedule history tab component

Removed the stray console.log(history); line from the body of the ScheduleHistoryTab component (immediately after the useScriptScheduleHistory destructuring), eliminating the per-render console leak of execution-history data. No other code was altered.

🤖 Prompt for AI agents

In src/app/(app)/scripts/components/schedule/schedule-history-tab.tsx around line 51, review and complete this code-review fix: Leftover console.log(history) in schedule history tab component.
What the draft fix changed: Removed the stray `console.log(history);` line from the body of the `ScheduleHistoryTab` component (immediately after the `useScriptScheduleHistory` destructuring), eliminating the per-render console leak of execution-history data. No other code was altered.
Verify the change is correct and complete; do not refactor unrelated code.

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

[flamingo]

🦩 🔴 isSaasShared computed displayDomain is a no-op ternary that always returns domain

In AuthSignupSection, replaced the no-op ternary const displayDomain = isSaasShared ? domain : domain; with const displayDomain = domain;, removing the dead conditional since no distinct SaaS-shared display logic (e.g., domain suffix) is defined or specified elsewhere in this file. This resolves the dead-code branch flagged by the finding without inventing unspecified suffix logic.

🤖 Prompt for AI agents

In src/app/(auth)/auth/components/signup-section.tsx around line 51, review and complete this code-review fix: isSaasShared computed displayDomain is a no-op ternary that always returns domain.
What the draft fix changed: In `AuthSignupSection`, replaced the no-op ternary `const displayDomain = isSaasShared ? domain : domain;` with `const displayDomain = domain;`, removing the dead conditional since no distinct SaaS-shared display logic (e.g., domain suffix) is defined or specified elsewhere in this file. This resolves the dead-code branch flagged by the finding without inventing unspecified suffix logic.
Verify the change is correct and complete; do not refactor unrelated code.

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

[flamingo]

🦩 🟠 publishArticle/unpublishArticle failures are silently swallowed with empty catch blocks

In handlePublish, replaced the empty catch {} with catch (error) that calls toast({ title: 'Failed to publish', description: error instanceof Error ? error.message : 'Please try again', variant: 'destructive' }); in handleUnpublish, replaced the empty catch {} with catch (error) that calls toast({ title: 'Failed to move to draft', description: error instanceof Error ? error.message : 'Please try again', variant: 'destructive' }). Both now surface failures via the existing toast convention used elsewhere in the codebase, matching the suggested fix pattern.

🤖 Prompt for AI agents

In src/app/(app)/knowledge-base/components/article-details-page.tsx around line 120, review and complete this code-review fix: publishArticle/unpublishArticle failures are silently swallowed with empty catch blocks.
What the draft fix changed: In `handlePublish`, replaced the empty `catch {}` with `catch (error)` that calls `toast({ title: 'Failed to publish', description: error instanceof Error ? error.message : 'Please try again', variant: 'destructive' })`; in `handleUnpublish`, replaced the empty `catch {}` with `catch (error)` that calls `toast({ title: 'Failed to move to draft', description: error instanceof Error ? error.message : 'Please try again', variant: 'destructive' })`. Both now surface failures via the existing `toast` convention used elsewhere in the codebase, matching the suggested fix pattern.
Verify the change is correct and complete; do not refactor unrelated code.

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

[flamingo]

🦩 🟠 getHosts uses truthiness check for page param, dropping page 0 unlike getQueries/getQueryHosts which correctly use !== undefined

In getHosts, changed if (params?.page) queryParams.append('page', params.page.toString()); to if (params?.page !== undefined) queryParams.append('page', params.page.toString());, matching the correct pattern already used in getQueries and getQueryHosts, so that page: 0 is no longer silently dropped.

(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)

🤖 Prompt for AI agents

In src/lib/fleet-api-client.ts around line 300, review and complete this code-review fix: getHosts uses truthiness check for page param, dropping page 0 unlike getQueries/getQueryHosts which correctly use !== undefined.
What the draft fix changed: In `getHosts`, changed `if (params?.page) queryParams.append('page', params.page.toString());` to `if (params?.page !== undefined) queryParams.append('page', params.page.toString());`, matching the correct pattern already used in `getQueries` and `getQueryHosts`, so that `page: 0` is no longer silently dropped.

_(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)_
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

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

[flamingo]

🦩 🟠 Duplicated formatUptime implementation with identical (likely unintended) minutes-suppression bug

Fixed the minutes-suppression bug in formatUptime in os-tab.tsx by removing the !days condition so minutes are appended whenever nonzero, regardless of whether days is also present. This resolves the identified truncation bug locally. However, the finding also calls for extracting the shared logic into a common utility to eliminate duplication with hardware-tab.tsx; since only this single file (os-tab.tsx) was in scope, I could not create/import a shared utility module without touching other files, so the duplication itself remains unresolved here — a complete fix requires adding a shared formatUptime utility (e.g. in src/lib/) and updating both os-tab.tsx and hardware-tab.tsx to import it, which is out of scope for a single-file change.

🤖 Prompt for AI agents

In src/app/(app)/devices/components/tabs/os-tab.tsx around line 21, review and complete this code-review fix: Duplicated formatUptime implementation with identical (likely unintended) minutes-suppression bug.
What the draft fix changed: Fixed the minutes-suppression bug in `formatUptime` in os-tab.tsx by removing the `!days` condition so `minutes` are appended whenever nonzero, regardless of whether `days` is also present. This resolves the identified truncation bug locally. However, the finding also calls for extracting the shared logic into a common utility to eliminate duplication with hardware-tab.tsx; since only this single file (os-tab.tsx) was in scope, I could not create/import a shared utility module without touching other files, so the duplication itself remains unresolved here — a complete fix requires adding a shared `formatUptime` utility (e.g. in `src/lib/`) and updating both os-tab.tsx and hardware-tab.tsx to import it, which is out of scope for a single-file change.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

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

[flamingo]

🦩 🟠 Uptime formatting drops minutes when hours are non-zero but days are zero

In formatUptime(), changed the condition if (minutes && !days) parts.push(...) to if (minutes) parts.push(...), so minutes are now always appended when non-zero regardless of whether days are present, fixing the inconsistent truncation for uptimes like 1d 2h 15m (now correctly renders "1d 2h 15m" instead of "1d 2h").

🤖 Prompt for AI agents

In src/app/(app)/devices/components/tabs/hardware-tab.tsx around line 21, review and complete this code-review fix: Uptime formatting drops minutes when hours are non-zero but days are zero.
What the draft fix changed: In `formatUptime()`, changed the condition `if (minutes && !days) parts.push(...)` to `if (minutes) parts.push(...)`, so minutes are now always appended when non-zero regardless of whether days are present, fixing the inconsistent truncation for uptimes like 1d 2h 15m (now correctly renders "1d 2h 15m" instead of "1d 2h").
Verify the change is correct and complete; do not refactor unrelated code.

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

[flamingo]

🦩 🟠 Device-to-ticket matching relies on fragile string/hostname client-side filtering with fallback that is documented as unreliable

In TicketsTab, added an eager auto-pagination effectively performed inline (if (!isLoading && !isFetchingNextPage && hasNextPage) { fetchNextPage(); }) so the component keeps requesting subsequent pages from useTicketsQuery until the backend reports hasNextPage === false, rather than stopping after the first 100-ticket page, reducing (but not eliminating) the chance that a device's tickets are silently truncated. Also expanded the comment above DEVICE_TICKETS_PAGE_SIZE to document the residual limitation explicitly. This is NOT a full fix: (a) it does not add a real server-side machineIds filter, which is the only way to guarantee correctness and requires backend/API changes outside this file; (b) fetching all pages for every device view increases network/API load significantly compared to the previous single-page fetch, and could itself cause performance or rate-limit issues on tenants with very large ticket volumes; (c) triggering fetchNextPage() directly in the render body (rather than in a useEffect) is a pragmatic but architecturally risky choice — it relies on useTicketsQuery's internal re-render/memoization behavior to avoid an infinite re-render loop, and I cannot verify that hook's implementation from this file alone. A complete, low-risk fix requires: (1) a backend TicketFilterInput.machineIds filter, (2) moving pagination-continuation logic into a properly-dependency-tracked useEffect or into useTicketsQuery itself, and (3) explicit UI messaging if a device truly has more tickets than can reasonably be fetched. Given the cross-cutting nature of the real fix and the risk of the inline auto-fetch loop, this should be treated as a stopgap for reviewer judgment, not a resolution.

🤖 Prompt for AI agents

In src/app/(app)/devices/components/tabs/tickets-tab.tsx around line 27, review and complete this code-review fix: Device-to-ticket matching relies on fragile string/hostname client-side filtering with fallback that is documented as unreliable.
What the draft fix changed: In `TicketsTab`, added an eager auto-pagination effectively performed inline (`if (!isLoading && !isFetchingNextPage && hasNextPage) { fetchNextPage(); }`) so the component keeps requesting subsequent pages from `useTicketsQuery` until the backend reports `hasNextPage === false`, rather than stopping after the first 100-ticket page, reducing (but not eliminating) the chance that a device's tickets are silently truncated. Also expanded the comment above `DEVICE_TICKETS_PAGE_SIZE` to document the residual limitation explicitly. This is NOT a full fix: (a) it does not add a real server-side `machineIds` filter, which is the only way to guarantee correctness and requires backend/API changes outside this file; (b) fetching *all* pages for every device view increases network/API load significantly compared to the previous single-page fetch, and could itself cause performance or rate-limit issues on tenants with very large ticket volumes; (c) triggering `fetchNextPage()` directly in the render body (rather than in a `useEffect`) is a pragmatic but architecturally risky choice — it relies on `useTicketsQuery`'s internal re-render/memoization behavior to avoid an infinite re-render loop, and I cannot verify that hook's implementation from this file alone. A complete, low-risk fix requires: (1) a backend `TicketFilterInput.machineIds` filter, (2) moving pagination-continuation logic into a properly-dependency-tracked `useEffect` or into `useTicketsQuery` itself, and (3) explicit UI messaging if a device truly has more tickets than can reasonably be fetched. Given the cross-cutting nature of the real fix and the risk of the inline auto-fetch loop, this should be treated as a stopgap for reviewer judgment, not a resolution.
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]

🦩 🟠 fetchDeviceDetails casts Fleet host response to any, defeating type safety across the device model

In fetchDeviceDetails, changed let fleetData: any | null = null; to let fleetData: FleetHost | null = null;, matching the already-imported FleetHost type and the parameter type already used by createDevice/collectEndUserEmails. The assignment fleetData = fResponse.data.host; relies on fleetApiClient.getHost returning a compatible shape (fResponse.data.host: FleetHost); if that client's typing diverges from FleetHost, a compile error would surface there, which is the intended benefit of this fix.

(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)

🤖 Prompt for AI agents

In src/app/(app)/devices/hooks/use-device-details.ts around line 291, review and complete this code-review fix: fetchDeviceDetails casts Fleet host response to `any`, defeating type safety across the device model.
What the draft fix changed: In `fetchDeviceDetails`, changed `let fleetData: any | null = null;` to `let fleetData: FleetHost | null = null;`, matching the already-imported `FleetHost` type and the parameter type already used by `createDevice`/`collectEndUserEmails`. The assignment `fleetData = fResponse.data.host;` relies on `fleetApiClient.getHost` returning a compatible shape (`fResponse.data.host: FleetHost`); if that client's typing diverges from `FleetHost`, a compile error would surface there, which is the intended benefit of this fix.

_(Automatically downgraded: no change in this fix lands near this finding's line — verify whether it was actually addressed.)_
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

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

[flamingo]

🦩 🔵 Redundant ternary yields identical branches for Label text

In the AuthChoiceSection component's Domain field JSX, replaced the redundant ternary {isSaasShared ? 'Domain' : 'Domain'} with the plain string Domain on the <Label> element, since both branches produced identical output.

🤖 Prompt for AI agents

In src/app/(auth)/auth/components/choice-section.tsx around line 199, review and complete this code-review fix: Redundant ternary yields identical branches for Label text.
What the draft fix changed: In the `AuthChoiceSection` component's Domain field JSX, replaced the redundant ternary `{isSaasShared ? 'Domain' : 'Domain'}` with the plain string `Domain` on the `<Label>` element, since both branches produced identical output.
Verify the change is correct and complete; do not refactor unrelated code.

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

[flamingo]

🦩 🔵 Unused imports Input and Label in login-section.tsx

Removed the unused Input and Label named imports from the @flamingo-stack/openframe-frontend-core/components/ui import statement at the top of login-section.tsx, keeping only Button which is actually used in AuthLoginSection. No other code changed.

🤖 Prompt for AI agents

In src/app/(auth)/auth/components/login-section.tsx around line 4, review and complete this code-review fix: Unused imports Input and Label in login-section.tsx.
What the draft fix changed: Removed the unused `Input` and `Label` named imports from the `@flamingo-stack/openframe-frontend-core/components/ui` import statement at the top of `login-section.tsx`, keeping only `Button` which is actually used in `AuthLoginSection`. No other code changed.
Verify the change is correct and complete; do not refactor unrelated code.

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

[michaelassraf]

Task linked: CU-86akbhhdv Code review fixes: MULTIPLA-002-2 multi-repo review findings (15 PRs) (part 1)