fix(labeler): kill-switch on in-flight runs, durable dead-letter retry, pointer tie-break (held review findings)#2166
Conversation
The current-pointer upsert guard accepted any run at an equal created_at_epoch_ms, but list/current ordering is (created_at_epoch_ms, id). A later lower-id run could displace a higher-id current sharing a millisecond. Match the pointer guard to the ordering so a lower-id run cannot overwrite a higher-id current at an equal epoch.
An admin pausing automation only halted discovery; already-dispatched Workflow runs never re-read automation_state and finalization had no pause predicate, so in-flight runs kept issuing automated labels after a successful pause. Re-read the kill-switch on Workflow entry (halting before AI/network work) and fold a fail-closed automation-not-paused predicate into the finalization CAS. A pause landing mid-flight no-ops the whole batch, leaving the run running for the Workflow retry to resume after unpause -- matching the signing-pause precedent.
The dead-letter retry committed status='retried' before decoding the payload and delivering the re-drive to the queue. A failed or undecodable delivery stranded the row 'retried': health checks (which count only 'new') stopped seeing it and a later operator retry returned 409, with nothing ever re-driven. Enqueue the discovery job before committing 'retried', so the status flips only once the queue accepts the job. An undecodable payload (422) or a queue failure (503) leaves the row 'new' -- still counted and retryable. A duplicate enqueue under concurrent double-retry is absorbed by the consumer's runKey dedup.
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 6959687 | Jul 21 2026, 07:15 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-cache | 6959687 | Jul 21 2026, 07:15 AM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-demo-do | 6959687 | Jul 21 2026, 07:15 AM |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is sound: these are the right fixes for the three held labeler findings, applied in the idiomatic places (Workflow entry guard + finalization CAS, pre-commit durable enqueue, and pointer upsert tie-break). Tracing the change confirms the kill-switch is fail-closed on both paths, the dead-letter retry now flips status only after the queue accepts the job, and the pointer guard matches the canonical (created_at_epoch_ms DESC, id DESC) ordering while remaining idempotent on same-run replay. The new tests exercise the concrete regression scenarios.
I did not find any logic bugs, regressions, or AGENTS.md violations in the changed code. The only thing that should be cleaned up is a stale interface comment: ConsoleMutationDeps.sendDiscoveryJob still claims the queue operation "runs in the deferred tail," but this PR moved it inline before commitMutation so the contract is misleading for future callers.
Findings
-
[suggestion]
apps/labeler/src/console-mutation-api.ts:166The docstring for
sendDiscoveryJobno longer matches how the retry path uses it. SincerunDeadLetterActionnow awaitsdeps.sendDiscoveryJob(job)before committingretried, the queue operation is not deferred off the response path; it is a synchronous durability gate on the request path. Update the comment so callers do not assume they can defer side effects or that the commit has already happened./** Re-enqueues a dead letter's discovery job on retry (design §6). A queue op * cannot join the D1 batch, so it is awaited before `retried` is committed; the * discovery consumer's `runKey` dedup absorbs a duplicate re-drive. */
github-actions
Bot
added
review/awaiting-author
github-actions
Bot
added
review/needs-rereview
|
Fixed the stale ~ 🤖 Claude Fable 5 |
fa4d25b
into
feat/plugin-registry-labelling-service
1 participant
What does this PR do?
Closes three held findings from the adversarial review of the labeler (RFC #694, umbrella #1909) — parked during the earlier fix PRs because they overlapped files those PRs were rewriting, now addressed off the merged tip. Each is TDD'd and the set passed an independent adversarial review. Targets the
feat/plugin-registry-labelling-serviceintegration branch.1.
fix(labeler): enforce the automation kill-switch on in-flight runs (#14). Automation pause was only checked before discovery; an already-dispatched Workflow never re-readautomation_stateand finalization had no pause predicate, so an admin could pause automation, get a success response, and in-flight runs kept issuing automated labels. Now enforced at two points: the Workflow entry (executeAssessmentInstancere-readsisAutomationPausedafter the terminal short-circuit and throwsAutomationPausedErrorbefore spending AI/network work) and, fail-closed, in the finalization CAS — aguardAutomationNotPausedpredicate (EXISTS(automation_state WHERE paused=0)) AND'd onto the same batch as the signing and subject-not-deleted guards, so a mid-flight pause no-ops the whole batch and leaves the runrunningfor the Workflow retry (halt-and-leave-resumable, matching the signing-pause precedent). Unknown/unreadable pause state fails closed (no issuance).2.
fix(labeler): make dead-letter retry durable before markingretried(#9). The dead-letter retry handler committedstatus='retried'in the mutation batch, then decoded the payload and enqueued in a deferred tail — so a failed or undecodable delivery stranded the rowretried: health counts onlynew, no sweep re-drivesretried, and a later operator retry hit 409. The decode + enqueue now happen beforecommitMutation: an undecodable payload returns 422 (permanent) and an enqueue failure returns 503 (transient), both leaving the rownew/actionable; aretriedrow now implies the job was durably accepted. Verified replay-safe (idempotency/audit rows are written only at commit, the guard's replay branch short-circuits before re-enqueue) and double-retry-safe (the consumer'srunKeydedup absorbs a duplicate).3.
fix(labeler): tie-break the current-pointer update on assessment id (#19). The current-assessment pointer upsert allowed overwrite at any equalcreated_at_epoch_ms, but list/current ordering is(created_at_epoch_ms DESC, id DESC)— so a later lower-id run sharing a millisecond could displace a higher-id current. The upsert guard now requiresexcluded.epoch > current.epoch OR (equal epoch AND excluded.id >= current.id), matching the canonical ordering (>=on the unique ULID id keeps idempotent same-run replay).Known follow-up (not introduced here): a pause longer than the Workflow retry window strands the in-flight run
runninguntil an operator re-drives it — the fix is correctly fail-closed (no labels issued), and automatic resumption of a long-paused run folds into the existing "reconciliation should re-dispatch stuck runs" hardening item.Type of change
Checklist
pnpm typecheckpasses (all three labeler projects)pnpm lintpasses (0 diagnostics)pnpm testpasses (labeler: 907 + 136 + 39) — TDD: each fix has a pre-fix failing repro (fix: resolve all lint warnings and fix failing playground toolbar test #14 finalizedpassedwith labels under a pause; Fix playground toolbar link to point to repo #9 returned 200 and flipped the row toretriedon a failed delivery; April fools or real? #19 a lower-id equal-epoch run displaced the higher-id current)pnpm formathas been run@emdash-cms/labeleris private.AI-generated code disclosure
Screenshots / test output