From cdb9840f4e00319e052b3c680ded9e4b4976baee Mon Sep 17 00:00:00 2001 From: Henry Chu Date: Sat, 31 Oct 2026 17:30:24 -0400 Subject: [PATCH 1/3] Fix the Fable review findings in the gateway, coder-cli and bridge - T3ToolBridge counts calls still inside their inline budget toward the per-thread cap, reserving the slot before the tool starts, so concurrent calls near the limit can no longer all pass the check (15 jobs + 8 concurrent calls accepted all 8 before; now one). - A negotiated helper whose stdin is already closed is stopped by signals right away instead of waiting a grace period for an undeliverable shutdown request. - coder-only.md: the web build carries the manifest again; note that on Windows the free-port probe may call a port free while another program listens on 0.0.0.0, which the gateway cannot detect within boundary 1 and its no-outbound-connection rule. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/mcp/bridge/T3ToolBridge.ts | 37 ++++++++++++++++------ docs/internals/coder-only.md | 10 ++++-- packages/coder-cli/src/helperConnection.ts | 7 ++-- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/apps/server/src/mcp/bridge/T3ToolBridge.ts b/apps/server/src/mcp/bridge/T3ToolBridge.ts index e045fa716dbd4..5f0fb94c99b53 100644 --- a/apps/server/src/mcp/bridge/T3ToolBridge.ts +++ b/apps/server/src/mcp/bridge/T3ToolBridge.ts @@ -134,6 +134,9 @@ export const makeBindingWith = (options: { readonly inlineBudgetMs?: number } = const crypto = yield* Crypto.Crypto; const jobScope = yield* Effect.scope; const jobs = new Map(); + // Calls still inside their inline budget, per thread; they count toward the cap before they + // become jobs, so concurrent calls cannot all pass the check. + const inlineCalls = new Map(); const run = ( scope: McpInvocationContext.McpInvocationScope, @@ -235,10 +238,12 @@ export const makeBindingWith = (options: { readonly inlineBudgetMs?: number } = yield* Fiber.interrupt(job.fiber); } } - // Refuse before the tool starts, so a refused call never half-applies. - let running = 0; + // Refuse before the tool starts, so a refused call never half-applies. The count and the + // reservation below run without yielding, so concurrent calls see each other. + const threadId = scope.thread.threadId; + let running = inlineCalls.get(threadId) ?? 0; for (const job of jobs.values()) { - if (job.threadId === scope.thread.threadId && job.fiber.pollUnsafe() === undefined) { + if (job.threadId === threadId && job.fiber.pollUnsafe() === undefined) { running++; } } @@ -248,13 +253,25 @@ export const makeBindingWith = (options: { readonly inlineBudgetMs?: number } = "Too many T3 tool calls are still running. Read their results with task_status first.", ); } - const fiber = yield* run(scope, request).pipe(Effect.forkIn(jobScope)); - const exit = yield* Fiber.await(fiber).pipe( - Effect.timeoutOption(options.inlineBudgetMs ?? INLINE_BUDGET_MS), - ); - if (Option.isSome(exit)) return yield* exit.value; - const taskId = `${BRIDGE_JOB_PREFIX}${yield* crypto.randomUUIDv4().pipe(Effect.orDie)}`; - jobs.set(taskId, { threadId: scope.thread.threadId, startedAt: now, fiber }); + inlineCalls.set(threadId, (inlineCalls.get(threadId) ?? 0) + 1); + const releaseInline = Effect.sync(() => { + const remaining = (inlineCalls.get(threadId) ?? 1) - 1; + if (remaining > 0) inlineCalls.set(threadId, remaining); + else inlineCalls.delete(threadId); + }); + const outcome = yield* Effect.gen(function* () { + const fiber = yield* run(scope, request).pipe(Effect.forkIn(jobScope)); + const exit = yield* Fiber.await(fiber).pipe( + Effect.timeoutOption(options.inlineBudgetMs ?? INLINE_BUDGET_MS), + ); + if (Option.isSome(exit)) return { _tag: "Done" as const, exit: exit.value }; + const taskId = `${BRIDGE_JOB_PREFIX}${yield* crypto.randomUUIDv4().pipe(Effect.orDie)}`; + // Registered before the inline slot is released, so the call is counted throughout. + jobs.set(taskId, { threadId, startedAt: now, fiber }); + return { _tag: "Job" as const, taskId }; + }).pipe(Effect.ensuring(releaseInline)); + if (outcome._tag === "Done") return yield* outcome.exit; + const taskId = outcome.taskId; return { status: "running", taskId, diff --git a/docs/internals/coder-only.md b/docs/internals/coder-only.md index 853bc885c379e..30950add8eba6 100644 --- a/docs/internals/coder-only.md +++ b/docs/internals/coder-only.md @@ -178,7 +178,10 @@ removed, or the gateway exits. A running forward's status adds `ready` once the Coder's `Ready!` line on stderr. The thread details card's Servers section saves the same rules (see [Web chat, composer, and files](#web-chat-composer-and-files)); its free-port check (`/api/port-forwards/free-port`, at most `MAX_LOCAL_PORT_CANDIDATES` ports past the requested one) -binds and closes a probe listener on `127.0.0.1` only and opens no connection. +binds and closes a probe listener on `127.0.0.1` only and opens no connection. On Windows a +specific-address bind can succeed while another program listens on `0.0.0.0` with the same port, +so the check may call such a port free (unverified on a Windows laptop); the gateway cannot probe +the wildcard address or connect out to tell, by boundary 1 and its no-outbound-connection rule. ```text browser -> 127.0.0.1 gateway -> coder ssh stdio -> workspace helper -> codex / claude @@ -980,8 +983,9 @@ checks that every name here still exists and that every modified upstream file i ### Build tooling and tests -- `apps/web/vite.config.ts` carries upstream's office/pdf asset handling and route splitting but - not the render scenarios project, `VITE_RELEASE_CHANNEL`, source maps, or the build manifest. +- `apps/web/vite.config.ts` carries upstream's office/pdf asset handling, route splitting and + build manifest (the gateway reads `.vite/manifest.json` to cache hashed assets) but not the render + scenarios project, `VITE_RELEASE_CHANNEL`, or source maps. - Root `vite.config.ts` is upstream's without the remote task cache and cached test tasks (`infra/` is cut), the per-package test projects (`vite.coder.config.ts` replaces upstream's `apps/server/vite.config.ts` and extends this file, which therefore loads `gitConfig.setup.ts`), diff --git a/packages/coder-cli/src/helperConnection.ts b/packages/coder-cli/src/helperConnection.ts index 7739a32ceedd8..fec9cff822a01 100644 --- a/packages/coder-cli/src/helperConnection.ts +++ b/packages/coder-cli/src/helperConnection.ts @@ -171,13 +171,14 @@ const terminateCoderProcess = ( } }; // A negotiated helper is asked to exit, so `coder ssh` ends its session itself; before that, - // only signals stop the launch. - if (!process.negotiated) endStdin(); + // or once stdin can no longer carry the request, only signals stop the launch. + const canRequestExit = process.negotiated && !stdin.destroyed && !stdin.writableEnded; + if (!canRequestExit) endStdin(); return terminateChildProcess({ child: process.child, exit: process.exit, graceMs: terminationGraceMs, - ...(process.negotiated ? { requestExit: () => endStdin(SHUTDOWN_REQUEST) } : {}), + ...(canRequestExit ? { requestExit: () => endStdin(SHUTDOWN_REQUEST) } : {}), }); }); From 39470f48d89f18f99d3621d168f12106c5e89dd6 Mon Sep 17 00:00:00 2001 From: Henry Chu Date: Sat, 31 Oct 2026 17:34:51 -0400 Subject: [PATCH 2/3] Fix the Fable review findings in the restored features - Gateway: a config save no longer retries forward rules that failed (every Forward click and failure cleanup restarted them), matching the documented behaviour; startup and changed rules still start. - Forward reuses a rule another tab already saved for the same server instead of adding a second one, and never removes a rule it did not add. - A Forward whose tab the browser blocks shows the local address in a toast. - Coder: markers on the #26201 non-carry in composerThreadItems.ts and composerContextReferences.ts. - Docs: the @ workspace list needs Labs > Linked environments; rewrap the Servers paragraph; the seams entry names the kept readOnly prop and managementFailure reasons. Co-Authored-By: Claude Opus 5.5 --- apps/coder-gateway/src/server.ts | 18 ++++++++++--- apps/web/src/coder/serverForwards.ts | 26 ++++++++++++++----- .../components/chat/ThreadServersPanel.tsx | 7 ++++- docs/internals/coder-only.md | 4 ++- docs/user/thread-sidebar.md | 4 +-- docs/user/workspaces.md | 3 ++- .../client-runtime/src/composerThreadItems.ts | 2 ++ .../shared/src/composerContextReferences.ts | 3 ++- 8 files changed, 52 insertions(+), 15 deletions(-) diff --git a/apps/coder-gateway/src/server.ts b/apps/coder-gateway/src/server.ts index 7f23b190bb67e..5030d6118e794 100644 --- a/apps/coder-gateway/src/server.ts +++ b/apps/coder-gateway/src/server.ts @@ -1511,7 +1511,13 @@ export function makeLocalCoderGateway( return deferred.promise; }; - const startConfiguredPortForwards = async (): Promise => { + /** + * Starts the saved rules. A config save passes `retryFailed: false`: a rule that failed stays + * stopped until it is restarted or changed (a changed rule's lifecycle is recreated as idle). + */ + const startConfiguredPortForwards = async (options?: { + readonly retryFailed?: boolean; + }): Promise => { if ((profileConfig.portForwards?.length ?? 0) === 0) return; const workspacesByDeployment = new Map< string, @@ -1538,6 +1544,12 @@ export function makeLocalCoderGateway( ); await Promise.allSettled( (profileConfig.portForwards ?? []).map(async (portForward) => { + if ( + options?.retryFailed === false && + SynchronizedRef.getUnsafe(portForwardLifecycle(portForward.id))._tag === "Failed" + ) { + return; + } const workspace = profileConfig.workspaces.find( (entry) => entry.id === portForward.workspaceId, ); @@ -1909,7 +1921,7 @@ export function makeLocalCoderGateway( for (const portForwardId of stalePortForwardIds) { portForwardLifecycles.delete(portForwardId); } - await startConfiguredPortForwards(); + await startConfiguredPortForwards({ retryFailed: false }); return profileConfig; }); sendText(response, 200, "application/json; charset=utf-8", JSON.stringify(savedConfig)); @@ -2501,7 +2513,7 @@ export function makeLocalCoderGateway( yield* Effect.promise(() => saveGatewayPort(options.portPath!, address.port)); } - yield* Effect.promise(startConfiguredPortForwards); + yield* Effect.promise(() => startConfiguredPortForwards()); return { url: `http://${CODER_GATEWAY_HOST}:${address.port}` }; }); diff --git a/apps/web/src/coder/serverForwards.ts b/apps/web/src/coder/serverForwards.ts index e26495c108ada..4e0741d8a9d5a 100644 --- a/apps/web/src/coder/serverForwards.ts +++ b/apps/web/src/coder/serverForwards.ts @@ -73,17 +73,31 @@ export async function forwardServer(input: { localPort: await findFreeCoderForwardPort(remotePort), remotePort, }; - // Saving starts the forward; the gateway answers once its process has spawned. - await updateConfig((current) => addServerForward(current, rule)); + // Saving starts the forward; the gateway answers once its process has spawned. Another tab may + // have forwarded the same server first; that rule is used instead of adding a second one. + let forwarded = rule; + await updateConfig((current) => { + const existing = findServerForward(current, rule.workspaceId, remotePort); + if (existing !== undefined) { + forwarded = existing; + return current; + } + return addServerForward(current, rule); + }); const deadline = Date.now() + FORWARD_READY_TIMEOUT_MS; for (;;) { - const status = (await loadCoderPortForwardStatuses()).find((entry) => entry.id === rule.id); + const status = (await loadCoderPortForwardStatuses()).find( + (entry) => entry.id === forwarded.id, + ); const expired = Date.now() >= deadline; // Coder versions that never print `Ready!` still open once the wait runs out. - if (status?.status === "running" && (status.ready === true || expired)) return rule; + if (status?.status === "running" && (status.ready === true || expired)) return forwarded; if (status?.status !== "running" && (status?.status !== "starting" || expired)) { - // A forward that never ran is not kept, so Settings shows no rule to explain. - await updateConfig((current) => removeServerForward(current, rule.id)); + // A forward this click added and that never ran is not kept, so Settings shows no rule to + // explain; a rule another tab added stays. + if (forwarded === rule) { + await updateConfig((current) => removeServerForward(current, rule.id)); + } throw new Error(status?.error ?? "The port forward did not start."); } await new Promise((resolve) => setTimeout(resolve, FORWARD_READY_POLL_MS)); diff --git a/apps/web/src/components/chat/ThreadServersPanel.tsx b/apps/web/src/components/chat/ThreadServersPanel.tsx index 9f98bc621efe1..174f26207cccb 100644 --- a/apps/web/src/components/chat/ThreadServersPanel.tsx +++ b/apps/web/src/components/chat/ThreadServersPanel.tsx @@ -89,7 +89,12 @@ export function ThreadServersPanel(props: { if (forwarded === null) return; const url = forwardedServerUrl(forwarded); if (tab) tab.location.href = url; - else window.open(url, "_blank", "noopener,noreferrer"); + else if (window.open(url, "_blank", "noopener,noreferrer") === null) { + // The browser blocked the tab; the forward still runs, so say where it is. + toastManager.add( + stackedThreadToast({ type: "success", title: "Server forwarded", description: url }), + ); + } }; const unforward = async (server: DiscoveredLocalServer) => { diff --git a/docs/internals/coder-only.md b/docs/internals/coder-only.md index 30950add8eba6..99d7155a10161 100644 --- a/docs/internals/coder-only.md +++ b/docs/internals/coder-only.md @@ -600,7 +600,9 @@ checks that every name here still exists and that every modified upstream file i `coderServer.ts` provides the runtime coordinator alone (upstream: `layerFromHost`); `rpc.ts`, client-runtime's `state/server.ts`, and `AcpSessionManagementSection.tsx` carry only those two. Session delete is cut because Kilo Code advertises no `session/delete`; logout and - agent provider configuration are account and provider management. + agent provider configuration are account and provider management. The section keeps upstream's + `readOnly` prop (upstream's browser test passes it) and `probe.ts` keeps upstream's + `managementFailure` reasons for the cut actions; neither is read here. - provider-acp's `adapter.ts` gives ACP sessions no MCP server and sends `t3ToolBridgeInstructions` with the first prompt through provider-core's `t3AcpPromptWithInstructions`; `mcpOverAcpBridge.ts` and `clientTerminals.ts` stay dormant (cutting them would add about 34 seams). HTTP MCP servers diff --git a/docs/user/thread-sidebar.md b/docs/user/thread-sidebar.md index a3069a3e46c6e..712a85f43a9cb 100644 --- a/docs/user/thread-sidebar.md +++ b/docs/user/thread-sidebar.md @@ -244,8 +244,8 @@ finishes; the call's own result shows its status. The thread details card lists the local servers that the thread's terminals or its Claude agent started in the workspace, with their ports. Not every one serves a web page. A server the agent -started stays listed after the agent's turn ends. Select a server to forward it to `127.0.0.1` on this computer and open it in a -new tab; the forward is saved under **Settings → Environments → Port forwarding** and uses the same +started stays listed after the agent's turn ends. Select a server to forward it to `127.0.0.1` on +this computer and open it in a new tab; the forward is saved under **Settings → Environments → Port forwarding** and uses the same port when it is free, otherwise the next free one. A forwarded server shows its local address, and selecting it opens that address. **Stop forwarding** removes the forward; the server keeps running. **Stop** ends the server's job and keeps the agent, the terminal, and T3 Coder running; a saved diff --git a/docs/user/workspaces.md b/docs/user/workspaces.md index 137124dd043ee..fb6d0eaf26d31 100644 --- a/docs/user/workspaces.md +++ b/docs/user/workspaces.md @@ -85,7 +85,8 @@ workspace stops its helper and the work running there; the task waits and report ended once the workspace is connected again. To point an agent at a linked workspace in a message, type `@` and part of the workspace's name in -the composer and pick it. The list shows the connected workspaces the current one links to. +the composer and pick it. The list shows the connected workspaces the current one links to, once +**Linked environments** is turned on in **Settings → Labs** for the current workspace. ## Data ownership diff --git a/packages/client-runtime/src/composerThreadItems.ts b/packages/client-runtime/src/composerThreadItems.ts index 43f356b7653ed..083bea7b9c93c 100644 --- a/packages/client-runtime/src/composerThreadItems.ts +++ b/packages/client-runtime/src/composerThreadItems.ts @@ -23,6 +23,8 @@ export interface ComposerThreadItem { readonly description: string; } +// Coder: threads on linked machines (#26201) are not carried, so there is no `linkedEnvironments` +// input and candidates come only from the composer's own environment. /** * Threads to reference from the composer, matched and ranked like the command * palette, without subagent or side chat threads. The agent can only read threads on its diff --git a/packages/shared/src/composerContextReferences.ts b/packages/shared/src/composerContextReferences.ts index b625537874070..546faa3d69d86 100644 --- a/packages/shared/src/composerContextReferences.ts +++ b/packages/shared/src/composerContextReferences.ts @@ -361,7 +361,8 @@ export function projectComposerContextForProvider(input: { return `${body}\n\n<${CONTEXT_ENVELOPE_TAG} version="1">\n${entries.join("\n")}\n`; } -// Thread reads are environment-local even when a draft moves between machines. +// Coder: threads on linked machines (#26201) are not carried, so there is no `linkedEnvironments` +// input. Thread reads are environment-local even when a draft moves between machines. export function threadContextsOutsideEnvironment(input: { text: string; records: ReadonlyArray; From c71814df90c98a0bb2c31baef8100ea0448dcc7f Mon Sep 17 00:00:00 2001 From: Henry Chu Date: Sat, 31 Oct 2026 17:36:11 -0400 Subject: [PATCH 3/3] Fix the Fable review findings in the sync tooling - sync-resolve-deleted.mjs finds keep candidates imported through the web app's ~/ alias or a @t3tools//... export, not only relative imports, so the documented bulk removal cannot delete such a file. - ThreadErrorBanner.tsx marks its ChatGPT usage-limit cut. - sync.md states the keep-candidate resolution and the lost check's scope (apps/ and packages/, outside tests). Co-Authored-By: Claude Opus 5.5 --- .../src/components/chat/ThreadErrorBanner.tsx | 3 ++ docs/internals/sync.md | 7 ++- scripts/sync-resolve-deleted.mjs | 52 +++++++++++++++++-- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/apps/web/src/components/chat/ThreadErrorBanner.tsx b/apps/web/src/components/chat/ThreadErrorBanner.tsx index 78a14104bd9fb..fcb0b13fc36a7 100644 --- a/apps/web/src/components/chat/ThreadErrorBanner.tsx +++ b/apps/web/src/components/chat/ThreadErrorBanner.tsx @@ -5,6 +5,9 @@ import { Button } from "../ui/button"; import { CircleAlertIcon, XIcon } from "lucide-react"; import { ExpandableText } from "../settings/ExpandableText"; +// Coder: no ChatGPT usage-limit branch (`ChatGptUsageButton`, the workspace admin request, and +// the `errorCode` prop); ChatGPT accounts and plan limits are cut. + export function getThreadErrorBannerMessage(error: string): string { let payload: unknown; try { diff --git a/docs/internals/sync.md b/docs/internals/sync.md index 8b977df0b5c8f..8fef199c9bb97 100644 --- a/docs/internals/sync.md +++ b/docs/internals/sync.md @@ -146,7 +146,9 @@ carried part of such a file. An upstream file added under a deleted path can still be needed: a carried file may import it (upstream #28041 adds `apps/server/src/auth/serverInternalPaths.ts`, which the carried `AssetAccess.ts`, `GitVcsDriverCore.ts`, and `WorkspaceFileSystem.ts` import). The script lists -such files apart, under "Keep candidates", with their importers. Keep each one a carried file +such files apart, under "Keep candidates", with their importers; it resolves relative imports, +the web app's `~/` alias, and `@t3tools//...` imports through each package's `exports`. +Keep each one a carried file needs (narrow the `DELETED_PATHS` entry if the directory is otherwise cut, and add the file to the seams doc); an importer that is itself an uncarried upstream addition (`binCli.ts` for `cli/up.ts`) means neither is needed. To remove every other listed upstream-added file at once: @@ -357,7 +359,8 @@ result. Read every line it prints; it changes nothing. Its checks (`--only { return ""; } }; +// Workspace packages by name, so `@t3tools//` imports resolve through their `exports`. +const workspacePackages = new Map( + git("ls-files", "-z", "--", "apps/*/package.json", "packages/*/package.json") + .split("\0") + .filter(Boolean) + .flatMap((manifest) => { + try { + const parsed = JSON.parse(readFileSync(manifest, "utf8")); + return [[parsed.name, { dir: NodePath.posix.dirname(manifest), exports: parsed.exports }]]; + } catch { + return []; + } + }), +); +const exportTarget = (value) => + typeof value === "string" ? value : (value?.import ?? value?.default ?? value?.types); +/** The repository path an import specifier names, for relative, package, and `~/` imports. */ +const resolveSpecifier = (importer, specifier) => { + if (specifier.startsWith("./") || specifier.startsWith("../")) { + return NodePath.posix.join(NodePath.posix.dirname(importer), specifier); + } + // The web app's `~` alias (apps/web/tsconfig.json and vite.config.ts). + if (specifier.startsWith("~/")) return `apps/web/src/${specifier.slice(2)}`; + const match = /^(@[^/]+\/[^/]+)(?:\/(.+))?$/.exec(specifier); + const pkg = match ? workspacePackages.get(match[1]) : undefined; + if (!pkg || typeof pkg.exports !== "object" || pkg.exports === null) return undefined; + const key = match[2] === undefined ? "." : `./${match[2]}`; + for (const [pattern, value] of Object.entries(pkg.exports)) { + const target = exportTarget(value); + if (typeof target !== "string") continue; + if (pattern === key) return NodePath.posix.join(pkg.dir, target); + const star = pattern.indexOf("*"); + if ( + star !== -1 && + key.startsWith(pattern.slice(0, star)) && + key.endsWith(pattern.slice(star + 1)) + ) { + const captured = key.slice(star, key.length - (pattern.length - star - 1)); + return NodePath.posix.join(pkg.dir, target.replace("*", captured)); + } + } + return undefined; +}; const importersOf = (path) => { const stem = NodePath.posix.basename(stripExtension(path)); let hits = ""; @@ -497,10 +540,11 @@ const importersOf = (path) => { .split("\0") .filter((file) => file && file !== path && !categoryOf(file)) .filter((file) => - [...readMerged(file).matchAll(/(?:from\s+|import\s*\(\s*)["'](\.{1,2}\/[^"']+)["']/g)].some( - ([, specifier]) => - stripExtension(NodePath.posix.join(NodePath.posix.dirname(file), specifier)) === - stripExtension(path), + [...readMerged(file).matchAll(/(?:from\s+|import\s*\(\s*)["']([^"']+)["']/g)].some( + ([, specifier]) => { + const resolved = resolveSpecifier(file, specifier); + return resolved !== undefined && stripExtension(resolved) === stripExtension(path); + }, ), ); };