Skip to content

feat(labeler): prolonged-error publisher notice via reconciliation cron (W10.5 follow-up)#2088

Merged
ascorbic merged 2 commits into
feat/plugin-registry-labelling-servicefrom
feat/labeler-prolonged-error
Jul 17, 2026
Merged

feat(labeler): prolonged-error publisher notice via reconciliation cron (W10.5 follow-up)#2088
ascorbic merged 2 commits into
feat/plugin-registry-labelling-servicefrom
feat/labeler-prolonged-error

Conversation

@ascorbic

@ascorbic ascorbic commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Implements the prolonged-error publisher notification — the sixth spec notification event, deferred during W10.5 because there was no stuck-run detection. Targets the feat/plugin-registry-labelling-service integration branch.

An assessment can end in the terminal error state (the labeler failed to complete its assessment — transient retries exhausted — not a finding against the publisher). Until now nothing told the publisher. This adds a two-stage escalation ladder driven by the reconciliation cron, for an errored run that stays the live, unsuperseded assessment for its subject:

  • 24h → a high-severity assessment-prolonged-error operational event, so operators can triage an infra-vs-publisher cause before anyone is emailed.
  • 72h → a neutral, actionable publisher notice through the W10.5 double-opt-in pipeline ("we couldn't complete the assessment; if the artifact URL changed, please check it").

The two thresholds and the operator-first ordering were ratified with the maintainer.

How it works:

  • Migration 0010_assessment_error_escalations.sql — a small tracking table making each stage fire-once across the 5-minute cron ticks.
  • Source reuse, no schema churn — the notice reuses source_type = 'issuance' keyed on the errored assessment id. An errored run never produces a block/warn notice, so the key never collides with a finalization notice, and no notifications.source_type CHECK rebuild is needed. resolveNoticeForSource routes an error-state assessment to the prolonged-error content, so the retry sweep re-renders identical copy.
  • Finalization stays silentassessmentNoticeContent still returns null for error, so notifyAssessmentOutcome no-ops; only the cron's 72h stage notifies.
  • Supersession halts the ladder when a newer run exists for the same (uri, cid); deleted subjects are skipped; the scan drops fully-escalated rows so a mass-error backlog never starves newer errors.
  • Concurrency — the operator-alert insert is gated on the escalation still being un-alerted (Cloudflare does not serialize a cron trigger against its own next tick), and the publisher stage is idempotent via the notifications source claim.
  • No private detail — the notice copy is fixed and neutral; the operational-event payload is the public assessment id + release cid only.

Note: actual email is gated on the operator-owned Cloudflare Email Sending domain onboarding (a separate pre-launch item), same as the rest of W10.5. Migration 0010 follows 0009 (the reconsideration table merged in #2083).

An adversarial review pass found and fixed two MED issues before this PR: the escalation scan now drops fully-escalated rows (so a >200-error backlog can't starve newer errors), and the operator-alert insert is gated against overlapping cron ticks. A third finding (the operator head-start collapsing on rollout/outage-recovery of pre-existing >72h errors) was reviewed and consciously accepted by the maintainer; steady-state keeps the full 24h→72h head-start.

Part of #1909. Follows #2083.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes (all three labeler projects)
  • pnpm lint passes (0 diagnostics on touched files)
  • pnpm test passes (12 prolonged-error integration tests; regression across notification-triggers/sweep, reconciliation, operational-events, and the reconsideration suite green)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable) — n/a: no admin/console UI; the publisher notice is server-rendered email copy (server-side strings are English-only for now, per CONTRIBUTING).
  • I have added a changeset (if this PR changes a published package) — n/a: @emdash-cms/labeler is private: true.
  • New features link to an approved Discussion — the umbrella Plugin registry labelling service #1909 tracks RFC RFC: Decentralized Plugin Registry #694; this is one item under it.

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 4.8 (implementation + adversarial review), orchestrated by Claude Fable 5

Screenshots / test output

Test Files  1 passed (1)
     Tests  12 passed (12)

Try this PR

Open a fresh playground →

A full working EmDash site, deployed from this branch. Each visit gets its own session-scoped sandbox: no login needed and no shared state. Try the admin, edit content, hit the public site.

Tracks feat/labeler-prolonged-error. Updated automatically when the playground redeploys.

…on (W10.5 follow-up)

The sixth notification event. An assessment stuck in the terminal error
state that stays the live, unsuperseded run escalates in two stages from
the reconciliation cron: at 24h a high-severity assessment-prolonged-error
operational_event so operators can triage an infra-vs-publisher cause, at
72h a neutral, actionable publisher notice through the W10.5 double-opt-in
pipeline.

- migration 0010: assessment_error_escalations tracking table makes each
  stage fire-once across the 5-minute cron ticks.
- the notice reuses source_type 'issuance' keyed on the errored assessment
  id (an errored run never produces a block/warn notice, so no key
  collision and no notifications CHECK rebuild); resolveNoticeForSource
  routes error-state to the prolonged-error content for sweep parity. The
  finalization path stays silent on error (assessmentNoticeContent still
  returns null), so only the cron's 72h stage notifies.
- supersession halts the ladder when a newer run exists for the same
  (uri,cid); deleted subjects are skipped; the scan drops fully-escalated
  rows so a mass-error backlog never starves newer errors.
- the operator-alert insert is gated on the escalation still being
  un-alerted so overlapping cron ticks cannot double-alert; the publisher
  stage is idempotent via the notifications source claim.
- notice carries no private detail; the operational_event payload is the
  public assessment id + cid only.

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

changeset-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8d70467

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added review/needs-review No maintainer or bot review yet size/XL labels Jul 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Scope check

This PR changes 866 lines across 8 files. Large PRs are harder to review and more likely to be closed without review.

If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs.

See CONTRIBUTING.md for contribution guidelines.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-do 8d70467 Jul 17 2026, 06:53 AM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-demo-cache 8d70467 Jul 17 2026, 06:53 AM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
emdash-playground 8d70467 Jul 17 2026, 06:53 AM

@emdashbot emdashbot 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.

This PR adds a coherent two-stage escalation ladder for terminal error assessments: a 24h operator alert and a 72h publisher notice, driven from the reconciliation cron and made idempotent via a new tracking table. The approach fits the labeler’s existing patterns — it reuses the issuance notification source, the operational-events subsystem, and the double-opt-in send path, and it correctly keeps finalization silent for error runs.

I reviewed the diff statically (no test/lint runs) and focused on the new migration, cron pass, notification trigger changes, and call-sites. The code is generally careful: the operator-alert insert is gated on the unalerted escalation row and batched with the mark update, supersession/deleted-subject filtering is correct, and dropping fully-escalated rows prevents a backlog from starving newer errors.

I found two concrete issues that should be addressed before merge:

  1. Missing index for the cron scanfindEscalatableErrors filters assessments.state = 'error' AND completed_at_epoch_ms <= ? and orders by completed_at_epoch_ms. The existing idx_assessments_state_created only covers (state, created_at_epoch_ms), so as error backlogs grow each 5-minute tick will scan/sort all error rows. This violates the repo’s index discipline and is a predictable operational regression.

  2. Publisher mark can stamp on transient failurenotifyProlongedError is wrapped in runTrigger, which swallows all errors. The cron then unconditionally calls markPublisherNotified. If an aggregator read throws or a D1 write fails before a notifications row is claimed, no notice was sent and no row exists, but the error is permanently excluded from future passes. The self-healing contract only covers crashes after a row is claimed, not failures before a row exists.

Both are fixable in small, localized changes. No AGENTS.md-level concerns around SQL injection, authorization, localization, or changesets surfaced (the labeler app is private, and the emails are server-side English-only).

publisher_notified_at_epoch_ms INTEGER,
created_at TEXT NOT NULL,
created_at_epoch_ms INTEGER NOT NULL
);

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.

[needs fixing] The findEscalatableErrors query filters assessments.state = 'error' AND assessments.completed_at_epoch_ms <= ? and orders by completed_at_epoch_ms ASC. The only existing assessment index is idx_assessments_state_created on (state, created_at_epoch_ms DESC), so SQLite can equality-match state but must still scan all errored rows and sort by completed_at. As the error backlog grows, a 5-minute cron tick becomes increasingly expensive.

That violates the repo’s index discipline (see 0006_assessment_checksum_index.sql for the same rationale). Add a partial index on completed_at_epoch_ms scoped to error rows:

Suggested change
);
);
-- Supports the cron's state+completed-at scan in escalation order without a
-- full table sort. Partial because only `error` rows are candidates.
CREATE INDEX idx_assessments_error_completed
ON assessments(completed_at_epoch_ms)
WHERE state = 'error';

Comment thread apps/labeler/src/prolonged-error.ts Outdated
Comment on lines +66 to +67
await notifyProlongedError(deps, assessment);
await markPublisherNotified(deps.db, error.id, now);

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.

[needs fixing] notifyProlongedError internally uses runTrigger, which catches and logs every error. The cron then unconditionally stamps publisher_notified_at. If the failure happened before sendNotification claimed a notifications row — for example an aggregator read threw, or a D1 write failed transiently — no source row exists, but the mark is still set. The next cron tick will skip this assessment forever, so a transient outage can permanently suppress the 72h publisher notice.

Stamp publisher_notified_at only when there is evidence the send actually reached the claim stage:

Suggested change
await notifyProlongedError(deps, assessment);
await markPublisherNotified(deps.db, error.id, now);
await notifyProlongedError(deps, assessment);
const claimed = await deps.db
.prepare(
`SELECT 1 FROM notifications WHERE source_type = 'issuance' AND source_id = ? LIMIT 1`,
)
.bind(assessment.id)
.first();
if (claimed) await markPublisherNotified(deps.db, error.id, now);

This still lets the intended dedup/self-heal for undeliverable/pending/failed rows work, but it retries when the send never reached the database at all.

@github-actions github-actions Bot added review/awaiting-author Reviewed; waiting on the author to respond and removed review/needs-review No maintainer or bot review yet labels Jul 17, 2026
…send failure

Addresses the emdashbot review on #2088.

- Add a partial index idx_assessments_error_completed on
  assessments(completed_at_epoch_ms) WHERE state='error' so the 5-minute
  cron scan is index-backed instead of full-scanning error rows.
- runTrigger now reports whether the trigger reached a terminal outcome
  (sent / confirmation / undeliverable / dedup hit) versus threw a transient
  error before any notifications row was claimed. The prolonged-error cron
  stamps publisher_notified_at only on a terminal outcome, so a transient
  pre-claim failure (aggregator read or D1 write throwing) leaves the row in
  the scan window to retry next tick rather than being permanently silenced.
  An unparseable subject URI counts as terminal (never retried).

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

Copy link
Copy Markdown
Collaborator Author

Both addressed (commit 8d70467).

1. Cron-scan index — added a partial index idx_assessments_error_completed ON assessments(completed_at_epoch_ms) WHERE state = 'error' in migration 0010. It matches the scan's filter+order exactly (state='error' AND completed_at_epoch_ms <= ? ORDER BY completed_at_epoch_ms) and stays compact by only covering error rows.

2. Mark-on-transient-failure — good catch; this was a real hole. runTrigger now returns whether it reached a terminal outcome (a dedup hit, or a normal sendNotification return — sent / confirmation-sent / undeliverable / claimed-then-failed which the sweep owns) versus threw a transient error before any notifications row was claimed. The cron stamps publisher_notified_at only on a terminal outcome, so a transient pre-claim failure (aggregator read or D1 write throwing) leaves the row in the scan window and the next tick retries. An unparseable subject URI counts as terminal (permanent, never retried forever). A load-bearing test drives the publisher stage with a throwing aggregator → asserts no mark and zero notifications rows, then a healthy tick → notice claimed + mark stamped.

13/13 prolonged-error + 161 across the shared-runTrigger regression (notification-triggers/sweep, reconsideration, operational-events, reconciliation) green; typecheck + lint + format clean.

~ 🤖 Claude Fable 5

@ascorbic ascorbic added the bot:review Trigger an emdashbot code review on this PR label Jul 17, 2026
@github-actions github-actions Bot added review/needs-rereview Author pushed changes since the last review and removed review/awaiting-author Reviewed; waiting on the author to respond labels Jul 17, 2026

@emdashbot emdashbot 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.

Re-review of PR #2088: both prior emdashbot[bot] findings are resolved.

  1. Cron-scan index — migration 0010 now adds the partial index idx_assessments_error_completed ON assessments(completed_at_epoch_ms) WHERE state = 'error', which matches the findEscalatableErrors filter and ORDER BY a.completed_at_epoch_ms ASC exactly. This addresses the index-discipline concern from the previous review.

  2. Publisher mark on transient failurerunTrigger now returns a boolean terminal/transient signal, notifyProlongedError propagates it, and runProlongedErrorEscalation only calls markPublisherNotified when the trigger reached a terminal outcome. A pre-claim aggregator or D1 failure no longer permanently drops a publisher notice.

Statically, the rest of the ladder looks sound: the operator-alert insert is batched with the operator_alerted_at mark and guarded by gateOnUnalertedEscalation; supersession, deleted-subject filtering, and dropping fully-escalated rows are correct; the notice copy is public-safe; and the assessment-prolonged-error event carries only public identifiers. No AGENTS.md-level concerns around SQL interpolation, authorization, admin UI localization, changesets (labeler is private), or content-table locale filtering surfaced.

I did not find any remaining blocking issues. LGTM.

@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Jul 17, 2026
@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-rereview Author pushed changes since the last review labels Jul 17, 2026
@ascorbic
ascorbic merged commit c065482 into feat/plugin-registry-labelling-service Jul 17, 2026
11 checks passed
@ascorbic
ascorbic deleted the feat/labeler-prolonged-error branch July 17, 2026 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: signed review/approved Approved; no new commits since size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant