Skip to content

fix: transcript delivery without an API key, add transcriptToUser - #34

Merged
Musiker15 merged 8 commits into
MSK-Scripts:mainfrom
Uncharted-Roleplay:fix/transcripts
Oct 23, 2026
Merged

Musiker15 merged 8 commits into
MSK-Scripts:mainfrom
Uncharted-Roleplay:fix/transcripts

Conversation

@shane615

@shane615 shane615 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Description

Transcript fixes found while reviewing the bot before deploying it for our community (Uncharted Roleplay). Each commit is one change:

  • The placeholder API key uploaded transcripts. MSK_API_KEY="YOUR_MSK_API_KEY_HERE" from .env.example was treated as "no key" only by checkApiKey(). uploadTranscript() and collectAttachments() only checked for a non-empty string, so a copied example file POSTed every transcript (plus up to 100 MB of attachments) to msk-scripts.de, which rejected it with 401. configuredKey() / isMskConfigured() now apply the same rule everywhere.
  • Without a key nothing is uploaded. The transcript is attached to the log channel as an HTML file, without the "upload failed" warning. A real upload failure still falls back to the file, with the warning.
  • The close DM respects closeOption.dmUser. It was forced whenever a fallback file existed (so on every close without a key), with a "service temporarily unavailable" note that wasn't true.
  • New closeOption.transcriptToUser (default true, unchanged behaviour). false keeps transcripts staff-only: the close DM carries neither link nor file. Useful because a reopen-then-close transcript contains everything staff wrote while the ticket was closed and hidden from the user.
  • The final transcript on delete falls back to a log-channel file too.
  • Question answers are in the transcript. The renderer showed only an embed's title and description; the answers live in the opening embed's fields, so they were missing from every transcript. Fields are now rendered (escaped) in both designs.
  • Emoji fan-out is capped. buildEmojiMap fetched every unique <:x:id> in parallel; a user could paste thousands of fake ids. Now at most 200, ten at a time; the rest use the :name: fallback.
  • Long answers no longer break ticket creation. Answer fields are truncated to Discord's limits (256 / 1024); a larger question maxLength made addFields throw after the channel had been created.
  • MSK requests have timeouts (60 s upload, 10 s otherwise), so a hung server cannot block a close or the startup key check.

Related Issue

None opened; these came out of a code review. Happy to open issues if you prefer that workflow.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that changes existing behavior)
  • 🌍 Translation / locale update
  • 📖 Documentation only
  • 🔧 Refactor / chore (no functional change)

How Was This Tested?

  • Node.js version: 24.20.0
  • npm test: all existing tests pass, plus new node:test suites: tests/transcript-delivery.test.js (no network call without a real key, log-channel file, dmUser / transcriptToUser combinations) and tests/transcript-fields.test.js (fields rendered and escaped, emoji cap).
  • Live test (2026-10-23), without an MSK key: closing posts the HTML transcript to the log channel with the question answers and embedded images; with transcriptToUser: false the DM has no transcript; the transcript is stored in the database. Not tested with a real MSK key (we run without one); the keyed upload path was only changed by the key check and the timeout.
  • Images in key-less transcripts are embedded (data URIs, up to 5 MB, oldest first): the Discord CDN links expire after about a day and die immediately when the channel is deleted. Tests in tests/inline-images.test.js.

Checklist

  • My code follows the project's conventions (CommonJS, no TypeScript, tb_ custom-id prefix, client.logger, etc.)
  • No new production dependencies were added (or it was discussed in an issue first)
  • User-facing strings go through client.t(...) and all three locale files (main.json, en.json, de.json) are in sync (no new user-facing strings)
  • Any DB schema change uses an inline migration in initDatabase() (no schema change)
  • The bot still works without MSK_API_KEY (premium features degrade gracefully)
  • The bot starts cleanly (npm start) and I verified my change on a test server (see above)
  • I updated the relevant docs (README.md / README_GER.md, config.example.jsonc, CLAUDE.md) where behavior, config keys, commands or the schema changed (README.md, README_GER.md, config.example.jsonc; the dashboard's config form does not have a toggle for transcriptToUser yet, it is editable in file mode)
  • I added a CHANGELOG.md entry under [Unreleased]
  • My commit messages are in English and follow Conventional Commits (feat:, fix:, docs:, …)

Note on the related PRs

This is one of three PRs from the same review (robustness, permissions, transcripts). They are independent and each applies to main on its own, but they touch some of the same lines (ticketActions.js, CHANGELOG.md). Following your order: after #32, before #33; I will rebase this once #32 is merged.

https://claude.ai/code/session_01Kp1SLbxxAbXw3f42C1qThH

@Musiker15

Copy link
Copy Markdown
Member

Thanks, this is a good catch, especially the placeholder key.

Confirmed on current main: uploadTranscript() and collectAttachments() only check for a non-empty key, so a copied .env.example really does send every transcript to msk-scripts.de and gets a 401 back. The forced DM with the misleading "temporarily unavailable" note, the missing embed fields (so the question answers never make it into the transcript), the unbounded emoji fetches and the missing timeouts are all still there too.

Tests pass on your branch (178/178).

Two small things:

  • The new constants MAX_TRANSCRIPT_EMOJIS / EMOJI_FETCH_CONCURRENCY sit between the JSDoc block of buildEmojiMap and the function, so the comment no longer belongs to it. Could you move them above the comment?
  • closeOption.transcriptToUser is a new config key, so it should also show up in the dashboard's config form. If you want, add it next to dmUser in web/src/botconfig/schema.js. Otherwise I'll do that after the merge. You don't need to rebuild web/dist, I'll handle that.

The branch conflicts with main right now (CHANGELOG.md, plus ticketActions.js once #32 is in). I'd merge it right after #32, so a rebase after that merge would be perfect.

@shane615

Copy link
Copy Markdown
Contributor Author

Thanks! Done:

  • Moved the emoji constants above the buildEmojiMap doc comment.
  • Added closeOption.transcriptToUser to web/src/botconfig/schema.js next to dmUser (web/dist not rebuilt). Like updateNotification.enabled, a missing key shows as off in the form although the bot treats it as on; the help text says so.
  • Rebased onto main.

The live test found one more gap in this PR's no-key path: the HTML file still pointed images at the Discord CDN. Those links expire after about a day, and immediately when the channel is deleted. Images are now embedded as data URIs, up to 5 MB per transcript so the file stays under the attachment cap, with tests in tests/inline-images.test.js. With that, closing without a key passed on our server: the transcript in the log channel has the question answers and the images, and the user's DM has no transcript with transcriptToUser: false. We haven't tested the keyed path, as we run without a key.

I'll rebase once #32 is in and mark it ready then.

@shane615

Copy link
Copy Markdown
Contributor Author

Rebased onto main now that #32 is in. ticketActions.js: #32's orphan helpers and this PR's buildTranscriptFile landed in the same spot, both kept. CHANGELOG.md: entries merged into the existing ### Fixed / ### Added lists after #32's. Tests pass. Marking it ready.

@shane615
shane615 marked this pull request as ready for review October 23, 2026 21:08
@Musiker15

Copy link
Copy Markdown
Member

Thanks! I compared the branch before and after the rebase commit by commit, the code is unchanged and tests pass (189/189). #38 is merged now and #39 comes next, so CHANGELOG.md will conflict again. Best to rebase once #39 is in, then I'll merge this one.

@Musiker15

Copy link
Copy Markdown
Member

#39 is merged now, so this is the last one before #33. Could you rebase onto main once more? CHANGELOG.md will conflict again, just put your entries after the #38 and #39 ones. I'll merge it right after.

…ptToUser

- The .env.example placeholder MSK_API_KEY was only treated as "no key" by
  the startup check; uploads and attachment collection still POSTed every
  transcript to msk-scripts.de with it. configuredKey()/isMskConfigured()
  now apply the same rule everywhere.
- Without a key the transcript is not uploaded at all: it is attached to the
  log channel as an HTML file, with no "upload failed" warning. A real upload
  failure still falls back to the file, with the warning.
- The close DM is sent only when closeOption.dmUser is on. It used to be
  forced whenever a fallback file existed, with a misleading "service
  unavailable" note.
- New closeOption.transcriptToUser (default true): false keeps transcripts
  staff-only — the close DM carries neither link nor file.
- The final transcript on delete also falls back to a log-channel file.
- MSK requests get timeouts (60 s upload, 10 s otherwise) so a hung server
  cannot block a close or the startup key check.

Claude-Session: https://claude.ai/code/session_01Kp1SLbxxAbXw3f42C1qThH
- Transcripts rendered only an embed's title and description, so the
  answers to the ticket questions (stored as fields of the opening embed)
  were missing from every transcript. Fields are now rendered, escaped,
  in both designs.
- buildEmojiMap fetched every unique custom emoji id in parallel; a user
  could paste thousands of fake ids. It now fetches at most 200, ten at a
  time; the rest fall back to their ":name:" text.
- The opening embed truncates answer fields to Discord's limits (name 256,
  value 1024). A longer question maxLength made addFields throw after the
  ticket channel had been created.

Claude-Session: https://claude.ai/code/session_01Kp1SLbxxAbXw3f42C1qThH
Review feedback: the JSDoc block ended up attached to the constants instead
of buildEmojiMap.

Claude-Session: https://claude.ai/code/session_01HZPZxD3edzJNFFsiWsHYY1
Toggle next to dmUser in Close Options. web/dist is not rebuilt here, the
maintainer rebuilds it on release.

Claude-Session: https://claude.ai/code/session_01HZPZxD3edzJNFFsiWsHYY1
Without an API key the transcript is a standalone HTML file, but images
still pointed at the Discord CDN: the links are signed and expire after
~24 h, and with deleteOnClose they died with the channel seconds after
the close, leaving only the file name. Images (png/jpeg/gif/webp/avif/
bmp) are now embedded as data URIs, oldest first, up to 5 MB so the file
stays under the 9 MB attachment cap. Found in the live test.

Claude-Session: https://claude.ai/code/session_01HZPZxD3edzJNFFsiWsHYY1
@shane615

Copy link
Copy Markdown
Contributor Author

Rebased onto main with #38 and #39 in. Only the changelog commit changed: the entries follow the #38 and #39 ones in ### Fixed; the other seven commits are identical (git range-diff). Tests pass.

@Musiker15
Musiker15 merged commit 5b4a151 into MSK-Scripts:main Oct 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants