feat/discovery

Clicked cards have no upper bound — permanently-failing actions cause infinite reappearance

domains/cross-domain/src/feature-discovery.decisions.ts:151

Once clickedAt is set, the seenCount >= RETIRE_AFTER_UNCLICKED_SHOWS guard is permanently bypassed. If the downstream action keeps failing (e.g., a persistently broken AWS endpoint, plan limitation), the card reappears every day with no escape short of explicit dismissal. Consider a RETIRE_AFTER_TOTAL_SHOWS cap (e.g., 10) applied unconditionally to bound the zombie-card scenario, or at minimum document that dismissal is the intended escape hatch in this scenario.

Test change from 'internal_site_ids' to 'monitoring' could mask a priority-ordering regression

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts:195

The old test verified priority fallthrough (dismissal retires 'monitoring' → 'internal_site_ids' is returned). The new test only verifies that clicking doesn't retire. No test explicitly covers that dismissal-triggered fallthrough still works correctly after this change. The existing dismissal retirement test covers it implicitly, but a targeted assertion would be clearer.

isBudgetSpent relies on 'seen' event preceding 'clicked' — no enforcement of ordering

domains/cross-domain/src/feature-discovery.decisions.ts:158

The 20h cooldown for a clicked-but-failed card depends on a 'seen' outcome having been recorded before 'clicked'. If a client sends 'clicked' without a preceding 'seen' (race condition or replay), lastSeenAt remains stale and the pacing guard may not block re-display in the same session. Pre-existing gap, but amplified now that clicks no longer retire cards.

Schema comment updated correctly to match new semantics

packages/database/src/schema/discovery-state.ts:14

Comment now accurately reflects the new retirement rules. No action needed.

Non-admin user could set clickedAt on adminOnly key if shown via stale client state

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts

adminOnly filtering is enforced in decideSpotlight (pure decision). If a non-admin user somehow sends a 'clicked' outcome for an adminOnly key (e.g., via direct API call or stale client render), their discovery state would record clickedAt. The card now never retires until dismissal. Impact is cosmetically scoped to their own UX state — no privilege escalation to the underlying feature — but worth verifying that the click outcome endpoint validates admin status before recording clickedAt.

No new PII or sensitive data introduced by this change

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts

The isRetired change and new tests operate entirely on DiscoveryState fields (clickedAt, seenCount, dismissCount). No user PII, credentials, or sensitive business data is introduced.

Schema comment continuation line missing JSDoc asterisk prefix

packages/database/src/schema/discovery-state.ts:15

The continuation of the added comment uses a leading space instead of ' * ' prefix, breaking the JSDoc block style used everywhere else. Should be: ` * (adoption ends a card naturally; a failed action must be retryable)`.

Regression test uses wrong key — cfe_setup is the actual prod bug, monitoring doesn't reproduce it

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts:189

The production bug was 'clicked cfe_setup + AWS failure bricked the whole surface'. cfe_setup is the adminOnly gate signal — its retirement suppresses everything else. The test uses 'monitoring', which is not a gate signal and doesn't reproduce the full failure path. A cfe_setup-keyed test case with clickedAt set should be added to prevent regressions of the specific live incident.

Missing test: dismissed AND clicked key — dismissCount threshold must still retire regardless of clickedAt

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts

isRetired returns true when dismissCount >= RETIRE_AFTER_DISMISSALS regardless of clickedAt. This edge case — a card that was clicked AND then dismissed N times — is the load-bearing invariant separating 'permanent exit via dismissal' from 'temporary exit via click'. Without a test, a future refactor adding clickedAt short-circuit logic could inadvertently exempt clicked keys from dismissal-based retirement.

No test: clicked card + fresh lastSeenAt still blocked by daily budget

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts

There is no test verifying that a clicked card with a fresh lastSeenAt (within the 20h pacing window) is correctly suppressed by the budget guard. If the budget check is accidentally skipped for clicked keys, the spotlight could show every page load after a failed action. Stale vs fresh lastSeenAt cases for clicked cards should be explicitly covered.

listEligibleSpotlights not tested with new clicked-card-still-eligible semantics

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts

The diff only changes decideSpotlight tests. listEligibleSpotlights (which powers the gap badge count in the Descubre hub) uses the same isRetired predicate. If a stale call-site copy exists or the predicate is re-evaluated differently, the old retire-on-click behavior could persist in the badge while the spotlight itself is fixed. A test confirming a clicked card still appears in the eligible list should be added.

Shows-based guard exemption test lacks a counterpart asserting that the same seenCount without clickedAt would retire

domains/cross-domain/src/__tests__/feature-discovery.decisions.test.ts:196

The 'clicked keys are exempt from shows-based guard' test proves the exemption holds but not that it's actually doing anything. Without a companion case (same seenCount, clickedAt=null, key is retired), the test would pass even if the seenCount guard was inadvertently removed.

Missing isAdopted abstraction makes the retirement invariant comment-enforced, not structural

domains/cross-domain/src/feature-discovery.decisions.ts

The contract 'adoption is detected via live state, not clickedAt' is enforced only by the absence of a clickedAt check and a comment. A future contributor adding a new key could re-introduce clickedAt-based retirement without violating any type contract. Extracting an isAdopted(key, signals) predicate and having isRetired not depend on clickedAt would make this invariant structural.

Comment should explicitly call out that shows-based guard is suspended once clicked

domains/cross-domain/src/feature-discovery.decisions.ts:148

The updated comment says 'Clicking NEVER retires a key' but doesn't state the corollary: once clicked, the seenCount cap is inert, and the only retirement path is dismissal. Adds a sentence like: 'A clicked card ignores the unclicked-shows cap and only retires via RETIRE_AFTER_DISMISSALS dismissals.'

RETIRE_AFTER_UNCLICKED_SHOWS constant name remains precise after the change

domains/cross-domain/src/feature-discovery.decisions.ts:135

The constant name accurately describes its new semantics (only applies when the card was never clicked). No rename needed.