Skip to content

fix(billing): short-cache planless customers to end post-checkout onboarding lockout - #123

Merged
Makisuo merged 5 commits into
mainfrom
jeremy/fix-onboarding-lockout-after-checkout
Jun 26, 2026
Merged

Makisuo merged 5 commits into
mainfrom
jeremy/fix-onboarding-lockout-after-checkout

Conversation

@JeremyFunk

@JeremyFunk JeremyFunk commented Jun 25, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

On the PR-preview deploy, a user who completed Stripe checkout (test card) was permanently bounced to /quick-start onboarding on every navigation, despite having a valid trial.

The root cause is a stale negative cache, not the checkout itself. After checkout, Autumn records the new subscription asynchronously. A customer with no active plan was cached for the full 5 minutes at two layers — the API per-org edge cache (getOrCreateCustomer) and the client react-query layer. The post-checkout return is a fresh page load, and the attach mutation that invalidates the cache already ran before the redirect to Stripe, so nothing re-invalidates on return. If the first post-return getOrCreateCustomer landed before Autumn finished syncing, the "no plan" snapshot stuck for 5 minutes and the __root.tsx plan gate re-bounced the user on every navigation.

This is pre-existing behavior, unrelated to the recharts upgrade, and it affects real paying customers — not just preview.

Fix

Treat a planless customer as an unsettled state that's cheap to re-check, at both layers:

  • API edge cache (autumn.http.ts): getOrCompute now accepts a content-dependent TTL. A confirmed active-plan customer keeps the full 300s; a planless or error-shaped 200 is cached for just 5s (CUSTOMER_CACHE_UNSETTLED_TTL_SECONDS).
  • Client (use-maple-customer.ts): dynamic staleTime (5 min when settled, 5s when planless) plus a self-stopping refetchInterval while planless, so the gate auto-recovers within seconds with no manual refresh. Polling stops the moment a plan is active and pauses while the tab is backgrounded.

Avoiding drift

The gate rule ("is this an actively selected paid plan?") would now live in two places — the web redirect gate and the API cache TTL. Extracted it into a single source of truth, @maple/domain/billing isActivePlanSubscription, structurally typed so autumn-js stays out of @maple/domain. Both plan-gating.ts (web) and autumn.http.ts (api) delegate to it.

Verification

  • bun typecheck — 23/23
  • Tests: domain billing (3), web plan-gating (19), api autumn (11), query-engine edge-cache (8)

🤖 Generated with Claude Code

…oarding lockout

After a user completes Stripe checkout, Autumn records the new subscription
asynchronously. A customer with no active plan was cached for the full 5 min at
both the API edge cache and the client react-query layer, so if the first
post-return getOrCreateCustomer landed in that window the "no plan" snapshot
stuck for 5 min and the /quick-start plan gate bounced the user on every
navigation.

- edge cache: getOrCompute accepts a content-dependent TTL; cache planless or
  error-shaped customer responses for 5s instead of 300s
- client: dynamic staleTime + a self-stopping refetchInterval while planless, so
  the gate auto-recovers within seconds without a manual refresh
- extract the active-plan predicate into @maple/domain/billing
  (isActivePlanSubscription) so the web redirect gate and the API cache TTL share
  one source of truth and can't drift

Tests: domain billing (3), web plan-gating (19), api autumn (11), query-engine
edge-cache (8). bun typecheck 23/23.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one informational observation below; mergeable as-is.

Reviewed changes — a fix for the post-checkout onboarding lockout where a stale "no plan" customer snapshot, cached for 5 min at both the API edge cache and the client react-query layers, kept bouncing a just-subscribed user to /quick-start until Autumn finished its async Stripe→subscription sync.

  • Content-dependent edge-cache TTL — getOrCompute's ttlSeconds now accepts number | ((value) => number), resolved once on write; autumn.http.ts gives active-plan 200s the full 300s and planless/error-shaped 200s just 5s (CUSTOMER_CACHE_UNSETTLED_TTL_SECONDS).
  • Self-stopping client poll — use-maple-customer.ts switches to function-form staleTime (5 min settled / 5s planless) plus a refetchInterval that polls every 5s while planless and returns false once a plan is active.
  • Shared anti-drift gate — extracted isActivePlanSubscription into @maple/domain/billing (structurally typed via PlanGatingSubscription so autumn-js stays out of @maple/domain); both web plan-gating.ts and api responseHasActivePlan delegate to it.
  • Tests — domain billing, api autumn (TTL policy via a recording backend), and edge-cache (function-form TTL resolves per computed value) all covered.

I verified the mechanism end-to-end against the actual dependency: autumn-js@1.2.29 bundles @tanstack/query-core@5.85.6, which honors function-form staleTime and refetchInterval and pauses the interval while the tab is unfocused, and its useCustomer spreads queryOptions straight into useQuery. The TTL function runs only on write, not on cache hits, and the API and web gates now share one predicate over the same structural shape — no drift. Implementation is sound.

ℹ️ Planless cohort now re-checks indefinitely, not just during the post-checkout window

The 5s unsettled TTL plus the client poll are exactly right for the brief Stripe→Autumn sync window. But the same path also catches every genuinely planless org — legacy-free tiers (isActivePlanSubscription returns false for free) and orgs that simply never subscribe. Those are now treated as permanently "unsettled": the client polls getOrCreateCustomer every 5s while foregrounded and the API edge cache holds them at a 5s TTL forever, versus the prior 5-min window.

In practice this is bounded — the root gate parks planless orgs on /quick-start, and the poll pauses when the tab is backgrounded — so it is not a correctness problem, just a sustained cost worth being aware of for any cohort expected to dwell in-app without a paid plan.

Technical details
# Planless cohort re-checks indefinitely

## Affected sites
- `apps/web/src/hooks/use-maple-customer.ts:30-32` — `staleTime`/`refetchInterval` key off `hasSelectedPlan`, which is `false` for legacy-`free` and never-subscribing orgs, so they poll every 5s indefinitely (foreground only).
- `apps/api/src/routes/autumn.http.ts:81-84` — same cohort caches at the 5s unsettled TTL forever, so each >5s client refetch that misses also re-hits upstream Autumn.

## Required outcome
- None required for correctness. Optionally confirm the never-subscribing / legacy-free cohort isn't expected to sit in-app long enough for the sustained 5s re-check (client + upstream) to matter.

## Open questions for the human
- Are legacy-`free` orgs a live cohort that dwells inside the app, or are they always redirected to `/select-plan` / `/quick-start` and effectively transient? If the former, a longer unsettled TTL (e.g. 15–30s) would cut the steady-state load while still resolving the post-checkout window fast.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏

@JeremyFunk JeremyFunk changed the title fix(billing): short-cache planless customers to end post-checkout onboarding lockout fix(onboarding): end post-checkout plan-gate lockout + signup dashboard flash (MAP-45) Jun 25, 2026

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — since the prior pullfrog review (ee3e57f9), this run covers the new signup-flash fix (MAP-45) that gates the __root.tsx optimistic dashboard render behind a per-org localStorage flag.

  • Per-org "seen a plan" flag — new selected-plan-cache.ts exposes hasKnownSelectedPlan / rememberSelectedPlan (keyed maple:billing:selected-plan:<orgId>), try/catch-wrapped and fail-closed on SSR / disabled / quota errors, with 8 covering tests.
  • Flag-writing effect — __root.tsx records the flag only on a settled, usable customer payload (skips while loading / on error / on unusable), so a transient blip can't flip it.
  • Loading-branch gate rework — while the customer query is loading, the dashboard renders only for ALLOWED_WITHOUT_PLAN paths or when hasKnownSelectedPlan(orgId) is set; otherwise it waits (return null) instead of flashing the dashboard before bouncing a fresh signup to /quick-start. The flag is cleared on settled-planless, so unsubscribe flashes exactly once.
  • Module-scope ALLOWED_WITHOUT_PLAN — hoisted out of the inner block (de-duped, no shadowing).

I traced the full gate: the fix #1 safety invariant is preserved — the loading branch never redirects, so the /quick-start bounce still fires only after the query settles (__root.tsx:166-167) and a paying user is never bounced before their plan is known. The quotaPreview path falls through to AppFrame unchanged, returning null from ClerkReverseRedirects is safe (Clerk providers live above; AppFrame only wraps AttributesProvider+Outlet), and the unsubscribe case flashes once then reverts to the wait path as documented. The prior review's lone informational observation (planless cohort re-checks indefinitely) is unchanged by these commits and remains mergeable as-is.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | Using Claude Opus | 𝕏

@JeremyFunk
JeremyFunk force-pushed the jeremy/fix-onboarding-lockout-after-checkout branch from 2806ecb to 8a7bbce Compare June 25, 2026 20:16
@JeremyFunk JeremyFunk changed the title fix(onboarding): end post-checkout plan-gate lockout + signup dashboard flash (MAP-45) fix(billing): short-cache planless customers to end post-checkout onboarding lockout Jun 25, 2026
@JeremyFunk

Copy link
Copy Markdown
Collaborator Author

@pullfrog pls merge main into this

@pullfrog

pullfrog Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Merged main into jeremy/fix-onboarding-lockout-after-checkout and pushed. Main had landed the typed /api/billing migration (retiring autumn.http.ts for billing.http.ts + the effect-atom billingCustomerAtom), which conflicted with this PR's autumn-js-based fix. Resolution:

  • billing.http.ts — ported this PR's content-dependent customer-cache TTL onto main's new file: responseHasActivePlan + CUSTOMER_CACHE_UNSETTLED_TTL_SECONDS (5s for planless/error-shaped 200s, full 300s once an active plan is confirmed), delegating to @maple/domain/billing isActivePlanSubscription. This preserves the core onboarding-lockout fix.
  • autumn.http.ts / autumn.http.test.ts — accepted main's deletion; the TTL-policy + responseHasActivePlan tests were re-pointed to billing.http.test.ts.
  • use-maple-customer.ts — took main's thin billingCustomerAtom accessor. The autumn-js per-hook retry/poll no longer exists post-migration; post-checkout recovery now rides the 5s API TTL on the next gate-driven refetch.
  • plan-gating.ts — merged imports (kept isActivePlanSubscription + main's typed @maple/domain/http types and isLegacyPlan helpers).

Verified: bun typecheck passes for all billing packages (the only failure is pre-existing, unrelated examples/effect-todo). Tests green — billing.http (9), plan-gating (30), domain billing (3).

Task list (4/4 completed)
  • Checkout PR branch and identify base branch
  • Fetch base branch and attempt merge
  • Resolve conflicts if any
  • Push and report

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | View workflow run | via Pullfrog | Using Claude Opus | 𝕏

Reconcile the planless short-cache fix with main's typed /api/billing
migration: port the content-dependent customer-cache TTL
(responseHasActivePlan + CUSTOMER_CACHE_UNSETTLED_TTL_SECONDS) onto
billing.http.ts, drop the retired autumn.http.ts proxy, and move
use-maple-customer.ts onto billingCustomerAtom.
Makisuo added a commit that referenced this pull request Jun 26, 2026
The "Grant runtime DB role" step fails on every PR preview with
`role "pscale_api_…" does not exist` — the ephemeral PlanetScale runtime role
isn't present when `db:grant` runs against the branch. It's been red repo-wide
since 2026-06-25 (PR #123 hit the same error). The grant is a no-op self-grant
for the MAPLE_PG_URL user anyway, so drop the step to unblock previews.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Makisuo added a commit that referenced this pull request Jun 26, 2026
* feat: diagnose scrape-target failures + auto-mitigate 429

Connecting PlanetScale would succeed on the initial test, then the scheduled
scrape would fail with HTTP 429 and sit at Down with an unhelpful, truncated
"target retur…" message and no remediation.

Two-part fix:

Backend (auto-mitigate): the scraper now backs off on a 429/503 instead of
hammering the rate-limited upstream every interval — exponential backoff capped
at 5 min, reset on the next success, honoring the upstream Retry-After header
when it is longer (propagated through the scrape proxy). PlanetScale's rate
limit clears on its own rather than the target staying Down.

Frontend (diagnose): a new scrape-error-diagnosis helper turns raw error
strings into a plain-language "what's wrong / how to fix" banner in the target
detail panel (PlanetScale-aware for 429/401/403/404/5xx/timeout/discovery,
generic fallback for Prometheus). Errors are no longer truncated away — full
text shows in the banner and on hover tooltips in the row + check history.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix: preserve start-to-start scrape cadence + simplify backoff calc

Address PR review:
- targetLoop now subtracts the scrape's elapsed time on the happy path so the
  period stays start-to-start at the configured interval (matching the prior
  Schedule.fixed), instead of scrapeDuration + interval. Backoff still runs the
  full delay from scrape end so Retry-After is honored. Added a slow-scrape
  timing test that pins the invariant.
- Drop the redundant baseMs floor in nextScrapeDelayMs (exponential is always
  >= baseMs) and correct the JSDoc.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* ci: drop the grant-runtime-role step from PR preview deploy

The "Grant runtime DB role" step fails on every PR preview with
`role "pscale_api_…" does not exist` — the ephemeral PlanetScale runtime role
isn't present when `db:grant` runs against the branch. It's been red repo-wide
since 2026-06-25 (PR #123 hit the same error). The grant is a no-op self-grant
for the MAPLE_PG_URL user anyway, so drop the step to unblock previews.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(ci): record why stg keeps db:grant but PR preview drops it

Pullfrog flagged the undocumented asymmetry. Add a note to deploy-stg.yml so a
future reader doesn't "clean up" the stg grant step the way PR preview did:
PR previews are ephemeral single-user (grant is a no-op), while stg may set
MAPLE_PG_RUNTIME_ROLE to a distinct role where the re-grant is load-bearing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* ci: drop the grant-runtime-role step from stg deploy too

Same self-grant no-op as PR preview (stg never sets MAPLE_PG_RUNTIME_ROLE), and
it hits the same failing path. Remove it instead of documenting the asymmetry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — the lockout fix survives the merge intact; one behavior-drift observation below worth a look before merging.

Reviewed changes — since the prior pullfrog review (ee3e57f9), two main merges (bb71914, e5b82fc) reconciled this PR with the typed /api/billing migration (#125). The reconciliation re-homed the Fix #1 API logic and moved the client hook onto an atom; I re-reviewed that reconciliation.

  • Ported the customer-cache TTL onto billing.http.ts — #125 renamed autumn.http.ts → billing.http.ts, so the merge re-applied responseHasActivePlan + the content-dependent getOrCompute TTL (300s active / 5s planless via CUSTOMER_CACHE_UNSETTLED_TTL_SECONDS) onto the new file. Logic is functionally identical to what was reviewed at ee3e57f9, and the retired autumn.http.ts proxy is fully gone.
  • Re-homed the client hook onto billingCustomerAtom — use-maple-customer.ts is now a thin accessor over the effect-atom billingCustomerAtom; the function-form staleTime / self-stopping refetchInterval that previously lived in this hook are no longer present.
  • Unchanged from the prior review — the shared @maple/domain/billing isActivePlanSubscription gate, plan-gating.ts delegation, and the edge-cache.ts function-form ttlSeconds (resolved once on write) are byte-identical and re-covered by their tests.

ℹ️ The client half of the fix was dropped in the merge — only the API-cache half remains

The original fix worked at two layers: the API edge cache (5s TTL on a planless snapshot) and a client-side self-stopping poll in use-maple-customer.ts that re-fetched every 5s while planless, so a just-subscribed user parked on /quick-start auto-advanced the moment Autumn finished syncing — no refresh, no navigation. The merge that re-homed the hook onto billingCustomerAtom (which has no refetchInterval and no short staleTime) removed that client poll.

The titular lockout is still fixed — the 5s API TTL plus effect-atom's refetch-on-remount means the next navigation after Autumn syncs recovers, so the permanent loop is broken. What's gone is the passive auto-recovery: a user who lands on /quick-start and simply waits will not advance until they navigate or refresh.

This leaves the PR description out of step with what ships — it still claims a use-maple-customer.ts refetchInterval that "auto-recovers within seconds with no manual refresh." Worth either restoring a poll/short-staleTime on billingCustomerAtom or updating the description and the "Avoiding drift" / two-layer framing so the next reader isn't misled.

Technical details
# Client-side post-checkout auto-recovery dropped during the #125 reconciliation

## Affected sites
- `apps/web/src/hooks/use-maple-customer.ts` — now a thin accessor over `billingCustomerAtom`; the prior function-form `staleTime` (5min settled / 5s planless) and self-stopping `refetchInterval` (5s while planless, `false` once active, paused backgrounded) are gone.
- `apps/web/src/lib/services/atoms/billing-atoms.ts:15-17` — `billingCustomerAtom = MapleApiAtomClient.query("billing", "getCustomer", { reactivityKeys: [...] })`; no `refetchInterval`, no short/unsettled `staleTime`. Its comment claims recovery via "refetches on remount + reactivity invalidation" — neither fires on a post-checkout *wait* (the `attach` invalidation already ran before the Stripe redirect; the return is a fresh load with no subsequent remount until the user navigates).
- `apps/web/src/routes/__root.tsx:182-183` — a planless settle still `Navigate`s to `/quick-start`; with no client poll there's no force that re-evaluates the gate while the user idles there.
- PR description ("Fix" → Client bullet, and "Avoiding drift" two-layer framing) — describes a client `refetchInterval` that no longer exists.

## Required outcome
- Decide whether passive post-checkout auto-recovery is still a requirement. If yes, the client must re-check while planless (the API `5s` TTL alone only helps a request that actually fires). If no, the PR description and two-layer framing should be corrected so the shipped behavior is documented accurately.

## Suggested approach (optional)
- Restore the unsettled poll at the atom layer: give `billingCustomerAtom` a short idle-TTL / `refetchInterval` (≈5s) that applies while the customer is planless and stops once a plan is active — mirroring the dropped `use-maple-customer.ts` logic but expressed in the effect-atom idiom (e.g. an `Atom.setIdleTTL` / interval-refresh wrapper gated on `isActivePlanSubscription`). Confirm it pauses while backgrounded, as the original did.
- Or, if recovery-on-next-navigation is deemed sufficient, drop the client-poll claims from the PR body and note that the fix is now API-cache-only.

## Open questions for the human
- Is a planless user expected to *sit* on `/quick-start` post-checkout (needing passive recovery), or do they reliably navigate/refresh within seconds (making remount-refetch enough)? This determines whether the client poll needs restoring or the description just needs correcting.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot | Fix it ➔ | View workflow run | Using Claude Opus | 𝕏

@Makisuo
Makisuo merged commit 704c4f8 into main Jun 26, 2026
7 checks passed
@Makisuo
Makisuo deleted the jeremy/fix-onboarding-lockout-after-checkout branch June 26, 2026 18:04
Makisuo added a commit that referenced this pull request Aug 24, 2026
* feat(errors): attach PRs to issues and verify the fix after merge

An error issue could carry a fix proposal but never a fix. `propose_fix`
accepted a `pr_url` as free text on an event payload — nothing resolved it,
nothing watched it, and nothing confirmed the fix worked, so issues sat in
`in_review` until someone remembered them.

A pull request is now a durable link on an issue. When it merges, the issue
enters a new `verifying` state with a quiet window sized from its severity and
its own pre-merge occurrence rate. When the window closes, a verification agent
runs and either closes the issue (low/medium severity) or posts a verdict for a
human (high/critical).

Two pieces of existing machinery made this cheap:

- The GitHub App integration already had a signature-verified webhook receiver
  and a sync queue. `pull_request` was the one event class it ignored.
- `isRegression` already decides whether an occurrence is a real regression by
  asking whether its build was in the set seen at resolution time. That is
  exactly the "did the fix work" predicate — a merge just needs the same
  snapshot taken at merge instead of at close. So the verdict rests on a
  deterministic signal the agent corroborates rather than one it invents.

Notable behaviour:

- If the error fires from a post-merge build while the window is still open,
  the fix demonstrably failed and the error tick settles it immediately, with
  no agent pass. Occurrences from builds that predate the merge are discounted
  as old clients still in the wild.
- Verification reads an absent build signal as no evidence, unlike
  `isRegression` which errs toward reopening. Erring the other way would refute
  every fix arriving from a service that does not report `service.version`.
- An investigation reaching `diagnosed` means the agent found a live cause, so
  for a verification that maps to "not fixed"; `inconclusive` corroborates the
  fix. `apply-diagnosis` is now subject-aware so a verification's
  `severityAssessment` cannot silently re-rank an issue a human already
  triaged.
- Three paths create the same link (propose_fix, the UI, the webhook's body
  scan), so linking is idempotent. The body scan only matches a dashboard issue
  URL or a `maple-issue:<uuid>` token, and confirms the issue belongs to the
  webhook's org first — a PR body is attacker-influenced text. `Fixes #123` is
  deliberately unsupported: Maple issue ids are UUIDs, so a bare integer can
  only name the host provider's own tracker.
- VCS stays ignorant of issues. The webhook reaches the errors side through a
  `PullRequestEventSink` port, bound in the composition roots, which also lets
  the sync-service tests assert forwarding without the error-issue stack.

Requires a manual change outside this repo: the GitHub App must add the
`Pull request` webhook subscription, or no delivery ever arrives.

* refactor(domain): parse URLs with Schema.URLFromString, not try/catch

`parsePullRequestUrl` and `extractIssueIdsFromText` wrapped `new URL(...)` in
`try/catch` to turn a thrown `TypeError` back into a value. Effect has the
primitive for this: `Schema.URLFromString`, decoded through
`Schema.decodeUnknownOption`, reports the failure as an `Option` instead.

Effect's own `SchemaTransformation.urlFromString` is what backs it, so this is
the same check the schema layer already performs everywhere else rather than a
second, hand-rolled one.

Both functions stay synchronous and total, which is what lets the webhook path,
the HTTP handler, and the tests keep calling them as plain predicates.

* fix(errors): stop the verification tick auto-closing on an interrupt

Review of the fix-verification feature turned up one wrong failure type and
a cluster of ways the tick could close an issue it should not have.

- `linkPullRequest` reported a failed upsert/hydrate as
  `ErrorIssuePullRequestInvalidError`. The URL had already parsed, so this
  told the caller their valid URL was unrecognizable and — because that tag
  is an anticipated error — suppressed the span exception, hiding a
  persistence anomaly from error dashboards. Both paths now fail with
  `ErrorPersistenceError`, which was already in the endpoint's union.
- `Effect.catchCause` catches interrupts too, so an isolate teardown during
  the enqueue became `{ enqueued: false }`, which the fallback reads as "no
  agent available" and answers with a terminal `verified` verdict. All three
  catches now re-raise interrupt-only causes, matching every sibling tick.
- A row whose verdict write failed deterministically aborted the tick before
  phase 2 every minute, freezing every other issue in `verifying` with
  all-zero counters. Verdict writes are guarded per row and report a new
  `failedRows` count.
- The two branches that apply a real verdict counted themselves as `skipped`,
  so `verdicts_applied` under-reported exactly the auto-closing paths.
- A per-issue persistence failure aborted the rest of a multi-issue webhook,
  which both docstrings promise it does not.
- `String(cause)` in log annotations dumped whole nested causes; use
  `summarizeCause` as the sibling enqueue already does.
- One live verification per issue rested on a read-then-insert while the
  queue consumer runs with `maxConcurrency: 2`. The partial index is now
  UNIQUE (folded into this branch's own migration) and the insert tolerates
  the conflict.
- Annotate the four new HTTP handlers and the enqueue's three unannotated
  exits, including `no_binding` — the outcome that can auto-close.

UI: the waiting card read "Waiting until in 6h"; link failures all reported
as a bad URL; the attach dialog cleared the pasted URL before the mutation
resolved; the rail cards had no inset; a fix-verification investigation
reached chat keyed by the investigation id rather than the issue's.

Tests: pin the deliberately inverted verdict mapping and the enqueue's four
outcomes, assert PR-event forwarding through the sink (the harness existed
but nothing read it), assert link failures by tag and add cross-tenant
cases. Two existing tests asserted nothing: the machine-owned-state guard
filtered by the same set the implementation uses, and a fixture UUID had a
version nibble `Schema.isUUID()` rejects, so the multi-issue extraction it
claimed was unreachable.

* fix(ci): mirror link_pull_request into the Slack approval canary, regen iOS spec

Both were already red on the parent commit — this branch added a mutating MCP
tool and a workflow state without updating the two places that mirror them.

`link_pull_request` reached `mutating.ts` and slack-agent's
`MUTATING_TOOL_NAMES`, but not the hardcoded list in approval.test.ts. That
list exists because the canary next to it skips when the API source is not
checked out, so it is the only gate a slack-agent-only checkout has — a
mutating tool missing from it ships to Slack without the approval prompt.

The iOS OpenAPI spec predates the `verifying` workflow state; regenerated.

* fix(ios): handle the verifying workflow state

Regenerating the OpenAPI spec added `verifying` to the workflow-state enum,
which made the three `WorkflowState` switches in Primitives.swift
non-exhaustive and failed the build. The spec was stale precisely because
this half was never done.

Teal, matching `workflow-badge.tsx` and for the reason its comment gives: a
merged fix being watched is distinct from `inReview`, where a human is
looking, and from `done`, where it is over. Token values are Tailwind's
teal-600/400 text and teal-500 fill, the same pair the web badge resolves to.

The state-filter picker builds from `allCases`, so it picks this up on its
own — correct here, since `verifying` is filterable even though it is
machine-owned and never offered as a manual transition.

* refactor(errors): decode the investigation subject discriminator with Schema

`subjectTypeOf` hand-rolled its narrowing — a `typeof` check, an `as` cast to
read `.type`, and a bare `string` out — for a value that has a schema.

Now `Schema.decodeUnknownOption` over an `InvestigationSubjectDiscriminator`,
returning `Option<InvestigationSubjectType>`. The literal set is derived from
`InvestigationSubject.members` rather than restated, so a fourth subject type
cannot leave it behind, and the guard is `Option.contains` against a narrowed
union instead of a magic string compared to `string`.

It decodes the discriminator ALONE, deliberately, and the tests pin that.
Reading the whole `InvestigationSubject` would answer `None` for a row whose
other fields drifted or predate the current shape — and `None` is the
dangerous answer here, not the safe one: it means "not a verification", which
is what lets a routine post-merge run re-rank a severity a human already set.
Excess-key tolerance is what makes a real stored subject decode at all, so
that is asserted rather than assumed.

Also covers `deterministicInvestigationEventId`, whose output is decoded
through a branded id whose UUID check would reject a bad version nibble.

* refactor(errors): the diagnosis writers own their database call

`applyDiagnosisWrites` and `applyInconclusiveWrites` took a raw client and
returned a Promise, so every caller had to remember to wrap them and supply
the retry, the span and the error mapping. Both now return an Effect and
reach `Database` themselves.

The drizzle callback inside stays a Promise, and that is the honest end of
this: `DatabaseApi.execute` is typed `(db) => Promise<T>` precisely because
drizzle's `transaction` is a Promise API with no Effect-native equivalent
here — the same shape a dozen sibling services use. Moving the wrapper inside
puts the seam at one place per logical call instead of at every call site;
it does not pretend the Promise is gone.

They fail with the raw `DatabaseError` rather than a domain persistence
error. The write straddles two domains — `investigations`, plus the
error-issue tables when an issue is linked — and the two callers map
differently, so mapping here would force one of them to re-wrap.
`InvestigationService` keeps its channel with `Effect.mapError` and provides
its captured `Database`; the contention retry is unaffected, living in
`makeDbExecute` rather than in the mapping.

The fan-out workflow gets `runDbStep`, which backs `Database` with the run's
own `PgConnectionScopeApi` — its `run` has exactly `execute`'s signature — so
every statement stays on the single connection that run opens and closes in
its `finally`, rather than dialing a socket per step. The one call site that
bundled a lens-run update with the diagnosis write is now two calls: those
were never atomic with each other, since `connection.run` is a span around a
logical call and not a transaction.

* fix(errors): claim the issue on the paths agents actually take

Across 50 live issues in the internal org, `lease_holder` was null on every
single one — `claim_error_issue` had never been called in production. The lease
existed to stop two agents fixing the same bug and had never once been held.

Claiming was not being skipped out of laziness. `triage -> in_review` is
deliberately not a legal edge, but `propose_fix` transitioned straight to
`in_review`, so proposing a fix on an untriaged issue — the state agents find
issues in — failed with "Illegal transition from 'triage' to 'in_review'". It
was the most common MCP error in the org, 18 occurrences in two days, and it
failed *after* writing the fix_proposed event and linking the PR. Agents
responded by hand-walking `transition_error_issue` instead, which works and
leaves the lease empty.

So the claim is now taken by the work rather than asked for beforehand:

- `propose_fix` acquires the lease and walks to `in_review` via a route the
  matrix actually permits (`fixProposalRoute`), from any state an issue can be
  worked from. Everything refusable is refused before anything is written, so a
  rejected call no longer leaves a half-written proposal.
- A transition to `in_progress` acquires the lease too — that is the path the
  agents actually used. Best-effort: another holder's lease is left alone
  rather than refusing a state change the caller is entitled to make.
- `propose_fix` can now fail with a lease conflict, which is the duplicate-work
  collision the lease exists to catch, finally caught where agents work.
- A `pr_url` that is not a pull request URL is a 400 instead of being swallowed
  and then reported back as `- PR: <url>`, which told an agent the fix was
  attached and would be verified when nothing had been linked at all.
- An illegal transition now names the moves that would work.
- `TERMINAL_WORKFLOW_STATES` -> `CLOSED_WORKFLOW_STATES`: `done` reopens to
  `regressed`, `verifying` and `triage`, so "no further transition is possible"
  was false. Both call sites mean "the work is over, drop the lease".
- `describeWorkflowTransitions` stops advertising machine-owned targets the
  transition tool then rejects.

Verified two ways: deterministic tests pinning the exact production failure
(triage to in_review end to end, the lease actually held, no fix_proposed on a
refused call, the lease-conflict path), and a new model-driven eval confirming
a real model still reaches for the claiming tools.

* fix(web): lead the copy-as-prompt with the claim, and stop calling it optional

The "Copy as prompt" text is what users paste into their coding agent, so it is
the instruction that actually reaches one. It read:

  5. Optionally claim the issue first with `claim_error_issue`, and once you
     have a patch, record it with `propose_fix`.

Last of five steps, after "propose a concrete fix", prefixed with "Optionally".
Not one issue in the internal org had ever been claimed.

The claim now leads and is unconditional, and the prompt closes with "Start by
claiming the issue." Also adds a `list_error_issue_events` step so an agent
reads Maple's own AI diagnosis and any earlier attempt before re-fixing a known
bug — guarded on the issue id, since that tool takes one and the legacy
errors-by-type page has none.

* refactor(errors): share the investigation fan-out start

`maybeEnqueueTriage` and `enqueueFixVerification` carried byte-identical copies
of the binding shape check, the `markFailed` update, the `Exit`-wrapped
`create()`, and the outcome span annotations — including two separate
declarations of `isFanoutWorkflowBinding`.

What differs between them is the subject, the snapshot and the insert, which
encode what a run is about. Starting it does not, so that half moves to
`startInvestigationFanout`. The verification path keeps its
`maple.verification.id` annotation through an `annotations` parameter: the tick
reads the start outcome back and can answer `no_binding` with a terminal verdict
that auto-closes the issue, so losing that attribute would leave the trace of
the close saying nothing about no agent having run.

* fix(query-engine): keep synthetic fingerprints out of toUInt64

`error_issues.fingerprint_hash` is shared by three issue kinds: "error" rows
hold the decimal UInt64 from ClickHouse, while "alert" and "integration" rows
reuse the column for a synthetic key (`alert:{ruleId}:{groupKey}`,
`planetscale:{database}:{event}`). `toUInt64('alert:…')` does not skip that
value, it aborts the whole query — so one alert-backed issue in a batch of
twenty 500s the request.

#573 filtered the list in the errors hub, which fixed the surface that was
firing. This is the same guard on the side no other caller can bypass:
`fingerprintHashIn` drops what the warehouse could not hold, which is exact
rather than lenient — a synthetic key cannot have error events by construction,
so nothing a caller could have matched is lost. An all-synthetic list emits a
false literal, because `IN ()` is a ClickHouse syntax error.

* fix(web): one workflow-state label map, and say whose move it is

Four copies of the workflow-state labels existed and the one in
`workflow-badge.tsx` had drifted — "Wontfix" where the ring, the state picker
and the bulk bar all said "Won't fix". Both duplicates now import
`WORKFLOW_LABEL`; the badge keeps only its tones.

A `verified` verdict on a high or critical issue deliberately does not close it
— the verdict is posted and a human decides. The card said "The fix holds" while
the issue sat open, with nothing to tell you whether Maple was still working or
waiting on you. It now says.

* docs(errors): write down the issue lifecycle, and tell agents the write flow

The `maple://instructions` MCP resource covered read-only observability and
stopped there — nothing about issues, leases, PR linking or verification. Agents
were given the whole write surface and no map of it, which is half of why the
flow was walked by hand.

Adds an "Error Issue Lifecycle" section: the flow end to end, the six rules that
matter (claim, read the timeline first, attach the PR, don't close an issue you
linked a PR to, `regressed`/`verifying` are not yours to set, severity drives
escalation), and a tool-picking list.

Adds `docs/error-issue-lifecycle.md` as the human counterpart — the entity model,
who owns which edge and why, the three places AI enters, and why verification
asks its question inverted. It also records what happened when claiming was a
rule an agent was merely told to follow, so the next tool that starts work on an
issue takes the lease itself.

The `transition_error_issue` and `link_pull_request` descriptions pick up the
same rules, so an agent that never reads the resource still gets them.
Makisuo added a commit that referenced this pull request Aug 26, 2026
…#642)

Attaching a PR is the hinge of the fix→verify loop: the link is what opens a
post-merge verification window and lets an issue close on evidence. That hinge
was a bare text box asking for a PR URL, so the flow was leave Maple, find the
PR on GitHub, copy, come back, paste — while the org's repositories were
already synced and could simply have been offered.

Fixes a real bug along the way. linkPullRequest only string-parsed the URL and
stored title: null, state: "open", waiting for a webhook to enrich the row. For
a PR that merged BEFORE it was attached — the normal human order of operations
— no `closed` event is ever redelivered, so the verification window never
opened and the issue sat in `in_review` indefinitely. The merge handling is now
factored out of onPullRequestEvent into a shared openMergedVerifications and
called from the link path too.

- VcsProviderClient gains fetchPullRequests / fetchPullRequest, implemented
  over /pulls. One page, sort=updated, no pagination walk: the PR that fixes a
  live issue is recent by construction. Deliberately not /search/issues, whose
  30-req/min secondary limit suits a keystroke-driven picker poorly — filtering
  happens client-side over the fetched page.
- GET /api/integrations/vcs/pull-requests exposes them, scoped to one connected
  repository so the picker's options can never cross a tenant boundary.
- Hydration crosses the VCS↔errors boundary through a new PullRequestLookup
  port, mirroring PullRequestEventSink but pointing the other way, so errors
  never imports the VCS layer. Strictly best-effort: an unconnected repo or an
  unreachable provider stores the link unenriched, exactly as before. A
  re-attach also repairs a stale row whose webhook was missed.
- listIssuePullRequests now carries suggestedRepository, so the picker opens on
  the right repo with no extra round trip. Four Postgres-only signals, first hit
  wins: a repo already linked to this issue, the only connected repo, an
  unambiguous service-name match, then wherever the org's fixes most recently
  landed. It abstains rather than guess when two repos match — a silent wrong
  preselection is only noticed after somebody attaches the wrong PR.
- The dialog moves into its own component so the panel, the verification slot,
  and the in_review transition can all open it. Pasting still works (full URL,
  #123, or a bare number), which stays the only way to attach a PR in a repo the
  org has not connected — something the link table deliberately supports.

Verified locally against a seeded installation: the suggester resolves by name
match and falls back to the most recent link when nothing matches; the paste
path attaches and stores an unenriched row when the provider is unreachable;
the not-connected fallback renders. Also fixed the new nudge button overflowing
the 255px rail.

The new runtime/vcs-source-layer.ts is added to the vendor-import allowlist in
.oxlintrc.json — it is a composition root, which is what lets the alerting
worker get a source service without naming a provider module itself.

This branch was previously deployed

1 inactive deployment
pr-preview — e5b82fc4 Deployed Jun 26, 2026 by Makisuo via deploy-pr-preview #486
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.

2 participants