diff --git a/apps/server/src/environment/ServerEnvironment.ts b/apps/server/src/environment/ServerEnvironment.ts index 7cf4f60bfd2a8..2ab2401b094da 100644 --- a/apps/server/src/environment/ServerEnvironment.ts +++ b/apps/server/src/environment/ServerEnvironment.ts @@ -244,6 +244,7 @@ export const make = Effect.gen(function* () { otlpEndpointCheck: true, projectWorktreeCleanup: true, settledWorktreeCleanup: true, + worktreeDiscardLocalChanges: true, worktreesDirectory: true, ...(hostPlatform === "darwin" ? { fastWorktreeCheckout: true } : {}), threadAutoSettlementScope: true, diff --git a/apps/server/src/policy/ManagedPolicy.test.ts b/apps/server/src/policy/ManagedPolicy.test.ts index d6c7710b57ead..5b845f937cfe2 100644 --- a/apps/server/src/policy/ManagedPolicy.test.ts +++ b/apps/server/src/policy/ManagedPolicy.test.ts @@ -167,6 +167,36 @@ it.layer(NodeServices.layer)("ManagedPolicy.load", (it) => { }), ); + it.effect("keeps a locked keep level from being bypassed by discarding local changes", () => + Effect.gen(function* () { + const policy = yield* loadFrom( + JSON.stringify({ + schemaVersion: 1, + organization: "Example Corp", + settings: { + "storageCleanup.worktreeKeepWhen": { value: "any-local-files", locked: true }, + worktreeCleanup: { value: null, locked: true }, + }, + }), + ); + const current = { + ...DEFAULT_SERVER_SETTINGS, + storageCleanup: { + ...DEFAULT_SERVER_SETTINGS.storageCleanup, + worktreeDiscardLocalChanges: true, + }, + }; + assert.isFalse( + policy.settings.applyLocks(current).storageCleanup.worktreeDiscardLocalChanges, + ); + const patch = { storageCleanup: { worktreeDiscardLocalChanges: true } }; + assert.deepEqual(policy.settings.vetWrite(DEFAULT_SERVER_SETTINGS, current, patch), { + _tag: "Locked", + path: "storageCleanup.worktreeDiscardLocalChanges", + }); + }), + ); + it.effect("tells clients the runtime mode ceiling", () => Effect.gen(function* () { const policy = yield* loadFrom( diff --git a/apps/server/src/policy/managedSettings.ts b/apps/server/src/policy/managedSettings.ts index 0d41d18ddce26..2727fdb4108f0 100644 --- a/apps/server/src/policy/managedSettings.ts +++ b/apps/server/src/policy/managedSettings.ts @@ -275,6 +275,13 @@ export function makeManagedSettings( } (setting.locked ? locks : defaults).push({ path, segments, value: setting.value }); } + // Fork: Never discards what a locked keep level protects, so that lock also pins Never off. + const discardPath = "storageCleanup.worktreeDiscardLocalChanges"; + if ( + locks.some((lock) => lock.path === "storageCleanup.worktreeKeepWhen") && + !locks.some((lock) => lock.path === discardPath) + ) + locks.push({ path: discardPath, segments: discardPath.split("."), value: false }); for (const lock of locks) { const unheld = OVERRIDDEN_BY.find( diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index de1ccfefcf64f..518c29e8e0805 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -501,10 +501,15 @@ const cleanupFixture = ( setSettings: (next: Partial) => { settings = { ...settings, ...next }; }, - setPolicy: (worktreeKeepWhen: WorktreeKeepWhen) => { + setPolicy: (policy: WorktreeKeepWhen | "never") => { settings = { ...settings, - storageCleanup: { ...settings.storageCleanup, worktreeKeepWhen }, + storageCleanup: { + ...settings.storageCleanup, + ...(policy === "never" + ? { worktreeDiscardLocalChanges: true } + : { worktreeKeepWhen: policy, worktreeDiscardLocalChanges: false }), + }, }; }, disable: () => { @@ -700,6 +705,23 @@ it.live("keeps nested submodule files protected by the current keep rule", () => ), ); +it.live("removes submodule worktrees with local files when nothing is kept", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree, submodule, child, command, setPolicy } = + yield* submoduleCleanupFixture(); + setPolicy("never"); + yield* fs.writeFileString(`${submodule}/tracked.txt`, "local edit\n"); + yield* command(submodule, ["submodule", "deinit", "child"]); + yield* fs.writeFileString(`${child}/notes.txt`, "notes after deinit\n"); + expect( + (yield* service.runNow).entries.find((entry) => entry.path === worktree), + ).toMatchObject({ outcome: "removed" }); + expect(yield* fs.exists(worktree)).toBe(false); + }), + ), +); + it.live("keeps unpushed submodule history after checkout and deinitialization", () => runCleanupTest( Effect.gen(function* () { @@ -2460,6 +2482,9 @@ describe("storage cleanup reports and local file policies", () => { ["any-local-files", "tracked.txt", "kept"], ["uncommitted-changes", "tracked.txt", "kept"], ["tracked-changes", "tracked.txt", "kept"], + ["never", ".env", "removed"], + ["never", "notes.txt", "removed"], + ["never", "tracked.txt", "removed"], ] as const)("applies %j", ([policy, file, outcome]) => runCleanupTest( Effect.gen(function* () { @@ -2776,6 +2801,49 @@ describe("storage cleanup reports and local file policies", () => { ); }); +describe("worktrees removed outside T3 Code", () => { + it.live.each([ + ["before-check", "git"], + ["before-check", "rm"], + ["while-measuring", "git"], + ["while-measuring", "rm"], + ] as const)("skips a worktree removed %s with %s", ([timing, method]) => + runCleanupTest( + Effect.gen(function* () { + const fixture = yield* pausedMeasurementFixture(); + const removeExternally = + method === "git" + ? fixture.command(fixture.repo, ["worktree", "remove", "--force", fixture.worktree]) + : fixture.fs.remove(fixture.worktree, { recursive: true }); + if (timing === "before-check") yield* removeExternally; + const run = yield* fixture.service.runNow.pipe(Effect.forkChild); + if (timing === "while-measuring") { + yield* Deferred.await(fixture.measuring); + yield* removeExternally; + } + yield* Deferred.succeed(fixture.resume, undefined); + const report = yield* Fiber.join(run); + expect(report.entries.filter((entry) => entry.kind === "worktree")).toEqual([]); + expect(report.counts.failed).toBe(0); + }), + ), + ); +}); + +it.live("keeps a worktree whose .git file another tool removed", () => + runCleanupTest( + Effect.gen(function* () { + const { service, fs, worktree } = yield* cleanupFixture(); + yield* fs.remove(`${worktree}/.git`); + expect((yield* service.runNow).entries[0]).toMatchObject({ + outcome: "kept", + reason: "Not a linked worktree", + }); + expect(yield* fs.exists(`${worktree}/tracked.txt`)).toBe(true); + }), + ), +); + describe("settled worktree rechecks", () => { it.live.each(["autoResumeLimitedThreads", "snoozeLimitedThreads"] as const)( "keeps a usage-limited checkout when %s is enabled during measurement", diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index c7e31d70e577b..a98a38af314e8 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -647,6 +647,7 @@ const make = Effect.gen(function* () { cwd: string, rules: WorktreeCleanupRules, ) { + if (rules.worktreeDiscardLocalChanges) return null; const status = yield* git.execute({ operation: "StorageCleanup.localChanges", cwd, @@ -757,7 +758,11 @@ const make = Effect.gen(function* () { if (changes !== null) return { force: false, reason: `submodule ${changes}` }; } const force = stored || paths.length > 0; - if (force && rules.worktreeKeepWhen !== "tracked-changes") { + if ( + force && + rules.worktreeKeepWhen !== "tracked-changes" && + !rules.worktreeDiscardLocalChanges + ) { const root = yield* fs.realPath(cwd); // Git status and foreach omit files inside deinitialized gitlink directories. const populatedPaths = new Set(paths.map(pathKey)); @@ -852,8 +857,14 @@ const make = Effect.gen(function* () { return "worktree is a symbolic link"; if (!roots.some((root) => inside(root, realPath))) return "outside the managed worktree folder"; - if ((yield* fs.stat(path.join(worktreePath, ".git"))).type !== "File") - return "not a linked worktree"; + const gitFile = yield* fs.stat(path.join(worktreePath, ".git")).pipe( + Effect.map((info) => info.type), + Effect.catchIf( + (error) => error.reason._tag === "NotFound", + () => Effect.succeed(null), + ), + ); + if (gitFile !== "File") return "not a linked worktree"; return null; }); const deletedRows = yield* sql<{ payload_json: string; workspaceRoot: string }>` @@ -925,6 +936,7 @@ const make = Effect.gen(function* () { if (deleted && settings.worktreeOnDelete) kept.push(worktreePath); entries.push({ ...entry, reason: sentence(reason) }); }; + let removing = false; // Reasons a provider session stop or finished deletion work can clear. const waitForStop = (reason: string) => { if (deleted && settings.worktreeOnDelete) waiting = true; @@ -1221,12 +1233,13 @@ const make = Effect.gen(function* () { const changes = yield* localChangesReason(worktreePath, settings); if (changes !== null) return keep(changes); } + removing = true; yield* removeLinkedWorktree({ operation: "StorageCleanup.cleanupGit", cwd: project.workspaceRoot, worktreePath, // Git requires force for submodules; recursive checks above cannot make removal atomic. - force: submodules.force, + force: submodules.force || settings.worktreeDiscardLocalChanges, execute: git.execute, run: (args) => cleanupGit(project.workspaceRoot, [...args], "git worktree remove"), }); @@ -1239,6 +1252,16 @@ const make = Effect.gen(function* () { }).pipe( Effect.catch((error) => Effect.gen(function* () { + // Other tools may remove the worktree while it is checked; that is not a failure. + if ( + !removing && + !(yield* fs.exists(worktreePath).pipe(Effect.orElseSucceed(() => true))) + ) { + yield* Effect.logInfo("storage cleanup skipped a worktree removed elsewhere", { + threadId: thread.id, + }); + return; + } entries.push({ ...entry, outcome: "failed", diff --git a/apps/web/src/components/settings/StorageSettings.browser.test.tsx b/apps/web/src/components/settings/StorageSettings.browser.test.tsx index d46e8210f410e..085b72868d07f 100644 --- a/apps/web/src/components/settings/StorageSettings.browser.test.tsx +++ b/apps/web/src/components/settings/StorageSettings.browser.test.tsx @@ -97,6 +97,7 @@ function setup() { ...DEFAULT_SERVER_SETTINGS, storageCleanup: { worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, worktreeAfterDays: null, worktreeSettledAfterDays: null, worktreeOnMerge: false, diff --git a/apps/web/src/components/settings/StorageSettings.tsx b/apps/web/src/components/settings/StorageSettings.tsx index 28f031b5e670e..09d68ce5d6448 100644 --- a/apps/web/src/components/settings/StorageSettings.tsx +++ b/apps/web/src/components/settings/StorageSettings.tsx @@ -176,6 +176,7 @@ const KEEP_WHEN_LABELS = { "any-local-files": "Any local files", "uncommitted-changes": "Uncommitted changes", "tracked-changes": "Edited tracked files", + never: "Never", } as const; function lastRunSummary( @@ -492,6 +493,19 @@ export function WorktreeCleanupSection() { isProjectScope ? updateSettings({ worktreeCleanup: { mode: "custom", rules: patch } }) : update(patch); + const effectiveKeepWhen = (rules: WorktreeCleanupRules) => + rules.worktreeDiscardLocalChanges ? "never" : rules.worktreeKeepWhen; + const keepWhen = effectiveKeepWhen(settings); + const keepWhenStatus = targets.some( + (target) => effectiveKeepWhen(resolveWorktreeCleanup(target.settings, null)) !== keepWhen, + ) + ? "Mixed across selected machines" + : undefined; + // Older servers drop the unknown key, so offer Never only where it is honored. + const canDiscardLocalChanges = connectedEnvironments.every( + (environment) => + environment.serverConfig?.environment.capabilities.worktreeDiscardLocalChanges === true, + ); if ( isProjectScope && @@ -568,7 +582,11 @@ export function WorktreeCleanupSection() { title="Delete worktrees with deleted threads" status={ruleStatus("worktreeOnDelete")} managedPaths={lockablePath("worktreeOnDelete")} - description="Remove unused worktrees when active or archived threads are deleted. Worktrees with local changes are kept." + description={`Remove unused worktrees when active or archived threads are deleted.${ + settings.worktreeDiscardLocalChanges + ? "" + : " Worktrees with local changes are kept." + }`} serverScoped={!isProjectScope} control={ { - if ( + if (next === "never") updateWorktree({ worktreeDiscardLocalChanges: true }); + else if ( next === "any-local-files" || next === "uncommitted-changes" || next === "tracked-changes" ) - updateWorktree({ worktreeKeepWhen: next }); + updateWorktree({ + worktreeKeepWhen: next, + worktreeDiscardLocalChanges: false, + }); }} > - {ruleStatus("worktreeKeepWhen") - ? "Mixed" - : KEEP_WHEN_LABELS[settings.worktreeKeepWhen]} + {keepWhenStatus ? "Mixed" : KEEP_WHEN_LABELS[keepWhen]} Any local files Uncommitted changes Edited tracked files + {canDiscardLocalChanges && Never} } diff --git a/apps/web/src/components/settings/scopedSettings.test.ts b/apps/web/src/components/settings/scopedSettings.test.ts index ace3df94d4348..01122dafc5839 100644 --- a/apps/web/src/components/settings/scopedSettings.test.ts +++ b/apps/web/src/components/settings/scopedSettings.test.ts @@ -169,6 +169,7 @@ describe("scoped settings writes", () => { worktreeOnMerge: true, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, }, }, }, @@ -194,6 +195,7 @@ describe("scoped settings writes", () => { worktreeOnMerge: true, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, }, { worktreeAfterDays: null, @@ -202,6 +204,7 @@ describe("scoped settings writes", () => { worktreeOnMerge: false, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, }, ]); }); @@ -459,6 +462,7 @@ describe("scoped settings writes", () => { worktreeOnMerge: true, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, }, }, }, @@ -490,6 +494,7 @@ describe("scoped settings writes", () => { worktreeOnMerge: true, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, }, }, }, @@ -505,6 +510,7 @@ describe("scoped settings writes", () => { worktreeOnMerge: false, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, }, }, }, diff --git a/apps/web/src/components/settings/settingsSearch.ts b/apps/web/src/components/settings/settingsSearch.ts index 768ce76c2238b..b84146a0960b7 100644 --- a/apps/web/src/components/settings/settingsSearch.ts +++ b/apps/web/src/components/settings/settingsSearch.ts @@ -229,7 +229,7 @@ export const SETTINGS_SEARCH_ITEMS = [ to: "/settings/worktrees", scope: "project-defaults", searchTerms: [ - "any local files uncommitted changes edited tracked files ignored env build output cleanup", + "any local files uncommitted changes edited tracked files never discard ignored env build output cleanup", ], }, { diff --git a/docs/adr/0028-worktree-cleanup-can-discard-local-changes.md b/docs/adr/0028-worktree-cleanup-can-discard-local-changes.md new file mode 100644 index 0000000000000..53b915c0b3db8 --- /dev/null +++ b/docs/adr/0028-worktree-cleanup-can-discard-local-changes.md @@ -0,0 +1,41 @@ +# ADR 0028: Worktree cleanup can discard local changes + +- Status: accepted +- Date: 2026-11-10 +- Compared with upstream: `pingdotgg/t3code` at `402d0623d` (2026-11-09) + +Upstream's **Keep worktrees with local changes** setting (`worktreeKeepWhen`) always +protects at least edited tracked files, so a settled worktree with stray edits is never +removed. The fork owner wants T3-managed worktrees gone once their thread has settled +for the configured days, whatever is in them. + +The setting gains a **Never** choice. [Storage cleanup](../../apps/server/src/storageCleanup.ts) +then skips its local-change checks and runs `git worktree remove --force`, which deletes +tracked edits and untracked and ignored files, including in submodules. The other +protections stay: only T3-managed worktrees, no active session, terminal, or shared +worktree, no branch or HEAD change since the check, no locked worktree, and no +submodule commits missing from a remote. Branches and thread history are kept. + +Never is stored as a separate boolean, `worktreeDiscardLocalChanges`, not as a fourth +`worktreeKeepWhen` value. The fork ships no desktop or mobile builds +([ADR 0011](0011-fork-release-source.md)), so upstream clients connect to fork servers. +Their schemas reject an unknown enum value, which would make the whole settings +snapshot undecodable, but drop an unknown key. Upstream clients therefore keep working +and show the underlying `worktreeKeepWhen`. Their writes omit the flag, so the +[settings merge](../../packages/shared/src/serverSettings.ts) keeps the stored flag, unless +the write picks a different keep level, which turns Never off. Upstream still shows the +underlying keep level while Never is on; choosing another one there must turn Never off, +and changing an unrelated setting must not. Servers advertise the +`worktreeDiscardLocalChanges` capability, and the web client offers Never only when +every selected server has it, since older servers drop the key. A managed policy that locks +`storageCleanup.worktreeKeepWhen` also locks the flag off +([managedSettings.ts](../../apps/server/src/policy/managedSettings.ts)), so Never cannot +bypass an administrator's keep level; upstream policies need no change. + +Other tools also remove worktrees. A worktree that is already gone when cleanup reaches +it was always skipped. One removed while cleanup checks it is now skipped too, unless +T3 Code had started removing it itself. A worktree whose `.git` file was removed is kept +as "not a linked worktree" instead of failing on every run. + +Drop the boolean and capability if upstream adds an equivalent choice; keep the +vanished-worktree handling unless upstream does the same. diff --git a/docs/user/project-settings.md b/docs/user/project-settings.md index 325eef97b2975..9935700eab5f4 100644 --- a/docs/user/project-settings.md +++ b/docs/user/project-settings.md @@ -252,7 +252,8 @@ Active sessions and shared worktrees prevent removal. **Keep worktrees with loca to **Uncommitted changes**, which protects tracked edits and untracked files but deletes ignored files such as `.env` and build output. **Any local files** also protects ignored files other than `node_modules`. **Edited tracked files** protects only tracked edits and allows untracked and -ignored files to be deleted. Branches and thread history stay; starting another turn recreates the checkout. +ignored files to be deleted. **Never** removes eligible worktrees with all their local changes, +including uncommitted edits. Branches and thread history stay; starting another turn recreates the checkout. Merge cleanup requires a merged pull request whose commits are included in the remote default branch. A squash or rebase merge on GitHub also counts when the pull request targeted the default branch and the worktree is still at the pull request's last commit. diff --git a/packages/contracts/src/environment.ts b/packages/contracts/src/environment.ts index 4cf75da665937..8d277c0382199 100644 --- a/packages/contracts/src/environment.ts +++ b/packages/contracts/src/environment.ts @@ -105,6 +105,8 @@ export const ExecutionEnvironmentCapabilities = Schema.Struct({ otlpEndpointCheck: Schema.optionalKey(Schema.Boolean), projectWorktreeCleanup: Schema.optionalKey(Schema.Boolean), settledWorktreeCleanup: Schema.optionalKey(Schema.Boolean), + /** Fork: server honors storage cleanup's worktreeDiscardLocalChanges rule. */ + worktreeDiscardLocalChanges: Schema.optionalKey(Schema.Boolean), worktreesDirectory: Schema.optionalKey(Schema.Boolean), /** Sent by macOS servers, which honor the fastWorktreeCheckout setting. */ fastWorktreeCheckout: Schema.optionalKey(Schema.Boolean), diff --git a/packages/contracts/src/settings.test.ts b/packages/contracts/src/settings.test.ts index c53377f465f07..4955c4b78c4bd 100644 --- a/packages/contracts/src/settings.test.ts +++ b/packages/contracts/src/settings.test.ts @@ -51,6 +51,7 @@ describe("storage cleanup settings", () => { worktreeOnDelete: false, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, browserArtifactsAfterDays: null, logsAfterDays: null, }); @@ -109,6 +110,9 @@ describe("storage cleanup settings", () => { expect( decodeServerSettingsPatch({ storageCleanup: { worktreeKeepWhen: "tracked-changes" } }), ).toEqual({ storageCleanup: { worktreeKeepWhen: "tracked-changes" } }); + expect( + decodeServerSettingsPatch({ storageCleanup: { worktreeDiscardLocalChanges: true } }), + ).toEqual({ storageCleanup: { worktreeDiscardLocalChanges: true } }); expect(() => decodeServerSettingsPatch({ storageCleanup: { worktreeKeepWhen: "unknown" } }), ).toThrow(); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index c012494499bd8..f32c6e5faa208 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -1131,6 +1131,10 @@ export const WorktreeCleanupRules = Schema.Struct({ worktreeOnMerge: Schema.Boolean, worktreeOnDelete: Schema.Boolean, worktreeUnchanged: Schema.Boolean, + // Fork-only; a separate key so upstream clients, which drop unknown keys, still decode settings. + worktreeDiscardLocalChanges: Schema.Boolean.pipe( + Schema.withDecodingDefault(Effect.succeed(false)), + ), }); export type WorktreeCleanupRules = typeof WorktreeCleanupRules.Type; @@ -1140,6 +1144,19 @@ export const WorktreeCleanup = Schema.NullOr( Schema.Struct({ mode: Schema.Literal("custom"), rules: WorktreeCleanupRules }), ]), ); +// Patches leave the fork-only flag optional so a write that omits it keeps the stored value. +const WorktreeCleanupPatch = Schema.NullOr( + Schema.Union([ + Schema.Struct({ mode: Schema.Literal("off") }), + Schema.Struct({ + mode: Schema.Literal("custom"), + rules: Schema.Struct({ + ...WorktreeCleanupRules.fields, + worktreeDiscardLocalChanges: Schema.optionalKey(Schema.Boolean), + }), + }), + ]), +); export type WorktreeCleanup = typeof WorktreeCleanup.Type; export const PROJECT_SCOPED_SERVER_SETTING_KEYS = [ @@ -1318,6 +1335,9 @@ export const StorageCleanupSettings = Schema.Struct({ worktreeOnMerge: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), worktreeOnDelete: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), worktreeUnchanged: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), + worktreeDiscardLocalChanges: Schema.Boolean.pipe( + Schema.withDecodingDefault(Effect.succeed(false)), + ), browserArtifactsAfterDays: StorageRetentionDays.pipe( Schema.withDecodingDefault(Effect.succeed(null)), ), @@ -1784,6 +1804,7 @@ export const ServerSettingsPatch = Schema.Struct({ worktreeOnMerge: Schema.optionalKey(Schema.Boolean), worktreeOnDelete: Schema.optionalKey(Schema.Boolean), worktreeUnchanged: Schema.optionalKey(Schema.Boolean), + worktreeDiscardLocalChanges: Schema.optionalKey(Schema.Boolean), }), }), ]), @@ -1797,6 +1818,7 @@ export const ServerSettingsPatch = Schema.Struct({ worktreeOnMerge: Schema.optionalKey(Schema.Boolean), worktreeOnDelete: Schema.optionalKey(Schema.Boolean), worktreeUnchanged: Schema.optionalKey(Schema.Boolean), + worktreeDiscardLocalChanges: Schema.optionalKey(Schema.Boolean), browserArtifactsAfterDays: Schema.optionalKey(StorageRetentionDays), logsAfterDays: Schema.optionalKey(StorageRetentionDays), }), @@ -1842,6 +1864,7 @@ export const ServerSettingsPatch = Schema.Struct({ Schema.Struct({ ...ProjectSettingsOverrides.fields, sourceControlWritingStyle: Schema.optionalKey(SourceControlWritingStyleSettings), + worktreeCleanup: Schema.optionalKey(WorktreeCleanupPatch), }), ), ), diff --git a/packages/shared/src/projectSettings.test.ts b/packages/shared/src/projectSettings.test.ts index 9316cb32cf331..aed9bfe9c1371 100644 --- a/packages/shared/src/projectSettings.test.ts +++ b/packages/shared/src/projectSettings.test.ts @@ -387,6 +387,7 @@ describe("resolveWorktreeCleanup", () => { worktreeOnMerge: false, worktreeUnchanged: false, worktreeKeepWhen: "any-local-files", + worktreeDiscardLocalChanges: false, }); expect(resolveWorktreeCleanup(off, otherProjectId)).toEqual(inherited); const custom = applyServerSettingsPatch(off, { @@ -412,7 +413,12 @@ describe("resolveWorktreeCleanup", () => { }); it("completes partial machine custom rules and preserves them across edits", () => { const initial = applyServerSettingsPatch(DEFAULT_SERVER_SETTINGS, { - storageCleanup: { worktreeAfterDays: 8, worktreeSettledAfterDays: 0, worktreeOnDelete: true }, + storageCleanup: { + worktreeAfterDays: 8, + worktreeSettledAfterDays: 0, + worktreeOnDelete: true, + worktreeDiscardLocalChanges: true, + }, }); const custom = applyServerSettingsPatch(initial, { worktreeCleanup: { mode: "custom", rules: { worktreeOnMerge: true } }, @@ -427,6 +433,7 @@ describe("resolveWorktreeCleanup", () => { worktreeOnMerge: true, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: true, }); expect( resolveWorktreeCleanup(applyServerSettingsPatch(edited, { worktreeCleanup: null }), null) diff --git a/packages/shared/src/projectSettings.ts b/packages/shared/src/projectSettings.ts index e499fd3eb0a72..b8b55655f5125 100644 --- a/packages/shared/src/projectSettings.ts +++ b/packages/shared/src/projectSettings.ts @@ -227,6 +227,7 @@ export function resolveWorktreeCleanup( worktreeOnMerge: false, worktreeOnDelete: false, worktreeUnchanged: false, + worktreeDiscardLocalChanges: settings.storageCleanup.worktreeDiscardLocalChanges, }; const { worktreeKeepWhen, @@ -235,6 +236,7 @@ export function resolveWorktreeCleanup( worktreeOnMerge, worktreeOnDelete, worktreeUnchanged, + worktreeDiscardLocalChanges, } = settings.storageCleanup; return { worktreeKeepWhen, @@ -243,5 +245,6 @@ export function resolveWorktreeCleanup( worktreeOnMerge, worktreeOnDelete, worktreeUnchanged, + worktreeDiscardLocalChanges, }; } diff --git a/packages/shared/src/serverSettings.test.ts b/packages/shared/src/serverSettings.test.ts index 4a28655ab4292..7b1b9d5ed35e5 100644 --- a/packages/shared/src/serverSettings.test.ts +++ b/packages/shared/src/serverSettings.test.ts @@ -110,10 +110,65 @@ describe("serverSettings helpers", () => { worktreeOnDelete: false, worktreeUnchanged: false, worktreeKeepWhen: "uncommitted-changes", + worktreeDiscardLocalChanges: false, browserArtifactsAfterDays: null, logsAfterDays: 30, }); }); + it("keeps discard-local-changes through upstream writes unless they pick another keep level", () => { + const projectId = ProjectId.make("project-1"); + const rules = { + worktreeKeepWhen: "uncommitted-changes", + worktreeAfterDays: null, + worktreeSettledAfterDays: 3, + worktreeOnMerge: false, + worktreeOnDelete: false, + worktreeUnchanged: false, + }; + const apply = (settings: ServerSettings, patch: unknown) => + applyServerSettingsPatch(settings, decodeServerSettingsPatch(patch)); + const discarding = apply(DEFAULT_SERVER_SETTINGS, { + storageCleanup: { worktreeDiscardLocalChanges: true }, + worktreeCleanup: { mode: "custom", rules: { worktreeDiscardLocalChanges: true } }, + projectSettingsOverrides: { + [projectId]: { + worktreeCleanup: { + mode: "custom", + rules: { ...rules, worktreeDiscardLocalChanges: true }, + }, + }, + }, + }); + // Upstream clients send the rules without the fork-only flag. + const resent = apply(discarding, { + storageCleanup: { worktreeKeepWhen: "uncommitted-changes", worktreeAfterDays: 8 }, + worktreeCleanup: { mode: "custom", rules: { worktreeAfterDays: 8 } }, + projectSettingsOverrides: { + [projectId]: { defaultAutoPull: true, worktreeCleanup: { mode: "custom", rules } }, + }, + }); + const discardFlags = (settings: ServerSettings) => [ + settings.storageCleanup.worktreeDiscardLocalChanges, + settings.worktreeCleanup?.mode === "custom" && + settings.worktreeCleanup.rules.worktreeDiscardLocalChanges, + settings.projectSettingsOverrides[projectId]?.worktreeCleanup?.mode === "custom" && + settings.projectSettingsOverrides[projectId].worktreeCleanup.rules + .worktreeDiscardLocalChanges, + ]; + expect(discardFlags(resent)).toEqual([true, true, true]); + const protective = { ...rules, worktreeKeepWhen: "any-local-files" }; + expect( + discardFlags( + apply(resent, { + storageCleanup: { worktreeKeepWhen: "any-local-files" }, + worktreeCleanup: { mode: "custom", rules: { worktreeKeepWhen: "any-local-files" } }, + projectSettingsOverrides: { + [projectId]: { worktreeCleanup: { mode: "custom", rules: protective } }, + }, + }), + ), + ).toEqual([false, false, false]); + }); it("replaces a source control host's patched field so a cleared account pin does not survive", () => { const pinned = applyServerSettingsPatch(DEFAULT_SERVER_SETTINGS, { sourceControlHosts: { diff --git a/packages/shared/src/serverSettings.ts b/packages/shared/src/serverSettings.ts index 5e54ccb7a9257..1df0bfff1f8c0 100644 --- a/packages/shared/src/serverSettings.ts +++ b/packages/shared/src/serverSettings.ts @@ -23,6 +23,7 @@ import { ServerSettings, type ServerSettingsPatch, withIdleCompaction, + type WorktreeCleanupRules, } from "@t3tools/contracts"; import * as Option from "effect/Option"; import * as Predicate from "effect/Predicate"; @@ -258,7 +259,11 @@ const byHost = (tokens: unknown) => const knownProjectOverrideKeys = new Set(Object.keys(ProjectSettingsOverrides.fields)); -function pickOverrideKeys(entry: ProjectSettingsOverrides | undefined, known: boolean) { +type ProjectSettingsOverridesPatch = NonNullable< + NonNullable[ProjectId] +>; + +function pickOverrideKeys(entry: ProjectSettingsOverridesPatch | undefined, known: boolean) { return Object.fromEntries( Object.entries(entry ?? {}).filter( ([key]) => knownProjectOverrideKeys.has(key) === known, @@ -266,10 +271,28 @@ function pickOverrideKeys(entry: ProjectSettingsOverrides | undefined, known: bo ); } +/** Upstream clients omit the fork-only discard flag: keep it unless they pick another keep level. */ +function withDiscardLocalChanges< + Rules extends Partial< + Pick + >, +>( + rules: Rules, + current: Pick | null, +) { + return { + ...rules, + worktreeDiscardLocalChanges: + rules.worktreeDiscardLocalChanges ?? + ((rules.worktreeKeepWhen ?? current?.worktreeKeepWhen) === current?.worktreeKeepWhen && + (current?.worktreeDiscardLocalChanges ?? false)), + }; +} + // Older clients omit providerInstances during whole-entry replacement; preserve it to avoid wiping overrides. function mergeProjectSettingsOverridesPatch( current: Readonly>, - patch: Readonly>, + patch: NonNullable, ): Record { const next = new Map(Object.entries(current)); for (const [projectId, entry] of Object.entries(patch)) { @@ -278,10 +301,22 @@ function mergeProjectSettingsOverridesPatch( continue; } const storedProviderInstances = current[projectId]?.providerInstances; + const storedCleanup = current[projectId]?.worktreeCleanup; next.set(projectId, { // Preserve downgraded storage, but reject new unknown writes so clients can detect unsupported settings. ...pickOverrideKeys(current[projectId], false), ...pickOverrideKeys(entry, true), + ...(entry.worktreeCleanup?.mode === "custom" + ? { + worktreeCleanup: { + mode: "custom" as const, + rules: withDiscardLocalChanges( + entry.worktreeCleanup.rules, + storedCleanup?.mode === "custom" ? storedCleanup.rules : null, + ), + }, + } + : {}), ...(Object.hasOwn(entry, "providerInstances") || storedProviderInstances === undefined ? {} : { providerInstances: storedProviderInstances }), @@ -332,7 +367,7 @@ function translateLegacyProjectOverridePatch( } const currentEntries: Readonly> = current.projectSettingsOverrides; - const entries = new Map( + const entries = new Map( Object.entries(rest.projectSettingsOverrides ?? {}), ); // Canonical entries outrank legacy maps, including deliberately omitted keys. @@ -344,7 +379,7 @@ function translateLegacyProjectOverridePatch( if (map === undefined) return; for (const [projectId, value] of Object.entries(map)) { if (canonicalProjectIds.has(projectId)) continue; - const entry: ProjectSettingsOverrides = { + const entry: ProjectSettingsOverridesPatch = { ...(entries.get(projectId) ?? currentEntries[projectId]), }; if (value === null || value === undefined) { @@ -498,7 +533,17 @@ export function applyServerSettingsPatch( }, }; } - const next = deepMerge(current, patchForMerge); + const next = deepMerge(current, { + ...patchForMerge, + ...(patchForMerge.storageCleanup === undefined + ? {} + : { + storageCleanup: withDiscardLocalChanges( + patchForMerge.storageCleanup, + current.storageCleanup, + ), + }), + }); const nextWithReplacementsBase = { ...next, favorites, @@ -520,7 +565,12 @@ export function applyServerSettingsPatch( ...(current.worktreeCleanup?.mode === "custom" ? current.worktreeCleanup.rules : {}), - ...worktreeCleanupPatch.rules, + ...withDiscardLocalChanges( + worktreeCleanupPatch.rules, + current.worktreeCleanup?.mode === "custom" + ? current.worktreeCleanup.rules + : next.storageCleanup, + ), }, } : worktreeCleanupPatch,