Repository navigation
Conversation
HttpRuntimeTransport now implements RuntimeTransport. Status and cancel answers keep only state and a settled result, and session verbs that the v2 tool contract does not define fail closed. Co-authored-by: jinye <doudouOUC@users.noreply.github.com>
The local-process provisioner now reports a durable kind, boots from the persisted seed, and reconciles a restored lease by attesting it again. A process that no longer answers is not found instead of guessed ready. Co-authored-by: jinye <doudouOUC@users.noreply.github.com>
qwen serve --profile hosted-harness now requires a loopback bind, a harness token, and the Runtime Broker origin, token, and capability digest. The broker client speaks the merged execution routes, and the local-process provisioner adopts workers by ready record. Co-authored-by: jinye <doudouOUC@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b638dd28c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| optsIn.managedRuntimeBrokerUrl ??= | ||
| process.env['QWEN_RUNTIME_BROKER_URL']; | ||
| optsIn.managedRuntimeBrokerToken ??= | ||
| process.env['QWEN_RUNTIME_BROKER_TOKEN']; | ||
| optsIn.hostedHarnessCapabilityDigest ??= |
There was a problem hiding this comment.
When --profile hosted-harness passes validation, these settings are never consumed again: a repository-wide search finds no production construction of BrokerManagedRuntimeProvider, so the daemon starts the ordinary tool path and sends no acquire, control, or execution requests to the configured Broker. Construct and attach the provider to the selected session runtime before advertising this profile as delegating Managed Tools.
AGENTS.md reference: AGENTS.md:L261-L265
Useful? React with 👍 / 👎.
| if (!opts.token?.trim()) { | ||
| throw new Error( | ||
| `--profile hosted-harness requires a Harness bearer token. Set ` + | ||
| `${environment.serverToken} or pass --token.`, |
There was a problem hiding this comment.
When an operator supplies the documented QWEN_SERVER_TOKEN environment variable without --token, this check sees only opts.token and rejects startup. The existing environment fallback runs later in resolveRemoteServeToken, so the error even recommends a configuration that cannot pass this validation; resolve the effective token first and validate that value.
AGENTS.md reference: AGENTS.md:L261-L265
Useful? React with 👍 / 👎.
| optsIn.managedRuntimeBrokerToken ??= | ||
| process.env['QWEN_RUNTIME_BROKER_TOKEN']; |
There was a problem hiding this comment.
When QWEN_RUNTIME_BROKER_TOKEN supplies this credential, it remains in process.env and is copied shortly afterward into daemonRuntimeBaseEnv, which becomes the ACP child's source environment. The child scrub list removes QWEN_SERVER_TOKEN but not this new Broker credential, so an agent or shell tool can read a service token intended only for Harness-to-Broker calls; remove it before freezing the base environment or add it to the child denylist.
AGENTS.md reference: AGENTS.md:L270-L274
Useful? React with 👍 / 👎.
| this.entries.set(request.sessionId, entry); | ||
| } | ||
| await entry.acquisition; |
There was a problem hiding this comment.
When the initial acquire fails because of a timeout, network error, or transient Broker response, the entry and its rejected acquisition promise remain in entries. Every subsequent getToolV2Client call for the same Runtime Session retrieves that entry and awaits the identical rejection, permanently poisoning the session instead of retrying; remove the same entry on acquisition rejection as the remote provider does for failed preparations.
Useful? React with 👍 / 👎.
| public CompletionStage<Void> acquire(RuntimeLease lease, | ||
| RuntimeSession session) { | ||
| return unsupportedSessionVerb(); |
There was a problem hiding this comment.
When the new Broker server is backed by the shipped HttpRuntimeTransport, every tool-sessions:acquire request fails here with 501. RuntimeBrokerService.acquireNewSession always calls transport.acquire before marking a session ready, and this is the module's only production RuntimeTransport implementation, so control and execution routes cannot become usable without an external custom transport; implement the session verbs or provide and wire the matching transport.
AGENTS.md reference: AGENTS.md:L266-L269
Useful? React with 👍 / 👎.
| complete(exchange, service.getExecution(harnessSessionId, | ||
| runtimeSessionId, executionCallId), |
There was a problem hiding this comment.
When the Broker process has restarted, this recovery GET calls service.getExecution, which requires an entry in the process-local live-session map and returns runtime_session_not_found even though the repositories still contain the session and execution. The TypeScript recovery client deliberately performs this GET without reacquiring, so persisted results cannot be inspected after restart; use a persisted ownership lookup for this read path.
AGENTS.md reference: AGENTS.md:L266-L273
Useful? React with 👍 / 👎.
| if (this.entries.size + this.pendingBuilds >= MAX_INVOCATIONS) { | ||
| throw new Error('Managed tool invocation capacity is exhausted.'); |
There was a problem hiding this comment.
In a long-lived managed session, every completed or cancelled invocation remains in entries until dispose(), so the 1,025th distinct tool call deterministically fails this capacity check even when no work is active. Each retained entry can also keep up to 1 MiB of progress, allowing nearly 1 GiB of stale progress to accumulate; remove settled prior-turn entries or maintain a bounded history that does not consume active admission capacity.
Useful? React with 👍 / 👎.
| if (filePath.split(path.sep).includes('..')) | ||
| throw new ManagedToolProtocolError(); |
There was a problem hiding this comment.
On Windows, a wire key such as ../outside passes this check because path.sep is \, but the later path.join(cwd, filePath) recognizes / and resolves it outside the execution directory. This defeats the protocol's explicit parent-segment rejection for restored file-history state; validate both / and \ components or resolve and verify containment.
AGENTS.md reference: AGENTS.md:L270-L274
Useful? React with 👍 / 👎.
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { randomUUID } from 'node:crypto'; |
There was a problem hiding this comment.
This external feature adds approximately 1,876 production lines under packages/core/src and changes multiple packages, exceeding the repository's 1,000-line non-blocking advisory threshold for core infrastructure. Route it through maintainer review rather than treating ordinary external approval as sufficient.
AGENTS.md reference: AGENTS.md:L21-L37
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06130ca95b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| execution = { | ||
| referenceDigest, | ||
| reserved: this.client.prepareExecution( | ||
| entry.request.sessionId, | ||
| entry.harnessSessionId, |
There was a problem hiding this comment.
When both internal attempts to reserve an execution fail because of a transient timeout, network error, or 5xx response, the rejected reserved promise remains in entry.executions; every later status, execute, or cancel for that invocation reuses the same rejection without contacting the Broker. The cached started promise below has the same behavior after a transient start or polling failure, so even an execution that subsequently settles at the Broker cannot be recovered through the issued client. Remove or reset the matching cached promise on rejection so the durable idempotency key and execution ID can be retried safely.
Useful? React with 👍 / 👎.
What this PR does
This is the hosted local closed loop split out of preview QwenLM#12358, on current QwenLM main (
90232f0eb, published here ascursor/upstream-main-3df3).qwen serve --profile hosted-harnessnow fails unless the bind is loopback, a harness bearer token is set, WebShell and the experimental Managed Gateway flags are off, and the Runtime Broker URL, token, and capability digest are present. The broker client (BrokerManagedRuntimeProvider) speaks acquire, prepare, start, cancel, resolve, and release. On the Java side the merged transport implementsRuntimeTransport, the local-process provisioner adopts a worker from its ready record, andRuntimeBrokerHttpServerexposes that contract. Static placements return a fixed lease.The two adapter commits are the same changes as #3 and #4, included here so this block stands alone. The HTTP listener is the same class as in the Spring control-plane block.
Why it's needed
Issue QwenLM#12380's first runnable slice is a Hosted Harness that returns model output without waiting for the Tool Runtime, then sends tool calls through the broker. That path was buried in the preview's serve entry, mixed with the experimental gateway. This block keeps the profile, the broker client, the v2 tool contract, and the local provisioner together.
Reviewer Test Plan
How to verify
In
packages/sdk-java/runtime-broker, runmvn -Dtest=HttpRuntimeTransportTest,LocalProcessRuntimeProvisionerTest -Dgpg.skip=true test checkstyle:check. Those tests passed on Linux JDK 21 before this branch was cut from the same commits.TypeScript tests were not run: this workspace has no
node_modules. A reviewer with a built tree should run the broker provider test and the hosted harness contract test:npx vitest run packages/cli/src/serve/broker-managed-runtime-provider.test.ts packages/cli/src/serve/hosted-harness-contract.test.ts packages/cli/src/serve/managed-runtime-provider.test.tsEvidence (Before & After)
N/A for screenshots. Java broker tests for the included adapter commits passed earlier on Linux. The new profile validation is a startup check in
runQwenServeImpland is not covered by a new test in this branch.Tested on
Java adapter tests were run on Linux on the same commits. The TypeScript profile wiring in this commit was not executed.
Environment (optional)
JDK 21.0.10, Maven 3.8.7. Node dependencies are not installed in this workspace.
Risk & Scope
runQwenServeImplvalidates the profile and reads broker settings from flags orQWEN_RUNTIME_BROKER_URL,QWEN_RUNTIME_BROKER_TOKEN, andQWEN_HOSTED_HARNESS_CAPABILITY_DIGEST. It does not yet constructBrokerManagedRuntimeProviderinside the session loop. On the preview that construction sits in the same function as the experimental gateway, ordinary execution engines, and daemon reload, and those paths are not in this diff. A process started with--profile hosted-harnesstherefore checks the deployment boundary and still serves the existing tool path until that mount is split out.scripts/run-managed-hosted-runtime-e2e.tsstays on the preview because it launchesRuntimeBrokerFixtureMainanddist/managed-runtime-worker.js, which are not this branch. Kubernetes provisioning is not included. Grep, notebook, and other managed tool migrations are not included; the v2 protocol and file-history types are, because the broker client and tool runtime share them.prepareon the HTTP server starts dispatch immediately, matching mergedcreateExecution. Resolve callsreconcileExecutionand does not apply the caller's resolution label as a result.--profiledefaults todefault. Broker flags withouthosted-harnessthrow at startup. This pull request is ondoudouOUC/qwen-codeand cannot merge intoQwenLM/qwen-codedirectly.Linked Issues
Related to QwenLM#12380. Split from QwenLM#12358. Does not close either.
中文说明
这是从预览 QwenLM#12358 拆出的 Hosted 本地闭环，基线是当前 QwenLM main（
90232f0eb，在本仓库记为cursor/upstream-main-3df3）。qwen serve --profile hosted-harness现在会在启动时检查：绑定地址必须是 loopback，必须有 Harness bearer token，不能开 WebShell，也不能带着实验性 Managed Gateway 参数，并且要有 Runtime Broker 的 URL、token 和 capability digest。Broker 客户端走 acquire、prepare、start、cancel、resolve、release。Java 侧合并后的 transport 实现了RuntimeTransport，local-process 供给器按 ready record 认领 worker，RuntimeBrokerHttpServer暴露这份契约。static 供给器返回固定租约。#3 和 #4 的两笔适配包含在本块里，这样本块可以单独看。HTTP 监听器和 Spring 控制面那一块是同一个类。
为什么需要
QwenLM#12380 的第一段可运行切片是 Hosted Harness：模型先返回，工具调用再经 Broker。这条路径原来埋在预览的 serve 入口里，和实验性 Gateway 缠在一起。这一块把 profile、Broker 客户端、v2 工具契约和本地供给器放在一起。
风险与范围
runQwenServeImpl会校验 profile，并从参数或QWEN_RUNTIME_BROKER_URL、QWEN_RUNTIME_BROKER_TOKEN、QWEN_HOSTED_HARNESS_CAPABILITY_DIGEST读取 Broker 设置。它还没有在会话循环里构造BrokerManagedRuntimeProvider。预览里这段构造和实验性 Gateway、普通执行引擎、daemon 重载写在同一个函数里，那些路径不在本 diff 中。因此带上--profile hosted-harness的进程会先卡住部署边界，工具路径仍是现有实现，直到挂载点被单独拆出来。冷启动 E2E 脚本仍留在预览上，因为它要拉起
RuntimeBrokerFixtureMain和dist/managed-runtime-worker.js。本环境没有安装node_modules，TypeScript 测试没有跑。Java 适配测试在同样的提交上于 Linux 通过。本 PR 在
doudouOUC/qwen-code，不能直接合并进QwenLM/qwen-code。关联 QwenLM#12380，从 QwenLM#12358 拆出，不关闭这两个单子。