feat(cloud): add reflex cloud whoami and reflex cloud token - #6918
feat(cloud): add reflex cloud whoami and reflex cloud token#6918Kastier1 wants to merge 8 commits into
reflex cloud whoami and reflex cloud token#6918Conversation
Inspecting which Reflex Cloud credentials a machine is actually using required reading `hosting_v1.json` by hand. Two commands make it first-class: - `reflex cloud whoami` resolves the token with the control plane and reports the account, org, tier and where the token was loaded from. It never starts a browser login and never prints the token. - `reflex cloud token --print/--set/--clear` reads, replaces or removes the stored token. `--set` validates before saving, so a bad token cannot silently replace a working one. `get_existing_access_token_with_source` exposes the existing config-file over-`REFLEX_ACCESS_TOKEN` precedence so both commands can report which source won, with `get_existing_access_token` delegating to it. Also isolates the hosting config in tests: `test_save_token_to_config` and `test_authenticated_token_found_but_invalid` wrote to and emptied the developer's real token file, logging them out on every test run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Greptile SummaryThe PR adds credential inspection and management commands, changes token precedence to favor the environment, and hardens hosting-config writes.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported destructive-write, clear-verification, legacy-cleanup, and environment-masked write-verification issues are addressed at the current head.
|
| Filename | Overview |
|---|---|
| packages/reflex-hosting-cli/src/reflex_cli/utils/hosting.py | Adds source-aware token lookup and atomic configuration helpers that preserve the previous file when serialization or replacement fails. |
| packages/reflex-hosting-cli/src/reflex_cli/v2/auth.py | Implements whoami and token-management commands, including config-only persistence verification that fixes the environment-precedence false failure. |
| packages/reflex-hosting-cli/src/reflex_cli/v2/deployments.py | Registers the two new authentication commands under the cloud CLI. |
| tests/units/reflex_cli/conftest.py | Redirects hosting configuration paths to temporary directories so tests cannot alter developer credentials. |
| tests/units/reflex_cli/utils/test_hosting.py | Covers token precedence, atomic-write preservation, malformed configurations, and best-effort legacy cleanup. |
| tests/units/reflex_cli/v2/test_auth.py | Exercises command output, validation, persistence verification, environment precedence, clearing outcomes, and secret-safe printing. |
Reviews (6): Last reviewed commit: "fix(cloud): verify `token --set` against..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
All reported issues were addressed across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Review found `save_token_to_config` and `delete_token_from_config` open the config with mode "w", which truncates before the write is attempted. A failing write (disk full, I/O error) therefore destroyed the existing token and project, and both helpers swallow the exception, so the caller saw only a warning. `token --set` read back and reported the failure, but only after the damage was done. Both now serialize to a temporary file alongside the target and move it into place, so a failed write leaves the previous credentials untouched and the temporary file is cleaned up. `token --clear` now reads back too: `delete_token_from_config` swallows filesystem errors, so success was previously reported without evidence. `token --print` forces the log level to ERROR while resolving. The shared console writes everything below ERROR to stdout, so `$(reflex cloud token --print --loglevel debug)` captured two Debug lines along with the token. Errors go to stderr, so stdout now carries the token or nothing. The config-file tests were rewritten against the real (now isolated) filesystem instead of asserting mock call counts, which pinned the old non-atomic write sequence, and cover the failed-write paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…g it
Follow-up review caught a regression from the atomic-write change:
`_read_hosting_config` swallowed read errors and returned `{}`, so
`delete_token_from_config` replaced a malformed config with `{}`,
destroying the token and project it could not parse. On main the read and
the write shared one try block, so a parse failure aborted before the
write. Confirmed against a main worktree: main leaves the malformed file
untouched, this branch emptied it.
`_read_hosting_config` now returns `{}` only for a missing file and
propagates read and parse errors. `delete_token_from_config` lets those
reach its existing handler, so an unreadable config is left alone.
`save_token_to_config` keeps its previous fallback of starting from an
empty config, so a corrupt file cannot block re-authenticating.
`token --clear` verified removal through a lookup that treats an
unreadable config as "no token", so it could report success while the
token was still on disk. It now reads the config directly through
`stored_access_token`, which distinguishes absent from unparseable, and
fails on either kind of unconfirmed removal.
The `--print` stdout test mocked the very helper whose debug records
contaminate stdout, so it passed with or without the fix. It now writes a
real config and exercises the lookup; verified it fails without the fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Two paths could escape the command's error handling as tracebacks. `delete_token_from_config` removed the legacy `hosting_v0.json` outside its try block, so an unlink failure propagated out and aborted `--clear` before the readback ran. The cleanup is now best-effort like the rest of the function, and uses `missing_ok=True` to close the exists/unlink race. The legacy file holds no token the CLI reads, so failing to remove it must not fail the removal that already succeeded. `stored_access_token` indexed whatever `json.load` returned, so a config holding valid JSON that is not an object raised `AttributeError` — not one of the types `--clear` catches. `_read_hosting_config` now rejects non-object JSON as a `ValueError`, which every caller already handles, making its dict return type honest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
masenf
left a comment
There was a problem hiding this comment.
select_project still truncates the config — the changelog's headline claim is false. hosting.py:1683-1689 is a third writer of hosting_v1.json and still uses open("w") + json.dump.
Every failure is reported as "rejected", including network failures. auth.py:82-85 and 165-168.
And a HTTP 401 reports as rejected: server error, because raise_for_status() results in TokenValidationError("server error"); only a JSON parse failure reaches TokenAccessDeniedError. So the command built to answer "is my token good?" says "server error" when the token is revoked, and "rejected" when the network is down.
--set is worse: it refuses to save on an unreachable control plane and tells you the token was rejected; although maybe that's okay, we don't necessarily want to set a token that was unverified.
Review feedback from @masenf, all confirmed against the code: `--set TOKEN` put a live credential in shell history and in the process list. It now accepts `-`, or a bare `--set`, to read the token from stdin, prompting without echo when stdin is a terminal. `--set ""` was reported as "specify exactly one of --print, --set or --clear (got none)", which reads as though --set had not been passed. The operations are now counted with `is None`, and an empty token is rejected on its own terms. `whoami` printed through the shared console, which applies rich markup and wraps to the terminal width. At 40 columns `print_table` truncated `1532f93f-41b6-4a78-893d-a…`, and the identifiers it hands back are the whole point of the command; `--json` was wrapped across lines, which happens to still parse but breaks anything reading a line at a time. Both paths now write directly. `get_existing_access_token_with_source` had its own copy of the config read; it uses `stored_access_token` now. Config reads and writes pin utf-8 rather than inheriting a platform default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@masenf pointed out the precedence is backwards. Exporting REFLEX_ACCESS_TOKEN to run a script against a specific account did nothing on a machine that had ever run `reflex login`, because the stored token won and the environment variable was consulted only when no token was stored. It failed silently, with no way to tell which credential was in use — the failure mode that motivated this PR. Exporting the variable is an explicit choice scoped to one invocation; the config file is ambient state left behind by an earlier login. This changes behavior only when both are present and differ, and `reflex cloud whoami` now reports which source won. Kept as its own commit so it can be dropped if this should ship separately from the new commands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@masenf thank you — this was a genuinely excellent review. Eight comments, eight real defects, and two of them (the credential on the command line, and the token precedence) were things I had looked straight at and reasoned about incorrectly rather than simply missed. The precedence one is the bug that motivated the whole PR; I had written it up as a footgun to document instead of a defect to fix. All eight are addressed across two commits, kept separate on purpose:
Two things I did not do, both noted in the threads: the general 382 tests in |
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The precedence flip broke `--set` for anyone with REFLEX_ACCESS_TOKEN exported. The write-verification read went through `get_existing_access_token_with_source`, which now returns the environment token first, so the guard saw `TokenSource.ENVIRONMENT` and reported "Unable to persist" and exit 1 — while the token had in fact been written to hosting_v1.json. Verification now uses `stored_access_token`, which reads the config alone and cannot be shadowed by the environment, matching how `--clear` already confirms removal. An unreadable config is reported separately from a token that failed to land. Caught in review by greptile and cubic; reproduced with the environment variable set, where the config held the new token and the command still exited 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Why
Answering "which credentials is this machine actually using?" meant reading
hosting_v1.jsonby hand. That came up chasing a customer whose token appeared not to update, and there was no supported way to confirm or disprove it.What
reflex cloud whoami— resolves the token against the control plane and reports the identity, plus which source the token came from:get_authenticated_client), so it is safe in CI and answers "am I logged in?" without changing the answer.token_fingerprintis a truncated sha256, so two machines can be compared in a support thread without anyone pasting a secret.--jsonfor scripting,--tokento inspect a specific token.reflex cloud token --print / --set TOKEN / --clear— exactly one required.--printwrites the raw token to stdout viaclick.echo, deliberately bypassingconsole.print, which wraps at 80 columns when piped and would corrupt a long token inexport REFLEX_ACCESS_TOKEN=$(reflex cloud token --print). The log level is forced to ERROR for this path, because the shared console writes everything below ERROR to stdout; stdout now carries the token or nothing.--settakes the token on stdin (-, a bare--set, or a hidden prompt on a tty) so live credentials stay out of shell history and the process list;--set TOKENstill works. It validates before saving, then reads back to confirm the write —save_token_to_configswallows write errors, so success was previously unverifiable. A rejected token exits non-zero and leaves the existing one untouched.--clearremoves the stored token and confirms removal, distinguishing three outcomes: gone (success), still present (exit 1), config unreadable (exit 1). It notes whenREFLEX_ACCESS_TOKENis still set and will now take over.get_existing_access_token_with_sourcereports which source a token came from, so both commands can show it. Its precedence is nowREFLEX_ACCESS_TOKENfirst, then the config file — flipped in its own commit, see below.get_existing_access_tokendelegates to it — no behavior change.stored_access_tokenreads the config directly, ignoring the environment and propagating read errors, so callers can tell "no token stored" from "cannot tell what is stored".Bug found in review: config writes were destructive
save_token_to_configanddelete_token_from_configopen the config with mode"w", which truncates on open, beforejson.dumpruns. Any write failure therefore destroyed the stored credentials — and since neither helper reports failure to the caller, it surfaced only as a log line. Reproduced by patchingjson.dumpto raise:Both now serialize to a temporary file alongside the target and move it into place with
Path.replace, closing the handle first so Windows can rename it and cleaning up the temp file on failure. A failed write leaves the previous credentials byte-identical. This also fixesreflex loginandreflex logout, which share these helpers.A follow-up review round caught a regression in that first fix: routing reads through a helper that returned
{}on failure turned "cannot read" into "is empty", sodelete_token_from_configreplaced a malformed config with{}. Verified against amainworktree — main left the file untouched. Reads now propagate errors,deleteleaves an unreadable config alone, andsavekeeps its fallback of starting fresh so a corrupt config cannot block re-authenticating.Incidental fix: tests were destroying the developer's login
Not cosmetic, and the reason this PR touches
conftest.py:test_save_token_to_configmocksPath.existsandPath.mkdirbut notPath.open, so it overwrote the realhosting_v1.jsonwith{"access_token": "test_token"}.test_authenticated_token_found_but_invalidcalls the realdelete_token_from_config, emptying it to{}.Verified with a sentinel: before,
pytest tests/units/reflex_clireduced the file to{}; after, it is untouched. An autouse fixture now pointsReflex.DIRand bothHOSTING_JSONpaths at a tmp dir, so no reflex_cli test can reach the real file.With that isolation in place, the config-file tests were rewritten to assert real on-disk contents instead of mock call counts. The old assertions (
mocked_open.call_count == 2) pinned the exact non-atomic write sequence, so they would have blocked the fix above while proving nothing about the resulting file.Testing
tests/units/reflex_cli/v2/test_auth.py;reflex_cli/v2/auth.pyat 100% coverage. Includes: a bad--setneither saves nor deletes, a failed write preserves the previous token and leaks no temp file, an unreadable config is preserved rather than replaced,--printround-trips a 300-char token verbatim and stays clean under--loglevel debug,--cleardistinguishes all three removal outcomes,whoaminever opens a browser, and the token never appears in either output mode.--printstdout test exercises the real lookup rather than mocking it, because the contaminating debug records originate inside the helper. Confirmed it fails when the fix is reverted.ruff check/formatclean.pyright reflex testsunchanged at 5 pre-existing errors (recharts/lucide stubs, unrelated).whoami,--printunder--loglevel debug,--clear, and the usage error.Docs need no change —
docs/.../cloud_cliref.pygenerates the CLI reference from the click tree, so both commands appear automatically. Command docstrings carry noArgs:/Raises:sections, matching every other command in the package, because a click docstring is its--helptext.Deliberate choices worth a reviewer's attention
hosting.validate_tokendirectly rather thanvalidate_token_with_retries, which deletes the cached token on access-denied — a bad side effect for--setin particular. The trade-off is no retry on a transient failure.--sethas no offline escape hatch; if validation cannot reach the control plane, nothing is saved. Strict on purpose, since silently storing an unusable token is the failure mode this PR exists to expose.--tokenandREFLEX_ACCESS_TOKENremain available meanwhile.--printsuppresses diagnostics rather than routing them to stderr. The stdout-for-non-errors split is a global convention of the shared console; rerouting it belongs in its own change.reflex cloud whoami --loglevel debuggives the lookup diagnostics.saveanddeletedeliberately disagree about an unreadable config:savestarts fresh so login always works,deleterefuses to touch what it cannot read. Both are tested as policies.Behavior change: token precedence (own commit, droppable)
REFLEX_ACCESS_TOKENnow beats the token stored byreflex login, rather than the reverse. Raised by @masenf, and it is the defect behind the customer report that motivated this PR: exporting the variable for a scripted deploy did nothing on any machine that had ever logged in, silently. Exporting is an explicit per-invocation choice; the config file is ambient state. Matchesgh/aws/docker.Only changes behavior when both are present and differ. Carried as commit
27b71f7b2with abreakingnews fragment so it can be dropped and shipped separately.Follow-ups, not in this PR
server error, notaccess denied:validate_tokenmaps a 401 (raise_for_status→HTTPStatusError⊂httpx.HTTPError) toTokenValidationError, and only a JSON parse failure reachesTokenAccessDeniedError. Sincevalidate_token_with_retriesonly clears the cached token onValueError, a revoked token is never evicted fromhosting_v1.json— and keeps shadowing a validREFLEX_ACCESS_TOKENindefinitely.print_tabletruncates values across the hosting CLI (1532f93f-41b6-4a78-893d-a…), so humans are pushed to--jsonto get a value they can paste back into the CLI.whoamisidesteps it by not usingprint_table; the general fix wants its own design decision.reflex-hosting-cliignoresREFLEX_DIR, which relocates the framework's data dir, so the token file diverges from the rest of reflex state. Fixing it moves an existing file and needs a read-fallback migration.🤖 Generated with Claude Code