Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…74) PG parity tests (RUN_PG_TESTS=1) not run in this environment (no DB available); default suite: 85 passed, 9 skipped (all gated PG tests).
…idempotent, cross-checked)
…dempotent, cross-checked)
…ic identifier CRUD
…rt conflict to 409
…d_grant (CodeRabbit #87)
…Neon cold-start /team autocomplete was intermittently returning empty: the discord bot's principal lookup (discordId -> person) resolves to an anonymous principal after PRINCIPAL_AUTOCOMPLETE_TIMEOUT_MS. On an idle Neon branch the first directory lookup hits a compute cold-start (~1.0-1.4s observed in staging/prod logs), exceeding the old 1000ms budget. Raise it to 2000ms and trim the team-lookup budget 1500->700ms (the principal lookup has already warmed the same DB, warm ~80ms), keeping the stacked sum at 2700ms (test bar: <=2800). Stopgap only; the durable fix is keeping the directory DB warm (disable Neon autosuspend) or caching the principal/team lookups. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The mock-timer test ticked a hardcoded 1500ms; with the budget raised to 2000ms the timeout no longer fired and the hung lookup never resolved (cascading to sibling tests). Import PRINCIPAL_AUTOCOMPLETE_TIMEOUT_MS and tick past it so the test tracks the constant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pgap fix(bot): absorb Neon cold-start in autocomplete (1000->2000ms principal budget)
…caller) After doc-visibility (#74) shipped, `/doc list` returned empty: the bot reads with a plain docs:read key and no asserted actor, which the doc service now treats as DENY. Thread the linked caller's person_id through doc command -> docService -> docClient as an X-On-Behalf-Of header on the read paths (list + show), so the doc service filters to what that user may see. Writes are unchanged (docs:write remains the trust boundary). Requires the bot's doc-system key to carry the act-as-user scope (rotated separately in staging/prod). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(bot): visibility-filter /doc reads as the caller (X-On-Behalf-Of)
CodeRabbit nitpicks on #89: symmetric getDoc "omits X-On-Behalf-Of" case, and the fail-closed path where an unlinked caller (principal=null) sends no onBehalfOf on /doc list and /doc show. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…log LLM degradation (review follow-up)
…iew follow-up) When an Opus stream emits 'error' without a following 'end' or 'close', the userId remained stuck in activeSubscriptions, causing the speaker to be dropped for the rest of the meeting. Now registering the clearActive handler on 'error' as well as 'end' and 'close' to ensure cleanup in all terminal-event paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…in uv image, pdf test threshold (CodeRabbit #119)
…del + service-count accuracy (CodeRabbit #119)
…-svc; review follow-up)
…DING
Data-flow diagram, Live and Stop paragraphs described persistent per-speaker
Transcribe streaming (the deferred model), and the wire-contract section still
showed ?key= WS auth. Updated to the current buffered re-transcription model
(consistent with the "Why stateful" + "Known limitations" sections) and the
first-message {"key":...} WS auth the bot now uses.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ce paths (CodeRabbit #119) Bare patterns only match the build-context root, but every service Dockerfile COPYs services/<name>/, so nested tests/.env.example still landed in images. Switched to recursive patterns. Verified the meeting image still builds and imports cleanly in-container. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat: meeting recording v2 — stateful meeting service + Discord voice surface (live transcription)
…eaker decoder (live-verify)
…ce E2EE Discord made end-to-end encryption (the DAVE protocol) mandatory for all voice connections in March 2026. @discordjs/voice 0.18.0 predates DAVE, so the voice websocket was rejected with close code 4017 and the bot silently timed out on join — /record never connected. 0.19.2 pulls in @snazzah/davey (the maintained DAVE implementation) and negotiates E2EE, so the bot reaches the READY voice state. Verified live end-to-end on Railway staging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
decode() best-effort-swallows a libopus decode error and returns b"", but the existing tests only exercised empty (returns before the try/except) and valid packets -- the except av.error.FFmpegError recovery was never run. Add a case feeding a non-empty invalid packet, asserting it returns b"" and that the decoder still handles a valid packet afterward. Also implicitly guards the av.error.FFmpegError attribute path against a future PyAV upgrade. Addresses CodeRabbit review on #120. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-decode fix: Discord voice E2EE (DAVE) + service-side Opus decode — makes /record work end-to-end
The minutes PDF used fpdf2 core fonts (Helvetica/Courier), which are latin-1 only, so `_safe()` replaced every unsupported char -- emoji, Cyrillic, many accented names -- with '?'. Bundle DejaVu Sans/Mono (regular+bold) and drop the lossy latin-1 replacement so accented and non-latin (Cyrillic/Greek/...) text renders correctly. CJK/emoji fall outside DejaVu's coverage and degrade to a missing-glyph box, never a '?'. Also polishes layout: page-number footer (Page X of Y), an accent-colored title with a hairline rule, accent-colored section headings, Unicode bullets, and a chat-style transcript (bold accent '[ts] Speaker:' prefix, regular mono body, blank line between turns). Fonts (DejaVu 2.37, free license) are copied from a local install -- no runtime download -- and bundled under src/assets/fonts/ with LICENSE_DEJAVU; the path is resolved relative to __file__ so it works in the container (Dockerfile COPY already includes them). Tests cover Unicode-font embedding and multi-page pagination with alias resolution. 71 tests pass, ruff clean. Closes #122. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Use fpdf2 tables where the data is structured, keeping prose/lists where it isn't: - Meta header -> borderless key/value table (bold labels, muted values). - Action items -> a checkbox (☐) checklist table, so wrapped action text stays aligned in its column. - Transcript -> Time / Speaker / Text table: accent header row repeated on every page, monospace times, bold speaker names, zebra striping, wrapping in the text column. Decisions stay bullets and summary stays a paragraph (a table adds nothing to a plain list / free text). action_items are unstructured strings (no owner field in the Minutes contract), so no Owner column is fabricated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat(meeting-svc): Unicode PDF fonts + minutes/transcript polish (#122)
…cord auth Three related changes to the meeting-recording feature: - Auto-stop: wireDiscordClient now listens for voiceStateUpdate and ends the recording when the last non-bot member leaves the recorded channel (same path as /record stop). meetingSurface stores the voice channel opaquely and exposes activeVoiceChannel(guildId) so the head-count check reads from the live session and stays discord.js-free. - No length cap: max_meeting_ms now defaults to None (was 4h). Meetings run until /record stop or auto-stop-on-empty. Set MAX_MEETING_MS to re-enable a bound. Caveat (until #121's incremental streaming): with no cap a marathon meeting buffers all PCM in memory and can OOM; auto-stop-on-empty mitigates the common case. - Auth: /record is a dedicated adapter path that bypasses router.js and thus the Policy Enforcement Point, so it never enforced its declared 'linked' policy. handleRecordInteraction now re-runs resolvePrincipal -> authorize('linked') itself, turning away unlinked callers with the standard message and failing closed if the directory is unavailable. Tests: activeVoiceChannel lifecycle; auth allow/deny/unavailable on /record start; auto-stop stop/no-op matrix. Bot suite (changed files) + full meeting service suite pass; ruff clean. (Two unrelated bot tests fail locally on Node 18.12 -- node:test 'mock' / diagnostics.tracingChannel -- and pass on CI's Node.) Docs updated in docs/MEETING-RECORDING.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
From an independent review pass: - Don't strand a running recording during a directory outage: /record stop and /record status are now 'public' (declared per-subcommand in record.js); only 'start' stays 'linked' (it's the resource-consuming op). stop is de-escalating and status is a local read -- gating them fail-closed removed the one memory escape hatch exactly when the directory is down. (Important) - Debounce auto-stop: a single stray 'last member left' event (client blip / voice-region failover) no longer irreversibly finalizes a live meeting. createAutoStop schedules the stop after AUTO_STOP_GRACE_MS and cancels it if a human returns -- on a later event or via a fire-time re-check. (Important) - Derive the auth policy from command metadata (activeSubcommand.auth ?? command.auth ?? 'linked') instead of hardcoding 'linked', so the dedicated path can't drift from record.js if it's ever retightened. (Minor) - Note the GuildMembers-intent / member-cache caveat on the head-count; the grace-timer re-check is the backstop. (Minor) - Tests: /record stop+status work unlinked/through an outage; auto-stop debounce (schedule, cancel-on-return, fire-time re-check, channel-move, humans-remain, not-recording); config max_meeting_ms=None default + gt=0-when-set; sessions no-drop when uncapped. Bot changed-file tests + full meeting suite (74) pass; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex review caught a bug in the debounce itself: pending timers keyed only by
guildId let a timer scheduled for recording A terminate a LATER recording B in
the same guild -- A empties and schedules; A is stopped and B starts; B's empty
event hits `pending.has(guildId)` and skips scheduling, so A's stale timer fires
and stops B before B's own grace elapsed.
Fix: meetingSurface exposes `activeSession(guildId)` -> { sessionId, voiceChannel }
(replaces activeVoiceChannel), and createAutoStop binds each pending timer to its
sessionId. A new recording clears a stale timer and schedules its own full-grace
one; a stale timer no-ops at fire time (sessionId mismatch). Added a cross-session
regression test. Bot tests + full meeting suite pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per product call: rather than removing the length cap entirely, keep a 4h max_meeting_ms as a SAFETY BACKSTOP. The normal end condition is /record stop or auto-stop-on-empty; the cap only bounds worst-case memory for a forgotten or marathon meeting (the current design buffers all PCM in memory until #121). Still overridable via MAX_MEETING_MS, and settable to None to disable. Docs + tests updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat(meeting): auto-stop on empty channel + 4h cap backstop + enforce /record auth
…bers Auto-stop-on-empty wasn't firing live. Root cause: the head-count read channel.members, which resolves each occupant to a GuildMember via guild.members.cache -- a cache only the privileged GuildMembers intent keeps populated. The bot doesn't request GuildMembers, so that resolution is unreliable and miscounted occupants (including the recorder bot itself, whose member often isn't cached), so the channel never looked empty. Fix: count from guild.voiceStates.cache, which the GuildVoiceStates intent (the one that powers voice receive -- already enabled) keeps populated with each occupant's user id + channel id, no member-cache dependency. Exclude the recorder bot by its own user id (client.user.id, threaded in via getBotId); other bots are excluded best-effort via any resolved member. GuildMembers stays un-requested. Confirmed against the discord.js 14.26 docs: 'GuildMembers intent is required to keep member data synchronized' / methods return incomplete data without it. Tests updated to model guild.voiceStates.cache + a bot id, incl. a regression test that a channel containing only the bot (member unresolved) reads as empty. Bot changed-file suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…count fix(meeting): auto-stop head-count from voiceStates (channel.members needs GuildMembers)
Redesign the minutes PDF around UTMIST branding, and let the LLM title each meeting instead of the code: - Branded layout: a cobalt (#002FCB, from the logo) masthead band on page 1 with the UTMIST logo in a white chip + "Meeting Minutes", then the meeting title, an at-a-glance meta strip (date, duration, N participants, N action items), and a branded footer (UTMIST + page numbers) on every page. Sections restyled to the brand (cobalt headings/rules, cobalt-header striped transcript table). - Logo asset: services/meeting/src/assets/brand/utmist-logo.png (cropped, 600px, transparent). It's the only image; all text stays real text, so the PDF remains fully text-parsable (pdftotext / search / LLM re-ingest). - LLM title: Minutes gains a "title" field; the minutes prompt now asks for a short descriptive title, and the PDF renders it (falling back to a meta title, then "Meeting Minutes"). sessions.py no longer invents a "Meeting <uuid>" title. - Fonts unchanged (bundled DejaVu Unicode): accents/Cyrillic/Greek render; CJK and emoji degrade to a missing-glyph box, never a "?". Tests: _document_title precedence, LLM title parse/absent/outage. 77 pass, ruff clean. Verified visually (branded masthead + LLM title render correctly). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat(meeting): branded UTMIST minutes PDF + LLM-generated title
The voice surface has been exercised on staging through the meeting-recording v2 series (PRs #119, #120, #127-#130), so /record moves off the testing-guild beta channel and registers globally like every other command. Flips `beta` to false in src/commands/record.js and updates the two tests that asserted record was the sole beta command — the beta partition is now empty. registerCommands still sends the empty beta body to DISCORD_GUILD_ID, which clears the stale guild-scoped /record copy on staging. Docs: adds /record to the root README command list (it was missing entirely) and adds the `meeting` service row to the services table. Notes in the bot README that /record needs the `meeting` service deployed with MEETING_BASE_URL + MEETING_API_KEY set on the bot in that environment. Prod prerequisite: `meeting` does not exist in the production Railway environment yet. It must be provisioned and wired into the bot BEFORE prod command registration, or /record shows up in the UTMIST server and fails. Part of #124. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promotes
staging→main(production release). Closes the long-deferred promotion tracked in #124 — 175 commits / 38 PRs of unreleased work.The divergence is clean: main's 3 "ahead" commits are empty merge commits, and
git diff origin/staging...origin/mainis empty, so main is a strict content subset. No cherry-picks or conflict resolution.What ships
Security & authz
X-On-Behalf-Of,act-as-userscope) and per-doc visibility grants — Document visibility + on-behalf-of authorization (#73, #74) #87, fix(bot): visibility-filter /doc reads as the caller (X-On-Behalf-Of) #89, test(bot): getDoc header omission + unlinked fail-closed coverage (CodeRabbit #89) #90parent_idvalidation (fix(team-tracking): distinguish bad parent_id (400) from slug conflict (409) #100), membership integrity (fix(team-tracking): membership FK→400, active_only future dates, temporal-overlap uniqueness #104)chatscope enforcement (fix(llm): enforce chat scope on POST /chat #96)Meeting recording (new service)
services/meeting— Discord voice → Amazon Transcribe → LLM minutes → branded PDF + audio, nothing persisted (feat: meeting recording v2 — stateful meeting service + Discord voice surface (live transcription) #119, feat(meeting-svc): Unicode PDF fonts + minutes/transcript polish (#122) #127, feat(meeting): auto-stop on empty channel + 4h cap backstop + enforce /record auth #128, fix(meeting): auto-stop head-count from voiceStates (channel.members needs GuildMembers) #129, feat(meeting): branded UTMIST minutes PDF + LLM-generated title #130)/recordpromoted from beta to stable (chore(bot): promote /record from beta to stable #131)Bot & directory
/add-email+ multi-valued email identifiers (feat: additional verified emails (multi-email identity mapping + /add-email bot commands) #91),/help(Add permission-aware /help command #93),/docpromoted to stable (docs(readme): /doc is now stable (6 stable commands, 0 beta) #107, feat(bot): promote /doc from beta to stable (global registration) #108)/doc addowner fix (fix(bot): /doc add owns the doc as the caller (owning_person_id) #94)Infra & docs
people:elevateprovisioning fix (fix(scripts): grant people:elevate to the discord-bot directory key #106)All three API services run
alembic upgrade headas a RailwaypreDeployCommand, so merging this triggers them against the prod Neon branches with no manual gate:006_email_provider_multivalued,007_membership_no_overlap003_doc_grants,004_docs_url_unique_activeTwo rewrite live data and cannot be undone:
004_docs_url_unique_activesoft-removes (active=false) every duplicate-URL doc except the earliest, then adds the partial unique index. Itsdowngrade()drops only the index.007_membership_no_overlapcollapses overlapping memberships to zero-length dateranges (updated_by='migration_007_dedup') before adding the gist EXCLUDE constraint.Both self-backfill, so they won't fail on dirty data — they'll silently rewrite it. Snapshot both prod Neon branches before merging.
Post-merge, required
/add-email,/help,/record,/verify-emailare new to prod and stay invisible untilnpm run register:all. Global registration takes ~1h to propagate./healthon all six services;/whoamiand an authenticated/doc show; one/record start→stopcycle.Prod prerequisites (done)
meetingservice provisioned in the production Railway environment, trackingmain, with all 10 vars set.discord-bothasMEETING_BASE_URL+MEETING_API_KEY.llmCONSUMER_KEYScarries bothdiscord-botandmeetingconsumers.Note
services/meeting/does not exist onmainuntil this merges, so prod meeting's current build is red by design and goes green on merge.🤖 Generated with Claude Code