Skip to content

Coordinate abandoned effect recovery - #51

Merged
cardmagic merged 7 commits into
mainfrom
feat/effect-recovery
Sep 15, 2026
Merged

cardmagic merged 7 commits into
mainfrom
feat/effect-recovery

Conversation

@cardmagic

@cardmagic cardmagic commented Sep 15, 2026

Copy link
Copy Markdown
Owner

An actor can now retain the exact effect returned by emit and opt into library-coordinated recovery. A stale processing owner is rechecked under the claim-locking protocol; retirement and the originating actor's recovery callback commit together. Fresh owners, pending retries, completed results, and dead effects retain distinct outcomes.

Closes #50. Ruby counterpart: cardmagic/solid-objects-ruby#68. Prepares version 0.15.0 with matching package/runtime metadata and a dated changelog.

Before, a watchdog could create overlapping external work:

this.emit("build_report", { arguments: { revision: ++this.revision } })
// A later watchdog has no supported ownership/reclamation transaction.
this.start()

After, onRecovery enables automatic recovery through the existing effect workers. No actor watchdog or explicit recovery request is required:

this.exportEffect = this.emit("build_report", {
  arguments: { revision: ++this.revision },
  onSuccess: "exportFinished", onFailure: "exportFailed",
  onRecovery: "recoverExport",
  recoveryTimeoutMilliseconds: 120_000,
})

The library checks owner heartbeats during effect polling and stale-process cleanup. The timeout measures heartbeat freshness, not elapsed effect duration. Only onRecovery owns replacement.

An application that also wants explicit status observations can add onStatus: "inspectExport" to the same emit call, then optionally stage a check from an actor operation:

checkExport(): void {
  if (this.exportEffect) this.requestEffectRecovery(this.exportEffect)
}

onStatus reports that requested check; it does not create a status subscription. Completed-status repair shares the guarded helper used by success, so duplicate notifications cannot apply the result twice. Full optional watchdog code and the transaction proposal are in docs/api.md and docs/effect-recovery.md.

The lock order is origin instance → effects → recovery bindings → current owner processes, with deterministic ordering within each group and database time sampled after locks. Actor-staged checks reuse the fenced transaction. Automatic recovery uses one transaction per candidate. A durable retirement binding plus the existing terminal completed storage state prevents reclamation and fences late completion/failure; callback insertion failure rolls back retirement. Bindings survive effect/message pruning until their originating instance is removed.

Compatibility: emit intentionally changes from void to EffectHandle; overrides and wrappers must return the handle. Install additive schema migration 9 and upgrade all effect/cleanup workers before opting in. Cloudflare preserves ordinary handle identity and rejects process-heartbeat recovery before commit. Retirement is not cancellation or exactly-once external execution; external actions still need idempotency.

Validation:

  • pnpm run check, pnpm test: 400 tests passed; adapter-specific skips are exercised separately.

  • Real PostgreSQL 17: pnpm run test:postgresql, 45 passed, including both claim/check race orderings, changed claimants, refreshed heartbeats, simultaneous recovery, callback rollback, and lookup uncertainty.

  • pnpm run test:cloudflare: 48 passed; pnpm run build and pnpm run test:package passed.

  • Test-first handle regression observed null instead of the persisted ID; retirement regression observed a second claim instead of a callback. Removing the instance-first completion fence reproduced deadlock detected; restored code passes.

  • CI also runs the recovery suite on PostgreSQL 14/18 and MySQL.

  • pnpm run test:recovery kills a worker after retirement commits and verifies a restarted worker delivers one durable recovery callback.

  • Automatic scans prefilter freshness using database time and respect claimScanLimit; a barrier regression verifies fresh candidates do not block unrelated pending claims. Committed retirement wakes actor workers.

  • A fresh strict TypeScript consumer installed the packed npm artifact and passed 14 end-to-end HTTP scenarios with separate worker processes. Coverage includes automatic recovery without an actor watchdog, SIGKILL, SIGSTOP/resume with late success/failure, explicit checks, retry/failure compatibility, pruning, foreign handles, and timeout rollback. The equivalent Ruby integration exposed and fixed missing in-handler heartbeats; JS already maintains them.

  • Transient heartbeat query failures now emit solid_objects.process.heartbeat_failed and retry on the next configured interval without consuming effect attempts. A failing-query regression passes after the fix and fails when it is removed. Both installed consumers also pass a real PostgreSQL stop/start while their HTTP effects remain held: the original owners resume heartbeats, explicit checks defer, and no duplicate execution occurs.

  • Cold-schema heartbeat and MySQL transmit integration cases use a 30-second test budget after repeated CI failures at the five-second default. Deterministic race barriers retain their shorter deadlines.

Return stable emit handles and retire abandoned effects atomically with durable recovery callbacks. Preserve owner heartbeat grace across cleanup and fence late completion using instance-before-effect locks.
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review the current head for recovery/claim races, transaction boundaries, callback idempotency, adapter parity, and public type safety.

@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds coordinated recovery for abandoned SQL effects and prepares release 0.15.0.

  • Returns stable effect handles from emit and supports automatic or explicitly requested recovery.
  • Persists recovery bindings and retires stale claims atomically with durable callbacks.
  • Preserves backend-specific behavior by rejecting unsupported recovery options on Cloudflare.
  • Adds migration, diagnostics, documentation, examples, and cross-adapter regression coverage.
  • The changes since the previous review only increase two integration-test execution budgets to 30 seconds.

Confidence Score: 5/5

The PR appears safe to merge; the latest changes only relax two integration-test time budgets without changing production behavior or assertions.

No new actionable defect or repository-rule violation was found. The earlier wake-up, scan-bounding, conditional-structure, and test-annotation findings are resolved at the current head; the EffectOptions.arguments finding was correctly withdrawn after confirming that annotation existed in the base. The wake-up, initial scan, timeout-conditional, and first unknown-annotation threads were manually resolved without explanatory replies.

Important Files Changed

Filename Overview
src/effect-recovery-coordinator.ts Coordinates bounded stale-owner checks, transactional retirement, durable callback insertion, and post-commit actor wake-up.
src/repository.ts Integrates recovery bindings and lock ordering into effect persistence, claims, completion, cleanup, and pruning.
src/actor.ts Returns stable effect handles and stages validated recovery options and explicit recovery requests.
src/worker.ts Keeps effect-owner heartbeats retryable after transient database failures without consuming effect attempts.
src/schema.ts Adds migration 9 and the durable effect-recovery binding table.
test/effect-recovery.test.ts Covers recovery outcomes, races, callback durability, heartbeat behavior, and now allows cold schema setup up to 30 seconds.
test/mysql.test.ts Retains the existing MySQL transmit integration while allowing its cross-database setup and execution up to 30 seconds.

Reviews (7): Last reviewed commit: "test: Allow time for cold MySQL schema s..." | Re-trigger Greptile

Comment thread src/effect-recovery-coordinator.ts
Comment thread src/repository.ts
Comment thread src/actor.ts Outdated
Comment thread test/effect-recovery.test.ts Outdated
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 8326f27. Addressed all four findings: bounded database-time freshness filtering, post-commit actor wakeups, flat timeout guards, and concrete race-test error types. Added deterministic PostgreSQL claim/check races, rollback and lookup-failure tests, and a crash-after-retirement proof. PostgreSQL: 44 passing; default suite: 399 passing; type and lint checks clean.

Comment thread test/effect-recovery.test.ts Outdated
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 38f0216. The remaining database-wrapper annotation now derives from Parameters<DatabaseConnection["get"]>[1]. Type/lint checks and all 29 PostgreSQL recovery tests pass. All prior review findings are addressed.

Comment thread src/actor.ts
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 65e21c0, which prepares 0.15.0. The last finding concerns an unchanged EffectOptions.arguments type from base 317e636; the inline reply links the original source and explains why narrowing this validated input would be an unapproved additional API break. Please assess that compatibility explanation. All introduced annotation findings are fixed. Package build and installed-package smoke checks pass.

Comment thread test/effect-recovery.test.ts Outdated
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head c677b4b. The remaining nested adapter selection now uses early returns in createTestDatabase. All introduced review findings are addressed; type/lint checks and all 44 PostgreSQL tests pass. The unchanged EffectOptions input contract remains preserved as discussed in the prior thread.

@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 81693fd. Consumer integration found that one transient database error permanently stopped heartbeat maintenance. The loop now reports the failure and retries at the next interval without consuming effect attempts. Red/green/reversal regression, 400 unit tests, 45 PostgreSQL tests, strict packed consumer checks, and a real PostgreSQL stop/start while HTTP effects remain active all pass.

@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 9de7c2c. The only changes since the 5/5 review are 30-second test budgets for the heartbeat regression with cold schema setup and the existing MySQL transmit integration. Both MySQL versions repeatedly exceeded the prior five-second default in the transmit case; recovery tests passed on rerun. The deterministic concurrency barrier deadlines and all production code are unchanged. Type checks and 45 PostgreSQL tests pass.

@cardmagic
cardmagic merged commit 9b492e3 into main Sep 15, 2026
27 of 28 checks passed
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.

Coordinate abandoned effect recovery with reclamation

1 participant