From e06c0f13a55ae69b46531dd7c24620937f0fa5a8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 21 Oct 2026 11:18:32 +0000 Subject: [PATCH 1/4] test(server): takeover tests for folded project settings --- .../server/src/project/ProjectService.test.ts | 134 +++++++++++++++++- 1 file changed, 133 insertions(+), 1 deletion(-) diff --git a/apps/server/src/project/ProjectService.test.ts b/apps/server/src/project/ProjectService.test.ts index 9347acef12c8f..a79ec4d2d32d0 100644 --- a/apps/server/src/project/ProjectService.test.ts +++ b/apps/server/src/project/ProjectService.test.ts @@ -1,6 +1,7 @@ import { assert, it } from "@effect/vitest"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { CommandId, type Project, ProjectId, ProviderInstanceId } from "@t3tools/contracts"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; @@ -56,13 +57,14 @@ const layerTestFor = ( | ProjectFaviconResolver.ProjectFaviconResolver | RepositoryIdentityResolver.RepositoryIdentityResolver >, + settingsOverrides: Parameters[0] = {}, ) => RuntimeLayer.layerProjectService.pipe( Layer.provideMerge(ProjectEnrichmentService.layer), Layer.provideMerge(layerWorkspacePaths), Layer.provideMerge(projectMetadataLayer), Layer.provideMerge(SqlitePersistence.layerMemory), - Layer.provideMerge(ServerSettings.layerTest()), + Layer.provideMerge(ServerSettings.layerTest(settingsOverrides)), Layer.provide(ServerConfig.layerTest(process.cwd(), { prefix: "project-service-test-" })), Layer.provide(NodeServices.layer), ); @@ -669,3 +671,133 @@ it.effect("rejects an update that waited on the lock while its project was delet ); }).pipe(Effect.provide(layerProjectServiceDependencies)), ); + +it.layer( + layerTestFor(layerMetadata, { projectSettingsFolded: true, defaultThreadEnvMode: "local" }), +)("ProjectService folded settings", (it) => { + it.effect.each([ + { + field: "scripts", + setting: "defaultProjectScripts", + value: [ + { + id: "setup", + name: "Setup", + command: "vp install", + icon: "configure", + runOnWorktreeCreate: true, + }, + ], + reset: [], + }, + { + field: "defaultModelSelection", + setting: "defaultModelSelection", + value: { instanceId: ProviderInstanceId.make("codex"), model: "gpt-6.1-sol" }, + reset: null, + }, + { + field: "defaultThreadEnvMode", + setting: "defaultThreadEnvMode", + value: "worktree", + reset: null, + }, + { field: "autoPull", setting: "defaultAutoPull", value: true, reset: false }, + ] as const)( + "updates $field in effective settings after project settings were folded", + ({ field, setting, value, reset }) => + Effect.gen(function* () { + const service = yield* ProjectService.ProjectService; + const settingsService = yield* ServerSettings.ServerSettingsService; + const projectId = ProjectId.make(`project:folded:${field}`); + yield* service.create({ + commandId: CommandId.make(`command:folded:${field}:create`), + projectId, + title: "Folded settings", + workspaceRoot: `/work/folded-${field}`, + }); + + const updated = yield* service.update({ + commandId: CommandId.make(`command:folded:${field}:update`), + projectId, + [field]: value, + }); + assert.deepEqual(updated[field], value); + const effective = resolveProjectSettings( + yield* settingsService.getSettings, + projectId, + ).settings; + assert.deepEqual(effective[setting], value); + assert.deepEqual(Option.getOrThrow(yield* service.getById(projectId))[field], value); + + yield* service.update({ + commandId: CommandId.make(`command:folded:${field}:rename`), + projectId, + title: "Renamed", + }); + assert.deepEqual( + resolveProjectSettings(yield* settingsService.getSettings, projectId).settings[setting], + value, + ); + + const cleared = yield* service.update({ + commandId: CommandId.make(`command:folded:${field}:reset`), + projectId, + [field]: reset, + }); + const resetEffective = resolveProjectSettings( + yield* settingsService.getSettings, + projectId, + ).settings; + const expected = field === "defaultThreadEnvMode" ? "local" : reset; + assert.deepEqual(resetEffective[setting], expected); + assert.deepEqual(cleared[field], field === "defaultThreadEnvMode" ? null : expected); + assert.deepEqual( + Option.getOrThrow(yield* service.getById(projectId))[field], + cleared[field], + ); + }), + ); + + it.effect("reads effective settings instead of stale persisted fields after folding", () => + Effect.gen(function* () { + const service = yield* ProjectService.ProjectService; + const settingsService = yield* ServerSettings.ServerSettingsService; + const sql = yield* SqlClient.SqlClient; + const projectId = ProjectId.make("project:folded:stale"); + yield* service.create({ + commandId: CommandId.make("command:folded:stale:create"), + projectId, + title: "Previously updated", + workspaceRoot: "/work/folded-stale", + }); + // These are values older project updates persisted without changing effective settings. + yield* sql` + UPDATE projection_projects + SET default_model_selection_json = ${JSON.stringify({ instanceId: "codex", model: "gpt-6.1-sol" })}, + default_thread_env_mode = 'worktree', auto_pull = 1, + scripts_json = ${JSON.stringify([{ id: "setup", name: "Setup", command: "vp install", icon: "configure", runOnWorktreeCreate: true }])} + WHERE project_id = ${projectId} + `; + yield* settingsService.updateSettings({ + projectSettingsOverrides: { + [projectId]: { + defaultModelSelection: null, + defaultThreadEnvMode: "local", + defaultAutoPull: false, + defaultProjectScripts: [], + }, + }, + }); + const effective = resolveProjectSettings( + yield* settingsService.getSettings, + projectId, + ).settings; + const project = Option.getOrThrow(yield* service.getById(projectId)); + assert.deepEqual(project.defaultModelSelection, effective.defaultModelSelection); + assert.equal(project.defaultThreadEnvMode, effective.defaultThreadEnvMode); + assert.equal(project.autoPull, effective.defaultAutoPull); + assert.deepEqual(project.scripts, effective.defaultProjectScripts); + }), + ); +}); From 6326a82bec42bf346ea92bdc7528d5d4713a2acc Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 21 Oct 2026 11:28:27 +0000 Subject: [PATCH 2/4] fix(server): apply project updates to folded settings --- apps/server/src/device/DeviceService.test.ts | 19 +++-- .../ThreadSettlementService.test.ts | 11 ++- .../server/src/project/ProjectService.test.ts | 78 +++++++++++++++++++ apps/server/src/project/ProjectService.ts | 77 +++++++++++++++++- .../src/provider/ProviderRegistry.test.ts | 5 +- apps/server/src/serverSettings.test.ts | 36 +++++++++ apps/server/src/serverSettings.ts | 13 +++- 7 files changed, 221 insertions(+), 18 deletions(-) diff --git a/apps/server/src/device/DeviceService.test.ts b/apps/server/src/device/DeviceService.test.ts index 53eaa6bb26c91..98ee12f93fbb9 100644 --- a/apps/server/src/device/DeviceService.test.ts +++ b/apps/server/src/device/DeviceService.test.ts @@ -203,14 +203,17 @@ const fixture = Effect.fn("fixture")(function* ( ready: Effect.void, getSettings: Ref.get(settings), updateSettings: (patch) => - Ref.updateAndGet(settings, (current) => ({ - ...current, - enableDeviceSupport: patch.enableDeviceSupport ?? current.enableDeviceSupport, - enableAgentDeviceAccess: - patch.enableAgentDeviceAccess ?? current.enableAgentDeviceAccess, - deviceOnboardingCompleted: - patch.deviceOnboardingCompleted ?? current.deviceOnboardingCompleted, - })), + Ref.updateAndGet(settings, (current) => { + const resolvedPatch = typeof patch === "function" ? patch(current) : patch; + return { + ...current, + enableDeviceSupport: resolvedPatch.enableDeviceSupport ?? current.enableDeviceSupport, + enableAgentDeviceAccess: + resolvedPatch.enableAgentDeviceAccess ?? current.enableAgentDeviceAccess, + deviceOnboardingCompleted: + resolvedPatch.deviceOnboardingCompleted ?? current.deviceOnboardingCompleted, + }; + }), streamChanges: Stream.empty, subscribeChanges: Effect.succeed(Stream.empty), }), diff --git a/apps/server/src/orchestration-v2/ThreadSettlementService.test.ts b/apps/server/src/orchestration-v2/ThreadSettlementService.test.ts index 10caf988e475f..bde99f4f84f5a 100644 --- a/apps/server/src/orchestration-v2/ThreadSettlementService.test.ts +++ b/apps/server/src/orchestration-v2/ThreadSettlementService.test.ts @@ -15,7 +15,6 @@ import { type OrchestrationV2ThreadShell, type PullRequestSummary, type ServerSettings as ContractServerSettings, - type ServerSettingsPatch, } from "@t3tools/contracts"; import { applyServerSettingsPatch } from "@t3tools/shared/serverSettings"; import * as Crypto from "effect/Crypto"; @@ -505,9 +504,15 @@ const makeHarness = Effect.fn("makeThreadSettlementHarness")(function* (options: const scriptRuns = yield* Queue.unbounded(); - const updateSettings = (patch: ServerSettingsPatch) => + const updateSettings: ServerSettings.ServerSettingsService["Service"]["updateSettings"] = ( + patch, + ) => Effect.gen(function* () { - const next = applyServerSettingsPatch(yield* Ref.get(settings), patch); + const current = yield* Ref.get(settings); + const next = applyServerSettingsPatch( + current, + typeof patch === "function" ? patch(current) : patch, + ); yield* Ref.set(settings, next); yield* PubSub.publish(settingsChanges, next); return next; diff --git a/apps/server/src/project/ProjectService.test.ts b/apps/server/src/project/ProjectService.test.ts index a79ec4d2d32d0..a1f3553a5415a 100644 --- a/apps/server/src/project/ProjectService.test.ts +++ b/apps/server/src/project/ProjectService.test.ts @@ -759,6 +759,84 @@ it.layer( }), ); + it.effect( + "preserves other overrides and restores file inheritance when clearing the workspace default", + () => + Effect.gen(function* () { + const service = yield* ProjectService.ProjectService; + const settingsService = yield* ServerSettings.ServerSettingsService; + const projectId = ProjectId.make("project:folded:preserve"); + yield* service.create({ + commandId: CommandId.make("command:folded:preserve:create"), + projectId, + title: "Preserve settings", + workspaceRoot: "/work/folded-preserve", + }); + const model = { instanceId: ProviderInstanceId.make("codex"), model: "gpt-6.1-sol" }; + yield* settingsService.updateSettings({ + projectSettingsOverrides: { + [projectId]: { + defaultModelSelection: model, + defaultThreadEnvMode: "local", + branchNamePrefix: "feature/", + }, + }, + }); + yield* service.update({ + commandId: CommandId.make("command:folded:preserve:update"), + projectId, + autoPull: true, + defaultThreadEnvMode: null, + }); + const settings = yield* settingsService.getSettings; + assert.deepEqual(settings.projectSettingsOverrides[projectId], { + defaultModelSelection: model, + branchNamePrefix: "feature/", + defaultAutoPull: true, + }); + // File-backed defaults only apply when the environment also inherits the file. + yield* settingsService.updateSettings({ defaultThreadEnvMode: null }); + const project = Option.getOrThrow(yield* service.getById(projectId)); + assert.isNull(project.defaultThreadEnvMode); + assert.deepEqual(project.defaultModelSelection, model); + assert.equal( + resolveProjectSettings(yield* settingsService.getSettings, projectId, project, { + defaultThreadEnvMode: "worktree", + }).settings.defaultThreadEnvMode, + "worktree", + ); + }), + ); + + it.effect("does not reapply a replayed command over newer Settings edits", () => + Effect.gen(function* () { + const service = yield* ProjectService.ProjectService; + const settingsService = yield* ServerSettings.ServerSettingsService; + const projectId = ProjectId.make("project:folded:replay"); + yield* service.create({ + commandId: CommandId.make("command:folded:replay:create"), + projectId, + title: "Replay settings", + workspaceRoot: "/work/folded-replay", + }); + const update = { + commandId: CommandId.make("command:folded:replay:update"), + projectId, + autoPull: true, + }; + yield* service.update(update); + yield* settingsService.updateSettings({ + projectSettingsOverrides: { [projectId]: { defaultAutoPull: false } }, + }); + const replayed = yield* service.update(update); + assert.isFalse(replayed.autoPull); + assert.isFalse( + resolveProjectSettings(yield* settingsService.getSettings, projectId).settings + .defaultAutoPull, + ); + }), + ); + it.effect("reads effective settings instead of stale persisted fields after folding", () => Effect.gen(function* () { const service = yield* ProjectService.ProjectService; diff --git a/apps/server/src/project/ProjectService.ts b/apps/server/src/project/ProjectService.ts index 1cfb2e093340b..3affc5311f1df 100644 --- a/apps/server/src/project/ProjectService.ts +++ b/apps/server/src/project/ProjectService.ts @@ -8,6 +8,8 @@ import { type ProjectSnapshot, type ThreadId, } from "@t3tools/contracts"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; +import { resolveProjectScripts } from "@t3tools/shared/projectScripts"; import * as KeyedLock from "@t3tools/shared/KeyedLock"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; @@ -93,6 +95,8 @@ export class ProjectOperationError extends Schema.TaggedError + new ProjectOperationError({ + operation: "read-settings", + projectId: row.projectId, + cause, + }), + ), + ); + if (!settings.projectSettingsFolded) return project; + const resolved = resolveProjectSettings(settings, row.projectId).settings; + return { + ...project, + defaultModelSelection: resolved.defaultModelSelection, + defaultThreadEnvMode: + settings.projectSettingsOverrides[row.projectId]?.defaultThreadEnvMode ?? null, + autoPull: resolved.defaultAutoPull, + scripts: resolveProjectScripts(settings, project), + }; }); const readRow = (projectId: ProjectId, options?: { readonly includeDeleted?: boolean }) => @@ -210,6 +234,40 @@ export const make = Effect.gen(function* () { ), ); + const updateProjectSettings = Effect.fn("ProjectService.updateProjectSettings")(function* ( + input: ProjectUpdateInput, + ) { + if ( + input.defaultModelSelection === undefined && + input.defaultThreadEnvMode === undefined && + input.autoPull === undefined && + input.scripts === undefined + ) + return; + yield* serverSettings + .updateSettings((settings) => { + const overrides = { ...settings.projectSettingsOverrides[input.projectId] }; + if (input.defaultModelSelection !== undefined) + overrides.defaultModelSelection = input.defaultModelSelection; + if (input.defaultThreadEnvMode === null) delete overrides.defaultThreadEnvMode; + else if (input.defaultThreadEnvMode !== undefined) + overrides.defaultThreadEnvMode = input.defaultThreadEnvMode; + if (input.autoPull !== undefined) overrides.defaultAutoPull = input.autoPull; + if (input.scripts !== undefined) overrides.defaultProjectScripts = input.scripts; + return { projectSettingsOverrides: { [input.projectId]: overrides } }; + }) + .pipe( + Effect.mapError( + (cause) => + new ProjectOperationError({ + operation: "update-settings", + projectId: input.projectId, + cause, + }), + ), + ); + }); + const commit = Effect.fn("ProjectService.commit")(function* (command: ProjectCommand) { const { projectId } = command; const dispatchError = (cause: unknown) => @@ -236,13 +294,22 @@ export const make = Effect.gen(function* () { now, }); if (Result.isSuccess(planned)) { - const { receipt } = yield* eventSink.commitProjectCommand({ + const { receipt, committed } = yield* eventSink.commitProjectCommand({ commandId: command.commandId, projectId, commandType: command.type, acceptedAt: now, event: planned.success, }); + if ( + committed && + command.type === "project.meta.update" && + receipt.status === "accepted" && + receipt.projectId === projectId && + receipt.commandType === command.type + ) { + yield* updateProjectSettings(command); + } return receipt; } return yield* eventSink.commitRejectedProjectCommand({ @@ -260,7 +327,11 @@ export const make = Effect.gen(function* () { ? planAndCommit : workspaceLocks.withLock(workspaceRoot, planAndCommit), ) - .pipe(Effect.mapError(dispatchError)); + .pipe( + Effect.mapError((cause) => + cause._tag === "ProjectOperationError" ? cause : dispatchError(cause), + ), + ); if (receipt.projectId !== projectId || receipt.commandType !== command.type) { return yield* dispatchError( `Command ${command.commandId} was already used by ${receipt.commandType} for ${receipt.projectId}.`, diff --git a/apps/server/src/provider/ProviderRegistry.test.ts b/apps/server/src/provider/ProviderRegistry.test.ts index a278470c286d0..3bae3542a9f80 100644 --- a/apps/server/src/provider/ProviderRegistry.test.ts +++ b/apps/server/src/provider/ProviderRegistry.test.ts @@ -262,7 +262,10 @@ function makeMutableServerSettingsService( updateSettings: (patch) => Effect.gen(function* () { const current = yield* Ref.get(settingsRef); - const next = applyServerSettingsPatch(current, patch); + const next = applyServerSettingsPatch( + current, + typeof patch === "function" ? patch(current) : patch, + ); encodeServerSettings(next); yield* Ref.set(settingsRef, next); yield* PubSub.publish(changes, next); diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index a23d44fc282d1..c6ff5f930895b 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -94,6 +94,42 @@ const recordProviderUsage = (provider: string, instanceId: string | null = provi }); it.layer(NodeServices.layer)("server settings", (it) => { + it.effect("merges functional project edits against the latest persisted settings", () => + Effect.gen(function* () { + const service = yield* ServerSettingsModule.ServerSettingsService; + const projectId = ProjectId.make("project:atomic-edit"); + yield* Effect.all( + [ + service.updateSettings((settings) => ({ + projectSettingsOverrides: { + [projectId]: { + ...settings.projectSettingsOverrides[projectId], + defaultAutoPull: true, + }, + }, + })), + service.updateSettings((settings) => ({ + projectSettingsOverrides: { + [projectId]: { + ...settings.projectSettingsOverrides[projectId], + defaultThreadEnvMode: "worktree", + }, + }, + })), + ], + { concurrency: "unbounded" }, + ); + const config = yield* ServerConfig.ServerConfig; + const fs = yield* FileSystem.FileSystem; + const persisted = yield* decodeServerSettingsJson( + yield* fs.readFileString(config.settingsPath), + ); + assert.deepEqual(persisted.projectSettingsOverrides[projectId], { + defaultAutoPull: true, + defaultThreadEnvMode: "worktree", + }); + }).pipe(Effect.provide(layerServerSettings())), + ); it.effect("serializes concurrent model preference edits without cancelling duplicate adds", () => Effect.gen(function* () { const service = yield* ServerSettingsModule.ServerSettingsService; diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index 5289b7b4d75b9..02ef163b80219 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -179,7 +179,7 @@ export class ServerSettingsService extends Context.Service< readonly getSettings: Effect.Effect; readonly updateSettings: ( - patch: ServerSettingsPatch, + patch: ServerSettingsPatch | ((current: ServerSettings) => ServerSettingsPatch), ) => Effect.Effect; readonly updateProviderInstance: ( @@ -240,7 +240,12 @@ const makeTest = Effect.fn("serverSettings.makeTest")(function* ( getSettings, updateSettings: (patch) => updateTestSettings((currentSettings) => - Effect.succeed(applyServerSettingsPatch(currentSettings, patch)), + Effect.succeed( + applyServerSettingsPatch( + currentSettings, + typeof patch === "function" ? patch(currentSettings) : patch, + ), + ), ), updateProviderInstance: (mutation, patch = {}) => updateTestSettings((currentSettings) => @@ -1154,7 +1159,9 @@ const make = Effect.gen(function* () { ), updateSettings: (patch) => updateAndPersistSettings((current) => - Effect.succeed(applyServerSettingsPatch(current, patch)), + Effect.succeed( + applyServerSettingsPatch(current, typeof patch === "function" ? patch(current) : patch), + ), ), updateProviderInstance: (mutation, patch = {}) => updateAndPersistSettings((current) => From 1c7996206076b5e04cfdbd05293fb3ae7b2f8d4c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 21 Oct 2026 13:28:02 +0000 Subject: [PATCH 3/4] fix(server): retry failed project settings persistence --- apps/server/src/orchestration-v2/EventSink.ts | 4 ++ .../server/src/project/ProjectService.test.ts | 66 ++++++++++++++++++- apps/server/src/project/ProjectService.ts | 14 ++-- 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/apps/server/src/orchestration-v2/EventSink.ts b/apps/server/src/orchestration-v2/EventSink.ts index d2bd8a7ac0315..4f9c9b17f2993 100644 --- a/apps/server/src/orchestration-v2/EventSink.ts +++ b/apps/server/src/orchestration-v2/EventSink.ts @@ -177,6 +177,7 @@ export interface EventSinkV2Shape { readonly commandType: string; readonly acceptedAt: DateTime.Utc; readonly event: UnsequencedProjectEvent; + readonly beforeCommit?: Effect.Effect; }) => Effect.Effect< { readonly receipt: CommandReceiptStore.ProjectCommandReceiptV2; readonly committed: boolean }, EventSinkV2Error @@ -734,6 +735,9 @@ const layerBase: Layer.Layer< } const event = yield* eventStore.appendProjectEvent(input.event); yield* projectStore.apply(event); + // A failed settings write must leave no accepted receipt to suppress its retry. + // Existing receipts return above, so successful replays never rerun this write. + if (input.beforeCommit !== undefined) yield* input.beforeCommit; const receipt = { ...reserved, resultSequence: event.sequence }; yield* commandReceipts.upsert(receipt); return { receipt, event }; diff --git a/apps/server/src/project/ProjectService.test.ts b/apps/server/src/project/ProjectService.test.ts index a1f3553a5415a..402b722edd144 100644 --- a/apps/server/src/project/ProjectService.test.ts +++ b/apps/server/src/project/ProjectService.test.ts @@ -1,6 +1,12 @@ import { assert, it } from "@effect/vitest"; import * as NodeServices from "@effect/platform-node/NodeServices"; -import { CommandId, type Project, ProjectId, ProviderInstanceId } from "@t3tools/contracts"; +import { + CommandId, + type Project, + ProjectId, + ProviderInstanceId, + ServerSettingsError, +} from "@t3tools/contracts"; import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; @@ -466,6 +472,64 @@ it.layer(layerTest)("ProjectService", (it) => { ); }); +it.effect("retries a failed settings write without accepting an incomplete project update", () => + Effect.gen(function* () { + const settings = yield* ServerSettings.ServerSettingsService; + const failWrite = yield* Ref.make(true); + const service = yield* ProjectService.make.pipe( + Effect.provideService(ServerSettings.ServerSettingsService, { + ...settings, + getSettings: settings.getSettings.pipe( + Effect.map((current) => ({ ...current, projectSettingsFolded: true })), + ), + updateSettings: (patch) => + Ref.getAndSet(failWrite, false).pipe( + Effect.flatMap((fail) => + fail + ? Effect.fail( + new ServerSettingsError({ + settingsPath: "/test/settings.json", + operation: "write-file", + cause: "injected transient write failure", + }), + ) + : settings.updateSettings(patch), + ), + ), + }), + ); + const sql = yield* SqlClient.SqlClient; + const projectId = ProjectId.make("project:settings-retry"); + yield* service.create({ + commandId: CommandId.make("command:settings-retry:create"), + projectId, + title: "Settings retry", + workspaceRoot: "/work/settings-retry", + }); + const input = { + commandId: CommandId.make("command:settings-retry:update"), + projectId, + autoPull: true, + }; + assert.equal((yield* service.update(input).pipe(Effect.flip))._tag, "ProjectOperationError"); + const leftovers = yield* sql<{ readonly count: number }>` + SELECT + (SELECT COUNT(*) FROM orchestration_events WHERE command_id = ${input.commandId}) + + (SELECT COUNT(*) FROM orchestration_command_receipts WHERE command_id = ${input.commandId}) AS count + `; + assert.equal(leftovers[0]?.count, 0); + assert.isFalse(Option.getOrThrow(yield* service.getById(projectId)).autoPull); + assert.isTrue((yield* service.update(input)).autoPull); + assert.isTrue( + (yield* settings.getSettings).projectSettingsOverrides[projectId]?.defaultAutoPull, + ); + yield* settings.updateSettings({ + projectSettingsOverrides: { [projectId]: { defaultAutoPull: false } }, + }); + assert.isFalse((yield* service.update(input)).autoPull); + }).pipe(Effect.provide(layerProjectServiceDependencies)), +); + it.effect("keeps project snapshots available when optional metadata enrichment fails", () => Effect.gen(function* () { const layerFailingMetadata = Layer.merge( diff --git a/apps/server/src/project/ProjectService.ts b/apps/server/src/project/ProjectService.ts index 3affc5311f1df..021388aa33a23 100644 --- a/apps/server/src/project/ProjectService.ts +++ b/apps/server/src/project/ProjectService.ts @@ -294,22 +294,16 @@ export const make = Effect.gen(function* () { now, }); if (Result.isSuccess(planned)) { - const { receipt, committed } = yield* eventSink.commitProjectCommand({ + const { receipt } = yield* eventSink.commitProjectCommand({ commandId: command.commandId, projectId, commandType: command.type, acceptedAt: now, event: planned.success, + ...(command.type === "project.meta.update" + ? { beforeCommit: updateProjectSettings(command) } + : {}), }); - if ( - committed && - command.type === "project.meta.update" && - receipt.status === "accepted" && - receipt.projectId === projectId && - receipt.commandType === command.type - ) { - yield* updateProjectSettings(command); - } return receipt; } return yield* eventSink.commitRejectedProjectCommand({ From f05e15112d6e70ec9f9c8aa5c100e361411805a1 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 21 Oct 2026 13:36:42 +0000 Subject: [PATCH 4/4] fix(server): avoid settings and project SQL lock inversion --- apps/server/src/orchestration-v2/EventSink.ts | 43 ++++++++++++------- .../server/src/project/ProjectService.test.ts | 41 ++++++++++++++++++ 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/apps/server/src/orchestration-v2/EventSink.ts b/apps/server/src/orchestration-v2/EventSink.ts index 4f9c9b17f2993..199902de6b5f5 100644 --- a/apps/server/src/orchestration-v2/EventSink.ts +++ b/apps/server/src/orchestration-v2/EventSink.ts @@ -12,6 +12,7 @@ import { type ProjectId, ThreadId, } from "@t3tools/contracts"; +import * as KeyedLock from "@t3tools/shared/KeyedLock"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; @@ -223,6 +224,7 @@ const layerBase: Layer.Layer< Effect.gen(function* () { const sql = yield* SqlClient.SqlClient; const commandReceipts = yield* CommandReceiptStore.CommandReceiptStoreV2; + const commandLocks = yield* KeyedLock.make(); const effectOutbox = yield* EffectOutbox.EffectOutboxV2; const eventStore = yield* EventStore.EventStoreV2; const projectionStore = yield* ProjectionStore.ProjectionStoreV2; @@ -719,6 +721,16 @@ const layerBase: Layer.Layer< const commitProjectCommandEffect = Effect.fn("orchestrationV2.EventSink.commitProjectCommand")( function* (input: Parameters[0]) { + const existing = yield* commandReceipts.getProjectByCommandId(input.commandId); + if (Option.isSome(existing)) return { receipt: existing.value, committed: false }; + if (Option.isSome(yield* commandReceipts.getByCommandId(input.commandId))) { + return yield* Effect.fail( + `Command ${input.commandId} was already used by a thread command.`, + ); + } + // Settings takes its own lock before reading SQL. Never hold the SQL writer while + // waiting for it. The command lock keeps concurrent collisions from rerunning this write. + if (input.beforeCommit !== undefined) yield* input.beforeCommit; const result = yield* commitThenPublish( Effect.gen(function* () { const reserved: CommandReceiptStore.ProjectCommandReceiptV2 = { @@ -735,9 +747,6 @@ const layerBase: Layer.Layer< } const event = yield* eventStore.appendProjectEvent(input.event); yield* projectStore.apply(event); - // A failed settings write must leave no accepted receipt to suppress its retry. - // Existing receipts return above, so successful replays never rerun this write. - if (input.beforeCommit !== undefined) yield* input.beforeCommit; const receipt = { ...reserved, resultSequence: event.sequence }; yield* commandReceipts.upsert(receipt); return { receipt, event }; @@ -903,7 +912,7 @@ const layerBase: Layer.Layer< ), ), commitCommand: (input) => - commitCommandEffect(input).pipe( + commandLocks.withLock(input.commandId, commitCommandEffect(input)).pipe( Effect.mapError( (cause) => new EventSinkWriteError({ @@ -914,7 +923,7 @@ const layerBase: Layer.Layer< ), ), commitRejectedCommand: (input) => - commitRejectedCommandEffect(input).pipe( + commandLocks.withLock(input.commandId, commitRejectedCommandEffect(input)).pipe( Effect.mapError( (cause) => new EventSinkWriteError({ @@ -925,19 +934,23 @@ const layerBase: Layer.Layer< ), ), commitProjectCommand: (input) => - commitProjectCommandEffect(input).pipe( - Effect.mapError( - (cause) => - new EventSinkWriteError({ commandId: input.commandId, eventCount: 1, cause }), + commandLocks + .withLock(input.commandId, commitProjectCommandEffect(input)) + .pipe( + Effect.mapError( + (cause) => + new EventSinkWriteError({ commandId: input.commandId, eventCount: 1, cause }), + ), ), - ), commitRejectedProjectCommand: (input) => - commitRejectedProjectCommandEffect(input).pipe( - Effect.mapError( - (cause) => - new EventSinkWriteError({ commandId: input.commandId, eventCount: 0, cause }), + commandLocks + .withLock(input.commandId, commitRejectedProjectCommandEffect(input)) + .pipe( + Effect.mapError( + (cause) => + new EventSinkWriteError({ commandId: input.commandId, eventCount: 0, cause }), + ), ), - ), stream: (input) => stream(input).pipe( Stream.mapError( diff --git a/apps/server/src/project/ProjectService.test.ts b/apps/server/src/project/ProjectService.test.ts index 402b722edd144..cc20ed5615358 100644 --- a/apps/server/src/project/ProjectService.test.ts +++ b/apps/server/src/project/ProjectService.test.ts @@ -530,6 +530,47 @@ it.effect("retries a failed settings write without accepting an incomplete proje }).pipe(Effect.provide(layerProjectServiceDependencies)), ); +it.effect("allows settings persistence to wait for an independent SQL reader", () => + Effect.gen(function* () { + const settings = yield* ServerSettings.ServerSettingsService; + const sql = yield* SqlClient.SqlClient; + const settingsWriteStarted = yield* Deferred.make(); + // Settings reloads SQL while holding its write semaphore. This reader starts outside + // the project transaction so it must acquire its own connection. + const settingsReader = yield* Deferred.await(settingsWriteStarted).pipe( + Effect.andThen(sql`SELECT 1`.pipe(Effect.orDie)), + Effect.forkChild({ startImmediately: true }), + ); + const service = yield* ProjectService.make.pipe( + Effect.provideService(ServerSettings.ServerSettingsService, { + ...settings, + getSettings: settings.getSettings.pipe( + Effect.map((current) => ({ ...current, projectSettingsFolded: true })), + ), + updateSettings: (patch) => + Deferred.succeed(settingsWriteStarted, undefined).pipe( + Effect.andThen(Fiber.join(settingsReader)), + Effect.andThen(settings.updateSettings(patch)), + ), + }), + ); + const projectId = ProjectId.make("project:settings-sql-lock"); + yield* service.create({ + commandId: CommandId.make("command:settings-sql-lock:create"), + projectId, + title: "Settings SQL lock", + workspaceRoot: "/work/settings-sql-lock", + }); + assert.isTrue( + (yield* service.update({ + commandId: CommandId.make("command:settings-sql-lock:update"), + projectId, + autoPull: true, + })).autoPull, + ); + }).pipe(Effect.provide(layerProjectServiceDependencies)), +); + it.effect("keeps project snapshots available when optional metadata enrichment fails", () => Effect.gen(function* () { const layerFailingMetadata = Layer.merge(