Skip to content

feat(mcp): update environment preferences - #8728

Closed
juliusmarminge wants to merge 5 commits into
agents/mcp-environment/readfrom
agents/mcp-environment/preferences
Closed

feat(mcp): update environment preferences#8728
juliusmarminge wants to merge 5 commits into
agents/mcp-environment/readfrom
agents/mcp-environment/preferences

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Aug 30, 2026

Copy link
Copy Markdown
Member

Problem

Agents could inspect current environment preferences but had no narrow, permission-aware way to update useful nonsecret server-owned defaults.

Change

Add t3_environment_preferences_update for an explicit typed allowlist: default thread environment mode, new-worktree origin behavior, provider update checks, normalized background-activity presets, and bounded source-control writing style.

Behavior

Omitted fields remain unchanged, explicit empty custom instructions clear them, and the result comes from normalized persisted settings rather than request inputs. Mutation requires a full-access/default caller. A fresh caller shell is checked under the canonical ThreadCommandExecutor lock held through settings persistence, so a concurrent V2 downgrade wins safely. Existing settings notifications continue to update web, desktop, and mobile clients. Provider configuration, credentials, paths, browser access, observability, and arbitrary settings remain excluded.

Focused validation

  • real settings persistence, notification, normalization, omission/clear, and Unicode code-point tests
  • Deferred-gated real V2 caller-downgrade race coverage
  • production HTTP tools/list, Claude read-only allowlist, and presentation coverage
  • 6 focused files / 18 tests passed across the complete native stack
  • server and contracts scoped typechecks, targeted formatting, and git diff --check

Dependency

Upper layer of native stack 8782. Depends on #8726 for the safe current-environment read foundation and preference projection.

Implemented by GPT-5.6-Sol via Codex in T3 Code.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: be30e0c9-f657-4a2e-902b-cf8e8d569c62

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Warning

Your free Security trial is over. An organization admin can activate Security or dismiss this notice.


Comment @coderabbitai help to get the list of available commands.

@juliusmarminge
juliusmarminge marked this pull request as ready for review August 30, 2026 02:14
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Aug 30, 2026
@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

⚠️ The latest CI run did not produce a thread transfer result for aae1adc.

This comment will update automatically after the next completed run.

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effect service conventions review: one deviation found in the new ThreadDispatchLockV2 service definition. The rest of the change (dependency acquisition via yield* Foo.Foo in EnvironmentMcpService.make, Layer.effect canonical layer, inline service interface, subpath namespace imports of effect/*, test-only Layer.succeed/Layer.mock/provideService seams) matches the conventions.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/orchestration-v2/KeyedSerialExecutor.ts Outdated

@macroscopeapp macroscopeapp Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One convention finding on the new test module's service imports. The prior finding on threadDispatchLockLayer in KeyedSerialExecutor.ts still stands but is not re-posted.

Posted via Macroscope — Effect Service Conventions

Comment thread apps/server/src/mcp/EnvironmentPreferencesMcpService.test.ts
@macroscopeapp

macroscopeapp Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This adds a production MCP mutation that durably changes environment-wide defaults and operational behavior, including thread environments, background activity, provider checks, and source-control writing. Its shared orchestration locking and cross-client settings notifications make the blast radius broader than a small isolated option.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

No code changes detected at aae1adc. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge
juliusmarminge force-pushed the agents/mcp-environment/preferences branch 2 times, most recently from daa079a to 7f56c9f Compare August 30, 2026 17:37
Comment thread docs/internals/environment-mcp.md
@juliusmarminge
juliusmarminge force-pushed the agents/mcp-environment/preferences branch from 7f56c9f to 57a6aeb Compare August 30, 2026 20:13
@juliusmarminge
juliusmarminge force-pushed the agents/mcp-environment/preferences branch 3 times, most recently from 44a966e to 39feca5 Compare September 5, 2026 06:23
@juliusmarminge

Copy link
Copy Markdown
Member Author

Superseded as the active MCP proposal by #10560 in the rebuilt MCP stack, following the maintainer request to review thin service wrappers separately from service hardening.

This is not a full feature-parity replacement. Updates require a live full-access/default caller under the existing ThreadCommandExecutor. Provider configuration, secrets and admin controls are not exposed. No new settings store or lock service.

Closing this mixed proposal without merging. The remote branch agents/mcp-environment/preferences and exact head aae1adc5097ced03fa716e4222db448e6425310f are retained so the service changes can be extracted for separate review later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant