Repository navigation
fix(web): keep synthetic fingerprints out of the errors sparkline query - #573
Merged
Merged
Conversation
`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}`).
The errors hub sent every visible issue's fingerprint to `errorsSpark`, which
lowers the list into `FingerprintHash IN (toUInt64('…'), …)`. One alert-backed
issue on screen made ClickHouse reject the whole batch, so the sparklines 500'd
for every row in the list.
Filter to error-kind issues before the round-trip — the same guard
`ErrorIssueReadModelsService.getIssue` already applies on the detail path, which
is why issue detail never hit this. Rows without a series already degrade to an
empty spark, and an all-alert list skips the request entirely.
JeremyFunk
had a problem deploying
to
pr-preview
August 23, 2026 21:18 — with 
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
🍁 Maple PR previewWarning Preview cleanup could not be confirmed. The Alchemy teardown outcome was Final commit |
Makisuo
added a commit
that referenced
this pull request
Aug 24, 2026
`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.
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.
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What broke
Loading
/errorsreturned a 500 fromerrorsSpark:error_issues.fingerprint_hashis shared across three issue kinds:erroralertalert:{ruleId}:{groupKey}integrationplanetscale:{database}:{event}The errors hub sent every visible issue's fingerprint to the batched sparkline
query, which lowers the list into
FingerprintHash IN (toUInt64('…'), …). Asingle alert-backed issue on screen made ClickHouse reject the whole batch — so
the sparklines failed for every row in the list, not just that one.
Issue detail never hit this because
ErrorIssueReadModelsService.getIssuealready skips the warehouse for non-error kinds.
Fix
Apply the same guard before the round-trip, as
sparkFingerprintHashes()inlib/models/error-signal.ts. Alert and integration rows already degradecorrectly:
buildErrorSignalsfalls back to an empty series, andgetErrorsSparkshort-circuits an empty list, so a view holding only alertissues skips the request altogether.
Not covered here
/internal/query-engine/errors-sparkstill lowers a caller-supplied liststraight into
toUInt64(), so a future caller passing a synthetic key gets a500 rather than a 400. Tightening the shared
FingerprintHashprimitive todecimal-only would be wrong —
parseFingerprintHashesin the v2 issues routelegitimately filters Postgres by
alert:values — so that hardening would needto live at the warehouse-bound boundary specifically. Left out of this fix.
Verification
bun run --cwd apps/web test src/lib/models/error-signal.test.ts— 12 passed,including a new case built from the fingerprint in the failing trace
bun run --cwd apps/web typecheck— cleanNot verified in a browser: reproducing needs an alert-backed issue in the hub
list, which local data does not have. The failing path is a data-selection
choice, covered by the unit test instead.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.