Skip to content

fix(server): expose thread executor to environment preferences - #17505

Closed
Xon333 wants to merge 1 commit into
pingdotgg:mainfrom
Xon333:fix/preferences-runtime-executor
Closed

Xon333 wants to merge 1 commit into
pingdotgg:mainfrom
Xon333:fix/preferences-runtime-executor

Conversation

@Xon333

@Xon333 Xon333 commented Oct 9, 2026 •

Copy link
Copy Markdown

Authorized calls to t3_environment_preferences_update require ThreadCommandExecutor, but the application runtime supplies that service only privately to other consumers. Export the same existing layer from the shared runtime so the production MCP context receives it and Effect layer memoization preserves the shared thread lock.

This is a missing-dependency correction for the existing tool, related to #15131. The production change is one line. It changes no preference defaults, permission rules or settings contract.

Related open PR #15337 fixes the same defect at the older MCP registration boundary. This PR uses the shared runtime boundary on base ec80933ac8cd02fec5c97b342462ccc9567cdb1e. The earlier proposal needs adaptation to maintained toolkit/context APIs; maintainers should select one implementation. This draft's distinct coverage includes OAuth client access, preservation of custom instructions and caller-mode rechecks while waiting for a thread lock.

Validation at head 886a31860b8aebbcc6c2066d5b38881f1960e9e4, Linux and Node 24.13.1 with frozen-lockfile dependencies:

  • Removing the exported layer fails the specific runtime regression with Service not found: t3/orchestration-v2/ThreadCommandExecutor.
  • The corrected runtime and environment-handler files previously passed 76 tests with one fork worker; scoped lint and formatting passed. The unchanged source did not need those successful checks repeated.
  • Package typecheck remains incomplete. Earlier native compiler attempts exited 137 after memory growth beyond 5 GB. The exact TypeScript 7.0.2 + Effect tsgo 0.46.1 compiler defaults to four independent checkers even with GOMAXPROCS=1.
  • One changed approach checked the unchanged full server project with --noEmit --project apps/server/tsconfig.json --checkers 1 --extendedDiagnostics, through the worker's bounded t3-check wrapper (2 CPUs, 2.5 GiB soft / 3 GiB hard RAM, no swap, 256 tasks). It retained all configured TypeScript and Effect diagnostics. After 900 seconds, systemd ended the scope for runtime timeout and killed the checker after the five-second stop timeout: exit 137, elapsed 905.41 seconds, reported memory peak 2.7 GiB, no compiler diagnostics. This was a timeout under memory pressure, not a proven source type error or OOM. No further heavy retry was made.
  • The exact-head CI run https://github.com/pingdotgg/t3code/actions/runs/37940662037 was queried once and reported action_required; no completed CI typecheck is claimed.

The regression uses the maintained toolkit registration helper, real runtime export and test settings. It does not start the complete installed server, execute a real provider turn or write live preferences. The PR remains draft while package validation and the overlapping proposal are unresolved; upstream maintainers retain the implementation and merge decision.

Implemented and assessed with GPT-6.1 Sol High through the Codex harness in T3 Code, retaining the selected Codex account and Standard service.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 9, 2026
@juliusmarminge

Copy link
Copy Markdown
Member

Note

Grok responding on behalf of Julius.

Thanks for working on this! #17571 just landed on main and fixes the same bug (#15131): layerEnvironmentToolkit now provides ThreadCommandExecutor to the environment toolkit, and there's a regression test that calls t3_environment_preferences_update through the real /mcp registration. That makes this PR redundant, so I'm closing it as superseded. If you think something here is still missing on main, reply and we can take another look.

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

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants