fix: allow editing .env files for GitOps-managed projects (#1748)#1853
fix: allow editing .env files for GitOps-managed projects (#1748)#1853cougz wants to merge 1 commit intogetarcaneapp:mainfrom
Conversation
…pp#1748) Restores original design intent where .env files can be edited in Arcane for environment-specific runtime configuration, even for projects synced from Git. Changes: - Removed canEditEnv derived variable that blocked .env editing for GitOps projects - Removed readOnly={!canEditEnv} from all .env CodePanel components (4 locations) - Removed test that incorrectly expected read-only env editor
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| bind:value={$inputs.envContent.value} | ||
| error={$inputs.envContent.error ?? undefined} | ||
| readOnly={!canEditEnv} | ||
There was a problem hiding this comment.
Line contains only a space character - should be removed entirely
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/(app)/projects/[projectId]/+page.svelte
Line: 546
Comment:
Line contains only a space character - should be removed entirely
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.| <CodePanel | ||
| bind:open={envOpen} | ||
| title=".env" | ||
| language="env" | ||
| bind:value={$inputs.envContent.value} | ||
| error={$inputs.envContent.error ?? undefined} | ||
| /> |
There was a problem hiding this comment.
Indentation is inconsistent - <CodePanel should have 11 tabs to match other CodePanel instances and its props
| <CodePanel | |
| bind:open={envOpen} | |
| title=".env" | |
| language="env" | |
| bind:value={$inputs.envContent.value} | |
| error={$inputs.envContent.error ?? undefined} | |
| /> | |
| <CodePanel | |
| bind:open={envOpen} | |
| title=".env" | |
| language="env" | |
| bind:value={$inputs.envContent.value} | |
| error={$inputs.envContent.error ?? undefined} | |
| /> |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/(app)/projects/[projectId]/+page.svelte
Line: 635-641
Comment:
Indentation is inconsistent - `<CodePanel` should have 11 tabs to match other CodePanel instances and its props
```suggestion
<CodePanel
bind:open={envOpen}
title=".env"
language="env"
bind:value={$inputs.envContent.value}
error={$inputs.envContent.error ?? undefined}
/>
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| bind:value={$inputs.envContent.value} | ||
| error={$inputs.envContent.error ?? undefined} | ||
| readOnly={!canEditEnv} | ||
There was a problem hiding this comment.
Line contains only a space character - should be removed entirely
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/(app)/projects/[projectId]/+page.svelte
Line: 705
Comment:
Line contains only a space character - should be removed entirely
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.| <CodePanel | ||
| bind:open={envOpen} | ||
| title=".env" | ||
| language="env" | ||
| bind:value={$inputs.envContent.value} | ||
| error={$inputs.envContent.error ?? undefined} | ||
| /> |
There was a problem hiding this comment.
Indentation is inconsistent - <CodePanel should have 11 tabs to match other CodePanel instances and its props
| <CodePanel | |
| bind:open={envOpen} | |
| title=".env" | |
| language="env" | |
| bind:value={$inputs.envContent.value} | |
| error={$inputs.envContent.error ?? undefined} | |
| /> | |
| <CodePanel | |
| bind:open={envOpen} | |
| title=".env" | |
| language="env" | |
| bind:value={$inputs.envContent.value} | |
| error={$inputs.envContent.error ?? undefined} | |
| /> |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/(app)/projects/[projectId]/+page.svelte
Line: 736-742
Comment:
Indentation is inconsistent - `<CodePanel` should have 11 tabs to match other CodePanel instances and its props
```suggestion
<CodePanel
bind:open={envOpen}
title=".env"
language="env"
bind:value={$inputs.envContent.value}
error={$inputs.envContent.error ?? undefined}
/>
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
|
Making it read only was my idea, as if its synced editing it would get overridden on the next sync. How would you propose solving that issue? |
Most people don't sync .env on their git repositories, but if people have .env on their .git then they should be on their own since you can't exclude them from a git pull. Generally people only .env.example or etc and those who want full sync will have it |
So there are two Use Cases we need to handle:
The backend sync logic (updateProjectForSyncInternal) already handles both cases correctly - this PR just removes the blocking restriction that prevented all GitOps users from editing. Do you see an issue here? Obviously, we could add a visual indicator in the UI showing that .env is Git-synced or implement a slider, but that's not really required for this PR. |
|
I'll try to fix it/build off of this branch to do it. If .env files are in a private git site hosted locally I see no issue with people syncing them, that only becomes a issue when people are pushing env files to public git sites imho. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
| expect(isReadOnly).toBe(true); | ||
| }); |
There was a problem hiding this comment.
Missing test for new expected behavior
The test verifying that the env editor is read-only for GitOps-managed projects was removed, but no replacement test was added to verify the new expected behavior — that the .env editor is editable for GitOps-managed projects.
Without this test, a future regression (e.g. accidentally reintroducing readOnly on the env CodePanel) could go undetected. Consider adding a counterpart test like:
test("should have env editor in editable mode when GitOps managed", async ({ page }) => {
const gitOpsProject = realProjects.find((p) => p.gitOpsManagedBy);
test.skip(!gitOpsProject, "No GitOps-managed projects found");
await page.goto(`/projects/${gitOpsProject!.id}`);
await page.waitForLoadState("networkidle");
const configTab = page.getByRole("tab", { name: /Configuration|Config/i });
await configTab.click();
await page.waitForLoadState("networkidle");
await page.waitForTimeout(1000);
const isReadOnly = await page.evaluate(() => {
const editors = (window as any).monaco?.editor?.getEditors() ?? [];
const envEditor = editors.find((e: any) => {
const model = e.getModel();
return model && model.getLanguageId() === "ini";
});
if (envEditor) {
return envEditor.getOption((window as any).monaco.editor.EditorOption.readOnly);
}
return null;
});
expect(isReadOnly).toBe(false);
});Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/spec/project.spec.ts
Line: 411-412
Comment:
**Missing test for new expected behavior**
The test verifying that the env editor is read-only for GitOps-managed projects was removed, but no replacement test was added to verify the new expected behavior — that the `.env` editor **is** editable for GitOps-managed projects.
Without this test, a future regression (e.g. accidentally reintroducing `readOnly` on the env `CodePanel`) could go undetected. Consider adding a counterpart test like:
```ts
test("should have env editor in editable mode when GitOps managed", async ({ page }) => {
const gitOpsProject = realProjects.find((p) => p.gitOpsManagedBy);
test.skip(!gitOpsProject, "No GitOps-managed projects found");
await page.goto(`/projects/${gitOpsProject!.id}`);
await page.waitForLoadState("networkidle");
const configTab = page.getByRole("tab", { name: /Configuration|Config/i });
await configTab.click();
await page.waitForLoadState("networkidle");
await page.waitForTimeout(1000);
const isReadOnly = await page.evaluate(() => {
const editors = (window as any).monaco?.editor?.getEditors() ?? [];
const envEditor = editors.find((e: any) => {
const model = e.getModel();
return model && model.getLanguageId() === "ini";
});
if (envEditor) {
return envEditor.getOption((window as any).monaco.editor.EditorOption.readOnly);
}
return null;
});
expect(isReadOnly).toBe(false);
});
```
How can I resolve this? If you propose a fix, please make it concise.3 participants
Summary
Fixes #1748 - Allow editing .env files for GitOps-managed projects
Restores original design intent where .env files can be edited in Arcane for environment-specific runtime configuration, even for projects synced from Git.
Related Issue
Fixes #1748
Background
The bug was introduced in PR #1632 (commit 7f05d17) when adding .env sync capability from Git repositories. The developer made .env editor read-only "for consistency" with compose editor, but this violated original design intent established in PR #1089 and PR #1471.
Key facts:
Changes
How Both Scenarios Work After Fix
The backend sync logic already handles both cases correctly:
Scenario 1 (.env in Git): Users CAN edit, but Git sync overwrites changes (Git is source of truth)
Scenario 2 (.env not in Git): Users CAN edit, and changes are preserved (local customization)
Testing
Technical Details
Frontend (frontend/src/routes/(app)/projects/[projectId]/+page.svelte):
Tests (tests/spec/project.spec.ts):
Verification Steps
To complete testing after merging:
Start dev environment
Run type checking
Manual testing
TEST_VAR=test_value)Disclaimer Greptiles Reviews use AI, make sure to check over its work.
To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike
Greptile Summary
This PR restores the original design intent that
.envfiles should be editable in Arcane even for GitOps-managed projects, fixing a regression introduced in PR #1632. The change is well-reasoned and consistent with the existing backend behavior and UI messaging.Key changes:
canEditEnv = $derived(!isGitOpsManaged)variable from+page.svelte(the only meaningful logic change)readOnly={!canEditEnv}from all four.envCodePanelinstances across the classic and tree layout viewsenvContentto the backend for GitOps projects while skippingnamePayloadandcomposePayload(lines 193–198), so the backend integration requires no changesgit_managed_env_notemessage informing users that the.envfile can be edited — the UI and code are now consistentConfidence Score: 4/5
tests/spec/project.spec.ts.Important Files Changed
canEditEnvderived variable and itsreadOnlybindings from all four.envCodePanel instances, allowing GitOps-managed projects to edit their env files. The save logic already correctly sendsenvContentto the backend regardless of GitOps status (lines 192–198). The UI already shows a note informing users the env file can be edited. Minor trailing whitespace left at lines 546 and 705 (previously flagged).Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[User on GitOps Project\nConfiguration Tab] --> B{isGitOpsManaged?} B -- Yes --> C[compose.yaml CodePanel\nreadOnly=true\nvia canEditCompose] B -- Yes --> D[.env CodePanel\nreadOnly=false\neditable ✅] B -- No --> E[compose.yaml CodePanel\nreadOnly=false] B -- No --> F[.env CodePanel\nreadOnly=false] D --> G[User edits .env\nhasChanges = true] G --> H[handleSaveChanges] H --> I{isGitOpsManaged?} I -- Yes --> J[namePayload = undefined\ncomposePayload = undefined\nenvContent = edited value] I -- No --> K[namePayload = name\ncomposePayload = compose\nenvContent = edited value] J --> L[projectService.updateProject] K --> LLast reviewed commit: 35ffb50