fix(cli): stop sanity logout from sending env tokens to the session endpoint#1584
Conversation
|
bugbot run |
📦 Bundle Stats —
|
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 2.2 KB | - |
| Internal (gzip) | 838 B | - |
| Bundled (raw) | 11.20 MB | - |
| Bundled (gzip) | 2.11 MB | - |
| Import time | 858ms | -6ms, -0.6% |
bin:sanity
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 782 B | - |
| Internal (gzip) | 423 B | - |
| Bundled (raw) | 9.90 MB | - |
| Bundled (gzip) | 1.78 MB | - |
| Import time | 2.07s | -8ms, -0.4% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1d)
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 114.1 KB | - |
| Internal (gzip) | 29.2 KB | - |
| Bundled (raw) | 21.76 MB | - |
| Bundled (gzip) | 3.46 MB | - |
| Import time | 768ms | +2ms, +0.2% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-build
Compared against stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1d)
@sanity/cli-build/_internal/build
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 113.8 KB | - |
| Internal (gzip) | 28.7 KB | - |
| Bundled (raw) | 17.76 MB | - |
| Bundled (gzip) | 3.56 MB | - |
| Import time | 1.11s | +6ms, +0.5% |
@sanity/cli-build/_internal/env
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 1.8 KB | - |
| Internal (gzip) | 644 B | - |
| Bundled (raw) | 1.31 MB | - |
| Bundled (gzip) | 333.8 KB | - |
| Import time | 123ms | +1ms, +0.8% |
@sanity/cli-build/_internal/extract
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 8.6 KB | - |
| Internal (gzip) | 2.7 KB | - |
| Bundled (raw) | 155.0 KB | - |
| Bundled (gzip) | 39.5 KB | - |
| Import time | 241ms | +2ms, +1.0% |
🗺️ ./_internal/env · ./_internal/extract · @sanity/cli-build:./_internal/build treemap too large to embed · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — create-sanity
Compared against stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1d)
| Metric | Value | vs stack/mint-and-claim/a1/06-ambient-reminder (f15a4f1) |
|---|---|---|
| Internal (raw) | 908 B | - |
| Internal (gzip) | 483 B | - |
| Bundled (raw) | 931 B | - |
| Bundled (gzip) | 491 B | - |
| Import time | ❌ ChildProcess denied: node | - |
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Coverage Delta
Comparing 12 changed files against main @ Overall Coverage
|
89fceb8 to
0c76d83
Compare
08d64c6 to
087dfc2
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 087dfc2. Configure here.
aae0c9c to
16b3d9a
Compare
92acb0c to
294038e
Compare
88b6a10 to
8c23560
Compare
840cb6e to
d2c413c
Compare
c92f162 to
563ef34
Compare
234e0c2 to
eeae721
Compare
e2e0180 to
97b4f4d
Compare
1c52e0c to
40d1b6c
Compare
09a38ca to
a5d9ee8
Compare
5242789 to
d86c767
Compare
7fccfad to
076ca58
Compare
c8e4942 to
0b63f3b
Compare
4a455df to
43146bd
Compare
d037f67 to
d77c564
Compare
0147139 to
07cd654
Compare
4b13919 to
fb712c1
Compare
f9f492c to
68fdc80
Compare
ea3e9f3 to
c856974
Compare
841af5d to
890f1e0
Compare
bc9be57 to
14cc694
Compare
bb55ef1 to
508af35
Compare
141de79 to
818801c
Compare
d32bf8a to
0e2e34f
Compare
5e024e7 to
9c79ff8
Compare
27e3879 to
50f78be
Compare
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f8100e5. Configure here.
…ndpoint An env-sourced SANITY_AUTH_TOKEN (often a robot token from `sanity new`'s .env) is not a login session: logging out cannot end it, and the session endpoint rejects robot tokens with a raw internal error. Logout now names the variable and instructs removal, targets only the stored login session, and surfaces API failures by status alone — response bodies can name internal services. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a1d8d81 to
f15a4f1
Compare
f8100e5 to
0c44527
Compare
jwoods02
left a comment
jwoods02
left a comment
There was a problem hiding this comment.
Reviewed the whole stack Line comments are spread across the individual PRs, but leaving some general feedback here
The minted project config ledger lifecycle is solid with lots of edge cases though out (not clobbering .env, announcing on other projects if user forgets about unclaimed project concurrent process protection etc)
Two general comments:
-
The two reminder mechanisms (tiered waterfall, machine-global, max 4 fires vs ambient line, directory-scoped, every command) confused me until I read runClaimNudges closely. The interplay (one slot per invocation, waterfall wins) only exists in a docstring. Worth a short written comment or little refactor maybe?
-
Messaging consistency. We state the same facts differently across five surfaces: mint outro, waterfall nudges, ambient line, farewells, init banner. I've left copy suggestions on the individual PRs but happy to do a holistic wording pass offline
-
Fable over comments in my opinion and a lot of the comments are more useful inside the context of your coding sessions than in the PR's IMO. Nit and happy to be overruled but I'd go through a comment cleanup pass, some can be deleted, some only need to be 1 liners.
Also worth knowing: these PR titles become the public changelog verbatim so maybe merge the PR's into 1, fix the changeset before merge into main? (I'm not a big stacked diff guy so unsure if there's cleaner ways here)
| const envToken = process.env.SANITY_AUTH_TOKEN?.trim() | ||
| if (envToken) { | ||
| this.warn( | ||
| 'SANITY_AUTH_TOKEN is set in the environment (often via ./.env) — logging out cannot end it. Remove that variable to stop acting as its identity.', |
There was a problem hiding this comment.
Can we improve this error message a bit?
|
all feedback addressed on #1592, canceling this PR to consolidate after offline sync |
2 participants
Description
this PR quashes a pre-existing edge case:
sanity logoutwas sending mint-and-claim robot tokens to populus session endpoints, which yielded raw, internal service errors. to address that, we're providing human/agent messaging on handling logout/login, while preserving non-401 api errorsStack
(each PR reviews against its base; merge bottom-up only):
stack/mint-and-claim/a1/01-terminal-presentationstack/mint-and-claim/a1/02-mint-corestack/mint-and-claim/a1/03-claim-remindersstack/mint-and-claim/a1/04-init-signpoststack/mint-and-claim/a1/05-remint-guardrailstack/mint-and-claim/a1/06-ambient-reminderstack/mint-and-claim/a1/07-logout-env-token← this diffWhat to review
commands/logout.ts: the env-token short-circuit and error sanitization.Testing
Layer-scoped unit tests included (test:source ≥ 1:1); every layer validated standalone (full typecheck + targeted tests at each checkout).
🤖 Generated with Claude Code
Note
Medium Risk
Touches CLI authentication and logout API behavior; changes are narrowly scoped with clear user messaging and tests, but mishandling tokens could affect session invalidation.
Overview
sanity logoutno longer usesgetCliToken(), which could preferSANITY_AUTH_TOKENand send robot/env credentials to/auth/logout. It now reads the stored session viagetCliUserConfig('authToken')and passes that token explicitly intologout().When
SANITY_AUTH_TOKENis set, the command warns that logout cannot end that identity and tells the user to remove the variable. With only an env token and no stored session, it skips the API and avoids the misleading “No login credentials” message. If both env and session exist, it still ends the stored session after the warning.Non-401 logout failures now report only HTTP status (not response bodies that may leak internal service names) and leave local credentials so the user can retry. Tests cover env-only, env+session, and error sanitization, and clear
SANITY_AUTH_TOKENin the suite so shell tokens do not affect other cases.Reviewed by Cursor Bugbot for commit a0bad1e. Bugbot is set up for automated code reviews on this repo. Configure here.