Skip to content

fetch secrets from <workspace>/.continue/.env folder before <workspace>/.env #5461

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

Merged
merged 7 commits into from
Jun 2, 2025

Conversation

uinstinct
Copy link
Contributor

@uinstinct uinstinct commented May 1, 2025

Description

Fetch secrets from /.continue/.env file before fetching from workspace .env

So now the order of fetching the first env file is ~/.continue/.env, <workspace>/.continue/.env, <workspace>/.env

https://discord.com/channels/1108621136150929458/1355378099004379248/1367363239792476211

Checklist

  • [] I've read the contributing guide
  • [] The relevant docs, if any, have been updated or created
  • [] The relevant tests, if any, have been updated or created

Screenshots

[ For visual changes, include screenshots. Screen recordings are particularly helpful, and appreciated! ]

Testing instructions

  1. Create a .env file inside the .continue directory
  2. Add key value pairs
  3. Use the secrets in config yaml
  4. See them reflected via config

@uinstinct uinstinct requested a review from a team as a code owner May 1, 2025 16:23
@uinstinct uinstinct requested review from tomasz-stefaniak and removed request for a team May 1, 2025 16:23
Copy link

netlify bot commented May 1, 2025

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit 610814e
🔍 Latest deploy log https://app.netlify.com/projects/continuedev/deploys/683af26e5440b20008ef6355

@tomasz-stefaniak
Copy link
Collaborator

@uinstinct could you add tests for this as documentation and to prevent future regressions?

Comment on lines 5 to 8
jest.mock("../../util/paths.ts", () => ({
...jest.requireActual("../../util/paths"),
getContinueDotEnv: jest.fn(),
}));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hey @tomasz-stefaniak

there is some problem when am trying to mock a local file as jest is unable to resolve it. it seems like a problem with how jest is setup. I tried various ways to get it working. Currently none of these work.

Any idea what should I do to a mock?

(I need to mock utils/paths for imported functions like getContinueDotEnv())

Copy link
Collaborator

Choose a reason for hiding this comment

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

@uinstinct have you checked some existing instances of jest.mock we have in the codebase?
image

I understand most of them are for node packages but maybe they can point you in the right direction. If you are still stuck, let me know and I can try to pull this into the sprint next week.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for replying Tomasz!
I had tried these actually but for some reason jest is unable to resolve it.

For alternative options, we can:

  1. use the filesystem env (problem: the test might affect the local users .env)
  2. use a conditional (process.env.TEST flag) inside getContinueDotEnv to produce a key value pair for testing

Copy link
Contributor Author

@uinstinct uinstinct May 11, 2025

Choose a reason for hiding this comment

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

Spent some more time debugging this. Looks like the module resolution was starting from core/test folder since jest was declared as global in core/test/jest.setup-after-env.js

So, ../util/paths would work but not ../../util/paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. Tried both unstable_mockModule and jest.mock('../util/paths') both which resolve correctly but do not mock implementations for some reason.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Update: this is a huge pain, I'm exploring migrating to Vitest 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🥲 Agree!!!
BTW, can i help in some way? I have some experience with migrating jest to vitest.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That would be great, actually! Do you think you'd be able to create a basic vitest setup? It could be on this branch, and it would be used to run LocalPlatformClient.test.ts. Once this proof of concept works we can migrate other tests to vitest.

This would also require adding an extra step here: https://github.com/continuedev/continue/blob/main/.github/workflows/pr_checks.yaml

Let me know what you think!

Copy link
Contributor Author

@uinstinct uinstinct May 31, 2025

Choose a reason for hiding this comment

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

implemented!

  • vitest works nicely (and with mocks) 🥳
  • added vitest running for LocalPlatformClient only in pr_check.yaml

Edit: Should I draft another PR to migrate core tests to vitest?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Edit: Should I draft another PR to migrate core tests to vitest?

Yeah, that would be great. It's one of our more important tasks at the moment.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 31, 2025
Copy link
Collaborator

@tomasz-stefaniak tomasz-stefaniak left a comment

Choose a reason for hiding this comment

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

Thanks! Great job and thanks for introducing vitest.

Edit: Should I draft another PR to migrate core tests to vitest?

I'd love that, it's very high on our priority list.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 2, 2025
@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs Jun 2, 2025
@tomasz-stefaniak tomasz-stefaniak merged commit ceb2e52 into continuedev:main Jun 2, 2025
33 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs Jun 2, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants