fix: REPL help and choice-option crashes under Click-vendoring Typer (>=0.25)#443
fix: REPL help and choice-option crashes under Click-vendoring Typer (>=0.25)#443soustruh wants to merge 6 commits into
Conversation
uv lock --upgrade -> Typer 0.26.7, Click 8.4.1, etc. fastapi is held at <0.137 in the [server] extra: with 0.137, serve --ui stops requiring a token on protected endpoints (/doctor, /version, /changelog, /agents reachable unauthenticated), reopening GHSA-ffpq-prmh-3gx2.
typer.main.get_command returns a TyperGroup; Typer >=0.25 vendors its own Click, so it is not a standalone click.Group subclass and isinstance(_, click.Group) collapsed the REPL command tree to empty (help and tab-completion showed only help/exit). Replace with a structural _is_group() TypeGuard at the three vendored-Click isinstance sites (repl, check_command_sync, test_permissions); stop silently swallowing the tree-build error.
The --mode/--poll-strategy (job run), --role/--default-role (project invite), --role (member-set-role) and --role-hint (dev-portal identity) options passed a standalone click.Choice into Typer. Under a Click-vendoring Typer (>=0.25) the BadParameter it raises is a different class than the one Typer's parser catches, so an invalid value escaped as an uncaught traceback instead of a clean exit-2 usage error. Replace the click.Choice options with StrEnum types so Typer validates with its own Click. Valid values and --help are unchanged.
soustruh
force-pushed
the
fix/typer-vendored-click
branch
from
ddd4632 to
328ae14
Compare
Two gaps the fixes left untested: - REPL `help` command output (not just the underlying tree): assert it lists command groups, guarding the empty-list regression at the command level. - invalid choice value -> clean exit 2 (no uncaught traceback) across all seven choice options (job run --mode/--poll-strategy, project invite --role/--default-role, member-set-role --role, dev-portal identity add/edit --role-hint).
padak
left a comment
padak
left a comment
There was a problem hiding this comment.
Review summary
Clean, well-tested, root-cause fix (not a workaround). All three changes trace to a single cause — Typer >=0.25 vendors its own Click (typer._click), so isinstance(x, click.Group) / standalone click.Choice objects misbehave against the vendored copy. Bundling them is the right call. LGTM after the uv.lock rebase below. (Comment only — not approving/requesting changes.)
What I verified
| Area | Result |
|---|---|
Dropping import click from job.py / project.py / dev_portal.py |
✅ Safe — the only click references in those files were the click.Choice calls this PR removes. |
StrEnum vs. the service/client validation (mode not in VALID_JOB_MODES, poll_strategy not in VALID_POLL_STRATEGIES, client.py:2622) |
✅ No regression — a StrEnum member behaves identically to a plain str under in frozenset[str], ==, json.dumps, and f-strings. The defense-in-depth checks in job_service.py / client.py keep working unchanged. |
_is_group() TypeGuard (duck-typing via list_commands) |
✅ Correct fix for the vendoring; TypeGuard gives ty proper narrowing. |
Drift guards in test_choice_enums.py |
✅ ProjectRole is correctly order-sensitive (the --role help text renders `" |
Bare except: pass → stderr log in _build_command_tree |
✅ Removes the silent-degrade-to-empty-tree path that hid the original bug. |
fastapi<0.137 cap |
✅ Legitimate — 0.137 reopens GHSA-ffpq-prmh-3gx2 (unauthenticated /doctor, /version, /changelog, /agents under serve --ui). |
| Version sync (marketplace.json + plugin.json + pyproject.toml) + changelog | ✅ All on 0.63.4; all three fixes documented. |
Nice touch keeping the default as typer.Option(JobMode(DEFAULT_JOB_MODE), ...) rather than JobMode.run — the default stays bound to the constant (single source of truth), and a drift between the constant and the enum would fail fast at import time, on top of the drift test.
Findings (all non-blocking)
🟡 Rebase needed — uv.lock conflict. PR is currently CONFLICTING. The only real conflict is uv.lock (main bumped cryptography in 0f9d184); pyproject.toml merges cleanly. A rebase + uv lock regen should clear it.
🟡 fastapi<0.137 cap has no tracking issue. The cap blocks future security updates to fastapi. The PR notes the follow-up ("serve --ui route-aware auth check needs updating"), but without an issue this risks becoming a silent long-term pin. Suggest filing one so the cap gets lifted deliberately rather than forgotten.
🟢 REPL direction (out of scope for this PR). Roughly half of this change invests in the REPL (help/tab-completion fix + two new REPL tests). There was an internal question on whether the REPL stays long-term. Flagging only so the decision is made deliberately — the choice-option fix and the fastapi cap are independently valuable and share the same root cause regardless of what happens to the REPL, so this does not affect mergeability here.
Verdict
Real user-facing bug (anyone installing via uv tool install / the curl script gets Typer 0.26.7 and hits both crashes; CI never saw it because the lockfile pinned 0.24.1). Mergeable after the uv.lock rebase.
…p cryptography 49.0.0)
…raction typer 0.26 CliRunner.invoke returns typer.testing.Result (0.24 reused click.testing.Result), so test helpers annotated `-> Result` imported from click.testing failed `ty check`. Import Result from typer.testing instead. Also cast benchmark.py stdio_total to float so the subtraction is numeric. Surfaced by whole-tree `ty check` (make typecheck) after the 0.63.4 deps bump -- the earlier per-file ty runs missed them.
|
The |
2 participants
Why
Typer >=0.25 vendors its own copy of Click (
typer._click), which breaks code thathands standalone-
clickobjects to Typer or checks against standaloneclicktypes.Users installing via
uv tool install/ the curl script get the latest Typer (0.26.7);the lockfile pinned 0.24.1, so
uv sync/ CI never hit it.helpand tab-completion showed onlyhelp/exit:_build_command_treegated its walk with
isinstance(x, click.Group), False for a vendoredTyperGroup,so the tree built empty (hidden by a bare
except). Same bug also inscripts/check_command_sync.pyandtests/test_permissions.py.click.Choicevalues (job run --mode/--poll-strategy,project invite --role/--default-role,project member-set-role --role,dev-portal identity --role-hint) raised an uncaught traceback instead of exit 2 — standaloneclick.BadParameterisn't caught by Typer's vendored-Click handler.What changed
repl.py(+check_command_sync.py,test_permissions.py): structural_is_group()TypeGuardreplacesisinstance(_, click.Group); swallowed tree-build error now logged.job.py/project.py/dev_portal.py: the sevenclick.Choiceoptions becomeStrEnumtypes; drift-guarded against the constants by
tests/test_choice_enums.py.<0.137: with 0.137,serve --uistops requiring a token on protected endpoints(
/doctor,/version,/changelog,/agentsreachable unauthenticated), reopeningGHSA-ffpq-prmh-3gx2.
Verification
Full suite (minus live-cred e2e) green on Typer 0.24.1 and 0.26.7;
ruff+tyclean.Adds a REPL
help-output test and a parametrized invalid-choice -> exit-2 test acrossall seven choice options.
Follow-ups (not in this PR)
typer[all]extra no longer exists in Typer 0.26 (harmlessuvwarning).serve --uiroute-aware auth check updated, then thecap can lift.