OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

Fix: Preserve API key when submitting from settings modal with asterisks

Open rbren opened this issue 8 months ago • 5 comments

This PR fixes an issue where the settings modal was setting the LLM API key field to asterisks instead of preserving the actual key when submitting the form.


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:1e79564-nikolaik   --name openhands-app-1e79564   docker.all-hands.dev/all-hands-ai/openhands:1e79564

rbren avatar Apr 01 '25 16:04 rbren

@openhands fix the lint errors in frontend

rbren avatar Apr 01 '25 16:04 rbren

I'm on it! @rbren can track my progress at all-hands.dev

openhands-ai[bot] avatar Apr 01 '25 16:04 openhands-ai[bot]

I successfully fixed the lint errors in the frontend code as requested in the PR comment.

Here's a summary of what I did:

  1. First, I explored the repository structure and identified the PR (#7626) that needed lint fixes.

  2. I examined the PR changes, which included:

    • A new test file: frontend/__tests__/components/shared/modals/settings/settings-api-key-handling.test.tsx
    • Modifications to: frontend/src/components/shared/modals/settings/settings-form.tsx
  3. I installed the necessary dependencies (ESLint and Prettier) to run the linting tools.

  4. I ran the lint and formatting commands:

    • npm run lint:fix to automatically fix linting issues
    • npm run build to verify the build process works correctly
  5. I verified that all lint errors were fixed by running additional checks on the specific files.

  6. I committed the changes with the message "Fix lint errors in frontend" and pushed them to the PR branch.

The build process completed successfully with no lint errors, indicating that the request has been addressed successfully. The PR should now pass any CI checks related to frontend linting.

Note: There was a warning about object rest destructuring in use-settings.ts, but this was unrelated to the PR changes and is just a warning, not an error.

openhands-ai[bot] avatar Apr 01 '25 16:04 openhands-ai[bot]

Test

development.md build from rev 1e7956409a151445ff08ccfb24143e6941572c04

I open the webapp. I do not get the prompt-dialog to enter my api key. I guess it most have remembered from my previous session.

I go to settings and change the agent a few times. I save. I start new conversation. Agent works. ✅ I go back to settings and enter an invalid key on purpose. I go back to the consersation and try to continue it.

Error authenticating with the LLM provider. Please check your API key Expected.

Looks good. 👍

happyherp avatar Apr 01 '25 19:04 happyherp

@rbren this doesn't work. When I set the LLM and API Key in the initial popup, it doesn't do anything. It doesn't even create the settings.json file.

mamoodi avatar Apr 01 '25 19:04 mamoodi

I open the webapp. I do not get the prompt-dialog to enter my api key. I guess it most have remembered from my previous session.

Yes, I bet you'd have to delete/move settings.json from wherever it's stored on your machine, so that it finds nothing when starting the app. 🤔

enyst avatar Apr 02 '25 14:04 enyst