fix: preserve .env overrides during git directory sync#3095
Merged
Conversation
Contributor
|
Container images for this PR have been built successfully!
Built from commit 23eb493 |
66131b1 to
2484b55
Compare
2484b55 to
23eb493
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Checklist
mainbranchm.*())What This PR Implements
Fixes: #2476
Changes Made
Testing Done
AI Tool Used (if applicable)
Additional Context
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
To have Greptile Re-Review the changes, mention
greptileai.Greptile Summary
This PR fixes a bug (issue #2476) where
.envoverrides edited in Arcane were silently wiped on every git directory sync because.envwas being raw-written from the git payload instead of being routed through the three-file override merge (gitEnvContent→.env.git,project.env,.env).gitops_sync_service.go):partitionReservedRootEnvFilesInternalsplits the git payload so.envis captured asgitEnvContent(not written directly), thenapplyGitSyncEnvToDirInternalruns the same three-file merge that single-file sync uses. A companionfilterReservedRootEnvFilesInternaldrops.envfrom the old tracked-files list soCleanupRemovedFilesdoes not treat it as "removed by git" and delete the live-copied stage.envbefore the merge step reads it (addressing the migration case).env.go,project_service.go):readOptionalProjectFileInternalnow returns anunreadableflag foros.ErrPermission;WriteManagedEnvFileskips writes to locked files rather than aborting the sync.project_service.go,load.go):validateComposeContentForUpdategains alenient boolparameter; git sync paths now tolerate undefined${VAR}references that would previously fail with typed-decode errors during validation.Confidence Score: 5/5
Safe to merge — the change is well-scoped to the directory sync path, backward-compatible via the legacy .env filter, and covered by three new regression tests.
The two concerns flagged in the previous review are both addressed: legacy sync.SyncedFiles entries containing .env are filtered before CleanupRemovedFiles sees them, and reserved-file drops from the git payload now emit slog.Warn entries. No new correctness issues were found after tracing the staging flow, the envContentChangedInternal signal, and the permission-locked skip path end-to-end.
No files require special attention.
Comments Outside Diff (2)
backend/internal/services/gitops_sync_service.go, line 87-107 (link)partitionReservedRootEnvFilesInternaldrops.env.git,project.env, and.env.globalfrom the git payload without emitting any log entry. Every other "skip" in this PR (permission-locked reads inreadOptionalProjectFileInternal,seedStageEnvFromCandidateDirInternal,WriteManagedEnvFile) logs aslog.Warn. A git repo that accidentally commits.env.gitorproject.env— or intentionally commits.env.global(which was previously raw-written) — will have those files silently dropped with no trace in the logs, making the behavior change invisible to operators.Prompt To Fix With AI
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!
backend/internal/services/gitops_sync_service.go, line 1711 (link)When an existing directory sync was created before this change,
sync.SyncedFilescontains.env; this call now compares it tosyncedFilesafter.envhas been filtered out, soCleanupRemovedFilesdeletes the copied live.envfrom the stage beforeapplyGitSyncEnvToDirInternalcan read it. With no prior.env.git/project.env, the merge falls back to the git.envand loses the Arcane-edited value during the first sync after upgrade.Rule Used: What: When implementing a feature, modify and reus... (source)
Prompt To Fix With AI
Reviews (3): Last reviewed commit: "fix: preserve .env overrides during git ..." | Re-trigger Greptile