Skip to content

Fix lease renewal and expired-claim recovery - #72

Merged
cardmagic merged 7 commits into
mainfrom
fix/activation-lease-recovery
Oct 12, 2026
Merged

cardmagic merged 7 commits into
mainfrom
fix/activation-lease-recovery

Conversation

@cardmagic

@cardmagic cardmagic commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Two activation-lease defects were independently found and reported. Continuous short turns kept postponing the cached renewal deadline without extending the stored lease; after a turn lost that lease, release cleared its expiry while leaving its claimed message in place, hiding it from recovery.

Preserve the renewal deadline across turns and check it inside the pass before the next claim. Keep the owner, token, generation, and expiry when any message claim remains, so the existing fenced recovery transaction can reclaim it on the next pass. Stale owners still cannot release a replacement activation.

No schema, setting, default, or public API changes. Prepares version 0.17.7 in package.json and src/version.ts, with dated changelog notes and the parity ledger targeting Ruby 0.17.4. Tag v0.17.7 after merging; no tag or publication is included. A pass limit of one is no longer required to avoid renewal starvation. An expired turn commits no durable state; its replacement turn runs before later messages. Reclaiming increments attempt_count from 1 to 2 and succeeds with maxAttempts: 1. Lease loss alone emits activation.lost, then recovery.reclaimed on reclaim, without message.failed or dead_letter.created; an actual handler error still uses the existing attempt limit. Handlers can execute again, so external side effects still require idempotency.

Regression evidence, collected before the fix and again with the fix removed:

Regression Before After
34 continuous 30 ms turns across passes; 200 ms lease, 50 ms renewal activation.lost; stored lease did not advance Pass; one generation, advancing expiry, 34 single completions
Ten short turns within one pass Expected 10, received 6 Pass
Expired blocking turn, same worker, maxAttempts: 1 Recovery pass expected 2, received 0 Pass; rollback, retry, later message, attempts and events checked
Second worker takeover before stale turn returns Pass Pass; one commit per message and replacement lease retained

Validation:

  • CI timing follow-up: allow 15 seconds for the 34-turn test to complete server database round trips; the 200 ms lease, 50 ms renewal, controlled server clocks, and all assertions are unchanged. Full default and MySQL suites passed after this adjustment.
  • Release preparation: pnpm run format:check; pnpm run test:package (build, pack, install, and consumer smoke checks); node scripts/release-notes.mjs 0.17.7.
  • Node 24: pnpm run check; pnpm test (627 passed, 32 conditional skips); pnpm run build.
  • pnpm exec vitest run test/activation-lease.test.ts: 4 passed on SQLite, PostgreSQL, and MySQL in dedicated databases; no skips in these focused runs.
  • PostgreSQL/MySQL CI scripts now include the lease suite. Server tests advance the worker/database clocks together with timers paused to exclude runner scheduling delays; SQLite retains real 200 ms leases and synchronous waits. All three defect regressions also fail against the original implementation on both server adapters (takeover remains passing). Full adapter runs: PostgreSQL 59 passed; MySQL 46 passed, 7 conditional skips. Repeated lease-suite runs reuse 13 tables and leave zero application rows on both adapters, fixing the review finding that random prefixes accumulated tables.
  • Copy-only external integration probe at supplied commit: 1000 fast; failure none; activation events [].
  • Inspected effect worker, reminder scheduler, and broadcast worker: they use per-item claims and process/claim staleness, not cached actor activations. Existing outbox and recovery tests pass; no changes needed for these two patterns.

Ruby parity: cardmagic/solid-objects-ruby#89. The parity ledger and changelog are updated.

Repair two independently reported activation-lease defects. Preserve the
renewal deadline across turns and check it before the next claim. Keep
the activation fence while a claimed message remains so expired-turn
recovery can run in the next pass.

Regressions failed before the fix with activation.lost, a six-turn pass
instead of ten, and a recovery pass that processed zero messages. Verify
renewal, same-worker recovery with maxAttempts: 1, and replacement-owner
fencing on SQLite, PostgreSQL, and MySQL.
@context7

context7 Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Docs7 for cardmagic/solid-objects-js

Result Status Action
Deployment ➖ Not used —
Content review ➖ Did not run. This site has no agent runs available this month. Wait for the monthly reset or check your Docs7 plan. —

Commit 7e98084

@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Critical impact] The PR appears safe to merge; no new actionable issues were found.

Summary

Fixes activation renewal between short turns and preserves expired claims for recovery.

  • Workers keep actor leases renewed across short turns.
  • Expired message claims stay available for a later recovery pass.
  • The package reports version 0.17.7.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Turn finishes] --> B{Lease still valid?}
  B -->|Yes| C[Keep renewal deadline]
  C --> D[Renew when due before next claim]
  B -->|No| E[Reject durable changes]
  E --> F[Keep activation fence while claim remains]
  F --> G[Next pass reclaims expired message]
  G --> H[Retry before later messages]
Loading

Reviews (6) · Last reviewed commit: "test: allow time for lease database roun..." · Reviewed by Greptile

Comment thread test/activation-lease.test.ts
Reuse and reset the activation lease tables so repeated server database
runs do not accumulate tables or retain messages. Include these tests in
the PostgreSQL and MySQL CI commands to exercise both adapters.

See #72
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review the latest commit 76f5755, including the test cleanup fix for the reported table accumulation and the added PostgreSQL/MySQL CI coverage. All local checks passed; please reassess the current head.

Comment thread test/activation-lease.test.ts Outdated
Shared CI scheduling can legitimately expire a 200 ms server lease
between passes. Advance the database and worker clocks together while
holding timers so the regression depends only on the intended turn gaps.
Keep the required real synchronous timing regression on SQLite.

Verify all three defect regressions still fail on the original worker
and repository against both PostgreSQL and MySQL, then pass with the
fixes restored.

See #72
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review the latest commit 3797354. The earlier table-cleanup finding is resolved. Adding the lease tests to server CI exposed a wall-clock scheduling failure (generation advanced between passes, with no activation loss); server tests now advance the database and worker clocks together while keeping timers paused. SQLite still runs the required real synchronous waits with a 200 ms lease.

Confirmed that the three defect tests fail against the original worker/repository on both PostgreSQL and MySQL and all four pass with the fixes restored. Full local default suite, adapter suites, formatting, and checks pass. Please reassess the current head.

Replace the nested adapter ternary with separate checks to follow the
repository style without changing database selection.

See #72
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 22c4075. Both reported findings are addressed: stable test tables with cleanup and adapter selection using early returns. Commit 3797354 also controls server test clocks to remove CI scheduling sensitivity, while SQLite retains the real-time synchronous regression. Please reassess the current head.

Align package and runtime versions, date the release notes, and track
the matching Ruby 0.17.4 activation lease fixes in the parity ledger.
Prepare the release for tagging after the pull request is merged.

See #72
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head d096204, which prepares version 0.17.7. Package and runtime versions match, the changelog is dated, and the parity ledger targets the accompanying Ruby 0.17.4. Full tests, checks, formatting, package build/install smoke tests, and release-note generation pass locally. No tag or publication has been made.

The 34-turn regression can exceed Vitest default five-second timeout
on shared MySQL CI runners. Allow fifteen seconds for test execution
while keeping the 200 ms lease, 50 ms renewal, and all assertions.

See #72
@cardmagic

Copy link
Copy Markdown
Owner Author

@greptileai Please review current head 7e98084. The 0.17.7 metadata is unchanged from the reviewed preparation commit. CI exposed the default five-second test timeout in the 34-turn server regression, so it now has a fifteen-second execution budget. The 200 ms lease, 50 ms renewal, and every correctness assertion are unchanged. Full local tests, MySQL adapter suite, checks, and formatting pass.

@cardmagic
cardmagic merged commit 666dc6c into main Oct 12, 2026
20 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.

1 participant