Records that the /docs section and homepage fix are merged (PR #8) and live, the proxy.ts PUBLIC_ROUTES hotfix that made /docs reachable without a session, and the authoring/middleware gotchas for future public route groups.
129 lines
9.6 KiB
Markdown
129 lines
9.6 KiB
Markdown
# HANDOFF — audit / cleanup / architecture engagement
|
||
|
||
**Branch:** `audit/cleanup-architecture` (40 commits ahead of `main`, **not pushed**).
|
||
**Status:** all gates green — `tsc` 0 errors · `eslint` 0 errors · **645 tests** · `next build` ✓.
|
||
**Full audit + plan + progress log:** `docs/AUDIT-2026-08.md`. **Architecture:** `ARCHITECTURE.md`.
|
||
|
||
> ⚠️ `main` auto-deploys to production on push (`.github/workflows/deploy.yml` → SSH → droplet
|
||
> `deploy.sh`). Do **not** push to `main` until this branch is reviewed and you intend to deploy.
|
||
|
||
## What was done
|
||
|
||
Delivered against the original brief (audit → fixes → tests → refactor → docs), then a second
|
||
separation-of-concerns pass on top.
|
||
|
||
- **Type safety.** 586 → **0** `tsc` errors; removed `next.config` `typescript.ignoreBuildErrors`, so
|
||
every build/deploy now type-checks. Root cause was the `@supabase/ssr` ↔ `supabase-js` version drift
|
||
(schema collapsed to `never`), fixed by annotating the client factories (no dependency bump).
|
||
- **Tooling / CI.** Added ESLint 9 flat config (`eslint.config.mjs`), a `typecheck` script, and a
|
||
checks-only CI workflow (`.github/workflows/ci.yml`: typecheck + lint + unit tests), separate from deploy.
|
||
- **Confirmed bugs fixed** (one commit + regression test each): invite `max_uses` TOCTOU (atomic CAS),
|
||
board invite re-arming, `appealIncident` admin-gate + orphan rollback, SSRF hex-IPv6, payment dues
|
||
regression (advance-only) + payment `member_id` IDOR scoping, three null-deref crashes, broken
|
||
member-directory visibility gate, equipment/proposals scoping + unverified-member gating, door-log
|
||
parser, CSV-import hang, and a **plaintext-secrets** security bug (ops "Add Secret" wrote the value
|
||
unencrypted from the browser — now via `createSecret`, AES-GCM).
|
||
- **Tests.** Replaced a tautological `actions.test.ts` (asserted string literals) with real authz/IDOR
|
||
tests; added authz-primitive tests and per-fix regression tests.
|
||
- **Refactor (all 10 slices).** `lib/validations.ts` (143 exports) → `lib/validations/*` barrel;
|
||
`lib/types.ts` → `types/domain/*`; `lib/actions/{classes,door,forms}.ts` → per-capability folders behind
|
||
the `lib/actions` barrel; the 663–956-line `ops`/`settings`/`members` client components decomposed into
|
||
thin orchestrators + `panels/`+`components/` (also fixed a form-field-focus footgun); dead code removed.
|
||
`types/database.ts` is left pristine (regenerate, don't hand-edit).
|
||
- **Separation of concerns.** **No client component writes to the DB directly anymore.** The ops
|
||
area-lead roster and the comms message send were the last holdouts — both now go through server actions
|
||
(validated, authz-gated, space-scoped, audited); comms derives sender identity server-side (was
|
||
impersonation-prone). Only browser-client usage left: auth (login/signup) and the comms realtime read.
|
||
- **Verification.** A re-audit found **zero refactor regressions** (every authz gate + `space_id` scope +
|
||
handler independently verified across the splits/decompositions). A final independent adversarial review
|
||
of the separation-of-concerns commits found **no introduced bug, security hole, or regression**.
|
||
|
||
## How to verify (from a clean checkout of the branch)
|
||
|
||
```
|
||
pnpm install --frozen-lockfile
|
||
pnpm run typecheck # 0 errors
|
||
pnpm run lint # 0 errors (warnings are tracked debt)
|
||
pnpm run test:run # 645 pass
|
||
pnpm build # compiles + type-checks all routes
|
||
```
|
||
|
||
## What is NOT done / residual risk
|
||
|
||
1. **Full authenticated UI testing did not run** — the dev environment had no Docker → no local Supabase.
|
||
Public pages + routing were browser-smoke-tested (render clean; authed routes redirect to `/login`),
|
||
and all routes compile via `next build`, but the decomposed **ops/settings/members** screens were not
|
||
clicked through against real data. **Do a manual smoke-test against a real backend before merge.**
|
||
2. **`types/database.ts` needs regeneration** against the live schema (`supabase gen types typescript
|
||
--schema public > types/database.ts`). That removes 3 hand-added RPC entries and ~11 `// TODO(types)`
|
||
join casts, and lets the governance rows in `types/domain/governance.ts` collapse to `Tables<>`/
|
||
`Enums<>`. Needs DB credentials. **Highest-leverage next step.**
|
||
3. **Deferred bug L3** — `markOnboardingStepDone` read-modify-writes a JSON column; a concurrent
|
||
double-submit can drop a step. Needs a DB-side atomic append (RPC / jsonb operator + migration).
|
||
4. **CI runs only unit tests** — the integration + Playwright e2e suites need a DB service wired in
|
||
(and the run should fail on 0 executed integration tests so a missing DB can't masquerade as passing).
|
||
5. **Pre-existing comms realtime dedup edge case** (`app/(app)/comms/comms-client.tsx` ~L82) — two
|
||
identical messages fired in quick succession before the first realtime event lands can collapse onto
|
||
one row (duplicate render / duplicate React key). Not introduced by this work (the old direct-insert
|
||
path had the same race); the robust fix is a client-generated message id / nonce for dedup.
|
||
6. **Commit granularity caveat** — commits are logically split (one per fix), and most were gated before
|
||
commit; a few in the final SoC round were split from a combined working tree and were not each
|
||
independently `tsc`/test-gated. The **final tree is fully green**. If you need per-commit bisectability,
|
||
verify per commit or squash-merge.
|
||
|
||
## Recommended merge path
|
||
|
||
Review Phases A + B + CI first (low-risk, high-value: type safety, bug fixes, tests). Then the refactor
|
||
slices (structural, verified by tsc + build + tests). Then the SoC pass. Merge via PR; a squash or
|
||
merge-commit is fine. After merge to `main` it deploys — have the manual UI smoke-test done first.
|
||
|
||
---
|
||
|
||
## Update — 2026-08-05 (merged to `main`, live)
|
||
|
||
The audit/cleanup branch was merged (PR #7) and deployed. The four residual items above are now resolved:
|
||
|
||
1. **Manual UI smoke-test — DONE.** The decomposed ops/settings/members screens (plus onboarding, dashboard,
|
||
/me) were clicked through against a real seeded backend; a live mutation (area-lead create) and the
|
||
certifications dialog were exercised. Zero console/server errors.
|
||
2. **`types/database.ts` regenerated — DONE.** Regenerated from the canonical `scripts/schema.sql` (built on
|
||
a local Supabase). All 11 `// TODO(types)` join casts removed, hand-added RPCs now native, and
|
||
`types/domain/governance.ts` collapsed to `Tables<>`/`Enums<>`. Two embeds that resolved to `any[]` were
|
||
traced to the deliberately-wide `ServerSupabase` gate type and fixed by using `createClient()` directly.
|
||
3. **Deferred bug L3 — FIXED.** `markOnboardingStepDone` now routes through `mark_onboarding_step_done`
|
||
(migration `055`): a single atomic, ownership-guarded dedup-append into `onboarding_progress`. Covered by
|
||
`integration/onboarding-progress.test.ts` (incl. a 12-way concurrency case).
|
||
4. **CI DB-backed suites — DONE.** `.github/workflows/ci.yml` now has an `integration-e2e` job that stands up
|
||
a Supabase service, runs the integration suite and Playwright e2e, and **fails if the integration suite
|
||
executes 0 tests** (so a missing DB can't masquerade as passing).
|
||
|
||
Items 5 (comms realtime dedup) and 6 (commit granularity) still stand as written.
|
||
|
||
## Docs section + homepage fix (branch `feat/docs-and-homepage-fix`)
|
||
|
||
- **Homepage redirect fix** (`app/(landing)/page.tsx`): it used to `redirect('/dashboard')` for any signed-in
|
||
visitor, so the marketing site (and the new docs) were unreachable while logged in. The redirect is gone;
|
||
the nav/hero/closing CTAs are now auth-aware (signed-in shows a "Dashboard" button).
|
||
- **Public `/docs` section** organized by Diátaxis (tutorials / how-to / reference / explanation), **34 pages**
|
||
covering every module. Source of truth is `lib/docs/nav.ts`; content is markdown under
|
||
`content/docs/<category>/<slug>.md`, read at build time (the routes are statically generated) and rendered
|
||
by `components/docs/docs-markdown.tsx` (reuses the existing `react-markdown` stack, no new deps). The
|
||
`(docs)` route group has its own theme (`docs.css`) matching the public site. Screenshots live in
|
||
`public/docs-media/` and are embedded as captioned figures. Docs prose contains no em-dashes by house style.
|
||
- **Authoring notes:** add a page = add it to `DOC_CATEGORIES` in `lib/docs/nav.ts` and drop the matching
|
||
`content/docs/<category>/<slug>.md`. No H1 in the body (the page chrome renders the title). Cross-link docs
|
||
as `/docs/<category>/<slug>` and app screens as `/route`. Screenshots go in `public/docs-media/` and embed
|
||
as `` (the alt text becomes the figure caption). House style: no em-dashes.
|
||
Every page was fact-checked against the code by an adversarial verify pass; the permission gotcha that
|
||
recurs: server actions often allow `board`, but the space-config UI (`/settings`, `/customize` config
|
||
panels, integrations) is admin-only.
|
||
- **Middleware gotcha (bit the first deploy):** `proxy.ts` (the Next 16 middleware) redirects any path not in
|
||
its `PUBLIC_ROUTES` allowlist to `/login`. **Any new public route group must be added to `PUBLIC_ROUTES`**,
|
||
or it 307s unauthenticated visitors to login. `/docs` is now listed; `/docs-media/*` static images already
|
||
bypass via the image matcher. Test public pages with cookies cleared: a browser "new tab" keeps the session,
|
||
so it is NOT a signed-out test.
|
||
|
||
**Shipped:** merged via PR #8 (+ a `proxy.ts` public-route hotfix) and deployed. Live at `hackerspace.sh/docs`
|
||
(verified public: `/docs` returns 200 with no session; `/dashboard` still redirects to `/login`). Gates:
|
||
`tsc` 0 · `lint` 0 errors · `build` ✓ (all 34 `/docs` pages prerendered) · CI green · deploy ✓.
|