From 21265a80615ed63e0e46d70bfd0be4c61635a5be Mon Sep 17 00:00:00 2001 From: Martins Aloba Date: Sun, 17 May 2026 07:18:36 -0500 Subject: [PATCH 1/3] ci(15-3): wire Deno _shared tests into CI + drift detector (5 cases) --- .github/workflows/ci.yml | 21 ++++++ .../sprint-status.yaml | 4 +- .../ci-deno-step-source-drift.test.ts | 75 +++++++++++++++++++ 3 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 src/lib/__tests__/ci-deno-step-source-drift.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f331590..b1eb899 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,6 +36,27 @@ jobs: - name: Tests run: npm test -- --no-coverage + - name: Setup Deno + # Story 15-3: install Deno for Edge Function _shared utility tests. + # Pinned to the latest v1 (Deno 2.0 has breaking changes; defer 2.0 + # migration to a follow-up story). The action caches the install for + # faster subsequent runs. + uses: denoland/setup-deno@v2 + with: + deno-version: v1.x + + - name: Deno tests (Edge Function _shared utilities) + # Story 15-3: wire Story 11-3 fetch-with-timeout + Story 12-11 + # parse-upstream-error Deno tests into CI. Pre-15-3 these were + # manual-run only. Now gated on PR merge. The `--allow-net=127.0.0.1` + # permission is required by the fetch-with-timeout happy-path test + # which spins up a local HTTP server. NO `--allow-all` — keep the + # permission surface minimal. + # pgTAP migrations (supabase/migrations/__tests__/*.sql) NOT wired + # here — requires a Postgres service container. Deferred to + # `15-3-followup-pgtap-ci-wiring`. + run: deno test --allow-net=127.0.0.1 supabase/functions/_shared/__tests__/ + - name: Dependency vulnerability gate # Story 12-10: prevent regression of `npm audit reports 0 high # vulnerabilities` (Epic 12 AC). Calibrated to `--audit-level=high` diff --git a/_bmad-output/implementation-artifacts/sprint-status.yaml b/_bmad-output/implementation-artifacts/sprint-status.yaml index 5405e25..71404e0 100644 --- a/_bmad-output/implementation-artifacts/sprint-status.yaml +++ b/_bmad-output/implementation-artifacts/sprint-status.yaml @@ -207,8 +207,8 @@ development_status: # Epic 15: Test Coverage & QA Infrastructure (P1) epic-15: in-progress # 2026-05-16: auto-flipped backlog → in-progress when 15-1 story file was created. Epic 14 retro AIs surfaced at kickoff per Epic 13 AI #4 accountability gate; "Proceed + acknowledge" mode selected (AI #8 absorbed by 15.3 scope; others orthogonal docs/workflow). 15-1-lib-unit-tests: done # PR #111. R1 patches: HIGH × 2 (useFakeTimers time-flake fix; dictation empty-original semantic-trap TODO + 15-1-followup filed) + MED × 3 (Case 17 distinctive-marker assertion; Case 14 ref-inequality; new Case 14a typed-preserves-case). +1 net R1 (2158 → 2159). All 5 gates green. Original: 2026-05-16: Story 15-1 implementation complete. 4 NEW test files (srs.test.ts 14 cases + pronunciation.test.ts 10 cases + cache.test.ts 20 cases + use-dictation-compare.test.ts 15 cases) = +59 net Jest cases (2099 → 2158; squarely within spec target +50-65). 0 source-module modifications — test-only story. All 5 quality gates green. CLAUDE.md paragraph added per Epic 14 retro AI #5. Original: Story 15-1 spec file created. FIRST Epic 15 story. Pure-function lib unit tests for 4 modules where direct tests don't yet exist: srs.ts (SM-2 algorithm full coverage), pronunciation.ts identifyWeakSounds (pure aggregator), cache.ts core API (getCache/setCache/invalidateCache/cacheWithFallback happy + TTL boundary), use-dictation.ts compareSentences (pure word-comparison helper). Coverage inventory completed: activity.ts already fully tested (Story 9-2 + 12-3); memory.ts + error-tracker.ts have substantial partial coverage (sanitize via 9-4 + dedup via 11-6); pronunciation.ts has history-cap test (12-12); the GAP-only scope targets the 4 untested-or-partially-tested pure-function surfaces. Async/DB-touching paths (assessPronunciation Edge wrapper, memory.ts extractFacts/persistMemories/retrieveMemories, error-tracker.ts non-dedup paths) DEFERRED to 15-1-followups or 15-2 hook-integration scope per Story 14-4 R1-22-patch lesson (broad enforcement-rule changes interact with every surface; test-writing stories at analogous risk). 3 operator-decision items Q1-Q3 all resolved per Recommended (defer Edge wrapper / memory async / error-tracker non-dedup). 0 source-module modifications — test-only story. Spec target: +50-65 net Jest cases (2099 → 2149-2164). Status: ready-for-dev. Awaiting /bmad-dev-story. - 15-2-hook-integration-tests: backlog - 15-3-edge-function-deno-tests: backlog + 15-2-hook-integration-tests: done # 2026-05-17: PR #112 (chained autopilot). NEW use-pronunciation.test.tsx (13 cases) — Story 12-12 FIFO + Story 15-1 identifyWeakSounds integration verified. +13 net Jest cases (2159 → 2172). 0 source-module modifications. All 5 gates green. 15-2-followup-existing-hook-flow-integration filed for use-auth/use-exercise/use-realtime-voice extensions. + 15-3-edge-function-deno-tests: in-progress # 2026-05-17: spec written. TIGHT scope — wire existing 2 Deno _shared tests into CI; pgTAP + new Edge Function tests deferred to 15-3-followups. 15-4-golden-flow-e2e: backlog 15-5-ai-schema-regression-tests: backlog 15-6-ci-coverage-gating: backlog diff --git a/src/lib/__tests__/ci-deno-step-source-drift.test.ts b/src/lib/__tests__/ci-deno-step-source-drift.test.ts new file mode 100644 index 0000000..349da92 --- /dev/null +++ b/src/lib/__tests__/ci-deno-step-source-drift.test.ts @@ -0,0 +1,75 @@ +/** + * Story 15-3 — source-drift detector for the Deno test CI step. + * + * Pins the new `Deno tests (Edge Function _shared utilities)` step in + * `.github/workflows/ci.yml` against silent regression: + * - Setup step uses the official `denoland/setup-deno@v2` action + * - Step name + `deno test` invocation + canonical permission flag + * - NEGATIVE: no over-permissive `--allow-all` + * - NEGATIVE: no silent-disable patterns (`continue-on-error: true` / + * blanket `if:` keys — Story 12-10 R1-H2 lesson) + * - Ordering: appears AFTER the `Tests` step so a failing `npm test` + * short-circuits before the Deno install (build-time efficiency) + */ + +import * as fs from "fs"; +import * as path from "path"; + +const REPO_ROOT = path.resolve(__dirname, "../../.."); + +function readCiYml(): string { + return fs.readFileSync(path.join(REPO_ROOT, ".github/workflows/ci.yml"), "utf-8"); +} + +describe("Story 15-3 — Deno test CI step source drift", () => { + const ci = readCiYml(); + + it("Case 1: Setup Deno step uses denoland/setup-deno@v2 action with pinned v1.x version", () => { + expect(ci).toMatch(/- name:\s*Setup Deno\b/); + expect(ci).toMatch(/uses:\s*denoland\/setup-deno@v2\b/); + expect(ci).toMatch(/deno-version:\s*v1\.x\b/); + }); + + it("Case 2: Deno test step name + canonical run command pinned", () => { + expect(ci).toMatch(/- name:\s*Deno tests \(Edge Function _shared utilities\)/); + // Canonical run command: deno test + scoped permission + scoped dir + expect(ci).toMatch( + /run:\s*deno test --allow-net=127\.0\.0\.1 supabase\/functions\/_shared\/__tests__\// + ); + }); + + it("Case 3: NEGATIVE guard — no --allow-all (over-permissive); permission surface stays minimal", () => { + // Find the Deno test run line specifically (avoid false positives if + // another step happens to mention --allow-all in a comment). + const runLineMatch = ci.match( + /run:\s*deno test [^\n]+supabase\/functions\/_shared\/__tests__\/[^\n]*/ + ); + expect(runLineMatch).not.toBeNull(); + expect(runLineMatch![0]).not.toContain("--allow-all"); + }); + + it("Case 4: NEGATIVE guard — Deno test step does NOT carry `continue-on-error: true` or unrestricted `if:` (Story 12-10 R1-H2 silent-disable defense)", () => { + // Extract the block from "name: Deno tests" to the next "- name:" or EOF. + const denoBlockMatch = ci.match( + /- name:\s*Deno tests \(Edge Function _shared utilities\)[\s\S]*?(?=\n\s*- name:|\n\s*$)/ + ); + expect(denoBlockMatch).not.toBeNull(); + const block = denoBlockMatch![0]; + expect(block).not.toMatch(/continue-on-error:\s*true/); + // No top-level `if:` key that would conditionally skip the step. + // (Note: `if:` inside the `run:` shell script is fine — only flag the + // step-level `if:` key.) + const stepLevelIf = block.match(/^\s{6}if:\s/m); // 6-space indent matches step-level keys + expect(stepLevelIf).toBeNull(); + }); + + it("Case 5: Ordering — Deno test step appears AFTER the Tests step (so npm test failures short-circuit before Deno install)", () => { + const testsIdx = ci.search(/- name:\s*Tests\b/); + const denoIdx = ci.search(/- name:\s*Deno tests \(Edge Function _shared utilities\)/); + const setupDenoIdx = ci.search(/- name:\s*Setup Deno\b/); + expect(testsIdx).toBeGreaterThan(0); + expect(denoIdx).toBeGreaterThan(testsIdx); + expect(setupDenoIdx).toBeGreaterThan(testsIdx); + expect(setupDenoIdx).toBeLessThan(denoIdx); // setup comes before test invocation + }); +}); From 25c36688215d3a72c09bfb5d6c389aa9a97b565c Mon Sep 17 00:00:00 2001 From: Martins Aloba Date: Sun, 17 May 2026 07:18:41 -0500 Subject: [PATCH 2/3] chore(15-3): Story 15-3 spec + CLAUDE.md + sprint-status done --- CLAUDE.md | 2 + .../15-3-edge-function-deno-tests.md | 82 +++++++++++++++++++ .../sprint-status.yaml | 4 +- 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 _bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md diff --git a/CLAUDE.md b/CLAUDE.md index 2bc4428..de0f5d7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -201,6 +201,8 @@ A reusable pattern for capping in-memory data structures + UI work-budgets. Two **Lib unit tests — `srs.ts` SM-2 algorithm + `pronunciation.ts` `identifyWeakSounds` + `cache.ts` core API + `use-dictation.ts` `compareSentences`:** post-Epic-15.1, the FIRST Epic 15 (Test Coverage & QA Infrastructure) story closes the largest pure-function-test gap surfaced by the Story 12-10 audit refresh footnote. Per the coverage inventory at story-creation time, `activity.ts` was already fully tested (Story 9-2 + 12-3); `memory.ts` + `error-tracker.ts` had substantial partial coverage (sanitize via Story 9-4 prompt-injection.test.ts + dedup via Story 11-6 error-tracker-dedupe.test.ts + e2e clustering via 11-6); `pronunciation.ts` had history-cap coverage (Story 12-12) but NOT `identifyWeakSounds`. Story 15-1 lands **GAP-only scope** across **4 pure-function surfaces** (no source-module modifications — test-only): **(1) NEW [`src/lib/__tests__/srs.test.ts`](src/lib/__tests__/srs.test.ts)** (14 cases) pins the full SM-2 algorithm contract — 3 incorrect-response cases (quality 0/1/2 ease-factor deltas −0.80/−0.54/−0.32 + reset to `repetitions=0`, `intervalDays=1`); 3 correct-response cases (quality 3/4/5 ease-factor deltas −0.14/0/+0.10 + interval progression 1 → 6 → round(prev × ef)); 3 ease-factor 1.3-floor clamping cases (boundary + already-at-floor + just-above-floor); 3-consecutive-correct chain verification; 365-day interval cap; midnight-snap `nextReview` math via Date.now() before/after delta check; no-mutation invariant via input-snapshot equality + determinism via twice-call equivalence. **(2) NEW [`src/lib/__tests__/pronunciation.test.ts`](src/lib/__tests__/pronunciation.test.ts)** (10 cases) pins `identifyWeakSounds` contract — empty input + no-weak happy path + below-threshold-but-count<3 NOT flagged + single-phoneme-aggregated-across-results + same-phoneme-aggregated-across-words-in-one-result + threshold boundaries (`avgScore < 70` strict + `count >= 3` boundary) + multi-phoneme ranking ASCENDING by avgScore (worst first) + no-mutation deep-equal. **(3) NEW [`src/lib/__tests__/cache.test.ts`](src/lib/__tests__/cache.test.ts)** (20 cases) covers the AsyncStorage path (existing `cache-flush.test.ts` covers write-queue idempotency; existing `cache-secure-routing.test.ts` covers the Story 12-7 SecureStore fork) — `getCache` empty/fresh/TTL-fresh-boundary/TTL-expired-boundary/corrupted-JSON/throw paths each with `captureError` routing pinned; `setCache` envelope shape `{data, timestamp, ttlMs}` + DEFAULT_TTL_MS (1 hour) fallback + namespacing `@companion_cache::` (different userIds never collide); `setCache` → `getCache` round-trip with nested object structure; `invalidateCache` happy + error paths; `cacheWithFallback` 4-path matrix (fetcher-succeeds-writes-cache, fetcher-throws-stale-cache-hit-returns-fromCache:true, fetcher-throws-no-cache-rethrows-original, fetcher-throws-cache-also-throws-captures-fallback-read-error-rethrows-original); constant pins for `CACHE_KEYS` (5 keys) + `CACHE_TTL` (6 durations) + `SECURE_CACHE_KEYS` size===1 with PROFILE in / SKILLS+VOCABULARY out. **(4) NEW [`src/hooks/__tests__/use-dictation-compare.test.ts`](src/hooks/__tests__/use-dictation-compare.test.ts)** (15 cases) pins the pure word-by-word `compareSentences` helper from [`src/hooks/use-dictation.ts:88`](src/hooks/use-dictation.ts#L88) (despite living in a hook file, the function is pure — no React, no async, no side effects, so it fits 15-1 scope). Test mocks `@/src/hooks/use-audio-player` (transitive `expo-audio` native crash in Jest) + `@/src/lib/openai` (transitive supabase chain) at module load to keep the import boundary clean. Cases cover: perfect-match → 100% / one-wrong-word with `typed` populated → round(2/3 × 100)=67% / missing-word semantics (user shorter → past-user-length positions are "missing", overlap positions compare normally) / extra-word (user longer → extra tokens IGNORED; only original-length iterated) / empty-userInput → all "missing" / empty-original → empty wordResults + accuracy=0 + isFullyCorrect=true vacuously / both-empty same / case-insensitive ("Bonjour" vs "bonjour") with display preserving original capitalization / accent-insensitive ("café" vs "cafe") with display preserving accent / trailing punctuation stripped / internal comma stripped / multi-space collapses / leading+trailing whitespace trimmed / apostrophe stripped ("l'eau" matches "leau") / fresh wordResults array per call (no shared module-level cache). **3 operator decisions resolved per Recommended** (Q1 defer `assessPronunciation` Edge wrapper to 15-1-followup; Q2 defer `memory.ts` async/DB-touching paths to 15-2; Q3 defer `error-tracker.ts` non-dedup paths to 15-2) — keeps 15-1 pure-function-only per the Epic 14 retro lesson that test-writing stories are at scope-drift risk. **Explicitly out of scope** (filed for follow-up if motivated): `memory.ts` `extractFacts`/`persistMemories`/`retrieveMemories`; `error-tracker.ts` `getTopErrors`/`extractErrorsFromCorrections`/`persistErrorPatterns`/`getRecentResolvedError`; `pronunciation.ts` `assessPronunciation` Edge Function wrapper; `cache.ts` `enqueueWrite`/`flushWriteQueue` (already in `cache-flush.test.ts`)/`clearUserCache`/`clearAllCache` multi-key sweeps; `use-dictation.ts` `analyzeErrorPatterns`. **Cross-story invariants preserved by construction:** zero source-module modifications (Story 15-1 is test-only); Story 9-3 Sentry allowlist zero-diff (cache tests verify EXISTING `cache-get`/`cache-set`/`cache-invalidate`/`cache-fallback-read` feature tags fire; no new tags); Story 12-7 SecureStore fork unchanged (cache.test.ts uses non-secure key `"skills"` to exercise AsyncStorage path); all Epic 14 invariants orthogonal. **+59 net Jest cases** (2099 → 2158; spec target +50-65 — squarely in range). 4 new test files; 0 modified source files. All 5 design-system gates green (type-check 0 errors / lint 0 warnings / prettier clean / check:tokens clean / jest 113 suites / 2158 tests). Verified 2026-05-16, story 15-1. +**Edge Function Deno tests wired into CI:** post-Epic-15.3 partially closes Epic 12 AI #7 + Epic 13 AI #8 (pgTAP CI wiring follow-through — Deno portion shipped; pgTAP portion deferred to `15-3-followup-pgtap-ci-wiring`). Pre-15-3 the 2 Deno test files (Story 11-3 `fetch-with-timeout_test.ts` + Story 12-11 `parse-upstream-error_test.ts`) were manual-run only; regressions could silently survive PR merges. Story 15-3 wires both into [`.github/workflows/ci.yml`](.github/workflows/ci.yml) via new `Setup Deno` step (`denoland/setup-deno@v2`, pinned `deno-version: v1.x`) + `Deno tests (Edge Function _shared utilities)` step running `deno test --allow-net=127.0.0.1 supabase/functions/_shared/__tests__/`. Permission surface minimal — NO `--allow-all`. Ordering: Deno steps AFTER `Tests` so npm-test failures short-circuit before Deno install. NEW [`src/lib/__tests__/ci-deno-step-source-drift.test.ts`](src/lib/__tests__/ci-deno-step-source-drift.test.ts) (5 cases) pins via Story 12-2 P12 + Story 12-10 R1-H2 patterns. TIGHT scope — pgTAP (5 SQL files) DEFERRED to `15-3-followup-pgtap-ci-wiring`; NEW Edge Function tests DEFERRED to `15-3-followup-edge-function-coverage`. 3 operator decisions resolved per Recommended. +5 net Jest cases (drift only; Deno tests run in CI). All 5 design-system gates green. Verified 2026-05-17, story 15-3. + ### Routing (`app/`) Expo Router file-based routing with three route groups: diff --git a/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md b/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md new file mode 100644 index 0000000..c36460a --- /dev/null +++ b/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md @@ -0,0 +1,82 @@ +# Story 15.3: Edge Function Deno tests in CI — wire existing `_shared/__tests__/*_test.ts` files into the GitHub Actions workflow + +Status: review + +## Story + +As **a developer**, I want **the existing `supabase/functions/_shared/__tests__/*_test.ts` Deno test files (currently manual-run only) wired into the GitHub Actions CI workflow** so that **a regression in `fetchWithTimeout` (Story 11-3) or `parseUpstreamError` (Story 12-11) silently survives a PR merge no longer**. + +## Background + +[`shippable-roadmap.md`](_bmad-output/planning-artifacts/shippable-roadmap.md) line 295 — Epic 15.3 deliverable: "Edge Function Deno tests — auth gate, rate limit, model allowlist, account-delete idempotency." + +**Carries Epic 12 AI #7 + Epic 13 AI #8** (pgTAP CI wiring) — surfaced at Epic 15 kickoff via the accountability gate. + +### Coverage inventory + +Existing manual-run tests: +- `supabase/functions/_shared/__tests__/fetch-with-timeout_test.ts` (Story 11-3 — 7 cases) +- `supabase/functions/_shared/__tests__/parse-upstream-error_test.ts` (Story 12-11 — 7 cases) +- `supabase/migrations/__tests__/rate_limit_test.sql` (Story 11-4 — 8 pgTAP) +- `supabase/migrations/__tests__/atomic_activity_rpcs_test.sql` (Story 12-3 — 11 pgTAP) +- `supabase/migrations/__tests__/match_error_pattern_test.sql` (Story 11-6 — 7 pgTAP) +- `supabase/migrations/__tests__/get_home_aggregate_test.sql` (Story 13-2 — 7 pgTAP) +- `supabase/migrations/__tests__/get_session_feedback_aggregate_test.sql` (Story 13-3 — 9 pgTAP) + +**15-3 scope (TIGHT):** wire the 2 existing Deno test files into CI. pgTAP wiring DEFERRED to `15-3-followup-pgtap-ci-wiring` (needs Postgres-in-CI service container infrastructure — a non-trivial workflow change). NEW Deno tests for additional Edge Functions DEFERRED to `15-3-followup-edge-function-coverage` (the spec line lists auth gate / rate limit / model allowlist / account-delete — none of which have unit tests today; each is a substantial new test file). + +## Acceptance Criteria + +### AC-A: CI workflow change + +1. NEW step `Deno tests (Edge Function _shared utilities)` added to `.github/workflows/ci.yml` AFTER the `Tests` step (which runs `npm test`). The step: + - Uses `denoland/setup-deno@v2` action to install Deno (latest stable) + - Runs `deno test --allow-net=127.0.0.1 supabase/functions/_shared/__tests__/` + - The `--allow-net=127.0.0.1` permission is required by `fetch-with-timeout_test.ts` which spins up a local HTTP server for the happy-path test +2. The new step runs ONLY the `_shared/__tests__/` directory (NOT `supabase/functions/*/__tests__/` which doesn't exist today + would inadvertently scope the gate to future test files that may have different permission needs) + +### AC-B: Drift detector + +3. NEW `src/lib/__tests__/ci-deno-step-source-drift.test.ts` (≥4 cases) reading `.github/workflows/ci.yml` from disk: + - POSITIVE pin: literal step name `Deno tests (Edge Function _shared utilities)` + - POSITIVE pin: `denoland/setup-deno@v2` action ref + - POSITIVE pin: `deno test` invocation with the canonical `--allow-net=127.0.0.1` permission flag + - NEGATIVE pin: no `--allow-all` (over-permissive) + - NEGATIVE pin: the step does NOT carry `continue-on-error: true` (Story 12-10 R1-H2 silent-disable defense) + +### AC-C: Quality gates + cross-story + +4. All 5 design-system gates green. +5. **Net test growth:** **+4 to +6 net Jest cases** (drift detector cases only — the Deno tests themselves run in CI but don't count toward the Jest suite). +6. **Cross-story invariants:** Story 11-3 `fetchWithTimeout` + Story 12-11 `parseUpstreamError` test bodies unchanged (15-3 wires them into CI but doesn't modify the tests themselves). +7. **No source-module modifications** beyond the ci.yml step + the new drift test file. + +## Operator Decisions + +| Q | Question | Recommended | +| --- | --- | --- | +| Q1 | Use `denoland/setup-deno@v2` vs install Deno via shell curl? | **(a) setup-deno@v2** — official action, cached, faster CI | +| Q2 | Pin Deno version explicitly or use `latest`? | **`v1.x`** — pinned `vx-latest` for the v1 branch (Deno 2.0 has breaking changes; defer migration to a follow-up). Specifically use the action's `deno-version: vx.x.x` with the latest patch of v1. | +| Q3 | Run pgTAP in CI now or defer? | **Defer** — pgTAP needs a Postgres service container; significant workflow complexity. File `15-3-followup-pgtap-ci-wiring`. | + +## Out of Scope + +- pgTAP CI wiring (deferred to follow-up) +- NEW Edge Function tests (auth gate / rate limit / model allowlist / account-delete idempotency — each is a substantial new test file; defer) +- Deno 2.0 migration + +## Tasks / Subtasks + +- [x] **Task 1:** Add `Deno tests (Edge Function _shared utilities)` step to `.github/workflows/ci.yml` (AC #1-2) +- [x] **Task 2:** Add `src/lib/__tests__/ci-deno-step-source-drift.test.ts` with ≥4 cases (AC #3) +- [x] **Task 3:** Quality gates + housekeeping (CLAUDE.md paragraph + sprint-status) + +## Dev Agent Record + +### Agent Model Used + +(to be filled in) + +### Completion Notes List + +### File List diff --git a/_bmad-output/implementation-artifacts/sprint-status.yaml b/_bmad-output/implementation-artifacts/sprint-status.yaml index 71404e0..d308e8d 100644 --- a/_bmad-output/implementation-artifacts/sprint-status.yaml +++ b/_bmad-output/implementation-artifacts/sprint-status.yaml @@ -35,7 +35,7 @@ # - Dev moves story to 'review', then runs code-review (fresh context, different LLM recommended) generated: 2026-03-25 -last_updated: 2026-05-16 # Story 15-1 done. R1 review (5 patches: HIGH × 2 useFakeTimers time-flake fix + dictation empty-original semantic-trap TODO; MED × 3 Case 17 tightening + ref-inequality + typed-case-preservation Case 14a). 2159 tests (+1 net R1). All 5 gates green. PR #111. +last_updated: 2026-05-17 # Story 15-3 done. Deno tests wired into CI; +5 net cases. Cumulative autopilot batch: 15-2 + 15-3 shipped. project: companion project_key: NOKEY tracking_system: file-system @@ -208,7 +208,7 @@ development_status: epic-15: in-progress # 2026-05-16: auto-flipped backlog → in-progress when 15-1 story file was created. Epic 14 retro AIs surfaced at kickoff per Epic 13 AI #4 accountability gate; "Proceed + acknowledge" mode selected (AI #8 absorbed by 15.3 scope; others orthogonal docs/workflow). 15-1-lib-unit-tests: done # PR #111. R1 patches: HIGH × 2 (useFakeTimers time-flake fix; dictation empty-original semantic-trap TODO + 15-1-followup filed) + MED × 3 (Case 17 distinctive-marker assertion; Case 14 ref-inequality; new Case 14a typed-preserves-case). +1 net R1 (2158 → 2159). All 5 gates green. Original: 2026-05-16: Story 15-1 implementation complete. 4 NEW test files (srs.test.ts 14 cases + pronunciation.test.ts 10 cases + cache.test.ts 20 cases + use-dictation-compare.test.ts 15 cases) = +59 net Jest cases (2099 → 2158; squarely within spec target +50-65). 0 source-module modifications — test-only story. All 5 quality gates green. CLAUDE.md paragraph added per Epic 14 retro AI #5. Original: Story 15-1 spec file created. FIRST Epic 15 story. Pure-function lib unit tests for 4 modules where direct tests don't yet exist: srs.ts (SM-2 algorithm full coverage), pronunciation.ts identifyWeakSounds (pure aggregator), cache.ts core API (getCache/setCache/invalidateCache/cacheWithFallback happy + TTL boundary), use-dictation.ts compareSentences (pure word-comparison helper). Coverage inventory completed: activity.ts already fully tested (Story 9-2 + 12-3); memory.ts + error-tracker.ts have substantial partial coverage (sanitize via 9-4 + dedup via 11-6); pronunciation.ts has history-cap test (12-12); the GAP-only scope targets the 4 untested-or-partially-tested pure-function surfaces. Async/DB-touching paths (assessPronunciation Edge wrapper, memory.ts extractFacts/persistMemories/retrieveMemories, error-tracker.ts non-dedup paths) DEFERRED to 15-1-followups or 15-2 hook-integration scope per Story 14-4 R1-22-patch lesson (broad enforcement-rule changes interact with every surface; test-writing stories at analogous risk). 3 operator-decision items Q1-Q3 all resolved per Recommended (defer Edge wrapper / memory async / error-tracker non-dedup). 0 source-module modifications — test-only story. Spec target: +50-65 net Jest cases (2099 → 2149-2164). Status: ready-for-dev. Awaiting /bmad-dev-story. 15-2-hook-integration-tests: done # 2026-05-17: PR #112 (chained autopilot). NEW use-pronunciation.test.tsx (13 cases) — Story 12-12 FIFO + Story 15-1 identifyWeakSounds integration verified. +13 net Jest cases (2159 → 2172). 0 source-module modifications. All 5 gates green. 15-2-followup-existing-hook-flow-integration filed for use-auth/use-exercise/use-realtime-voice extensions. - 15-3-edge-function-deno-tests: in-progress # 2026-05-17: spec written. TIGHT scope — wire existing 2 Deno _shared tests into CI; pgTAP + new Edge Function tests deferred to 15-3-followups. + 15-3-edge-function-deno-tests: done # 2026-05-17: PR pending. NEW ci.yml steps (Setup Deno + Deno tests) + drift detector (5 cases). +5 net Jest cases (2159 → 2164). Closes Epic 12 AI #7 + Epic 13 AI #8 (Deno portion); pgTAP DEFERRED to 15-3-followup-pgtap-ci-wiring. All 5 gates green. 15-4-golden-flow-e2e: backlog 15-5-ai-schema-regression-tests: backlog 15-6-ci-coverage-gating: backlog From 356c8166b05d7386dc8258396427ede291e298c9 Mon Sep 17 00:00:00 2001 From: Martins Aloba Date: Sun, 17 May 2026 14:35:49 -0500 Subject: [PATCH 3/3] =?UTF-8?q?fix(15-3):=20R1=20patches=20HIGH=20=C3=97?= =?UTF-8?q?=203=20+=20MED=20=C3=97=204?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3-agent adversarial review (Blind Hunter + Edge Case Hunter + Acceptance Auditor) surfaced 7 actionable findings: HIGH: - BH-2/EH-1: name test files explicitly in run command (vacuous-pass defense — `deno test ` would have silently exited 0 OR exited non-zero with a confusing "No test modules found" — either way a future rename/move slips past the gate) - BH-3/EH-2: parse-upstream-error_test.ts docstring claimed `--allow-all` while CI uses `--allow-net=127.0.0.1` — fixed docstring to match CI invocation - EH-4: Case 5 ordering regex `/- name:\\s*Tests\\b/` matched BOTH `Tests` and PR #116's `Tests with coverage threshold` because \\b word boundary triggers between "Tests" and space. Anchored to line-end via `/^\\s{6}- name:\\s*Tests\\s*$/m`. MED: - BH-5: Case 4 silent-disable defense only covered the Deno test step; extended to also pin the Setup Deno step (tightly-coupled — silent disable on EITHER fails the gate). Pin both via explicit loop. - BH-6: added `timeout-minutes: 3` so a hung worker can't burn the GitHub Actions 6-hour default. - BH-7: rewrote misleading v1/v2 comment — `denoland/setup-deno@v2` is the action installer version; `deno-version: v1.x` pins the runtime version. Two independent axes. - EH-10: added `--no-check` to skip redundant TS typecheck (npm type-check step already covers the source-of-truth files; saves ~5s/run + decouples from Deno's TS engine drift). Story file housekeeping (per AA): Status review → done; populated File List section. Deferred (filed as follow-ups): - 15-3-followup-action-sha-pinning (BH-1/EH-3 — warrants global PR for all 3 actions: checkout, setup-node, setup-deno) - 15-3-followup-deno-cache (BH-4/EH-6 — perf + outage resilience) - 15-3-followup-real-body-read-timeout-test (EH-8 — needs TCP server) PR #116 `ci.yml` merge conflict acknowledged: operator resolves at merge time (recommend: merge this PR before #116; Case 5 regex defends against the order being flipped). All 4 quality gates green: type-check 0 / lint 0 / prettier clean / jest 2164 tests pass (no net case change — patches tightened existing cases in place). --- .github/workflows/ci.yml | 37 ++++++-- .../15-3-edge-function-deno-tests.md | 21 ++++- .../ci-deno-step-source-drift.test.ts | 86 +++++++++++++------ .../__tests__/parse-upstream-error_test.ts | 13 ++- 4 files changed, 118 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b1eb899..c9bdb82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,9 +38,13 @@ jobs: - name: Setup Deno # Story 15-3: install Deno for Edge Function _shared utility tests. - # Pinned to the latest v1 (Deno 2.0 has breaking changes; defer 2.0 - # migration to a follow-up story). The action caches the install for - # faster subsequent runs. + # `denoland/setup-deno@v2` is the ACTION installer (second major + # release of the installer); `deno-version: v1.x` pins the Deno + # RUNTIME. Two independent version axes — don't conflate. Deno 2.0 + # runtime migration is deferred to a follow-up story. The action + # caches the install for faster subsequent runs. + # SHA-pinning of this action (and actions/checkout / setup-node) + # deferred to `15-3-followup-action-sha-pinning` per R1 BH-1. uses: denoland/setup-deno@v2 with: deno-version: v1.x @@ -48,14 +52,31 @@ jobs: - name: Deno tests (Edge Function _shared utilities) # Story 15-3: wire Story 11-3 fetch-with-timeout + Story 12-11 # parse-upstream-error Deno tests into CI. Pre-15-3 these were - # manual-run only. Now gated on PR merge. The `--allow-net=127.0.0.1` - # permission is required by the fetch-with-timeout happy-path test - # which spins up a local HTTP server. NO `--allow-all` — keep the - # permission surface minimal. + # manual-run only. Now gated on PR merge. + # + # Test files are named EXPLICITLY (R1 BH-2 / EH-1 vacuous-pass + # defense): if either file is renamed or moved, the step fails + # loudly with `Module not found` instead of silently passing on an + # empty directory match. A future file added to _shared/__tests__/ + # must be appended here to be picked up. + # + # Permission flags: + # --allow-net=127.0.0.1 — required by the fetch-with-timeout + # happy-path test which spins up a local HTTP server. + # --no-check — runtime-only verification (R1 EH-10); the + # `npm run type-check` step already covers TS soundness for the + # source modules; Deno's redundant typecheck adds ~5s + a + # drift surface against std lib type updates. + # NO `--allow-all` — keep the permission surface minimal. # pgTAP migrations (supabase/migrations/__tests__/*.sql) NOT wired # here — requires a Postgres service container. Deferred to # `15-3-followup-pgtap-ci-wiring`. - run: deno test --allow-net=127.0.0.1 supabase/functions/_shared/__tests__/ + # Deno deps cache deferred to `15-3-followup-deno-cache` per R1 BH-4. + timeout-minutes: 3 + run: | + deno test --no-check --allow-net=127.0.0.1 \ + supabase/functions/_shared/__tests__/fetch-with-timeout_test.ts \ + supabase/functions/_shared/__tests__/parse-upstream-error_test.ts - name: Dependency vulnerability gate # Story 12-10: prevent regression of `npm audit reports 0 high diff --git a/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md b/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md index c36460a..73cd00b 100644 --- a/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md +++ b/_bmad-output/implementation-artifacts/15-3-edge-function-deno-tests.md @@ -1,6 +1,6 @@ # Story 15.3: Edge Function Deno tests in CI — wire existing `_shared/__tests__/*_test.ts` files into the GitHub Actions workflow -Status: review +Status: done ## Story @@ -75,8 +75,25 @@ Existing manual-run tests: ### Agent Model Used -(to be filled in) +Claude Sonnet 4.6 (claude-sonnet-4-6) via /bmad-dev-story + /bmad-code-review workflows in autopilot mode. ### Completion Notes List +- **CI step wired**: `.github/workflows/ci.yml` gains `Setup Deno` (denoland/setup-deno@v2 + deno-version v1.x) + `Deno tests (Edge Function _shared utilities)` step after the existing `Tests` step. +- **Drift detector**: 5 cases at `src/lib/__tests__/ci-deno-step-source-drift.test.ts` — pins action + version + canonical run command + 2 negative guards (--allow-all + silent-disable) + ordering. +- **R1 patches applied** (HIGH × 3 + MED × 4): BH-2/EH-1 explicit file paths in run command (vacuous-pass defense); BH-3/EH-2 fix --allow-all docstring drift in parse-upstream-error_test.ts; EH-4 anchor Case 5 regex with `/m + $` to defend against PR #116 `Tests with coverage threshold` step; BH-5 extend Case 4 to cover BOTH Setup Deno + Deno test step blocks; BH-6 add `timeout-minutes: 3`; BH-7 rewrite misleading v1/v2 comment to clarify action-version vs runtime-version axes; EH-10 add `--no-check` to skip redundant TS check. +- **Deferred**: BH-1/EH-3 SHA-pin all 3rd-party actions → `15-3-followup-action-sha-pinning` (warrants global PR); BH-4/EH-6 Deno cache → `15-3-followup-deno-cache`; EH-8 real body-read timeout test → `15-3-followup-real-body-read-timeout-test`; EH-5 PR #116 merge conflict — operator resolves at merge time (recommended: merge this PR before #116). +- **Quality gates green**: type-check 0 errors / lint 0 warnings / prettier clean / jest drift detector 5/5 pass. + ### File List + +**New:** + +- `src/lib/__tests__/ci-deno-step-source-drift.test.ts` — 5 drift cases (round-1 includes BH-5 + EH-4 + R1 BH-2/EH-1/EH-10 pin updates) + +**Modified:** + +- `.github/workflows/ci.yml` — added `Setup Deno` + `Deno tests (Edge Function _shared utilities)` steps after the existing `Tests` step (round-1 includes explicit file paths + --no-check + timeout-minutes per R1 patches) +- `supabase/functions/_shared/__tests__/parse-upstream-error_test.ts` — docstring R1 fix (--allow-all → --allow-net=127.0.0.1) +- `_bmad-output/implementation-artifacts/sprint-status.yaml` — 15-3 → done +- `CLAUDE.md` — Story 15-3 architecture paragraph appended diff --git a/src/lib/__tests__/ci-deno-step-source-drift.test.ts b/src/lib/__tests__/ci-deno-step-source-drift.test.ts index 349da92..d85671a 100644 --- a/src/lib/__tests__/ci-deno-step-source-drift.test.ts +++ b/src/lib/__tests__/ci-deno-step-source-drift.test.ts @@ -30,41 +30,77 @@ describe("Story 15-3 — Deno test CI step source drift", () => { expect(ci).toMatch(/deno-version:\s*v1\.x\b/); }); - it("Case 2: Deno test step name + canonical run command pinned", () => { + it("Case 2: Deno test step name + canonical run command pinned (explicit file paths + --no-check + --allow-net=127.0.0.1)", () => { expect(ci).toMatch(/- name:\s*Deno tests \(Edge Function _shared utilities\)/); - // Canonical run command: deno test + scoped permission + scoped dir - expect(ci).toMatch( - /run:\s*deno test --allow-net=127\.0\.0\.1 supabase\/functions\/_shared\/__tests__\// + // Canonical run command form (R1 BH-2/EH-1 vacuous-pass defense: + // explicit file names instead of directory match; R1 EH-10: + // --no-check skips redundant TS check): + expect(ci).toMatch(/deno test --no-check --allow-net=127\.0\.0\.1/); + // Both _test.ts files named explicitly so a rename/move fails loudly + // at "Module not found" instead of silently passing an empty match. + expect(ci).toMatch(/supabase\/functions\/_shared\/__tests__\/fetch-with-timeout_test\.ts/); + expect(ci).toMatch(/supabase\/functions\/_shared\/__tests__\/parse-upstream-error_test\.ts/); + // Bounded execution time so a hung worker can't burn the GitHub + // Actions 6-hour default timeout (R1 BH-6). + const denoBlockMatch = ci.match( + /- name:\s*Deno tests \(Edge Function _shared utilities\)[\s\S]*?(?=\n\s{6}- name:|\n\s*$)/ ); + expect(denoBlockMatch).not.toBeNull(); + expect(denoBlockMatch![0]).toMatch(/timeout-minutes:\s*3\b/); }); it("Case 3: NEGATIVE guard — no --allow-all (over-permissive); permission surface stays minimal", () => { - // Find the Deno test run line specifically (avoid false positives if - // another step happens to mention --allow-all in a comment). - const runLineMatch = ci.match( - /run:\s*deno test [^\n]+supabase\/functions\/_shared\/__tests__\/[^\n]*/ - ); - expect(runLineMatch).not.toBeNull(); - expect(runLineMatch![0]).not.toContain("--allow-all"); - }); - - it("Case 4: NEGATIVE guard — Deno test step does NOT carry `continue-on-error: true` or unrestricted `if:` (Story 12-10 R1-H2 silent-disable defense)", () => { - // Extract the block from "name: Deno tests" to the next "- name:" or EOF. + // Scope to the Deno test step's block so a comment elsewhere + // mentioning --allow-all isn't false-flagged. const denoBlockMatch = ci.match( - /- name:\s*Deno tests \(Edge Function _shared utilities\)[\s\S]*?(?=\n\s*- name:|\n\s*$)/ + /- name:\s*Deno tests \(Edge Function _shared utilities\)[\s\S]*?(?=\n\s{6}- name:|\n\s*$)/ ); expect(denoBlockMatch).not.toBeNull(); - const block = denoBlockMatch![0]; - expect(block).not.toMatch(/continue-on-error:\s*true/); - // No top-level `if:` key that would conditionally skip the step. - // (Note: `if:` inside the `run:` shell script is fine — only flag the - // step-level `if:` key.) - const stepLevelIf = block.match(/^\s{6}if:\s/m); // 6-space indent matches step-level keys - expect(stepLevelIf).toBeNull(); + // Strip comment lines so the "NO `--allow-all`" comment doesn't + // false-flag against the negative guard. + const blockNoComments = denoBlockMatch![0].replace(/^\s*#.*$/gm, ""); + expect(blockNoComments).not.toMatch(/--allow-all\b/); + }); + + it("Case 4: NEGATIVE guard — BOTH Setup Deno + Deno test steps lack silent-disable patterns (R1 BH-5: tightly-coupled steps need parity)", () => { + // R1 BH-5 lesson: silent-disable on the Setup Deno step would leave + // `deno` uninstalled; the subsequent test step would fail with + // `deno: command not found` — BUT if BOTH steps carry the disable, + // CI green-passes vacuously. Pin both blocks. + const blocks: [string, RegExp][] = [ + ["Setup Deno", /- name:\s*Setup Deno\b[\s\S]*?(?=\n\s{6}- name:|\n\s*$)/], + [ + "Deno tests", + /- name:\s*Deno tests \(Edge Function _shared utilities\)[\s\S]*?(?=\n\s{6}- name:|\n\s*$)/, + ], + ]; + for (const [label, blockRegex] of blocks) { + const m = ci.match(blockRegex); + if (!m) { + throw new Error(`${label} block not found in ci.yml`); + } + const block = m[0]; + if (/continue-on-error:\s*true/.test(block)) { + throw new Error( + `${label} step carries \`continue-on-error: true\` (R1 BH-5 silent-disable)` + ); + } + // No step-level `if:` key (6-space indent). `if:` inside a `run:` + // shell script is fine — only flag the step-level YAML key. + if (/^\s{6}if:\s/m.test(block)) { + throw new Error(`${label} step carries step-level \`if:\` key (R1 BH-5 silent-disable)`); + } + } + // Belt-and-suspenders structural pin: both regexes matched. + expect(blocks.length).toBe(2); }); - it("Case 5: Ordering — Deno test step appears AFTER the Tests step (so npm test failures short-circuit before Deno install)", () => { - const testsIdx = ci.search(/- name:\s*Tests\b/); + it("Case 5: Ordering — Deno test step appears AFTER the no-coverage Tests step (so npm test failures short-circuit before Deno install)", () => { + // R1 EH-4: anchor the `Tests` regex to line-end (`$/m`) so the + // `Tests with coverage threshold` step that PR #116 inserts cannot + // match this pattern. `\b` alone would match both "Tests" and + // "Tests with..." (both have a word boundary after "Tests"). + const testsIdx = ci.search(/^\s{6}- name:\s*Tests\s*$/m); const denoIdx = ci.search(/- name:\s*Deno tests \(Edge Function _shared utilities\)/); const setupDenoIdx = ci.search(/- name:\s*Setup Deno\b/); expect(testsIdx).toBeGreaterThan(0); diff --git a/supabase/functions/_shared/__tests__/parse-upstream-error_test.ts b/supabase/functions/_shared/__tests__/parse-upstream-error_test.ts index d35f3a1..33a7a05 100644 --- a/supabase/functions/_shared/__tests__/parse-upstream-error_test.ts +++ b/supabase/functions/_shared/__tests__/parse-upstream-error_test.ts @@ -1,11 +1,16 @@ /** * Story 12-11 — parseUpstreamError runtime tests (Deno-runnable). * - * RUN MANUALLY: `deno test --allow-all supabase/functions/_shared/__tests__/parse-upstream-error_test.ts` + * RUN MANUALLY: `deno test --no-check --allow-net=127.0.0.1 supabase/functions/_shared/__tests__/parse-upstream-error_test.ts` * - * Epic 15.3 (`15-3-edge-function-deno-tests`) owns CI integration for this - * directory — Story 12-11 deliberately does NOT modify `ci.yml`. The dev - * confirms these tests green locally before merge. + * Story 15-3 R1 patch (BH-3 / EH-2): the pre-15-3 `--allow-all` directive + * was over-permissive — these tests don't require env/read/write + * permissions. The CI step uses the scoped `--allow-net=127.0.0.1` form; + * the local-run invocation must match so a contributor's local pass + * implies CI pass. + * + * Epic 15.3 (`15-3-edge-function-deno-tests`) now runs this in CI via the + * `Deno tests (Edge Function _shared utilities)` step in `.github/workflows/ci.yml`. * * Companion Jest tests: * - `src/lib/__tests__/upstream-error-sanitization-source-drift.test.ts`