fix(archive): carbon-copy transcript fidelity + close-workflow robustness (v3.2.1)#7
Merged
Merged
Conversation
…ness (v3.2.1) Archived ticket/application transcripts are now an exact copy of the conversation. The prior builder silently truncated any message over 500 chars and, on archive failure, deleted the source channel — destroying the only remaining copy. Carbon-copy (transcriptBuilder + fetchAllMessages): - Remove the 500-char truncation. Oversized formatted messages are now SPLIT across chunks on line boundaries with code-fence balancing; every emitted chunk is guaranteed <= Discord's 2000-char hard limit (a final defensive hard-slice backstops the invariant). - Capture stickers, polls (question + per-answer vote counts), and embed media (image/thumbnail/footer/author/linked title). Sticker-only and poll-only messages are no longer filtered out; the include filter is aligned with the renderer. Robustness (ticket + application closeWorkflow): - On archive failure, PRESERVE the source channel instead of deleting it, and revert the ticket/application status (in all 4 callers) so the close can be retried. Previously a failed archive deleted the conversation. - Re-close into a deleted archive thread (Discord 10003) recreates the thread and repoints the archive row; non-10003 errors bubble (and preserve the channel). force:true on the fetch bypasses a stale cache. - Persist the archive row BEFORE posting chunks so a mid-archive chunk failure can't orphan the thread + duplicate it on retry. - Per-chunk failure logging attributes a partial-archive failure to its index. Bug: POST /tickets/:id/assign now persists assignedTo + assignedAt (was only setting a channel permission overwrite). Testing: +application closeWorkflow suite (B1/B2 coverage via a new deps seam mirroring the ticket workflow), +fetchAllMessages mapping suite, +B1 recovery / channel-preservation / chunk-failure / carbon-copy fidelity tests. registerApplicationHandlers gains an injectable archive param so its api test injects instead of mock.module()-ing the shared workflow (avoids the process-global poison that the ticket side already designs around). 1278 tests pass; build + biome clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 38 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThis release delivers lossless transcript handling with sticker and poll support, archive failure recovery that preserves channels for retry, automatic recovery when archive threads are deleted, and persisted ticket assignment metadata. Transcript chunking now splits oversized messages on line boundaries with code-fence preservation instead of truncating; workflows persist archive records before posting chunks to enable safe retries; and event handlers revert status on failure instead of leaving channels in inconsistent states. ChangesTranscript Data Models & Formatting
Archive & Close Workflows with Failure Recovery
Event Handlers & API Routes with Status Reversion
Release Version Update
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR hardens the ticket/application archival workflows to prevent transcript data loss and makes archived transcripts a “carbon copy” of the original Discord conversation, including additional message components (stickers/polls/expanded embeds). It also fixes ticket assignment persistence so the DB reflects the assignee and assignment time.
Changes:
- Remove per-message truncation and introduce chunk splitting to guarantee transcript posts stay within Discord’s 2000-character limit while preserving full content.
- Make close/archive workflows retry-safe: preserve source channels on archive failure, persist archive rows before chunk posting, and recover when an existing archive thread was deleted (Discord 10003).
- Persist
assignedTo/assignedAtinPOST /tickets/:id/assign, and expand unit test coverage for transcript mapping and close workflows.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/utils/ticket/transcriptBuilder.test.ts | Updates/expands transcript builder tests to cover non-truncation, chunk splitting, and new content types (stickers/polls/embeds). |
| tests/unit/utils/ticket/closeWorkflow.test.ts | Adds robustness tests for archive failure behavior, chunk-send failures, cache-bypassed thread fetch, and 10003 recovery. |
| tests/unit/utils/fetchAllMessages.test.ts | Adds mapping tests to ensure fetchMessagesAsTranscript populates the transcript-builder fields correctly (embeds/stickers/polls/components). |
| tests/unit/utils/application/closeWorkflow.test.ts | Adds an application close-workflow suite mirroring ticket workflow failure/retry protections. |
| tests/unit/utils/api/ticketHandlers.test.ts | Updates close handler expectations (revert status on archive failure) and adds tests for persisted assignment behavior. |
| tests/unit/utils/api/applicationHandlers.test.ts | Switches from mock.module() to dependency injection and updates failure expectations (revert status/no audit log). |
| src/utils/ticket/transcriptBuilder.ts | Implements carbon-copy transcript formatting, adds stickers/polls/embed media support, and introduces splitting logic to keep chunks <= 2000 chars. |
| src/utils/ticket/closeWorkflow.ts | Preserves channels on archive failure, persists archive rows before chunk posting, adds 10003 recovery, and adds per-chunk failure logging. |
| src/utils/fetchAllMessages.ts | Extends transcript mapping to include embed media, stickers, and polls. |
| src/utils/application/closeWorkflow.ts | Mirrors ticket workflow robustness (deps injection, preserve on failure, 10003 recovery, per-chunk logging). |
| src/utils/api/handlers/ticketHandlers.ts | Reverts ticket status on archive failure and persists assignedTo/assignedAt on assign. |
| src/utils/api/handlers/applicationHandlers.ts | Adds injectable archive function seam and reverts application status on archive failure. |
| src/events/ticket/close.ts | Reverts ticket status + surfaces an error message when archive fails (channel preserved). |
| src/events/application/close.ts | Reverts application status + surfaces an error message when archive fails (channel preserved). |
| package.json | Bumps version to 3.2.1. |
| CLAUDE.md | Updates documented version to 3.2.1. |
| CHANGELOG.md | Adds 3.2.1 changelog entry describing the fidelity/robustness fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+244
to
+249
| for (const line of message.split('\n')) { | ||
| if (line.length > limit) { | ||
| // Pathological single line (e.g. a long URL / base64 blob with no | ||
| // newline). Flush what we have, then emit the line in ≤limit segments. | ||
| flush(false); | ||
| for (const seg of hardSlice(line, limit)) pieces.push(seg); |
Owner
Author
There was a problem hiding this comment.
Addressed in 12dab87: splitFormattedMessage now (1) detects the full leading > run and re-applies the quote prefix to every hard-sliced segment of an over-limit line, and (2) strips the full quote-prefix run for fence detection + closes/reopens fences at the exact blockquote depth they were opened (so double-quoted embed fences > > \``` balance correctly). Added 2 regression tests.
… review) splitFormattedMessage had two edge-case rendering bugs (no data loss, but they hurt the carbon-copy "professional rendering" goal): - Hard-slicing a single >limit line emitted the raw segments, dropping the `> ` blockquote prefix on continuation pieces (and any open code fence). Now the quote prefix is detected and re-applied to every segment, with headroom reserved for a fence close/reopen wrapper. - Fence detection stripped only one `> `, so a fence nested in an embed body (double-quoted: `> > ```) never toggled inFence and wasn't balanced across a split. Detection now strips the full leading `> ` run, and the fence is closed/reopened at the exact depth it was opened. +2 regression tests (double-blockquote fence balancing, long-line prefix preservation). 1280 tests pass; build + biome clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/application/closeWorkflow.ts (1)
143-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPotential data loss if
existingArchiveexists with falsymessageId.If an
ArchivedApplicationrow exists but has a null/undefinedmessageId, neither theif (!existingArchive)branch (line 124) nor theelse if (existingArchive.messageId)branch (line 143) executes. The try block completes successfully,archivedremainstrue, and the channel is deleted without archiving the transcript.While rows are always created with a
messageId, defensive handling prevents data loss from DB corruption or edge cases.🛡️ Proposed fix: add else clause to handle missing messageId
await sendTranscriptChunks(post, transcript.chunks, { guildId, channelId }); } + } else { + // existingArchive exists but messageId is missing — treat as first-time archive + const newPost = await forumChannel.threads.create({ + name: threadName, + message: { content: transcript.header, allowedMentions: { parse: [] } }, + }); + existingArchive.messageId = newPost.id; + await deps.archivedAppRepo.save(existingArchive); + await sendTranscriptChunks(newPost, transcript.chunks, { guildId, channelId }); + } } catch (error) {Alternatively, add a guard at the top of the re-close branch to fail explicitly:
} else if (existingArchive.messageId) { + } else { + throw new Error(`ArchivedApplication for guild ${guildId} has no messageId`); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/application/closeWorkflow.ts` around lines 143 - 179, The re-close logic currently only runs when existingArchive.messageId is truthy, which lets a present ArchivedApplication with a null/undefined messageId fall through and cause data loss; update the conditional handling around existingArchive so that when existingArchive exists but has a falsy messageId you create a new forum thread, set existingArchive.messageId to the new thread.id, persist via deps.archivedAppRepo.save(existingArchive), and then call sendTranscriptChunks (same flow as the newPost branch), or simply change the `else if (existingArchive.messageId)` to `else` and reuse the newPost branch logic to ensure transcripts are always posted and persisted for any existingArchive.
🧹 Nitpick comments (2)
src/utils/ticket/closeWorkflow.ts (1)
330-339: 💤 Low valueConsider reusing
mergeForumTagsfor consistency.The inline tag merge duplicates the logic in
mergeForumTags(line 163). While functionally correct (sinceforumTagIdshas at most one element), using the helper would improve DRY consistency with the thread-recreation path at line 304.♻️ Suggested refactor
if (forumTagIds.length > 0) { - const existingTags = existingArchive.forumTagIds || []; - const newTagId = forumTagIds[0]; - if (!existingTags.includes(newTagId)) { - const mergedTags = [...existingTags, newTagId]; + const mergedTags = mergeForumTags(existingArchive.forumTagIds, forumTagIds); + if (mergedTags.length > (existingArchive.forumTagIds?.length ?? 0)) { await deps.applyForumTags(forumChannel, existingArchive.messageId, mergedTags); existingArchive.forumTagIds = mergedTags; await deps.archivedTicketRepo.save(existingArchive); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/ticket/closeWorkflow.ts` around lines 330 - 339, Replace the inline tag-merge block with a call to the existing mergeForumTags helper to keep behavior consistent and avoid duplicated logic: when forumTagIds.length > 0, compute the merged tags by calling mergeForumTags(existingArchive.forumTagIds || [], forumTagIds[0]) (or the helper's expected signature), then call deps.applyForumTags(forumChannel, existingArchive.messageId, mergedTags), update existingArchive.forumTagIds = mergedTags, and save via deps.archivedTicketRepo.save(existingArchive); reference mergeForumTags, applyForumTags, existingArchive, forumTagIds, forumChannel, and archivedTicketRepo to locate the code.tests/unit/utils/ticket/closeWorkflow.test.ts (1)
9-10: 💤 Low valueStale comment: repository is now fully injected, not patched.
Lines 9-10 state the repo "stays on the AppDataSource.getRepository runtime patch," but lines 123-141 show
archivedTicketRepo: fakeRepois now passed directly via thedepsargument. The header should be updated to reflect the fully-injected approach.📝 Suggested fix
-// are INJECTED via the function's `deps` argument — not mock.module() — so the -// real transcriptBuilder still runs and import order can't affect binding. The -// repository seam is the one exception: it stays on the AppDataSource -// .getRepository runtime patch (lazyRepo resolves it lazily at call-time). +// are INJECTED via the function's `deps` argument — not mock.module() — so the +// real transcriptBuilder still runs and import order can't affect binding. The +// repository seam is ALSO injected via `deps.archivedTicketRepo`, avoiding +// any AppDataSource monkey-patching.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/utils/ticket/closeWorkflow.test.ts` around lines 9 - 10, The test header comment is stale: update the header to remove reference to AppDataSource.getRepository runtime patch and the "repository seam" as a patch; instead state that repositories (e.g., archivedTicketRepo) are fully injected via the deps argument in closeWorkflow.test.ts where archivedTicketRepo: fakeRepo is passed; update any mention of lazyRepo/AppDataSource.getRepository to reflect the fully-injected dependency pattern used in the closeWorkflow (deps) setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/events/application/close.ts`:
- Around line 53-76: Replace the silent .catch(() => {}) handlers on the
fallback interaction responses in the close workflow with error logging: when
calling interaction.editReply(...) and interaction.reply(...), catch the error
and call enhancedLogger.error(...) including a descriptive message and context
object with guildId, channelId, and applicationId (and optionally the caught
error). Keep the existing tl.transcriptCreate.error message and ephemeral flag;
only change the catch blocks to log via enhancedLogger.error rather than
swallowing the error.
In `@src/events/ticket/close.ts`:
- Around line 52-76: Replace the silent .catch(() => {}) blocks on
interaction.editReply and interaction.reply with error logging and handling so
failures are not swallowed: when calling interaction.editReply({ content:
tl.transcriptCreate.error }) and interaction.reply({ content:
tl.transcriptCreate.error, flags: [MessageFlags.Ephemeral] }) change their catch
handlers to log the caught error via enhancedLogger.error(...) (include context:
guildId, channelId, ticketId and LogCategory.SYSTEM) and, if appropriate,
surface or route the error to your existing error handling utilities; keep the
ticketRepo.update and enhancedLogger.warn behavior unchanged but ensure any
reply/edit failures are recorded with the transcript failure information.
In `@src/utils/api/handlers/applicationHandlers.ts`:
- Around line 98-103: The early-return branch that currently returns { success:
true, archived: false } must also revert the application's status back to its
pre-workflow value to avoid leaving it stranded closed; before returning in that
branch (the one that handles the client.channels.fetch()/pre-workflow failure),
call applicationRepo.update({ id: app.id, guildId }, { status: app.status })
(same rollback used after archiveAndCloseApplication()) and then return the
result object so both paths consistently restore the original status.
In `@src/utils/api/handlers/ticketHandlers.ts`:
- Around line 55-60: The branch that does client.channels.fetch(...).catch(() =>
null) can return { success: true, archived: false } on a transient fetch failure
and thus strand tickets as closed; update that branch in ticketHandlers.ts so
when the fetched channel is null you perform the same rollback as the failed
archive path: call ticketRepo.update({ id: ticket.id, guildId }, { status:
ticket.status }) and return { success: false, ticketId: ticket.id, archived:
false } (i.e., treat a missing channel fetch like an archive failure rather than
success).
In `@src/utils/ticket/transcriptBuilder.ts`:
- Around line 244-250: The hard-slicing branch in transcriptBuilder (inside the
loop that calls hardSlice when line.length > limit) is dropping quote prefixes
and fence context; update the logic around hardSlice(line, limit) so each
emitted segment is wrapped with the current quote prefix (e.g., ">" or nested
prefixes) and, if inFence is true, close the fence before emitting the segment
and reopen it immediately after (so each segment is a syntactically valid fenced
block), instead of pushing raw seg; modify the code that pushes into pieces (and
any use of flush) to apply these prefix/fence adjustments for every seg emitted
by hardSlice, and add/adjust unit test to cover one fenced line exceeding
softLimit to ensure the regression is prevented.
- Around line 129-155: Replace the hardcoded user-facing strings in
formatSticker, formatPoll, and formatEmbedBody with values from the shared lang
module: import lang from "src/lang" at the top, use lang.transcript.sticker (or
similar key you add) instead of "Sticker", lang.transcript.poll for "Poll",
lang.transcript.vote and lang.transcript.votes (or a pluralization helper in
lang) for vote(s), and lang.transcript.image / lang.transcript.thumbnail for
"image" and "thumbnail"; update formatPoll to use the lang keys for vote
singular/plural and formatSticker/formatEmbedBody to use the lang keys for
labels and link text while preserving existing emoji and markup. Ensure you add
the new lang keys in the centralized language files so translations are
available.
- Around line 76-79: Replace the hardcoded CHUNK_SOFT_LIMIT and CHUNK_HARD_LIMIT
numbers by adding exports for these limits in the shared constants module (e.g.,
extend TEXT_LIMITS or export CHUNK_SOFT_LIMIT and CHUNK_HARD_LIMIT from
src/utils/constants.ts) and then import those symbols into transcriptBuilder.ts
instead of the literals; update the top of transcriptBuilder.ts to import the
new constants (reference CHUNK_SOFT_LIMIT and CHUNK_HARD_LIMIT) so tests and
archive path logic use the centralized values and no numeric literals remain in
this file.
---
Outside diff comments:
In `@src/utils/application/closeWorkflow.ts`:
- Around line 143-179: The re-close logic currently only runs when
existingArchive.messageId is truthy, which lets a present ArchivedApplication
with a null/undefined messageId fall through and cause data loss; update the
conditional handling around existingArchive so that when existingArchive exists
but has a falsy messageId you create a new forum thread, set
existingArchive.messageId to the new thread.id, persist via
deps.archivedAppRepo.save(existingArchive), and then call sendTranscriptChunks
(same flow as the newPost branch), or simply change the `else if
(existingArchive.messageId)` to `else` and reuse the newPost branch logic to
ensure transcripts are always posted and persisted for any existingArchive.
---
Nitpick comments:
In `@src/utils/ticket/closeWorkflow.ts`:
- Around line 330-339: Replace the inline tag-merge block with a call to the
existing mergeForumTags helper to keep behavior consistent and avoid duplicated
logic: when forumTagIds.length > 0, compute the merged tags by calling
mergeForumTags(existingArchive.forumTagIds || [], forumTagIds[0]) (or the
helper's expected signature), then call deps.applyForumTags(forumChannel,
existingArchive.messageId, mergedTags), update existingArchive.forumTagIds =
mergedTags, and save via deps.archivedTicketRepo.save(existingArchive);
reference mergeForumTags, applyForumTags, existingArchive, forumTagIds,
forumChannel, and archivedTicketRepo to locate the code.
In `@tests/unit/utils/ticket/closeWorkflow.test.ts`:
- Around line 9-10: The test header comment is stale: update the header to
remove reference to AppDataSource.getRepository runtime patch and the
"repository seam" as a patch; instead state that repositories (e.g.,
archivedTicketRepo) are fully injected via the deps argument in
closeWorkflow.test.ts where archivedTicketRepo: fakeRepo is passed; update any
mention of lazyRepo/AppDataSource.getRepository to reflect the fully-injected
dependency pattern used in the closeWorkflow (deps) setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a397bc78-ac77-44fb-bc62-56765c85cd54
📒 Files selected for processing (17)
CHANGELOG.mdCLAUDE.mdpackage.jsonsrc/events/application/close.tssrc/events/ticket/close.tssrc/utils/api/handlers/applicationHandlers.tssrc/utils/api/handlers/ticketHandlers.tssrc/utils/application/closeWorkflow.tssrc/utils/fetchAllMessages.tssrc/utils/ticket/closeWorkflow.tssrc/utils/ticket/transcriptBuilder.tstests/unit/utils/api/applicationHandlers.test.tstests/unit/utils/api/ticketHandlers.test.tstests/unit/utils/application/closeWorkflow.test.tstests/unit/utils/fetchAllMessages.test.tstests/unit/utils/ticket/closeWorkflow.test.tstests/unit/utils/ticket/transcriptBuilder.test.ts
Comment on lines
+129
to
+155
| function formatSticker(s: TranscriptSticker): string { | ||
| if (!s.url) return `> 🏷️ Sticker: ${s.name}`; | ||
| return `> 🏷️ Sticker: [${s.name}](${s.url})`; | ||
| } | ||
|
|
||
| function formatPoll(poll: TranscriptPoll): string[] { | ||
| const lines = [`> 📊 **Poll:** ${poll.question}`]; | ||
| for (const answer of poll.answers) { | ||
| const votes = `${answer.voteCount} vote${answer.voteCount === 1 ? '' : 's'}`; | ||
| lines.push(`> • ${answer.text} — ${votes}`); | ||
| } | ||
| return lines; | ||
| } | ||
|
|
||
| function formatEmbedBody(embed: TranscriptEmbed): string[] { | ||
| const parts: string[] = []; | ||
| if (embed.title) parts.push(`**${embed.title}**`); | ||
| if (embed.author) parts.push(`*${embed.author}*`); | ||
| if (embed.title) parts.push(embed.url ? `**[${embed.title}](${embed.url})**` : `**${embed.title}**`); | ||
| if (embed.description) parts.push(embed.description); | ||
| if (embed.fields && embed.fields.length > 0) { | ||
| for (const field of embed.fields) { | ||
| parts.push(`*${field.name}:* ${field.value}`); | ||
| } | ||
| } | ||
| if (embed.imageUrl) parts.push(`🖼️ [image](${embed.imageUrl})`); | ||
| if (embed.thumbnailUrl) parts.push(`🖼️ [thumbnail](${embed.thumbnailUrl})`); | ||
| if (embed.footer) parts.push(`— ${embed.footer}`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Route the new transcript labels through src/lang.
Sticker, Poll, vote(s), image, and thumbnail are all new user-facing strings, but they are hardcoded here. Please move them behind the shared language module so transcript output remains localizable. As per coding guidelines, "Use the centralized lang module for all user-facing strings. Import from src/lang and use lang.feature.key or lang.featureSetup.key for setup strings. Never hardcode strings."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/ticket/transcriptBuilder.ts` around lines 129 - 155, Replace the
hardcoded user-facing strings in formatSticker, formatPoll, and formatEmbedBody
with values from the shared lang module: import lang from "src/lang" at the top,
use lang.transcript.sticker (or similar key you add) instead of "Sticker",
lang.transcript.poll for "Poll", lang.transcript.vote and lang.transcript.votes
(or a pluralization helper in lang) for vote(s), and lang.transcript.image /
lang.transcript.thumbnail for "image" and "thumbnail"; update formatPoll to use
the lang keys for vote singular/plural and formatSticker/formatEmbedBody to use
the lang keys for labels and link text while preserving existing emoji and
markup. Ensure you add the new lang keys in the centralized language files so
translations are available.
…ch revert, constants)
- events/{ticket,application}/close.ts: log the user-notification failure
instead of swallowing it with `.catch(() => {})` (project rule: never
swallow errors silently).
- api/handlers/{ticket,application}Handlers.ts: the close/archive routes now
distinguish a genuinely-gone channel (Discord 10003 → nothing to archive,
terminal close) from a transient/permission fetch failure (→ revert the
status so a retry can still archive). Previously any channel-fetch failure
stranded a live ticket/application as 'closed'. +4 tests (transient + 10003,
both routes).
- transcriptBuilder: chunk soft/hard limits moved to TEXT_LIMITS in
constants.ts (no hardcoded magic numbers).
Deferred (replied on PR): routing the transcript labels through src/lang — the
builder's header labels are all currently hardcoded English too, so localizing
only the new sticker/poll labels would be inconsistent; transcript i18n is a
separate builder-wide pass.
1284 tests pass; build + biome clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Thanks @coderabbitai — addressed in ac7940c:
Deferred: routing transcript labels ( 1284 tests pass; |
|
Tip For best results, initiate chat on the files or code changes.
The deferral for transcript label localization is entirely reasonable. Localizing only the new labels while the rest of the builder (headers, separators, etc.) stays hardcoded English would introduce an inconsistent mix. A single builder-wide i18n pass is the right call. Would you like me to open a tracking issue for the transcript builder i18n work so it doesn't get lost? 🐇 ✨ ✏️ Learnings added
🧠 Learnings used |
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
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.
Summary
Archived ticket & application transcripts are now an exact copy of the conversation. The prior builder silently truncated any message over 500 chars and, on archive failure, deleted the source channel — destroying the only remaining copy.
Fixes the message-loss reported in production, plus 8 findings from an adversarial pre-merge review.
Carbon-copy fidelity
Close-workflow robustness (ticket + application)
force:truebypasses a stale cache.Bug fix
POST /tickets/:id/assignnow persistsassignedTo+assignedAt(was only a permission overwrite).Testing
application/closeWorkflowsuite (B1/B2 via a deps seam mirroring the ticket workflow) + newfetchAllMessagesmapping suite + B1/channel-preservation/chunk-failure/carbon-copy tests.registerApplicationHandlersgains an injectable archive param so its api test injects instead ofmock.module()-ing the shared workflow (avoids the process-global poison the ticket side already designs around).bun run build+biomeclean.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes – Version 3.2.1
New Features
Bug Fixes