Stop two stale ids reaching the API, and steer the drag test off free days

Post-merge verification of the six merged branches turned up two real
frontend defects and one stale test expectation.

Settings mounts before WorkspaceContext has populated, so PartnerInvitePanel
asked for invitations with an empty workspace id. The empty path segment
collapses in Vite's proxy, FastAPI matches the single-segment workspace route
with workspace_id="invitations", and the UUID parse 422s. Guarded the same way
useWorkspaceMembers already does.

PlanSyncProvider could sync a workspace id seeded from localStorage before
WorkspaceContext refreshed it — a workspace a reseed had since deleted. It
logged the resulting 404 and kept retrying it on every planVersion bump. A 404
now refreshes the workspace context instead, correcting the id rather than
narrating the failure forever.

The drag-create-delete expectation was simply out of date: vacation-typed saves
correctly exclude free days, and targetRange() only skipped months ending on a
Sunday, so a Monday-ending month put a Sunday in the range. It now skips any
free day, computed independently of the calendar data the backend itself reads.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-21 09:22:30 +02:00
parent 44a18713a5
commit 2c46cf762d
6 changed files with 145 additions and 18 deletions
@@ -92,4 +92,14 @@ describe('PartnerInvitePanel', () => {
expect(await screen.findByRole('alert')).toHaveTextContent('list boom')
})
it('does not request invitations while workspaceId is still empty (Settings mounts before WorkspaceContext populates it)', async () => {
const { rerender } = render(<PartnerInvitePanel workspaceId="" />)
await waitFor(() => expect(screen.getByText('Laddar')).toBeInTheDocument())
expect(mockListInvitations).not.toHaveBeenCalled()
// Once the real workspace id lands, the panel fetches for it.
rerender(<PartnerInvitePanel workspaceId="ws-1" />)
await waitFor(() => expect(mockListInvitations).toHaveBeenCalledWith('ws-1'))
})
})
@@ -30,6 +30,15 @@ export function PartnerInvitePanel({ workspaceId }: PartnerInvitePanelProps) {
const [revokeError, setRevokeError] = useState<string | null>(null)
const loadInvitations = useCallback(async () => {
// Settings mounts (via RequireAuth) before WorkspaceContext has
// populated `workspaceId`, so this can run once with `workspaceId`
// still `''`. An empty path segment collapses in Vite's dev proxy, so
// `GET /api/workspace//invitations` becomes `GET /api/workspace/invitations`
// — FastAPI then matches `workspace_id="invitations"` against the
// `{workspace_id}` route and 422s on UUID parsing. Same guard as
// `useWorkspaceMembers.ts`; the effect below re-runs once the real id
// lands (it's a dependency of this callback).
if (!workspaceId) return
setIsListLoading(true)
setListError(null)
try {
@@ -185,6 +185,27 @@ describe('PlanSyncProvider', () => {
consoleErrorSpy.mockRestore()
})
it('refreshes the workspace context instead of just logging when the target workspace 404s (stale id)', async () => {
const refreshContext = vi.fn()
mockWorkspace({ workspaceId: 'ws-stale', planVersion: 0, bumpPlanVersion, refreshContext })
vi.mocked(api.syncPlan).mockRejectedValueOnce(
new ApiRequestError(404, { code: 'not_found', message: 'Workspace ws-stale not found', details: {} }),
)
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
renderHook(() => usePlanSyncContext(), { wrapper })
await waitFor(() => expect(refreshContext).toHaveBeenCalledTimes(1))
// A 404 is the expected "stale reference" case, not an unhandled
// background failure — it must not also go through the generic
// console.error path.
expect(consoleErrorSpy).not.toHaveBeenCalled()
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('ws-stale'))
consoleWarnSpy.mockRestore()
consoleErrorSpy.mockRestore()
})
it('logs a non-Error rejection without swallowing it', async () => {
vi.mocked(api.syncPlan).mockRejectedValueOnce('network exploded')
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
+28 -11
View File
@@ -59,7 +59,7 @@ const PlanSyncContext = createContext<PlanSyncContextValue | null>(null)
* a "plan input" write itself.
*/
export function PlanSyncProvider({ children }: { children: React.ReactNode }) {
const { workspaceId, planVersion } = useWorkspace()
const { workspaceId, planVersion, refreshContext } = useWorkspace()
const { isAuthenticated } = useAuth()
const [latestProposal, setLatestProposal] = useState<PlanProposal | null>(null)
const [latestProposalVersion, setLatestProposalVersion] = useState(0)
@@ -98,15 +98,32 @@ export function PlanSyncProvider({ children }: { children: React.ReactNode }) {
setLatestProposal(result.proposal)
setLatestProposalVersion((v) => v + 1)
} catch (err) {
// No toast exists for a background sync — this is the minimum bar
// from the module doc comment: never an empty catch, always logged
// through the shared API-error model instead of silently dropped.
const message = isApiError(err)
? `${err.status} ${err.error.message}`
: err instanceof Error
? err.message
: String(err)
console.error(`Background plan sync failed for workspace ${id}: ${message}`)
if (isApiError(err) && err.status === 404) {
// `id` doesn't exist server-side any more — not the active
// workspace, just a stale one this effect was handed (e.g.
// `workspaceId` seeded synchronously from localStorage on cold load
// — see `WorkspaceContext.tsx` — racing ahead of that context's own
// `refreshContext()` call, or a workspace a reseed since dropped).
// Retrying `id` forever would just 404 on every future planVersion
// bump too, so refetch the caller's *current* workspace from the
// server instead of only logging; that both corrects `workspaceId`
// (re-triggering this effect for the real id) and overwrites the
// stale value this provider never itself sets/clears.
console.warn(
`Background plan sync targeted stale workspace ${id} (not found) — refreshing workspace context`,
)
void refreshContext()
} else {
// No toast exists for a background sync — this is the minimum bar
// from the module doc comment: never an empty catch, always logged
// through the shared API-error model instead of silently dropped.
const message = isApiError(err)
? `${err.status} ${err.error.message}`
: err instanceof Error
? err.message
: String(err)
console.error(`Background plan sync failed for workspace ${id}: ${message}`)
}
} finally {
inFlightRef.current = false
}
@@ -116,7 +133,7 @@ export function PlanSyncProvider({ children }: { children: React.ReactNode }) {
pendingWorkspaceIdRef.current = null
await runSyncOnce(nextId)
}
}, [])
}, [refreshContext])
useEffect(() => {
// `workspaceId` is seeded synchronously from `localStorage` (see