Repository navigation
Userinfo case lines via bounded member case index - #383
Conversation
Moderators can inspect cached member fields, warn counts, and recent cases with graceful degradation when guild data or Discord lookup is unavailable.
|
@pullfrog review |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Three issues need addressing before merging: the Clippy gate fails, retained cases can be reported as absent, and initial cached voice channels are discarded.
Reviewed changes across all seven files, including command dispatch, startup seeding, cache contracts, moderation reads, rendering, and regression coverage.
- Command registration: Adds moderator-only
!userinfoand/userinfo, with an optional target defaulting to the caller. - Profile snapshot: Combines cached names, join time, roles, and voice state with snowflake-derived account age and a REST user fallback.
- Moderation history: Adds warning counts and up to eight formatted guild-store case lines, with distinct unavailable and empty states.
- Output and coverage: Bounds the default response to Discord's UTF-16 limit and adds command documentation plus rendering and local integration tests.
Validation: all 11 existing userinfo tests passed locally. Two focused probes using the unchanged production snapshot module and real local data-layer APIs failed at the intended assertions described inline. cargo clippy --locked -p jmodguard-bot --all-targets -- -D warnings and cargo fmt --all --check both failed; the Clippy error also matches the PR's CI log. Full preflight and live Discord/remote-provider tests were not run.
gpt-6.1-sol ｜ 𝕏
|
@pullfrog review |
There was a problem hiding this comment.
Important
Two previously reported behavior bugs remain: retained cases can be shown as absent, and initial cached voice channels are rejected. Both were reproduced against the current implementation; the earlier Clippy issue is fixed and its thread is resolved.
Reviewed changes across the full seven-file PR, including the latest renderer-wiring and formatting fix.
- Command integration: Adds moderator-only prefix and slash
userinfocommands with an optional target defaulting to the caller, seeded through the existing command registry. - Profile lookup: Combines cached identity, join time, roles, and voice state with account age and a REST identity fallback.
- Moderation history: Reads warning totals and formats up to eight guild-store case lines, distinguishing unavailable data from empty history.
- Rendering and coverage: Adds bounded UTF-16 output, command documentation, cache/rendering tests, and a local database integration test.
- Validation: All 11 existing userinfo tests,
cargo fmt --all --check, andcargo clippy --locked -p jmodguard-bot --all-targets -- -D warningspassed. Two temporary fixture extensions failed at the intended voice-channel and retained-ban assertions; they were removed, leaving a clean worktree. Full local preflight and live Discord/provider tests were not run.
gpt-6.1-sol ｜ 𝕏
Walk member_history with next_cursor until eight case rows or the timeline ends so stay/join rows on early pages do not hide cases. When get_user fails, fill username/global name with missing and still render warns and case lines.
Load warns/cases via member_case_entries, treat missing voice guild_id as cached for GuildCreate payloads, and keep mixed-history pagination as a test helper only.
|
@pullfrog review |
There was a problem hiding this comment.
Important
One upgrade-path issue remains: userinfo can report no cases while retained cases are still awaiting index backfill. The earlier mixed-timeline and initial voice-state issues are fixed; their threads are resolved.
Reviewed changes across all eight files, including command registration, cache/REST lookup, moderation storage, rendering, documentation, and regression coverage.
- Command integration: Adds moderator-only prefix and slash
userinfocommands with an optional target defaulting to the caller. - Profile snapshot: Reads cached identity, join time, roles, and voice state, derives account age, and preserves moderation output when REST identity lookup fails.
- Case retrieval: Adds a bounded case-only query with captured guild-store scope and deletion checks, avoiding interference from newer membership observations.
- Response rendering: Adds UTF-16-bounded template output, reason-tag scrubbing, clipping, documentation, and local tests.
- Validation: All 13 existing userinfo tests, bot Clippy with
-D warnings, formatting, and service-boundary checks passed. Temporary real SQLite probes confirmed the mixed-timeline fix and reproduced the upgrade omission before backfill, with the same case visible after backfill; probes were removed and the worktree is clean. Full local preflight and live Discord/provider tests were not run.
gpt-6.1-sol ｜ 𝕏
Use index-first SQL for member_case_entries, add EXPLAIN regression, and cover retained cases after join/depart noise plus voice-state parsing.
|
@pullfrog review |
There was a problem hiding this comment.
Important
One previously reported upgrade-path bug remains: userinfo can report Cases: none while retained cases are awaiting index backfill. The latest change bounds the lookup but does not establish that the index is complete.
Reviewed changes across all nine files, covering command registration and dispatch, cache/REST identity lookup, moderation reads, indexed case retrieval, rendering, documentation, and regression tests.
- Command integration: Adds moderator-only prefix and slash
userinfocommands with an optional target defaulting to the caller. - Profile snapshot: Combines cached identity, join time, roles, and voice state with account age and a REST identity fallback that preserves moderation output on lookup failure.
- Case retrieval: Adds a scoped, index-ordered case-only lookup that limits the index walk before loading case payloads.
- Response rendering: Formats warning totals and recent cases, strips internal ban tags, and clips responses to Discord's UTF-16 limit.
- Validation: All 15 userinfo tests, the indexed-query regression, bot Clippy with
-D warnings, and formatting passed. A temporary extension of the real SQLite upgrade fixture reproduced the omission before backfill and passed after backfill; the probe was removed and the worktree is clean. - Check limits: The service-boundary script passed its source guards and root/bot dependency checks, then stopped with
failed to download aws-lc-rs v1.18.1because--offline was specified. Full preflight and live Discord/remote-provider testing were not run.
gpt-6.1-sol ｜ 𝕏
Return MemberSyncError::Stale when the index is empty but guild_cases still hold rows while case_backfill_done is false, so userinfo shows unavailable instead of Cases: none during migration backfill.
|
@pullfrog review |
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 7 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/db/guild_data/member_directory.rs" line_range="2840" />
<code_context>
+
+ /// Recent moderation cases for one member, newest first. Fetches at most
+ /// `limit` rows and reports whether a newer ninth case exists.
+ pub async fn member_case_entries(
+ &self,
+ scope: GuildDataScope,
</code_context>
<issue_to_address>
**New cases disappear from history**
When a moderation case is recorded after the backfill cursor passes it or backfill completes, `record_case` writes to `guild_cases` without adding the case to `guild_member_case_index`, while `member_case_entries` reads only that index, so userinfo omits the new case or reports none.
Ensure newly recorded cases are added to `guild_member_case_index`.
Also at `src/db/guild_data/member_directory.rs:2835`.
</issue_to_address>
### Comment 2
<location path="src/db/guild_data/member_directory.rs" line_range="2835" />
<code_context>
+ "SELECT i.action_kind,i.occurred_at_ms,i.case_id,json_extract(c.payload,'$.actor_id'),json_extract(c.payload,'$.reason'),json_extract(c.payload,'$.outcome'),CASE WHEN c.source_key LIKE 'discord:%' THEN 'discord_audit' WHEN EXISTS(SELECT 1 FROM guild_case_sources s WHERE s.guild_id=c.guild_id AND s.enrollment_id=c.enrollment_id AND s.case_id=c.case_id AND s.source_kind=1) THEN 'application_audit_linked' WHEN json_type(c.payload,'$.source.gateway_observation') IS NOT NULL THEN 'gateway_observation_unlinked' ELSE 'application' END FROM (SELECT guild_id,enrollment_id,user_id,case_id,action_kind,occurred_at_ms FROM guild_member_case_index INDEXED BY guild_member_cases_user WHERE guild_id=?1 AND enrollment_id=?2 AND user_id=?3 ORDER BY occurred_at_ms DESC,case_id DESC LIMIT ?4) AS i JOIN guild_cases c ON c.guild_id=i.guild_id AND c.enrollment_id=i.enrollment_id AND c.case_id=i.case_id AND c.deleted=0 ORDER BY i.occurred_at_ms DESC,i.case_id DESC"
</code_context>
<issue_to_address>
**Removed cases hide retained history**
When newer indexed cases are deleted and older live cases fall beyond the subquery limit, `member_case_entries_lookup_sql` applies `LIMIT` before its outer join filters `c.deleted=0`, so removed cases consume the limit and userinfo shows too few cases or none despite older live history.
Filter deleted cases before limiting the indexed case rows.
Also at `src/db/guild_data/member_directory.rs:2836`.
</issue_to_address>
### Comment 3
<location path="src/db/guild_data/member_directory.rs" line_range="2892" />
<code_context>
+ if truncated {
+ entries.pop();
+ }
+ if entries.is_empty() {
+ let backfill_done: bool = tx.query_row(
+ "SELECT case_backfill_done FROM guild_member_seed WHERE guild_id=?1 AND enrollment_id=?2",
</code_context>
<issue_to_address>
**Partial history appears complete**
When backfill is incomplete and the target already has indexed cases plus unindexed cases, `member_case_entries` checks backfill state only when `entries` is empty, so existing indexed rows suppress the check and userinfo presents incomplete history as complete.
Check the backfill state even when indexed entries are present.
Also at `src/db/guild_data/member_directory.rs:2840`.
</issue_to_address>
### Comment 4
<location path="crates/bot/src/commands/userinfo.rs" line_range="537" />
<code_context>
+ }
+ }
+
+ let warns =
+ match crate::db::list_warns(&env.app.db, env.guild_id.get() as i64, target.get() as i64)
+ .await
</code_context>
<issue_to_address>
**Warning counts load every row**
When a member has a large warning history, `userinfo` calls `list_warns` and materializes every warning, including reason text, just to compute `rows.len()`, causing unnecessary database work and memory use.
Use a count-only database query for the warning total.
Also at `crates/bot/src/commands/userinfo.rs:538-540`.
</issue_to_address>
### Comment 5
<location path="src/db/guild_data/member_directory.rs" line_range="2835" />
<code_context>
+ "SELECT i.action_kind,i.occurred_at_ms,i.case_id,json_extract(c.payload,'$.actor_id'),json_extract(c.payload,'$.reason'),json_extract(c.payload,'$.outcome'),CASE WHEN c.source_key LIKE 'discord:%' THEN 'discord_audit' WHEN EXISTS(SELECT 1 FROM guild_case_sources s WHERE s.guild_id=c.guild_id AND s.enrollment_id=c.enrollment_id AND s.case_id=c.case_id AND s.source_kind=1) THEN 'application_audit_linked' WHEN json_type(c.payload,'$.source.gateway_observation') IS NOT NULL THEN 'gateway_observation_unlinked' ELSE 'application' END FROM (SELECT guild_id,enrollment_id,user_id,case_id,action_kind,occurred_at_ms FROM guild_member_case_index INDEXED BY guild_member_cases_user WHERE guild_id=?1 AND enrollment_id=?2 AND user_id=?3 ORDER BY occurred_at_ms DESC,case_id DESC LIMIT ?4) AS i JOIN guild_cases c ON c.guild_id=i.guild_id AND c.enrollment_id=i.enrollment_id AND c.case_id=i.case_id AND c.deleted=0 ORDER BY i.occurred_at_ms DESC,i.case_id DESC"
</code_context>
<issue_to_address>
**Oversized cases bypass read bounds**
When a case is admitted under a custom byte budget larger than the default, `member_case_entries_lookup_sql` extracts each case reason without checking payload length, so valid large records are fully loaded before `clip_reason` truncates them, defeating bounded reads and consuming excess memory.
Apply an explicit payload-size bound before extracting case fields, and report oversized records as unavailable.
</issue_to_address>
### Comment 6
<location path="crates/bot/src/commands/userinfo/tests.rs" line_range="229" />
<code_context>
+ entries: vec![case_row(1_000, "case-two")],
+ next_cursor: None,
+ };
+ let entries = collect_case_entries_from_pages(&[page1, page2]);
+ assert_eq!(entries.len(), 2);
+ let lines = case_lines_from_entries(&entries);
</code_context>
<issue_to_address>
**Case pagination regression goes untested**
When a regression in the production path prevents retrieving cases beyond its initial results, when production case loading fails to retrieve cases on later pages, `paginate_case_lines_past_stay_rows_on_first_page` (tests.rs:210-233) still passes because it supplies fabricated pages to the test-only `collect_case_entries_from_pages` helper; it does not exercise the production lookup.
Test the mixed stay/case history through the production case-loading path rather than relying on a pagination helper defined in the test.
</issue_to_address>
### Comment 7
<location path="crates/bot/src/commands/userinfo/tests.rs" line_range="495" />
<code_context>
+ global_name: None,
+ nickname: None,
+ joined_at_ms: Some(500 + round),
+ observed_at_ms: 10_000 + round,
+ is_join: true,
+ },
</code_context>
<issue_to_address>
**History noise does not mask cases**
When a limited mixed-history lookup returns only the newest rows, when case lookup regresses to a limited mixed-history query, `retained_case_visible_after_join_depart_noise` (tests.rs:473-513) still passes because its join/depart timestamps are 10,000–20,011, while the case is recorded at the current time and remains the newest row.
Record the case at an explicit timestamp older than the injected join/depart history, and ensure the noise exceeds the lookup window.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 7 findings to address first, and the command returns member details and warning/case history in its normal response, so a visibility or permission mistake could disclose sensitive guild information to channel readers. Reverting removes the command but cannot retract messages already seen.
Blocking findings: src/db/guild_data/member_directory.rs:2840, src/db/guild_data/member_directory.rs:2835, src/db/guild_data/member_directory.rs:2892, crates/bot/src/commands/userinfo.rs:537, src/db/guild_data/member_directory.rs:2835, and 2 more
There was a problem hiding this comment.
Important
Two upgrade-path issues remain: a partially populated index can still return incomplete recent history as a successful result, and the new empty-index fallback performs an unbounded scan of retained case payloads.
Reviewed changes across all nine files, covering command integration, cached profile lookup, moderation reads, indexed case retrieval, rendering, documentation, and regression coverage.
- Command integration: Adds moderator-only prefix and slash
userinfocommands with an optional target defaulting to the caller. - Profile snapshot: Combines cached identity, join time, roles, and voice state with account age and a REST identity fallback that preserves moderation output on lookup failure.
- Case retrieval: Adds captured-scope, index-ordered case reads and stale detection for empty results during pending backfill.
- Response rendering: Displays warning totals and recent case summaries, scrubs internal ban tags, and bounds output to Discord's UTF-16 limit.
- Validation: All 15 userinfo tests, both added database regressions, the original schema-upgrade test, bot Clippy with
-D warnings, formatting, and service-boundary checks passed. A temporary extension of the real SQLite upgrade fixture reproduced the partial-index omission; a separate SQLite query-plan/VM-step probe confirmed linear fallback work. Temporary edits were removed and the worktree is clean. - Test limits: Full local preflight and live Discord/remote-provider testing were not run; the probes establish local database behavior, not provider acceptance or production latency.
gpt-6.1-sol ｜ 𝕏
Treat unfinished case-index backfill as stale for any indexed result, read completion from guild_member_seed instead of scanning guild_cases, and add regression tests plus EXPLAIN coverage. Use count_warns for userinfo.
|
@pullfrog review |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/db/guild_data/member_directory.rs" line_range="2835" />
<code_context>
+ "SELECT i.action_kind,i.occurred_at_ms,i.case_id,json_extract(c.payload,'$.actor_id'),json_extract(c.payload,'$.reason'),json_extract(c.payload,'$.outcome'),CASE WHEN c.source_key LIKE 'discord:%' THEN 'discord_audit' WHEN EXISTS(SELECT 1 FROM guild_case_sources s WHERE s.guild_id=c.guild_id AND s.enrollment_id=c.enrollment_id AND s.case_id=c.case_id AND s.source_kind=1) THEN 'application_audit_linked' WHEN json_type(c.payload,'$.source.gateway_observation') IS NOT NULL THEN 'gateway_observation_unlinked' ELSE 'application' END FROM (SELECT guild_id,enrollment_id,user_id,case_id,action_kind,occurred_at_ms FROM guild_member_case_index INDEXED BY guild_member_cases_user WHERE guild_id=?1 AND enrollment_id=?2 AND user_id=?3 ORDER BY occurred_at_ms DESC,case_id DESC LIMIT ?4) AS i JOIN guild_cases c ON c.guild_id=i.guild_id AND c.enrollment_id=i.enrollment_id AND c.case_id=i.case_id AND c.deleted=0 ORDER BY i.occurred_at_ms DESC,i.case_id DESC"
</code_context>
<issue_to_address>
**Deleted cases hide older history**
When deleted cases occupy the newest index entries, `member_case_entries_lookup_sql` limits index rows before filtering `c.deleted=0`, so tombstones consume the window and older live cases disappear from userinfo.
Filter out deleted cases before applying the result limit while keeping the lookup bounded.
</issue_to_address>
### Comment 2
<location path="src/db/guild_data/member_directory.rs" line_range="2835" />
<code_context>
+ "SELECT i.action_kind,i.occurred_at_ms,i.case_id,json_extract(c.payload,'$.actor_id'),json_extract(c.payload,'$.reason'),json_extract(c.payload,'$.outcome'),CASE WHEN c.source_key LIKE 'discord:%' THEN 'discord_audit' WHEN EXISTS(SELECT 1 FROM guild_case_sources s WHERE s.guild_id=c.guild_id AND s.enrollment_id=c.enrollment_id AND s.case_id=c.case_id AND s.source_kind=1) THEN 'application_audit_linked' WHEN json_type(c.payload,'$.source.gateway_observation') IS NOT NULL THEN 'gateway_observation_unlinked' ELSE 'application' END FROM (SELECT guild_id,enrollment_id,user_id,case_id,action_kind,occurred_at_ms FROM guild_member_case_index INDEXED BY guild_member_cases_user WHERE guild_id=?1 AND enrollment_id=?2 AND user_id=?3 ORDER BY occurred_at_ms DESC,case_id DESC LIMIT ?4) AS i JOIN guild_cases c ON c.guild_id=i.guild_id AND c.enrollment_id=i.enrollment_id AND c.case_id=i.case_id AND c.deleted=0 ORDER BY i.occurred_at_ms DESC,i.case_id DESC"
</code_context>
<issue_to_address>
**New cases disappear from history**
When the case-index backfill has completed and a moderator records another case for the member, `record_case` inserts new rows into `guild_cases` but does not add them to `guild_member_case_index`; after backfill completes, this lookup only reads that index, so userinfo reports `Cases: none` or omits newer cases.
Update the case index atomically when recording a case, or add an equivalent database trigger.
</issue_to_address>
### Comment 3
<location path="src/db/guild_data/member_directory.rs" line_range="2840" />
<code_context>
+
+ /// Primary read used before returning indexed case rows during upgrade backfill.
+ pub(crate) fn member_case_entries_backfill_gate_sql() -> &'static str {
+ "SELECT case_backfill_done FROM guild_member_seed WHERE guild_id=?1 AND enrollment_id=?2"
+ }
+
</code_context>
<issue_to_address>
**Saved cases stay unavailable**
When the guild has saved cases but its member-directory seed has not been created, the backfill-gate query returns no row when a guild has no `guild_member_seed`, so `member_case_entries` errors and `load_case_lines` renders `Cases: unavailable` even when `guild_cases` contains saved moderation cases. This leaves userinfo unable to show case history until member-directory initialization runs.
Make case-history retrieval initialize or backfill the case index for this state, or query saved cases through a bounded fallback rather than treating a missing seed as unavailable.
</issue_to_address>
### Comment 4
<location path="crates/bot/src/commands/userinfo/tests.rs" line_range="229" />
<code_context>
+ entries: vec![case_row(1_000, "case-two")],
+ next_cursor: None,
+ };
+ let entries = collect_case_entries_from_pages(&[page1, page2]);
+ assert_eq!(entries.len(), 2);
+ let lines = case_lines_from_entries(&entries);
</code_context>
<issue_to_address>
**Case retrieval bypasses pagination test**
When the production loader mishandles stay rows newer than a case, `collect_case_entries_from_pages` processes only the test’s handcrafted pages, so `paginate_case_lines_past_stay_rows_on_first_page` passes even if `load_case_lines_from_store` omits cases after stay rows. The production retrieval behavior can be broken without failing this test.
Exercise `load_case_lines_from_store` with stored stay rows newer than a case.
</issue_to_address>
### Comment 5
<location path="crates/bot/src/commands/userinfo/tests.rs" line_range="270" />
<code_context>
+ member_cached: false,
+ guild_cached: false,
+ };
+ apply_missing_user_identity(&mut snapshot);
+ let lines = vec
cursor
Bot
changed the title
|
@pullfrog review |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. The response includes warning counts and moderation case reasons, so invoking the command in a visible channel can disclose that history beyond the moderators allowed to run it. Reverting stops future responses but cannot retract messages already sent.
There was a problem hiding this comment.
Important
The follow-up integrity check still makes case reads linear in retained member history, and the new REST-failure regression attempts live Discord access. Both findings are detailed inline.
Reviewed changes Full review of the moderator userinfo command and its member-case data path against main, including the changes associated with #159.
- Command integration: Adds prefix/slash dispatch, moderator defaults, optional target resolution, and command documentation.
- Profile presentation: Combines cached member, role, and voice data with REST identity fallback, warning counts, recent cases, unavailable states, and bounded template rendering.
- Case storage: Adds index-backed recent-case reads with payload budgets and backfill readiness checks, audit-import index maintenance, and empty-backfill completion handling.
- Regression coverage: Adds command/rendering integration cases and database tests for ordering, backfill, imported cases, read budgets, and trigger-owned indexing.
Verification: eight member_case_entries tests, the trigger-owned insert regression, and 16 userinfo tests passed locally. The remaining REST-failure test passed under a rejecting loopback HTTPS proxy, which captured its attempted Discord connection without contacting the provider. A separate local SQLite probe used the repository migrations and exact production SQL to measure query work; these checks do not establish live Discord or remote libSQL behavior. Full preflight was not run; the PR's reported CI checks are passing.
gpt-6.1-sol ｜ 𝕏
Bound the member-case index gap to the lookup window, keep the userinfo REST failure test on a local proxy, and clear stored userinfo command rows in migration 0022 so the new built-in follows schema 22.
|
@pullfrog review |
There was a problem hiding this comment.
Important
One output-safety issue remains: target-controlled profile text can become active mentions in userinfo replies.
Reviewed changes Full review of all 19 changed files for the user-info portion of #159, against current main.
- Command integration: Adds moderator-only prefix and slash
userinfocommands, with an optional target defaulting to the caller and the existing built-in access controls. - Profile and rendering: Combines cached identity, membership, roles, and voice state with account age, REST identity fallback, warning totals, and UTF-16-bounded case summaries.
- Case reads: Adds explicit payload read budgets, completed-backfill gating, and a recent-window index-gap check; updates the shared rejoin-note consumer and audit-import indexing.
- Upgrade handling: Adds core migration 0022 to remove stored
userinfodefinitions, restores the shipped global row through seeding, and updates schema publication contracts and documentation. - Validation: All 17 userinfo tests, 30 rejoin tests, 10 member-case tests, the migration/reseeding regression, formatting, and publication contract tests passed locally. A temporary loopback HTTP probe failed at the intended mention-safety assertion; it was removed, leaving a clean worktree. Current GitHub CI is green.
- Test boundaries: Local probes used real rendering/sending code, Serenity, SQLite, and loopback HTTP fixtures. Full local preflight and live Discord/provider testing were not run; notification behavior was checked against Discord's documented contract, not live delivery.
gpt-6.1-sol ｜ 𝕏
Member and case fields in userinfo go through the rejoin report sanitizer, and prefix and slash replies for that command send an empty allowed_mentions set. Other commands keep normal mention parsing.
|
@pullfrog review |
There was a problem hiding this comment.
Important
userinfo can disclose a hidden voice channel's name and ID to moderators who cannot view it. One confirmed visibility regression is detailed inline.
Reviewed changes across the complete PR, including command delivery, bounded case reads, indexing, and schema migration.
- Userinfo command: Adds prefix/slash reports with cached profile and voice data, warning counts, recent cases, REST fallback, and bounded rendering.
- Case-index reads: Adds read budgets and backfill/readiness checks, updates audit ingestion, and adapts rejoin reporting.
- Safe report delivery: Escapes member/case text and suppresses mentions specifically for
userinfo, retaining other commands' mention behavior. - Migration and coverage: Adds core migration 0022, updates publication schema contracts and documentation, and extends regression tests.
Validation: 21 data-layer and 61 bot targeted tests passed, along with cargo fmt --all --check and bash scripts/publish-image-test.sh. A temporary regression probe reproduced the hidden-channel disclosure and was removed afterward; the worktree is clean. Testing used local SQLite, Serenity cache fixtures, and local HTTP fixtures, not live Discord or remote-provider acceptance. Full preflight and remote-database checks were not run.
gpt-6.1-sol ｜ 𝕏
Userinfo reports voice through the existing visibility-aware lookup. A hidden or undecided channel is unavailable, without its name or id, including channel mentions in case reasons.
|
@pullfrog review |
There was a problem hiding this comment.
Important
One reproducible panic remains: a case reason containing <#0> aborts userinfo rendering instead of producing a report.
Reviewed changes Reviewed the complete PR against main, including command dispatch, report rendering, bounded case reads, and the schema upgrade.
- Userinfo command: Adds prefix and slash reports using cached member details, REST identity fallback, warning counts, and recent cases.
- Safe report delivery: Escapes profile/case text, suppresses mentions specifically for userinfo, and applies caller visibility checks to voice channels and channel mentions in reasons.
- Bounded case history: Adds explicit read budgets, incomplete-index and recent-orphan checks, audit-import indexing, and empty-case backfill completion; updates the shared rejoin reader.
- Schema and documentation: Adds migration 0022 to clear stored userinfo definitions, restores the shipped definition through seeding, and updates schema publication contracts and command documentation.
Validation passed: 20 userinfo tests, 30 rejoin tests, nine member-case lookup tests, the command cleanup/reseeding migration test, publication contract fixtures, cargo fmt --check, and git diff --check. A temporary regression probe confirmed the inline finding and was removed afterward; the worktree is clean. These were local SQLite and HTTP fixtures, not live Discord/Bunny verification; the full local preflight was not run.
gpt-6.1-sol ｜ 𝕏
A cached-guild userinfo report panicked on a case reason of <#0> because ChannelId::new rejects zero. Mentions and id tokens now use the fallible snowflake parsers, and an invalid id is reported as unavailable.
|
@pullfrog review |
There was a problem hiding this comment.
Important
Case-reason redaction still exposes private-thread IDs to moderators who cannot access those threads; one reproduced finding is inline.
Reviewed changes across all 19 files for the userinfo portion of #159, including the latest invalid-snowflake fix.
- Command integration: Adds moderator-only prefix and slash
userinfo, with an optional target defaulting to the caller, plus schema22 cleanup of stored definitions and publication-contract updates. - Profile and moderation reads: Combines cached member details and REST identity fallback with count-only warnings and bounded recent guild-store cases.
- Case-index lifecycle: Gates complete-history reads on backfill readiness, checks a bounded index window, enforces payload budgets, and maintains audit-import index entries.
- Output safety: Escapes profile and case text, suppresses response mentions, checks channel visibility, rejects invalid snowflakes, and bounds rendered output.
- Validation: All 21 userinfo tests, 12 database/migration tests, 42 rejoin-filtered tests, bot Clippy with
-D warnings, formatting, and publication contract tests passed; a temporary production-path regression reproduced the private-thread disclosure and was removed, leaving a clean worktree. - Test boundaries: Validation used local SQLite, real Serenity cache objects, and loopback HTTP/WebSocket fixtures; full local preflight and live Discord/remote-provider testing were not run, while the current GitHub
CI passedcheck is green.
gpt-6.1-sol ｜ 𝕏
A fail-closed case read, including a stored case over the read budget, still sends the rejoin note with an operator notice. The store budget is unchanged. Private-thread mentions stay hidden unless the caller has Manage Threads or known membership.
|
@pullfrog review |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes across all 22 changed files for the user-info portion of #159, including the current private-thread and rejoin-delivery fixes.
- Command integration: Adds moderator-only prefix and slash
userinfo, with an optional target defaulting to the caller, cached profile fields, account age, warning totals, and a REST identity fallback that preserves moderation output on ordinary lookup failure. - Bounded case history: Uses captured guild-store scope, a member-specific ordered index, explicit payload read limits, and fail-closed checks for incomplete backfill and recent index gaps; preserves insertion/removal trigger behavior and audit-import indexing.
- Safe output: Escapes member and case text, strips internal ban tags, suppresses userinfo mentions, bounds responses to Discord's UTF-16 limit, and withholds hidden, unknown, or inaccessible private-thread channel references.
- Rejoin reports: Keeps notes deliverable when case reads refuse oversized records, without relaxing the read budget or exposing the refused payload.
- Upgrade and publication: Advances core schema to 22, removes persisted
userinfodefinitions before reseeding the shipped command, and updates publication contracts and documentation. - Validation: Passed 22 userinfo tests, nine member-case database regressions, 31 rejoin tests, the shared voice private-thread regression, and the command-migration/reseeding test. Formatting, whitespace, and publication-contract checks also passed; the current GitHub CI checks are green. The worktree remains clean.
- Test boundaries: Local tests use real SQLite and Serenity code with simulated Gateway data and local HTTP fixtures, not live Discord. Full local preflight and live-provider testing were not run; the remote-database CI job passed.
gpt-6.1-sol ｜ 𝕏
Part of #159
Part of #159
Summary
member_case_entrieswith aCaseLimitsread budget. The index-gap check uses the same recent window as the lookup (LIMITonguild_member_cases_user), and a read still returnsStalewhile case-index backfill is unfinished.record_caserelies on the insert trigger.0022_clear_userinfo_command_rowsremoves storeduserinfocommand rows (core schema 22). Seed restores the shipped global row.main, including rejoin prior cases and built-in command access./userinfoand/voiceboth stay.unavailableand omits the name and id. Channel mentions in case reasons follow the same rule.unavailable. Those ids are parsed with the fallible snowflake constructors, so a retained case reason cannot abort prefix or slash userinfo.unavailableand omits the id. An archived public thread stays with parent visibility. A private thread is not named as a voice channel without that access.Verification
cargo test --locked -p jmodguard,-p jmodguard-bot, and-p jmodguard-api(including the API library without default features)cargo clippy --lockedon those packages with-D warnings,cargo fmt --check, andgit diff --check --merge-base origin/mainbash scripts/publish-image-test.shandbash scripts/ci-remote-database.shrest_lookup_failure_still_renders_warns_and_casescallsuserinfo::runthrough a local rejecting proxy and checks that/users/91was requestedprofile_and_case_text_is_sanitized_like_rejoin_reportsanduserinfo_prefix_and_slash_replies_do_not_activate_mentionscover escaped nicknames, role names, channel names, and case reasons on prefix and slash sendsuserinfo_prefix_and_slash_hide_voice_the_caller_cannot_viewcovers a visible channel, a moderator deniedVIEW_CHANNEL, and an uncached channel, on prefix and slashuserinfo_prefix_and_slash_reject_invalid_mention_idscovers<#0>,<@0>,<@&0>, andu64::MAX+1on prefix and slash with the guild cachedmember_rejoin_delivers_when_indexed_case_exceeds_read_budgetwrites an oversize case under a raised write budget, still refuses the default read budget, and sends the rejoin note with the budget noticeuserinfo_prefix_and_slash_hide_private_threads_without_accesscovers a bot-cached private thread, an archived public thread, and an archived private thread, on prefix and slash, including Manage Threads and a parent denyprivate_thread_channel_is_not_named_without_manage_threadskeeps a private thread out of the voice reportSummary by Sourcery
Add a safe, visibility-aware userinfo command backed by bounded member case history.
New Features:
userinfosupport for prefix and slash commands, including cached profile details, warning counts, recent case history, and caller-aware voice/channel visibility.Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: