fix(gateway): make privileged Discord intents opt-in

[marcusrbrown]

Closes #646.

What changes

The Discord gateway's default intent set drops to the non-privileged baseline (Guilds + GuildMessages). The two privileged intents — MessageContent and GuildMembers — become opt-in via a single env var.

# Opt in to either or both privileged intents
DISCORD_PRIVILEGED_INTENTS=MessageContent,GuildMembers

Allowed values are exactly those two, case-sensitive, comma-separated, whitespace-tolerant. An unknown token (typo, wrong case, or any other intent name) fails the gateway at startup with an operator-friendly error naming the bad value and listing what's allowed. The matching DISCORD_PRIVILEGED_INTENTS_FILE secret-file fallback comes for free from readOptionalSecret.

createDiscordClient keeps its Set-based merge — DEFAULT_INTENTS is just smaller now. Callers that opt in via the new env var get their privileged intents composed on top of the baseline; callers that don't run with the minimum needed for slash commands and @-mention events.

Why

Discord marks MessageContent and GuildMembers privileged because they grant broad-scope reads against guild data. Until now the gateway requested both unconditionally even though the runtime workload (slash commands, @-mention responses in guild channels) doesn't need either. Flipping the default narrows the blast radius for every deployment that doesn't have a content-reading reason to ask for them.

Migration

Existing deployments that depend on the privileged intents must set DISCORD_PRIVILEGED_INTENTS=MessageContent,GuildMembers (or whichever subset they actually use) on next deploy. Without it, the bot stays connected and serves mentions in guild channels but stops receiving message content and full member lists.

Notes

• validateTokenIsFake is a small test-only guard: a beforeAll in client.test.ts refuses to run the suite when DISCORD_TOKEN looks like a real token, catching the common case of a .env accidentally sourced into a dev shell.
• The privileged-intent allowlist uses Object.prototype.hasOwnProperty.call rather than in so prototype-property tokens (constructor, __proto__, etc.) hit the explicit error path instead of leaking into Discord client construction.
• DiscordClientOptions.intents and DEFAULT_INTENTS are now readonly GatewayIntentBits[], which lets main.ts pass config.privilegedIntents directly without a defensive spread.
• New operator documentation lives in packages/gateway/AGENTS.md under ## Configuration knobs.

Verification

• Gateway tests: 101 → 126 (+25)
• Lint and type-check clean
• Two follow-up todos are tracked locally:
  • Add a regression test proving main.ts threads config.privilegedIntents into createDiscordClient.
  • Plumb DISCORD_PRIVILEGED_INTENTS_FILE through deploy/compose.yaml with bind.create_host_path: false, matching the other secret-file mounts.

[fro-bot]

Verdict: CONDITIONAL

The security posture flip is correct and well-motivated. The config parsing, allowlist guard, and duplicate-dedup logic are all sound. The GuildPresences test confirms the allowlist isn't just blocking non-privileged intents accidentally. Two issues need to be addressed before merge.

Blocking issues

1. main.ts passes only privilegedIntents — the merge semantics are asymmetric and the behavior gap is untested.

createDiscordClient({intents: config.privilegedIntents, logger}) passes only the privileged intents array. Inside createDiscordClient, when options.intents is defined (including an empty array [] from an unset env var), the code branches into the Set-merge path: [...new Set([...DEFAULT_INTENTS, ...options.intents])]. This is actually fine — the defaults are always included — but the behavior when privilegedIntents is [] is: options.intents is [], which is not undefined, so the Set branch runs and produces [Guilds, GuildMessages] — the correct baseline. However, the PR description explicitly calls this out as an untracked follow-up todo:

Add a regression test proving main.ts threads config.privilegedIntents into createDiscordClient.

Shipping a known-untested wiring path in a security-posture change is a gap. The empty-array case (privilegedIntents === []) hitting the non-undefined branch rather than the options.intents === undefined short-circuit is a subtle correctness dependency — if createDiscordClient is ever refactored to treat [] as "no intents" rather than "use defaults," main.ts silently breaks. A focused integration test (even a unit-level spy on createDiscordClient) should be included in this PR, not deferred.

2. validateTokenIsFake regex is anchored wrong — test prefix allows testing123 but also test-my-production-bot.

const FAKE_TOKEN_PATTERN = /^(?:test-token-fake|fake|test|MOCK)/i

The pattern matches any string starting with test (case-insensitive), which includes strings like testnet-mainnet-token-abc123 or testing.env.value. The PR description says the intent is to match "a string starting with… test" — but this is case-insensitive, so TEST, Test, etc. also pass. The guard's purpose is to prevent real tokens from slipping through, not to reject valid fake values. Given that real Discord bot tokens are base64 blobs starting with a numeric user-ID segment (e.g. MTIzNDU2.), the regex is permissive in a direction that weakens the guard. A real token will not start with test, so this doesn't cause a practical bypass — but the pattern is looser than stated. The test suite doesn't cover testing123 or test-my-real-token. Consider anchoring more tightly or adding a word-boundary/delimiter, e.g. /^(?:test-token-fake|fake-token|mock-token|test)(?:[^a-z]|$)/i.

Non-blocking concerns

• The expectClientIntents helper in client.test.ts uses a double unknown cast to reach the BitField constructor. This is brittle against discord.js internal API changes. A comment noting why this approach was chosen (and that it's test-only) would help the next reader. Not a bug, just maintenance risk.

• runtime-effect.test.ts: removing the // eslint-disable-next-line vitest/expect-expect comments is fine given the eslint config update, but the ESLint rule change (assertFunctionNames: ['expect', 'expect*', 'assert*']) is a project-wide change bundled into a gateway-scoped PR. This is low risk but worth noting for reviewers of other packages.

Missing tests

• Integration/wiring test: main.ts passing config.privilegedIntents through to createDiscordClient. The PR itself acknowledges this is missing.
• validateTokenIsFake with testing123 and other test-prefixed-but-realistic-looking strings, to document the intended acceptance boundary.

Risk assessment: MED

The default-intent narrowing is a breaking change for existing deployments that rely on MessageContent or GuildMembers without setting the new env var — bots will silently stop receiving message content and member events on next deploy. The migration note is clear and correct. The config-parse logic and allowlist guard are solid. The untested main.ts wiring and the slightly-too-permissive token guard regex are the residual risk.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	26144344550
Cache	hit
Session	ses_1bc093282ffe8c0a0nqWZ6dkW2

[fro-bot]

Verdict: PASS

Both blocking issues from the previous review pass are resolved in commit a5b9b1b.

Blocking issues

None.

Resolution of prior blocking issues:

1. main.ts wiring now tested. makeDiscordClientFromConfig was extracted as a named, exported helper and covered by three focused tests in main.test.ts — empty intents, MessageContent-only, and both-privileged. The vi.mock approach is appropriate given the Effect-based top-level program. The wiring contract is now explicit and regression-protected.

2. Token guard regex tightened correctly. The new pattern /^(?:test-token-fake|fake-token|mock-token|test)(?:[^a-z0-9]|$)/i closes the over-permissive test* prefix. Bare fake and MOCK now correctly throw; testing123 throws; test- variants pass. The updated test suite documents both the accept and reject edges including the intentional design choice that test-my-real-bot is accepted (operators using a test- prefix are considered to be explicitly opting in to a fake-token label). That tradeoff is reasonable and clearly documented in the test comment.

Non-blocking concerns

• The expectClientIntents double-cast in client.test.ts is still present and still somewhat brittle against discord.js internals, but it's test-only and unchanged from the prior pass — carrying it forward as acknowledged maintenance risk is acceptable.

Missing tests

None. The previously missing tests have been added.

Risk assessment: LOW

The security posture change is correct, well-tested (+141 additions from the original +637 to this pass's +778 net), and the migration path is clearly documented. The operator blast radius is bounded to deployments that don't set DISCORD_PRIVILEGED_INTENTS on next deploy — and those deployments will see connection stay up with the narrowed non-privileged baseline rather than a crash. Risk is LOW for this revision.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	26145938999
Cache	hit
Session	ses_1bc093282ffe8c0a0nqWZ6dkW2