feat(mcp): update environment preferences#8728

closed5
agents/mcp-environment/read
agents/mcp-environment/preferences
openedAug 30, 2026byJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 3d ago
3 unresolved threads · Jump to file
0 / 15 viewed15 files changedLine totals unavailable

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

M
macroscopeappBotAug 30, 2026

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

J
Julius MarmingeAug 30, 2026

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

M
macroscopeappBotAug 30, 2026

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

J
Julius MarmingeAug 30, 2026

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

M
macroscopeappBotAug 30, 2026

🟢 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.

suggestion
1The 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, 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.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
text
1In file @docs/internals/environment-mcp.md around line 35:
2
3The 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
5Also 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.
J
Julius MarmingeAug 30, 2026

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.