Skip to content

Commit 359ccfb

Browse files
HandyS11claude
andcommitted
Correct three clan comments that describe code that does not exist
ClanCapabilityProvider's CacheTtl doc justified its value with "the heal path reconciles every server on a timer". WorkspaceHostedService heals at startup and on ChannelDestroyed only; there is no timer. Say what actually bounds the staleness: reconciles are event-driven and Invalidate, not expiry, is what guarantees a transition is seen. ClanMessageKeyParityTests claimed Features.Clans cannot see the internal WorkspaceMessageKeys. It can — Features.Workspace grants it InternalsVisibleTo. The constants are independent by choice, which is what makes the assertion meaningful. ClanMotdWriter claimed to re-check the permission because roles can change between render and click. It reads the same cached snapshot the button was rendered from, so a demotion the poller has not observed yet still gets through. Describe what it does defend against: a forged component id and a button clicked from scrollback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent bf8fccb commit 359ccfb

3 files changed

Lines changed: 15 additions & 8 deletions

File tree

src/RustPlusBot.Features.Clans/State/ClanCapabilityProvider.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@ internal sealed class ClanCapabilityProvider(IServiceScopeFactory scopeFactory,
1717
: IWorkspaceCapabilityProvider
1818
{
1919
/// <summary>
20-
/// How long an answer is reused. Two gated specs mean two probes per server per reconcile, and
21-
/// the heal path reconciles every server on a timer; this is kept well under the reconcile
22-
/// interval so a clan transition is still picked up promptly.
20+
/// How long an answer is reused. Two gated specs mean two probes per server per reconcile, so a
21+
/// short reuse window collapses them into one store read. It stays short because reconciles are
22+
/// event-driven (startup, channel deletion, server registration, clan transition) rather than
23+
/// timed: a cached answer must never outlive the transition that prompted the reconcile, and
24+
/// <see cref="Invalidate"/> — not expiry — is what guarantees a transition is seen.
2325
/// </summary>
2426
private static readonly TimeSpan CacheTtl = TimeSpan.FromSeconds(5);
2527

src/RustPlusBot.Features.Clans/Writing/ClanMotdWriter.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ namespace RustPlusBot.Features.Clans.Writing;
66
/// <summary>
77
/// Applies a clan MOTD change, enforcing the acting player's clan permission before touching the
88
/// socket. The permission check is duplicated here rather than trusted from the button's presence:
9-
/// component payloads can be forged, and roles can change between render and click.
9+
/// a component id can be forged, and a button rendered before a demotion can still be clicked from
10+
/// scrollback afterwards. It is not a fresh authority check — it reads the same cached clan
11+
/// snapshot the button was rendered from, so a demotion the poller has not yet observed still gets
12+
/// through; the game's own API is the last word there.
1013
/// </summary>
1114
/// <param name="store">Supplies the stored clan snapshot for the permission check.</param>
1215
/// <param name="query">Performs the write on the live socket.</param>

tests/RustPlusBot.Features.Workspace.Tests/Messages/ClanMessageKeyParityTests.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
namespace RustPlusBot.Features.Workspace.Tests.Messages;
44

55
/// <summary>
6-
/// Pins that the clan renderers' key literals (declared in Features.Clans, which cannot see
7-
/// Features.Workspace's internal <see cref="WorkspaceMessageKeys" />) still match the keys the
8-
/// reconciler looks renderers up by. The reconciler compares with <c>StringComparer.Ordinal</c>,
9-
/// so a silent rename on either side would unhook a renderer without any other test catching it.
6+
/// Pins that the clan renderers' key literals still match the keys the reconciler looks renderers
7+
/// up by. Features.Clans can see Features.Workspace's internal <see cref="WorkspaceMessageKeys" />
8+
/// (it is granted <c>InternalsVisibleTo</c>) but deliberately declares its own public constants
9+
/// instead, so the two sides are independent literals. The reconciler compares them with
10+
/// <c>StringComparer.Ordinal</c>, so a silent rename on either side would unhook a renderer
11+
/// without any other test catching it.
1012
/// </summary>
1113
public sealed class ClanMessageKeyParityTests
1214
{

0 commit comments

Comments
 (0)