Skip to content

Conversation

@jfrench9
Copy link
Member

@jfrench9 jfrench9 commented Nov 4, 2025

Summary

This PR refactors the AuthResponse model to include additional token management parameters, specifically token expiry information and refresh threshold settings. The changes improve token lifecycle management capabilities within the robosystems client.

Key Changes

  • Enhanced AuthResponse Model: Added new fields to track token expiration and refresh thresholds
  • Code Cleanup: Removed unused code from the Cypher query execution module
  • Improved Token Management: Better support for proactive token refresh based on configurable thresholds

Key Accomplishments

  • Expanded authentication response handling with 40 lines of new functionality
  • Streamlined existing code by removing 6 lines of unused imports/code
  • Established foundation for more robust token lifecycle management

Breaking Changes

⚠️ Potential Breaking Change: The AuthResponse model structure has been modified. Existing code that directly instantiates or serializes AuthResponse objects may need updates to accommodate the new fields.

Testing Notes

  • Verify that existing authentication flows continue to work with the enhanced model
  • Test token refresh logic with the new threshold parameters
  • Validate backward compatibility with systems that may not provide the new fields
  • Ensure proper handling of optional/default values for new parameters

Infrastructure Considerations

  • The authentication service may need updates to provide the new token expiry and threshold values
  • Client applications should be prepared to handle both old and new response formats during transition period
  • Consider monitoring token refresh patterns to optimize threshold values in production

🤖 Generated with Claude Code

Branch Info:

  • Source: chore/auth-response-params
  • Target: main
  • Type: feature

Co-Authored-By: Claude noreply@anthropic.com

…hold

This commit enhances the `AuthResponse` model by adding two new fields: `expires_in`, which indicates the token expiry time in seconds, and `refresh_threshold`, which suggests a recommended refresh time before the token expires. Additionally, the `_parse_response` function in `execute_cypher_query.py` has been cleaned up by removing outdated NDJSON handling code, improving overall response parsing efficiency.

Testing Notes:
- Verify that the new fields in `AuthResponse` are correctly serialized and deserialized.
- Ensure that the `_parse_response` function behaves as expected with various response formats.
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 63 to 65
if response.status_code == 200:
content_type = response.headers.get("content-type", "")
if (
"application/x-ndjson" in content_type
or response.headers.get("x-stream-format") == "ndjson"
):
return None
response_200 = response.json()
return response_200

Choose a reason for hiding this comment

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

P1 Badge Avoid parsing NDJSON streams as JSON

The 200-response branch unconditionally calls response.json(), but NDJSON streaming endpoints return line-delimited JSON rather than a single JSON document. This call now raises JSONDecodeError or buffers the stream, preventing QueryClient.execute_query from detecting the NDJSON path (it expects parsed to be None when streaming). The previous logic explicitly returned None for NDJSON content types, so this change breaks streaming queries that rely on NDJSON.

Useful? React with 👍 / 👎.

This commit updates the `_parse_response` function in `execute_cypher_query.py` to return `None` for NDJSON responses, improving the robustness of response parsing. This change ensures that the function correctly identifies NDJSON content types and avoids unnecessary parsing attempts.

Testing Notes:
- Verify that NDJSON responses are handled correctly without errors.
- Ensure that other response types continue to be processed as expected.
@jfrench9 jfrench9 merged commit 8c903c6 into main Nov 4, 2025
1 check passed
@jfrench9 jfrench9 deleted the chore/auth-response-params branch November 4, 2025 03:03
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.

2 participants