feat: find a message by request id or idempotency key - #57
Merged
Merged
Conversation
A caller that lost its `MessageReference` had no way to rebuild one, so
a web request that timed out, a process that restarted, or a retry that
arrived on another node could not read what the first attempt produced.
The enqueue path already deduplicates by idempotency key, so the row
could be found. Nothing public could find it.
`runtime.findBy({ requestId })` answers a request id and
`reference.findBy({ idempotencyKey })` answers a key, so the receiver
supplies the scope the key needs and a caller cannot write a lookup the
indexes cannot serve.
Every lookup runs the hook the original call ran, against the stored
operation and arguments, because a request id is not a capability. An
absent row, an actor this process no longer registers, and a caller the
policy refuses all return `undefined`, so the lookup cannot be used to
ask whether a request id exists.
`outcome()` reports the status, the result, the persisted error, the
rejection, and the attempt count.
Schema version 12 adds a unique index on `messages.request_id`. The
table constrained only `(actor_type, actor_id, request_id)`, which
cannot serve a lookup that names the request id alone. Ruby has carried
a global unique index since its first migration and needed no
equivalent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
`findBy` returned `undefined` for a message retention removed and for a
message that never existed, so a client that retried after a timeout
could not tell a lost result from a request that never arrived.
Orleans solves this in grain state: a grain keeps its deduplication
history with the grain rather than in a separate tombstone store. An
actor now does the same. The repository already writes the instance row
in the transaction that completes, rejects, or kills a turn, so the
remembered keys ride on a write that happens anyway. No second store,
no second write, and no separate retention.
`reference.findBy({ idempotencyKey })` throws `MessagePruned` for a key
the actor remembers and returns `undefined` for a key no caller ever
sent. The memory is actor state, so a caller that `authorizeQuery`
refuses reads `undefined` for both. `retainedIdempotencyKeys` bounds the
memory at 64 keys per actor. A lookup by request id cannot make the
distinction, because the runtime generates a request id and no actor
remembers one. Schema version 13 adds the column.
The previous commit added `messageOutcome` and `findBy` to
`ActorRuntime` and implemented neither on the Durable Objects backend,
so `quality`, `floor`, and `browser` were all red on one error:
Class 'CloudflareRuntime' incorrectly implements interface
'ActorRuntime'
Both now work there, including the remembered keys. A Durable Object
indexes only its own messages, so `findBy({ requestId })` without a
reference raises `UnsupportedCapability`. The parity row that still
called result lookup `Planned` now reads `Native`.
The doctor and three tests each carried a hand-copied list of schema
versions, and all four broke on version 13. They read `SCHEMA_VERSIONS`
instead.
Validation:
- pnpm run check, pnpm run build, pnpm test (515 passed)
- pnpm run test:cloudflare (56 passed)
- SOLID_OBJECTS_DATABASE_URL=postgresql://... pnpm run test:postgresql
- SOLID_OBJECTS_DATABASE_URL=mysql://... pnpm run test:mysql
Each new test was watched to fail first. Before the runtime work:
`TypeError: this.runtime.findBy is not a function` on Durable Objects,
and `no such column: completed_idempotency_keys` on SQL. Without the
authorization gate: `MessagePruned: the message for idempotency key
"checkout-7f3a" was pruned` where `undefined` was expected. Without the
bound: `expected [ 'key-0', 'key-1', 'key-2', …(2) ] to deeply equal
[ 'key-2', 'key-3', 'key-4' ]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A parity audit against the Ruby branch found three gaps. The Durable Objects `lookup` gates on `authorizeQuery` before it reads, so `findBy` threw `Unauthorized` there while the SQL runtime answers `undefined`. A lookup that throws where it is refused and answers absent where the row is gone is a way to ask whether a key exists, which is the guarantee this feature documents. `findBy` now answers absent on both backends, and the portable runtime contract holds both to it. Without the catch the contract fails with `Unauthorized: message lookup is not authorized`. The doctor passed on an instance table without `completed_idempotency_keys`, so a half-applied version 13 read as healthy. Without the column in `EXPECTED_COLUMNS` the new test reads back `schema matches this runtime`. The changelog did not say the doctor verifies the new column. A test proves two keyed messages in one activation pass are both remembered, because the repository reads the locked instance row rather than the claim-time copy on the turn. Validation: pnpm test (518), pnpm run test:cloudflare (57), check, build, format:check, PostgreSQL 18, MySQL 8.4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A parity audit found the same untested branch in both languages. A key can finish twice on one actor: retention removes the message, the enqueue path no longer deduplicates it, and the caller sends it again. The list must move the key to the end rather than repeat it, and nothing proved that. Without the move the new test reads `[ 'first', 'second', 'first' ]`. Validation: pnpm test (519 passed), check, format:check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An idempotency key has no length limit: the column is TEXT on SQLite and PostgreSQL. A probe confirmed a 1000-character key is remembered whole. Before this branch such a key sat in one message row that retention removed. The remembered list holds 64 of them on the instance row, which survives as long as the actor, so the growth became unbounded and persistent. On Durable Objects it also reached `encodedRecord`, which throws `PayloadTooLarge` above the SQLite row limit from inside the commit transaction. `retainedIdempotencyKeysBytes` bounds the serialized list at 16 KB on both backends. An actor drops its oldest keys until the list fits, so a key long enough to fill the limit by itself is never remembered and its lookup answers `undefined` rather than throwing. Validation: pnpm test (521), pnpm run test:cloudflare (58), check, build, format:check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five findings. The missing Cloudflare `findBy` and `messageOutcome` were already implemented on this branch; the other four were valid. `messageOutcome` read the row and then read the status separately, so a worker finishing between the two produced an outcome no snapshot ever held. `repository.messageWithStatus` returns both from one connection, and a test breaks `messageStatus` to prove the outcome no longer needs it. `readableMessage` and `readableState` caught every exception and reported absence, so an authorization service outage was indistinguishable from a missing row. They catch `Unauthorized` and `UnknownActorType` only. Without that the new test reads `promise resolved "undefined" instead of rejecting`. `Outcome<Result = unknown>` broke the rule against `unknown` annotations; it defaults to `JsonValue`, matching `invoke` and `sendMessage`. The test's `argumentsValue` is a `JsonObject`. The uniqueness test updated a row whose id was "impossible", so it changed nothing and passed with or without the index. It now assigns one message's request id to a second real message and asserts the database refuses. Validation: pnpm test (523), pnpm run test:cloudflare (58), check, build, format:check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`messageStatus` repeated the row read that `messageWithStatus` already performs, and `statusOf` carried a leftover scope block from the extraction. It reads through `messageWithStatus` instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `quality` job failed on a test that passed on the same commit in the
sibling run:
expected MessageFailed / PayloadTooLarge
received SyncTimeout, status "ready", timeoutMilliseconds 402
The test drove an oversized record through a synchronous call with a
500 ms budget. Moving about 1 MB through a Durable Object under CI load
takes longer than that, so the caller gave up before the turn reached
`encodedRecord` and the assertion read a timeout instead of the rollback
it was written for. The budget was never part of what the test asserts.
It now sends the message, polls until the message is dead, and reads the
stored error through `outcome()`, so no deadline can decide the result.
That also exercises `messageOutcome` on the Durable Objects runtime,
which this branch added.
The test dates from the original Durable Objects backend and the flake
predates this branch.
Validation: five runs of the Cloudflare suite, four of them concurrent,
all 20 tests passing. Raising the record limit to 99_999_000 makes the
test fail, so it still reports the defect it names.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Four valid findings from Greptile.
The pruned answer authorized the synthetic `__snapshot__` query while a
surviving row authorizes its stored operation, so a caller allowed to
read state but denied the operation could tell `MessagePruned` from
`undefined` and learn the operation had run. Each remembered key now
carries its operation, and `readableOperation` runs the same hook both
paths use. Without it the new test resolves where it must reject.
The Durable Objects lookup demanded a synthetic
`authorizeQuery("__lookupMessage__")` before authorizing the stored
operation, so a policy that allows only declared queries refused an
otherwise authorized lookup and diverged from the SQL runtime. The
synthetic check is gone; the stored operation is still authorized.
The Durable Objects `PayloadTooLarge` fallback marked a message dead
without remembering its key, so that one terminal path answered absent
where every other dead message answers pruned.
`unknown` appeared in a catch annotation and two assertion chains.
`errorRecord` and `rejectionRecord` build the values from validated
fields instead, and `rememberedList` parses to a declared type.
Validation: pnpm test (524), pnpm run test:cloudflare (58), check,
build, format:check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`parity.md` said distributed rate limits and global admission control "do not exist yet, matching the open Ruby roadmap item". That roadmap item is gone: the three scaling limits are hot, request-critical, and loss-tolerant, and every invocation writes one permanent message row, so `fit.md` already ruled out checking on the request path. Solid Objects Pro answers them with grouped and ephemeral operations, and both documents now say so rather than pointing at a milestone. The README listed "Rate limits and account quotas" among the fits. The low-rate quota is the case that fits, so it reads that way now. Validation: pnpm test (524), check, format:check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Retain original arguments within the existing byte bound and deny\nlegacy entries that cannot reproduce authorization. Remove the\nlookup reference cast and cover SQL and Durable Object policies. See #57
Owner
Author
|
@greptileai Please review the latest commit 3293548. All outstanding findings have been addressed, including original-argument authorization for pruned lookups on both backends. |
Handle missing lookups and invalid references with guard clauses, as requested in the PR review. The full 59-test Cloudflare suite and Cloudflare type check pass. See #57
Owner
Author
|
@greptileai The remaining nested lookup conditional is flattened in 0fbdec6. All 59 Cloudflare tests and the Cloudflare TypeScript check pass. Please review the latest commit. |
A parity audit against the Ruby branch found the SQL runtime handing out mutable values where Ruby and the Durable Objects runtime both freeze. `messageOutcome` built its result with `normalizeJson`, which copies but does not freeze, while `messageResult` beside it, the Cloudflare path, and Ruby all use a read-only copy. A caller could mutate the nested hashes and arrays of a durable result, and the error and rejection records were not frozen either. Greptile reported this shape on the Ruby branch and it was fixed there; the same defect on this side went unreported. Without the fix the new test reads `expected false to be true`. Two tests close gaps the audit found in this file: a frozen result, and a message whose actor type this process no longer registers, which already answered absent but nothing held it there. The error record differs between the runtimes and the parity document did not say so. Ruby carries `class_name`, `message`, and `backtrace`; this runtime persists only `name` and `message`, because `safeError` has never stored a stack. Validation: pnpm test (528), pnpm run test:cloudflare (59), check, build, format:check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports cardmagic/solid-objects-ruby#79, which works on #74 there.
Why
A caller reads a result through a
MessageReference, and a caller who losesthat reference cannot rebuild one. A web request that timed out, a process that
restarted, or a retry that arrived on another node knows its own request id or
its own idempotency key, and neither was enough.
The enqueue path already deduplicates by idempotency key, so the row could be
found. Nothing public could find it.
The receiver carries the scope
requestIdruntimeidempotencyKeyNaming neither key, naming both, or naming an idempotency key without a
reference each throw a
TypeError.Authorization
A request id is not a capability, so every lookup runs the same message or query
hook the original call ran, against the stored operation and arguments.
An absent row, an actor this process no longer registers, and a caller the
policy refuses all return
undefined. A lookup that threw where it was refusedand returned
undefinedwhere it was absent would be a way to ask whether arequest id exists.
Outcome
A lookup that only answered for successes would not be usable for recovery, so
outcome()carries the persisted error for a dead message and the rejection fora rejected one, beside the status and the attempt count.
Schema version 12
This runtime needed the index the issue asks for.
messagesconstrained onlyUNIQUE (actor_type, actor_id, request_id), which cannot serve a lookup thatnames the request id alone. Version 12 adds a unique index on
request_id.Every value is a fresh
randomUUID(), so existing rows cannot collide.Ruby needed no equivalent: its first migration has always carried
index :request_id, unique: true.Two differences from the Ruby port, both recorded in
docs/parity.mdThis runtime stores a result for every completed message, so a lookup
answers one for asynchronous work. Ruby serializes a result only when
delivery_mode == "sync", so a lookup there answers the status, the error, andthe rejection, but not the result of async work. A test pins the behaviour on
each side rather than leaving it to be discovered.
The index above, which Ruby did not need.
Tests
test/result-lookup.test.ts, 25 tests, each watched failing first:lookup by request id and by idempotency key, the runtime and the reference
agreeing, a message that has not run, a dead message with its error and attempt
count, a rejected message with its rejection, a completed outcome with its
result, an async message reporting its result, an unknown request id and an
unknown key, all three
TypeErrorcases, a refusing runtime receivingundefinedthat absence cannot be told from, authorization against the storedoperation and arguments, the query hook for a getter, a key scoped to its own
actor, and a rebuilt reference that waits and sees the commit.
The pruned tests: a pruned key raising for a completed, a rejected, and a dead
message, the error naming its key, a refused caller reading
undefined, thebound holding at three keys, and a keyless message remembering nothing. Without
the raise they failed with
expected promise to reject; without the gate therefused caller received
MessagePruned; without the bound the memory held[ 'key-0', 'key-1', 'key-2', …(2) ].test/cloudflare/recovery.test.tsrepeats the pruned case, the bound, and theunsupported request id lookup against a real Durable Object.
Validation
vitest runtscpnpm run buildA pruned message and one that never existed
The issue also asks that the two be told apart. Orleans answers this in grain
state: a grain keeps its deduplication history with the grain, not in a separate
tombstone store. An actor now does the same.
The repository already writes the instance row in the transaction that
completes, rejects, or kills a turn, so the remembered keys ride on a write that
happens anyway. No second store, no second write, and no separate retention.
retainedIdempotencyKeysbounds the memory and defaults to 64 keys for eachactor. A key the actor already remembers moves to the end rather than repeating,
and a message that carried no key remembers nothing.
Each remembered key retains the original operation and arguments. Pruned
lookups apply the same message or query authorization policy as live lookups.
Arguments count toward the existing serialized byte limit. Older entries
without arguments return absence because authorization cannot be reproduced.
A denied caller cannot distinguish a pruned key from an unused key.
Only a key lookup can make the distinction. The runtime generates a request id,
so no actor remembers one, and
findBy({ requestId })answersundefinedinboth cases.
Schema version 13 adds
instances.completed_idempotency_keys.The Durable Objects backend was left red
The previous commit added
messageOutcomeandfindBytoActorRuntimeandimplemented neither on
CloudflareRuntime, so three CI jobs failed on oneerror:
Both now work there, including the remembered keys, which live in the same
instance record. A Durable Object indexes only its own messages, so
findBy({ requestId })without a reference raisesUnsupportedCapability;every other form works. The portable runtime contract now covers
findByandoutcome(), so both backends answer the same test.The parity row that still called result lookup
Plannednow readsNative.Four hand-copied schema version lists
The doctor and three tests each carried
1, 2, ... 12by hand, and all fourbroke on version 13. They read
SCHEMA_VERSIONSfromsrc/schema.tsinstead.Review fixes and regression evidence
Argument-dependent authorization is covered by a regression that first failed
with
MessagePruned: the message for idempotency key "protected" was prunedfor a denied caller. The fixed test verifies absence for that caller and
MessagePrunedfor an authorized caller. Legacy history without argumentsalso returns absence. Serialized size tests account for the retained arguments.
Validation: JavaScript suite 525 passed / 32 skipped, then the focused suite
34 passed after adding the legacy regression; Cloudflare suite 59 passed.
Main, examples, Cloudflare, and all build TypeScript configurations pass;
parameter, documentation, browser-import, and changed-file formatting checks pass.