bert.hausmans/crewli
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(refactor): audit axios↔store coupling for decoupling work

Browse Source 
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>
This commit is contained in:
bert.hausmans
2026-05-04 22:06:20 +02:00
parent 831f36e618
commit 5eac201d88
1 changed files with 367 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

367
dev-docs/TECH-AXIOS-STORE-COUPLING-AUDIT.md Normal file
View File

@@ -0,0 +1,367 @@
# 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 1–2:** `axios` import + named-type imports.
- **Lines 8–16:** `apiClient` factory — `axios.create({ baseURL, withCredentials, headers, timeout: 30000 })`. Pure config, no store deps.
- **Lines 27–37 (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, 75–80, 82, 85, 88–95, 97, 99–101:** 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 2–4
involve `window.location.href` mutations that are awkward in
Vitest+happy-dom.
---
## A.4 — Consumer audit for `@/lib/axios`
```bash
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 Q1–Q4.
Branch: `refactor/axios-store-coupling`.
Phase A commit message:
`docs(refactor): audit axios↔store coupling for decoupling work`.