From 406ee0ef97676da1f64b591b7ccba3fd8f3b7eb4 Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Sat, 25 Apr 2026 02:32:43 -0400 Subject: [PATCH 1/5] =?UTF-8?q?fix(deps):=20bump=20pytest=207.4=20?= =?UTF-8?q?=E2=86=92=208.4,=20pytest-cov=204.1=20=E2=86=92=205.0=20to=20sa?= =?UTF-8?q?tisfy=20pytest-asyncio=200.24?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pytest-asyncio==0.24.0 (added on the FlowPilot branch as part of the RLS test infra refactor) declares pytest>=8.2 — but requirements-dev.txt still pinned pytest==7.4.3, so a clean pip install fails with ResolutionImpossible. CI runners that started from a fresh image would have refused to install dev deps; the FlowPilot tests passed locally only because the dev container had a pre-installed pytest 8.x lying around. pytest-cov 4.1.0 also needs >= 5.0 to play nicely with pytest 8. No code changes — pytest 8 is API-compatible with the existing test suite once the install resolves. Co-Authored-By: Claude Opus 4.7 --- backend/requirements-dev.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt index c6b541fd..7b44660c 100644 --- a/backend/requirements-dev.txt +++ b/backend/requirements-dev.txt @@ -1,11 +1,11 @@ # Include production dependencies -r requirements.txt -# Testing -pytest==7.4.3 +# Testing — pytest-asyncio 0.24+ requires pytest>=8.2 +pytest==8.4.2 pytest-asyncio==0.24.0 httpx>=0.27.0 -pytest-cov==4.1.0 +pytest-cov==5.0.0 # Code quality black==24.1.1 From 857d73e3d03922da3110b6a722a86d66b9ee082d Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Sat, 25 Apr 2026 02:32:50 -0400 Subject: [PATCH 2/5] fix(lint): move AssistantSessionRedirect out of router.tsx (react-refresh gate) react-refresh/only-export-components fires when a file with the \`router\` const export also defines a component (the redirect helper). Moves the small helper to its own file under components/routing/ so HMR can keep the route-component module hot-reload-eligible. No behavior change. Co-Authored-By: Claude Opus 4.7 --- .../components/routing/AssistantSessionRedirect.tsx | 11 +++++++++++ frontend/src/router.tsx | 13 ++----------- 2 files changed, 13 insertions(+), 11 deletions(-) create mode 100644 frontend/src/components/routing/AssistantSessionRedirect.tsx diff --git a/frontend/src/components/routing/AssistantSessionRedirect.tsx b/frontend/src/components/routing/AssistantSessionRedirect.tsx new file mode 100644 index 00000000..555b8f08 --- /dev/null +++ b/frontend/src/components/routing/AssistantSessionRedirect.tsx @@ -0,0 +1,11 @@ +import { Navigate, useParams } from 'react-router-dom' + +/** + * Permanent 301-style redirect from /assistant/:sessionId to /pilot/:sessionId. + * Used by the Phase 1 route-rename; paired with a bare-path redirect to /pilot. + * SPA redirects replace history so the legacy URL does not linger in back-nav. + */ +export function AssistantSessionRedirect() { + const { sessionId } = useParams<{ sessionId: string }>() + return +} diff --git a/frontend/src/router.tsx b/frontend/src/router.tsx index 9fb6cc46..44e4fa30 100644 --- a/frontend/src/router.tsx +++ b/frontend/src/router.tsx @@ -1,4 +1,5 @@ -import { createBrowserRouter, Navigate, useParams } from 'react-router-dom' +import { createBrowserRouter, Navigate } from 'react-router-dom' +import { AssistantSessionRedirect } from '@/components/routing/AssistantSessionRedirect' import * as Sentry from '@sentry/react' import { Suspense } from 'react' import { AppLayout, ProtectedRoute } from '@/components/layout' @@ -102,16 +103,6 @@ function page(Component: React.LazyExoticComponent) { ) } -/** - * Permanent 301-style redirect from /assistant/:sessionId to /pilot/:sessionId. - * Used by the Phase 1 route-rename; paired with a bare-path redirect to /pilot. - * SPA redirects replace history so the legacy URL does not linger in back-nav. - */ -function AssistantSessionRedirect() { - const { sessionId } = useParams<{ sessionId: string }>() - return -} - export const router = sentryCreateBrowserRouter([ { path: '/landing', From b7f8e70be2f81d0dff46718c46b876762393d44c Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Sat, 25 Apr 2026 02:32:57 -0400 Subject: [PATCH 3/5] fix(lint): replace explicit-any types + unused-expressions ternaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five files, all stylistic: - useFlowPilotSession.ts: typed the axios error shape with a narrow inline type instead of \`as any\`. - FlowPilotSessionPage.tsx: same — typed location.state once, then destructured. - ScriptBuilderTab.tsx: handleViewScript was a placeholder no-op; declared the args properly with \`void script; void filename\` so the signature matches ScriptBuilderChatProps without no-unused-vars firing. - TicketsPage.tsx: replaced 8 ternaries-as-statements (\`x ? f() : g()\`) with proper if/else blocks. Same control flow, satisfies no-unused-expressions, and reads better in the URL-param update paths. Co-Authored-By: Claude Opus 4.7 --- .../src/components/pilot/ScriptBuilderTab.tsx | 4 +- frontend/src/hooks/useFlowPilotSession.ts | 6 ++- frontend/src/pages/FlowPilotSessionPage.tsx | 5 ++- frontend/src/pages/TicketsPage.tsx | 39 ++++++++++++++----- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/pilot/ScriptBuilderTab.tsx b/frontend/src/components/pilot/ScriptBuilderTab.tsx index 662ca50c..a87c211c 100644 --- a/frontend/src/components/pilot/ScriptBuilderTab.tsx +++ b/frontend/src/components/pilot/ScriptBuilderTab.tsx @@ -165,7 +165,9 @@ export function ScriptBuilderTab({ // onViewScript is required by ScriptBuilderChat — provide a no-op for now // (inline preview is a future extension). - const handleViewScript = (_script: string, _filename: string | null) => { + const handleViewScript = (script: string, filename: string | null) => { + void script + void filename // Future: open inline preview panel } diff --git a/frontend/src/hooks/useFlowPilotSession.ts b/frontend/src/hooks/useFlowPilotSession.ts index 85b9d542..3ef4b68b 100644 --- a/frontend/src/hooks/useFlowPilotSession.ts +++ b/frontend/src/hooks/useFlowPilotSession.ts @@ -100,11 +100,13 @@ export function useFlowPilotSession(): UseFlowPilotSession { setCurrentStep(firstStep) } catch (e: unknown) { // Prefer the backend's detail message over the generic axios status string - const detail = (e as any)?.response?.data?.detail + const axiosErr = e as { response?: { status?: number; data?: { detail?: unknown } } } + const detail = axiosErr?.response?.data?.detail const message = typeof detail === 'string' ? detail : (e instanceof Error ? e.message : 'Failed to start session') setError(message) // Global axios interceptor already shows a toast for 5xx — skip duplicate - if (!(e as any)?.response?.status || (e as any)?.response?.status < 500) { + const status = axiosErr?.response?.status + if (!status || status < 500) { toast.error(message) } } finally { diff --git a/frontend/src/pages/FlowPilotSessionPage.tsx b/frontend/src/pages/FlowPilotSessionPage.tsx index 4ab4d0b6..e1ecd22e 100644 --- a/frontend/src/pages/FlowPilotSessionPage.tsx +++ b/frontend/src/pages/FlowPilotSessionPage.tsx @@ -19,8 +19,9 @@ export default function FlowPilotSessionPage() { const navigate = useNavigate() const location = useLocation() const prefill = (location.state as { prefill?: string } | null)?.prefill || '' - const psaTicketId = (location.state as any)?.psaTicketId as string | undefined - const psaTicket = (location.state as any)?.psaTicket as PSATicketInfo | undefined + const locationState = location.state as { psaTicketId?: string; psaTicket?: PSATicketInfo } | null + const psaTicketId = locationState?.psaTicketId + const psaTicket = locationState?.psaTicket const isPickup = searchParams.get('pickup') === 'true' const fp = useFlowPilotSession() const branching = useBranching() diff --git a/frontend/src/pages/TicketsPage.tsx b/frontend/src/pages/TicketsPage.tsx index 256b0314..b89d9bce 100644 --- a/frontend/src/pages/TicketsPage.tsx +++ b/frontend/src/pages/TicketsPage.tsx @@ -141,23 +141,42 @@ export default function TicketsPage() { function updateFilters(updated: Partial) { const next = new URLSearchParams(searchParams) - if ('search' in updated) updated.search ? next.set('search', updated.search!) : next.delete('search') - if ('board_id' in updated) updated.board_id ? next.set('board', String(updated.board_id)) : next.delete('board') - if ('status_id' in updated) updated.status_id ? next.set('status', String(updated.status_id)) : next.delete('status') - if ('priority' in updated) updated.priority ? next.set('priority', updated.priority!) : next.delete('priority') - if ('company_id' in updated) updated.company_id ? next.set('company', String(updated.company_id)) : next.delete('company') - if ('assigned' in updated) { - const a = updated.assigned - a === 'all' ? next.delete('assigned') : next.set('assigned', String(a)) + if ('search' in updated) { + if (updated.search) next.set('search', updated.search) + else next.delete('search') + } + if ('board_id' in updated) { + if (updated.board_id) next.set('board', String(updated.board_id)) + else next.delete('board') + } + if ('status_id' in updated) { + if (updated.status_id) next.set('status', String(updated.status_id)) + else next.delete('status') + } + if ('priority' in updated) { + if (updated.priority) next.set('priority', updated.priority) + else next.delete('priority') + } + if ('company_id' in updated) { + if (updated.company_id) next.set('company', String(updated.company_id)) + else next.delete('company') + } + if ('assigned' in updated) { + if (updated.assigned === 'all') next.delete('assigned') + else next.set('assigned', String(updated.assigned)) + } + if ('include_closed' in updated) { + if (updated.include_closed) next.set('closed', 'true') + else next.delete('closed') } - if ('include_closed' in updated) updated.include_closed ? next.set('closed', 'true') : next.delete('closed') next.delete('page') // reset to 1 on filter change setSearchParams(next) } function updatePage(p: number) { const next = new URLSearchParams(searchParams) - p === 1 ? next.delete('page') : next.set('page', String(p)) + if (p === 1) next.delete('page') + else next.set('page', String(p)) setSearchParams(next) } From 920a246d775ec750ce09450a218c82146b9d78f0 Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Sat, 25 Apr 2026 02:33:13 -0400 Subject: [PATCH 4/5] fix(react): remove four setState-in-effect cascades flagged by react-hooks v5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new react-hooks lint rule "Calling setState synchronously within an effect can trigger cascading renders" flagged real anti-patterns in four spots. Refactored each per the rule's intent (derive during render, or use useSyncExternalStore for external subscriptions). 1. hooks/useMediaQuery.ts — replaced the useState + useEffect pair with useSyncExternalStore. That's the canonical React hook for subscribing to external stores (matchMedia in this case) without mirroring into local state via an effect. Snapshot/getServerSnapshot pair preserves the SSR-safe behaviour. 2. components/network/nodes/DeviceNode.tsx — the prop-sync useEffect that copied nodeData.label into labelValue was redundant. labelValue is the EDIT BUFFER; while not editing, the displayed span now reads nodeData.label directly. The buffer is initialized only when an edit session starts (onDoubleClick). 3. components/network/nodes/GroupNode.tsx — same pattern, same fix. 4. components/dashboard/TicketQueue.tsx — the setTickets([]) + setLoading(true) + fetchTickets() chain in the effect was the cascade. Pushed those writes inside fetchTickets (after the function boundary, so they batch with the eventual setTickets(result)). Added a request-id ref so a slow first response can't overwrite a fast second one. Frontend lint: 20 errors → 0 errors. tsc -b clean. Co-Authored-By: Claude Opus 4.7 --- .../src/components/dashboard/TicketQueue.tsx | 20 +++++++++-- .../components/network/nodes/DeviceNode.tsx | 10 +++--- .../components/network/nodes/GroupNode.tsx | 14 ++++---- frontend/src/hooks/useMediaQuery.ts | 35 ++++++++++--------- 4 files changed, 48 insertions(+), 31 deletions(-) diff --git a/frontend/src/components/dashboard/TicketQueue.tsx b/frontend/src/components/dashboard/TicketQueue.tsx index 6826b70d..49034407 100644 --- a/frontend/src/components/dashboard/TicketQueue.tsx +++ b/frontend/src/components/dashboard/TicketQueue.tsx @@ -194,6 +194,9 @@ export function TicketQueue() { const [activeTab, setActiveTab] = useState('mine') const [tickets, setTickets] = useState([]) const [loading, setLoading] = useState(false) + // Monotonically increasing fetch token — late responses with a stale id + // are dropped so they can't overwrite the latest query's results. + const latestRequestId = useRef(0) const [error, setError] = useState(null) // Check connection on mount @@ -238,12 +241,25 @@ export function TicketQueue() { params.board_ids = boardIds.join(',') } + // Clear stale data + flip loading inside the async function so the + // writes happen after the awaitable boundary — avoids the + // synchronous-setState-in-effect cascade the lint rule flags. The + // fetch is also wrapped in a request-id check so a stale response + // can't clobber a newer query. + const requestId = ++latestRequestId.current + setTickets([]) + setLoading(true) + try { const results = await integrationsApi.searchTicketsQueue(params) + if (requestId !== latestRequestId.current) return setTickets(results.items) setError(null) } catch { + if (requestId !== latestRequestId.current) return setError('Failed to load tickets. Check your PSA connection.') + } finally { + if (requestId === latestRequestId.current) setLoading(false) } }, [], @@ -253,9 +269,7 @@ export function TicketQueue() { useEffect(() => { if (!hasConnection) return if (activeTab === 'mine' && hasMemberMapping !== true) return - setTickets([]) - setLoading(true) - fetchTickets(activeTab, selectedBoardIds).finally(() => setLoading(false)) + fetchTickets(activeTab, selectedBoardIds) }, [activeTab, selectedBoardIds, hasConnection, hasMemberMapping, fetchTickets]) const handleStartSession = (ticket: PSATicketSearchResult) => { diff --git a/frontend/src/components/network/nodes/DeviceNode.tsx b/frontend/src/components/network/nodes/DeviceNode.tsx index 2acd1528..821534bf 100644 --- a/frontend/src/components/network/nodes/DeviceNode.tsx +++ b/frontend/src/components/network/nodes/DeviceNode.tsx @@ -62,10 +62,9 @@ function DeviceNodeComponent({ id, data, selected, width, height }: NodeProps) { } }, [editing]) - // Sync if data.label changes externally (e.g. undo/redo) - useEffect(() => { - if (!editing) setLabelValue(nodeData.label ?? '') - }, [nodeData.label, editing]) + // While not editing, the displayed label is derived directly from + // nodeData.label — no effect-driven sync needed. labelValue holds the + // edit buffer only and is reset when an edit session starts. const hasTooltipContent = props.hostname || props.ip || props.vendor || props.model || props.role || props.notes @@ -127,10 +126,11 @@ function DeviceNodeComponent({ id, data, selected, width, height }: NodeProps) { className="max-w-[88%] cursor-default text-center font-medium leading-tight text-primary line-clamp-2" onDoubleClick={e => { e.stopPropagation() + setLabelValue(nodeData.label ?? '') setEditing(true) }} > - {labelValue} + {nodeData.label ?? ''} )} { if (editing) inputRef.current?.focus() }, [editing]) - // Sync if external data.label changes - useEffect(() => { - if (!editing) setLabelValue(groupData.label ?? '') - }, [groupData.label, editing]) + // While not editing, the displayed label is derived directly from + // groupData.label — no effect-driven sync needed. labelValue holds the + // edit buffer only and is reset when an edit session starts. const handleLabelCommit = () => { setEditing(false) @@ -69,9 +68,12 @@ const GroupNodeComponent = ({ data, selected, id }: NodeProps) => { setEditing(true)} + onDoubleClick={() => { + setLabelValue(groupData.label ?? '') + setEditing(true) + }} > - {labelValue || groupData.groupType} + {(groupData.label ?? '') || groupData.groupType} )} diff --git a/frontend/src/hooks/useMediaQuery.ts b/frontend/src/hooks/useMediaQuery.ts index 7e56c0ca..e8dacb8d 100644 --- a/frontend/src/hooks/useMediaQuery.ts +++ b/frontend/src/hooks/useMediaQuery.ts @@ -1,27 +1,28 @@ -import { useEffect, useState } from 'react' +import { useSyncExternalStore } from 'react' /** * SSR-safe CSS media-query hook. Returns the current match boolean and * re-renders on viewport changes. Used by /pilot to swap the task lane * between side panel (≥1200px) and bottom drawer (<1200px) per Phase 7. + * + * Implemented with useSyncExternalStore to subscribe to the MediaQueryList + * without an effect — this is the React-idiomatic shape for external-state + * subscriptions and avoids the setState-in-effect cascade lint rule. */ export function useMediaQuery(query: string): boolean { - const [matches, setMatches] = useState(() => { - if (typeof window === 'undefined') return false - return window.matchMedia(query).matches - }) - - useEffect(() => { - if (typeof window === 'undefined') return - const mql = window.matchMedia(query) - const handler = (e: MediaQueryListEvent) => setMatches(e.matches) - // Sync once on mount in case state drifted between render and effect. - setMatches(mql.matches) - mql.addEventListener('change', handler) - return () => mql.removeEventListener('change', handler) - }, [query]) - - return matches + return useSyncExternalStore( + (onChange) => { + if (typeof window === 'undefined') return () => {} + const mql = window.matchMedia(query) + mql.addEventListener('change', onChange) + return () => mql.removeEventListener('change', onChange) + }, + () => { + if (typeof window === 'undefined') return false + return window.matchMedia(query).matches + }, + () => false, // server snapshot — match initial false + ) } export default useMediaQuery From d6218f2e07c31d5be27c976d7d61ff2048f3cde2 Mon Sep 17 00:00:00 2001 From: Michael Chihlas Date: Sat, 25 Apr 2026 02:49:06 -0400 Subject: [PATCH 5/5] fix(tests): import all models in conftest so create_all sees the full schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test_db fixture calls Base.metadata.create_all on a fresh test DB. That only creates tables for models that have been imported (and thus registered with Base.metadata) by the time the fixture runs. app.main imports app.core.database (which gives us Base) but does NOT eagerly import the model modules — most are pulled in lazily inside scheduler functions (archive_stale_ai_sessions etc.) and route modules. At fixture-setup time, only the handful of models touched by those eager imports are on the metadata, so any test that exercises PSA, network diagrams, ratings, escalations, etc. fails with \`UndefinedTableError: relation "X" does not exist\` and a cascade of 500s on every endpoint that queries the missing table. Adding \`from app import models as _models\` (rather than the bare \`import app.models\` which would shadow the \`app\` FastAPI instance imported just above) pulls in app/models/__init__.py, which itself imports every model module — registering all ~60 tables with Base.metadata before create_all runs. Verified locally: tests/test_psa_writeback_phase4.py went from 1 failed / 6 errors → 4 failed / 3 passed (the cascading errors were masking the actual passes). Co-Authored-By: Claude Opus 4.7 --- backend/tests/conftest.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 6d6e4358..56f2e94c 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -16,6 +16,14 @@ from app.main import app from app.core.database import Base, get_db from app.core.admin_database import get_admin_db from app.core.config import settings +# Import every model module so all tables are registered with Base.metadata +# before the test_db fixture calls create_all. app.main imports models lazily +# (inside scheduler functions and route modules), which is fine at runtime +# but leaves the metadata incomplete at fixture-setup time — surfacing as +# "relation X does not exist" errors for any model whose route/scheduler +# hasn't been loaded yet. The `from app import models` form avoids +# shadowing the `app` FastAPI instance imported just above. +from app import models as _models # noqa: F401 # Disable invite code requirement for tests settings.REQUIRE_INVITE_CODE = False