Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI user
participant CLI as jmp `auth` command
participant Config as local config
participant OIDC as oidc helpers
Note over User,CLI: User runs `jmp auth status`
User->>CLI: invoke `auth status`
CLI->>Config: read token
alt token missing
Config-->>CLI: no token
CLI-->>User: print "no token" message
else token present
Config-->>CLI: return token
CLI->>OIDC: decode_jwt(token)
alt decode error
OIDC-->>CLI: raise error
CLI-->>User: print decode error
else decoded
OIDC-->>CLI: payload
CLI->>OIDC: get_token_expiry(token)
OIDC-->>CLI: expiry or None
CLI->>OIDC: get_token_remaining_seconds(token)
OIDC-->>CLI: remaining seconds
CLI-->>User: formatted expiry, status, sub/iss
end
end
sequenceDiagram
participant Shell as interactive shell
participant Monitor as token monitor task
participant Config as local config
participant Cancel as cancel scope / runtime
Note over Shell,Monitor: Shell starts background monitor
Shell->>Monitor: start _monitor_token_expiry(config, cancel_scope)
Monitor->>Config: read token periodically
alt token missing
Config-->>Monitor: no token (no action)
else token present
Config-->>Monitor: token
Monitor->>Config: get_token_remaining_seconds(token)
Config-->>Monitor: seconds remaining
alt remaining <= 0
Monitor->>Cancel: trigger cancellation / exit (code 3)
else remaining <= TOKEN_EXPIRY_WARNING_SECONDS
Monitor-->>Shell: log warning about impending expiry
else
Monitor-->>Shell: no action
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @packages/jumpstarter-cli/jumpstarter_cli/auth.py:
- Around line 38-44: There is a missing function: implement
get_token_remaining_seconds in jumpstarter_cli_common.oidc and export it so the
import in auth.py succeeds; the function should accept the token string, parse
the expiry (exp) claim safely and return remaining seconds (or None) and
raise/return clear errors on malformed input. Also update auth.py to construct
the expiry datetime in UTC by calling datetime.fromtimestamp(exp,
tz=timezone.utc) (add timezone import) to ensure consistent timezone handling
when formatting exp_dt.
🧹 Nitpick comments (1)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (1)
46-62: Status logic is correct and user-friendly.The status display provides clear, color-coded feedback with helpful guidance. The time calculations and thresholds are appropriate.
Minor refactoring opportunities for improved maintainability:
Optional: Extract time formatting helper
def format_duration(seconds): """Format seconds into hours and minutes.""" hours = int(abs(seconds) / 3600) mins = int((abs(seconds) % 3600) / 60) return hours, mins # Then at lines 47-48 and 60-61: hours, mins = format_duration(remaining)You could also define named constants for the thresholds:
EXPIRING_SOON_THRESHOLD = 300 # 5 minutes WARNING_THRESHOLD = 3600 # 1 hour
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧠 Learnings (5)
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-05-28T15:09:35.768Z
Learnt from: NickCao
Repo: jumpstarter-dev/jumpstarter PR: 463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧬 Code graph analysis (2)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/config.py (1)
opt_config(99-100)packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (1)
decode_jwt(135-136)
packages/jumpstarter-cli/jumpstarter_cli/jmp.py (1)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (1)
auth(9-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pytest
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: e2e
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (5)
packages/jumpstarter-cli/jumpstarter_cli/jmp.py (1)
8-8: LGTM! Clean integration of the new auth command.The import and command registration follow the established patterns in the codebase and are placed consistently with other commands.
Also applies to: 25-25
packages/jumpstarter-cli/jumpstarter_cli/auth.py (4)
1-12: LGTM! Clean module setup.The imports are appropriate and the auth command group is properly defined with clear documentation.
15-26: LGTM! Proper config handling.The use of
opt_config(exporter=False)is appropriate for this client-side command, and the defensive token retrieval with clear user feedback is well implemented.
27-36: LGTM! Robust error handling.The exception handling for token decoding and validation of the expiry claim provide good defensive programming with clear user feedback.
64-68: LGTM! Safe display of token metadata.The conditional display of subject and issuer fields uses safe dictionary access and provides useful context without exposing sensitive information.
| remaining = get_token_remaining_seconds(token_str) | ||
| exp_dt = datetime.fromtimestamp(exp) | ||
|
|
||
| click.echo(f"Token expiry: {exp_dt.strftime('%Y-%m-%d %H:%M:%S')}") | ||
|
|
||
| if remaining is None: | ||
| return |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the auth.py file to see the current state and imports
cat -n packages/jumpstarter-cli/jumpstarter_cli/auth.py | head -80Repository: jumpstarter-dev/jumpstarter
Length of output: 2801
🏁 Script executed:
# Search for get_token_remaining_seconds function definition
rg "def get_token_remaining_seconds" -A 10Repository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Check imports in the auth.py file to see if timezone is already imported
rg "^from datetime import|^import datetime" packages/jumpstarter-cli/jumpstarter_cli/auth.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 100
🏁 Script executed:
# Search for get_token_remaining_seconds implementation in the common module
find . -name "oidc.py" -path "*/jumpstarter_cli_common/*" | head -5Repository: jumpstarter-dev/jumpstarter
Length of output: 135
🏁 Script executed:
# Also search for get_token_remaining_seconds anywhere in the codebase
rg "def get_token_remaining_seconds" -A 15Repository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Read the oidc.py file to see get_token_remaining_seconds implementation
cat -n packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 5740
🏁 Script executed:
# Get the rest of the oidc.py file
tail -n +140 packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 110
🏁 Script executed:
# Search for get_token_remaining_seconds in the entire codebase
rg "get_token_remaining_seconds" -A 10Repository: jumpstarter-dev/jumpstarter
Length of output: 1682
🏁 Script executed:
# Search for where get_token_remaining_seconds is actually defined
rg "def get_token_remaining_seconds"Repository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Check if it might be in __init__.py or exported from somewhere
rg "get_token_remaining_seconds" -lRepository: jumpstarter-dev/jumpstarter
Length of output: 119
🏁 Script executed:
# Let's check the jumpstarter_cli_common package structure
ls -la packages/jumpstarter-cli-common/jumpstarter_cli_common/Repository: jumpstarter-dev/jumpstarter
Length of output: 894
🏁 Script executed:
# Search for get_token_remaining_seconds across all Python files
rg "get_token_remaining_seconds" -t pyRepository: jumpstarter-dev/jumpstarter
Length of output: 303
🏁 Script executed:
# Let's check the full length of oidc.py
wc -l packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 137
🏁 Script executed:
# Read the entire oidc.py file to see if get_token_remaining_seconds exists
cat packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py | grep -n "get_token_remaining_seconds"Repository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Check if the function might be in the __init__.py
cat packages/jumpstarter-cli-common/jumpstarter_cli_common/__init__.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Check if there are any other Python files in jumpstarter_cli_common that might define this
find packages/jumpstarter-cli-common -name "*.py" -type f | xargs grep -l "def get_token_remaining_seconds"Repository: jumpstarter-dev/jumpstarter
Length of output: 55
🏁 Script executed:
# Let's verify the current oidc.py file one more time, getting the full content
cat packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 4760
🏁 Script executed:
# Search everywhere for get_token_remaining_seconds definition
find . -name "*.py" -type f | xargs grep -n "def get_token_remaining_seconds" 2>/dev/nullRepository: jumpstarter-dev/jumpstarter
Length of output: 55
🏁 Script executed:
# Check if it's maybe defined in other files in packages/jumpstarter-cli-common
find packages/jumpstarter-cli-common -name "*.py" -type f -exec cat {} \; | grep -n "get_token_remaining_seconds"Repository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Let's check git history to see if the function was recently removed
git log --oneline -10 packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 99
🏁 Script executed:
# Check if there's maybe a type stub or stub file
find packages/jumpstarter-cli-common -name "*.pyi"Repository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Let's try importing and see what happens - search for any try-except or conditional import
rg "from jumpstarter_cli_common.oidc import" -B 3 -A 3Repository: jumpstarter-dev/jumpstarter
Length of output: 1221
🏁 Script executed:
# Check the specific commit that added the auth status
git show 308045d --statRepository: jumpstarter-dev/jumpstarter
Length of output: 40311
🏁 Script executed:
# Let's see if there's more context in that commit
git show 308045d -- packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py | head -100Repository: jumpstarter-dev/jumpstarter
Length of output: 3457
🏁 Script executed:
# Check if get_token_remaining_seconds was added in a later commit
git log --oneline -- packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 99
🏁 Script executed:
# Check the current status - try to search for where this function might be defined elsewhere
rg "def get_token_remaining_seconds" --type pyRepository: jumpstarter-dev/jumpstarter
Length of output: 53
🏁 Script executed:
# Check if there are any stub files or type definitions
find . -name "*.pyi" -o -name "py.typed" | xargs grep -l "get_token_remaining_seconds" 2>/dev/nullRepository: jumpstarter-dev/jumpstarter
Length of output: 55
🏁 Script executed:
# Let's check if maybe it's supposed to be defined but isn't yet
rg "token.*remaining" -t pyRepository: jumpstarter-dev/jumpstarter
Length of output: 303
🏁 Script executed:
# Let's try to understand what this function should do - check for similar patterns
rg "remaining" packages/jumpstarter-cli/jumpstarter_cli/auth.py -B 2 -A 2Repository: jumpstarter-dev/jumpstarter
Length of output: 1453
The get_token_remaining_seconds function is not defined in oidc.py and must be implemented before this code can run.
The import at line 5 will fail with an ImportError because this function does not exist in jumpstarter_cli_common.oidc. This function needs to be implemented first. Additionally, consider using UTC for the timestamp conversion at line 39 to ensure consistency across timezones:
from datetime import datetime, timezone
# At line 39:
exp_dt = datetime.fromtimestamp(exp, tz=timezone.utc)🤖 Prompt for AI Agents
In @packages/jumpstarter-cli/jumpstarter_cli/auth.py around lines 38 - 44, There
is a missing function: implement get_token_remaining_seconds in
jumpstarter_cli_common.oidc and export it so the import in auth.py succeeds; the
function should accept the token string, parse the expiry (exp) claim safely and
return remaining seconds (or None) and raise/return clear errors on malformed
input. Also update auth.py to construct the expiry datetime in UTC by calling
datetime.fromtimestamp(exp, tz=timezone.utc) (add timezone import) to ensure
consistent timezone handling when formatting exp_dt.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (1)
38-44: Consider passing the already-decoded payload to avoid redundant JWT decoding.The JWT is decoded twice: once at line 28 (
decode_jwt) and again implicitly at line 38 whenget_token_remaining_secondsinternally callsdecode_jwt. While not incorrect, this is inefficient.🔎 Suggested refactor to eliminate double decoding
Option 1: Pass the expiry directly since you already have it from
payload:- remaining = get_token_remaining_seconds(token_str) + import time + remaining = exp - time.time()Option 2: Add a helper function that accepts the pre-decoded payload:
# In oidc.py def get_remaining_seconds_from_exp(exp: int) -> float: import time return exp - time.time()Then use it in auth.py:
- remaining = get_token_remaining_seconds(token_str) + from jumpstarter_cli_common.oidc import get_remaining_seconds_from_exp + remaining = get_remaining_seconds_from_exp(exp)packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (1)
155-168: LGTM with minor style suggestion!The function logic is correct and the docstring clearly explains the behavior for valid, expired, and tokens without expiry. The implementation properly handles all cases.
💡 Style suggestion: Move inline import to module level
The inline import of
timeat line 163 is unconventional. Consider moving it to the top of the file with other imports:import json import os +import time from dataclasses import dataclassThen remove the inline import from the function.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pypackages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/jmp.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (9)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (5)
1-6: LGTM!The imports are appropriate and complete. The
timezoneimport correctly addresses the past review concern about timezone-aware datetime handling.
8-12: LGTM!Clean Click group definition with appropriate documentation.
21-36: LGTM with past review concerns addressed!The token validation and JWT decoding logic is solid. Error handling appropriately covers missing tokens and malformed JWTs. The UTC timezone usage at line 39 correctly addresses the past review concern.
46-62: LGTM!The time calculation logic is mathematically correct across all branches:
- Expired tokens show elapsed time using hours/minutes
- Tokens expiring within 5 minutes show minutes/seconds
- Valid tokens show appropriate time remaining
The color-coded status messages (red for expired/critical, yellow for warning, green for valid) provide clear user feedback.
64-68: LGTM!Displaying subject and issuer provides useful context for the authenticated session.
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (4)
26-32: LGTM!Minor formatting improvements with no functional impact.
Also applies to: 97-97
136-140: LGTM!Excellent improvement adding explicit error handling to
decode_jwt. The try-except block appropriately catches JWT parsing errors and raises a clearValueErrorwith context, making it easier for callers to handle malformed tokens.
147-152: LGTM!Clean helper function with appropriate type hints and clear documentation. Properly handles tokens without expiry claims.
171-185: LGTM!Well-implemented helper function with comprehensive documentation. The logic correctly handles tokens with and without expiry claims, and the
buffer_secondsparameter is a thoughtful addition for preemptive token refresh scenarios.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (1)
16-66: Consider using the new helper functions from oidc.py.The status command implementation is solid with good error handling and user-friendly output. However,
oidc.pynow providesget_token_remaining_seconds()andis_token_expired()helpers that encapsulate the logic you're manually implementing at line 39.Using these helpers would reduce code duplication and improve maintainability.
🔎 Optional refactor to use helper functions
import time from datetime import datetime, timezone import click from jumpstarter_cli_common.config import opt_config -from jumpstarter_cli_common.oidc import decode_jwt +from jumpstarter_cli_common.oidc import decode_jwt, get_token_expiry, get_token_remaining_seconds @click.group() def auth(): """ Authentication and token management commands """ @auth.command(name="status") @opt_config(exporter=False) def token_status(config): """ Display token status and expiry information """ token_str = getattr(config, "token", None) if not token_str: click.echo(click.style("No token found in config", fg="yellow")) return try: payload = decode_jwt(token_str) except Exception as e: click.echo(click.style(f"Failed to decode token: {e}", fg="red")) return - exp = payload.get("exp") + exp = get_token_expiry(token_str) if not exp: click.echo(click.style("Token has no expiry claim", fg="yellow")) return - remaining = exp - time.time() + remaining = get_token_remaining_seconds(token_str) exp_dt = datetime.fromtimestamp(exp, tz=timezone.utc) click.echo(f"Token expiry: {exp_dt.strftime('%Y-%m-%d %H:%M:%S')}") if remaining < 0: hours = int(abs(remaining) / 3600) mins = int((abs(remaining) % 3600) / 60) click.echo(click.style(f"Status: EXPIRED ({hours}h {mins}m ago)", fg="red", bold=True)) click.echo(click.style("Run 'jmp login' to refresh your credentials.", fg="yellow")) elif remaining < 300: # Less than 5 minutes mins = int(remaining / 60) secs = int(remaining % 60) click.echo(click.style(f"Status: EXPIRING SOON ({mins}m {secs}s remaining)", fg="red", bold=True)) click.echo(click.style("Run 'jmp login' to refresh your credentials.", fg="yellow")) elif remaining < 3600: # Less than 1 hour mins = int(remaining / 60) click.echo(click.style(f"Status: Valid ({mins}m remaining)", fg="yellow")) else: hours = int(remaining / 3600) mins = int((remaining % 3600) / 60) click.echo(click.style(f"Status: Valid ({hours}h {mins}m remaining)", fg="green")) # Show additional token info if payload.get("sub"): click.echo(f"Subject: {payload.get('sub')}") if payload.get("iss"): click.echo(f"Issuer: {payload.get('iss')}")
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pypackages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pypackages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧠 Learnings (5)
📚 Learning: 2025-11-27T09:58:41.875Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/creating-new-drivers.mdc:0-0
Timestamp: 2025-11-27T09:58:41.875Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/client.py : Driver client CLIs should either implement known classes that provide a CLI interface, or implement their own CLI interface. Use `CompositeClient` from `jumpstarter_driver_composite.client` for composite drivers with child drivers.
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `driver.py` file containing the driver implementation
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-driver-*/jumpstarter_driver_*/ : Driver packages must implement a `client.py` file containing the client implementation
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-11-27T09:58:55.346Z
Learnt from: CR
Repo: jumpstarter-dev/jumpstarter PR: 0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-11-27T09:58:55.346Z
Learning: Applies to packages/jumpstarter-cli-*/pyproject.toml : CLI packages must depend on `jumpstarter` and `jumpstarter-cli-common` in their `pyproject.toml`
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
📚 Learning: 2025-05-28T15:09:35.768Z
Learnt from: NickCao
Repo: jumpstarter-dev/jumpstarter PR: 463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
Applied to files:
packages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧬 Code graph analysis (2)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/config.py (1)
opt_config(99-100)packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (1)
decode_jwt(137-141)
packages/jumpstarter-cli/jumpstarter_cli/jmp.py (1)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (1)
auth(10-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: build
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
🔇 Additional comments (5)
packages/jumpstarter-cli/jumpstarter_cli/jmp.py (1)
8-8: LGTM! Auth command integration follows established patterns.The import and registration of the
authcommand is consistent with how other CLI commands are integrated.Also applies to: 25-25
packages/jumpstarter-cli/jumpstarter_cli/auth.py (1)
9-13: LGTM! Clear command group definition.The auth group is properly structured with a descriptive docstring.
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (3)
3-3: LGTM! Import needed for new helper functions.
137-141: LGTM! Improved error handling with clearer messages.The updated
decode_jwtnow raises a more descriptiveValueErrorwhen JWT parsing fails, which improves debugging and user experience.
148-184: LGTM! Well-designed JWT helper functions.The three new helper functions provide clean abstractions for common JWT operations:
get_token_expiry: Simple extraction of exp claimget_token_remaining_seconds: Clear calculation with positive/negative/None semanticsis_token_expired: Useful buffer support for proactive expiry checksAll have clear docstrings and appropriate return types.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/jumpstarter-cli/jumpstarter_cli/shell.py:
- Around line 108-116: The expired-token warning builds a reconnect command
using the selector but selector can be None; update _warn_about_expired_token to
handle a None selector by omitting the "-s <selector>" portion (or by using an
empty string) so the printed command is valid, and keep the call site in the
except block (where _warn_about_expired_token(lease_used.name, selector) is
invoked) unchanged; ensure the function checks the selector parameter before
inserting the flag into the reconnect command string.
🧹 Nitpick comments (1)
packages/jumpstarter-cli/jumpstarter_cli/shell.py (1)
55-56: Broad exception handler silently suppresses errors.The bare
except Exceptionwill swallow any error fromget_token_remaining_secondsor other operations, making debugging difficult if issues arise. Consider logging or at least narrowing the exception types.♻️ Suggested improvement
- except Exception: - return + except Exception as e: + click.echo(click.style(f"\nToken monitor error: {e}", fg="yellow"), err=True) + return
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/jumpstarter-cli/jumpstarter_cli/shell.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-cli/jumpstarter_cli/shell.py
🧬 Code graph analysis (1)
packages/jumpstarter-cli/jumpstarter_cli/shell.py (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (1)
get_token_remaining_seconds(156-167)packages/jumpstarter/jumpstarter/common/utils.py (1)
launch_shell(80-156)packages/jumpstarter-cli-common/jumpstarter_cli_common/signal.py (1)
signal_handler(9-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: build
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: e2e
- GitHub Check: Pages changed - jumpstarter-docs
🔇 Additional comments (4)
packages/jumpstarter-cli/jumpstarter_cli/shell.py (4)
9-9: LGTM!Import correctly added for the new token expiry monitoring functionality.
19-24: LGTM!Clear and actionable user messaging with appropriate color-coded warnings and a helpful reconnect command.
59-71: LGTM!Formatting improvements only; no semantic changes.
91-93: LGTM!Clean integration of the token monitor alongside the existing signal handler, both sharing the same cancel scope.
provide status of the current session: Token expiry: 2026-01-06 19:39:42 Status: Valid (9h 17m remaining) Subject: ... Issuer: https://... Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/jumpstarter-cli/jumpstarter_cli/shell.py (1)
30-61: Improve error handling in token monitoring.The generic exception handler silently suppresses all errors, which could hide real issues like bugs in the monitoring logic or problems with token extraction.
♻️ Suggested improvement
Consider logging exceptions or at least specific expected exceptions:
await anyio.sleep(30) - except Exception: - return + except Exception as e: + # Log unexpected errors but don't crash the shell + click.echo(click.style(f"\nToken monitoring error: {e}", fg="yellow"), err=True) + returnOr handle only specific expected exceptions if you want to fail silently for those cases.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pypackages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/jmp.pypackages/jumpstarter-cli/jumpstarter_cli/shell.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-cli/jumpstarter_cli/jmp.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pypackages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/shell.py
🧬 Code graph analysis (1)
packages/jumpstarter-cli/jumpstarter_cli/auth.py (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/config.py (1)
opt_config(99-100)packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (3)
decode_jwt(137-141)format_duration(191-209)get_token_remaining_seconds(156-167)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: build
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: e2e
🔇 Additional comments (9)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (3)
137-141: LGTM! Improved error handling.The updated error handling provides clear feedback when JWT decoding fails, and properly chains exceptions for better debugging context.
148-167: LGTM! Clean token expiry utilities.The functions correctly handle token expiry calculations with appropriate null-safety for tokens without exp claims. The return semantics (positive for valid, negative for expired) are clearly documented.
191-209: LGTM! User-friendly duration formatting.The function provides appropriate granularity for different time scales, making expiry information easily readable.
packages/jumpstarter-cli/jumpstarter_cli/shell.py (3)
23-27: LGTM! Excellent user guidance.The warning provides clear information about the failure and actionable next steps for the user to reconnect to the active lease.
94-94: LGTM! Clean integration of token monitoring.The lease tracking and monitoring task integration are well-structured. Starting the monitor alongside signal handling ensures timely expiry detection.
Also applies to: 98-98, 104-104
114-120: LGTM! Proper token expiry handling on cancellation.The code correctly re-fetches the token to check current state (rather than relying on cached values) and provides clear user feedback with a distinct exit code.
packages/jumpstarter-cli/jumpstarter_cli/auth.py (3)
13-15: LGTM! Clean command group definition.
18-31: LGTM! Excellent status presentation.The function provides clear visual feedback with appropriate color coding and helpful suggestions for users with expiring/expired tokens.
34-67: LGTM! Robust token status implementation.The command handles all edge cases appropriately:
- Missing tokens
- Invalid JWT format
- Tokens without expiry claims
The timezone-aware timestamp formatting and display of token metadata provide comprehensive status information.
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py:
- Around line 174-189: The is_token_expired function currently returns False for
tokens with exactly zero remaining seconds due to using '<'; update the
comparison in is_token_expired (which calls get_token_remaining_seconds) to use
'<=' so tokens with remaining <= buffer_seconds are considered expired,
preserving the existing behavior when get_token_remaining_seconds returns None.
- Around line 148-168: get_token_expiry may return non-numeric values from
decode_jwt, so update the logic to validate and cast the "exp" claim to a
numeric type before doing arithmetic in get_token_remaining_seconds: either
adjust get_token_expiry to normalize the claim (if exp is None return None; if
exp is an int/float return it; if exp is a numeric string parse to int/float and
return; otherwise return None) or keep get_token_expiry as-is but in
get_token_remaining_seconds fetch exp, check its type, attempt to cast
string->float/int (handling ValueError) and treat invalid values as None, then
subtract time.time() from the validated numeric value. Ensure references:
decode_jwt, get_token_expiry, get_token_remaining_seconds.
In @packages/jumpstarter-cli/jumpstarter_cli/shell.py:
- Around line 30-62: Update _monitor_token_expiry to re-read config.token at the
start of each loop iteration (so it notices re-authentication) and replace the
bare "except Exception: return" with explicit handling: catch
cancellation-specific exceptions (e.g., anyio.get_cancelled_exc_class() or
CancelledError) to return/stop, and for all other exceptions log or re-raise
them so they are not silently swallowed; ensure get_token_remaining_seconds,
TOKEN_EXPIRY_WARNING_SECONDS, warned, and cancel_scope usage remain intact and
that unexpected errors produce a visible message (or are propagated) rather than
a silent return.
🧹 Nitpick comments (3)
packages/jumpstarter-cli/jumpstarter_cli/shell.py (2)
23-28: Useselector(or drop it) in_warn_about_expired_tokenguidance.
selectoris unused, and the reconnect hint may be insufficient if the user normally relies on--selector(or other context) to find the same target.Proposed tweak
def _warn_about_expired_token(lease_name: str, selector: str) -> None: @@ - click.echo(click.style(f"To reconnect: JMP_LEASE={lease_name} jmp shell", fg="cyan")) + selector_hint = f" --selector {selector}" if selector else "" + click.echo(click.style(f"To reconnect: JMP_LEASE={lease_name} jmp shell{selector_hint}", fg="cyan"))
96-103: Expired-token precheck:ConnectionErroris semantically misleading for auth errors.The handler does catch
ConnectionErrorand triggers reauthentication when "expired" appears in the error message. However,ConnectionErrorsemantically suggests a network/connection problem, not an authentication failure. The codebase already definesReauthenticationFailedexception (injumpstarter/common/exceptions.py) which would better communicate the actual issue. Consider raising that instead, or use a dedicated auth-specific exception to improve code clarity and UX.packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (1)
191-209: Optional:format_durationignores sign (no “ago/in”).If you ever use it for expired tokens (negative seconds), output can be ambiguous (“5m 0s” vs “5m 0s ago”). If this is only for remaining time, it’s fine.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.pypackages/jumpstarter-cli/jumpstarter_cli/auth.pypackages/jumpstarter-cli/jumpstarter_cli/shell.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-cli/jumpstarter_cli/auth.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
Ruff should be used for code formatting and linting, excluding
jumpstarter-protocolpackage
Files:
packages/jumpstarter-cli/jumpstarter_cli/shell.pypackages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py
🧬 Code graph analysis (1)
packages/jumpstarter-cli/jumpstarter_cli/shell.py (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (2)
format_duration(191-209)get_token_remaining_seconds(156-167)packages/jumpstarter/jumpstarter/common/exceptions.py (1)
ConnectionError(42-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: build
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (2)
packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py (1)
137-142: Good:decode_jwtnow fails with a clear, actionable error.Wrapping decode/parsing issues into a stable
ValueErrorshould improve CLI UX vs leaking lower-level exceptions.packages/jumpstarter-cli/jumpstarter_cli/shell.py (1)
104-132: The suggested fix would make this worse, not better. Addingcancellable=Truedoes not interrupt the subprocess.With
cancellable=True, the awaiting task cancels and returns immediately (abandoning the thread), but the thread continues runningPopen(...).wait()in the background. Users would see "Token expired! Exiting shell." while the shell subprocess keeps running orphaned in a worker thread with no way to terminate it.The underlying issue is real: you cannot interrupt a blocking subprocess operation via task cancellation alone. To actually fix this:
- Manage the subprocess directly so it can be killed on token expiry (store the
Popenobject, callprocess.terminate()in a cancellation handler)- Or soften the messaging: "Token expired; please exit the shell manually."
The current code at least waits for the subprocess to finish, even if the message is misleading.
Likely an incorrect or invalid review comment.
| def is_token_expired(token: str, buffer_seconds: int = 0) -> bool: | ||
| """Check if token is expired or will expire within buffer_seconds. | ||
|
|
||
| Args: | ||
| token: JWT token string | ||
| buffer_seconds: Consider expired if less than this many seconds remain | ||
|
|
||
| Returns: | ||
| True if token is expired or will expire within buffer | ||
| False if token is still valid (or has no exp claim) | ||
| """ | ||
| remaining = get_token_remaining_seconds(token) | ||
| if remaining is None: | ||
| return False | ||
| return remaining < buffer_seconds | ||
|
|
There was a problem hiding this comment.
is_token_expired should use <= to match expiry semantics and your shell logic.
With the current <, a token with exactly 0 seconds remaining is treated as not expired (and buffer_seconds=0 won’t count it as expired).
Proposed fix
- return remaining < buffer_seconds
+ return remaining <= buffer_seconds🤖 Prompt for AI Agents
In @packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py around lines
174 - 189, The is_token_expired function currently returns False for tokens with
exactly zero remaining seconds due to using '<'; update the comparison in
is_token_expired (which calls get_token_remaining_seconds) to use '<=' so tokens
with remaining <= buffer_seconds are considered expired, preserving the existing
behavior when get_token_remaining_seconds returns None.
| async def _monitor_token_expiry(config, cancel_scope) -> None: | ||
| """Monitor token expiry and warn user.""" | ||
| token = getattr(config, "token", None) | ||
| if not token: | ||
| return | ||
|
|
||
| warned = False | ||
| while not cancel_scope.cancel_called: | ||
| try: | ||
| remaining = get_token_remaining_seconds(token) | ||
| if remaining is None: | ||
| return | ||
|
|
||
| if remaining <= 0: | ||
| click.echo(click.style("\nToken expired! Exiting shell.", fg="red", bold=True)) | ||
| cancel_scope.cancel() | ||
| return | ||
|
|
||
| if remaining <= TOKEN_EXPIRY_WARNING_SECONDS and not warned: | ||
| duration = format_duration(remaining) | ||
| click.echo( | ||
| click.style( | ||
| f"\nToken expires in {duration}. Session will continue but cleanup may fail on exit.", | ||
| fg="yellow", | ||
| bold=True, | ||
| ) | ||
| ) | ||
| warned = True | ||
|
|
||
| await anyio.sleep(30) | ||
| except Exception: | ||
| return | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check AnyIO version and project dependencies
if [ -f "pyproject.toml" ]; then
echo "=== pyproject.toml anyio version ==="
grep -A 2 -B 2 "anyio" pyproject.toml | head -20
fi
if [ -f "requirements.txt" ]; then
echo "=== requirements.txt anyio ==="
grep "anyio" requirements.txt
fi
if [ -f "setup.py" ]; then
echo "=== setup.py anyio ==="
grep "anyio" setup.py
fi
if [ -f "poetry.lock" ]; then
echo "=== poetry.lock anyio ==="
grep -A 5 'name = "anyio"' poetry.lock | head -20
fiRepository: jumpstarter-dev/jumpstarter
Length of output: 107
🏁 Script executed:
#!/bin/bash
# Search for other exception handling patterns in the codebase, especially with anyio
echo "=== Exception handling with anyio.sleep or cancellation ==="
rg "except.*cancell" -B 2 -A 2 --type py
echo ""
echo "=== Bare except Exception patterns ==="
rg "except Exception:" -B 3 -A 2 --type py | head -50
echo ""
echo "=== How get_cancelled_exc_class is used ==="
rg "get_cancelled_exc_class|CancelScope" --type py | head -30Repository: jumpstarter-dev/jumpstarter
Length of output: 8599
🏁 Script executed:
#!/bin/bash
# Check the imports and context of the monitored function
cat -n packages/jumpstarter-cli/jumpstarter_cli/shell.py | head -35Repository: jumpstarter-dev/jumpstarter
Length of output: 1668
🏁 Script executed:
#!/bin/bash
# Verify the full function and surrounding context
cat -n packages/jumpstarter-cli/jumpstarter_cli/shell.py | sed -n '30,65p'Repository: jumpstarter-dev/jumpstarter
Length of output: 1526
🏁 Script executed:
#!/bin/bash
# Check if there are other similar monitoring/task functions in shell.py
rg -A 15 "while not cancel_scope.cancel_called" packages/jumpstarter-cli/jumpstarter_cli/shell.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 730
🏁 Script executed:
#!/bin/bash
# Check if token is re-authenticated elsewhere in the session
rg "config.token|setattr.*token" packages/jumpstarter-cli/jumpstarter_cli/ -B 2 -A 2Repository: jumpstarter-dev/jumpstarter
Length of output: 2222
Catch cancellation separately; surface unexpected errors instead of silently failing.
The bare except Exception: return silently hides real failures (unexpected token formats, network issues in get_token_remaining_seconds, etc.) and makes the monitoring feature unreliable.
Additionally, reading the token once outside the loop misses re-authentication events. Token re-auth can occur during a session (as seen in login.py where config.token is updated dynamically), so the monitor should check for a fresh token on each iteration.
Proposed fix
async def _monitor_token_expiry(config, cancel_scope) -> None:
"""Monitor token expiry and warn user."""
- token = getattr(config, "token", None)
- if not token:
- return
-
warned = False
while not cancel_scope.cancel_called:
try:
+ # Re-read in case token changes during re-auth
+ token = getattr(config, "token", None)
+ if not token:
+ return
remaining = get_token_remaining_seconds(token)
if remaining is None:
return
@@ -47,8 +50,10 @@ async def _monitor_token_expiry(config, cancel_scope) -> None:
await anyio.sleep(30)
- except Exception:
+ except get_cancelled_exc_class():
return
+ except Exception as e:
+ click.echo(click.style(f"\nToken monitor stopped: {e}", fg="yellow"))
+ returnCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/jumpstarter-cli/jumpstarter_cli/shell.py around lines 30 - 62,
Update _monitor_token_expiry to re-read config.token at the start of each loop
iteration (so it notices re-authentication) and replace the bare "except
Exception: return" with explicit handling: catch cancellation-specific
exceptions (e.g., anyio.get_cancelled_exc_class() or CancelledError) to
return/stop, and for all other exceptions log or re-raise them so they are not
silently swallowed; ensure get_token_remaining_seconds,
TOKEN_EXPIRY_WARNING_SECONDS, warned, and cancel_scope usage remain intact and
that unexpected errors produce a visible message (or are propagated) rather than
a silent return.
|
nice! |
2 participants
provide status of the current session:
Token expiry: 2026-01-06 19:39:42
Status: Valid (9h 17m remaining)
Subject: ...
Issuer: https://...
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.