Monorepo Testing / CI / DX Review

Reviewer: Claude (Opus 4.7) — 2026-05-14 Repo: /home/ta/projects/monorepo/ (Astro multi-site + SvelteKit PWA + Python packages)

Critical

1. CI is broken on every push — case-mismatched directory path

What: .github/workflows/test.yml references working-directory: sites/Template (capital T) and sites/Template/... paths in 7+ places, but the actual directory on disk is sites/template (lowercase). The CI memo in CLAUDE.md claims “no CI/CD service required” and “all testing runs locally” — but a workflow file exists and runs on every push/PR to main. On the Ubuntu (case-sensitive) runners, every job fails immediately at cd sites/Template.
Where: /home/ta/projects/monorepo/.github/workflows/test.yml lines using sites/Template (typecheck, e2e matrix, lint, lighthouse). Also hashFiles('sites/Template/pnpm-lock.yaml') — that file doesn’t exist either (workspace has only root pnpm-lock.yaml).
Why it matters: Either (a) CI has been silently red for a long time and is being ignored, or (b) someone believes CI is working when it isn’t. Either case is dangerous for a solo founder going to production. The CLAUDE.md story (“no CI today”) contradicts the actual .github/ directory — pick one and make reality match.
Fix:
- Decision 1: Do you want CI on? If no, rm -rf .github/workflows/ and align with the documented “local-only” story.
- Decision 2: If yes, sed-replace sites/Template → sites/template in test.yml and localize.yml. Also drop hashFiles('sites/Template/pnpm-lock.yaml') → hashFiles('pnpm-lock.yaml') (lockfile lives at root in a pnpm workspace).
- Add a status badge to README so silent failures stop being silent.

2. `pnpm lint` is currently broken at HEAD (7 errors, exit 1)

What: Running pnpm lint from root surfaces 7 no-var errors in sites/template/src/pages/brands.astro plus 10 no-explicit-any warnings. Exit code 1.
Where: /home/ta/projects/monorepo/sites/template/src/pages/brands.astro lines 106-139.
Why: The /test-all slash command runs pnpm lint and would fail here. But this isn’t caught by either git hook — pre-commit runs lint-staged (only on staged files) and pre-push doesn’t run pnpm lint at all. So full-repo lint debt accumulates silently between sessions. If/when CI is fixed (item 1), every push goes red.
Fix: Auto-fixable per ESLint output — pnpm lint:fix should clear the 7 no-var errors. Then add pnpm lint to .husky/pre-push so this can’t regress. Cheap: ESLint on a warm cache is a few seconds.

3. Python tests are completely orphaned from the JS test pipeline

What: packages/sd-math/tests/ has 11 substantive test files (term loan, interest-only, historical, monthly variants for CA/US). packages/sd-api/tests/ has API tests. None are invoked by pnpm test, /test-all, pre-commit, or pre-push. The root package.json test script only filters @smart-debt/design-tokens.
Where: /home/ta/projects/monorepo/package.json scripts; /home/ta/projects/monorepo/.husky/pre-push; /home/ta/projects/monorepo/.claude/commands/test-all.md.
Why: sd-math is the LevPro VB6 port — a financial calculation engine where regressions silently produce wrong investment math. This is the highest-stakes code in the repo and has zero automated guardrail at commit/push time.
Fix: Add "test:py": "cd packages/sd-math && uv run pytest && cd ../sd-api && uv run pytest" to root package.json, then a "test:all": "pnpm test && pnpm test:py && pnpm --filter @smart-debt/sd-app run test". Wire into .husky/pre-push and update /test-all.md skill to include Python + sd-app vitest. (apps/sd-app/src/lib/calculators/ca/index.test.ts is also currently orphaned — same fix.)

Important

4. `/test-all` skill is incomplete vs. its claim

What: .claude/commands/test-all.md claims “equivalent to CI pipeline” and “all unit and lint checks”, but:
- Does NOT run Python tests (sd-math, sd-api) — see #3.
- Does NOT run apps/sd-app vitest (pnpm --filter @smart-debt/sd-app test).
- Does NOT run apps/sd-app svelte-check (the check script).
- Does NOT run sites/mbr astro check or its Playwright tests (mbr has tests/accessibility.spec.ts + smoke.spec.ts).
- Does NOT run sites/sdc build/check (sdc has no test script — separate gap).
Where: /home/ta/projects/monorepo/.claude/commands/test-all.md.
Why: A “full test suite” command that misses ~60% of the testable surface is worse than no command — it creates false confidence.
Fix: Rewrite to actually iterate every workspace’s test script via pnpm -r run test (after each package defines a no-op or real test), plus an explicit pnpm test:py, plus mbr E2E. Or split into /test-fast (current) and /test-all (everything).

5. Pre-push hook only runs Template E2E — mbr, sd-app, Python all skipped

What: .husky/pre-push runs (a) design-tokens unit tests, (b) Template astro check, (c) Template chromium E2E. mbr site, sd-app, sd-math, sd-api are never touched.
Where: /home/ta/projects/monorepo/.husky/pre-push.
Why: Same false-confidence problem as #4 at push time. Solo founder is the last line of defense — push hook needs to actually check what’s been touched.
Fix: Either (a) make it comprehensive: add pnpm -r --if-present run test and pnpm test:py, accept the longer push time, or (b) make it changed-only: detect which workspaces have changes via git diff --name-only origin/main...HEAD and run only their tests. Option (b) is the right pattern for monorepos and stays fast.

6. No root TypeScript strictness; per-site `strict` not uniformly enforced

What: Root tsconfig.json has no compilerOptions.strict and no extends. sites/template and sites/mbr inherit astro/tsconfigs/strict (good). apps/sd-app sets strict: true (good). But none set noUncheckedIndexedAccess, noImplicitOverride, or exactOptionalPropertyTypes — the strict-er options that catch real bugs in financial math code.
Where: All 5 tsconfig.json files listed above.
Why: sd-math is doing array math (amortization, time-series). noUncheckedIndexedAccess: true is exactly the rule that catches array[i] returning undefined and silently propagating NaN through a 30-year projection.
Fix: Add "noUncheckedIndexedAccess": true to apps/sd-app/tsconfig.json minimum. Consider for site tsconfigs too. Root tsconfig — either delete it (purely for path mapping that nothing uses since each site duplicates the paths) or make it a real tsconfig.base.json that all packages extend.

7. `deploy.sh` lacks pre-flight verification

What: Both sites/template/deploy.sh and sites/mbr/deploy.sh go straight from “check token exists” to pnpm run build to wrangler pages deploy. No test run before deploy, no git-clean check, no --dry-run mode for sanity, no rollback notes. sites/mbr/deploy.sh runs wrangler pages project create … || true on every invocation — silent failure-tolerant which masks real config drift.
Where: /home/ta/projects/monorepo/sites/template/deploy.sh, /home/ta/projects/monorepo/sites/mbr/deploy.sh. (sites/sdc has no deploy.sh at all — also a gap if sdc is supposed to deploy.)
Why: A failed pre-push hook can be bypassed with --no-verify; deploy.sh is the one place that should re-assert correctness before pushing to production CF.
Fix: Add before build: (a) git diff --quiet || { echo "uncommitted changes"; exit 1; } (or warn), (b) pnpm test + pnpm --filter <site> test (astro check), (c) optional pnpm test:e2e --project=chromium. Add --preview to mbr (template already has it). Document rollback in docs/ — wrangler pages deployment list + wrangler pages deployment ….

8. `.env` parsing in deploy.sh will break on values with spaces/special chars

What: export $(grep -v '^#' .env | xargs) is the classic broken pattern — xargs mangles quoted values, breaks on = in values, and silently drops anything containing spaces. Used in both deploy.sh files.
Where: sites/template/deploy.sh lines 14-18; sites/mbr/deploy.sh lines 5-9.
Why: If CLOUDFLARE_API_TOKEN is fine today but you ever add a multi-word var (a description, a JSON value), deploy will silently set a malformed env and break in opaque ways.
Fix: Replace with set -a; . ./.env; set +a (POSIX-compliant, handles quoting correctly).

9. sd-app PWA has only one test file

What: apps/sd-app/ is a SvelteKit PWA doing financial calculations and i18n routing. Only test file found: src/lib/calculators/ca/index.test.ts. No tests for routing, i18n locale switching, currency formatting, or the US calculator path.
Where: /home/ta/projects/monorepo/apps/sd-app/src/.
Why: Given the FSS mission (helping investors with leveraged-investing math), calculator correctness is the product. One test file is well short of safe.
Fix: Goal: every file under src/lib/calculators/ and src/lib/formatters.ts has a sibling .test.ts. Add playwright E2E for the /[country]/[lang]/ route matrix smoke (8 routes given 2x2 + tools).

Minor

10. README onboarding gaps (specific)

README.md directory tree shows sites/Template/ (capitalized) and sites/sdc.com/ — neither matches reality (sites/template, sites/sdc). New dev will cd sites/Template and on Linux that fails.
No mention of Python toolchain or uv despite packages/sd-api and packages/sd-math requiring it. New dev cannot run the full repo from README alone.
No pnpm install && pnpm test smoke instruction; no mention of .husky auto-install (pnpm prepare script is husky which works, but the dev needs to know it ran).
Fix: README “Quick Start” should be a 5-command block that ends with all-green tests. Add a “Python packages” subsection: cd packages/sd-math && uv sync && uv run pytest.

11. No Dependabot / Renovate configured

No .github/dependabot.yml or renovate.json. Heavy dep surface (Astro 5, SvelteKit 2, Playwright, Vite 6, Tailwind 3) with security-sensitive sub-deps. Solo founder = no human triage capacity.
Recommend Renovate over Dependabot for this stack: better pnpm workspace support, can group minor/patch into one PR per week (less noise), supports automerge: true for dev-deps + passing tests. Minimum: a .github/dependabot.yml with weekly schedule grouping minor + patch.

12. Playwright config — `webServer.timeout: 120000` and no `reuseExistingServer: false` for CI

playwright.config.ts uses reuseExistingServer: !process.env.CI — correct. But webServer.command: 'pnpm run dev' not pnpm run preview after a build — CI runs the dev server. That’s slower and tests the dev path, not production output. For E2E that’s defensible but not ideal; production-mode bugs (e.g. CSS purging issues) won’t be caught.
Fix: consider a playwright.config.ci.ts that runs astro build && astro preview for one CI project.

13. Visual regression baselines committed but no review workflow

sites/template/tests/visual-regression.spec.ts-snapshots/ and dark-mode.spec.ts-snapshots/ are committed. No mention in CLAUDE.md of how to update them (--update-snapshots) or how OS-rendering differences (WSL vs CI Ubuntu) are reconciled. Likely source of flakes when CI is re-enabled.
Fix: document the snapshot-update flow in sites/template/tests/README.md (doesn’t exist).

14. `sites/sdc/` has no test script and no Playwright tests

sites/sdc/package.json only has dev/build/preview/astro — no test. If sdc is intended as a production site (per CLAUDE.md “brand sites consume those components”), it needs minimum astro check and a smoke E2E.

15. Husky pre-commit doesn’t run TypeScript check on staged Astro files

lint-staged skips .astro files (per MEMORY.md: removed due to parse failures). So a developer can stage broken-type Astro and commit cleanly; the error surfaces at pre-push or in CI. Not blocking but worth a TODO.

Solo-founder CI recommendation (concise)

Given the FSS mission and stakes (financial math for retail investors), my recommendation:

Keep local-only hooks as the primary fast path, but add ONE minimal GitHub Actions job — and fix the broken one that already exists.

The 80/20 minimum:

ci.yml — one job, runs on push to main. Steps:
- pnpm install --frozen-lockfile
- pnpm test (vitest) + pnpm --filter @smart-debt/sd-app run test (vitest)
- pnpm --filter template-site run test + pnpm --filter mbr-site run test (astro check)
- pnpm --filter @smart-debt/sd-app run check (svelte-check)
- pnpm lint
- uv sync && uv run pytest in packages/sd-math (highest-stakes code)
- Skip Playwright entirely in CI — keep that local. Solo founder doesn’t need 7-browser CI; you ran them locally before push.
Status badge in README → silent failures become loud.

This gives you: cross-machine reproducibility, dependency-update sanity (when you add Renovate), and a guardrail against --no-verify pushes. Runtime under 3 minutes. Skipping Playwright in CI is the deliberate trade — those tests are about visual/behavioural confidence on your own hardware and don’t add much over what you’ve already verified locally.

Delete .github/workflows/localize.yml if it’s not actively used (didn’t audit; verify).

Summary count

3 Critical (CI broken, lint broken, Python tests orphaned)
6 Important (test-all incomplete, pre-push narrow, TS strictness, deploy.sh issues x2, sd-app test coverage)
6 Minor (README, Renovate, Playwright config, snapshots, sdc gap, husky/.astro)