Skip to content

feat(auth): require explicit login subcommand#182

Merged
eddietejeda merged 8 commits into
mainfrom
auth-no-login-shortcut
Jun 28, 2026
Merged

feat(auth): require explicit login subcommand#182
eddietejeda merged 8 commits into
mainfrom
auth-no-login-shortcut

Conversation

@eddietejeda

Copy link
Copy Markdown
Contributor

Summary

Removes the shortcut where bare hotdata auth automatically triggered a browser login. Now it prints the auth help — listing login, register, logout, and status — consistent with how other commands (e.g. query) behave when invoked without a subcommand.

Users must now type hotdata auth login explicitly to authenticate.

Changes

  • src/main.rs: the None arm of Commands::Auth prints auth subcommand help instead of calling auth::login().
  • src/main.rs: exempted bare auth from the update gate since it no longer hits the API.
  • src/command.rs: updated the Login doc comment to drop the "same as hotdata auth with no subcommand" note.

Testing

  • cargo build — clean
  • cargo test — all 221+ tests pass
  • Verified hotdata auth prints help and no longer initiates login

…` shortcut

Bare `hotdata auth` previously triggered an automatic browser login. Now it
prints the `auth` help (listing login/register/logout/status), matching how
other commands behave when invoked with no subcommand. Users must type
`hotdata auth login` to authenticate.

Also exempt bare `auth` from the update gate since it no longer hits the API.
claude[bot]
claude Bot previously approved these changes Jun 24, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped change. Follows the existing query no-subcommand handling pattern (main.rs:203-211) exactly, and the update-gate exemption is appropriate since bare auth no longer hits the API.

Comment thread src/main.rs
Comment on lines +179 to +187
None => {
use clap::CommandFactory;
let mut cmd = Cli::command();
cmd.build();
cmd.find_subcommand_mut("auth")
.unwrap()
.print_help()
.unwrap();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior change leaves several user-facing hints that still tell users bare hotdata auth will log them in — they're now incorrect and should be updated in this PR:

  • src/sdk.rs:460-463 (blocking): "Run {} to log in, or pass --api-key." with "hotdata auth". This is the clearest break — it only references bare hotdata auth, which now just prints help. A user hitting an expired session who follows this hint will not be logged in. It should point at hotdata auth login.
  • src/sdk.rs:818: "... Run 'hotdata auth login' (or 'hotdata auth') to re-authenticate." — the (or 'hotdata auth') parenthetical is now wrong.
  • src/config.rs:267: "... Run 'hotdata auth login' (or 'hotdata auth') or specify --workspace-id." — same stale parenthetical.
  • src/config.rs:287: "Run 'hotdata auth login' (or 'hotdata auth') to generate a new config file." — same.
  • src/jwt.rs:15 (doc comment): "user must hotdata auth again" — minor, but worth updating for accuracy.

The PR's goal is consistency, so leaving these pointing at the removed shortcut is contradictory.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  • src/sdk.rs:460-463 — The session-expired hint reads Run hotdata auth to log in, or pass --api-key, but after this PR bare hotdata auth only prints help and no longer logs in. Following this hint will not authenticate the user. It must point at hotdata auth login.

Action Required

Update the user-facing strings that still reference bare hotdata auth as a login/re-auth path so they're consistent with the new behavior:

  • src/sdk.rs:460-463 — change hotdata authhotdata auth login.
  • src/sdk.rs:818 — drop or fix the (or 'hotdata auth') parenthetical.
  • src/config.rs:267 and src/config.rs:287 — drop or fix the (or 'hotdata auth') parenthetical.
  • src/jwt.rs:15 doc comment (user must hotdata auth again) — minor, update for accuracy.

The implementation of the change itself looks correct and mirrors the existing Commands::Query no-subcommand pattern; the update-gate exemption is also appropriate.

`hotdata auth profiles` writes ~/.hotdata/profiles.yml seeded with a
`default` profile pointing at the production API and app URLs:

  profiles:
    default:
      HOTDATA_API_URL: https://api.hotdata.dev/v1
      HOTDATA_APP_URL: https://app.hotdata.dev

Refuses to overwrite an existing file. Local-only, so it is exempt from
the API update gate.
Comment thread src/main.rs
.unwrap()
.print_help()
.unwrap();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (re-raised from cycle 1, still unaddressed): This PR changes bare hotdata auth to print help instead of logging in, but the user-facing hints that still tell users hotdata auth will log them in were not updated. They are now incorrect:

  • src/sdk.rs:460-463 (most severe): "Run {} to log in, or pass --api-key." with "hotdata auth".cyan(). This references only bare hotdata auth, which now just prints help — a user hitting an expired session who follows this hint will not be logged in. Should point at hotdata auth login.
  • src/sdk.rs:818: "... Run 'hotdata auth login' (or 'hotdata auth') to re-authenticate." — the (or 'hotdata auth') parenthetical is now wrong.
  • src/config.rs:288: "... Run 'hotdata auth login' (or 'hotdata auth') or specify --workspace-id." — same stale parenthetical.
  • src/config.rs:308: "Run 'hotdata auth login' (or 'hotdata auth') to generate a new config file." — same.
  • src/jwt.rs:15 (doc comment): "user must hotdata auth again" — minor, but worth updating for accuracy.

The PR's stated goal is consistency; leaving these pointing at the removed shortcut is contradictory. The sdk.rs:460-463 case is a real correctness break (the hint sends users down a dead end), so this remains blocking.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  • Stale auth hints not updated (re-raised from cycle 1). This PR makes bare hotdata auth print help instead of logging in, but several user-facing hints still tell users hotdata auth will authenticate them:
    • src/sdk.rs:460-463 — "Run hotdata auth to log in, or pass --api-key." references only the removed shortcut. A user with an expired session who follows this hint is led to a dead end (it now just prints help). This is a correctness break.
    • src/sdk.rs:818, src/config.rs:288, src/config.rs:308 — stale (or 'hotdata auth') parentheticals.
    • src/jwt.rs:15 — doc comment "user must hotdata auth again" (minor).

Action Required

Update the hints above to point at hotdata auth login, and drop the now-incorrect (or 'hotdata auth') parentheticals. The sdk.rs:460-463 case is the must-fix — it actively misdirects users with expired sessions.

The new auth profiles functionality and its tests look good.

@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/config.rs 50.00% 1 Missing ⚠️
src/main.rs 85.71% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

The profiles.yml duplicated existing behavior: api_url/app_url are already
overridable via the HOTDATA_API_URL / HOTDATA_APP_URL env vars, and the
scaffolded file restated the built-in defaults without anything reading it.
Comment thread src/main.rs
.unwrap()
.print_help()
.unwrap();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (re-raised from cycles 1 & 2, still unaddressed): This PR changes bare hotdata auth to print help instead of logging in, but the user-facing hints that still tell users hotdata auth will log them in were not updated. None of src/sdk.rs, src/config.rs, or src/jwt.rs were touched in this PR, so they remain stale and now incorrect:

  • src/sdk.rs:460-463 (real correctness break): "Run {} to log in, or pass --api-key." with "hotdata auth".cyan(). This references only bare hotdata auth, which now just prints help — a user hitting an expired session who follows this hint will not be logged in. It must point at hotdata auth login.
  • src/sdk.rs:818: "... Run 'hotdata auth login' (or 'hotdata auth') to re-authenticate." — the (or 'hotdata auth') parenthetical is now wrong.
  • src/config.rs:267: "... Run 'hotdata auth login' (or 'hotdata auth') or specify --workspace-id." — same stale parenthetical.
  • src/config.rs:287: "Run 'hotdata auth login' (or 'hotdata auth') to generate a new config file." — same.
  • src/jwt.rs:15 (doc comment): "user must hotdata auth again" — minor, but worth updating for accuracy.

The PR's stated goal is consistency; leaving these pointing at the removed shortcut is contradictory. The sdk.rs:460-463 case sends users down a dead end, so this remains blocking.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  • src/sdk.rs:460-463 — The session-refresh failure hint still tells users to Run hotdata auth to log in. After this PR, bare hotdata auth only prints help, so a user with an expired session who follows this hint is sent to a dead end and is not logged in. Must point at hotdata auth login.
  • src/sdk.rs:818, src/config.rs:267, src/config.rs:287 — the (or 'hotdata auth') parentheticals are now incorrect and should be removed.
  • src/jwt.rs:15 — doc comment "user must hotdata auth again" is now inaccurate (minor).

Action Required

Update the stale hints in src/sdk.rs, src/config.rs, and src/jwt.rs so they no longer point at the removed bare-hotdata auth shortcut. The sdk.rs:460-463 case is a genuine correctness break. This was raised in cycles 1 and 2 and remains unaddressed in the code (author replies alone do not resolve a blocking issue).

Comment thread src/main.rs
.unwrap()
.print_help()
.unwrap();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (re-raised from cycles 1, 2 & 3 — still unaddressed): This PR changes bare hotdata auth to print help instead of logging in, but the user-facing hints that still tell users hotdata auth will log them in were not updated. sdk.rs, config.rs, and jwt.rs are untouched in this PR, so they remain stale and are now incorrect:

  • src/sdk.rs:485-486 (real correctness break): "Run {} to log in, or pass --api-key." with "hotdata auth".cyan(). This references only bare hotdata auth, which now just prints help — a user hitting an expired session who follows this hint will not be logged in. It must point at hotdata auth login.
  • src/sdk.rs:843: "... Run 'hotdata auth login' (or 'hotdata auth') to re-authenticate." — the (or 'hotdata auth') parenthetical is now wrong.
  • src/config.rs:267: "... Run 'hotdata auth login' (or 'hotdata auth') or specify --workspace-id." — same stale parenthetical.
  • src/config.rs:287: "Run 'hotdata auth login' (or 'hotdata auth') to generate a new config file." — same.
  • src/jwt.rs:15 (doc comment): "user must hotdata auth again" — minor, but worth updating for accuracy.

The PR's stated goal is consistency; leaving these pointing at the removed shortcut is contradictory. The sdk.rs:485-486 case sends users down a dead end, so this remains blocking.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Blocking Issues

  • src/sdk.rs:485-486"Run {} to log in, or pass --api-key." with "hotdata auth".cyan() now points users at the bare hotdata auth shortcut, which this PR changed to print help instead of logging in. A user with an expired session who follows this hint will not be logged in. Must point at hotdata auth login.
  • Stale (or 'hotdata auth') parentheticals / references that are now incorrect: src/sdk.rs:843, src/config.rs:267, src/config.rs:287, and the src/jwt.rs:15 doc comment.

Action Required

Update the above user-facing hints (none of these files are touched in this PR yet) to reference hotdata auth login instead of bare hotdata auth. This has been raised in cycles 1, 2, and 3 and remains unaddressed in the code — an author reply alone does not resolve it. The PR's stated goal is consistency, so leaving these pointing at the removed shortcut is contradictory.

…change

Bare `hotdata auth` now prints help instead of logging in, so the runtime
hints, bundled docs/skills, and internal comments that told users `hotdata
auth` (no subcommand) authenticates were misleading. Point them all at
`hotdata auth login`, and tighten two tests that asserted the weaker
substring.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
claude[bot]
claude Bot previously approved these changes Jun 28, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All prior blocking issues (cycles 1–4) are resolved in this cycle: the stale user-facing hints in src/sdk.rs, src/config.rs, and src/jwt.rs now point at hotdata auth login and the (or 'hotdata auth') parentheticals are gone. Tests updated accordingly. LGTM.

Adds an integration test for the new no-subcommand arm (the 0%-patch-coverage
lines Codecov flagged): `hotdata auth` exits 0 with the clap help block listing
its subcommands and does NOT start the browser login flow. Runs offline and
without credentials.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
claude[bot]
claude Bot previously approved these changes Jun 28, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All prior blocking feedback addressed: the stale hotdata auth hints in src/sdk.rs, src/config.rs, and src/jwt.rs now point at hotdata auth login, and the behavior change is covered by a new offline test. LGTM.

Collapse the now-shorter hint strings and sort the test import — fixes the
fmt CI check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All prior blocking issues are resolved: the user-facing hints in src/sdk.rs, src/config.rs, and src/jwt.rs now point at hotdata auth login instead of the removed bare-hotdata auth shortcut, and the related tests were updated to match. The behavior change is consistent across code, docs, and tests.

@eddietejeda eddietejeda merged commit 1038e25 into main Jun 28, 2026
14 checks passed
@eddietejeda eddietejeda deleted the auth-no-login-shortcut branch June 28, 2026 05:01
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.

1 participant