Skip to content

[LSP] verify and submit api key #533

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Jul 9, 2025

PR Type

Feature


Description

  • Add LSP commands to verify and submit API key

  • Adapt API key retrieval for LSP quiet mode

  • Refactor LSP server init to defer optimizer setup

  • Quote API key export and clear cache on update


Changes diagram

flowchart LR
  F["apiKeyExistsAndValid"] --> D["_initialize_optimizer"]
  A["provideApiKey"] --> B["save_api_key_to_rc"]
  B --> C["clear API key cache"]
  C --> D
  D --> E["return success or error"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
cfapi.py
Add LSP-aware version check handling                                         

codeflash/api/cfapi.py

  • Log debug message instead of exit in LSP quiet mode
  • Return None for outdated CLI in quiet mode
  • +3/-0     
    env_utils.py
    Prefer shell config for API key in LSP                                     

    codeflash/code_utils/env_utils.py

  • In LSP quiet mode prefer shell config over env var
  • Swap retrieval order based on console.quiet flag
  • +6/-1     
    beta.py
    Implement LSP API key features                                                     

    codeflash/lsp/beta.py

  • Add ProvideApiKeyParams dataclass
  • Implement apiKeyExistsAndValid and provideApiKey handlers
  • Initialize optimizer only after valid API key
  • +50/-0   
    server.py
    Refactor LSP server init and logging                                         

    codeflash/lsp/server.py

  • Rename initializer to prepare_optimizer_arguments
  • Store args and defer optimizer instantiation
  • Add Debug mapping in show_message_log
  • +6/-4     
    Formatting
    shell_utils.py
    Quote API key in shell export                                                       

    codeflash/code_utils/shell_utils.py

    • Wrap API key in quotes in export line
    +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    github-actions bot commented Jul 9, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit a0ff82f)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Cache clearing

    provide_api_key calls get_user_id.cache_clear() but get_user_id is not decorated with a cache, so this may raise an AttributeError and lead to a misleading generic error.

    get_codeflash_api_key.cache_clear()
    get_user_id.cache_clear()
    Optimizer initialization

    _initialize_optimizer_if_valid uses server.args assuming it's set by prepare_optimizer_arguments, but if omitted, server.args could be None and cause failures when instantiating the optimizer.

    def _initialize_optimizer_if_valid(server: CodeflashLanguageServer) -> dict[str, str]:
        user_id = get_user_id()
        if user_id is None:
            return {"status": "error", "message": "api key not found or invalid"}
    
        from codeflash.optimization.optimizer import Optimizer
    
        server.optimizer = Optimizer(server.args)
        return {"status": "success", "user_id": user_id}
    Version check fallback

    In quiet mode, an outdated CLI version logs and returns None instead of exiting; ensure callers of get_user_id handle None correctly in the LSP flow.

    if console.quiet:  # lsp
        logger.debug(msg)
        return None
    sys.exit(1)

    Copy link

    github-actions bot commented Jul 9, 2025

    PR Code Suggestions ✨

    Latest suggestions up to a0ff82f
    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Guard cache_clear call

    Wrap the get_user_id.cache_clear() call in a guard to avoid an AttributeError when
    the function isn't cached. This ensures it only runs if the method exists.

    codeflash/lsp/beta.py [160]

    -get_user_id.cache_clear()
    +if hasattr(get_user_id, "cache_clear"):
    +    get_user_id.cache_clear()
    Suggestion importance[1-10]: 9

    __

    Why: The call to get_user_id.cache_clear() will raise an AttributeError if get_user_id isn't decorated with a cache, so guarding it prevents a runtime crash.

    High
    Security
    Escape quotes in export

    Escape any double quotes in the api_key before embedding it into the export line to
    avoid syntax errors or injection issues in shell RC files.

    codeflash/code_utils/shell_utils.py [45]

    -return f'{SHELL_RC_EXPORT_PREFIX}"{api_key}"'
    +sanitized_key = api_key.replace('"', '\\"')
    +return f'{SHELL_RC_EXPORT_PREFIX}"{sanitized_key}"'
    Suggestion importance[1-10]: 5

    __

    Why: Sanitizing api_key by escaping quotes prevents malformed export lines or injection issues in shell RC files, improving security with minimal overhead.

    Low

    Previous suggestions

    Suggestions up to commit 38e4e02
    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Use correct server instance

    Use the passed-in _server parameter rather than the module-level server so you
    configure the correct LanguageServer instance.

    codeflash/lsp/beta.py [137]

    -server.optimizer = Optimizer(server.args)
    +_server.optimizer = Optimizer(_server.args)
    Suggestion importance[1-10]: 9

    __

    Why: Using the global server instead of the _server parameter will configure the wrong LanguageServer instance and break per-request state handling.

    High
    General
    Log validation exceptions

    Log the exception details before returning to aid debugging of unexpected failures
    during API key validation.

    codeflash/lsp/beta.py [139-140]

    -except Exception:
    +except Exception as e:
    +    logger.exception("Error validating API key")
         return {"status": "error", "message": "something went wrong while validating the api key"}
    Suggestion importance[1-10]: 5

    __

    Why: Adding exception logging improves debuggability for API key validation failures, though it doesn't alter core functionality.

    Low
    Log saving exceptions

    Capture and log the exception so any issues during saving the API key are recorded
    for troubleshooting.

    codeflash/lsp/beta.py [158-159]

    -except Exception:
    +except Exception as e:
    +    logger.exception("Error saving API key")
         return {"status": "error", "message": "something went wrong while saving the api key"}
    Suggestion importance[1-10]: 5

    __

    Why: Capturing and logging errors when saving the API key aids troubleshooting, but it's a minor enhancement rather than a critical fix.

    Low
    Consolidate optimizer import

    Move the Optimizer import to the module top-level and remove duplicate inline
    imports to reduce redundancy and improve performance.

    codeflash/lsp/beta.py [135-154]

    +# At the top of the module
     from codeflash.optimization.optimizer import Optimizer
     
    -# ... repeated inside each feature handler ...
    -from codeflash.optimization.optimizer import Optimizer
    +# Inside feature handlers, remove inline imports
    Suggestion importance[1-10]: 4

    __

    Why: Moving Optimizer imports to the module top-level reduces redundancy and slightly improves performance, but it's a low-impact refactor.

    Low

    @mohammedahmed18 mohammedahmed18 marked this pull request as ready for review July 11, 2025 18:43
    Copy link

    Persistent review updated to latest commit a0ff82f

    @mohammedahmed18 mohammedahmed18 requested a review from KRRT7 July 13, 2025 10:21
    @misrasaurabh1 misrasaurabh1 requested a review from Saga4 July 14, 2025 20:54
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant