Files
crewli/dev-docs/TECH-AXIOS-STORE-COUPLING-AUDIT.md
bert.hausmans 5eac201d88 docs(refactor): audit axios↔store coupling for decoupling work
Phase A of TECH-AXIOS-STORE-COUPLING. Read-only inventory of the
four lib/axios.ts → stores/ touchpoints (lines 3, 5, 63, 75 carry
per-line boundary disables), plugin load-ordering analysis, test
coverage matrix, consumer audit (30 importers, all using the
`apiClient` named export), and Vite mixed-import warning
confirmation.

Surfaces four open questions for Phase B sign-off:
  Q1 callback-injection vs event-bus  → recommends callback-injection
  Q2 location of `Deps` type          → recommends inside lib/axios.ts
  Q3 test scope for this session      → recommends defer to backlog
  Q4 plugin filename                  → recommends 3.axios-bindings.ts

No code changed. No BACKLOG.md edit. Awaiting Bert's Phase B
sign-off before implementing Phase C.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-05-04 22:06:20 +02:00

18 KiB
Raw Blame History

TECH-AXIOS-STORE-COUPLING — Phase A audit

Date: 2026-05-04 Branch: refactor/axios-store-coupling Scope: read-only audit. No code changed in this phase. Goal: make apps/app/src/lib/axios.ts a pure HTTP module by routing its four stores/ references through a callback seam wired up from a new plugins/ plugin. Phase A inventories what exists today; Phase B asks Bert to resolve four open questions; Phase C implements.


A.1 — Touchpoint inventory in apps/app/src/lib/axios.ts

The file is 109 lines on main at 831f36e. The four boundary-rule disables are at lines 3, 5, 63, 75 (single-line drift versus the BACKLOG note of "regels 3-4, 61, 72" — caused by minor formatting changes since 1c, semantically identical).

Touchpoint 1 — useOrganisationStore (static)

Field Value
Disable line 3
Import line 4 (import { useOrganisationStore } from '@/stores/useOrganisationStore')
Invocation line 20 (const orgStore = useOrganisationStore())
Interceptor request
Access read state — orgStore.activeOrganisationId (string | null)
Semantic purpose inject the active organisation ULID into the X-Organisation-Id request header so the backend can scope the query to the active tenant
Shape needed in Deps getActiveOrgId: () => string | null

Touchpoint 2 — useNotificationStore (static)

Field Value
Disable line 5
Import line 6 (import { useNotificationStore } from '@/stores/useNotificationStore')
Invocation line 59 (const notificationStore = useNotificationStore())
Interceptor response (error path)
Access calls .show(message, level) action
Semantic purpose toast a message via v-snackbar for 403 / 404 / 422 / 503 / 5xx / network errors
Shape needed in Deps notify: (message: string, level: 'error' | 'warning') => void (the only two levels used in the current code)

Touchpoint 3 — useImpersonationStore (dynamic)

Field Value
Disable line 63
Import line 64 (const { useImpersonationStore } = await import('@/stores/useImpersonationStore'))
Invocation line 65 (const impersonationStore = useImpersonationStore())
Interceptor response (error path), under status === 403 && error.response?.data?.impersonation_ended
Access calls .clearState() then window.location.href = '/platform'
Semantic purpose when the backend revokes an impersonation session mid-flight (403 with impersonation_ended), drop the local impersonation state and full-page-redirect back to /platform so the original admin session restores
Shape needed in Deps onImpersonationRevoked: () => void (the redirect is the side-effect of clearing state; whether the callback owns the redirect or just the clear is a Phase B Q2-adjacent question — see notes)

Touchpoint 4 — useAuthStore (dynamic)

Field Value
Disable line 75
Import line 76 (const { useAuthStore } = await import('@/stores/useAuthStore'))
Invocation line 77 (const authStore = useAuthStore())
Interceptor response (error path), under status === 401
Access reads .isInitialized; if true, calls .handleUnauthorized()
Semantic purpose on 401, if the auth store has finished its initial /auth/me probe, clear local auth state and redirect to /login. The isInitialized guard exists because the initial /auth/me call itself returns 401 for unauthenticated visitors and that is not a logout-trigger
Shape needed in Deps onAuthFail: () => void — the callback closure should encapsulate both the isInitialized check and the handleUnauthorized() call (see Phase B Q2). Alternatively, two callbacks (isAuthInitialized: () => boolean + onAuthFail: () => void) but that's a leakier seam.

Non-store behavior in axios.ts (preserve, do not refactor)

  • Lines 12: axios import + named-type imports.
  • Lines 816: apiClient factory — axios.create({ baseURL, withCredentials, headers, timeout: 30000 }). Pure config, no store deps.
  • Lines 2737 (request interceptor): reads sessionStorage.getItem('crewli_impersonation'), parses JSON, sets X-Impersonate-User header. This is not a boundary violation — it touches sessionStorage directly, not a store. It is conceptually parallel to touchpoint 1 but already store-free, so it stays as-is in Phase C unless Bert prefers a parallel callback (getImpersonationTargetId: () => string | null) for symmetry. Recommendation: leave alone in Phase C — the seam should only encapsulate the store-coupled paths. Refactoring this is scope creep.
  • Line 40, 50, 56: console.log/error under import.meta.env.DEV — pure logging.
  • Lines 73, 7580, 82, 85, 8895, 97, 99101: the rest of the response error mapping — same status-code branching, all toast-via-notify once touchpoint 2 is callback-ified.
  • Line 108: export { apiClient } — single named export. Critical: 30 importers across apps/app/src/ consume import { apiClient } from '@/lib/axios'. The named export must keep working post-refactor. Phase C only adds a new registerInterceptors export; nothing is renamed or removed.

Singleton vs. factory

apiClient is a top-level singleton (created at module-eval time). The two interceptor registrations execute as a side-effect of the first import. Phase C must split this: the singleton stays, but interceptor registration moves to an explicit registerInterceptors(client, deps) call invoked from the new bindings plugin. Until that call runs, the client is interceptor-less — fine, because the bindings plugin runs in the plugin-init phase before any composable or component issues an HTTP call (Pinia + the bindings plugin both initialize during registerPlugins(app) in main.ts, before app.mount).


A.2 — Plugins ordering and proposed filename

How plugins load

main.ts calls registerPlugins(app) from @core/utils/plugins.ts. The helper is a Vuexy convention that uses import.meta.glob(['../../plugins/*.{ts,js}', '../../plugins/*/index.{ts,js}'], { eager: true }), collects the keys, alphabetically sorts the path strings, and calls each module's default(app) in that order. There is no manual registration in main.ts — drop a file in src/plugins/ and it gets picked up.

Current alphabetical load order

Sort key (path string) File Role Pinia ready after this?
../../plugins/1.router/index.ts 1.router/index.ts app.use(router) + guards no
../../plugins/2.pinia.ts 2.pinia.ts createPinia() + app.use(store) yes (this is where it happens)
../../plugins/iconify/index.ts iconify/index.ts iconify registration yes
../../plugins/layouts.ts layouts.ts createLayouts() yes
../../plugins/vuetify/index.ts vuetify/index.ts Vuetify registration yes
../../plugins/webfontloader.ts webfontloader.ts webfont preload yes

Ordering invariant: anything that needs Pinia stores must sort after 2.pinia.ts. The numeric prefix is the established convention for explicit ordering (router=1, pinia=2). Anything without a numeric prefix sorts after the numeric ones because digits sort before letters in standard lexicographic comparison.

Proposed filename

apps/app/src/plugins/3.axios-bindings.ts

Rationale:

  • 3. makes the post-Pinia ordering explicit and human-readable alongside the existing 1.router / 2.pinia numeric prefixes. Future readers reading top-to-bottom in a directory listing see the load sequence at a glance.
  • A single .ts file (not a directory + index.ts) — the wiring is ~30 lines of closure plumbing, no internal modules to split.
  • Name axios-bindings matches the BACKLOG entry's wording. "Bindings" reads as "bind axios interceptors to store callbacks" — the file's job is exactly that wiring.

If Bert prefers a non-numeric name (e.g. just axios-bindings.ts, sorting after the numeric files), that works too — Pinia is still already up. Numeric-prefix is recommended for parity with the existing router/pinia ordering convention. Open as Phase B Q4.


A.3 — Test coverage matrix for the four acceptance scenarios

apps/app runs Vitest against 49 tests across 10 files (sanity smoke in tests/, plus __tests__/ colocated under layouts/, composables/api/, and components/form-failures/).

Acceptance scenario Existing test? Notes
1. X-Organisation-Id header injection No No test exercises the request interceptor.
2. 401 → handleUnauthorized() flow No useAuthStore has no colocated tests; no interceptor test.
3. 403 + impersonation_ended revocation No useImpersonationStore has no colocated tests; no interceptor test.
4. 4xx/5xx error toast via useNotificationStore No useNotificationStore has no colocated tests; no interceptor test.

How @/lib/axios is used in existing tests

All 6 existing axios-touching test files use vi.mock('@/lib/axios', () => ({ apiClient: { ... } })) to swap the entire module out. None of them load real interceptors. List:

  • src/composables/api/__tests__/useFormFailures.spec.ts
  • src/components/form-failures/__tests__/FormFailuresTable.spec.ts
  • src/components/form-failures/__tests__/DismissFailureDialog.spec.ts
  • src/components/form-failures/__tests__/FormFailureDetail.spec.ts
  • src/components/form-failures/__tests__/RetryFailureDialog.spec.ts
  • src/components/form-failures/__tests__/ResolveFailureDialog.spec.ts

These tests will keep passing post-refactor regardless of how registerInterceptors looks — they replace the module wholesale.

Stores tested directly?

No — apps/app/src/stores/__tests__/ does not exist. None of the four stores referenced from axios.ts has a unit test. The Vitest harness (tests/sanity.spec.ts) was just landed in WS-6 sessie 3b (TECH-APP-VITEST closure); the colocated __tests__/ folders only cover layouts, form-failures components, and useFormFailures.

Recommendation for Phase B Q3

Default to option C from the brief: add as follow-up backlog entry TECH-AXIOS-INTERCEPTOR-TESTS, do not add tests in this session.

Reasoning:

  • All 4 acceptance scenarios are untested today, so the refactor is not regressing existing coverage.
  • Building an interceptor-test fixture that loads the real apiClient, registers registerInterceptors with mocked deps, and asserts on axios-mock-adapter (or nock-style) intercepted requests is genuinely >2h of infrastructure work in a project that has only had Vitest for a few sessies. The brief's default scope rule applies.
  • The 6 module-mocked tests give us a regression net for "the apiClient named export still works": if any .get/.post chain breaks, those component tests fall over. Combined with manual smoke-testing the four flows in dev, that's adequate for this session.

If Bert overrules this and wants tests in-session, the smallest viable path is option B targeted on just touchpoint 1 (X-Organisation-Id header injection) — it's the cheapest to test (request-time, no mocked window navigation) and the most boring to verify by hand, making it the highest-value test-per-effort. Touchpoints 24 involve window.location.href mutations that are awkward in Vitest+happy-dom.


A.4 — Consumer audit for @/lib/axios

grep -rln "from '@/lib/axios'" apps/app/src/ | wc -l    # 30
grep -rln "vi.mock('@/lib/axios'" apps/app/src/ | wc -l  # 6
grep -rln "import('@/lib/axios')" apps/app/src/ | wc -l  # 1 (AccountTab.vue, dynamic)

Group 1 — composables/api (22 files)

All 22 files in src/composables/api/use*.ts import { apiClient } and use .get / .post / .patch / .put / .delete. Vanilla axios usage.

Group 2 — stores (2 files)

src/stores/useAuthStore.ts and src/stores/useImpersonationStore.ts both import { apiClient }. (Other stores don't talk to axios.) Important: these are the two stores that are ALSO referenced from axios.ts via dynamic import — that's the cycle the Vite warnings flag. Removing those dynamic imports from axios.ts resolves the cycle automatically.

Group 3 — components / pages (6 files)

  • components/platform/ImpersonateDialog.vue
  • components/account-settings/AccountTab.vue (dynamic import)
  • components/common/ImageUploadField.vue
  • pages/login.vue
  • pages/forgot-password.vue
  • pages/reset-password.vue
  • pages/verify-email-change.vue

These violate the project rule "never import axios directly in a component (use a composable)" — that's a separate cleanup not in this session's scope. Listing for completeness.

Group 4 — tests that mock the module (6 files)

Listed in A.3. They use vi.mock and replace apiClient with a stub; they don't care about the interceptor wiring.

Breaking-change surface

Zero. Every consumer reads { apiClient } from @/lib/axios. Phase C only:

  • Adds a new export registerInterceptors.
  • Adds a new export Deps type (or hosts it in @/types/ per Phase B Q2).

Existing imports keep working unchanged. No fanout.


A.5 — Vite mixed-import warning confirmation

Ran pnpm build from apps/app/ on main@831f36e. Both warnings reproduce as the BACKLOG describes. The relevant excerpt:

[plugin vite:reporter]
(!) /…/apps/app/src/stores/useAuthStore.ts is dynamically imported by
    /…/apps/app/src/lib/axios.ts but also statically imported by
    /…/apps/app/src/App.vue, /…/AccountTab.vue, /…/SecurityTab.vue,
    /…/EventMetricCards.vue, /…/EventTabsNav.vue,
    /…/OrganisationSwitcher.vue, … (≈30 callers total),
    dynamic import will not move module into another chunk.

[plugin vite:reporter]
(!) /…/apps/app/src/stores/useImpersonationStore.ts is dynamically
    imported by /…/apps/app/src/lib/axios.ts but also statically
    imported by /…/apps/app/src/App.vue, /…/ImpersonateDialog.vue,
    /…/ImpersonationBanner.vue, /…/DefaultLayoutWithVerticalNav.vue,
    dynamic import will not move module into another chunk.

Build still succeeds (10.96s, 49 tests pass). The warnings are diagnostic only, but they are an explicit acceptance criterion for Phase C: removing the two await import('@/stores/...') calls in axios.ts should drop both warnings to zero. If either warning persists post-refactor, it means another file in the codebase is also doing await import('@/stores/useAuthStore') or '.../useImpersonationStore'. Quick grep -rn "await import.*useAuthStore\|await import.*useImpersonationStore" apps/app/src/ during Phase C will confirm; expectation is that axios.ts is the only offender.


A.6 — Open questions for Bert (Phase B sign-off)

Q1. Approach 1 (callback-injection) or Approach 2 (event-bus)?

Recommendation: Approach 1 (callback-injection).

The four touchpoints map cleanly to four callbacks (or three + one read), all single-subscriber, all synchronous in their domain (the heavy work is the callee's, not the seam's). No case where multiple subscribers would want to react to the same event. No case where the subscriber needs to register/unregister dynamically at runtime. An event bus would add a second indirection, a stringly-typed event name surface, and break TypeScript's ability to enforce that every event has exactly the deps it needs.

Approach 2 would only beat Approach 1 if we wanted the bindings plugin to subscribe to events emitted from places other than axios.ts. We don't.

Q2. Where should the typed Deps interface live?

Recommendation: inside lib/axios.ts, exported.

Rationale:

  • The Deps shape is the contract of lib/axios.ts. It belongs to the module that consumes it. The bindings plugin imports it to know what to provide; that's a normal import type flow and doesn't break the boundaries matrix (plugins → lib is allowed).
  • Hoisting it to src/types/ would be more "architecturally pure" but adds a hop with no payoff: nothing else in the codebase consumes this contract, and types/ is for shared domain/value-object types, not for module-internal interfaces.
  • The architectural test that matters is "does lib/axios.ts import from @/stores/...?" — that's what the boundaries rule enforces. Where the type lives doesn't affect that.

If Bert prefers the types/-hoist for consistency, no objection; the diff cost is one extra file. State preference in Phase B.

Q3. Test scope for this session.

Recommendation: option C — add follow-up backlog entry TECH-AXIOS-INTERCEPTOR-TESTS, no new tests in this session.

Reasoning in A.3. All four scenarios are untested today; the refactor preserves behavior 1:1; the 6 existing module-mock tests are an adequate "named-export still works" net; building real interceptor-fixture infrastructure is >2h.

If Bert prefers option B (add tests for touchpoint 1 only — the X-Organisation-Id header injection — as the cheapest meaningful test), that's a reasonable middle ground; estimate ~1h.

Q4. Confirm plugin filename and ordering.

Recommendation: apps/app/src/plugins/3.axios-bindings.ts.

Numeric prefix 3. for parity with 1.router/ and 2.pinia.ts, making the post-Pinia load order explicit. Alternative: drop the prefix (axios-bindings.ts) — it would still sort after the numeric files because digits precede letters lexicographically. Numeric is preferred for readability.


A.7 — Deliverable

This file is dev-docs/TECH-AXIOS-STORE-COUPLING-AUDIT.md. Phase A ends here. No code edits, no BACKLOG.md change, no plugin file, no axios.ts change yet. Awaiting Phase B sign-off on Q1Q4.

Branch: refactor/axios-store-coupling.

Phase A commit message: docs(refactor): audit axios↔store coupling for decoupling work.