feat(mcp): update environment preferences#8728
3 unresolved threads ยท Jump to file
- apps/server/src/mcp/EnvironmentPreferencesMcpService.test.ts:38โ51 ยท <!-- MURMUR_IGNORE --> `mcp/EnvironmentPreferencesMcpService.test.ts:51` These two service modules
- apps/server/src/orchestration-v2/KeyedSerialExecutor.ts:60โ66 ยท The new `ThreadDispatchLockV2` service exports its layer as `threadDispatchLockLayer`; the conventio
- docs/internals/environment-mcp.md:35โ35 ยท ๐ข **Low** `internals/environment-mcp.md:35` The documentation incorrectly states that the mutation
apps/server/src/mcp/EnvironmentPreferencesMcpService.test.ts ยท Earlier revision cfc5143f ยท right 38โ51
Original diff
@@ -0,0 +1,357 @@
+import { assert, expect, it } from "@effect/vitest";
+import * as NodeServices from "@effect/platform-node/NodeServices";
+import {
+ CommandId,
+ DEFAULT_SERVER_SETTINGS,
+ EnvironmentId,
+ ENVIRONMENT_MCP_MAX_WRITING_INSTRUCTIONS,
+ EnvironmentMcpPreferencesUpdateInput,
+ ProjectId,
+ ProviderInstanceId,
+ ServerSettings,
+ ThreadId,
+ type OrchestrationV2ThreadShell,
+} from "@t3tools/contracts";
+import { fromLenientJson } from "@t3tools/shared/schemaJson";
+import * as DateTime from "effect/DateTime";
+import * as Deferred from "effect/Deferred";
+import * as Effect from "effect/Effect";
+import * as Fiber from "effect/Fiber";
+import * as FileSystem from "effect/FileSystem";
+import * as Layer from "effect/Layer";
+import * as Option from "effect/Option";
+import * as Ref from "effect/Ref";
+import * as Schema from "effect/Schema";
+import * as Stream from "effect/Stream";
+
+import * as ServerSecretStore from "../auth/ServerSecretStore.ts";
+import * as ServerConfig from "../config.ts";
+import * as ServerEnvironment from "../environment/ServerEnvironment.ts";
+import {
+ type KeyedSerialExecutor,
+ makeKeyedSerialExecutor,
+ ThreadDispatchLockV2,
+ threadDispatchLockLayer,
+} from "../orchestration-v2/KeyedSerialExecutor.ts";
+import { OrchestratorV2 } from "../orchestration-v2/Orchestrator.ts";
+import { makeLayer as makeProviderAdapterRegistryLayer } from "../orchestration-v2/ProviderAdapterRegistry.ts";
+import {
+ layer as threadManagementLayer,
+ ThreadManagementService,
+} from "../orchestration-v2/ThreadManagementService.ts";
+import { makeOrchestratorV2ReplayLayerWithRegistry } from "../orchestration-v2/testkit/ProviderReplayHarness.ts";
+import { SqlitePersistenceMemory } from "../persistence/Layers/Sqlite.ts";
+import { ProviderRegistry } from "../provider/Services/ProviderRegistry.ts";
+import { makeProviderRegistryMock } from "../provider/testUtils/providerRegistryMock.ts";
+import * as ServerSettingsModule from "../serverSettings.ts";
+import {
+ EnvironmentMcpService,
+ layer as environmentMcpLayer,
+ make as makeEnvironmentMcpService,
+} from "./EnvironmentMcpService.ts";apps/server/src/mcp/EnvironmentPreferencesMcpService.test.ts ยท right 38โ51
mcp/EnvironmentPreferencesMcpService.test.ts:51
These two service modules are imported with renamed member aliases (layer as threadManagementLayer, layer as environmentMcpLayer, make as makeEnvironmentMcpService), which erases the module namespace at a service boundary. The convention here is a namespace import so the module's public shape stays visible (ThreadManagement.ThreadManagementService / ThreadManagement.layer, EnvironmentMcp.EnvironmentMcpService / EnvironmentMcp.make / EnvironmentMcp.layer) โ the sibling EnvironmentMcpService.test.ts already avoids aliasing.
Suggested fix: switch to import * as ThreadManagement from "../orchestration-v2/ThreadManagementService.ts"; and import * as EnvironmentMcp from "./EnvironmentMcpService.ts";, then update the four usages in this file (threadLayer, the environmentMcpLayer.pipe(...) provide, makeEnvironmentMcpService.pipe(...), and the yield* EnvironmentMcpService / yield* ThreadManagementService acquisitions) to the namespaced members.
No longer relevant as of 1c6dee0730ba0a8399e9b5b3b488507e729b89e0
Fixed at 1c6dee0730b. The integration test now imports ThreadManagementService and EnvironmentMcpService as module namespaces and accesses each service, layer, and make member through that namespace.
apps/server/src/orchestration-v2/KeyedSerialExecutor.ts ยท Earlier revision fef7ed30 ยท right 60โ66
Original diff
@@ -53,3 +56,14 @@ export const makeKeyedSerialExecutor = <Key>(): Effect.Effect<KeyedSerialExecuto
),
} satisfies KeyedSerialExecutor<Key>;
});
+
+/** The single in-process admission boundary for commands targeting a V2 thread. */
+export class ThreadDispatchLockV2 extends Context.Service<
+ ThreadDispatchLockV2,
+ KeyedSerialExecutor<ThreadId>
+>()("t3/orchestration-v2/KeyedSerialExecutor/ThreadDispatchLockV2") {}
+
+export const threadDispatchLockLayer = Layer.effect(apps/server/src/orchestration-v2/KeyedSerialExecutor.ts ยท right 60โ66
The new ThreadDispatchLockV2 service exports its layer as threadDispatchLockLayer; the convention is a canonical export const layer for the module that owns a service (every other orchestration-v2 service module here does that, reserving suffixed names like layerMemory/layerWithLegacyImporter for alternative layers).
Suggested fix: move the tag plus its layer into a dedicated module (e.g. ThreadDispatchLock.ts) that exports layer, and consume it as a namespace at the six call sites (ThreadDispatchLock.ThreadDispatchLockV2, ThreadDispatchLock.layer). Renaming the export to layer in this file and importing * as KeyedSerialExecutor also satisfies the convention if you prefer to keep it colocated with the generic executor.
Posted via Macroscope โ Effect Service Conventions
Fixed at 1c6dee0730b. ThreadDispatchLockV2 remains in KeyedSerialExecutor.ts with the exact existing Context service key; the layer export is now the canonical layer and every service consumer uses the KeyedSerialExecutor module namespace. No second lock or renamed service identity was introduced.
docs/internals/environment-mcp.md ยท Earlier revision 7f56c9f2 ยท right 35โ35
Original diff
@@ -22,3 +22,17 @@ The preference projection is a literal allowlist. It must be extended field by field; never spread a server settings or provider configuration object into an MCP result. Environment reads use cached provider state and do not call provider refresh or the usage service. + +`t3_environment_preferences_update` accepts a separate literal patch schema and +constructs an existing `ServerSettingsPatch` field by field. The mutation runs +under `ThreadDispatchLockV2` for the calling thread. It reloads the caller under +that same lock, requires `full-access` plus `default`, and holds the lock through +`ServerSettingsService.updateSettings`. Ordinary V2 policy writers use the same +lock, so a concurrent downgrade is observed before persistence. The settings +service remains responsible for normalization, durable storage, and existing +cross-client notifications. + +The mutation exposes preset background profiles, not arbitrary timing
docs/internals/environment-mcp.md ยท right 35โ35
๐ข Low internals/environment-mcp.md:35
The documentation incorrectly states that the mutation exposes only preset background profiles, so consumers are led to believe the supported custom BackgroundActivityProfile value cannot be selected. Mention custom alongside the presets.
| 1 | The mutation exposes preset and custom background profiles, not arbitrary timing |
Also found in 1 other location(s)
docs/user/environment-mcp.md:29
The background-activity list says the update tool accepts only
balanced,performance, orbattery-saver, but its input schema acceptsBackgroundActivityProfile, which also includescustom. Agents following this documentation will incorrectly believe they cannot select the supportedcustomprofile.
๐ Reply "fix it for me" or copy this AI Prompt for your agent:
| 1 | In file @docs/internals/environment-mcp.md around line 35: |
| 2 | |
| 3 | The documentation incorrectly states that the mutation exposes only preset background profiles, so consumers are led to believe the supported `custom` `BackgroundActivityProfile` value cannot be selected. Mention `custom` alongside the presets. |
| 4 | |
| 5 | Also found in 1 other location(s): |
| 6 | - docs/user/environment-mcp.md:29 -- The background-activity list says the update tool accepts only `balanced`, `performance`, or `battery-saver`, but its input schema accepts `BackgroundActivityProfile`, which also includes `custom`. Agents following this documentation will incorrectly believe they cannot select the supported `custom` profile. |
No change is needed here. The update input intentionally uses BackgroundActivityProfile, whose only values are balanced, performance, and battery-saver (packages/contracts/src/settings.ts:629-634). custom belongs to the separate BackgroundActivityProfileSelection output/settings schema and requires timing overrides; this MCP surface deliberately does not accept arbitrary timing overrides. The current user and internal docs accurately describe the mutation boundary.