Skip to content

Worker & background jobs: id collisions, session rollback, email robustness (4 PRs) #21

Description

@ahueb

Verified defects in the worker container: the job loop, the inbound-email poller, and the notification sweeps.

Originally verified at origin/main @ b7edcbc (2026-07-30). Re-verified 2026-08-11 against the open PR-stack tip (issue-29-authorship-grounding @ b1d54da = main + #30/#31/#32); line numbers below refer to that tree. PR #31 (email-fix) fixed parts of V3 — marked below.

Priority (triage 2026-08-11)

  • Live today: V2 (a wedged job leaves a new signup on the "Building Your Profile" spinner permanently) and V4's no-rollback hazard (all three sweeps still run every 300 s and still writeget_or_create_pref inserts on a composite-PK table each cycle — even with sending quieted). V4's phantom-sent + dead-ladder halves may also be live: the 2026-08-06 prod quieting disabled email_notification_preferences rows, but the proposal_review sweep is gated on User columns instead (check_and_send_notifications:195-199, deliberate per models/email_notification.py:99-105) — verify prod user frequencies before assuming it is dormant.
  • Latent, armed by the email bring-up: V11, V3-remainder and COR-32 are unreachable today (enable_inbound_email defaults False and is unset in prod) and all three arm simultaneously at step 4 of docs/inbound-email.md ("flag on + worker recreate"). They must land before ENABLE_INBOUND_EMAIL flips, and the runbook should list them as prerequisites (relevant to PR email-fix: repair the reply-to-review email flow (merges after #30) #31).

Suggested order: V2 → V4 → V11 → V3.

PR V11 — Claim a canonical-id writer slot in the worker process (trivial; prerequisite for enabling inbound email)

Still unfixed. src/agent/ids.py:47-50 defines four slots (WRITER_ENGINE=0, WRITER_WEB=1, WRITER_GRANTBOT=2, WRITER_ENGINE_AUX=3); the module default is TsMinter(WRITER_WEB) (ids.py:111). set_default_writer_id is called in src/main.py:113 (WEB), src/agent/main.py:54 (ENGINE_AUX) and src/agent/grantbot.py:724/757 (GRANTBOT) — but src/worker/main.py has no import of src.agent.ids at all.

The worker mints on two paths, both inside the inbound-email handler: email_inbound.py:626record_pi_messagepi_inbox.py:120/151 mint_local_ts(), and email_inbound.py:590 migrate_public_thread_to_privateprivate_channels.py:269. So worker and app share residue class 1 — the exact cross-process collision R1 exists to prevent, whose documented resolution is the uq_agent_messages_run_ts conflict handler dropping one message, unrecoverable now that the DB is the only durable store.

Both paths are only reachable when enable_inbound_email=True (config.py:152, default False; worker/main.py:162) — latent today, armed at runbook step 4, which mentions no writer slot.

(The backfill scripts are not affected: they use Slack's ts and never mint.)

Fix: add a WRITER_WORKER id and claim it at src/worker/main.py entry; correct the "Three processes" count at specs/local-db-conversations.md:66-67 — now wrong three ways over (four slots defined, five with the worker, six counting REMEDIATION_WRITER_SLOT = 99 in scripts/migrate/remediate_duplicates.py:131).

PR V2 — Worker session rollback + stale-job reaper (small; live)

  • COR-17worker/main.py:100-111 except Exception: … await db.commit() with no await db.rollback() first. run_profile_pipeline shares the session and only flush()es, so any flush-time DB error poisons the transaction; the failure-path commit then raises, escapes process_job into the outer handler (:170), and nothing persists — the row keeps status='processing', last_error=None. Fix: await db.rollback() before the failure-path commit.
  • COR-18 — no reaper for orphaned processing rows (started_at is written at :49 and read nowhere in the tree); failed jobs re-queue with no backoff (all 3 attempts burn in ~15 s at worker_poll_interval=5); completed_at is set on failure (:110); the enum's 'failed' value is never written. Impact: onboarding.py:79-88 self-heals only when job is None, so a wedged job = a permanent onboarding spinner with no retry. Fix: a stale-processing reaper + exponential backoff.

PR V3 — Inbound-email poison-pill & instruction-drop hardening (small; prerequisite for enabling inbound email)

  • COR-19 — status after PR email-fix: repair the reply-to-review email flow (merges after #30) #31:

    sub-claim state
    poison objects retried forever fixed in email-fix: repair the reply-to-review email flow (merges after #30) #31 — quarantined to failed/ after MAX_S3_PROCESS_ATTEMPTS=3 (email_inbound.py:167-181, test-pinned)
    MAX_REPLIES_PER_TOKEN_PER_HOUR never enforced fixed in email-fix: repair the reply-to-review email flow (merges after #30) #31
    charset=unknown-8bitLookupError root cause intact (:343-346; codec lookup fails before errors="replace" applies). Net effect changed: a legitimate PI reply with an odd charset is now silently lost into failed/ after ~3 minutes instead of looping
    string rating → TypeError still present (:288 rating < 1 on a str; classify_reply returns raw json.loads output at :468 with no coercion) → the reply is quarantined and lost rather than filed
    list_objects_v2(MaxKeys=50) unpaginated still present (:139); starvation is now bounded by the quarantine, but lexicographic head-of-list order still governs which 50 keys a cycle sees

    New in re-verification: side effects inside process_inbound_email (SES confirmation :301, Slack post :658, private-channel migration :590) happen before db.commit() (:153) — a commit failure rolls back the DB idempotency guards but not the sent mail / Slack post, so a retry duplicates them. Fix: guard decode + coerce the rating; paginate; order side effects after commit (or make them idempotent).

  • COR-32 (remainder) — still present. When _handle_instruction can't resolve a bot token/channel it returns False (:648-656, and the blanket except at :669-671), yet the caller (:306-319) still calls mark_notification_responded and the poller deletes the S3 object → the PI instruction is silently consumed: notification retired (a resend hits the status != "sent" bail at :230), no post, no email. The inactive-agent (:557-563) and private-origin (:607-613) paths do notify the PI — this one doesn't. Fix: on a failed post, don't mark-responded and don't delete the object.

PR V4 — Email-notification transactional safety (medium; partially live today)

All four sub-claims still present; two are worse than originally filed:

  1. No rollbackrollback appears zero times in email_notifications.py. The three per-item except blocks (:208-214, :715-719, :906-910) log and fall through to the sweep-final commit (:216/:720/:911). One failure poisons the session, and the sweep-level rollback then discards rows for users whose SES send already succeeded → those users get a duplicate email next cycle with a fresh reply token.
  2. Phantom-sent — the row is created status="sent" and flushed before the SES send (:323-332 vs the send at :475-489; identical shape in _send_new_proposal_email, :962-998). On SES failure nothing clears the row → that user never receives another proposal_review email; the new_proposal email is permanently lost.
  3. "expired" never written — sole occurrence is the model comment; phantom rows are immortal and reply tokens never expire.
  4. Downgrade ladder deadconsecutive_missed has one increment site (:295), unreachable once an unanswered sent row exists (bail at :240-250), and every retirement path resets it, so it oscillates 0→1→0; MISSED_THRESHOLD=3 is unreachable and :499-523 is dead code. Additionally: a delegate reply marks only the delegate's rows (user_id == at :600-606), leaving the PI's row sent forever → the PI is silently blocked.

Fix: db.rollback() per item; commit-after-send (or a claim row); write an expiry status; retire the PI's row on delegate response.

Definition of done: each PR ships a test that fails against the pre-fix code. This issue takes worker/main.py from 0 % coverage. V11 and V3 additionally add their prerequisites to docs/inbound-email.md's bring-up checklist.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions