Files
hackerspace.sh/docs/HANDOFF-audit-cleanup.md
Moheeb Zara 85f3655e6f docs: update HANDOFF with shipped docs state + the proxy public-route fix
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.
2026-08-05 11:39:46 -07:00

129 lines
9.6 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 `![caption](/docs-media/<name>.jpg)` (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 ✓.