Skip to content

feat(activation): record a browser-blocked step in its own durable bucket (#5617)#5629

Merged
koala73 merged 3 commits into
mainfrom
fix/5617-activation-blocked-outcome-bucket
Jul 26, 2026
Merged

feat(activation): record a browser-blocked step in its own durable bucket (#5617)#5629
koala73 merged 3 commits into
mainfrom
fix/5617-activation-blocked-outcome-bucket

Conversation

@koala73

@koala73 koala73 commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Closes #5617.

The gap

PR #5615 routed a denied notification permission into the activation wizard's blocked state and closed the resulting signal loss at the event level (pro-activation-step-blocked). The durable side stayed collapsed: a blocked step landed in skippedSteps with no marker, so anything querying persisted per-account state in Convex after the fact could not tell a browser refusal from a voluntary skip.

That is what the data is actually for — sizing the push-denial population, and deciding whether re-prompting an account is worth anything (it never is, once permission is denied).

Shape

A fourth bucket rather than a widening of an existing one, so behavioral consumers are untouched:

  • ActivationStepOutcome gains blocked. It reads as pending in the summary (never failed — we never attempted a write), counts as pending in the exit funnel event, and still shows the finish-setup chip. A record change, not a UX change.
  • buildActivationOutcomeBuckets gains blockedSteps and now routes every outcome explicitly: its else catch-all would have persisted the new outcome as a failed write.
  • New pure selectAdvanceOutcome(state) is the single seam for all three ways a step is advanced past without a confirm (Continue, Escape/dismiss, never reached). The issue proposed writing blocked only on the advance-skip arm; that would have made blockedSteps a sample of denied users who clicked Continue. Steps blocked at mount — permission already denied before the wizard opened — record identically.
  • recordProActivationOutcome + schema accept blockedSteps, validated for disjointness like every other bucket.

Deviations from the issue's proposed shape

Both widen correctness rather than scope; flagging them explicitly:

  1. Mount-blocked steps also record as blocked (the issue scoped this to blockedByConfirm). They are the same denial cohort, and likely its larger half.
  2. step-skipped is no longer emitted for a denial. The durable buckets and the funnel were counting different cohorts — skippedSteps excluded denials while the funnel's skip count included them. Each step now emits exactly one event. This shifts the pro-activation-step-skipped baseline down by the denial cohort. It also fixes a gap predating this issue: a step blocked at mount previously emitted only stepSkipped, so that cohort was invisible on the event stream entirely.

Rollout ordering (please read before merging)

This is a server-contract change, and Convex deploys through a separate workflow (convex-deploy.yml) from the Vercel frontend. The mutation arg is v.optional, which covers old-client-to-new-backend. The reverse — new client, not-yet-updated backend — rejects the write for the duration of the skew window. It is best-effort and retried, and now reports to Sentry instead of failing silently, but the Convex deploy should land before the Vercel promotion. Three reviewers independently flagged this; the conservative alternative is to split this into two merges (schema+mutation first, client second) if you'd rather not rely on ordering.

Schema rollback is one-way: once rows carry blockedSteps, deleting the field fails the Convex deploy. Revert the write path, not the schema line (noted in-code).

Verification

  • typecheck clean; biome clean on all changed files; e2e and convex tsc projects show zero new errors (e2e has 22 pre-existing page.evaluate resolution errors, unchanged).
  • e2e: 24/24 pass (real browser). Two assertions in this spec would have gone red — npm run typecheck does not cover e2e/, whose tsconfig includes only src.
  • convex: 1006 tests; tsx: 16748. Two pre-existing failures unrelated to this change: the dist/dashboard.html built-output CSS tests (need vite build first) and a load-sensitive poolSelection perf-shape timeout verified to fail identically on a clean tree.
  • All 15 mutations of the new logic were verified to redden a test, including the else catch-all, the mid-flow flush, and both event-routing paths.

Review

Reviewed by an 11-reviewer pass (correctness, security, adversarial, api-contract, data-migration, reliability, testing, maintainability, standards + always-on). Security returned no findings. Three real defects were found and fixed in the second commit — two CI-red e2e assertions and an unflushed mid-flow denial. The cross-model pass could not run (the local Codex CLI install is broken).

Adjacent work: #5621 has an in-flight branch touching recordProActivationOutcome; rebase order may matter.

https://claude.ai/code/session_011wvTSMvKG7hLkJgRezBJFi

koala73 added 2 commits July 26, 2026 00:57
…cket (#5617)

PR #5615 made a denied notification permission move the activation wizard's
alerts step into its blocked state, and closed the resulting signal loss at the
EVENT level (`pro-activation-step-blocked`). The durable side stayed collapsed:
a blocked step landed in `skippedSteps` with no marker, so anything querying
persisted per-account state in Convex after the fact could not tell a browser
refusal from a voluntary skip -- which is exactly what sizing the push-denial
population and deciding whether re-prompting an account is worth anything both
need.

Adds a fourth bucket rather than widening an existing one, so behavioral
consumers are untouched:

- `ActivationStepOutcome` gains `blocked`. It maps to `pending` in the summary
  (never `failed` -- we never attempted a write), counts as `pending` in the
  exit event, and still shows the finish-setup chip. A record change, not UX.
- `buildActivationOutcomeBuckets` gains `blockedSteps` and now routes every
  outcome explicitly: its `else` catch-all would have persisted the new outcome
  as a failed write.
- `selectStepEvent('blocked')` returns `stepBlocked`, not `stepSkipped`.
- New pure `selectAdvanceOutcome(state)` is the single seam for all three ways
  a step can be advanced past without a confirm (Continue, Escape/dismiss,
  never reached). Writing `blocked` on only the Continue arm would have made
  the bucket a sample of "denied users who clicked Continue". Steps blocked at
  mount now record identically to those blocked mid-flow.
- `recordProActivationOutcome` and the schema accept `blockedSteps`: optional
  server-side for mixed deploys, validated for disjointness like every other
  bucket, and written on every snapshot (each one is a full replacement).
- `ProActivationFlowDependencies.recordOutcome` now uses the service's own
  `ProActivationOutcomeSnapshot` instead of a hand-copied structural shape that
  silently described less the moment a bucket was added.

All 11 mutations of the new logic were verified to redden a test.

Claude-Session: https://claude.ai/code/session_011wvTSMvKG7hLkJgRezBJFi
…5617)

Code-review follow-ups on the durable `blocked` bucket. Three of these were
real defects in the first commit, two of them CI-red:

- e2e/pro-activation.spec.ts asserted the exact outcome payload in two places
  (`toEqual` on the snapshot object, and `['skipped','skipped','skipped']` for
  the denied-permission flow). Both would have gone red. `npm run typecheck`
  does not cover e2e/ — its tsconfig includes only `src` — so nothing local
  caught this; the spec now passes (24/24, run against a real browser).
- A mid-flow denial was never flushed. `handleConfirm`'s `blocked` arm updated
  in-memory state but, unlike the `verified` arm, never called
  `recordProgress()`. A subscriber denied by the browser who then closed the
  tab stayed recorded as a voluntary skip forever — precisely the cohort this
  bucket exists to size. The denial is now durable the instant it happens,
  which required raising MAX_PRO_ACTIVATION_OUTCOME_REVISION to
  `members.length + 2` (one extra progress write is now reachable).
- The durable buckets and the funnel events counted different cohorts: a
  blocked step recorded as `blocked` but still emitted `stepSkipped`, so the
  funnel's skip count included denials while `skippedSteps` excluded them. A
  new `reportAdvanceEvent` seam routes the event off the resolved outcome, so
  each step emits exactly one: `stepSkipped` for a real skip, `stepBlocked` for
  a refusal. This also closes a gap that predates #5617 — a step blocked at
  MOUNT previously emitted only `stepSkipped`, leaving that cohort invisible on
  the event stream entirely. NOTE: this shifts the `step-skipped` baseline down
  by the denial cohort. That is the intended correction, not a side effect.

Also:
- A permanently failing outcome write now reports to Sentry instead of only
  `console.warn` — an invisible durable-write failure is the exact blind spot
  #5621 is filed about.
- The mutation patches `blockedSteps` straight from the arg rather than
  coercing to `[]`. Convex removes a field patched as `undefined`, so a client
  too old to report blocked steps leaves the field ABSENT ("could not report")
  instead of asserting "none were blocked" — a claim that is false for a client
  that classified denials as skips.
- ProActivationChip re-derived outcomes with its own ternary; it now shares
  `selectAdvanceOutcome`. Behavior is identical today, which is exactly how the
  two would have drifted.
- Schema rollback note: once rows carry `blockedSteps`, deleting the field
  fails the Convex deploy. Revert the write path, not the schema line.
- Four comments still claimed a blocked step resolves as `skipped`.

Tests: +5 (mid-flow flush through the real shell, Sentry on permanent failure,
progress-revision persistence, omit-clears-rather-than-strands, revision cap
top-of-range). All 15 mutations of the new logic verified to redden a test.

Claude-Session: https://claude.ai/code/session_011wvTSMvKG7hLkJgRezBJFi
@vercel

vercel Bot commented Jul 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldmonitor Ready Ready Preview, Comment Jul 26, 2026 6:42am

Request Review

@mintlify

mintlify Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
WorldMonitor 🟢 Ready View Preview Jul 26, 2026, 6:34 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@koala73
koala73 merged commit 405c93a into main Jul 26, 2026
27 checks passed
@koala73
koala73 deleted the fix/5617-activation-blocked-outcome-bucket branch July 26, 2026 06:42
@mintlify

mintlify Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
WorldMonitor 🟡 Building Jul 26, 2026, 6:32 AM

💡 Tip: Enable Workflows to automatically generate PRs for you.

benngee85 pushed a commit to benngee85/VISTA that referenced this pull request Jul 26, 2026
…cket (koala73#5617) (koala73#5629)

* feat(activation): record a browser-blocked step in its own durable bucket (koala73#5617)

PR koala73#5615 made a denied notification permission move the activation wizard's
alerts step into its blocked state, and closed the resulting signal loss at the
EVENT level (`pro-activation-step-blocked`). The durable side stayed collapsed:
a blocked step landed in `skippedSteps` with no marker, so anything querying
persisted per-account state in Convex after the fact could not tell a browser
refusal from a voluntary skip -- which is exactly what sizing the push-denial
population and deciding whether re-prompting an account is worth anything both
need.

Adds a fourth bucket rather than widening an existing one, so behavioral
consumers are untouched:

- `ActivationStepOutcome` gains `blocked`. It maps to `pending` in the summary
  (never `failed` -- we never attempted a write), counts as `pending` in the
  exit event, and still shows the finish-setup chip. A record change, not UX.
- `buildActivationOutcomeBuckets` gains `blockedSteps` and now routes every
  outcome explicitly: its `else` catch-all would have persisted the new outcome
  as a failed write.
- `selectStepEvent('blocked')` returns `stepBlocked`, not `stepSkipped`.
- New pure `selectAdvanceOutcome(state)` is the single seam for all three ways
  a step can be advanced past without a confirm (Continue, Escape/dismiss,
  never reached). Writing `blocked` on only the Continue arm would have made
  the bucket a sample of "denied users who clicked Continue". Steps blocked at
  mount now record identically to those blocked mid-flow.
- `recordProActivationOutcome` and the schema accept `blockedSteps`: optional
  server-side for mixed deploys, validated for disjointness like every other
  bucket, and written on every snapshot (each one is a full replacement).
- `ProActivationFlowDependencies.recordOutcome` now uses the service's own
  `ProActivationOutcomeSnapshot` instead of a hand-copied structural shape that
  silently described less the moment a bucket was added.

All 11 mutations of the new logic were verified to redden a test.

Claude-Session: https://claude.ai/code/session_011wvTSMvKG7hLkJgRezBJFi

* fix(activation): close the review gaps on the blocked outcome bucket (koala73#5617)

Code-review follow-ups on the durable `blocked` bucket. Three of these were
real defects in the first commit, two of them CI-red:

- e2e/pro-activation.spec.ts asserted the exact outcome payload in two places
  (`toEqual` on the snapshot object, and `['skipped','skipped','skipped']` for
  the denied-permission flow). Both would have gone red. `npm run typecheck`
  does not cover e2e/ — its tsconfig includes only `src` — so nothing local
  caught this; the spec now passes (24/24, run against a real browser).
- A mid-flow denial was never flushed. `handleConfirm`'s `blocked` arm updated
  in-memory state but, unlike the `verified` arm, never called
  `recordProgress()`. A subscriber denied by the browser who then closed the
  tab stayed recorded as a voluntary skip forever — precisely the cohort this
  bucket exists to size. The denial is now durable the instant it happens,
  which required raising MAX_PRO_ACTIVATION_OUTCOME_REVISION to
  `members.length + 2` (one extra progress write is now reachable).
- The durable buckets and the funnel events counted different cohorts: a
  blocked step recorded as `blocked` but still emitted `stepSkipped`, so the
  funnel's skip count included denials while `skippedSteps` excluded them. A
  new `reportAdvanceEvent` seam routes the event off the resolved outcome, so
  each step emits exactly one: `stepSkipped` for a real skip, `stepBlocked` for
  a refusal. This also closes a gap that predates koala73#5617 — a step blocked at
  MOUNT previously emitted only `stepSkipped`, leaving that cohort invisible on
  the event stream entirely. NOTE: this shifts the `step-skipped` baseline down
  by the denial cohort. That is the intended correction, not a side effect.

Also:
- A permanently failing outcome write now reports to Sentry instead of only
  `console.warn` — an invisible durable-write failure is the exact blind spot
  koala73#5621 is filed about.
- The mutation patches `blockedSteps` straight from the arg rather than
  coercing to `[]`. Convex removes a field patched as `undefined`, so a client
  too old to report blocked steps leaves the field ABSENT ("could not report")
  instead of asserting "none were blocked" — a claim that is false for a client
  that classified denials as skips.
- ProActivationChip re-derived outcomes with its own ternary; it now shares
  `selectAdvanceOutcome`. Behavior is identical today, which is exactly how the
  two would have drifted.
- Schema rollback note: once rows carry `blockedSteps`, deleting the field
  fails the Convex deploy. Revert the write path, not the schema line.
- Four comments still claimed a blocked step resolves as `skipped`.

Tests: +5 (mid-flow flush through the real shell, Sentry on permanent failure,
progress-revision persistence, omit-clears-rather-than-strands, revision cap
top-of-range). All 15 mutations of the new logic verified to redden a test.

Claude-Session: https://claude.ai/code/session_011wvTSMvKG7hLkJgRezBJFi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Activation outcomes: a browser-blocked step is indistinguishable from a voluntary skip in the durable record

1 participant