# Task 5 Review — Trigger GitHub Action via repository_dispatch

**Base:** 4b5b3b7 **Head:** ae5ec76  
**Reviewer:** code-reviewer agent  
**Date:** 2026-06-24

---

### Spec Compliance ✅

Every constraint from the global spec and brief is satisfied verbatim:

| Constraint | Status | Evidence |
|---|---|---|
| URL `https://api.github.com/repos/paoloalby/alleanza-dvr/dispatches` | ✅ | `dispatch.ts:9` — hardcoded via `REPO` constant |
| Method `POST` | ✅ | `dispatch.ts:10` |
| Header `Authorization: Bearer ${token}` | ✅ | `dispatch.ts:12` |
| Header `Accept: application/vnd.github+json` | ✅ | `dispatch.ts:13` |
| Header `Content-Type: application/json` | ✅ | `dispatch.ts:14` |
| Body `event_type: 'generate-dvr-pdf'` | ✅ | `dispatch.ts:17` |
| Body `client_payload.agenzia_id` (snake_case) | ✅ | `dispatch.ts:17` |
| `env` from `$env/dynamic/private` | ✅ | `dispatch.ts:1` |
| Throw if token missing | ✅ | `dispatch.ts:8` |
| Throw if `!res.ok` | ✅ | `dispatch.ts:19` |
| `Promise<void>` return type | ✅ | `dispatch.ts:6` |

---

### Strengths

- **Test verifica la forma della richiesta** (non solo che `fetch` sia stato chiamato): URL esatto, method, body parsato con `JSON.parse`, header `Authorization` — tutti ispezionati sugli argomenti reali passati al mock. `dispatch.test.ts:30-33`.
- **Mock `$env` dichiarato a module scope** (`vi.mock(...)` sopra il describe), garantendo che il mock sia in place prima dell'`import` dinamico nel test. Pattern corretto per SvelteKit.
- **`REPO` come costante modulo** (`dispatch.ts:3`) evita la stringa duplicata nell'URL.
- **Messaggio di errore utile** nel throw del token mancante (`dispatch.ts:8`) e nello status nel throw del `!res.ok` (`dispatch.ts:19`).
- **`User-Agent`** aggiunto — buona pratica per chiamate a API GitHub, non richiesto dalla spec ma non contraddice nulla.
- Codice conciso: 19 righe per il modulo, 18 per il test, nessun boilerplate superfluo.

---

### Issues

#### Critical
Nessuno.

#### Important
Nessuno.

#### Minor

**M1 — `dispatch.test.ts:23` — `beforeEach(vi.restoreAllMocks)` non copre `vi.stubGlobal`**

`vi.restoreAllMocks()` ripristina i mock creati con `vi.spyOn`, ma `vi.stubGlobal` richiede `vi.unstubAllGlobals()` per essere pulito tra un test e l'altro. Con un solo test nella suite questo non crea problemi pratici, ma se in futuro si aggiungono test (es. caso token mancante, caso `!res.ok`) il `fetch` mockato dal test precedente rimarrebbe in scope, portando a test order-dependent.

Soluzione: aggiungere `vi.unstubAllGlobals()` all'interno del `beforeEach` (o usare `afterEach`).

```ts
beforeEach(() => {
  vi.restoreAllMocks();
  vi.unstubAllGlobals();
});
```

**M2 — Nessun test per i branch di errore**

La spec richiede due path di errore (token mancante + `!res.ok`). Il test copre solo il happy path. Non è un blocco — la spec del brief mostra solo il test del payload giusto — ma i due `throw` restano non coperti da asserzione.

---

### Assessment

**Task quality: Approved**

L'implementazione è spec-compliant al 100%: URL, method, tutti e tre gli header richiesti, body con `client_payload.agenzia_id` snake_case, gestione errori token mancante e `!res.ok`. Il test verifica concretamente la forma della richiesta ispezionando gli argomenti del mock, non si limita a controllare che `fetch` sia stato chiamato. I due minor sopra citati (cleanup `stubGlobal` nel `beforeEach`, assenza test branch errore) non compromettono la correttezza della funzionalità né la affidabilità della suite con la sua cardinalità attuale.