deckhearth/.convoys/cleanup-mobile-nav-dead-props.md
varutasu 7efb6efe69
docs: post-convoy cleanup for 7-convoy 2026-05-26 wave (#33)
Updates ship-readiness.md, AGENTS.md, and 7 convoy files to reflect
the as-shipped state of the 2026-05-26 7-convoy multitask wave:

- PR #26 tighten-visual-diff-path-filter (P3)
- PR #27 purge-weak-creds-from-helpers (P2, closes the umbrella)
- PR #28 cleanup-mobile-nav-dead-props (P3)
- PR #29 lint-against-cjs-in-esm-scripts (P3, surfaced by PR #25)
- PR #30 single-sql-client (P1 #8 RESOLVED)
- PR #31 single-auth-provider (P1 #9 RESOLVED)
- PR #32 migration-tool (P1 #11 RESOLVED)

Milestone: 5 of 6 P1 quality items RESOLVED. Only fix-lint-baseline
(P1 #11.5) remains in the P1 lane.

Newly queued follow-ups:
- purge-quick-login-from-loginpage (surfaced by PR #27)
- purge-neondatabase-serverless-fully (surfaced by PR #30,
  unblocked by PR #32's migration tool adoption)

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-26 23:15:21 -05:00

228 lines
10 KiB
Markdown
Raw 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.

# cleanup-mobile-nav-dead-props (P3 polish — single-prop hygiene)
**Status:** SHIPPED 2026-05-26 (PR TBD)
**Classification:** hygiene
**Priority:** P3 polish (not a bug, not a security issue; dead-prop
removal is purely a clarity-of-surface cleanup)
**Convoy owner:** parent (no architect — single-line prop removal in
one component + one caller; surfaced and pre-decided in a sibling
convoy)
**Opened:** 2026-05-26
## Background
`components/MobileNavigation.js` accepts `{ user, onMenuOpen }` but
never reads `user.*` — the bottom-bar items (Cards, Decks, Dashboard,
Community, More) are static and don't depend on auth state or role.
This was originally surfaced as **R8** in the `fix-layout-default-user`
convoy (see `.convoys/fix-layout-default-user.md` § R8 and § "Anything
flagged but not acted on") and deferred there with explicit
instructions: *"If the implementer is tempted to delete the prop, they
MUST stop — that's god-component-split / single-auth-provider
territory."* The deferral was correct for that convoy's scope; it is
no longer needed because the prop is genuinely dead at the current
static-bar reality, and removing it does not require a wider auth
refactor.
The follow-up was queued as `cleanup-mobile-nav-dead-props` in
`.convoys/ship-readiness.md` § Queued convoys, with a note that it
may fold into `god-component-split` (P2 #13) if that lands first.
God-component-split has not landed; this small hygiene convoy ships
first.
## Audit results
Pre-edit audit (the spec's "don't blindly trust the queue entry"
clause):
1. **Reading `components/MobileNavigation.js`** — the file is 171
lines. Line 5 destructures `{ user, onMenuOpen }`. Lines 6170 use
`onMenuOpen` exactly once (line 39, as the `onClick` for the "More"
button). `user` does not appear elsewhere — no `user.email`,
`user.role`, `user.id`, no conditional render gated on `user`, no
pass-through to a child component. The bottom-bar `navigationItems`
array is hardcoded and does not branch on auth state.
2. **`rg '\buser\b' components/MobileNavigation.js`** before edit: 1
hit (the destructure on line 5). After edit: 0 hits.
3. **`rg "MobileNavigation" components/ pages/ --type js`**: two
import + JSX-callsite pairs in the codebase:
- `components/Layout.js` (active) — line 5 import, line 598-601 JSX
call passing `user={user}` and `onMenuOpen={...}`.
- `components/Layout.js.backup` (no-go-zone per
`.cursor/rules/no-go-zones.mdc` § "Append-only / historical" —
"legacy snapshot; delete with a real PR, never edit") — line 5
import, line 264 JSX call. Left untouched per the no-go-zone
rule; if/when the `.backup` file is eventually deleted, this dead
call disappears with it.
Audit verdict: `user` is genuinely dead. Cleanup is safe.
## The fix
Two-file, three-line diff:
1. **`components/MobileNavigation.js` line 5**: remove `user` from the
destructured props.
- Before: `export default function MobileNavigation({ user, onMenuOpen }) {`
- After: `export default function MobileNavigation({ onMenuOpen }) {`
2. **`components/Layout.js` lines 598-601**: remove the `user={user}`
JSX attribute from the only active call site.
- Before:
```
<MobileNavigation
user={user}
onMenuOpen={() => setIsMobileMenuOpen(true)}
/>
```
- After:
```
<MobileNavigation
onMenuOpen={() => setIsMobileMenuOpen(true)}
/>
```
No new code. No refactors. No tests added (the component has no
direct test coverage; `test/components/Layout.test.js` tests Layout's
logged-out branch and does not assert on `MobileNavigation`'s prop
shape).
## Out of scope
- The unused `import { useState } from 'react'` on
`components/MobileNavigation.js` line 3. The hook is imported but
not called. This is a pre-existing dead import unrelated to the
`user` prop; the convoy spec explicitly forbids "refactor anything
else in MobileNavigation.js (this is a single-prop removal)". A
future hygiene pass can sweep it (or it'll get caught by an
eventual lint-no-unused-imports rule).
- `components/Layout.js.backup` — no-go-zone, untouched.
## Verification plan
1. `rg '\buser\b' components/MobileNavigation.js` → 0 hits (post-edit
confirmation that the prop is truly gone, not just renamed).
2. `rg "MobileNavigation" components/ pages/ --type js` → confirm
each active call site passes only `onMenuOpen`.
3. `npm run lint` → 128 problems baseline preserved (no regression
introduced; no new dead-code/unused-var warnings created by the
change).
4. `npm run test:run` → 21/21 pass. Specifically,
`test/components/Layout.test.js` continues to pass — its
regression-lock assertions for the logged-out Layout branch
(Gotcha #8) do not depend on `MobileNavigation`'s prop shape, so
the dead-prop removal is invisible to that suite.
5. `npm run build` skipped — relying on Vercel preview CI. Trade-off:
single-prop removal in a leaf component is extremely low risk of
build-time regression, and the Playwright smoke + visual-diff
workflows on the PR will catch any Layout-rendering issue before
merge.
## Risks
- **R1 — A future feature that wants per-user bottom-bar items would
need to re-add the prop.** Hypothetical examples: showing an
unread-count badge on a "Notifications" tab gated on `user.id`, or
hiding the "Community" tab for unauthenticated visitors. **Accepted.**
Re-adding a prop is a one-line change when the feature actually
lands; carrying a dead prop "just in case" obscures the current
surface and adds nothing. The cleanup is correct for the
current static-bar reality; future features pay their own
add-the-prop cost.
- **R2 — `components/Layout.js.backup` still references the old prop
shape.** **Accepted.** The backup is a no-go-zone (per
`.cursor/rules/no-go-zones.mdc`) and is dead code by definition.
Touching it would violate the rule; leaving it as a stale snapshot
is the convention. When the backup is eventually deleted in a
separate convoy, this stale call disappears with it.
## Acceptance criteria
- 2 files modified, 3 lines net change (1 line edit + 1 attribute
removal from a multi-line JSX block).
- `npm run lint` exit 1 with 128 problems (baseline preserved).
- `npm run test:run` 21/21 pass.
- `rg '\buser\b' components/MobileNavigation.js` → 0 hits post-edit.
## Owns
Parent (single-prop removal in a leaf component; no architect or
implementer subagent required; audit confirms the queue entry's
premise).
## As-shipped
Single squash commit `171f5af` (PR #28, merged 2026-05-27T03:53:29Z
UTC / local 2026-05-26). Parent-owned end-to-end per the convoy spec
— no architect, no implementer subagent dispatched. Single-prop
removal in a leaf component, exactly as planned; no mid-execution
surprises.
**Diff: 3 files, +156 / -2.** `components/MobileNavigation.js` (1
line edit — removed `user` from the destructured props) +
`components/Layout.js` (1 attribute removal from the JSX call site —
removed `user={user}`) + `.convoys/cleanup-mobile-nav-dead-props.md`
(the planning document, committed atomically with the fix). The +156
addition figure is dominated by the planning doc; actual source diff
is 3 lines net.
**The two surface edits:**
1. **`components/MobileNavigation.js` line 5:** `export default function
MobileNavigation({ user, onMenuOpen }) {` → `export default function
MobileNavigation({ onMenuOpen }) {`.
2. **`components/Layout.js` lines 598-601:** removed the `user={user}`
JSX attribute from the only active `<MobileNavigation>` call site.
**Audit verdict pre-fix confirmed.** The pre-edit grep
(`rg '\buser\b' components/MobileNavigation.js`) returned 1 hit (the
destructure on line 5); post-edit grep returned 0 hits. The bottom-bar
items (Cards, Decks, Dashboard, Community, More) are static and don't
depend on auth state or role — the `user` prop was genuinely dead.
**`components/Layout.js.backup` left untouched** per the
`.cursor/rules/no-go-zones.mdc` § "Append-only / historical" rule
("legacy snapshot; delete with a real PR, never edit"). The backup
still references the old `MobileNavigation({ user, onMenuOpen })`
prop shape; when the `.backup` file is eventually deleted in a
separate convoy, that stale call disappears with it. This is the
documented out-of-scope handling.
**Verification (all gates green at merge):**
- `rg '\buser\b' components/MobileNavigation.js` → 0 hits post-edit
- `rg "MobileNavigation" components/ pages/ --type js` → 1 active
call site (`components/Layout.js`) passing only `onMenuOpen`, plus
the historical `.backup` reference (intentional)
- `npm run lint` → 125 problems (post-PR-#31 baseline preserved; no
new dead-code/unused-var warnings created by the change)
- `npm run test:run` → 21/21 pass. Specifically,
`test/components/Layout.test.js`'s 5 regression-lock assertions for
the logged-out Layout branch (Gotcha #8) continue to pass — they
don't assert on `MobileNavigation`'s prop shape, so the dead-prop
removal is invisible to the suite
- CI on PR #28: Lint ✓ | Vitest 21/21 ✓ | Playwright smoke 3/3 ✓ |
`forbidden-endpoints` ✓ | `forbidden-cors-headers` ✓ | Vercel
preview deploy ✓ | Aggregate gate ✓
- `Screenshot diff`: triggered (PR #28 touches `components/**` which
matches the visual-diff path filter) — `continue-on-error: true`
swallow per the documented Decision-4 end state of
`adopt-playwright-smoke` (no baseline committed yet). Not a
regression; the dead-prop removal cannot move a single rendered
pixel because the prop was never read.
**Out-of-scope items preserved as documented:**
- Pre-existing dead `import { useState } from 'react'` on
`components/MobileNavigation.js` line 3 — untouched per the convoy
spec's "single-prop removal" boundary. A future hygiene pass can
sweep it.
- `components/Layout.js.backup` — untouched per no-go-zone rule.
**Operator action required going forward:** **none.** No env vars, no
schema, no infra.
**Spec deviation:** none.
**No follow-up surfaced.** The convoy's documented R1 (hypothetical
future feature that wants per-user bottom-bar items) is an "accept
the cleanup, pay the add-the-prop cost when the feature actually
lands" call — not a queued follow-up.