Skip to content

feat(mcp): route all tools to linked environments - #304

Merged
lukemaj merged 4 commits into
mainfrom
feat/mcp-all-peer
Oct 14, 2026
Merged

lukemaj merged 4 commits into
mainfrom
feat/mcp-all-peer

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What: Every T3 MCP toolkit can target a linked environment while retaining its caller and link access limits.
Why: Existing peer forwarding covered only a small set of orchestration tools, leaving most capabilities local.
So what: Agents can use destination-owned services across machines after this PR passes both exact-head reviews and CI; the next normal build delivers it.

Closes #302. Continues #164 / PR #290. Extends the existing component, with no new transport or design round.

The shared MCP declaration boundary routes tools after their existing access check. Destination declarations check them again, with mode narrowing and source/destination capability intersection. Caller-bound tools use contextThreadId, restricted to this link's live destination credential. Full-access/default direct links can manage environment resources; narrower links, linked descendants, foreign thread selection and onward links remain fenced.

Delegation keeps its source task record and completion wake: top-level environmentId aliases the existing target.environmentId path, rejecting conflicting destinations. Scheduled tasks store immutable link origin, carry it into ordinary/checked launches and crash replays, and refuse foreign binding or execution. Existing UI form edits cannot erase ownership; the nullable fork column preserves legacy tasks without taking an upstream migration number.

Spectrum registration keeps the caller's link origin on its transcript and every Color. Image results cross as bytes before presentation filtering. Attachment preparation supplies the destination upload origin; remote launch attachments retain their existing refusal, while destination uploads can be sent through the attachment tools. HTML prepares local images on the destination and stores its self-contained output in the source thread, preserving the attachment reference already consumed by web, desktop and mobile.

Upstream first: inspected the complete later peer PR diffs. pingdotgg/t3code #16684 remains open and has the same limited forwarding already ported; #17129 concerns link setup. No later merged implementation forwards the remaining groups. Source links and the per-tool table are in #302. File overlap is recorded on #166.

Proof

At 23cdfa11a29aabf88c564f4114d1942c64774231, rebased onto main 366f2fff6f25f47c14ca86163d8ee8e4dd6c0961:

  • npx -y pnpm@11.10.0 install completed in the exclusive worktree.
  • npx -y pnpm@11.10.0 exec vp test run <files>: 396 passed in 31 files, four optional real-browser tests skipped. Scope: PeerToolRouting, PeerForwarding, PeerLinks; McpToolAccess and its race test, McpSessionRegistry, McpHttpServer, linkOrigin; toolkit core and thread/project/environment/preview/previewControls/device/pullRequests handler tests; Spectrum Launch integration; HtmlRender; orchestrator and preview tool schema tests; RemoteDelegation; OrchestratorMcpService/activity/toolkit integration; real ThreadLaunch; scheduled service/schedule/webhook; checked engine/integration; legacy scheduled-origin schema migration.
  • Real sockets, OAuth grants, tool schemas and access declarations prove destination execution and narrower-link refusal per group. Real Spectrum and scheduled-task persistence prove descendant ownership and actual onward refusal; restarted scheduled services retain origin. Both delegation destination spellings preserve the source parent record/completion wake, use target Prism and refuse a request above the link ceiling. Real HTML services/stores plus the shared client parser prove the source attachment contains the prepared destination document and image bytes, with no orphan peer page. Domain fakes stop other tests at their service boundaries.
  • Targeted tsc --noEmit -p apps/server/tsconfig.json and -p packages/contracts/tsconfig.json passed.
  • Lint and formatting on the 49 changed TypeScript files passed; lint reports three existing warnings: two unused callbacks in McpSessionRegistry and optional chaining in an unchanged ThreadLaunchService test. Fork stack ownership check passed with 36 features.
  • Fixed the initial CI failures at their owners: optional branded schema field descriptions, an exact-key test that incorrectly froze a compatible parameter schema, and missing carried-hook records. No checker weakened.

No real linked-machine, device or browser interaction was performed. No repo-wide local check, version bump, deployment or restart. Website impact: none, because this changes server MCP reach and does not change marketing or hosted client content.

Elon record

Requirements and who asked: User, 2026-10-14: full linked MCP reach, existing scopes and link narrowing, per-tool inventory, upstream-first, focused proof, exact-head Sol/Grok reviews and authorized merge.
Deleted: New transport, new design round, arbitrary file copying, onward link chaining, version bumps, repo-wide local checks and deployment.
Bottleneck: Routing must preserve domain ownership as well as access: source-owned delegation, durable scheduled descendants, authenticated destination context and client-consumable artifacts.
Checked myself: Read the upstream peer diffs and current handlers/access/transport; executed focused OAuth/peer tests, real Spectrum persistence and HTML attachment consumption, scoped typechecks/lint and fork ownership validation.

Implementation: gpt-6.1-sol, Codex harness in T3 Code. Independent reviewers: gpt-6.1-sol and grok-4.7, each required to publish findings followed by success on this PR's exact final head.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XL labels Oct 14, 2026
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #304 is ready for the required independent reviews at 75fb6b5.
Why: Focused forwarding/access proof and server/contracts typechecks passed; merge still requires two independent verdicts and green CI.
So what: Sol gpt-6.1-sol and Grok grok-4.7 children review this exact candidate, post findings, then publish review/independent-sol and review/independent-grok statuses.

No deployment or restart is part of this PR. Source-owned remote launch attachments retain their existing refusal; destination-owned attachments can be uploaded and sent with the routed attachment tools. Caller-bound tools require contextThreadId on the destination. Direct full-access/default links gain environment-level operations within that grant, while narrowed links, linked descendants, foreign-thread contexts and onward forwarding remain refused.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 366f2ff · PR result: 23cdfa1 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Sol review found two blocking defects at 75fb6b5677bca70f23be3d08f52a16f5c05a65ad.
Why: Spectrum forwarding drops link ownership, and remote HTML publication returns an attachment reference that the originating client resolves locally.
So what: The implementation owner should fix these paths, add focused behavioral proof, and request fresh exact-head reviews before merge.

Verdict: changes required. Reviewed the full Issue #302 and PR #304 bodies, the cumulative diff from 14f6e5d5b3465e12fb77362cc0ef88db5f8f1fac to 75fb6b5677bca70f23be3d08f52a16f5c05a65ad, and the surrounding handlers, services and client consumers. This is an independent review, with no implementation changes.

1. P1: Spectrum forwarding escapes the peer's thread and onward-link fences

Location: the new direct-peer exception, the now-routed Spectrum tool, and Spectrum shell creation.

Trigger: call start_spectrum through a full-access/default peer link, passing callerThreadId and omitting contextThreadId, as the new Spectrum routing test does. startsThreads invokes assertNotLinked, but the new directFullAccessPeer exception now bypasses Spectrum's explicit peer refusal. The handler calls readThread(input.callerThreadId) without assertSameLink, so that ID can name the user's own thread or a thread from a different link; it is not restricted to the authenticated link's context.

Even when the caller thread is link-owned, SpectrumLaunchService.shell constructs the transcript and all Color threads without copying or stamping linkOrigin. Registration commits those shells directly. Once a Color starts its normal provider run, its scope has no linked client and its shell has no origin, so callerLinkOrigin returns undefined. It can then change unrelated threads, mutate the environment and use the destination's onward links through the normal MCP paths. This defeats the explicit requirement that linked descendants retain their fences and cannot forward onward; it also leaves the initiating peer unable to manage its own new Spectrum through writesThreads, because those new threads have no matching origin.

Required correction: keep Spectrum's destination caller within the authenticated link and retain that origin on the transcript and every Color. The full-access environment-mutation exception must not implicitly authorize unrelated caller selection or unmarked descendant creation. Prove this with the real Spectrum registration service and persisted shells, including a foreign caller refusal and refusal of an actual Color's onward forwarding. The current routing fixture mocks SpectrumLaunchService.register, so its green result cannot detect either failure.

2. P2: Forwarded html_render reports success but cannot display its page

Location: the new routed HTML input, the handler's returned reference, and the client environment selection.

Trigger: an agent in environment A calls html_render with environment B and a valid B-owned contextThreadId. The real HtmlRender.publish writes the HTML only into B's attachments directory and returns {attachmentId, title, height, heights?}. It does not append a timeline item to B's context thread. Forwarding returns that same htmlRender reference to A's provider tool call, where htmlRenderFromToolItem turns it into an inline timeline row. HtmlRenderTimelineRow always uses the originating thread's activeThreadEnvironmentId, and HtmlRenderFrame requests the attachment from that environment. A has no such file, so the page cannot load; B has no tool-result timeline entry to display it either. The success message nevertheless says it was shown above the reply.

Required correction: make remote publication produce a reachable render in its intended destination thread, with attachment resolution against the environment that owns it. Prove publication and reference consumption, not merely that a mocked publish method was invoked. The routing test's box-html reference does not exercise the storage or client path. This also needs a decision for the separate mobile consumer and desktop's web client.

Coverage and proof limits

Inspected every changed routing group: schedules, thread lifecycle/metadata/configuration/search/transfers, queues, pending requests, projects, worktrees, preferences, secrets, attachments, Issue/PR links, browser/preview controls, devices, HTML and Spectrum. Also traced destination context ownership, credential lookup/expiry/revocation, source/target capability intersection, mode rechecks, thread mutation checks, the direct full-access exception, onward refusal, production image registration/decoding, destination upload origins, local metadata stripping, wait chunking/run pinning and namespaced retry keys. No additional supported blocking finding was established in those inspected paths.

The supplied 166-test/16-file result, typechecks and targeted lint are the implementer's reported evidence; I inspected the relevant test sources and did not rerun commands. Real-socket coverage exercises transport, OAuth, schemas and declarations, while domain mocks leave the two effects above unproven. No browser, live linked-machine/device execution, installation or restart was performed. Returned device launcher/config paths and preview-recording paths remain destination-local; using those files or driving the returned CLI requires execution in that destination environment, rather than automatic local file transfer. Screenshots cross as bytes, and attachment upload origins explicitly identify the destination.

Elon record

Requirements and who asked: User requested independent exact-head review for #302/#304, full linked MCP reach with existing access/mode/link fences, and a findings comment followed by review/independent-sol.
Deleted: Stylistic findings, redesign, implementation changes, duplicate test runs, browser use and live-machine mutations.
Bottleneck: Mocked destination services establish forwarding but miss persisted ownership and client-visible artifact consumption.
Checked myself: Read the cumulative diff, Issue/PR bodies, real Spectrum shell/registration paths, peer fences, HTML publication and web asset consumers at the exact reviewed SHA.

Reviewer: gpt-6.1-sol, Codex harness in T3 Code, high reasoning effort.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #304 has a new proved candidate at 298cbdc055bd3062cf2b3c2c9f3e977820178d42.
Why: Sol's two blockers required real Spectrum ownership persistence and HTML attachment consumption, and CI exposed missing hook records and schema-test assumptions.
So what: Fresh exact-head independent reviews and green CI remain the merge gate; merge authority is already delegated.

Sol finding responses: Spectrum checks the destination caller's link and mode limits; transcript and Colors inherit linkOrigin, with registration guards retaining modes/origin across commit. The real SQL-backed registration test proves every persisted descendant retains its origin and actual PeerForwarding rejects its onward calls. A real-socket test refuses a caller from another link. HTML prepares on the destination and publishes that self-contained output in the source thread's attachment store, with a bounded prepared-document schema/service boundary. Real services, destination image bytes, both stores and the shared client reference parser prove the returned source attachment is readable, has one bootstrap and leaves no orphan page on the peer. Web, desktop and mobile keep their current local reference contract.

At this exact candidate, 185 focused tests pass in 20 files; four optional real-browser cases skip. Targeted server/contracts typechecks, changed-file lint/format and fork stack ownership check pass. Lint has only the two existing registry callback warnings. No browser/live-machine checks, repo-wide local checks or restart. Earlier Grok round is still active on the old SHA; its eventual findings will be reconciled, never transferred as a verdict for this head.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Sol review requires changes at 298cbdc055bd3062cf2b3c2c9f3e977820178d42 because peer-created scheduled runs lose link ownership.
Why: The full-access direct-peer exception bypasses the scheduler's intentional refusal, while scheduled tasks and their later thread launches cannot carry linkOrigin.
So what: The implementation owner should retain the link fence through scheduling and execution, add focused persistence/onward-refusal proof, and obtain fresh exact-head reviews before merge.

Verdict: changes required, with one P1 blocker. This fresh independent round reads the complete Issue #302 and PR #304 bodies, both prior Sol findings and the response, the cumulative 40-file diff from 1bc11912ede0e53e9aa8d75f4e160f3e0873d699 to 298cbdc055bd3062cf2b3c2c9f3e977820178d42, and the surrounding authorization, services, persistence and artifact consumers. No implementation changes or merge were performed.

P1: Peer-created scheduled threads escape the link and onward-link fences

Changed entry point: the direct full-access exception in assertNotLinked, reached by the newly routed schedule_task.

Trigger: a full-access/default agent in environment A calls schedule_task with environment B, an existing B project with a default model, a normal interval schedule, a prompt and bindToCurrentThread:false, omitting contextThreadId. B receives a direct authenticated full-access peer scope with no calling thread. startsThreads calls assertNotLinked, but the new exception now admits it despite the handler's explicit refusal for scheduled work that cannot carry the link.

scheduleTask saves a task with threadId:null and the mode ceiling, but no origin. Neither ScheduledTask nor ScheduledTaskUpsertInput has an origin field. The ordinary no-fork-field hook saves that row unchanged. When the task fires, the real scheduler calls ThreadLaunchService.launch without linkOrigin. Thread creation copies an origin only when explicitly supplied; the orchestrator does not infer one from the task or creation source.

The resulting full-access agent has neither scope.client.linked nor a shell linkOrigin. callerLinkOrigin therefore returns undefined, allowing it to mutate unrelated threads/projects and call B's onward links. This is the same fence escape previously fixed for Spectrum, now through scheduled descendants. Allowing direct full-access environment management must not turn work that a peer creates into unrestricted local work. The new per-group schedule test exercises deletion with a mocked domain service, so it cannot detect this creation/execution path.

Required correction: retain authenticated link ownership in the durable scheduled-work state and every thread it creates, including later firing after persistence/restart, while preserving the full-access direct environment-management outcome. Review schedule rebinding and execution against that ownership as well. Add proof through real scheduled-task persistence and launch, then verify that the persisted thread cannot mutate an unrelated thread/environment or make an actual onward PeerForwarding call. An environment-mutation exception must not waive startsThreads' origin requirement merely because the caller is direct/full-access.

Prior findings adjudication

  • Spectrum ownership: resolved in the inspected correction. start_spectrum checks the selected caller's same-link ownership and modes; shell creation copies origin to the transcript and every Color, and the registration guard retains the loaded caller's modes/origin through commit. The SQL-backed integration test reads every persisted descendant and exercises actual PeerForwarding refusal before any onward link/client call. The real-socket test refuses another link's caller before registration.
  • HTML consumption: resolved in the inspected correction. The destination prepares the document and local images without storing an orphan page; the source validates the prepared variant and stores it through publishPrepared, which enforces the 25 MiB bound. The real-services/two-store test reads the returned source attachment through the shared reference parser, checks the peer image bytes and single bootstrap, and checks the destination store for no orphan page. Web and desktop's web client resolve that source reference in the originating environment; mobile's ThreadHtmlRender does the same through useAssetUrlState(props.environmentId, resource).

Coverage and proof limits

Reviewed routing/access and relevant consumers for schedules; thread lifecycle, metadata, configuration, search and transfers; queues and pending requests; projects/worktrees/preferences/secrets; attachments; Issue/PR links; browser/preview/device/HTML; and Spectrum. Inspected same-link context credential lookup, expiry/revocation, capability intersection, mode ceilings/live-run checks, direct versus descendant fences, onward refusal, production image registration/decoding, destination upload origins, local metadata removal, wait chunking/run pinning and namespaced retry keys. No additional supported blocker was established in those paths.

Inspected the supplied focused log showing 185 passed in 20 files, four skipped, the relevant test assertions, and the targeted typecheck/lint/format logs. These are reused implementation-run evidence, not tests executed by this reviewer. The schema-key test retains the original required fields while allowing compatible additions; schema descriptions and fork hook records repair their owners, without modifying CI/checker logic. Live GitHub CI is green on this head. CI and the focused mocks do not cover the scheduled-origin defect above. No duplicate tests, repo-wide local checks, live linked-machine/device/browser interaction, installation, server launch, restart or deployment occurred. The schedule finding is established by the reachable production source path, not a new runtime reproduction.

Destination panel state and destination-local device launcher/config or preview recording paths remain destination resources. Driving or consuming those local files requires execution on that destination; this review does not impose implicit local file transfer. Screenshots cross as bytes and pending uploads identify their destination origin. Remote launch attachment refusal is retained; remote attachment sends use destination resources.

Elon record

Requirements and who asked: User requested this independent exact-candidate review for #302/#304, full linked MCP reach retaining scopes/capabilities/modes/link ownership/live checks/no onward links, and a findings comment followed by review/independent-sol.
Deleted: Stylistic requirements, redesign, implementation edits, extra agents, duplicate test runs, browser/device use and machine mutations.
Bottleneck: The shared full-access exception admits scheduled creation without the durable ownership that subsequent execution needs.
Checked myself: Read the full cumulative diff, Issue/PR bodies and prior responses; traced actual task save, ordinary/fork scheduling hooks, scheduler launch, thread creation and peer fences; inspected persisted Spectrum/onward-refusal and real HTML/source-consumption proof plus current CI.

Reviewer: gpt-6.1-sol, Codex harness in T3 Code, high reasoning effort. No Grok verdict is claimed for this candidate.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of 75fb6b5677bca70f23be3d08f52a16f5c05a65ad fails.

Why: Two reachable ownership breaks: delegate_task's new top-level environmentId reparents the task, and start_spectrum creates children with no link fence.

So what: Fix both on the current PR head and get a new exact-head review. This status does not cover any other commit. I did not merge or change the branch.

Reviewed commit: 75fb6b5677bca70f23be3d08f52a16f5c05a65ad. Assigned base: 14f6e5d5b3465e12fb77362cc0ef88db5f8f1fac. Context: review/independent-grok.

This commit is not an ancestor of the open PR head 298cbdc055bd3062cf2b3c2c9f3e977820178d42. Their merge-base is 14f6e5d5b3465e12fb77362cc0ef88db5f8f1fac. I reviewed only the assigned commit, by git show, and did not review 298cbdc.

Required

1. Top-level environmentId on delegate_task drops parent ownership

Trigger: a source agent calls delegate_task with the routing field environmentId set to a linked environment (and contextThreadId of a link-owned destination thread), and does not use target.environmentId.

Required: Issue #302 says delegate_task already forwards target.environmentId through RemoteDelegation and keeps parent ownership and report-back. The tool description says the same.

Actual: McpRouting.invoke (apps/server/src/mcp/McpRouting.ts lines 158-165) special-cases only t3_thread_wait. Every other remote tool, including delegate_task, is sent whole by PeerForwarding.call. PeerForwarding.forward strips top-level environmentId and leaves contextThreadId. The destination then runs delegateTask locally (OrchestratorMcpService.ts lines 1955 and 2030-2034): parentThreadId is the borrowed destination thread. The source handler never runs, so RemoteDelegation never records the task on the source parent. Completion wakes the destination thread. task_status on the source does not see the task. The decoded result still returns that destination taskId, so the source agent is told it owns a task it does not.

Without contextThreadId, the destination linked client has no thread and actsAsCaller fails, so the field this PR adds ("Run this tool on a linked environment") does not perform delegate_task at all. Setting top-level environmentId to one environment and target.environmentId to a third hits the onward-link refusal instead of RemoteDelegation.

The ownership-preserving path still works when top-level environmentId is omitted and target.environmentId is set. PeerToolRouting.test.ts at this commit does not call delegate_task.

2. start_spectrum through a full-access link does not keep the link fence

Trigger: the link is full-access/default. A source agent calls start_spectrum with top-level environmentId and callerThreadId of any existing destination thread, and omits contextThreadId.

Required: linkOrigin.ts says work a link starts, and work that work starts, carries the link, and descendants keep that fence. assertSameLink is not waived for a direct full-access peer. Issue #302 says destination link checks stay authoritative. The tool's own refusal is "Spectrum cannot be started through a peer link."

Actual:

  • Destination startsThreads (McpToolAccess.ts) calls assertNotLinked. directFullAccessPeer (linkOrigin.ts lines 52-68) passes because the forwarded session has no caller thread, client.linked is true, and the mode header is full-access/default.
  • The handler (spectrum/mcpToolkit.ts lines 74-93) takes only input. It ignores the StartedModes.linkOrigin that startsThreads computed, and it never calls assertSameLink.
  • readThread (threadAccess.ts line 168) loads any thread in the environment. Reads are not fenced.
  • LaunchService.shell (LaunchService.ts lines 76-100) copies the named thread's owner, modes, branch, and worktree, and does not set linkOrigin. register builds both the transcript thread and the Color threads with that shell.

Those threads are ordinary local children of whatever thread id was named, including a user thread or another link's thread. They are not confined to this link, so agents on them can use the destination's own links. That is the onward path callerLimits refuses to the peer itself.

The routing test expects spectrum-start and mocks SpectrumLaunchService.register, so it never observes the missing stamp. A narrowed link still fails directFullAccessPeer. Passing contextThreadId makes caller.caller the borrowed thread, so the refusal holds for that shape only.

Optional

device_open returns a quick-start command whose executable path is on the destination host. device_* MCP calls still run there. The returned text tells the agent to run that command. Worth one sentence in the tool result so a source agent does not shell it locally.

html_render publishes on the borrowed destination thread. That matches the Issue #302 row that artifacts belong to that thread. The success string still says the page is shown above the caller's reply, which for a source agent is the other environment's thread.

Prepare and discard of an upload use different owners if only one call passes contextThreadId (thread id versus client:<session>). Discard then fails closed.

Cleared on this commit

  • invoke overwrites contextCapabilities from the source credential before the forward. capabilityFor denies a source credential that lacks the tool capability. Destination withContext intersects that list with the borrowed credential. An OAuth link session itself has no preview or device grant; those tools work only by borrowing a live thread credential.
  • contextThreadId borrows only when the destination thread exists, is not deleted, and linkOrigin.sessionId is this peer session, and only after assertTargetWithinLimits against the link ceiling. A remote source does not borrow; withContext returns before borrow when isRemote is true.
  • loadCaller and loadThreadCaller re-read the caller thread and compare its current modes with the client ceiling. readsAsCaller relies on the borrow-time check rather than a second check. Target threads of plain reads stay unfenced, which linkOrigin.ts already states.
  • Mode limit is the T3-Mode-Limit header on each tools/call. A reused MCP session does not keep the first caller's modes. The header can only narrow.
  • Direct full-access does not apply to a borrowed thread, a descendant (caller.caller set), or a narrowed ceiling. Onward PeerForwarding fails whenever callerLinkOrigin is set. Listing another environment's links does not grant their use.
  • Remote launch attachments stay refused in PeerForwarding.forward for t3_thread_launch and for a delegate_task that takes the generic forward. RemoteDelegation's target.environmentId launch object does not copy attachments.
  • t3_attachment_prepare_upload sets uploadOrigin from the resolved peer URL before success-schema decode.
  • Linked image tools return structuredContent. The source registration formats the image. device_screenshot is covered by the routing test. preview_snapshot's schema includes screenshot.data; that round trip is not in the routing test.
  • t3_thread_wait is chunked in waitForThread (50s), pins runId after the first chunk, and decodeWait drops contextThreadId and contextCapabilities. Chunks omit environmentId, so the destination does not chunk again. A retry with no pinned runId can still observe a newer run. That predates this change.
  • Forwarded clientRequestId is fwd: plus a hash of the source request namespace and the id, so two source agents do not collide on the shared peer namespace. create_threads copies the parent linkOrigin.

Proof not claimed

I did not re-run the author's 166 tests, typecheck, or lint. Domain services in PeerToolRouting.test.ts are mocked; transport, OAuth, declarations, schemas, and mode headers are real there. CI was not a review gate. No live multi-machine run. There is no test here for forged contextCapabilities, wait chunking, or these two failures.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: PR #304 has a fresh proved candidate at 23cdfa11a29aabf88c564f4114d1942c64774231, based on main 366f2fff6f25f47c14ca86163d8ee8e4dd6c0961.
Why: Sol found scheduled descendants losing ownership, and Grok found top-level delegation moving parent ownership to the peer.
So what: Both reviewers inspect this complete candidate again; no earlier status counts, and green CI remains required before the authorized merge.

All supported findings are corrected:

  • Sol round 1 Spectrum/HTML: findings, response. Spectrum checks caller link/modes and persists origin on every shell; real SQL persistence plus actual onward refusal and foreign-caller socket tests pass. HTML prepares on the peer and stores the generated self-contained page in the source thread; real services, both attachment stores and the shared client parser prove source consumption without an orphan peer page.
  • Sol round 2 scheduled ownership: findings. Task creation derives origin from authenticated scope, SQL stores it immutably, UI edits retain it, and ordinary launches and checked-send replay carry it. Foreign task updates/deletion/manual execution and rebinding/execution against foreign threads are refused; foreign webhook URLs are hidden. A real MCP service, task persistence, fresh scheduler service, real thread launch and actual PeerForwarding call prove the descendant fence; checked-engine persistence proves admitted origin survives replay. The nullable fork schema is idempotent and preserves legacy task history, outside upstream's numbered migration history.
  • Grok round 1 delegation: findings. The top-level destination aliases target.environmentId into the existing RemoteDelegation path, preserving the source parent rather than borrowing one on the peer. Conflicting destinations fail before side effects. Real sockets/OAuth, source event persistence and continuation delivery prove both spellings keep the same source task, source status and completion wake. Both also use target Prism and refuse requests above the link ceiling. Its duplicate Spectrum finding is addressed by the earlier correction. Destination-local device launcher and recording paths remain destination resources, as recorded in feat(mcp): forward all capabilities to linked environments #302.

On this exact head: 396 focused tests pass in 31 files, four optional browser tests skipped. Targeted server/contracts typechecks pass. All 49 changed TypeScript files pass formatting/lint with only three pre-existing warnings (two registry callbacks and an unchanged optional-chaining expression in ThreadLaunchService.test). Fork ownership check passes 36 features. Rebased onto PR #309 and installed its lockfile/patch before current proof. No repo-wide local check, live-machine/browser/device interaction, version bump, memory write, restart or deployment.

Requirements and who asked: User requested full linked MCP reach with original scopes and narrowing, faithful group proof, source task ownership, and exact-head Sol/Grok reviews plus CI before merge.
Deleted: Blind delegation transport, unmarked scheduled descendants, arbitrary file copying, onward links, duplicated ownership facts, repo-wide checks and deployment.
Bottleneck: Independent findings revealed durable ownership paths that domain mocks had missed.
Checked myself: Traced every finding to its state/effect owner, executed real persistence/launch/wake/attachment tests, inspected targeted checks, and verified the live PR head after push.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Sol review found no blocking defect at 23cdfa11a29aabf88c564f4114d1942c64774231.
Why: The cumulative implementation preserves the inspected access boundaries, durable link ownership, source delegation ownership and source-consumable HTML output; the earlier findings are resolved.
So what: This exact candidate passes this Sol review; the planner still owns the separate Grok gate and final merge decision.

Verdict: pass, with no supported actionable blocker in the inspected scope. Reviewed the full Issue #302 and PR #304 bodies, the prior findings and current response, and the complete cumulative 53-file diff from main 366f2fff6f25f47c14ca86163d8ee8e4dd6c0961 to 23cdfa11a29aabf88c564f4114d1942c64774231, including the relevant surrounding consumers, effects and persistence. No earlier verdict was transferred to this candidate.

Prior findings adjudication

  • Sol round 1 / Grok round 1 Spectrum: resolved. The MCP handler checks the selected caller's same-link ownership and modes. Shell construction copies linkOrigin to the transcript and every Color; the transactional registration guard checks the loaded caller's origin, modes, project and owner. Inspected the real SQL-backed descendant test, its actual PeerForwarding refusal before onward transport, and the socket/OAuth foreign-caller refusal. The direct full-access environment exception does not unmark the resulting descendants.
  • Sol round 1 HTML: resolved. The destination prepares HTML and inlines destination image bytes without storing an orphan page. The source decodes that prepared variant and calls publishPrepared, enforcing the 25 MiB bound and storing the reference in the source thread's attachment store. Inspected the real-services/two-store test, actual stored image bytes, shared reference parser and web/mobile consumers. Desktop uses the web consumer. All resolve the resulting source attachment in the originating environment.
  • Sol round 2 scheduled ownership: resolved. Creation derives origin from authenticated scope. The nullable fork column is initialized transactionally after upstream migrations, preserves legacy rows and survives repeated initialization. Origin is retained by service edits and omitted from SQL conflict updates, preventing form saves from erasing or replacing creation ownership. Ordinary launches carry it; checked sends pin it in the durable payload and retain it through replay. Same-link task update/delete/run-now checks, foreign webhook URL hiding, save-time binding validation and execution-time binding checks close the previously identified path. Inspected the real MCP service, SQL persistence, fresh scheduler instance, real launch and actual onward-forwarding refusal proof, plus checked payload replay and legacy migration assertions.
  • Grok round 1 delegation: resolved. Top-level environmentId is normalized into the existing source-owned target.environmentId path. Conflicting destinations fail before task creation or destination launch. Inspected the two-spelling real socket/OAuth tests and source event/continuation assertions: retries converge on the source task, completion wakes the source parent, source status remains readable, destination Prism selects the child, and above-link mode requests fail.
  • Prior optional objections: Device launcher/config and preview-recording paths remain destination-owned and require execution or consumption there. They do not imply local file copying or establish a physical forwarding limitation. Upload prepare/discard ownership still requires consistent context, and mismatches fail closed. The old HTML success/consumption objection is resolved by source publication above. No remaining blocker follows from these notes.

Inspected coverage and limits

Traced declarations and routing for schedules and durable execution; thread launch/create/fork/merge, lifecycle, configuration, metadata, search and transfers; queues and pending requests; projects, worktrees, preferences and secrets; attachments; Issue/PR links; browser/preview controls, devices, HTML and Spectrum. Checked relevant consumers and effects rather than relying solely on routing mocks.

The context binding requires a same-link, nondeleted destination thread and an existing unexpired/revocable registry credential, checks its modes against the link ceiling, and intersects destination credential grants with source capabilities. Source routing overwrites transport capability metadata; destination declarations and mutation paths retain capability, live-run, ownership and mode checks. Direct full-access/default environment calls remain distinct from linked descendants, and PeerForwarding refuses onward links whenever caller origin exists. Also inspected local metadata stripping, source-owned delegation, remote launch attachment refusal, destination uploadOrigin, image bytes before presentation filtering, wait chunking/run pinning and namespaced retry keys. Forks inherit source origin; existing thread mutations do not rewrite unrelated ownership.

Reused and inspected the supplied exact-candidate evidence: focused log 396 passed in 31 files, four optional browser cases skipped; server/contracts typecheck logs with no TypeScript errors; 49-file formatting result and three baseline lint warnings. Relevant new test assertions were read directly. Live GitHub CI is green on the exact head, including lint, typecheck, build, test/server/web and fork checks. These are implementation-run checks and CI, not tests executed by this reviewer. Transport/OAuth/schema/declaration tests are real; most per-group domain services are mocked. The additional persistence/launch, completion delivery and HTML storage tests cover the prior missing effects, but do not constitute live linked-machine or client verification.

No duplicate tests, repo-wide local checks, browser/device interaction, live linked-machine use, installation, server launch/restart, deployment, implementation edits or merge occurred. PR #304 is linked to this review thread through MCP. This verdict states no supported blocker in the reviewed scope, not proof that every external runtime path is defect-free.

Elon record

Requirements and who asked: User requested independent exact-head Sol review for #302/#304, every tool's linked reach with existing scopes/capabilities/modes/link ownership/live checks/no onward links, and findings followed by review/independent-sol on the exact candidate.
Deleted: Stylistic findings, speculative limitations, redesign, implementation changes, extra agents, duplicate test runs and live-machine mutations.
Bottleneck: Durable ownership and artifact consumption must survive beyond the immediate routed handler; routing mocks alone cannot establish that.
Checked myself: Read the complete cumulative diff, full Issue/PR bodies and prior findings/responses; traced access, state/effect owners and web/mobile consumers; inspected focused persistence/launch/wake/artifact assertions and logs; verified current GitHub head/base, CI and authenticated account.

Reviewer: gpt-6.1-sol, Codex harness in T3 Code, high reasoning effort. Reviewed SHA: 23cdfa11a29aabf88c564f4114d1942c64774231. Authenticated GitHub account: lukemaj.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent review of 23cdfa11a29aabf88c564f4114d1942c64774231 finds no blocking defect. Every traced MCP tool still reaches a linked environment through environmentId, and the three earlier blockers stay fixed at the effect that persists ownership or consumes the artifact.

Why: Routing declarations are not the fence. Same-link origin, source task records, Spectrum shells, and source HTML storage are.

So what: review/independent-grok is success on this exact SHA. Merge stays a human decision. review/independent-sol is a separate pending review and is not decided here.

Reviewed head 23cdfa11a29aabf88c564f4114d1942c64774231, base 366f2fff6f25f47c14ca86163d8ee8e4dd6c0961, model grok-4.7 on the Grok harness. Read-only. No merge.

Prior findings

  1. Sol round 1 (Spectrum caller and HTML peer attachment), comment 6171873244, candidate 75fb. Cleared on this SHA. start_spectrum still refuses linked descendants (apps/server/src/spectrum/mcpToolkit.ts 124). A direct full-access/default peer passes assertNotLinked (apps/server/src/mcp/linkOrigin.ts 56-71) and then assertSameLink on the named caller thread (mcpToolkit.ts 82-86). shell() copies that caller's linkOrigin onto the Spectrum thread and every Color (apps/server/src/spectrum/LaunchService.ts 66-85, 235-251). The commit adds forkSpectrumRunning by spread, so origin stays (LaunchService.ts 259-268). The registration guard rejects a caller whose origin or owner changed before commit (apps/server/src/spectrum/registrationPlan.ts 22-29). html_render on a linked client returns a prepared document and does not publish (apps/server/src/mcp/toolkits/html/handlers.ts 46-51). Source routing decodes that variant and publishPrepared stores it on the source thread (apps/server/src/mcp/McpRouting.ts 183-200). The success schema lists PreparedDocument first (apps/server/src/mcp/toolkits/html/tools.ts 91-104). Page size is bounded at 25 MiB in publishPrepared (apps/server/src/htmlRender/HtmlRender.ts 39-45, 430-457).

  2. Sol round 2 (scheduled threads lose origin), comment 6172383327, candidate 298c. Cleared on this SHA. schedule_task derives origin from the caller and passes it only on create (apps/server/src/mcp/OrchestratorMcpService.ts 1634-1636). Upsert keeps the stored origin whenever the row exists (apps/server/src/scheduledTasks/ScheduledTaskService.ts 1068-1069, 1149-1150). The SQL conflict update does not assign link_origin_json (ScheduledTaskService.ts 597-621). validateBinding runs before save and again before run (726-741, 809). Launch carries the stored origin (858). Checked sends pin task.linkOrigin into the payload (apps/server/src/scheduledTaskChecks/engine.ts 298) and dispatch uses that payload (apps/server/src/scheduledTaskChecks/ScheduledTaskChecks.ts 327-342). The engine test drives a later task origin and asserts the stored payload is unchanged (apps/server/src/scheduledTaskChecks/engine.test.ts 1261-1276). Update, delete, and run-now call assertSameLink (OrchestratorMcpService.ts 1695 and 1752, apps/server/src/mcp/toolkits/thread/handlers.ts 70). Webhook URLs are omitted unless the caller owns the task and may run it (OrchestratorMcpService.ts 1562-1573, 262-274). The nullable column is added after migrations only when missing (apps/server/src/persistence/forkScheduledTaskOriginsSchema.ts 5-14). A legacy null origin stays null.

  3. Grok round 1 (delegation reparented onto the destination), comment 6172389707, candidate 75fb. Cleared on this SHA. delegate_task with a top-level environmentId is handled locally. A conflicting target.environmentId fails before the handler (McpRouting.ts 152-167). RemoteDelegation records delegated_task.remote.request on the source parent with completionWake (apps/server/src/peer/RemoteDelegation.ts 275-288), then launches the child with t3_thread_launch (442-467). The source requestNamespace is the retry namespace (248-252, 435-441). The Spectrum half of that review is the same fix as item 1.

Coverage

Traced, not only declared: contextThreadId binds the newest unexpired same-session credential inside the link ceiling (apps/server/src/mcp/McpRouting.ts 67-109, apps/server/src/mcp/McpSessionRegistry.ts 212-218). Revoked rows are removed. Source invoke replaces contextCapabilities with the source credential (McpRouting.ts 179-181). Destination loadCaller rechecks the client ceiling against the bound thread (apps/server/src/mcp/threadAccess.ts 90-96). callerLimits refuses onward links whenever the caller already has an origin (apps/server/src/peer/PeerForwarding.ts 107-118). Remote t3_thread_launch and delegate_task still refuse attachments (151-161). Wait chunks pin runId, and an idle thread returns timedOut: false (apps/server/src/peer/PeerForwarding.ts 215-236, apps/server/src/peer/recoveryWait.ts 96-98 and 163). Linked image tools return schema-complete bytes before local presentation filtering (apps/server/src/mcp/McpHttpServer.ts 541-548 and 723-729). PeerMcpClient keeps image bytes and sets uploadOrigin (apps/server/src/peer/PeerMcpClient.ts 245-259). Device open and preview recording claim files under the destination attachments directory. Fork planning copies the source thread, including linkOrigin (apps/server/src/orchestration-v2/ThreadForkService.ts 88-114). Queue, pending, configure, organize, merge, issues, and pull-request writes go through writesThreads or the bound caller, and the destination runs those checks again. Direct full-access project and preference mutations do not write thread origin. A new fork whose caller has no thread gets owner: null on that new thread only (apps/server/src/orchestration-v2/Orchestrator.ts 3672-3673). The source thread is unchanged and the copy keeps linkOrigin.

Proof limits

The author reports 396 focused tests, targeted tsc, lint, and green CI. This review did not re-run those commands, did not query GitHub check runs, and did not open a live link, browser, or device. The verdict is from the code and tests read at this SHA. A clean review does not prove every untraced path.

Elon record

Requirements and who asked: The user, 2026-10-14, asked for an independent high-effort review of PR 304 / Issue 302 at exact 23cdfa11: every current MCP tool acts on a linked environment through environmentId, with access declarations, both grants, link-mode narrowing, live-run and ownership checks, and no onward links. Delegation stays source-owned. Prior findings had to be re-checked. No merge.

Deleted: Style notes, rerunning the focused suite, repo-wide checks, live machines, extra agents, and messaging the planner.

Bottleneck: Ownership at the persisted effect (schedule row, source delegation record, Spectrum shells, source HTML file), not the routing field.

Checked myself: Read routing, access, link origin, forwarding, session registry, Spectrum launch and registration, schedule SQL and checked replay, HTML prepare and publish, image and upload return, delegation record and launch, recovery wait, and fork copy at 23cdfa11. Worktree HEAD matched that SHA and was clean.

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

Labels

size:XL 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.

feat(mcp): forward all capabilities to linked environments

1 participant