fix: durable commit retries — journal, replay, and /v1/events fallback (v0.5.0) - #89
Conversation
Committed spend could vanish: a transiently failing commit lived only in an in-memory daemon thread (or an unreferenced asyncio task), so any process exit — even a clean one — dropped it. Once the reservation's grace period elapsed (max 60s, default 5s), the server's expiry sweep returned the reserved budget to the pool and the ledger permanently under-counted spend that had already happened. No crash was required: the default retry schedule (~15.5s) plus a frozen expires_at meant a short server outage produced the same silent loss. Fixes: - New runcycles/journal.py: file-per-commit CommitJournal (atomic temp+rename write, delete on terminal outcome). Every scheduled retry is journaled first; the first engine per journal directory replays surviving entries for its base_url on the next run. Corrupt files are renamed *.corrupt for operator triage. - Event fallback: a commit answered RESERVATION_EXPIRED (budget already back in the pool) is recovered via POST /v1/events — the spec's post-hoc direct-debit endpoint — reusing the commit idempotency key and tagging metadata with recovered_reservation_id/recovery_reason. Wired into both lifecycles and both streaming context managers, for first-attempt and retry-time expiry alike. - atexit flush: sync retry threads get a bounded window (retry_flush_timeout, default 10s) to finish on clean exit; whatever remains stays journaled. - AsyncCommitRetryEngine now holds task references until completion (previously eligible for GC mid-flight) and journals when no event loop is available instead of dropping. - retry_enabled=False now journals instead of silently dropping (old behavior only when the journal is also disabled). Config: journal_enabled (default true), journal_dir (default ~/.runcycles/commit-journal), retry_flush_timeout; env CYCLES_JOURNAL_ENABLED / CYCLES_JOURNAL_DIR / CYCLES_RETRY_FLUSH_TIMEOUT. 460 tests pass at 100% coverage; ruff and mypy --strict clean. AUDIT.md, CHANGELOG.md, README.md updated.
Minor bump (not patch): this release adds new public API surface (journal_enabled/journal_dir/retry_flush_timeout config fields, runcycles.journal module, engine flush()/schedule_event()) and new default runtime behavior (on-disk commit journal, atexit flush, POST /v1/events recovery), plus the previously unreleased TENANT_CLOSED/LIMIT_EXCEEDED error-code support. - pyproject.toml: 0.4.3 -> 0.5.0 - CHANGELOG.md: [Unreleased] -> [0.5.0] - 2026-07-27 - AUDIT.md: header dated entries stamped v0.5.0
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| engine.flush(timeout=5.0) # clean up before the test ends | ||
|
|
||
| def test_atexit_hook_flushes_registered_engines(self, tmp_path: Path) -> None: | ||
| import runcycles.retry as retry_mod |
| logger.warning("Skipping corrupt journal entry: %s", path, exc_info=True) | ||
| try: | ||
| path.replace(path.with_suffix(".corrupt")) | ||
| except OSError: |
| _live_engines.add(engine) | ||
| if not _atexit_registered: | ||
| atexit.register(_flush_all_engines) | ||
| _atexit_registered = True |
…, flush deadline Addresses all four review findings: 1. [P1] Replay isolated by credentials: journal records now live in a per-identity subdirectory keyed by a non-secret truncated SHA-256 fingerprint of (base_url, api_key). Co-located clients with different servers or API keys can no longer replay each other's records — a foreign record previously got 401/403 and was discarded as terminal, permanently losing the spend. 2. [P1] HTTP 429 / LIMIT_EXCEEDED is transient, not terminal: both the commit and event classifiers now detect rate limiting (status 429 or error code), retain the journal entry, keep retrying, and make the next attempt wait at least the server's Retry-After (consumed once, max'd against the normal backoff). Consistent with ErrorCode.is_retryable, which already classified LIMIT_EXCEEDED retryable. 3. [P2] Replay claim scoped to the identity subdirectory: because the claim now covers exactly one (server, credential) identity, an engine for server A can never block server B's entries from replaying out of a shared journal_dir. 4. [P2] Process-wide flush deadline: _flush_all_engines() computes one deadline from the max engine timeout and passes each engine only the remaining budget, so worst-case shutdown is retry_flush_timeout, not engine_count x retry_flush_timeout. No journal-layout migration needed — v0.5.0 is unreleased. 469 tests pass at 100% coverage; ruff and mypy --strict clean. AUDIT.md, CHANGELOG.md, README.md updated.
|
All four findings addressed in 17799c1 — thanks, all confirmed valid:
No journal-layout migration needed since v0.5.0 is unreleased. 469 tests at 100% coverage; ruff + mypy --strict clean. Docs (README/CHANGELOG/AUDIT) updated to describe the identity partitioning, 429 semantics, and process-wide flush bound. |
| def test_flush_all_engines_with_no_engines(self) -> None: | ||
| import weakref | ||
|
|
||
| import runcycles.retry as retry_mod |
| # whole process, not per engine. | ||
| import weakref | ||
|
|
||
| import runcycles.retry as retry_mod |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
… identity, journal permissions 1. [P1] Rate-limited first commit no longer releases the reservation. All four lifecycle variants (sync/async lifecycle, sync/async streaming) previously routed a first-attempt 429/LIMIT_EXCEEDED into the generic client-error branch, which released the reservation — actively returning reserved budget for spend that already happened. They now detect rate limiting before that branch and schedule the commit for retry, passing the response's Retry-After into the engine via a new schedule(..., retry_after_ms=) parameter that seeds the first backoff. 2. [P1] Auth failures retained + rotation-safe journal identity. 401/403 on a retried commit or event fallback is now terminal for the current run but retains the journal entry (previously discarded — the only durable record of the spend was destroyed by a misconfigured or mid-rotation key). The identity fingerprint now uses the configured tenant as the principal when set — stable across API-key rotation, and any same-tenant credential can settle the records — falling back to the API key when no tenant is configured (documented, with manual file-move migration since replay is idempotent). The two fixes compose: with 401/403 retained, any residual identity mispartition is noise, not loss. 3. [P2] Journal directories are created 0700 and record files 0600, best-effort (no-op semantics on platforms without POSIX modes; a chmod failure never blocks the write). Records carry subjects, spend amounts, metrics, and arbitrary commit metadata. 481 tests pass at 100% coverage; ruff and mypy --strict clean. AUDIT.md, CHANGELOG.md, README.md updated.
|
Round-2 findings addressed in 0e9e0e7 — all three confirmed valid:
481 tests at 100% coverage; ruff + mypy --strict clean. README/CHANGELOG/AUDIT updated. |
…a-hashing) CodeQL flagged auth_fingerprint hashing the API key with bare SHA-256. For a high-entropy key that is not exploitable, but the no-tenant fallback can embed a user-chosen (potentially weak) key, and the directory name is world-visible metadata. The fingerprint is now PBKDF2-HMAC-SHA256 (600k iterations, fixed salt derived from base_url for cross-process determinism), truncated to 16 hex chars as before. An lru_cache pays the KDF cost once per identity per process. Fingerprint values change; no migration concern — v0.5.0 is unreleased. 481 tests pass at 100% coverage; ruff and mypy --strict clean.
…sisted Retry-After, KDF cost, unique temp files 1. [P1] First-attempt 401/403 no longer releases. All four lifecycle variants now handle authentication failures like first-attempt 429: journal the commit via the retry engine (whose auth-retention path keeps the record) and never release — releasing returned budget for spend that already happened, or dropped the commit unjournaled when the release also failed. 2. [P2] Retry-After survives restarts. The floor is persisted in the journal record as an absolute not_before_ms (set at schedule time and re-journaled whenever a retry sees a new 429 with Retry-After) and restored as a relative delay on replay; a floor already in the past falls back to normal backoff. 3. [P2] PBKDF2 rounds reduced 600k -> 30k (~20 ms cold vs ~0.36 s) and the identity cache grown to 256. The principal is normally a high-entropy machine credential, so rounds only defend the weak-key fallback; password-storage round counts stalled engine setup and blocked async callers. PBKDF2 is kept (rather than a fast keyed HMAC) because CodeQL's py/weak-sensitive-data-hashing distinguishes computationally expensive algorithms and a fast HMAC risks re-triggering the alert cleared in 462d668. 4. [P2] Journal temp files use unique per-writer names (<name>.<pid>.<random>.tmp) so concurrent processes settling the same reservation cannot truncate each other's temp file or atomically publish partial JSON that the corrupt-file handler would quarantine. Failed publishes clean up their temp file; stale temp files from crashed writers are invisible to replay (*.json glob). 491 tests pass at 100% coverage; ruff and mypy --strict clean. AUDIT.md, CHANGELOG.md, README.md updated.
|
Round-3 findings addressed in 5afef2c — all four confirmed valid; one implemented with a different mechanism than suggested (finding 3, rationale below):
491 tests at 100% coverage; ruff + mypy --strict clean. README/CHANGELOG/AUDIT updated. |
| except OSError: | ||
| try: | ||
| tmp.unlink(missing_ok=True) | ||
| except OSError: |
…4xx, delay clamps From the five-way adversarial self-review of the durability rollout: - Filename sanitization is ASCII-explicit (was Unicode-aware via str.isalnum), matching TS/Java: sibling SDKs sharing a tenant identity directory must compute identical filenames or a record they settle can never be discarded and replays forever (cross-SDK P1). - The two cross-SDK PBKDF2 fingerprint vectors are pinned in this suite (the reference SDK previously asserted only stability, so a derivation drift would pass here and break TS/Java interop). - Whitespace-only tenant falls back to the key principal (matches Java's isBlank; previously landed in a different identity dir than Java). - Honored Retry-After and restored journal floors clamped to 1 hour. - HTTP 410 triggers the expired/event-fallback path by status, so a proxy-mangled body cannot route an expired commit into release/discard. - Unclassifiable 4xx (codeless or forward-compat unknown code) is no longer a genuine rejection anywhere: engine retains the journal entry; all four lifecycle wirings journal instead of releasing. - Base journal directory also permission-tightened; stale temp files from crashed writers reaped after 1 hour. 506 tests pass at 100% coverage; ruff and mypy --strict clean.
| """ | ||
| try: | ||
| path.chmod(mode) | ||
| except OSError: |
| try: | ||
| if tmp.stat().st_mtime < cutoff: | ||
| tmp.unlink(missing_ok=True) | ||
| except OSError: |
|
Fleet-wide adversarial self-review (5 reviewers: one per PR + cross-SDK consistency) — fixes landed in f4c7d6e:
506 tests at 100% coverage; ruff + mypy strict clean. |
2 participants
Problem
Committed spend could vanish.
CommitRetryEngine.schedule()held pending commits only in in-memory daemon threads (async: unreferenced asyncio tasks — eligible for GC mid-flight). Any process exit, even a clean one, dropped them. Once the reservation's grace period elapsed (max 60s, default 5s), the server's expiry sweep (expire.lua) returned the reserved budget to the pool — permanently under-counting spend that had already happened.No crash was required: the default retry schedule totals ~15.5s against a frozen
expires_at(the heartbeat stops before commit), so a short server outage produced the same silent loss. Andretry_enabled=Falsedropped failed commits with only a warning.Fix
Durable journal (
runcycles/journal.py) — every commit scheduled for retry is journaled to disk first (file-per-commit, atomic temp+rename, default~/.runcycles/commit-journal) and deleted only on a terminal outcome. The first engine created per journal directory replays surviving entries for itsbase_urlon the next run. Corrupt files are renamed*.corruptfor operator triage. Journal I/O is best-effort and never breaks the commit path.POST /v1/eventsrecovery — a commit answeredRESERVATION_EXPIRED(budget already returned to the pool) is re-recorded via the spec's post-hoc direct-debit endpoint: spec-conformantEventCreateRequest, commit idempotency key reused (separate server-side namespace → exactly-once across restarts and shared journal dirs),metadata.recovered_reservation_id/recovery_reasonmarkers for reconciliation, nooverage_policy(spec defaultALLOW_IF_AVAILABLEnever rejects). Wired into both lifecycles and both streaming context managers, for first-attempt and retry-time expiry alike.RESERVATION_FINALIZEDstill treated as settled.Clean-exit flush — a process-wide
atexithook gives in-flight sync retry threads a bounded window (retry_flush_timeout, default 10s) to finish; anything unfinished stays journaled.Async engine fixes — task references held until completion (GC hazard); no-event-loop schedules journal instead of dropping.
retry_enabled=Falsenow journals for later replay; the old drop behavior remains only when the journal is also disabled.Config
journal_enabled(defaulttrue),journal_dir(default~/.runcycles/commit-journal),retry_flush_timeout(default10.0); envCYCLES_JOURNAL_ENABLED/CYCLES_JOURNAL_DIR/CYCLES_RETRY_FLUSH_TIMEOUT.Note the default-on behavior change: the SDK now writes journal files, registers an atexit hook, and can call
POST /v1/events.journal_enabled=Falserestores fire-and-forget.Release
Bumps to v0.5.0 (minor, not patch — new API surface + new default runtime behavior), shipping together with the previously unreleased
TENANT_CLOSED/LIMIT_EXCEEDEDsupport already in[Unreleased].Verification
mypy --strictcleantests/test_journal.py: journal roundtrip/corruption/IO-failure paths, engine durability (journal on schedule, discard on terminal outcomes, retain on exhaustion), expired→event fallback (sync + async), replay (once-per-dir, base_url filter, event-mode entries, deferred async replay), atexit flush, and lifecycle/streaming wiringReview hardening (post-review commit)
journal_dir/<sha256(base_url + principal)[:16]>/(principal = configured tenant when set, else the API key), so co-located clients with different servers or principals never replay each other's records, and one identity's replay claim cannot starve another's.LIMIT_EXCEEDEDis transient everywhere — including the first commit attempt (previously released the reservation); journal entry retained, next attempt waits at least the server'sRetry-After.0700, files0600where supported.retry_flush_timeoutbounds total shutdown wait, not per engine.