docs: say background pickup needs run(signal) - #27
Conversation
install() migrates the schema and starts nothing. The README's
programming-model example completes without run() because the caller's
own path executes the call, so the page never had to mention roles. A
reader who builds a two-process harness from it hits a process that
registers actors, installs, and then waits forever, because nothing
polls. An external prober did exactly that against 0.14.0 and read the
unclaimed messages as stranded.
The README now says it after the model example, with the AbortController
shape a real process uses, and docs/operations.md says it in the first
paragraph. Both add the part that keeps the reader calm: nothing is lost
while no process runs, the message stays ready until one does.
test/background-pickup.test.ts keeps the sentence honest. A sent message
reads ready 200ms after install, then completed once run(signal) starts
the roles. Driving a worker before the first assertion turns it red, so
the assertion detects pickup rather than asserting a constant.
The same example addressed its actor as runtime.ref(Cart, "cart-123")
while every other reference example in the README uses the static form.
It now reads Cart.ref("cart-123").
Closes #22.
Greptile SummaryThe PR clarifies that
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "fix: make the model example configure th..." | Re-trigger Greptile |
Greptile caught that the example addressed Cart.ref("cart-123") while
building the runtime with createRuntime(), which deliberately leaves the
process default unset. A reader running the page as written would get
"SolidObjects.configure must be called before Actor.ref" before either
cart operation ran. Verified by running both variants against a built
package: configure() prints [ 1, 2 ], createRuntime() throws.
The example now calls configure(), which is what the browser quickstart
further down the page already does, so both examples address actors the
same way.
The pickup test used message.wait() to observe the second phase, and
wait() drives a caller worker through runOnce(). That path could have
completed the message even if the roles started by run(signal) never
claimed it, which is the one thing the test exists to prove. It now
polls message.status(), a read that processes nothing. Replacing
run(signal) with a resolved promise leaves the status at ready, so the
completion is the roles' work and nothing else.
|
Both findings were valid and are fixed in d6bfcf1. Static reference lacks default runtime (P1). Correct, and I had this backwards: Verified by running both variants against a built package rather than reasoning about it: Wait path masks background pickup (P2). Also correct. The test is checked in both directions:
The Ruby counterpart (cardmagic/solid-objects-ruby#52) needs neither fix: it has no |
|
@greptileai review |
Closes #22.
Why
install()migrates the schema and starts nothing. The README'sprogramming-model example completes anyway, because a direct call runs on the
caller's own path, so the page never had a reason to mention roles. A reader
who builds a two-process harness from that example gets a process that
registers actors, installs, and then waits forever, because nothing polls until
runtime.run(signal)starts the roles. An external prober did exactly thatagainst 0.14.0, watched ready messages sit for 30 seconds, and concluded
messages were stranded.
Nothing was wrong with the runtime. The page was missing one sentence.
What changed
README, right after the model example:
with the shape a real process uses:
docs/operations.md, first paragraph: the same claim, plus the boundary that
makes it make sense. A direct call or an explicit
syncneeds no running role.The same example addressed its actor as
runtime.ref(Cart, "cart-123")while every other reference example in the README uses the static form. It now
reads
Cart.ref("cart-123").createRuntimesets the default runtime, so theexample still runs as written.
Keeping the sentence honest
test/background-pickup.test.tspins the documented behavior:A test that only ever sees
readyproves nothing, so I checked that the firstassertion detects pickup. Replacing the wait with
runtime.worker().runOnce()turns it red:
Ruby counterpart
cardmagic/solid-objects-ruby#52 covers the same gap for the gem. Ruby was in
better shape already: the README has a Worker requirements table that says a
missing worker leaves the message pending. What it lacked was the pointer at
the point of use, in the
asyncsection, and the equivalent statement in theoperations guide. That PR adds both, plus the matching test.
Validation
pnpm run check: clean.pnpm test: 343 pass, 13 skipped.