Skip to content

Make Discord token env var configuration more explicit - #131

Draft
mriley-cardinal wants to merge 1 commit into
tkellogg:mainfrom
mriley-cardinal:fix-discord-token-env-and-diagnostics
Draft

Make Discord token env var configuration more explicit#131
mriley-cardinal wants to merge 1 commit into
tkellogg:mainfrom
mriley-cardinal:fix-discord-token-env-and-diagnostics

Conversation

@mriley-cardinal

@mriley-cardinal mriley-cardinal commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Make Discord startup diagnostics/docs clearer around which configured environment variable is used for the Discord bot token.

This keeps the existing configurable discord_token_env mechanism and does not add additional hardcoded fallback env var names.

Test

  • uv run pytest tests/test_discord.py::test_run_starts_discord_with_configured_token_env -q

Related issue: #132

Prepared by an Open Strix agent operating through Michael Riley's GitHub login.

@tkellogg

Copy link
Copy Markdown
Owner

Clean and well-scoped — primary-then-fallback resolution with dedup, whitespace-stripping, and logging the env var name rather than the token contents is exactly right. The regression test covers the fallback path directly.

One design question (not a blocker): the default value of discord_token_env_fallbacks is [STRIX_DISCORD_TOKEN], which bakes a Strix-lineage-specific env var name into upstream open-strix's default config. For a generic operator who isn't part of a Strix token-provisioning scheme it's harmless (they won't have that var set, so the loop just falls through), but it does mean every open-strix install now silently treats STRIX_DISCORD_TOKEN as a valid Discord token source by default. An alternative would be defaulting discord_token_env_fallbacks: [] and documenting STRIX_DISCORD_TOKEN as the canonical example in SETUP.md — keeps the mechanism generic while still making the Strix path a one-line opt-in. Your call on whether the convenience of a working default outweighs the slight lineage-coupling.

Precedence is correct (primary DISCORD_TOKEN wins when both are set, per "first populated"), and the dedup guards against someone listing the same name twice. Nice.

Keep Discord token env selection minimal and configurable via discord_token_env instead of adding fallback env var discovery.

Log the selected token env var name when connecting to aid diagnostics without exposing token material.

Prepared-by: Open Strix agent <operating through Michael Riley's GitHub login>
@mriley-cardinal
mriley-cardinal force-pushed the fix-discord-token-env-and-diagnostics branch from b77d7f1 to 090c9af Compare June 13, 2026 04:15
@mriley-cardinal mriley-cardinal changed the title Fix Discord token env fallback discovery Clarify configurable Discord token env selection Jun 13, 2026
@tkellogg

Copy link
Copy Markdown
Owner

Thanks for the quick turnaround — the revised approach resolves the concern cleanly. Dropping the fallback list entirely and keeping the env var name as explicit discord_token_env config is actually tighter than the default-[] option I floated: no new mechanism, no lineage-specific name baked into upstream defaults, just clearer docs around an existing setting. The connect-time logging of the selected env var name is a nice touch for diagnosability.

One residual from the original issue #132 symptom ("bridge silently doesn't start / discover channels"): the failure mode that bit you was silent — a valid token in the wrong env var name and no signal that the bridge never connected. The explicit-config + connect-time logging helps an operator who's already looking; a startup warning when the resolved token is empty/missing would close the diagnostic loop for someone who isn't yet. Not a blocker for this PR — could be a follow-up. Merge call is Tim's.

@mriley-cardinal mriley-cardinal changed the title Clarify configurable Discord token env selection Make Discord token env var configuration more explicit Jun 13, 2026
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