feat: interactive discli setup wizard - #36
Conversation
Replaces the five manual steps in README's Setup section with one interactive command: store a token that has been checked against Discord, generate an invite URL carrying the permissions you pick, choose a permission profile, name the environment variables voice providers read, and finish with doctor's own report. Two gates run before any prompt, in this order: `enforce_profile()` first. setup writes the token and sets the active profile, so it is `config set` and `permission set` fused into one command and must be full-only -- otherwise a readonly agent runs the wizard and promotes itself to full, the same hole `moderation` had with `permission set`. Denying before prompting also matches confirm_destructive(): a forbidden command that answers "needs a terminal" first has told the caller to go find a terminal for something it was never allowed to do. Then the terminal check. discli is built to be driven by agents over a pipe, so blocking on a prompt there is a hang rather than an error. `--json` or a non-tty stdin exits 1 and names the non-interactive equivalents on stderr, leaving stdout parseable. The token is validated before it is saved. Saving first leaves a broken token in config.json that every later command picks up, so the failure surfaces far from the paste that caused it. The login also reports which bot the token belongs to -- pasting a different application's token otherwise succeeds silently and quietly retargets every command. Reuses rather than reimplements: run_rest_action/enforce_profile from client.py, save_config from config.py, DEFAULT_PROFILES and set_active_profile from security.py, and _gather/_format_text/ _voice_extras_installed/DISCLI_PERMISSIONS from doctor.py. The doctor helpers are underscore-private and imported as-is, to avoid churning doctor.py and its tests for no behavior change. Nothing is persisted for voice: tts.py and stt.py read credentials from the environment only, so the step prints export/setx lines instead of writing a file discli would never consult. Notes for reviewers: - BUNDLES is pinned against doctor's DISCLI_PERMISSIONS by test, so the invite the wizard generates and the permissions doctor checks for cannot drift apart. - The terminal check sits behind a monkeypatchable `_is_interactive()` because CliRunner never supplies a tty stdin; without that seam no prompt path is testable and the guard tests pass for the wrong reason. - `_active_profile_name()` duplicates a few lines from cli.py's `permission_show`. Extracting it into security.py is the cleaner fix but widens this change; left local deliberately. Closes #35 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six real bugs, three contract violations, and four cleanups. Each fix has a test that was watched failing first. Bugs: The generated invite failed the wizard's own step 5. The moderation bundle granted manage_messages without bypass_slowmode, which is exactly the regression doctor's PERMISSION_SPLITS check reports -- so setup printed an invite URL and then, four steps later, told you to re-invite the bot. bypass_slowmode joins the bundle and doctor's DISCLI_PERMISSIONS. setup was schedulable. parse_action accepted it, and a fire would block forever on click.prompt inside the scheduler's worker thread with nobody at the keyboard -- last_run never written, entry wedged silently. It now sits in a new _NEEDS_A_HUMAN set with a reason of its own, since _NON_TERMINATING's "runs indefinitely" is the wrong explanation. An active custom profile locked the prompt. The local profile-name reader ignored the `profiles` map that get_active_profile() honours, so the default was a click.Choice value Click rejects, and with show_choices off the valid set was never shown. Nine of the sixteen new tests failed on any machine with DISCLI_PROFILE set or a restricted permissions.json: enforce_profile denies setup before the terminal check, so they failed for a reason unrelated to what they assert. An autouse fixture now neutralises both. The wizard exited 0 while printing [FAIL] lines, so `discli setup && echo ready` said ready for a broken install. It exits like doctor does. _is_interactive() checked stdin only, but click writes prompts to stdout: `discli setup > log` kept a tty on stdin, sent every question into the file, and left a terminal that looked hung -- the exact failure the guard exists to prevent. Both streams are checked. Contract violations against the README this branch added: The voice step hardcoded default="none", so re-running offered to un-configure a working setup -- against the documented promise that every step shows what is already configured. It defaults to the exported DISCLI_TTS/DISCLI_STT. The skip only existed when the extras were missing, so anyone who installed voice for `voice play` alone was marched through both provider prompts. The step is titled "(optional)"; now it is. Saving a token while DISCORD_BOT_TOKEN is set reported OK, but cli.py resolves the envvar first -- leaving the user certain they had switched bots while every command used the old one. It now says so at the point the choice is overridden, not only before the prompt. Cleanups: _invite_url delegates to discord.utils.oauth_url, which builds the same URL and tracks Discord's endpoint. _identify caps the guild lookup at ten rather than paginating every guild of a bot in hundreds to print one reassurance line. get_active_profile_name()/get_profiles() land in security.py, replacing the third copy of that read in setup and the inline copy in cli.py's permission_show. The provider export dialect is chosen by shell rather than os.name, because os.name is "nt" in Git Bash too, and the Windows branch no longer says "add to your shell profile" for setx, which does not affect the current console. The bundle/doctor sync test now asserts both directions and covers PERMISSION_SPLITS -- the gap that let the bypass_slowmode bug through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Code review found 15 issues; 14 were real. All fixed in 44bc2b1, each with a test watched failing first. Suite is now 367 passed, 1 skipped (up from 351). Bugs
Contract violations against this branch's own README
Cleanups
The bundle/doctor sync test now asserts both directions and covers Not takenThe review flagged Found, not fixed here
|
Closes #35.
Replaces the five manual steps in README §Setup with one interactive command.
Two gates before any prompt
enforce_profile()first. setup writes the token and sets the active profile, so it isconfig set+permission setfused and must be full-only. Without it areadonlyagent runs the wizard and promotes itself tofull— the same holemoderationhad withpermission set. Denying before prompting also matchesconfirm_destructive(): a forbidden command that answers "needs a terminal" first has told the caller to go find a terminal for something it was never allowed to do.Worth knowing:
--profilewidens as well as narrows, so this stops an agent handed a restricted default, not one that can pass arbitrary global flags. Same guaranteeconfig setalready has, no stronger.Then the terminal check. discli is built to be driven by agents over a pipe, so blocking on a prompt is a hang, not an error.
The refusal goes to stderr, so stdout stays parseable.
Design notes
tts.py/stt.pyread credentials from the environment only, so the step printsexport/setxlines rather than writing a file discli would never consult. Platform-aware, and the placeholder is shell-quoted (<your-key>unquoted is a redirection).run_rest_action/enforce_profile(client.py),save_config(config.py),DEFAULT_PROFILES/set_active_profile(security.py),_gather/_format_text/_voice_extras_installed/DISCLI_PERMISSIONS(doctor.py). Step 5 prints doctor's report verbatim rather than offering a second opinion.Tests
15 new tests in
tests/test_setup.py; suite is 351 passed, 1 skipped.Built test-first. Three times a test passed without proving anything, so the production code was inverted to watch it fail properly — the no-save-on-bad-token test only earned its keep once it printed
{'token': 'bad3'}, and the bundle/doctor sync test once it was fed a bogus permission.Two seams worth flagging for review:
_is_interactive()exists as a function becauseCliRunnernever supplies a tty stdin. Without that seam no prompt path is testable and the guard tests pass for the wrong reason.wizardfixture pins_voice_extras_installedandsecurity.PERMISSIONS_PATH; otherwise the tests branch on whether the developer happens to have voice installed and on their own~/.discli.setupis also added totest_only_full_may_reconfigure_discli, andBUNDLESis pinned against doctor'sDISCLI_PERMISSIONSso the invite the wizard generates and the permissions doctor checks for cannot drift apart.Reviewer judgment calls
_active_profile_name()duplicates a few lines fromcli.py'spermission_show. Extracting it intosecurity.pyis the cleaner fix but widens this change; left local deliberately. Happy to change.doctor.pyand its tests for no behavior change.docs/guides/cli-usage.mdxis untouched — it doesn't documentdoctoreither, so this follows that precedent.Out of scope
Creating the Discord application itself (no API for it), writing to shell profiles, and storing provider API keys.
Unrelated drift noticed, not fixed
CLAUDE.mddescribestts.pyas having "ElevenLabs and OpenAI implementations" — it also has Deepgram Aura (get_tts_provideraccepts all three).🤖 Generated with Claude Code