Add project-scoped dotenv sync for new worktrees#767
Add project-scoped dotenv sync for new worktrees#767Snowy7 wants to merge 2 commits intopingdotgg:mainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
the
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function collapseRelativeSegments(value: string): string | null { | ||
| const normalized = normalizeSeparators(value); | ||
| const segments = normalized.split("/"); | ||
| const resolved: string[] = []; | ||
|
|
||
| for (const rawSegment of segments) { | ||
| const segment = rawSegment.trim(); | ||
| if (segment.length === 0 || segment === ".") { | ||
| continue; | ||
| } | ||
| if (segment === "..") { | ||
| return null; | ||
| } | ||
| resolved.push(segment); | ||
| } |
There was a problem hiding this comment.
🟢 Low src/dotenvSync.ts:8
collapseRelativeSegments returns null for any path containing .., so valid paths like config/../.env are incorrectly rejected even though they resolve safely to .env. Consider popping the last segment when encountering .. and only returning null when the stack is empty, indicating traversal above the project root.
for (const rawSegment of segments) {
const segment = rawSegment.trim();
if (segment.length === 0 || segment === ".") {
continue;
}
if (segment === "..") {
- return null;
+ if (resolved.length === 0) {
+ return null;
+ }
+ resolved.pop();
+ continue;
}
resolved.push(segment);
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/shared/src/dotenvSync.ts around lines 8-22:
`collapseRelativeSegments` returns `null` for any path containing `..`, so valid paths like `config/../.env` are incorrectly rejected even though they resolve safely to `.env`. Consider popping the last segment when encountering `..` and only returning `null` when the stack is empty, indicating traversal above the project root.
Evidence trail:
packages/shared/src/dotenvSync.ts lines 8-24 at REVIEWED_COMMIT - the `collapseRelativeSegments` function, specifically lines 18-19 show `if (segment === "..") { return null; }` which immediately returns null for ANY path containing `..` without attempting to resolve it.
| for (const rawSegment of segments) { | ||
| const segment = rawSegment.trim(); | ||
| if (segment.length === 0 || segment === ".") { | ||
| continue; | ||
| } | ||
| if (segment === "..") { | ||
| return null; | ||
| } | ||
| resolved.push(segment); | ||
| } |
There was a problem hiding this comment.
🟢 Low src/dotenvSync.ts:13
collapseRelativeSegments calls trim() on each path segment, so directories or files with leading or trailing whitespace (e.g., " images ") are silently corrupted to "images". The function returns an incorrect path that does not match the actual filesystem entry.
for (const rawSegment of segments) {
- const segment = rawSegment.trim();
- if (segment.length === 0 || segment === ".") {
+ if (rawSegment === ".") {
continue;
}
- if (segment === "..") {
+ if (rawSegment === "..") {
return null;
}
- resolved.push(segment);
+ resolved.push(rawSegment);
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/shared/src/dotenvSync.ts around lines 13-22:
`collapseRelativeSegments` calls `trim()` on each path segment, so directories or files with leading or trailing whitespace (e.g., `" images "`) are silently corrupted to `"images"`. The function returns an incorrect path that does not match the actual filesystem entry.
Evidence trail:
packages/shared/src/dotenvSync.ts lines 8-23 at REVIEWED_COMMIT - the `collapseRelativeSegments` function shows `const segment = rawSegment.trim();` on line 13, confirming that each path segment is trimmed, which would corrupt paths containing directories or files with leading/trailing whitespace.
There was a problem hiding this comment.
Pull request overview
Adds a project-scoped “dotenv sync” feature to automatically copy selected .env* files from the main project root into newly created worktrees (and optionally sync on demand), with persistence and server-side support.
Changes:
- Introduces shared normalization/validation utilities and new contract types/RPC methods for dotenv discovery and syncing.
- Adds web UI (Actions → Dotenv sync…) with allowlist management + auto-detect of
.env*candidates. - Implements server-side dotenv scanning, worktree copy service, and persistence/migrations for project config.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/dotenvSync.ts | Adds shared path normalization/validation utilities for dotenv sync paths. |
| packages/shared/package.json | Exposes ./dotenvSync export for shared utilities consumption. |
| packages/contracts/src/ws.ts | Adds WS method names + request schema tags for dotenv listing and worktree syncing. |
| packages/contracts/src/project.ts | Adds projects.listDotenvEntries input/result schemas. |
| packages/contracts/src/orchestration.ts | Adds ProjectDotenvSyncConfig persisted on projects with schema-level path validation. |
| packages/contracts/src/ipc.ts | Extends NativeApi to include dotenv listing and sync methods. |
| packages/contracts/src/git.ts | Adds git RPC schemas for syncing dotenv files into worktrees. |
| apps/web/src/wsNativeApi.ts | Wires new WS requests into the web Native API client. |
| apps/web/src/types.ts | Extends Project type to include dotenvSync config. |
| apps/web/src/store.ts | Maps read model projects to include dotenvSync state. |
| apps/web/src/store.test.ts | Updates store tests for new dotenvSync field. |
| apps/web/src/components/ProjectScriptsControl.tsx | Adds menu entry + dialog plumbing for dotenv sync under Actions. |
| apps/web/src/components/ProjectDotenvSyncDialog.tsx | New UI dialog to manage allowlist, detect candidates, and run manual sync. |
| apps/web/src/components/ChatView.tsx | Persists dotenv sync config, adds manual sync/detect actions, and syncs on worktree creation before setup/turn start. |
| apps/web/src/components/ChatView.browser.tsx | Updates browser snapshot fixtures for new project field. |
| apps/server/src/wsServer.ts | Adds websocket routing for dotenv listing and git dotenv sync. |
| apps/server/src/workspaceEntries.ts | Adds dotenv discovery scan + expands ignored directory handling (incl. .turn). |
| apps/server/src/workspaceEntries.test.ts | Adds tests for dotenv discovery behavior and ignored directories. |
| apps/server/src/serverLayers.ts | Registers the new WorktreeDotenvSync service layer. |
| apps/server/src/provider/Layers/ProviderService.ts | Minor refactor of upsertSessionBinding call shape. |
| apps/server/src/persistence/Services/ProjectionProjects.ts | Extends projection project schema to include dotenvSync. |
| apps/server/src/persistence/Migrations/014_ProjectionProjectsDotenvSync.ts | Adds DB column for persisted dotenv sync config JSON. |
| apps/server/src/persistence/Migrations.ts | Registers the new migration. |
| apps/server/src/persistence/Layers/ProjectionProjects.ts | Reads/writes dotenv_sync_json and decodes/encodes config JSON. |
| apps/server/src/orchestration/projector.ts | Projects dotenvSync into the read model on create/update events. |
| apps/server/src/orchestration/decider.ts | Includes dotenvSync in project creation and meta update decisions. |
| apps/server/src/orchestration/decider.projectScripts.test.ts | Updates decider tests to include dotenvSync. |
| apps/server/src/orchestration/commandInvariants.test.ts | Updates invariants tests to include dotenvSync. |
| apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts | Loads dotenv_sync_json into snapshot read model. |
| apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.test.ts | Updates snapshot query tests for new field. |
| apps/server/src/orchestration/Layers/ProjectionPipeline.ts | Projects dotenvSync through the projection pipeline. |
| apps/server/src/git/Services/WorktreeDotenvSync.ts | New service interface for syncing dotenv files into worktrees. |
| apps/server/src/git/Layers/WorktreeDotenvSync.ts | New implementation that validates paths and copies files into worktrees. |
| apps/server/src/git/Errors.ts | Adds WorktreeDotenvSyncError error type. |
| apps/server/src/checkpointing/Layers/CheckpointDiffQuery.test.ts | Updates checkpoint snapshot test data for new project field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const contents = yield* fileSystem.readFile(plan.sourcePath).pipe( | ||
| Effect.mapError((cause) => | ||
| toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Failed to read dotenv source file: ${plan.relativePath}`, | ||
| cause, | ||
| ), | ||
| ), | ||
| ); | ||
| yield* fileSystem.writeFile(plan.targetPath, contents).pipe( | ||
| Effect.mapError((cause) => | ||
| toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Failed to copy dotenv file into worktree: ${plan.relativePath}`, | ||
| cause, | ||
| ), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
When copying dotenv files into the worktree, the target file permissions aren’t preserved. This can accidentally make secrets world/group readable depending on the process umask (e.g., a 0600 source becoming 0644). Preserve the source mode (or explicitly chmod to a restrictive default) after writing, and consider using a copy primitive that keeps metadata where available.
| const syncFiles: WorktreeDotenvSyncShape["syncFiles"] = (input) => | ||
| Effect.gen(function* () { | ||
| const normalizedPathsResult = normalizeDotenvSyncPaths(input.paths); | ||
| if (normalizedPathsResult.error) { | ||
| return yield* toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| normalizedPathsResult.error, | ||
| ); | ||
| } | ||
|
|
||
| const sourceRoot = path.resolve(input.cwd); | ||
| const worktreeRoot = path.resolve(input.worktreePath); | ||
| const plans = yield* Effect.forEach( | ||
| normalizedPathsResult.normalizedPaths, | ||
| (relativePath) => | ||
| Effect.gen(function* () { | ||
| const sourcePath = path.resolve(sourceRoot, relativePath); | ||
| const targetPath = path.resolve(worktreeRoot, relativePath); | ||
| const sourceRelative = path.relative(sourceRoot, sourcePath).replaceAll("\\", "/"); | ||
| const targetRelative = path.relative(worktreeRoot, targetPath).replaceAll("\\", "/"); | ||
| if ( | ||
| sourceRelative.startsWith("../") || | ||
| sourceRelative === ".." || | ||
| targetRelative.startsWith("../") || | ||
| targetRelative === ".." | ||
| ) { | ||
| return yield* toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Resolved dotenv path escapes the workspace: ${relativePath}`, | ||
| ); | ||
| } | ||
|
|
||
| const sourceInfo = yield* fileSystem.stat(sourcePath).pipe( | ||
| Effect.mapError((cause) => | ||
| toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Dotenv source file does not exist: ${relativePath}`, | ||
| cause, | ||
| ), | ||
| ), | ||
| ); | ||
| if (sourceInfo.type !== "File") { | ||
| return yield* toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Dotenv source is not a file: ${relativePath}`, | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| relativePath, | ||
| sourcePath, | ||
| targetPath, | ||
| }; | ||
| }), | ||
| { concurrency: 1 }, | ||
| ); | ||
|
|
||
| yield* Effect.forEach( | ||
| plans, | ||
| (plan) => | ||
| Effect.gen(function* () { | ||
| yield* fileSystem.makeDirectory(path.dirname(plan.targetPath), { recursive: true }).pipe( | ||
| Effect.mapError((cause) => | ||
| toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Failed to prepare worktree path for ${plan.relativePath}`, | ||
| cause, | ||
| ), | ||
| ), | ||
| ); | ||
| const contents = yield* fileSystem.readFile(plan.sourcePath).pipe( | ||
| Effect.mapError((cause) => | ||
| toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Failed to read dotenv source file: ${plan.relativePath}`, | ||
| cause, | ||
| ), | ||
| ), | ||
| ); | ||
| yield* fileSystem.writeFile(plan.targetPath, contents).pipe( | ||
| Effect.mapError((cause) => | ||
| toWorktreeDotenvSyncError( | ||
| "WorktreeDotenvSync.syncFiles", | ||
| `Failed to copy dotenv file into worktree: ${plan.relativePath}`, | ||
| cause, | ||
| ), | ||
| ), | ||
| ); | ||
| }), | ||
| { concurrency: 1 }, | ||
| ); | ||
|
|
||
| return { | ||
| copiedPaths: normalizedPathsResult.normalizedPaths, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
This introduces new server-side behavior (validating paths + copying files into worktrees) but there are no tests covering WorktreeDotenvSync. Given existing vitest coverage for other git layers, add tests for: successful copy (including nested dirs), overwrite behavior, missing source file error, and rejecting paths that escape the root/aren’t .env*.
| const PROJECT_DOTENV_SYNC_MAX_PATHS = 16; | ||
| const PROJECT_DOTENV_SYNC_PATH_MAX_LENGTH = 512; | ||
|
|
||
| function normalizeDotenvSyncPathForSchema(value: string): string | null { | ||
| const trimmed = value.trim(); | ||
| if (trimmed.length === 0 || trimmed.length > PROJECT_DOTENV_SYNC_PATH_MAX_LENGTH) { | ||
| return null; | ||
| } | ||
|
|
||
| const normalized = trimmed.replaceAll("\\", "/"); | ||
| if (normalized.startsWith("/") || normalized.startsWith("//") || /^[a-zA-Z]:\//.test(normalized)) { | ||
| return null; | ||
| } | ||
|
|
||
| const resolved: string[] = []; | ||
| for (const rawSegment of normalized.split("/")) { | ||
| const segment = rawSegment.trim(); | ||
| if (segment.length === 0 || segment === ".") { | ||
| continue; | ||
| } | ||
| if (segment === "..") { | ||
| return null; | ||
| } | ||
| resolved.push(segment); | ||
| } | ||
|
|
||
| const collapsed = resolved.join("/"); | ||
| if (collapsed.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| const fileName = collapsed.split("/").at(-1) ?? ""; | ||
| if (!fileName.startsWith(".env")) { | ||
| return null; | ||
| } | ||
|
|
||
| return collapsed; | ||
| } |
There was a problem hiding this comment.
Path normalization/validation logic is duplicated here and in @t3tools/shared/dotenvSync (and is also re-checked server-side). Keeping multiple implementations in sync is error-prone and can lead to client/server disagreeing on what’s valid. Consider consolidating into a single shared normalizer (e.g., export a canonical normalize function from contracts or a small shared package) and reuse it for both schema checks and runtime validation.
|
this can easily be done in userland by project script and toggling the "run automatically on worktree creation". don't see an obvious reason why we need to have this flow natively so closing |
What Changed
Actionscontrol.env*pathsthread.turn.startSync nowaction for the active worktreeWhy
Note: I am aware this PR might not align with the contribution guidelines of having small changes since this is a 1k line PR, but it solves an issue within the scope that I think is worth noting. Feel free to close this PR if it is not needed, or re implement the issue in a better way.
Closes #764.
New worktrees currently do not get local dotenv files from the main checkout. That makes setup commands and first-run flows fail in projects that depend on
.env,.env.local, or nested app-level dotenv files.This keeps the behavior explicit and project-scoped instead of turning it into a generic file-copy feature.
I am opening this as a draft because it is larger than your contribution guidelines prefer, and I would rather get direction than pretend it is already in the right shape.
UI Changes
Actions -> Dotenv sync...Screen.Recording.2026-03-10.045225.mp4
Checklist