feat(mcp): expose selected environment preferences#10560

merged2
agents/mcp-thin/preview
agents/mcp-thin/preferences
openedSep 7, 2026byJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 3d ago
1 unresolved thread · Jump to file
Selected thread: apps/server/src/mcp/toolkits/environment/handlers.ts:78 — This new handler layer introduces real backend behavior that no test exercises: the cross-environment `capability_denied` guard in `access`, the live full-access/default caller guard, and the 4,000-character `customInstructions` truncation plus `truncated` flag in `preferences`. `core.test.ts` and the adapter test only assert tool names/schemas. Consider adding a focused `environment/handlers.test.ts` that builds `EnvironmentHandlersLive` over test layers for the external services only (`ServerSettingsService.layerTest`, a stub `ServerEnvironment` descriptor, and the MCP invocation context), asserting the denied paths and the truncation boundary rather than mocking the handler logic itself. <sup>Posted via Macroscope — <a href="https://github.com/pingdotgg/t3code/runs/101847660547">Effect Service Conventions</a></sup> <!-- macroscope-meta: {"kind":"check_run","config":".macroscope/check-run-agents/effect-service-conventions.md","check":"Effect Service Conventions"} -->
0 / 7 viewed7 files changedLine totals unavailable

apps/server/src/mcp/toolkits/environment/handlers.ts · Earlier revision fcfc8f36 · right 43–78

Original diff
@@ -0,0 +1,79 @@
+import { OrchestratorMcpFailure, type ServerSettings } from "@t3tools/contracts";
+import * as Effect from "effect/Effect";
+import * as Environment from "../../../environment/ServerEnvironment.ts";
+import * as ThreadCommandExecutor from "../../../orchestration-v2/ThreadCommandExecutor.ts";
+import * as Settings from "../../../serverSettings.ts";
+import { McpInvocationContext } from "../../McpInvocationContext.ts";
+import { readCaller, readMutationCaller, unavailable } from "../../threadAccess.ts";
+import { EnvironmentToolkit } from "./tools.ts";
+
+function preferences(settings: ServerSettings) {
+  const {
+    defaultThreadEnvMode,
+    newWorktreesStartFromOrigin,
+    enableProviderUpdateChecks,
+    backgroundActivity,
+    sourceControlWritingStyle,
+  } = settings;
+  const characters = Array.from(sourceControlWritingStyle.customInstructions);
+  return {
+    defaultThreadEnvMode,
+    newWorktreesStartFromOrigin,
+    enableProviderUpdateChecks,
+    backgroundActivity: { profile: backgroundActivity.profile },
+    sourceControlWritingStyle: {
+      ...sourceControlWritingStyle,
+      customInstructions: characters.slice(0, 4000).join(""),
+      truncated: characters.length > 4000,
+    },
+  };
+}
+const access = (writable = false) =>
+  Effect.gen(function* () {
+    const context = yield* writable ? readMutationCaller() : readCaller();
+    const environment = yield* Environment.ServerEnvironment;
+    const descriptor = yield* environment.getDescriptor;
+    if (descriptor.environmentId !== context.scope.environmentId)
+      return yield* new OrchestratorMcpFailure({
+        code: "capability_denied",
+        message: "This credential belongs to another environment.",
+      });
+    return { ...context, descriptor, settings: yield* Settings.ServerSettingsService };
+  });
+export const EnvironmentHandlersLive = EnvironmentToolkit.toLayer({
+  t3_environment_read: () =>
+    Effect.gen(function* () {
+      const { descriptor, settings } = yield* access();
+      const current = yield* settings.getSettings.pipe(Effect.mapError(unavailable));
+      return {
+        environmentId: descriptor.environmentId,
+        label: descriptor.label,
+        serverVersion: descriptor.serverVersion,
+        platform: descriptor.platform,
+        preferences: preferences(current),
+      };
+    }),
+  t3_environment_preferences_update: (patch) =>
+    Effect.gen(function* () {
+      const scope = yield* McpInvocationContext;
+      const executor = yield* ThreadCommandExecutor.ThreadCommandExecutor;
+      return yield* executor.withLock(
+        scope.threadId,
+        Effect.gen(function* () {
+          const { caller, settings } = yield* access(true);
+          if (
+            caller.archivedAt !== null ||
+            caller.runtimeMode !== "full-access" ||
+            caller.interactionMode !== "default"
+          )
+            return yield* new OrchestratorMcpFailure({
+              code: "capability_denied",
+              message: "Preference updates require a live full-access/default thread.",
+            });
+          return preferences(
+            yield* settings.updateSettings(patch).pipe(Effect.mapError(unavailable)),
+          );
+        }),
+      );
+    }),

apps/server/src/mcp/toolkits/environment/handlers.ts · right 43–78

M
macroscopeappBotSep 7, 2026

This new handler layer introduces real backend behavior that no test exercises: the cross-environment capability_denied guard in access, the live full-access/default caller guard, and the 4,000-character customInstructions truncation plus truncated flag in preferences. core.test.ts and the adapter test only assert tool names/schemas.

Consider adding a focused environment/handlers.test.ts that builds EnvironmentHandlersLive over test layers for the external services only (ServerSettingsService.layerTest, a stub ServerEnvironment descriptor, and the MCP invocation context), asserting the denied paths and the truncation boundary rather than mocking the handler logic itself.

Posted via Macroscope — Effect Service Conventions

J
Julius MarmingeSep 7, 2026

Added shared core projection coverage in fe7137d675f513720df0499b5ff566f4ef40c7d5: the actual preference mapper omits provider/private fields and truncates 4,001 emoji to 4,000 without breaking code points. Core tests pass 4/4 here, and the composed core/integration batch is 6/6. Caller scope remains in the shared helper; the writable check stays under the existing ThreadCommandExecutor through updateSettings. The maintainer requested shared MCP tests and existing service tests instead of a dedicated handler suite per tool, so persistence/notification tests remain with ServerSettingsService.

M
macroscopeappBotSep 7, 2026

The shared projection test resolves the truncation/allowlisting portion. However, I still don’t see coverage for the cross-environment capability_denied path or the full-access/default caller guard, so the original concern is only partially addressed.