Enable fh to generate and revoke device tokens

[lucperkins]

This PR adds an fh token device create --org <org> --description <description> command that enables generating coarse-grained device tokens for FlakeHub. It also enables revoking tokens.

Summary by CodeRabbit

• New Features
  • Added a new Token subcommand to the CLI for generating device tokens.
  • Users can now create authentication tokens for their organization by specifying the organization name and token description.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

A new token subcommand has been added to the FlakeHub CLI, enabling device token generation for authentication. The implementation includes a token module with CLI command handling, a generic POST helper for API requests, token request/response types, and integration into the main command dispatcher.

Changes

Cohort / File(s)	Summary
Token Subcommand Infrastructure src/cli/cmd/mod.rs	Introduces token module, adds Token(token::TokenSubcommand) variant to FhSubcommands enum, implements generate_device_token() method on FlakeHubClient, provides generic post() async helper for serialized POST requests, defines TokenRequest and TokenResponse types.
Token Subcommand Implementation src/cli/cmd/token.rs	New module implementing TokenSubcommand with Device variant; accepts org and description parameters via Clap; implements CommandExecute trait to invoke token generation and print resulting token.
Command Dispatch src/main.rs	Adds match arm routing Token subcommand variant to its execute method in main command dispatcher.
Version Update Cargo.toml	Bumps package version from 0.1.27 to 0.1.28.

Sequence Diagram

sequenceDiagram
    participant CLI as CLI Parser
    participant Handler as TokenSubcommand Handler
    participant Client as FlakeHubClient
    participant API as Remote API
    
    CLI->>Handler: execute(api_addr, org, description)
    Handler->>Client: generate_device_token(api_addr, org, description)
    Client->>Client: construct TokenRequest
    Client->>API: POST /token with TokenRequest
    API-->>Client: TokenResponse {token, actor_id}
    Client->>Client: extract token field
    Client-->>Handler: Result<String>
    Handler->>Handler: print token to stdout
    Handler-->>CLI: ExitCode::SUCCESS

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A token, a key to unlock the gate,
FlakeHub's new command makes auth first-rate,
With posts and with parsing, the rabbit hops free,
Device tokens flow like lettuce from a tree! 🥬✨

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title mentions both generating AND revoking device tokens, but the changeset only implements token generation functionality.	Update the title to accurately reflect the implemented changes: 'Enable fh to generate device tokens' (removing 'and revoke') or implement token revocation functionality to match the title.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch lucperkins/eng-1248-enable-fh-to-create-device-tokens

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/cli/cmd/mod.rs`:
- Around line 347-359: The post function currently deserializes the response
directly which hides HTTP error statuses; modify async fn post<In,Out> to fetch
the response with client.post(...).send().await?, inspect the response status
(e.g., response.status().is_success() or response.error_for_status()), and only
call response.json::<Out>().await when successful; on non-success, read the
response body (as text) and return an FhError that includes the HTTP status and
body to mirror how metadata/resolve/auth_status handle errors (referencing the
post function, make_base_client, and FhError).

🧹 Nitpick comments (1)

src/cli/cmd/mod.rs (1)

302-310: #[serde(skip_serializing)] is a no-op here.

TokenResponse only derives Deserialize, so a serialization-related attribute on actor_id has no effect. If the intent is to ignore actor_id during deserialization when it's missing, you'd want #[serde(default)]; otherwise the attribute can simply be removed to avoid confusion.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@flake.nix`:
- Line 122: Replace the deprecated pkgs.nixfmt-rfc-style reference with the
unified nixfmt package so both the formatter configuration and the check script
use the same name; specifically, change the formatter declaration that currently
uses pkgs.nixfmt-rfc-style to use nixfmt and ensure runtimeInputs (runtimeInputs
= [ nixfmt ]) and any other references to the formatter/check tooling all point
to nixfmt to keep names consistent.

🧹 Nitpick comments (2)

src/cli/cmd/mod.rs (2)

302-310: #[serde(skip_serializing)] has no effect on a Deserialize-only struct.

TokenResponse only derives Deserialize, so skip_serializing on actor_id is a no-op. If the intent is to ignore actor_id during deserialization, this attribute won't do that (and you'd want skip_deserializing or just leave it as-is since you destructure with ..). If the intent is just documentation that you don't use the field, the #[allow(dead_code)] already communicates that.

Suggested cleanup

         #[derive(Deserialize)]
         struct TokenResponse {
             token: String,
 
             // We don't need this yet but we may eventually
             #[allow(dead_code)]
-            #[serde(skip_serializing)]
             actor_id: String,
         }

---

292-319: create_token returns color_eyre::Result while post returns Result<_, FhError>.

This is fine since FhError can convert into color_eyre::Report via ?, but note the inconsistency: most FlakeHubClient methods that call get return Result<_, FhError>, while create_token returns color_eyre::Result. Consider aligning the return type for consistency with the rest of the client API, unless you plan to add richer error context here later.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/cli/cmd/mod.rs (1)

440-458: ⚠️ Potential issue | 🔴 Critical

#[cfg(test)] on make_base_client removes it from non-test builds — this is the root cause of every pipeline failure.

The function is gated with #[cfg(test)] (line 440), so it doesn't exist in release/debug builds. Every call site (lines 182, 223, 251, 328, 351) fails with cannot find function make_base_client. The cascading unused-import errors for ACCEPT, Client, HeaderMap, HeaderValue follow from the same cause.

The intent is clearly to skip the auth-token file read during tests. Remove #[cfg(test)] from the function and keep only the inner #[cfg(not(test))] gate.

Proposed fix

-#[cfg(test)]
 async fn make_base_client(authenticated: bool) -> Result<Client, FhError> {
     let mut headers = HeaderMap::new();
     headers.insert(ACCEPT, HeaderValue::from_static("application/json"));

🧹 Nitpick comments (1)

src/cli/cmd/mod.rs (1)

297-310: #[serde(skip_serializing)] is a no-op on a Deserialize-only struct.

TokenResponse derives only Deserialize, so skip_serializing on actor_id has no effect. If you want to document that the field is intentionally unused, the existing #[allow(dead_code)] already does that.

Proposed cleanup

         #[derive(Deserialize)]
         struct TokenResponse {
             token: String,
 
             // We don't need this yet but we may eventually
             #[allow(dead_code)]
-            #[serde(skip_serializing)]
             actor_id: String,
         }