Skip to content

Interval 1: worker reliability fixes and replay delivery reporting - #13

Closed
quangshuynh wants to merge 3 commits into
mainfrom
audit/interval-1-reliability
Closed

quangshuynh wants to merge 3 commits into
mainfrom
audit/interval-1-reliability

Conversation

@quangshuynh

Copy link
Copy Markdown
Member

Summary

Interval 1 reliability audit. Fixes six confirmed defects, each with a regression test that was watched failing before the fix. No schema change, no public API shape change.

Worker and delivery (b7ac225)

  • The worker now verifies the schema revision on startup, as the API and CLI already do and as docs/operations/worker.md already claimed. It exits 1 instead of polling.
  • Every outbound attempt is held to an overall deadline of the connect and read timeouts combined. Before, each client timeout bounded only one wait, so a destination sending a byte at a time could hold a request open indefinitely. With 0.25 s timeouts a trickling receiver held an attempt for 20+ s. That outlives the lease (causing duplicate sends) and stalls the whole batch and worker shutdown.
  • The response is streamed and its body is never read, so a destination can't control how much memory an attempt uses.
  • The read transaction that load_submissions opens is ended before the HTTP calls go out, so no transaction is held across outbound requests.
  • If one completion fails to record (for example a dropped DB connection), the worker still records the rest of the batch. Before, it abandoned them all, and every one was sent again after its lease ran out.
  • The worker logs a warning at startup when FORMS_WORKER_LEASE_SECONDS does not outlast one attempt.

Ingestion (d7bcf89)

  • An idempotent replay now reports delivery.queued from the stored submission instead of the endpoint's current webhook configuration. Before, a replay was wrong whenever the webhook had been added or removed since the original request.

The docs (worker, webhooks, delivery semantics, configuration reference, idempotency) are updated to match. Delivery is still at least once; nothing here claims exactly once.

Verification

  • pytest: 609 passed, 84 skipped (the PostgreSQL tests skip without a database)
  • ruff check ., ruff format --check ., mypy, mkdocs build --strict: clean
  • The PostgreSQL integration suite was not run locally because Docker Desktop's engine would not start on the dev machine. This PR relies on CI's postgres-integration job for that coverage.

Not in this PR (tracked for later intervals)

  • Tag v0.2.1 points at a commit whose __version__ is still 0.2.0, and it has no release notes
  • No Dockerfile, compose file or deployment guide
  • /health is liveness only, with no readiness check
  • A delivery that crashes the worker before its attempt is recorded is reclaimed indefinitely without the attempt being counted
  • A streaming export holds a database transaction for the whole download

🤖 Generated with Claude Code

quangshuynh and others added 3 commits October 17, 2026 11:32
- Verify the schema revision on worker startup, as the API and CLI already
  do and as docs/operations/worker.md claimed. The worker now exits 1
  instead of polling against a database it was not built for.
- Hold every outbound attempt to an overall deadline of the connect and
  read timeouts combined. The client timeouts each bound a single wait, so
  a destination trickling bytes could keep a request open indefinitely,
  outliving the lease and stalling the batch and worker shutdown.
- Stream the response and never read its body, so a destination cannot
  dictate how much memory an attempt uses.
- End the read transaction opened by load_submissions before sending, so
  no transaction is held across outbound HTTP calls.
- Record the rest of a batch when one completion fails to record, instead
  of abandoning every remaining result and forcing duplicate sends.
- Warn at startup when FORMS_WORKER_LEASE_SECONDS does not outlast one
  attempt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An idempotent replay reported delivery.queued from the endpoint's current
webhook configuration. If the webhook was added or removed after the
original request, the replay claimed a delivery that did not exist, or
denied one that did. It now reports whether the stored submission owes a
delivery.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI now installs SQLAlchemy 2.1, where Select is generic over a variadic
row type, so the existing bound Select[Any] no longer accepts a two-entity
select and mypy fails in list_submissions. Bound the type variable by a
small protocol for "has where()" instead, which type checks on both 2.0
and 2.1 (pyproject allows sqlalchemy>=2.0). No runtime change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant