Skip to content

fix(server): keep retried linked Stops within their original work - #22536

Open
zainmitchell wants to merge 7 commits into
pingdotgg:t3code/peer/forwardingfrom
zainmitchell:contribution/16684-stop-retry-boundary
Open

zainmitchell wants to merge 7 commits into
pingdotgg:t3code/peer/forwardingfrom
zainmitchell:contribution/16684-stop-retry-boundary

Conversation

@zainmitchell

@zainmitchell zainmitchell commented Oct 20, 2026 •

Copy link
Copy Markdown

A partially permitted Stop can fail on a child, then interrupt that child's newer run when the same Stop is retried. Keep the traversal and child run selection bounded by the original Stop receipt's event sequence, including after restart, and report the run identified by the original interrupt request.

This ports @juliusmarminge's 6c0e5cf onto the retained #16684 branch. It also fixes the reviewed guard edge: a queued run cancelled after Stop no longer hides the older active run. When nothing is active, the bound checks the last executed run, preserving newer completed work. Current public thread.stop remains available; client decoding strips the internal retry bound. Existing idle-orphan handling and forwarding behavior are retained; a retry rejected for newer work returns before orphan cleanup.

Scope and approval

Submitted under the focused bug-fix exception: these changes make the existing Stop request idempotent across child traversal, run selection and its reported result. No new workflow or prior maintainer approval is claimed.

Verification

New tests on the base sources fail on newer-work interruption and incorrect MCP results. Bounding by latestRun again fails both cancelled-queue regressions. An independent run of ThreadStop, contracts rpc, and PeerForwarding passed 50/50. Nearby MCP, cancellation, access and thread-management checks pass. Server/contracts typechecks and scoped format pass; lint has only existing warnings. Toolkit-integration Windows path failures reproduce on base sources.

No new two-PC runtime campaign. Earlier runtime evidence applies only to its stated revisions. Inherited background-work/cancelled-queue and unbounded idle-orphan cases remain outside this patch. Some no-target tests set completion directly in the projection, while the start evidence is committed.

Original fix: Julius Marminge. Current-base implementation: Claude Opus 5.5 High through Claude Code. Review and independent verification: GPT-6.1-Sol through Codex in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 20, 2026
(cause) => new OrchestratorProjectionError({ threadId: command.threadId, cause }),
),
);
if (!hadStarted) return undefined;

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.

🟠 High orchestration-v2/Orchestrator.ts:9505

A queued run that was later cancelled can become latestRun and make this guard return early, so retrying a delegated Stop leaves the original still-running run unstopped. The guard checks whether latestRun started before throughSequence, but the subsequent target selection can choose a different active run; validate the selected interrupt target instead.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/Orchestrator.ts around line 9505:

A queued run that was later cancelled can become `latestRun` and make this guard return early, so retrying a delegated Stop leaves the original still-running run unstopped. The guard checks whether `latestRun` started before `throughSequence`, but the subsequent `target` selection can choose a different active run; validate the selected interrupt target instead.

@macroscopeapp

macroscopeapp Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a contained, regression-tested server bug fix that bounds retried delegated Stops to their original event history without changing the public RPC surface. A separate High-severity finding flags an edge case in the run-selection guard that should be addressed by the repository’s correctness process.

Needs human approval because:

  • 1 blocking correctness issue found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge
juliusmarminge force-pushed the t3code/peer/forwarding branch 2 times, most recently from c9246fe to 14c6cc9 Compare October 20, 2026 22:45
juliusmarminge and others added 6 commits October 20, 2026 16:06
An OAuth client on /mcp is capped by the access it was approved with.
When another environment calls on behalf of one of its agents, that agent
may run under narrower modes, and the target has no way to know.

/mcp now reads `T3-Mode-Limit: <runtimeMode>/<interactionMode>` from OAuth
clients and caps the request at the narrower of the two. The header can
only narrow: a broader one changes nothing, and a malformed one is
refused with 400. The client's interaction mode can now be capped too;
before, it was always default.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A launch had no retry key, so an agent (or another environment calling on
its behalf) that lost the response could only guess whether the thread
was created, and a retry started a second one.

t3_thread_launch now takes an optional clientRequestId. The command,
thread and message ids derive from the caller and the key, and the launch
service already replays a command it accepted, so a retry returns the
first thread. Without a key every launch is new, as before.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An environment can now sign in to another one's /mcp as an outside agent,
with a pairing code from that environment, without a browser. It drives the
peer's existing MCP OAuth flow server-side, keeps the token in the server
secret store and the rest in peer_environment_links (migration 059). The
peer lists the link as "T3 Code · <label>" in Connections, where it can be
revoked.

The token is only ever sent to an address whose descriptor answers as the
linked environment. PeerMcpClient calls the peer's tools and carries the
caller's limits in T3-Mode-Limit on every request. Linking refuses this
environment itself and peers that cannot honour that header.

`t3 environment link|list|unlink` and the peerLinks.* RPCs manage links.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
An agent can now work on a linked environment's threads through the same T3
MCP tools. t3_project_list, orchestrator_capabilities, t3_thread_launch,
t3_thread_list, t3_thread_read, t3_thread_send, t3_thread_wait and
t3_thread_interrupt take an optional environmentId; t3_environment_links
lists the links.

Each tool's own access declaration still runs here first, so a read-only or
ended caller is refused before anything leaves. The forwarded call carries
the caller's modes in T3-Mode-Limit, and the linked environment holds it to
those and the link's access. Retry keys are hashed with the caller's
namespace, since every caller here shares the link's one session there. A
forwarded wait goes out in pieces of at most 50 s so no route drops it idle.
A remote launch cannot carry attachments yet.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
t3_project_create and t3_project_clone take environmentId, so an agent can
bring a repository to a machine before launching or delegating work there.
Both still need full access here, and the linked environment holds the call
to the link's access.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s a linked environment

Forwarding was wired tool by tool, so only the first eleven tools took
environmentId. It now lives in McpToolAccess.toLayer: a tool whose
parameters take OrchestratorMcpEnvironmentTarget runs in the linked
environment it names once its declaration has checked the caller here.
The local handler never sees environmentId. Tools that act as the calling
thread itself cannot take one, and the layer refuses to build if they do.

Queues, pending requests, thread settings and organizing, forks and
merge-backs, scheduled tasks, pull requests, worktree refs, project
read/update/delete and environment preferences now reach a link too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Port the Stop retry safeguards from 6c0e5cf and its follow-ups onto the
current base:

- A delegated Stop walk reads the original Stop's command receipt and walks
  only the tasks delegated through that sequence, so a retry (including after
  a restart) never stops tasks delegated later.
- Child Stops carry that sequence as an internal-only `throughSequence`; the
  public `thread.stop` decoding drops it, so clients cannot set it.
- A bounded child Stop checks that the run it would reach had started by the
  bound. It validates that run rather than the newest one, so a queued run
  cancelled after the original Stop no longer blocks the retry, and newer
  finished work is left alone when there is nothing to interrupt.
- The MCP Stop result comes from the original interrupt request, so a replay
  reports the same run.

Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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: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