feat(v1.1/W2): TanStack Query foundation + typed API client + Sanctum auth#6
Merged
Merged
Conversation
… auth
Wave W2 of the v1.1 cycle establishes the data layer that backs every
read + mutation in v1.1. The SPA stays fully functional on fixture data
after this PR merges; W3 swaps pages page-by-page.
Files added
- resources/js/lib/api/types.ts — 19 hand-written types mirroring OpenAPI 3.1 v1.5
- resources/js/lib/api/errors.ts — typed error hierarchy (ApiError + 5 subclasses)
- resources/js/lib/api/client.ts — axios singleton + Sanctum XSRF interceptor + response error mapper
- resources/js/lib/api/endpoints.ts — 22 typed endpoint helpers
- resources/js/lib/queries/queryClient.ts — admin-tuned shared QueryClient
- resources/js/lib/queries/keys.ts — centralised query-key factory
- resources/js/lib/queries/hooks.ts — 13 read hooks
- resources/js/lib/mutations/hooks.ts — 10 mutation hooks (incl. R21 confirm-token protocol)
- resources/js/env.d.ts — VITE_API_BASE type declaration
- tests/js/lib/api/{client,endpoints}.test.ts — 35 specs
- tests/js/lib/queries/hooks.test.tsx — 9 specs
- tests/js/lib/mutations/hooks.test.tsx — 9 specs
- tests/js/lib/queries/wrapper.tsx — shared <QueryClientProvider> test factory
- tests/js/lib/api/server.ts — shared MSW v2 setupServer
Files modified
- resources/js/main.tsx — QueryClientProvider + ReactQueryDevtools wrapping
- resources/js/App.tsx — auth:expired event listener + dedicated toast (R11)
- resources/js/lib/ui.tsx — toast root now passes through data-testid (R11)
- tests/js/setup.ts — MSW lifecycle wiring
- vite.config.ts — axios + TanStack Query in optimizeDeps.include
- CHANGELOG.md — [Unreleased] → v1.1.0 entry
- package.json + package-lock.json — added @tanstack/react-query, axios, devtools, msw
Test delta
- Vitest 7 → 64 (+57)
- PHPUnit 8 → 8 (unchanged — no PHP touched)
- Build: 346 KB / 99 KB gzipped (~+56 KB raw / ~+14 KB gzipped)
R-rules honoured
- R11 toast testid passthrough + auth-expired-toast testid
- R14 typed errors thrown, never silent success
- R19 every dynamic path segment via encodeURIComponent
- R21 two-call confirm-token protocol on invokeTool / replayAudit / resetBreaker
- R30 client never sets X-Tenant-Id; host middleware owns tenant resolution
- Standalone-agnostic invariant preserved (zero AskMyDocs host refs)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ab36cfcaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+123
to
+124
| qc.invalidateQueries({ queryKey: keys.servers.detail(id) }); | ||
| qc.invalidateQueries({ queryKey: keys.servers.tools(id) }); |
There was a problem hiding this comment.
When a server handshake refreshes the server's advertised tools, this only invalidates the detail and per-server tools queries. The global useTools() hook is cached under keys.tools.all() with a 30s staleTime, so if the operator has visited the Tools page and then runs a handshake, returning to the global tools view can continue showing the pre-handshake tool set until the cache ages out. Please invalidate keys.tools.all() here as well.
Useful? React with 👍 / 👎.
CI's `npm ci` rejected the previous lockfile with: ``` npm error Missing: @emnapi/core@1.10.0 from lock file npm error Missing: @emnapi/runtime@1.10.0 from lock file ``` These are platform-specific optional transitive deps (from `node-fetch` ecosystem on Linux) that the local Windows `npm install` did not materialise into `package-lock.json`. Result: `package.json` ↔ `package-lock.json` were out-of-sync from CI's perspective. Fix: regenerated the lockfile via `rm -rf node_modules package-lock.json && npm install` so the platform-specific optional deps are properly recorded. Local verification after regen: - `npm test` — 64/64 green (unchanged) - `npm run typecheck` — clean - `npm run build` — 346.13 KB / 98.53 KB gzipped (unchanged) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces the v1.1/W2 data-layer foundation for the admin SPA: a typed axios API client (Sanctum/XSRF + error normalization), typed endpoint helpers, and TanStack Query query/mutation hooks, plus MSW-backed Vitest coverage and minimal shell wiring (QueryClientProvider + auth-expired toast).
Changes:
- Added typed API surface (
types.ts,errors.ts,client.ts,endpoints.ts) including confirm-token protocol handling and auth-expired signaling. - Added TanStack Query client defaults, query-key factory, and a set of read/mutation hooks.
- Added MSW server + Vitest setup + new unit tests for client/endpoints/hooks; wired QueryClientProvider + devtools in the app shell.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Pre-bundles axios/TanStack deps for faster Vite dev cold start. |
| tests/js/setup.ts | Adds Vitest lifecycle hooks to start/reset/stop MSW server; adjusts test runtime API base. |
| tests/js/lib/queries/wrapper.tsx | Test helper to create an isolated QueryClientProvider per spec. |
| tests/js/lib/queries/hooks.test.tsx | Happy/failure-path coverage for read hooks using MSW. |
| tests/js/lib/mutations/hooks.test.tsx | Mutation hook coverage including confirm-token destructive flows. |
| tests/js/lib/api/server.ts | Shared MSW node server instance. |
| tests/js/lib/api/endpoints.test.ts | Endpoint helper coverage including confirm-token round trips and path encoding. |
| tests/js/lib/api/client.test.ts | Axios client interceptor/error-mapping coverage + auth:expired event spec. |
| resources/js/main.tsx | Wraps app in QueryClientProvider and mounts React Query devtools in DEV. |
| resources/js/lib/ui.tsx | Toast DOM now passes through data-testid from toast payload. |
| resources/js/lib/queries/queryClient.ts | Creates the shared QueryClient with admin-tuned defaults and retry policy. |
| resources/js/lib/queries/keys.ts | Centralized query-key factory used by hooks/mutations. |
| resources/js/lib/queries/hooks.ts | Introduces the GET/read hooks backed by endpoint helpers. |
| resources/js/lib/mutations/hooks.ts | Introduces mutation hooks with invalidation + optimistic revoke flow. |
| resources/js/lib/api/types.ts | Adds hand-written OpenAPI-mirroring TypeScript types/envelopes/filters. |
| resources/js/lib/api/errors.ts | Adds typed ApiError hierarchy used by axios client + TanStack Query. |
| resources/js/lib/api/endpoints.ts | Adds typed endpoint functions + confirm-token protocol + SSE subscription helper. |
| resources/js/lib/api/client.ts | Adds axios singleton/factory with Sanctum XSRF echoing + response error normalization. |
| resources/js/env.d.ts | Declares Vite env typings for VITE_API_BASE and mode flags. |
| resources/js/App.tsx | Listens for auth:expired and pushes a dedicated toast. |
| package.json | Adds axios/TanStack Query/MSW deps and updates Node engine constraints. |
| package-lock.json | Locks new dependencies and reflects updated engines metadata. |
| CHANGELOG.md | Documents the W2 foundation, API surface, hooks, and test additions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+165
to
188
| const onAuthExpired = () => { | ||
| toast.push({ | ||
| kind: 'err', | ||
| title: 'Session expired', | ||
| body: 'Please reload the page to sign in again.', | ||
| testId: 'auth-expired-toast', | ||
| }); | ||
| }; | ||
| document.addEventListener('app:toggle-theme', onToggleTheme as any); | ||
| document.addEventListener('app:toggle-paused', onTogglePaused as any); | ||
| document.addEventListener('app:start-tour', onStartTour as any); | ||
| document.addEventListener('app:nav', onNavEvt as any); | ||
| document.addEventListener('app:open-audit', onOpenAudit as any); | ||
| document.addEventListener('auth:expired', onAuthExpired as any); | ||
| return () => { | ||
| document.removeEventListener('app:toggle-theme', onToggleTheme as any); | ||
| document.removeEventListener('app:toggle-paused', onTogglePaused as any); | ||
| document.removeEventListener('app:start-tour', onStartTour as any); | ||
| document.removeEventListener('app:nav', onNavEvt as any); | ||
| document.removeEventListener('app:open-audit', onOpenAudit as any); | ||
| document.removeEventListener('auth:expired', onAuthExpired as any); | ||
| }; | ||
| }, [nav]); | ||
| }, [nav, toast]); | ||
|
|
Comment on lines
+162
to
+171
| // The API client interceptor fires this event on every 401 so the SPA | ||
| // can surface a single, deduplicated session-expired toast (W2 wiring; | ||
| // actual sign-out / refresh flow lives further upstream). | ||
| const onAuthExpired = () => { | ||
| toast.push({ | ||
| kind: 'err', | ||
| title: 'Session expired', | ||
| body: 'Please reload the page to sign in again.', | ||
| testId: 'auth-expired-toast', | ||
| }); |
Comment on lines
+8
to
+9
| // Tests build their own client via `queries/testWrapper.tsx` with retries | ||
| // disabled and `gcTime: 0` so cache state doesn't leak between specs. |
| HostApiKey, | ||
| HostApiKeyCreateEnvelope, | ||
| HostTenant, | ||
| HostUser, |
Comment on lines
+285
to
+299
| /** | ||
| * Subscribe to the audit-invocation event stream. Returns a cleanup function | ||
| * that closes the underlying `EventSource`. Errors are surfaced via the | ||
| * `onError` callback; the EventSource auto-reconnects per spec until closed. | ||
| */ | ||
| export function subscribeEvents( | ||
| onMessage: (event: AuditEvent) => void, | ||
| onError?: (err: Event) => void, | ||
| ): () => void { | ||
| if (typeof EventSource === 'undefined') { | ||
| // jsdom + node — no-op subscription useful for tests. | ||
| return () => undefined; | ||
| } | ||
|
|
||
| // SSE doesn't carry the XSRF cookie reliably through cross-origin EventSource; |
Comment on lines
+21
to
+26
| beforeEach(() => { | ||
| // Reset the singleton client between tests so interceptor mutations don't | ||
| // leak across files. Re-create against the canonical test base URL. | ||
| setApiClient(createApiClient(BASE)); | ||
| document.cookie = ''; | ||
| }); |
…nvalidation + dedupe + DX polish ## Findings & fixes ### P2 — useHandshake() did not invalidate flat tools cache (Codex) After a successful handshake the per-server tools cache was invalidated but `keys.tools.all()` (the flat `useTools()` cache) was not. Operators visiting the Tools page after a handshake would see pre-handshake data for up to 30s `staleTime`. **Fix**: `useHandshake.onSuccess` now also invalidates `keys.tools.all()`. ### P2 — useEffect re-registered listeners on every render (Copilot) The effect depended on the whole `toast` object, which the provider returns fresh on every render. Result: every parent re-render re-registered all 6 document listeners (including `auth:expired`), risking listener-storm + duplicate fires during the cleanup race. **Fix**: destructure the stable `push` callback inside `Shell()` and depend on `[nav, push]` only. ### P2 — auth:expired toast was not actually deduplicated (Copilot) The comment claimed dedupe but every 401 pushed a fresh toast. A stale Sanctum cookie can fire dozens of 401s as TanStack Query retries queued requests in parallel. **Fix**: module-scoped `authExpiredToastShown` flag. First 401 shows the toast, subsequent ones are silently swallowed until the user reloads. ### P2 — queryClient.ts docblock pointed at a non-existent file (Copilot) Comment referenced `queries/testWrapper.tsx`; the actual test wrapper is at `tests/js/lib/queries/wrapper.tsx`. **Fix**: docblock path corrected. ### P2 — unused HostUser import in endpoints.ts (Copilot) **Fix**: removed. ### P2 — subscribeEvents() lacked test coverage (Copilot) The new EventSource wiring (with JSON parsing + fallback for environments where `EventSource === undefined`) had no test pinning the no-op contract. **Fix**: 2 new tests — verify (a) `subscribeEvents()` returns a function in jsdom (where EventSource is undefined); (b) the function doesn't throw on invocation. Pin the contract against future regressions that might throw instead of returning a no-op. ### P2 — document.cookie='' unreliable in jsdom (Copilot) `document.cookie = ''` is silently ignored by jsdom (it attempts to set a cookie with empty name + empty value). The previously-set XSRF-TOKEN cookie persisted into subsequent tests, making the suite order-dependent. **Fix**: `document.cookie = 'XSRF-TOKEN=; Max-Age=0; path=/'` — the explicit expiry actually clears the cookie. ## Test impact - Vitest: 64 → 66 tests (+2 new for subscribeEvents EventSource- missing path). - Typecheck: clean. - npm run build: unchanged bundle size (no source code growth on the bundle path). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@copilot review |
Reviewed. The review findings are already addressed in commit Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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
Wave W2 of the v1.1 cycle — the data-layer foundation that backs every read + mutation in v1.1. After this PR merges the SPA stays fully functional on fixture data; W3 swaps pages over to the live hooks page-by-page so the fixture-vs-real-data delta is reviewable in isolation.
What landed
Typed API surface mirroring OpenAPI 3.1 v1.5
resources/js/lib/api/types.ts— hand-written types for all 19 OpenAPI schemas (HostUser / HostTenant / HostApiKey / McpServer / Tool / AuditRow / AuditDetail / BreakerState / Resource / Prompt / AuditEvent / ConfirmTokenMint / ApiErrorPayload / ValidationErrorPayload + filters + envelopes). 1:1 with the wire.resources/js/lib/api/errors.ts— typed error hierarchy:ApiErrorbase +NetworkError/AuthExpiredError/FeatureDisabledError/ConfirmTokenError/ValidationError. TanStack Query'sonErrordistinguishes byinstanceof.resources/js/lib/api/client.ts— singleton axios instance with:baseURLresolved fromimport.meta.env.VITE_API_BASE→window.__MCP_PACK_ADMIN__.api_base→/api/admin/mcp-packdefault.withCredentials: true(Sanctum cookie auth).XSRF-TOKENcookie + echo asX-XSRF-TOKENheader on every non-GET (URL-decoded).AuthExpiredError+ globalauth:expiredCustomEvent; 403feature_disabled→FeatureDisabledError; 422 confirmation codes →ConfirmTokenError; 422 with Laravel validation shape →ValidationError; no-response →NetworkError.resources/js/lib/api/endpoints.ts— 22 typed endpoint helpers, every dynamic path segment viaencodeURIComponent.invokeTool/replayAudit/resetBreakerimplement the two-call confirm-token protocol — 202 throwsConfirmTokenErrorcarrying the minted token.TanStack Query hooks
resources/js/lib/queries/queryClient.ts— admin-tuned defaults (staleTime: 30s, no window-focus refetch, retry policy that short-circuits on auth / feature-disabled).resources/js/lib/queries/keys.ts— centralised query-key factory.resources/js/lib/queries/hooks.ts— 13 read hooks:useMe,useTenants,useApiKeys,useServers,useServer,useServerTools,useTools,useResources,useResource,usePrompts,usePrompt,useAudit,useAuditDetail,useBreakers.resources/js/lib/mutations/hooks.ts— 10 mutation hooks:useUpdatePreferences,useCreateApiKey,useRevokeApiKey(optimistic),useCreateServer,useUpdateServer,useDeleteServer,useHandshake,useInvokeTool,useReplayAudit,useResetBreaker.Shell wiring
resources/js/main.tsx—<QueryClientProvider>wraps<App />;<ReactQueryDevtools />gated onimport.meta.env.DEV.resources/js/App.tsx— listens for theauth:expiredCustomEvent and surfaces a toast withdata-testid="auth-expired-toast". NO read-path swap — every page still renders fixture data; W3 handles the swap.resources/js/lib/ui.tsx— toast root now passes throughdata-testid(R11).vite.config.ts—axios+@tanstack/react-query+@tanstack/react-query-devtoolsadded tooptimizeDeps.include.Tests (Vitest 7 → 64, +57)
tests/js/lib/api/client.test.ts— 11 specs covering XSRF echo + every error-mapping branch.tests/js/lib/api/endpoints.test.ts— 24 specs covering one happy-path per endpoint + confirm-token round-trips.tests/js/lib/queries/hooks.test.tsx— 9 specs exercising loading/success/cache.tests/js/lib/mutations/hooks.test.tsx— 9 specs covering create / update / handshake + the destructive flow.tests/js/setup.ts— wires MSW v2setupServerlifecycle.tests/js/lib/queries/wrapper.tsx— shared<QueryClientProvider>test factory.Dependencies
@tanstack/react-query^5axios^1@tanstack/react-query-devtools^5msw^2Bundle size
main-*.jsmain-*.cssDelta is within the projected envelope for TanStack Query + axios.
R-rules honoured
data-testid; newauth-expired-toasttestid.instanceof ApiError.encodeURIComponent.invokeTool/replayAudit/resetBreaker. 202 throwsConfirmTokenErrorcarrying the minted token; UI re-calls withconfirmTokenin the body.X-Tenant-Idfrom JS state; host middleware owns tenant resolution.resources/js/.Verification
npm run typecheck— cleannpm test— 64/64 passingnpm run build— succeeds (346 KB / 99 KB gz bundle)php vendor/phpunit/phpunit/phpunit— 8/8 passing (no PHP touched)Test plan
npm run devboots, every fixture-backed page renders identically to v1.0.1./api/admin/mcp-pack/me401 response, observe theauth-expired-toastflash.Next
W3 wires read-paths page-by-page (Dashboard → Servers → Tools → Audit → Resources/Prompts → Breakers → Settings). The fixture imports stay in place until each page is migrated and its E2E spec is updated to assert against live shape.
🤖 Generated with Claude Code