[Mathih13]

Review of #604 found two problems in the roster bounds.

A Party mirror with 6 to 40 rows is damaged. party_facts, admit_party_command and admit_party_command_authority bounded every Group at the Raid cap, so such a mirror passed. Four places also repeated the same capped roster read and length check.

Each of these now reads the roster through bounded_roster. It takes the Group kind and bounds the read by GroupKind::member_cap. A Raid keeps its 40-member bound. A Party is bounded at 5 again. The error each site returns is unchanged. has_known_party_member_in_partition also uses the helper, with the Party cap on purpose. The debug fixture in bridge.rs uses it too.

The mirror writer now refuses the damage at the boundary. sync_group_mirror refuses more members than the Group kind holds. Packages stage at most 5 rows for a Party and 10 for a Raid, so they are not affected. The reader bounds stay as defence in depth.

Tests:

• a_world_shard_mirror_refuses_a_party_past_five: the writer refuses a Party of 6 and leaves no Group row. A Party of 5 and a Raid of 6 pass.
• a_party_mirror_past_five_is_stale_at_both_companion_order_readers: the writer now blocks a damaged Party, so the test plants one with an SQL update that turns a mirrored Raid of six into a Party. The certified list matches the rows, so only the Party cap can refuse. Both the authority reducer and apply_admitted_party_command refuse. With the target site forced to GroupKind::Raid, the test fails.
• a_raid_past_five_certifies_a_companion_order_for_any_member also applies an order in a Raid of 10. It passes the same bound.

Not covered: the roster bound in party_facts. It is not a reducer. A fixture reducer to reach it would add a generated Gateway binding, so I left it out. The packages suite covers it after a repin.

Refs #534

Test commands (all pass):

• cargo test -p lyracore-module --test raid_convert -- --ignored (6 tests)
• cargo test -p lyracore-module --lib --features=debug_reducers group:: (27 tests)
• cargo clippy -p lyracore-module --all-targets --features=debug_reducers (no warnings)
• cargo check --target wasm32-unknown-unknown -p lyracore-module --features=debug_reducers

Written by Claude Sonnet 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 18 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: df83dcc6-d4ad-4531-89b8-213e1c06ad7a

📥 Commits

Reviewing files that changed from the base of the PR and between ca5d540 and 751e432.

📒 Files selected for processing (3)

• module/src/bridge.rs
• module/src/group.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: origin/main...36c4e2e (1 commit)

Spec source: #534 (from Refs #534), plus the #604 review problem this PR's description states. Standards source: CODING_STANDARDS.md.

Standards

P2: the mirror writer accepts the state the readers now call damaged (module/src/group.rs, sync_group_mirror / mirror_kind_and_slots)
sync_group_mirror only checks members.len() > RAID_MAX_MEMBERS. It does not check the cap for the kind it was sent. The new test depends on this: mirror_group(&realm, 900, 10, 6, GroupKind::Party) uses assert_call, so the operator path commits a 6-member Party. The readers then refuse that Group on every call (StalePartyMirror / FightLimit). The damage gets in at the boundary. Rule: "Treat ... remote state as untrusted input. Return a useful error ... at those boundaries" and "Make illegal states hard to represent." Fix: in mirror_kind_and_slots, refuse members.len() > kind.member_cap(). The test then needs another way to plant a damaged mirror, or it can test the refusal at the writer.

P3: the new test covers two behaviours, and one is already covered (module/tests/raid_convert.rs)
a_party_mirror_past_five_is_damaged_while_a_raid_of_ten_certifies: the Raid-of-10 half repeats a_raid_past_five_certifies_a_companion_order_for_any_member, which already certifies a 10-member Raid. Rule: "One test covers one behavior." Keep the Party half and name the test for it.

P3, possible Duplicated Code: mirror_group vs mirror_args
The new helper rebuilds the 11-argument sync_group_mirror call that mirror_args already builds 100 lines above. One builder that takes group id, GUIDs and kind would serve both.

P3, possible Duplicated Code: has_known_party_member_in_partition is a fifth copy
It still does the same take(GROUP_MAX_MEMBERS + 1) + length check by hand. bounded_roster(ctx, id, GroupKind::Party) keeps the deliberate Party cap and removes the last copy. Rule: "Remove a displaced path when the replacement covers it."

Nit: admit_party_command_authority spells one refusal three ways
The stale closure is defined after an inline StalePartyMirror for the missing Group, and the current != expected branch below still spells it out. Define stale first and use it at all three sites. Also, let members = ctx.db.game_group_member(); now sits between the roster read and the sort, away from its only use in the loop.

bounded_roster itself is fine. It is deep enough to pass the deletion test, it returns None and never a prefix, and its doc comment states that.

Spec

(a) Missing or partial

• playerbots: act on raid membership beyond five members #534: "a bot in a raid scopes its party facts and Companion Orders to its own subgroup, or to the whole raid where vanilla behavior needs it". Not in this PR. The PR says Refs, not Closes, so this is expected. playerbots: act on raid membership beyond five members #534 stays open.
• PR description: "party_facts, admit_party_command and admit_party_command_authority bounded every Group at the Raid cap". The fix touches all three, but the new test only covers admit_party_command_authority. Nothing shows a 6-row Party getting FightLimit from party_facts or StalePartyMirror from admit_party_command. Add one test at the party_facts seam.

(b) Scope creep
None. The bridge.rs fixture change is part of the stated deduplication.

(c) Implemented but looks wrong

• Same root cause as Standards P2. The PR treats a 6-to-40-row Party as damage the readers must catch, but the writer still lets the normal push create it. The bound is right in the readers and missing at the source.

---

Summary. Standards: 5 findings. Worst is P2, sync_group_mirror admits a Party past its cap. Spec: 2 findings plus the expected open #534 scope. Worst is that 2 of the 3 changed readers have no test.

---

Generated by Claude Code

[Mathih13]

Addressed all findings in 5fe2fc4.

• P2: sync_group_mirror now refuses more members than the Group kind holds. The reader tests plant a damaged Party with an SQL update on a mirrored Raid.
• P3: the Raid-of-10 half is gone from the Party test. The two builders are one mirror_args. has_known_party_member_in_partition uses bounded_roster with the Party cap.
• Nit: admit_party_command_authority defines stale first and uses it at every site. members moved to its use.

Claude Sonnet 5.5 in Claude Code.