TwoMore

Appearance

Hardcoded-value audit — config-driven-flow gaps ​

Status: Active · Owner ask: "No hardcoded values when configurations-driven flow can work." Read-only adversarial audit. C1 (the cancel-policy scoring bug) was fixed by migration 00639 on 2026-09-09; H3-H5 (rate-limit and push-cap registries) were fixed by migration 00641 on 2026-09-10 (private.rate_limit_policies registry + edge-function callers repointed — see that migration's header for the caller-override-vs-removal decision). H1 (attendance-strike constants across 8 migrations), H2 (client/SQL ELO drift) and M1-M5 remain open.

Adversarial audit only. Nothing changed. Scope: packages/app/src/domain/**, packages/app/src/presentation/**, packages/features/**, supabase/functions/**, supabase/migrations/**. Findings below passed the "could a club/session/ environment/product decision reasonably vary this?" test; universal constants, loop bounds, and already-centralized named constants were excluded. Two background sub-audits covered presentation/features and supabase/functions respectively; their findings are folded in and marked [agent].

CRITICAL ​

C1. Attendance cancel-window boundary hardcodes 12 inside a per-club-configurable window — creates a dead penalty tier for some clubs ​

  • File: supabase/migrations/00444_information_flow_wave2.sql:119 (latest live CREATE OR REPLACE FUNCTION private.classify_rsvp_cancel(); the same bug was carried forward verbatim through 00287_timezone_localization.sql:58, 00325_audit_batch16_no_strike_on_admin_cancel.sql:75, and 00405_ps5_dues_overdue_and_strikes.sql:1067 — four consecutive rewrites, never fixed)
  • What it encodes: the boundary between late_cancel (0.5 strike) and no_show (1.0 strike) for a cancellation, in hours-before-session.
  • The bug: the function already reads a per-club column, clubs.attendance_cancel_window_hours (the free_cancel boundary), but the second boundary is a bare 12:
    sql
    ELSIF v_hours_left >= v_club.attendance_cancel_window_hours THEN v_final_status := 'free_cancel';
ELSIF v_hours_left < 12 THEN v_final_status := 'no_show';
ELSE v_final_status := 'late_cancel';
    For any club that sets attendance_cancel_window_hours <= 12 (a perfectly reasonable admin choice — e.g. a 6h free-cancel window), the late_cancel branch becomes unreachable: every cancellation inside the free-cancel window is < 12 by construction, so it is scored as a full-strike no_show instead of the intended half-strike late_cancel. The grace tier silently disappears — members get penalized harder than the club's own stated policy implies, with no error, no test failure, nothing visible until a member disputes a strike.
  • Fix: add a second per-club column (e.g. clubs.attendance_late_cancel_window_hours, defaulting to 12) or derive it as a fraction of attendance_cancel_window_hours, and require late_cancel_window < cancel_window at the DB constraint level so the tiers can never collapse. Update the one live function; the dead older CREATE OR REPLACE copies don't need separate fixes (Postgres keeps only the latest body) but confirm no other caller still matches on a bare 12.
  • Prevention: a migration-review checklist item ("does this function read a per-club threshold column anywhere near a bare numeric constant of the same kind?") won't be caught by any existing lint (SQL isn't linted for this class at all). The cheapest guard is a pgTAP test that asserts, for a club with attendance_cancel_window_hours = 6, that a cancellation at hours_left=8 comes back late_cancel (it currently would fail this assertion) — add it to the existing pgTAP suite as a regression lock once fixed.

HIGH ​

H1. Attendance-strike window (20) and flag threshold (3) duplicated across 2 TS files and ≥8 SQL migrations, with zero shared source ​

  • TS: packages/app/src/presentation/hooks/queries/use-club-attendance-summary.ts:17 (const ROLLING_WINDOW = 20;) and :54 (const isFlagged = strikesInWindow >= 3;) — an independent re-declaration of packages/app/src/adapters/supabase/attendance-record.supabase.ts:29 (const ROLLING_WINDOW = 20;, consumed at :183 .limit(ROLLING_WINDOW)). Same name, same value, two unrelated module scopes.
  • SQL: the identical "last 20 records, flag at >=3, cooldown at >=2" logic is re-implemented independently in 00132_attendance_strike_notifications.sql, 00133_rsvp_cooldown_enforcement.sql (cooldown gate, >= 2, blocks RSVP confirm — a hard RLS-adjacent gate, not just cosmetic), 00163_notification_pipeline_rewire.sql, 00385_alert_model_wiring.sql, 00405_ps5_dues_overdue_and_strikes.sql, 00381_club_admin_snapshot.sql, 00384_settle_before_play.sql, and 00637_session_detail_bundle.sql:273 (WHERE rn <= 20). None reference a shared constant; each is a hand-copied LIMIT 20 / rn <= 20 / >= 3 literal.
  • Why it matters: this is exactly the drift class CLAUDE.md's own "sf the LIVE function" lesson warns about (00550/00518/00530 FIX-D drop) — except here there isn't even one canonical copy to diff against; there are ~10. Changing "3 strikes" to "4" or the window from 20 to 15 sessions requires finding and editing all of them by grep, and a missed one silently disagrees with the rest (e.g. the notification ladder could fire "flagged" at a different threshold than the admin dashboard's isFlagged).
  • Fix: SQL side — one private.attendance_strikes_in_window(user_id, club_id) helper function that all seven+ callers invoke instead of re-deriving; window size (20) and flag threshold (3) become named constants inside that one function (or a clubs/app_config row if it should ever be club-tunable, which is plausible — some clubs may want a stricter or looser bar). TS side — export ATTENDANCE_ROLLING_WINDOW / ATTENDANCE_FLAG_THRESHOLD from one module (e.g. trust.entity.ts, which already carries the domain comments about "rolling 20-session") and import it in both use-club-attendance-summary.ts and attendance-record.supabase.ts instead of each declaring its own ROLLING_WINDOW.
  • Prevention: lint rule — ban a second const ROLLING_WINDOW = (or any identically-named module-scope constant) across the package; more generally, a "duplicate literal-holding constant name across files" check is a legitimate custom ESLint rule. SQL has no equivalent gate today; the pragmatic prevention is the single-helper-function refactor above (removes the possibility of drift rather than detecting it after the fact).

H2. Client-side ELO delta preview uses stale rating constants — confirmed wrong in two live cases ​

  • TS (preview, actively rendered): packages/app/src/domain/rules/elo.rules.ts:11-39 (K_FACTOR, K_FACTOR_PROVISIONAL, K_FACTOR_HIGH_RATED, K_FACTOR_STRIKE, PROVISIONAL_THRESHOLD, HIGH_RATED_THRESHOLD, UPSET_K_MULTIPLIER, PEAK_FLOOR_PERCENTAGE, RD_SHRINK, RD_MIN, STRIKE_THRESHOLD), consumed live by packages/app/src/presentation/hooks/use-elo-delta-preview.ts and packages/features/sessions/src/scorecard/match-profiles.tsx to show "win +X / lose −Y" before a match is scored.
  • SQL (authoritative, applies the real rating change): supabase/migrations/00590_apply_match_ratings_decomposed.sql — the same constant set, re-declared as local CONSTANT values in several helper functions (private.match_rating_score_factor, private.match_rating_base_k, the per-player delta function around line 354-464).
  • Confirmed drift, not hypothetical: 00590 (via 00469's fix) added MARGIN_SUPPRESSION_GAP = 300 (line 262 — suppresses the score-factor run-up bonus once the effective rating gap exceeds 300, an anti-margin-gaming fix) and STRIKE_REARM_WINDOW = 5 / STRIKE_REARM_SUM_THRESHOLD = 120 (lines 362-363 — re-applies the accelerated strike K-factor to an established player on a hot streak). elo.rules.ts's getScoreFactor/getKFactor/calculateEloV2 have no equivalent logic — confirmed by reading both functions side by side. So today, for any match where the two effective ratings differ by >300, or where an established player (25+ matches) is mid a 5-match hot streak summing >120, the client's pre-match "win +X/lose −Y" preview computes a materially different number than what the server will actually apply.
  • Fix: either (a) make the preview call a STABLE SQL RPC that runs the real helper functions read-only (no ratings write), or (b) port the margin-suppression and strike-rearm logic into elo.rules.ts and add a parity test that runs both the TS and a supabase db local instance's SQL function against the same fixture set and asserts equal deltas. Given this is a genuinely dual-language computation (Deno/TS client vs Postgres), a shared literal module is not possible — the fix must be either "one side calls the other" or "a parity test that fails loudly on any future edit to either side."
  • Prevention: name the class ("a rating/scoring constant declared in both a TS domain/rules file and a SQL migration") and add a CI check that greps both for the constant names (K_FACTOR, PROVISIONAL_THRESHOLD, etc.) and fails if one side has a name the other doesn't — cheap, mechanical, and would have caught this the day 00469/00590 shipped.

H3. Push-notification frequency caps hardcoded inline with no config table [agent: presentation/features + supabase/functions audits corroborate] ​

  • File: supabase/migrations/00166_push_frequency_caps.sql:71,82,124,130 — global cap < 10 pushes/24h, category cap < 3 pushes/24h, both INTERVAL '24 hours', all as bare literals directly in a WHERE clause (not even named CONSTANTs the way apply_match_ratings does in the same codebase).
  • Duplication: the edge function supabase/functions/send-push/index.ts:26 independently hardcodes BATCH_LIMIT = 100, matching the RPC's own batch_limit INT DEFAULT 100 default that is re-declared identically across seven migration snapshots (00114, 00166, 00183, 00194, 00226, 00287, 00444) of signals_pick_pushable. The edge function always overrides the default explicitly, so the two 100s are currently inert but nothing enforces they stay equal if either is tuned independently.
  • Why it's a real policy, not a constant: notification frequency tolerance is exactly the kind of thing product/growth would want to tune (raise the cap during a launch push, lower it if users complain about noise) — and today doing so means finding and editing a WHERE ... < 10 deep inside a 140-line PL/pgSQL function.
  • Fix: a notification_policy (or app_config) table row read at the top of signals_pick_pushable, or at minimum named CONSTANTs at the top of the function matching the discipline already used in apply_match_ratings in the same migration set.
  • Prevention: no lint rule reaches into SQL bodies; the practical guard is a code-review convention (this repo already documents one: "every SECURITY DEFINER function... explicit GRANT" — add "every numeric policy threshold is a named CONSTANT at the top of the function, never inline in a WHERE" as a matching AGENTS.md/canon rule).

H4. Rate-limit values scattered as bare literals across ~15 migration call sites, with one already-occurred regression ​

  • Files (representative, not exhaustive): private.check_rate_limit(user, action, max_calls, window_seconds) is a well-designed generic rate limiter (good pattern — the mechanism itself is config-driven per call). But every call site hardcodes its own numbers inline with no registry: 00070_rpc_rate_limits.sql:116 ('submit_score', 30, 60), 00292_dm_group_membership.sql:87,171 (5, 60 / 3, 60), 00294_dm_block_and_report.sql:153 (10, 3600), 00295_dm_reactions.sql:65 (60, 60), 00541_social_mutation_rate_limits.sql:108,154,206,252,298 (15,3600 / 40,3600 / 30,86400 / 5,86400 / 20,86400), 00603_venue_picker_evidence_boundary.sql:209,271 (20, 3600), 00548_restore_score_submission_rate_limits.sql:104,155 (60, 60).
  • Already bit once: 00548_restore_score_submission_rate_limits.sql's own header says the submit_score/verify_score rate limit was "silently dropped since 00070" and had to be restored — i.e. this exact class of literal (a rate-limit threshold with no registry to audit against) has already caused a real regression in this codebase, not a hypothetical one.
  • Fix: a rate_limit_policies(action TEXT PRIMARY KEY, max_calls INT, window_seconds INT) table that check_rate_limit joins against instead of taking max_calls/window_seconds as caller-supplied literals — turns ~15 scattered call-site literals into one auditable table, and makes "what are all our rate limits" a single SELECT * instead of a repo-wide grep.
  • Prevention: a pgTAP test enumerating expected (action, max, window) triples against the live table would catch a silent drop immediately (this is exactly the gap that let 00548's regression go unnoticed until someone hit it). Mechanical lint: not feasible for SQL literals today.

H5. Edge-function rate-limit ceilings and per-invocation cron work-budgets hardcoded independently of their own DB-side timeout [agent: supabase/functions audit] ​

  • supabase/functions/static-map/index.ts:63 — STATIC_MAP_RATE_LIMIT_MAX_CALLS = 120 (per-IP req/min), no config-table backing; changing it needs a redeploy, not an ops toggle.
  • supabase/functions/search-venues/index.ts:166 — inline 60 passed as the max-calls argument to checkServiceRoleRateLimit(..., 'search-venues', 60).
  • supabase/functions/corroborate-venues/refresh-worker.ts:18-21 — APPLY_HEADROOM = 20_000, MAX_ACTIONS = 120, MAX_PREPARE_PAGES = 32 (how much corroboration work fits in one cron invocation) vs. supabase/migrations/00453_corroborate_venues_cron.sql:26 — timeout_milliseconds := 150000 (the cron's own wall-clock budget). Both encode the same underlying constraint ("how much work fits in one run") from two unrelated files with no shared derivation; a future cron reschedule (shrinking the timeout) could silently starve MAX_ACTIONS without anyone revisiting the edge function.
  • Fix: same shape as H3/H4 — an edge_rate_limits(action, max_calls, window_seconds) table read once per cold start for the rate-limit ceilings; for the cron/edge budget pair, derive MAX_ACTIONS from the cron's own timeout_milliseconds (pass it as an invocation parameter) rather than keeping two independent numbers in sync by hand.
  • Prevention: a rule flagging a bare-int literal passed as the 3rd/4th argument to checkServiceRoleRateLimit/check_rate_limit_for_key in supabase/functions/** is mechanically feasible (custom ESLint rule scoped to that call signature).

MEDIUM ​

M1. Search-debounce reinvented with drifting literals, bypassing the existing TIMING.SEARCH_DEBOUNCE_MS [agent: presentation/features audit] ​

  • TIMING.SEARCH_DEBOUNCE_MS = 400 already exists at packages/app/src/config/constants.ts:8 as the designated home.
  • Bypassed with a bare 300 in three files: packages/features/clubs/src/settings/club-guests/invite-guest-modal.tsx:44, packages/features/clubs/src/members/nominate-guest-modal.tsx:57, packages/features/profile/src/add-friend-screen.tsx:126 — identical setTimeout(() => setDebouncedQuery(next), 300) pattern.
  • Bypassed with a different bare 350 in two more: packages/features/clubs/src/create-club-step1.tsx:200, packages/features/clubs/src/create-club/step1/decor-section.tsx:152.
  • Fix: extract a shared useDebouncedQuery(value, TIMING.SEARCH_DEBOUNCE_MS) hook (this is a reuse gap, not just a constant swap — five call sites hand-roll the same setTimeout/clearTimeout pair).
  • Prevention: a lint rule banning a bare numeric-literal second argument to setTimeout inside packages/features/** (mirrors the existing no-numeric-spacing-in-features pattern already in the custom plugin) would catch this class mechanically.

M2. use-session-weather.ts and use-profile-search.ts bypass existing TIMING/STALE_TIME homes [agent: presentation/features audit] ​

  • packages/app/src/presentation/hooks/queries/use-session-weather.ts:110,235 — bare gcTime: 1000 * 60 * 60 (1 hour) twice in the same file, while TIMING.ONE_HOUR_MS already exists for exactly this value.
  • packages/app/src/presentation/hooks/queries/use-profile-search.ts:16 — staleTime: 30_000 doesn't match any named STALE_TIME bucket in packages/app/src/query-keys.ts:17-29 (realtime=60s/frequent=2m/standard=5m/stable=15m) — an orphan value with no declared home.
  • Fix: gcTime: TIMING.ONE_HOUR_MS at both sites; either add a new named STALE_TIME bucket for the 30s case or promote the local override to a documented module constant.
  • Prevention: lint rule banning numeric-literal gcTime/staleTime values that arithmetically equal an existing TIMING/STALE_TIME constant (mechanical: precompute the constant values, flag any literal matching one).

M3. Pagination/page-size literals independently reinvented across ~8 hook files, no shared config [agent: presentation/features audit] ​

  • Page size 50: POST_COMMENT_PREFETCH_PAGE_SIZE (prefetch.ts:41), SIGNAL_FEED_PAGE_SIZE (use-signals.ts:25), DM_THREAD_PAGE_SIZE/DM_MESSAGE_PAGE_SIZE (use-dm.ts:14-15), COMMENT_PAGE_SIZE (use-posts.ts:17).
  • Page size 20/30: DISCOVER_CLUBS_PAGE_SIZE (use-discover-clubs.ts:12), OPEN_PICKUPS_PAGE_SIZE (use-open-pickups.ts:32), use-profile-search.ts:11 default param, VENUE_DIRECTORY_PAGE_SIZE (use-venue-directory.ts:57) vs. a completely unnamed triplicated limit: 30 in use-venue-owner-claim-history.ts:103,123,143.
  • use-sessions.ts:37,62 — bare limit: 62 duplicated twice in one file with a comment explaining it's "~31/month × 2" but no extracted constant.
  • Severity note: each is individually named (except the venue-owner-claim-history triplicate), so this is a design-system gap rather than a single bug — but changing "how many items load per page" today means touching 8+ independently-declared constants that all happen to agree by coincidence, not by reference.
  • Fix: a shared PAGE_SIZE.{small,medium,large} (or similarly named) table in packages/app/src/config/, and at minimum hoist the use-venue-owner-claim-history.ts and use-sessions.ts unnamed repeats to a local constant.
  • Prevention: none mechanical for the cross-file case (values are named, so a literal-matching lint rule won't fire); the venue-owner-claim-history in-file triplicate is catchable by a "repeated numeric literal within one file" rule.

M4. Season activity multiplier hardcoded to Korean climate, with no market-aware home ​

  • packages/app/src/domain/rules/progression.rules.ts:17-30 — SEASON_MULTIPLIERS (a fixed Jan-Dec table, e.g. 7: 0.7 for Korean summer heat, 1: 0.75 for winter) drives calculateAdaptiveChallengeTarget.
  • Why it qualifies: the codebase already has a precedent for exactly this shape of variation — market.config.ts (measurement system, week start) and its domain-layer sibling rating-region.config.ts both key market conventions off a market code. A Southern-hemisphere or non-Korean market would have its "low activity" months flipped (e.g. July/August are peak season, not off-season) — this table has no market key at all, unlike its siblings.
  • Severity: low urgency today (single-market product per the roadmap), but it is the kind of literal that "an environment could reasonably vary" — flagged per the audit's own stated bar rather than because it's an active bug.
  • Fix: when/if a second market ships, key SEASON_MULTIPLIERS off market the same way getMarketConventions does; until then, a one-line comment noting the Korea-only assumption (there already is one) is sufficient — no urgent change needed.

M5. Home-screen proximity windows are inlined domain literals with no shared domain-layer home (architecture gap, not a simple "use existing config" fix) ​

  • packages/app/src/domain/rules/user-state.rules.ts:161-162 — POST_GAME_WINDOW_MS = 4 * 60 * 60 * 1000 (4h), RECAP_WINDOW_MS = 24 * 60 * 60 * 1000 (24h); :201 — imminent-session threshold < 120 minutes inline; :255-258 — near-future range days >= 2 && days <= 7 inline; :289-291 — player-maturity thresholds (1-9 beginner / 10-49 regular / 50+ veteran) inline.
  • Why these can't just import TIMING: the file's own comment (line 14-18) explains date helpers were inlined here specifically because domain/ cannot import from @/config (ARCH-1, zero-outer-imports) — so even though TIMING.DEADLINE_URGENCY_MS already models a structurally identical "24h window" concept, this file architecturally cannot reach it. The result is that every domain rule file re-invents its own local time-window literals rather than sharing one domain-layer constants module.
  • Fix: not "import TIMING" (blocked by design) — instead, a domain-layer constants file (sibling to tier.config.ts/rating-region.config.ts, e.g. home-state.config.ts) that other domain rules can import without crossing the outer-layer boundary, so a future domain file needing "4 hours" doesn't reinvent it a third time.
  • Prevention: none mechanical; this is an architecture-shape gap (no domain-layer analog of config/constants.ts) rather than a bypass of an existing one.

Verified — NOT findings (checked, already governed correctly) ​

  • deriveRecruitmentStatus almost-full/quorum thresholds (domain/utils/recruitment-status.ts) — named ALMOST_FULL_MIN_CAPACITY/ALMOST_FULL_SPOTS_RATIO exports, single source, already the canon example.
  • feature-entitlements.ts per-plan limits (clubsMax, membersPerClubMax, etc.) — correctly table-shaped, the right pattern to point at.
  • clubs.max_members, clubs.attendance_cancel_window_hours, clubs.host_fee_exempt, sessions.payment_hold_minutes, clubs.guest_visit_limit — all real per-club/per-session DB columns already backing the corresponding policy; not literals.
  • Naver API daily budgets (NAVER_SEARCH_DAILY_BUDGET etc.) — already env-var-driven with only the fallback default as a literal; acceptable.
  • ELO tier bands, K-factor family constants, guest-priority weights, season-award thresholds — all centralized as named, documented module constants (not scattered), even where not club-configurable; they meet the "single source of truth" bar even if not database-configurable.

Summary ​

#FindingSeverityClass
C1Attendance cancel 12h boundary vs. per-club attendance_cancel_window_hoursCriticallatent bug from hardcoding
H1Attendance strike window(20)/flag(3) — 2 TS + 8 SQL copiesHighduplication
H2ELO preview (TS) vs. live rating engine (SQL) constant driftHighduplication, confirmed stale
H3Push frequency caps inline, no config tableHighmissing config home
H4Rate-limit values scattered, one past regressionHighmissing config home
H5Edge rate-limit ceilings + cron/edge budget pairHighmissing config home
M1Search-debounce reinvented (300/350 vs TIMING=400)Mediumbypassed existing config
M2weather gcTime / profile-search staleTime bypass TIMING/STALE_TIMEMediumbypassed existing config
M3Page-size literals reinvented ~8xMediummissing shared config
M4Season multiplier hardcoded to KoreaMediumenvironment-variance gap
M5Home proximity windows inlined, no domain-layer config homeMediumarchitecture gap

10 real findings (1 critical, 5 high, 5 medium... wait 11 total — recount is fine, see table). Two background sub-audits' full findings are preserved above with [agent] tags; nothing from them was dropped, only de-duplicated against my own independent findings (H2/H3 overlap was merged rather than double-counted).

Markdown remains the source of truth. Run yarn docs:check before handoff.