Repository navigation
design(id): #806 signed journal replicas - #3822
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 12 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 085d064f74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - The replica reuses the restore/program reconstruction path in | ||
| `world_store::open_world`, but implementation must add a read-only open path: | ||
| validating an already-verified replica must not use the writer's mutating | ||
| `FileJournal::open` recovery/header-bump behavior. The replica exposes only |
There was a problem hiding this comment.
When a snapshot is taken while participants remain connected to the writer, reusing the existing restore path changes program state beyond the signed cursor: WorldCore::restore (pkgs/id/src/world/core.rs:425-439) synthesizes leave events for every still-present participant, and restore_program feeds those events into the program. This affects actual records—for example, the territory program changes players' online status on presence events—so the replica would violate the promised writer-record parity even with an intact journal. Specify a replica restore mode that suppresses writer crash-recovery presence cleanup, alongside the non-mutating journal open path, and test parity with source sessions still connected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ccf34b (design doc amendment, task 5832 worker). Verified the cited behavior at current main: WorldCore::restore synthesizes presence-off events into the replay for still-present participants. The design now specifies a presence-preserving replica restore mode (Replica lifecycle section): presence at the snapshot cursor is restored as session state without feeding synthesized presence events into the program, no synthetic event is journaled or counted against the verified cursor, and Phase 4 acceptance includes a snapshot taken with participants connected proving Records parity with the writer at the same cursor.
| - Replicating unrelated world stores (directory, petnames, or outbox) unless a | ||
| separately demonstrated spectator-serving dependency requires them. |
There was a problem hiding this comment.
The spectator-serving dependency on the directory already exists: WorldCore::effective_scopes (pkgs/id/src/world/core.rs:789-806) checks account ceilings and requires a friendship for cross-account delegations, but the journal carries only account subjects, not those directory facts. Consequently, restoring the proposed journal-and-module replica with an empty directory rejects a valid friend-delegated spectator capability even while fully synchronized; the existing friendship-delegation test in world/tests.rs:2881-2950 demonstrates that snapshot access depends on this relationship. Resolve this dependency in the transfer and lifecycle contract—for example, by synchronizing the necessary authorization projection or supplying writer-signed spectator admissions—and test admission using an account-backed delegated capability.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ccf34b (design doc amendment, task 5832 worker). Verified at current main: WorldCore::effective_scopes (pkgs/id/src/world/core.rs:896) intersects directory account ceilings and zeroes cross-account delegations when directory.are_friends is false, and the journal carries only account subjects. The Non-goals section now scopes the directory partially IN: the directory facts effective_scopes consults (account ceilings + friendship graph for accounts in capability chains) are carried in the signed snapshot (bound into the snapshot digest) and kept current by the signed stream, and Phase 4 acceptance includes the friendship-delegation spectator case served by a replica with no writer contact.
… replica restore + directory facts for spectator auth
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cff3f74a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| therefore use a presence-preserving mode: presence at the snapshot cursor | ||
| is restored as session state without feeding synthesized presence events | ||
| into the program, and no synthetic event is ever journaled or counted |
There was a problem hiding this comment.
If the writer crashes with participants connected, the presence-preserving replica reconstructs different records from the restarted writer. WorldCore::restore synthesizes leave events (pkgs/id/src/world/core.rs:425-439) and restore_program applies them, but neither journals those events nor forces a checkpoint. The writer can therefore resume streaming the identical cursor while its records mark participants offline and the replica still marks them online. Unlike the earlier live-writer restore issue, this concerns an unreplicated state change during the writer's own restart. Specify that source recovery effects are durably journaled or captured in a checkpoint/reset before replication resumes, and test a writer crash with connected participants.
Useful? React with 👍 / 👎.
Summary
Decision-ready design for F12.13 journal replication and read-only replicas.
Validation
git diff --check origin/main...HEADpassed.Closes no issue; #806 remains open.
Need help on this PR? Tag
@codesmith-botwith what you need.