[Mathih13]

The test that proves the Companion Order transport across World Shards lived in the playerbots testsuite. It reached Gateway private modules (accept, config, the stdb bindings, codec and the durable test support). Core admitted that file only when LYRACORE_COORDINATOR_TEST_SOURCE named it, and only packages CI ran it. The transport is Core's, so a Core change could break it and Core CI would stay green.

Now Core owns the test. A Module built with debug_reducers and no Package handler falls back to a neutral fixture command, fixture.command, with a bot guid as payload and fixture.command.result as the reply. Its apply step returns Applied and writes nothing, so every other outcome comes from the transport. A Package handler always wins over it.

gateway/src/stdb/subscriptions_party_command_durable_tests.rs builds three World Shards and Realm-core on one Standalone. The companions are plain session-less Characters on the seeded Account, and each leader has its own Account and Account Claim. Six tests cover these cases:

• Realm-core certification refuses a roster that changed after the Gateway read it.
• An issuer's sequence continues on the Shard it transferred to.
• A Command Receipt survives both Gateway crash boundaries. It travels with its Character on a Transfer and stays put on a same-database Transfer.
• A lost receipt past the guarantee finishes as OutcomeUnknown without a second apply.
• Receipt capacity holds a command without an acknowledgement. Dispatch lanes stay fair, and a late command expires once under its own claim.

Module durable CI runs them in shard 1 as "Run the cross-Shard command transport tests". No Package is installed there.

module/tests/raid_convert.rs read the Suppressed outcome that meant "no handler" as proof that a ten-member Raid passes the Core Gates. With the fixture handler it now reads Applied, which says the same thing more directly.

gateway/build.rs existed only for LYRACORE_COORDINATOR_TEST_SOURCE. I deleted it, its include in subscriptions.rs and the paragraph in the test-support README. Nothing else used it.

One behaviour to weigh: every publish bakes in debug_reducers. On a Realm with no client-command Package, a player could send fixture.command. The transport Gates still apply. Without a Package no session-less Character has a live body, so the Gates refuse it and the player gets a Refusal reply. The playerbots issuer fence (Superseded) is the Package's decision, so the Core test asserts only the sequence. playerbots_orders keeps the fence test.

Paired with LyraCoreProject/packages#75, which deletes the playerbots copy and its runner command. Merge Core first, then the packages PR.

Validation: clippy with -D warnings passes on the Gateway tests and on the Module, with and without debug_reducers. The durable tests ran only in CI, because this workstation must keep heavy local builds to a minimum.

Made by Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You'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 38 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: c877e8ec-ca86-4fc4-9ba7-8bd1ed810f53

📥 Commits

Reviewing files that changed from the base of the PR and between 7399e78 and e1de669.

📒 Files selected for processing (7)

• .github/workflows/module-durable.yml
• crates/lyracore-test-support/README.md
• gateway/build.rs
• gateway/src/stdb/subscriptions.rs
• gateway/src/stdb/subscriptions_party_command_durable_tests.rs
• module/src/bridge.rs
• module/tests/raid_convert.rs

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Mathih13]

Code review: c8e9a7c

This review has two separate axes: Standards and Spec. No issue is linked, so the PR description is the spec. I read the code but ran nothing.

Priority key: 🔴 weigh before merge · 🟠 should fix · 🟡 judgement / optional

Standards

Correctness of module/src/bridge.rs: no defects found. client_command_handler() has both debug_reducers variants. FIXTURE_COMMAND and parse_fixture_command are compiled out with the feature off. A Package handler wins (GAME_CLIENT_COMMAND.or(Some(FIXTURE_COMMAND))). All three former call sites use the helper (bridge.rs:307, :547, :632).

• 🟠 CODING_STANDARDS.md, Tests, "One test covers one behavior".
  • command_receipts_recover_both_gateway_crash_boundaries (:548-681) covers lease expiry, target-applied-source-unaware, intent-id collision, in-transit and gone refusals, receipt travel and replay.
  • command_capacity_waits_without_ack_then_recovers_or_expires (:894-1010) covers capacity, lane isolation, rotation, release and expiry.
  • The #[allow(clippy::too_many_lines)] at :547, :685 and :893 shows the split is due. Split them, or give each allow a narrow reason.
• 🟠 Tests, "survive an internal refactor" and "Keep internal details behind the interface". The tests drive claim_party_command_intent, begin_transfer, dispatch_party_command_intents and defer_party_command_intent by hand with magic claim tokens (101, 201, 2_001, 4_000+). :430-436 reads source.0.coord().conn.db.game_party_command_intent() directly.
• 🟠 std::thread::sleep(Duration::from_millis(2_100)) (:568) hard-codes the lease length. If the lease changes, the test fails or turns slow. Poll on the observable state.
• 🟡 CONTEXT.md _Avoid_ words (AGENTS.md, Domain language).
  • "rejects" in command_realm_admission_rejects_a_changed_unrelated_member (:861). The glossary says Refusal.
  • "database" where a Shard is meant: the database params, "fresh source databases" (:608), and possibly command_receipt_stays_with_a_same_database_transfer (:775). If "same-database" means a Transfer within one Shard, name it that way.
• 🟡 Tests, "Derive expected values from the Spec". admitted() (:464-466) recomputes receipt_retain_until_micros with the implementation's formula. Capacity 32 and 16 (:910, :933) are copies of Module limits, but the lane count is read from lyracore_shared. Read all of them from one source.
• 🟡 bridge.rs:546 comment ("no addon remains...") is now true only for builds without debug_reducers. Every publish has the feature, so the comment is now wrong in practice.
• 🟡 Smells (judgement only):
  • Duplicated Code: the topology preamble (shard_map, TopologyEnv::install, coordinator, publish_locators) repeats in all six tests. Poll-then-run_bot_transfer repeats at :627-631 and :800-804. .to_ascii_lowercase().contains("applied"…) repeats 4×.
  • Data Clumps: leader_one/actor_one, leader_two/actor_two in CommandTopology (:70-81) could be one Leader { guid, actor }.
  • Primitive Obsession: outcomes compared as strings (== "true", format!("{id}|Applied")).
  • Middle Man: rows() (:349) only forwards.
  • Mysterious Name: GROUP = 5_098_000, queue, applied.

Spec

• ✅ There are six tests and each listed case has an assertion: StalePartyMirror for the roster, OutcomeUnknown for the lost receipt, and WaitingForCapacity, lane fairness and a single Expired with ClaimLost for capacity.
• ✅ CI step "Run the cross-Shard command transport tests" is in module-durable.yml:104-107 with if: matrix.shard == 1. The filter matches the module at subscriptions.rs:8329. It passes --ignored --test-threads=1.
• ✅ No references remain to LYRACORE_COORDINATOR_TEST_SOURCE, gateway/build.rs or the README paragraph.
• ✅ No scope creep. The bridge.rs change is only the fallback and its reply path.
• 🔴 The safety argument is not proven. The PR says: "Without a Package no session-less Character has a live body, so the Gates refuse it."
  • sessionless::action_gate / group_action_gate check ownership, no pending Transfer, offline, and a game_world_entity row. Nothing checks that a Package exists.
  • The new tests get session-less bodies from Core paths alone: debug_spawn_player_entity (:383) and run_bot_transfer.
  • debug_spawn_player_entity (module/src/debug/mod.rs:395) ships in every publish. In the lines I read, unlike its neighbours at :721 and :908, it does not call require_operator.
  • So on a Realm with no Package, a body can exist without a Package. A player could then get Applied on fixture.command instead of a Refusal.
  • The apply step writes nothing and a player reaches only their own Characters, so the impact is low. The PR description should still say this correctly. Alternatively, gate the fallback on something stronger than debug_reducers, for example a test-only flag or an operator-provisioned marker.
  • Please confirm whether a client identity can call debug_spawn_player_entity. If it can, that is an older defect outside this PR, but it is worth its own issue.

Summary: Standards has 3 should-fix findings, 3 judgement calls and some smells. The worst is that two tests each cover many behaviours. Spec has 1 finding: the claim that the Gates refuse fixture.command without a Package relies on bodies that Core itself can create.

---

Generated by Claude Code