diff --git a/HOSTED_SHELL_UNICODE_E2E.md b/HOSTED_SHELL_UNICODE_E2E.md new file mode 100644 index 00000000000..e1b8d3a6d42 --- /dev/null +++ b/HOSTED_SHELL_UNICODE_E2E.md @@ -0,0 +1,144 @@ +# Hosted Shell Unicode E2E + +This guide covers the real-process gate for the fix described in the +[English design](docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.md) +and the [Chinese design](docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.zh-CN.md). +It covers one behavior: a Hosted Shell call that carries an unpaired UTF-16 +surrogate is refused before Workspace admission, the refusal is durable, and the +model can correct the call in the same turn. + +## What the test proves + +The gate runs one JUnit parameterized test with two capture paths, `local` and +`o2`. For each capture path it proves these points: + +- Eight standalone malformed batches produce ordered, paired tool errors. The + matrix is `command` and `description`, each with `\ud800`, `\udc00`, + `😀\ud800`, and `\udc00\ud800`. +- The only Broker requests these batches make end in `/runtimes:warm`, which + means no `acquire`, `prepare`, or `start` request. Runtime warming starts + before the model output, so the test allows it. +- The driver observes no new entry in the publication reservation list. +- Neither the malformed shell command nor its valid `write_file` sibling creates + a file. +- Session detach and load keep the original arguments and the paired errors. +- A corrected command runs once in the same prompt turn. It preserves Chinese + text, emoji, combining characters, and the literal six-character `\ud800`. +- A second Session on the same Workspace runs a valid call. Neither path becomes + `recoveryBlocked`. + +The test uses the `hosted-workspace-shell/1` profile. The `/2` profile has unit +coverage only. + +## Prerequisites + +- Node.js 22 or later. +- JDK 21. Set `JAVA_HOME` to it. +- Maven 3.8.9 or later. +- Installed workspace dependencies. + +From the repository root: + +```bash +corepack pnpm install --frozen-lockfile +``` + +## Build the CLI + +The Java gate runs the bundled CLI as the Runtime worker, so build it first: + +```bash +npm run build +npm run bundle +``` + +The gate reads `dist/cli.js`. Set `qwen.cli.entry` to that file when you run the +test. + +## Install the Java prerequisite + +The Runtime Broker module no longer has its own POM; the server module uses the +`qwencode` module: + +```bash +mvn -q -f packages/sdk-java/qwencode/pom.xml -DskipTests -Dgpg.skip=true install +``` + +## Run the gate + +Compile the server and its tests first: + +```bash +mvn -f packages/managed-agent-server/pom.xml -DskipTests test-compile +``` + +Then run the single test method through the failsafe plugin: + +```bash +mvn -f packages/managed-agent-server/pom.xml \ + -Phosted-workspace-tools \ + -Dnode.executable="$(command -v node)" \ + -Dqwen.cli.entry="$PWD/dist/cli.js" \ + -Dit.test='HostedWorkspaceToolTurnIT#malformedShellUnicodeIsCorrectableBeforeAcquisition' \ + failsafe:integration-test failsafe:verify +``` + +Do not add `-Dtest=none` or `-Dsurefire.failIfNoSpecifiedTests=false`. The +module sets `failIfNoTests` to `true`, and that combination stops the build at +surefire with `No tests were executed!`. + +## Success conditions + +The run passes when all of these lines appear: + +```text +HOSTED_SHELL_UNICODE_OK: local, 9 refusals per capture, durable reload, Unicode correction, second Session +HOSTED_SHELL_UNICODE_OK: o2, 9 refusals per capture, durable reload, Unicode correction, second Session +Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 +BUILD SUCCESS +``` + +The Druid `testWhileIdle is true, validationQuery not set` line is expected and +does not indicate a failure. + +## Recorded result + +Measured on macOS arm64 with Node.js 22.23.3, JDK 21.0.13, and Maven 3.9.9, on +the merge of `main` `ce8a94706b` into the PR branch: + +```text +HOSTED_SHELL_UNICODE_OK: local, 9 refusals per capture, durable reload, Unicode correction, second Session +HOSTED_SHELL_UNICODE_OK: o2, 9 refusals per capture, durable reload, Unicode correction, second Session +[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.79 s -- in com.alibaba.qwen.code.managedagent.HostedWorkspaceToolTurnIT +[INFO] BUILD SUCCESS +``` + +## Test environment + +The gate runs in one JVM and starts these real components: + +- The managed-agent server with a temporary H2 database in MySQL mode. +- An `EmbeddedRuntimeBroker`. +- A real Node worker process that runs `dist/cli.js`. +- A deterministic fake OpenAI server that sends the malformed JSON escapes. + +O2 uses the real Java publication controller and SQL grant and data services. The +object store is an in-memory test adapter. No model API key and no OSS account +are needed. + +This is not a MySQL, OSS, Linux, or Windows acceptance run. It also does not +cover a real `hosted-workspace-shell/2` process. + +## Troubleshooting + +- `POM ... does not exist`: use `packages/sdk-java/qwencode/pom.xml`. The + `runtime-broker` POM was consolidated and no longer exists. +- `No tests were executed!`: you passed `-Dtest=none`. The module sets + `failIfNoTests` to `true`, so surefire stops the build. Use the command above. +- `Pass -Dnode.executable with an absolute Node.js 22+ path`: add + `-Dnode.executable="$(command -v node)"`. +- `AssertionError` about `dist/cli.js` not being a regular file: run + `npm run bundle` and check `qwen.cli.entry`. +- `recoveryBlocked` in the failure dump: the refusal was not recognized on + reload. Check that the assistant message and both tool errors were persisted, + and that the batch has no native tool intent. diff --git a/docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.md b/docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.md new file mode 100644 index 00000000000..84db0c82879 --- /dev/null +++ b/docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.md @@ -0,0 +1,339 @@ +# Hosted Shell Unicode admission + +[English](2026-10-08-hosted-shell-unicode-admission.md) | [简体中文](2026-10-08-hosted-shell-unicode-admission.zh-CN.md) + +## 1. Status and evidence + +This is the fix design for [#13010](https://github.com/QwenLM/qwen-code/issues/13010), +dated 2026-10-08 and implemented in the local workspace. Section 9 records the +local verification. It does not represent production release authorization, and +the issue is not closed. + +Two rounds of design audit are complete: round 1 found two problems, both +revised; a fresh round 2 found no new substantive problem. This does not mean +runtime acceptance passed. + +The issue and its full comment history were read through `gh`. The historical +evidence is the +[O2 round 9 report](https://github.com/QwenLM/qwen-code/pull/12894#issuecomment-5888143498) +on `3da25b7c`: an unpaired surrogate in Shell arguments blocked both capture +paths, and a second Session on the same Workspace still received +`workspace_busy` after eight minutes. That report and its log links should be +kept. The report did not run an independent main build, so it is not a +reproduction on current main. + +The source review is based on local `040f06c7b141235153d69c3c795da9f8550eb900`. +Upstream main was `fe4d4e345c3f52fd6d8455646cdd3c782ca8b227` when queried; the +GitHub comparison shows that `hosted-workspace-tool-turn.ts`, +`hosted-workspace-broker.ts` and `managed-tool-protocol.ts` did not change +between the two. Hook lifecycle code and tool-turn tests did change; the work +was fast-forwarded to that upstream main before implementation and those changes +were read. Runtime reproduction and implementation evidence are in section 9. + +Baseline facts before implementation: + +- `HostedWorkspaceToolTurn.executeNative` builds both the local capture and the + O2 publication request in one place. It checks Shell argument shape, but not + UTF-16 well-formedness. +- When a batch has a `validationError`, the existing path already persists the + assistant message and paired tool errors without acquiring a lease or + executing. O2 now uses this path as well; the historical O2 behaviour of + throwing a plain validation error is obsolete. +- `PermissionRequest` currently runs before `prepareRequests`. `PreToolUse` runs + after the lease is acquired and can rewrite arguments, after which + `prepareRequests` runs again. `execute` also calls `completeHookResults`, + which includes `PostToolBatch`. +- `/runtimes:warm` already starts at construction. An earlier prompt Hook or an + earlier valid batch may already hold the Workspace lease. +- Java `BrokerValues` and the Broker/transport already reject incomplete text, + but a late rejection does not replace the early, durable, model-correctable + refusal this issue requires. +- `packages/core/src/tools/managed-shell.ts` from the comment no longer exists; + the suggested `hasUnpairedSurrogate` is a private function of the + external-context integration and cannot be imported across packages. + +## 2. Problem, goals, and scope + +JSON arguments can carry an escaped, unpaired UTF-16 high or low surrogate. After +parsing succeeds, a later encoder or the Java boundary may reject or replace the +value. If an execution owner was already acquired, the uncertain outcome keeps +the Workspace fenced. A malformed Shell call must therefore be rejected before +it enters that execution chain. + +Goals: + +1. On both Hosted Shell capture paths, reject an incomplete `command` and a + string `description` before acquisition and reservation of the refused batch. +2. Give the model a durable, ordered, correctable function error; a corrected + call later in the same turn can execute. +3. Preserve valid Unicode and the algorithm and bytes of the canonical digest. +4. Cover Hook rewrites so that incomplete Shell input cannot reach a later + command Hook or the Runtime. +5. Keep blocking on uncertain persistence and on side effects that were already + admitted. + +Scope is Hosted `run_shell_command` `/1`, `/2`, local capture, and O2. When the +existing H3 branch is selected, the same field check must not disappear, but this +fix does not enable background Shell. Ordinary CLI Shell, Monitor, MCP +arguments, other file-tool text, general JSON validation, recovery of already +stuck Sessions, public Shell admission, and a new protocol version are out of +scope. Valid unrelated tools keep their current behaviour. + +## 3. Chosen approach + +Add one small pure Shell-text validator in +`packages/cli/src/serve/hosted-shell-input.ts`, consumed by the two real callers: +the tool turn and the Hosted Hook session. Do not export it from core and do not +build a general sanitizer. Keep the existing type, required-field, and profile +checks. Detect unpaired surrogates in string fields with +`/[\uD800-\uDFFF]/u`: the Unicode flag makes a valid surrogate pair one +supplementary-plane code point, so it does not match the surrogate range. The +`u` flag must not be omitted. + +Use fixed error messages that name only the field and never contain its value: + +- `Hosted Shell command contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.` +- `Hosted Shell description contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.` + +Do not trim, normalize, delete, or replace code units. The printable six-character +`\ud800` sequence is valid text; a parsed unpaired `\uD800` code unit is not. An +omitted or empty description remains valid. A missing or non-string command and a +non-string description keep their existing errors; unknown keys and other shape +problems keep being rejected by the existing validation. + +Reject instead of replace: changing command bytes can change side effects and +breaks the correspondence among input, approval, and execution. Keep +`managedToolDigest`, the payload JSON, Tool v3, Java validation, and the resource +schema unchanged. Do not migrate the integration's private function into core +just to reuse this one check. + +## 4. Admission and Hook order + +Use the following order and keep the existing profile/identity, truncated +argument, and inline-size refusal semantics: + +1. Check the model's Shell text before `PermissionRequest` and before any Hook + that carries arguments. If any call fails, refuse the whole batch through the + persisted argument-refusal path, including valid siblings. +2. An initially Unicode-refused batch runs no command Hook and no approval + Action, and skips the post-tool callbacks in `completeHookResults`, including + `PostToolBatch`. The shared completion entry still rebuilds the refusal + evidence, and its callbacks do not receive the original incomplete command. +3. Inside `HostedHookSession.fireOccurrence`, after each successfully applied + sequential Hook result, validate the effective Shell input before admitting + the next Hook. Cover the `tool_input` merge performed for `updatedInput` and + `sequentialInput`, not only the final aggregate. Stop at the first invalid + effective input even if a later Hook could repair it. Keep the results of the + children that already ran and the original plan, aggregate the executed + prefix through the existing result resource, and return its invalid effective + input so the tool-turn validator produces the fixed, model-correctable + error. Do not add a general interrupt, do not end the whole model turn, and + do not admit a later child. +4. `settleSavedMarker` must rebuild the same ordered prefix and stop condition + from the saved child results, and evaluate the stop condition before looking + for a later child. Then, if the process crashes after an invalid child result + is saved but before the plan marker result is committed, the marker can be + finished from that prefix instead of waiting forever for a child that was + deliberately not created. When an earlier child result is uncertain, keep + blocking under the existing rule. Parallel Hooks keep receiving the original + input and are validated on the final aggregate before native execution; do not + cancel admitted sibling Hooks and do not change their ordering. +5. When `PermissionRequest` returns `updatedInput`, validate the effective Shell + text again before publishing the approval input or native acquisition. A + malformed rewrite refuses the whole native tool batch. The Hook itself may + already have acquired the Runtime; keep the existing owner and let normal + settlement handle it. +6. Check again in `prepareRequests`, including the call after `PreToolUse`. If a + Hook introduced incomplete text, ask for no approval, reserve no publication, + and start no native tool in that batch; refuse every call. Acquisition, + publisher registration, or the first approval may already have happened, so + do not claim the batch was completely inert, and do not force-clear the owner. +7. For a Unicode refusal caused by a Hook rewrite, do not forward the incomplete + effective arguments to a post-tool callback; skip that batch's post-tool + callbacks, keep the committed Hook decisions and results, and persist the + tool refusal normally. Do not fabricate an executed tool intent for a refused + call. +8. Only effective input that passes validation enters the existing + reservation/start path. The rule that a valid rewrite is re-approved stays + unchanged. + +Reuse the existing batch-refusal persistence logic, extracting it only where +these entries need it. A refusal before the assistant message is committed writes +the assistant once and one ordered response per call; a refusal after the +assistant message is committed writes only the responses and must not append a +second assistant message. + +Do not rely on a transient flag to skip post-tool callbacks. At the +`completeHookResults` entry shared by live `execute` and cold resume +`resumeHookResults`, rebuild the original assistant group and that batch's saved +`PermissionRequest`/`PreToolUse` results from the original occurrence/call +identity, and restore the effective input by the same rules used at admission, +including a prefix terminated by the Unicode check. Return the saved responses +and run no callback and re-commit no assistant/result only when invalid Shell +text, a complete paired refusal group, and the absence of native tool intents for +the batch together prove a Unicode refusal. Do not decide from the error string +alone and do not skip every batch that has no intent; an ordinary +argument/approval refusal keeps its current post-batch behaviour. Contradictory +native intent or an incomplete refusal record is a recovery error, not evidence +of a safe refusal. No new persisted flag or schema field is needed; the next +valid batch decides again from its own records. + +## 5. Persistence, ownership, and compatibility + +Error responses contain only the fixed reason, the original function-call +identity, and the existing response shape; valid siblings receive the existing +explicit not-executed error. Error messages and diagnostics do not echo the +invalid field value. Persist the original assistant function-call group through +the existing JSON-escaped record without rewriting the original command and +without claiming it executed. Prove that the group persists byte-for-byte and +reloads using the real Java Store, not only an in-memory TS mock. + +For a fresh refusal batch without Hooks, require zero acquire, zero publisher +registration, zero publication reservation, zero prepare, zero start/execute, +and zero worker side effects. Runtime warming happens before the model output; +allow it and record it separately. The approach therefore does not promise +completely zero Broker HTTP traffic or stop independent provisioning. + +A later invalid batch must not acquire, reserve, or start again, but it may +inherit the lease held by earlier valid work. Normal final settlement releases +the owner; the validator neither releases the owner nor rewrites an existing +execution to `not_started`. When the refusal commit succeeds it is a +model-correctable result, not recovery-blocked; when the assistant or result +commit fails and the outcome is uncertain, keep the existing recovery blocking. +A Shell that did not execute cannot prove journal consistency. + +An incomplete command is rejected before its approval, and no approval is +created for that command. No configuration, migration, public DTO, protocol +version, new error-code enum, or Shell capability enablement is needed. +Subsequent source implementation must follow the maintainer review rules for +core and cross-package changes; this design is not PR approval and does not +authorize rollout. + +## 6. Implementation and verification plan + +Before implementation, a read-only `test-engineer` reproduces the issue with an +independent real Java Store/Broker/Harness/worker environment on a build of main +at that time. Try the installed global `qwen` first; when a private profile is +unreachable, record it as unavailable and do not count it as a baseline pass. +Use a deterministic model fixture to cover local capture and O2 separately; +input-admission tests do not need a real OSS bucket. Save the round 9 logs and +the new exact-head before/after results, and put the E2E plan and report in +`.qwen/e2e-tests/`. + +Main implementation/test files: + +- `packages/cli/src/serve/hosted-workspace-tool-turn.ts`. +- `packages/cli/src/serve/hosted-shell-input.ts` and its collocated tests. +- `packages/cli/src/serve/hosted-hook-session.ts` and tests, changing only the + Shell effective-input boundary and saved-marker reconstruction. +- `packages/cli/src/serve/hosted-workspace-tool-turn.test.ts`, plus Hook tests as + needed to prove callback order and suppression. +- A native Hosted integration fixture that covers real Java persistence, file + side effects, and a second Session on the same Workspace. After rebasing, + choose an existing driver that really drives both capture paths; do not add a + second production admission entry just to test this fix. + +| Test | Evidence that must be provided | +| ------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Unicode negatives | Both fields, both paths: unpaired high/low surrogate, high surrogate followed by ASCII, reversed pair, high-high, low-low, valid pair followed by an unpaired surrogate. Cover any position, including after a valid emoji. | +| Unicode compatibility | ASCII, Chinese, supplementary-plane emoji, valid surrogate pair, combining characters, ESC, U+0001, DEL, U+2028/U+2029, and the literal `\ud800` in command/description keep byte-identical bytes; command semantics stay unchanged. | +| Batch refusal | Invalid call first/last, mixed with valid Shell and Write; all errors persisted in order, every sibling explicitly not executed, no tool intent and no publisher reservation. | +| Correction | A call corrected in the same turn executes exactly once; the result reaches the next model request; final settlement succeeds and reload and later turns work. | +| Ownership | A new refusal holds no Workspace lease and a valid call in a second Session completes; a later refusal with an existing lease keeps the owner until normal settlement. | +| Persistence failure | Fail the assistant/result write separately and lose the response of a committed operation; no execution, no false success, existing recovery blocking kept, no duplicated history. | +| Hooks | The original invalid input reaches no permission/pre/post/batch Hook; an invalid `PermissionRequest` or `PreToolUse` rewrite cannot native start or reach a later callback that carries invalid arguments. A real two-Hook ordered plan covers `updatedInput` and the `tool_input` merge: H1 introduces invalid text, H2 is never admitted even if H2 could repair it. A crash after the H1 result and before the marker result resumes the marker without the H2 child; a valid rewrite is still re-approved and reaches H2. | +| Hook cold completion | Reload the original invalid and Hook-rewritten refusal groups and Hook records into a new tool turn, call the real `resumeHookResults` caller, and prove no post-batch callback, no duplicated history, and no native operation. Ordinary refusal replay and the next valid batch keep their current behaviour. | +| Store and model transport | The real Java Store persists and reloads the original escaped assistant group and paired errors; the deterministic model receives them and corrects the call. Acceptance fails if encoding changes or transport rejects the group. | +| Limits and identity | Existing inline-size, unsupported-tool, duplicate-call, and incomplete-argument guards still apply; correctness is not promised when the existing limits are exceeded. | +| Regression and mutation | Existing local/O2 refusal, approval, capture, and Hook tests pass; removing each admission check and each callback-suppression check must fail the corresponding negative case; removing the `u` flag must fail the valid-emoji witness. | + +Run these in the respective package directories: + +```bash +# Repository root +npm run build && npm run typecheck + +# packages/cli +npx vitest run src/serve/hosted-workspace-tool-turn.test.ts +npx vitest run src/serve/hosted-shell-input.test.ts src/serve/hosted-hook-session.test.ts +# Add the actually changed Hook/native driver suites, then the real-stack gate. +``` + +Section 9 records the checks that were completed. A catalog-only or worker-only +result cannot prove pre-acquisition admission; unrelated full test suites are not +required. + +## 7. Acceptance and risk + +The fix is complete only when every verification above passes on the same final +source version, both capture paths refuse invalid input before the first native +side effect, valid text keeps its bytes, the corrected call and the second +Session work, and a failed refusal persistence still blocks safely. Public Shell +enablement and recovery of historically stuck Sessions remain separate work. + +The main risks are a wrong validator position relative to Hooks, post-batch +completion forwarding invalid input again, duplicated assistant commits after a +rewrite, and relying only on TS persistence evidence. Inline limits and side +effects already in flight can legitimately prevent a correctable continuation; +this must be recorded explicitly. + +Writing this design needs no additional product decision. Before source +delivery there must be a current-main reproduction and Java byte-for-byte +round-trip evidence; any failure must change the implementation/design, not +silently replace the invalid command or claim the issue is fixed. + +## 8. Independent audit record + +The user asked for a fresh Sub Agent audit and reverse audit each round, updated +between rounds, for at most seven rounds. Each round checks both whether the +approach can close the issue and whether the new refusal/ordering breaks valid +behaviour or makes a verification claim unprovable. + +| Round | Problem and resolution | Result | +| ----- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ | +| 1 | The new auditor audit_round_1 found sequential Hook propagation before the outer check (P1) and lost callback suppression on cold recovery (P2). Added the shared Hosted validator, sequential-prefix terminal reconstruction, persisted completion decision, and the corresponding negative/compatibility witnesses. | Both accepted and revised; round 2 independently re-checked them. | +| 2 | The new auditor audit_round_2 independently checked the issue/source, auditing forward behaviour and reverse compatibility/recovery negative cases. No new substantive problem was found; the round 1 revisions covered the risks. The report keeps the implementation constraints for the event return shape and nested `PermissionRequest` without widening valid Hook semantics. | Met the user's "no new problem" stop condition and ended the audit loop (max 7, actual 2). | + +Local audit reports: `.qwen/pr-reviews/issue-13010-design-round-1.md` and +`.qwen/pr-reviews/issue-13010-design-round-2.md` (git-ignored working artifacts). + +## 9. Implementation and local verification + +The implementation uses the shared pure validator before the permission Hook and +in request preparation, terminates an incomplete sequential Hook prefix on both +the live and saved-marker paths, and rebuilds the persisted Unicode refusal +evidence before callbacks. It shares the existing ordered refusal path; it adds +no persisted schema, core validator, digest algorithm, or production Java +protocol. + +Before the fix, a build of main admitted a U+D800 command to +acquire/prepare/start on the local path and still had an active prompt after 30 +seconds. Eight controlled source fallbacks independently observed acquisition. +The early O2 fixture lacked a publication endpoint, so its recovery block alone +cannot prove Unicode caused the lockup. These observations did not reproduce the +historical eight-minute incident. + +The final real-process Maven verify gate passed both the local and O2 parameters. +Each path covers eight incomplete command/description batches and one more batch +that is refused and then corrected in the same turn. The model checks the +original arguments and ordered paired errors before and after detach/load. The +Broker and Store proxy observe no native admission or publication request for a +fresh invalid batch; the primary Session's SQL execution ledger has exactly one +execution, the corrected one. A second Session on the same Workspace completes a +valid call on both paths. + +This stack uses temporary H2, an Embedded Broker, and a real Node worker +process. O2 uses the real Java SQL grant/data services and publication +controller, with an in-memory test adapter for the object store; this is not +MySQL, OSS, or Linux isolation acceptance. Controlled CLI regression separately +covers `/1` and `/2`, both capture paths, invalid first/last calls mixed with +valid Shell/Write siblings, persistence failure and lost replies after commit, +retention of an existing owner, real Hook rewrites, and cold completion. Seven +mutations (original admission, permission rewrite preparation, `PreToolUse` +rewrite, live/cold sequential stop, callback suppression, Unicode flag) each fail +their witness. Source was restored verbatim before the final regression. + +Exact commands, environment, success conditions, results, and troubleshooting are +in the [local E2E guide](../../../HOSTED_SHELL_UNICODE_E2E.md). The local +implementation code review has no Critical and two E2E Suggestions, both accepted +and fixed. diff --git a/docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.zh-CN.md b/docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.zh-CN.md new file mode 100644 index 00000000000..a1c8bf52385 --- /dev/null +++ b/docs/design/managed-agents/2026-10-08-hosted-shell-unicode-admission.zh-CN.md @@ -0,0 +1,265 @@ +# Hosted Shell Unicode 准入 + +[English](2026-10-08-hosted-shell-unicode-admission.md) | [简体中文](2026-10-08-hosted-shell-unicode-admission.zh-CN.md) + +## 1. 状态与证据 + +这是 [#13010](https://github.com/QwenLM/qwen-code/issues/13010) 的修复方案， +日期为 2026-10-08，已在本地工作区实施。第 9 节记录本机验证， +不代表生产发布授权或 issue 已关闭。 + +设计审计已完成两轮：第 1 轮发现两项问题并已修订；全新第 2 轮没有发现新的实质 +问题。这不代表运行时验收通过。 + +已通过 `gh` 阅读 issue 及完整评论。历史证据是 `3da25b7c` 上的 +[O2 第 9 轮报告](https://github.com/QwenLM/qwen-code/pull/12894#issuecomment-5888143498)： +Shell 参数中的孤立代理项使两条捕获路径都被阻塞，同 Workspace 的第二个 Session +在八分钟后仍收到 `workspace_busy`。应保留该报告及其日志链接。报告没有运行独立 +main 构建，因此不算当前 main 的复现。 + +源码检查基于本地 `040f06c7b141235153d69c3c795da9f8550eb900`。查询时上游 main +为 `fe4d4e345c3f52fd6d8455646cdd3c782ca8b227`；GitHub 对比显示两者之间 +`hosted-workspace-tool-turn.ts`、`hosted-workspace-broker.ts` 和 +`managed-tool-protocol.ts` 未变。Hook 生命周期代码及 tool-turn 测试有所变化， +实施前已 fast-forward 到该上游 main，并阅读这些变化。 +运行时复现与实施证据见第 9 节。 + +实施前的基线事实： + +- `HostedWorkspaceToolTurn.executeNative` 同时构造 local capture 与 O2 + publication 请求。它检查 Shell 参数形状，但不检查 UTF-16 是否完整。 +- 批次存在 `validationError` 时，已有路径会持久化 assistant 和配对的工具错误， + 无须获取租约或执行。O2 现在也走这条路径；历史上 O2 直接抛出普通校验错误的 + 行为已经过时。 +- `PermissionRequest` 当前在 `prepareRequests` 前运行。`PreToolUse` 在获取租约 + 后运行，可以改写参数，随后再次调用 `prepareRequests`。`execute` 还会调用 + `completeHookResults`，其中包含 `PostToolBatch`。 +- 构造时已经启动 `/runtimes:warm`。更早的 prompt Hook 或此前的合法批次可能已 + 持有 Workspace 租约。 +- Java 的 `BrokerValues` 以及 Broker/transport 已拒绝不完整文本，但晚期拒绝 + 不能代替本 issue 要求的、提前且持久的模型可纠正拒绝。 +- 评论中的 `packages/core/src/tools/managed-shell.ts` 已不存在；建议复用的 + `hasUnpairedSurrogate` 是 external-context integration 的私有函数，不能直接 + 跨包导入。 + +## 2. 问题、目标与范围 + +JSON 参数可以携带转义的孤立 UTF-16 高代理项或低代理项。解析成功后，后续编码器 +或 Java 边界可能拒绝或替换该值。若此前已经获取执行 owner，不确定结果就会保留 +Workspace fence。因此应在进入该执行链路前拒绝错误的 Shell 调用。 + +目标： + +1. 在两条 Hosted Shell 捕获路径中，拒绝不完整的 `command` 和字符串 + `description`，先于被拒批次的执行获取与预留。 +2. 给模型持久、有序、可纠正的 function error；同一 turn 后续纠正的调用可以执行。 +3. 保留合法 Unicode，以及 canonical digest 的算法和字节。 +4. 覆盖 Hook 改写，避免将不完整的 Shell 输入转发给后续 command Hook 或 Runtime。 +5. 对不确定的持久化和此前已准入的副作用继续保持阻塞。 + +范围为 Hosted `run_shell_command` 的 `/1`、`/2`、local capture 和 O2。 +现有 H3 分支被选择时，同样的字段检查不能消失，但本修复不启用后台 Shell。 +普通 CLI Shell、Monitor、MCP 参数、其他文件工具文本、通用 JSON 校验、已有阻塞 +Session 的恢复、公开 Shell 准入和新协议版本不在范围内。合法的无关工具保持原行为。 + +## 3. 选定方案 + +在 `packages/cli/src/serve/hosted-shell-input.ts` 添加一个小型纯 Shell 文本校验 +函数，由 tool turn 和 Hosted Hook session 两个真实消费者使用。不从 core 导出， +不建立通用 sanitizer。保留既有类型、必填字段和 profile 校验。对字符串字段使用 +`/[\uD800-\uDFFF]/u` 检测孤立代理项：Unicode 标志使合法代理对成为一个补充 +平面码点，不匹配代理项范围。不能遗漏 `u`。 + +使用固定错误消息，只指出字段，不包含其值： + +- `Hosted Shell command contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.` +- `Hosted Shell description contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.` + +不 trim、不规范化、不删除或替换码元。可打印的六字符 `\ud800` 序列是合法文本； +解析得到的孤立 `\uD800` 码元不是。省略或空的 description 仍合法。缺失或非字符串 +command、非字符串 description 继续由已有错误处理；未知 key 和其他形状问题继续 +由已有校验拒绝。 + +选择拒绝而不是替换：修改命令字节可能改变副作用，也会破坏输入、审批、执行之间 +的对应关系。保持 `managedToolDigest`、payload JSON、Tool v3、Java 校验和资源 +schema 不变。不为复用这一个检查而将 integration 的私有函数迁入 core。 + +## 4. 准入与 Hook 顺序 + +采用以下顺序，并保留既有 profile/identity、截断参数和 inline-size 拒绝语义： + +1. 在 `PermissionRequest` 和任何携带参数的 Hook 前检查模型的 Shell 文本。 + 任一调用失败时，通过持久化参数拒绝路径拒绝整个批次，包含合法 sibling。 +2. 初始 Unicode 被拒批次不运行 command Hook、审批 Action，跳过 + `completeHookResults` 中的 post-tool callback，包含 `PostToolBatch`。 + 共用 completion 入口仍重建拒绝证据，其 callback 不接收原始不完整命令。 +3. 在 `HostedHookSession.fireOccurrence` 内，每次应用成功的顺序 Hook 结果后， + 先校验生效的 Shell 输入，再准入下一个 Hook。覆盖 `updatedInput` 和 + `sequentialInput` 执行的 `tool_input` 合并，不能仅检查最终 aggregate。 + 首次出现非法生效输入即停止，即使后续 Hook 可以修正也不继续。保留已执行 child + 的结果及原 plan，通过既有 result resource 聚合已执行前缀；返回其非法生效输入， + 让 tool-turn 校验器生成固定、模型可纠正的错误。不引入通用 interrupt，不结束 + 整个模型 turn，不准入后继 child。 +4. `settleSavedMarker` 必须从保存的 child 结果重建相同顺序前缀及停止条件，先判断 + 停止条件，再查找后继 child。这样在非法 child 结果已保存、plan marker 结果未 + 提交时崩溃，可以从该前缀结束 marker，不会永远等待故意没有创建的 child。 + 更早的 child 结果不确定时仍按既有规则阻塞。并行 Hook 继续接收原输入，在 native + 执行前校验最终 aggregate，不取消已准入的 sibling Hook，也不改变其排序。 +5. `PermissionRequest` 返回 `updatedInput` 时，在发布审批输入或 native acquisition + 前再次校验生效的 Shell 文本。错误的改写拒绝整个 native 工具批次。该 Hook 本身 + 可能已经获取 Runtime，保留原 owner，交给正常 settlement 处理。 +6. 在 `prepareRequests` 中再次检查，包含 `PreToolUse` 后的调用。如果 Hook 引入 + 不完整文本，不再询问审批、不预留 publication、不启动该批次的任何 native 工具， + 拒绝全部调用。acquisition、publisher registration 或第一次审批可能已经发生， + 不能宣称此前完全无活动，也不能强行清除 owner。 +7. 对 Hook 改写导致的 Unicode 拒绝，不向 post-tool callback 转发不完整的生效 + 参数；跳过该批次的 post-tool callback，保留已提交的 Hook 决定与结果，正常 + 持久化工具拒绝。不能为被拒调用伪造已执行的 tool intent。 +8. 只有校验通过的生效输入进入既有 reservation/start 路径。合法输入改写后重新 + 审批的规则保持不变。 + +复用已有批次拒绝的持久化逻辑，仅在这些入口需要时提取。assistant 提交前的拒绝 +写一次 assistant，并为每个调用写有序响应；assistant 提交后的拒绝只写响应，不能 +再追加一份 assistant。 + +不能依赖临时 flag 跳过 post-tool callback。在实时 `execute` 和冷恢复 +`resumeHookResults` 共用的 `completeHookResults` 入口，通过原始 occurrence/call +identity 重建原 assistant group 和该批次保存的 `PermissionRequest`/`PreToolUse` +结果，按准入的相同规则恢复生效输入，包含被 Unicode 检查终止的前缀。只有非法 +Shell 文本、完整配对的拒绝 group，以及该批次没有 native tool intent 共同证明 +Unicode 拒绝时，才返回保存的响应，不运行 callback，不重复提交 assistant/result。 +不能仅用错误字符串判断，也不能跳过所有没有 intent 的批次；普通参数/审批拒绝 +保持原 post-batch 行为。矛盾的 native intent 或不完整拒绝记录属于恢复错误，不能 +当作安全拒绝的证据。不需要新增持久 flag 或 schema 字段，下一合法批次从自己的 +记录重新判断。 + +## 5. 持久化、所有权与兼容性 + +错误响应仅包含固定原因、原 function-call identity 和既有响应形状；合法 sibling +收到既有的明确未执行错误。错误消息和诊断不回显非法字段值。通过既有 JSON 转义 +持久记录保存原 assistant function-call group，不改写原命令，不声称已经执行。 +必须使用真实 Java Store 证明该 group 原样持久化及 reload，不能仅使用内存 TS mock。 + +对于全新、无 Hook 的被拒批次，要求 acquire、publisher registration、publication +reservation、prepare、start/execute 和 worker 副作用均为零。Runtime warming 在 +模型输出前已经发生，允许它并单独记录。因此方案不承诺完全没有 Broker HTTP 流量， +也不停止独立 provisioning。 + +后续非法批次不能再次 acquire/reserve/start，但可能继承先前合法工作持有的租约。 +正常最终 settlement 释放 owner；校验器不释放 owner，也不将既有 execution 改为 +`not_started`。拒绝提交成功时是模型可纠正结果，而非 recovery-blocked；assistant +或 result 提交失败、结果不确定时保留既有 recovery blocking。Shell 未执行不能 +证明 journal 一致。 + +不完整命令先于它的审批被拒绝，不为该命令创建审批。不需要配置、迁移、公开 DTO、 +协议版本、新错误码枚举或 Shell capability 启用。后续源码实施须遵循 core/跨包 +改动的维护者审核规则；本设计不代表 PR 审批，也不授权 rollout。 + +## 6. 实施与验证计划 + +实施前由只读 `test-engineer` 使用独立的真实 Java Store/Broker/Harness/worker +环境，在当时 main 构建上复现。先尝试已安装的全局 `qwen`，private profile 不可达 +时记录 unavailable，不能记为基线通过。用确定性模型 fixture 分别覆盖 local capture +与 O2；输入准入测试不要求真实 OSS bucket。保存第 9 轮日志和新 exact-head +before/after 结果，将 E2E 计划与报告放入 `.qwen/e2e-tests/`。 + +主要实施/测试文件： + +- `packages/cli/src/serve/hosted-workspace-tool-turn.ts`。 +- `packages/cli/src/serve/hosted-shell-input.ts` 及 collocated tests。 +- `packages/cli/src/serve/hosted-hook-session.ts` 及测试，仅修改 Shell 生效输入边界 + 和 saved-marker 重建。 +- `packages/cli/src/serve/hosted-workspace-tool-turn.test.ts`，并按需要增加 Hook 测试， + 证明 callback 顺序与抑制。 +- 覆盖真实 Java 持久化、文件副作用和同 Workspace 第二个 Session 的 native Hosted + 集成 fixture。rebase 后选择确实能驱动两条捕获路径的既有 driver，不为测试该修复 + 增设第二个生产准入入口。 + +| 测试 | 必须提供的证据 | +| --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Unicode 反例 | 两个字段、两条路径：孤立高/低代理项、高代理项后 ASCII、逆序对、高高、低低、合法代理对后孤立代理项。覆盖任意位置，包含合法 emoji 后。 | +| Unicode 兼容性 | ASCII、中文、补充平面 emoji、合法代理对、组合字符、ESC、U+0001、DEL、U+2028/U+2029 和字面 `\ud800` 的 command/description 字节完全保留；命令语义保持。 | +| 批次拒绝 | 非法调用位于首/尾，与合法 Shell、Write 混合；所有错误有序持久化，每个 sibling 明确未执行，无 tool intent 或 publisher reservation。 | +| 纠正 | 同 turn 纠正的调用恰好执行一次；结果到达下一次模型请求；最终 settlement 成功，reload 和后续 turn 正常。 | +| 所有权 | 新拒绝不持有 Workspace 租约，第二个 Session 的合法调用完成；已有租约时的后续拒绝保留 owner，直到正常 settlement。 | +| 持久化失败 | 分别失败 assistant/result 写入，并丢失已提交操作的响应；无执行、无假成功、保留既有恢复阻塞、不重复历史。 | +| Hooks | 原始非法输入不进入 permission/pre/post/batch Hook；PermissionRequest 与 PreToolUse 的非法改写不能 native start 或进入后续携带非法参数的 callback。真实双 Hook 顺序 plan 覆盖 updatedInput 与 tool_input 合并：H1 引入非法文本，H2 从未准入，即使 H2 能修复也不运行。H1 result 后、marker result 前崩溃，恢复无需 H2 child 即可结束 marker；合法改写仍重新审批并到达 H2。 | +| Hook 冷恢复完成 | 将原始非法和 Hook 改写拒绝 group、Hook 记录 reload 到新 tool-turn，走真实 resumeHookResults 调用方，证明无 post-batch callback、无重复历史、无 native 操作。普通拒绝 replay 和下一合法批次行为不变。 | +| 存储和模型传输 | 真实 Java Store 持久化/reload 原始转义 assistant group 和配对错误；确定性模型收到它们并纠正调用。编码改变或传输拒绝该 group 时验收失败。 | +| 限额与 identity | 既有 inline-size、unsupported-tool、duplicate-call 和 incomplete-argument 守卫仍适用；超出现有限额时不承诺可纠正。 | +| 回归与变异 | 既有 local/O2 拒绝、审批、捕获和 Hook 测试通过；分别移除每个准入检查与 callback 抑制检查，对应反例必须失败；去掉 `u` 时合法 emoji 见证必须失败。 | + +在相应包目录运行： + +```bash +# Repository root +npm run build && npm run typecheck + +# packages/cli +npx vitest run src/serve/hosted-workspace-tool-turn.test.ts +npx vitest run src/serve/hosted-shell-input.test.ts src/serve/hosted-hook-session.test.ts +# Add the actually changed Hook/native driver suites, then the real-stack gate. +``` + +第 9 节记录已完成的检查。catalog-only 或 worker-only +结果不能证明 pre-acquisition admission；不要求运行无关完整测试集。 + +## 7. 验收与风险 + +只有上述全部验证在同一最终源码版本通过，两条捕获路径先于首次 native 副作用拒绝 +非法输入，合法文本字节不变，纠正后的调用和第二个 Session 正常，拒绝持久化失败仍 +安全阻塞，才算修复完成。公开 Shell 启用和历史阻塞 Session 恢复仍属独立工作。 + +主要风险是校验相对 Hook 的位置错误、post-batch completion 再次转发非法输入、 +改写后重复提交 assistant，以及仅依赖 TS 持久化证据。inline 限额和此前仍在进行的 +副作用可能合理地阻止可纠正 continuation，必须明确记录。 + +编写本设计不需要额外产品决策。源码交付前必须有当前 main 复现和 Java 历史原样 +往返证据；任一失败都应修改实施/设计，不能静默替换非法命令或宣称 issue 已修复。 + +## 8. 独立审计记录 + +用户要求每轮由全新 Sub Agent 进行审计和反向审计，轮间更新，最多七轮。 +每轮同时检查方案能否关闭 issue，以及新增拒绝/顺序是否破坏合法行为，或使验证 +声明无法证明。 + +| 轮次 | 问题与处理 | 结果 | +| ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------ | +| 1 | 新审计者 audit_round_1 发现外层检查前的顺序 Hook 传播（P1）和冷恢复丢失 callback 抑制（P2）。已补 Hosted 共用校验器、顺序前缀终态重建、持久 completion 判定，以及对应反例/兼容性见证。 | 两项均接受并修订；第 2 轮已独立复核。 | +| 2 | 新审计者 audit_round_2 独立核对 issue/源码，审计正向行为与反向兼容性/恢复反例。未发现新实质问题，第 1 轮修正已覆盖。报告保留事件返回形状和嵌套 PermissionRequest 的实施约束，不扩大合法 Hook 语义。 | 满足用户“无新问题”停止条件，结束审计循环（最多 7 轮，实际 2 轮）。 | + +本地审计报告：`.qwen/pr-reviews/issue-13010-design-round-1.md` 和 +`.qwen/pr-reviews/issue-13010-design-round-2.md`（git-ignored 工作产物）。 + +## 9. 实施与本机验证 + +实施在 permission Hook 前和 request preparation 中使用共享纯校验器， +在 live 与 saved-marker 路径终止不完整的顺序 Hook 前缀，并在 callback 前 +重建持久化 Unicode 拒绝证据。共享既有有序拒绝路径；未增加持久化 schema、 +core 校验器、digest 算法或生产 Java 协议。 + +修复前 main 的打包产物在 local 路径将 U+D800 command 准入 +acquire/prepare/start，30 秒后仍有活跃 prompt。8 项受控源码 fallback +独立观察到 acquisition。早期 O2 fixture 缺少 publication endpoint， +其 recovery block 不能单独证明 Unicode 导致锁死。这些观察未复现历史 +八分钟事故。 + +最终真实进程 Maven verify gate 的 local 与 O2 两个参数均通过。每条路径 +包括 8 个不完整 command/description 批次，以及同 turn 拒绝后纠正的另一个 +批次。模型在 detach/load 前后检查原始参数和有序配对错误。Broker 与 Store +proxy 观察新鲜非法批次没有 native admission/publication 请求；主 Session +的 SQL 执行账本仅有一次纠正后的执行。同 Workspace 的第二个 Session +在两条路径均完成合法调用。 + +此栈使用临时 H2、Embedded Broker 和真实 Node worker 进程。O2 使用真实 +Java SQL grant/data 服务和 publication controller，object store 为内存 +测试适配器；这不是 MySQL、OSS 或 Linux 隔离验收。受控 CLI 回归另行覆盖 +/1 与 /2、两条 capture 路径、非法首尾与合法 Shell/Write siblings、持久化 +失败和提交后丢回复、已有 owner 保留、真实 Hook 改写和 cold completion。 +7 项 mutation（原始准入、permission 改写 preparation、PreToolUse 改写、 +live/cold 顺序停止、callback 抑制、Unicode 标志）分别使对应 witness 失败。 +最终回归前，源码已逐字恢复。 + +精确命令、环境、成功条件、结果与排查见 +[本地 E2E 文档](../../../HOSTED_SHELL_UNICODE_E2E.md)。本地实施代码审查 +没有 Critical，有两项 E2E Suggestion，均接受并修复。 diff --git a/integration-tests/helpers/hosted-relay-headers.test.ts b/integration-tests/helpers/hosted-relay-headers.test.ts index 2c5941ee8e7..868d12078ba 100644 --- a/integration-tests/helpers/hosted-relay-headers.test.ts +++ b/integration-tests/helpers/hosted-relay-headers.test.ts @@ -117,7 +117,7 @@ describe('Hosted proxy header relay', () => { } }); - it('keeps every store relay driver calling the shared helper', () => { + it('keeps the Hosted relays calling the shared helper', () => { // The sdk-java workflow step runs this file after `cd integration-tests` // while the repo-root lanes use `--root ./integration-tests`, so resolve // the drivers relative to this file, never process.cwd(). Read source @@ -140,6 +140,7 @@ describe('Hosted proxy header relay', () => { 'hosted-process-crash-driver.ts', 'hosted-recovery-blocked-wedge-driver.ts', 'hosted-shell-output-driver.ts', + 'hosted-shell-unicode-driver.ts', 'hosted-store-failure-driver.ts', 'hosted-workflow-fault-driver.ts', 'hosted-workspace-tool-turn-driver.ts', diff --git a/integration-tests/helpers/hosted-relay-headers.ts b/integration-tests/helpers/hosted-relay-headers.ts index 9f80ea0c20a..484de010ab0 100644 --- a/integration-tests/helpers/hosted-relay-headers.ts +++ b/integration-tests/helpers/hosted-relay-headers.ts @@ -42,7 +42,7 @@ export function relayedHeaders(headers: Headers): Record { } // The wholesale upstream relays — hosted-{latency,process-crash, -// shell-output,store-failure,workspace-tool-turn,channel-fault}-driver.ts — +// shell-output,shell-unicode,store-failure,workspace-tool-turn,channel-fault}-driver.ts — // pass through this one site, pinned by the caller list in // hosted-relay-headers.test.ts. The drivers' other upstream replies write // fixed header literals and must stay that way: a driver-local copy of the diff --git a/integration-tests/helpers/hosted-shell-unicode-driver.ts b/integration-tests/helpers/hosted-shell-unicode-driver.ts new file mode 100644 index 00000000000..1b4a69fa08f --- /dev/null +++ b/integration-tests/helpers/hosted-shell-unicode-driver.ts @@ -0,0 +1,345 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'node:assert/strict'; +import { createHash, randomUUID } from 'node:crypto'; +import { access, readFile } from 'node:fs/promises'; +import { createServer } from 'node:http'; +import path from 'node:path'; +import { + fakeToolCall, + startFakeOpenAIServer, + type FakeOpenAIToolCall, +} from '../fake-openai-server.js'; +import { HostedHarnessProcess, waitUntil } from './hosted-harness-process.js'; +import { relayUpstream } from './hosted-relay-headers.js'; + +const config = JSON.parse(await readFile(process.argv[2], 'utf8')) as { + tenantId: string; + storeUrl: string; + brokerUrl: string; + secondarySessionId: string; + sessions: Array<{ + sessionId: string; + workspaceId: string; + directory: string; + toolProfile: string; + fault: string; + }>; +}; +const controls: string[] = []; +const publications: string[] = []; +const proxy = createServer(async (req, res) => { + try { + const chunks: Buffer[] = []; + for await (const chunk of req) chunks.push(Buffer.from(chunk)); + const payload = Buffer.concat(chunks); + const brokerRequest = req.url!.startsWith('/internal/runtime-broker/'); + if (brokerRequest) controls.push(req.url!); + if (req.url!.startsWith('/internal/managed-tool-publications/')) + publications.push(req.url!); + const headers: Record = {}; + for (const [name, value] of Object.entries(req.headers)) { + if ( + value !== undefined && + !['host', 'connection', 'content-length', 'transfer-encoding'].includes( + name, + ) + ) { + headers[name] = Array.isArray(value) ? value.join(',') : value; + } + } + const response = await fetch( + new URL(req.url!, brokerRequest ? config.brokerUrl : config.storeUrl), + { + method: req.method, + headers, + ...(payload.length ? { body: payload } : {}), + signal: AbortSignal.timeout(30_000), + }, + ); + relayUpstream(res, response, Buffer.from(await response.arrayBuffer())); + } catch (error) { + res.writeHead(503); + res.end(String(error)); + } +}); +await new Promise((resolve) => proxy.listen(0, '127.0.0.1', resolve)); +const address = proxy.address(); +assert(address && typeof address !== 'string'); + +const validText = '中文😀é\\ud800'; +let scenario: { field: string; value: string } | undefined; +let corrected = false; +let repairInTurn = false; +type Message = { + role: string; + content?: unknown; + tool_call_id?: string; + tool_calls?: FakeOpenAIToolCall[]; +}; +const refused = new Map< + string, + { call: FakeOpenAIToolCall; result?: Message } +>(); +let correctedCallId = ''; +const correction = () => { + const file = + sessionId === config.secondarySessionId + ? 'unicode-secondary-ok.txt' + : 'unicode-ok.txt'; + const call = fakeToolCall('run_shell_command', { + command: "printf '%s' '中文😀é\\ud800' > " + file, + description: validText, + }); + correctedCallId = call.id; + return { toolCalls: [call] }; +}; +const model = await startFakeOpenAIServer(({ body }) => { + const messages = (body.messages ?? []) as Message[]; + const start = messages.findLastIndex((entry) => entry.role === 'user'); + const results = messages + .slice(start + 1) + .filter((entry) => entry.role === 'tool'); + if (results.length) { + if (scenario) { + assert.equal( + results.length, + repairInTurn && results.length === 3 ? 3 : 2, + ); + for (const result of results.slice(0, 2)) { + const snapshot = refused.get(result.tool_call_id!); + assert(snapshot, 'Unexpected tool response'); + const call = messages + .flatMap((entry) => entry.tool_calls ?? []) + .find((entry) => entry.id === snapshot.call.id); + assert.deepEqual(call, snapshot.call); + snapshot.result = result; + } + assert.equal(results[0].tool_call_id, [...refused.keys()].at(-2)); + assert.equal(results[1].tool_call_id, [...refused.keys()].at(-1)); + assert.match( + JSON.stringify(results[0].content), + /unpaired UTF-16 surrogate/, + ); + assert.match(JSON.stringify(results[1].content), /not executed/); + if (repairInTurn && results.length === 2) return correction(); + } + if (corrected || repairInTurn) { + assert.equal(results.at(-1)!.tool_call_id, correctedCallId); + assert.doesNotMatch(JSON.stringify(results.at(-1)!.content), /"error"/); + } + return { content: 'UNICODE_DONE' }; + } + if (scenario) { + const calls = [ + fakeToolCall('run_shell_command', { + command: 'printf invalid > invalid.txt', + [scenario.field]: scenario.value, + }), + fakeToolCall('write_file', { + file_path: 'sibling.txt', + content: 'must not execute', + }), + ]; + for (const call of calls) refused.set(call.id, { call }); + return { toolCalls: calls }; + } + if (corrected) return correction(); + for (const snapshot of refused.values()) { + assert(snapshot.result, 'Missing paired refusal'); + const call = messages + .flatMap((entry) => entry.tool_calls ?? []) + .find((entry) => entry.id === snapshot.call.id); + assert.deepEqual( + call, + snapshot.call, + 'Original arguments changed after reload', + ); + const result = messages.find( + (entry) => entry.tool_call_id === snapshot.call.id, + ); + assert.deepEqual( + result, + snapshot.result, + 'Paired refusal changed after reload', + ); + } + return { content: 'HISTORY_RELOADED' }; +}); +const cli = new HostedHarnessProcess(); +let sessionId = ''; +let clientId = ''; +async function json(route: string, body?: unknown, expected = 200) { + const response = await cli.request(route, { + method: body === undefined ? 'GET' : 'POST', + headers: { ...cli.headers(clientId), 'Content-Type': 'application/json' }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); + const text = await response.text(); + assert.equal(response.status, expected, text); + return text ? JSON.parse(text) : undefined; +} +async function prompt() { + const promptId = randomUUID(); + const prompt = [{ type: 'text', text: 'UNICODE_' + promptId }]; + await json( + '/session/' + sessionId + '/prompt', + { + promptId, + prompt, + payloadDigest: + 'sha256:' + + createHash('sha256').update(JSON.stringify(prompt)).digest('hex'), + }, + 202, + ); + let status; + try { + await waitUntil(async () => { + status = await json('/session/' + sessionId + '/status'); + assert.equal( + status.recoveryBlocked, + false, + JSON.stringify({ status, controls, output: cli.output }), + ); + return !status.hasActivePrompt; + }, 30_000); + } catch (error) { + console.error( + JSON.stringify({ + scenario, + status, + controls, + modelRequests: model.requests.length, + output: cli.output, + }), + ); + throw error; + } + const transcript = await json( + '/session/' + sessionId + '/transcript?cursor=0&limit=256', + ); + assert( + transcript.events.some( + (event: { type: string; promptId?: string }) => + event.type === 'turn_complete' && event.promptId === promptId, + ), + JSON.stringify(transcript) + cli.output, + ); +} +try { + const proxyUrl = 'http://127.0.0.1:' + address.port; + await cli.start(model.baseUrl, { + extraArgs: [ + '--managed-runtime-broker-url', + proxyUrl, + '--managed-runtime-broker-token', + 'hosted-tools-broker-token', + ], + }); + for (const session of config.sessions) { + refused.clear(); + sessionId = session.sessionId; + clientId = ''; + const connection = { + baseUrl: proxyUrl, + tenantId: config.tenantId, + workspaceId: session.workspaceId, + writerId: cli.bootId, + leaseDurationMs: 60_000, + }; + const profile = { + managedSessionStore: connection, + toolProfile: session.toolProfile, + ...(session.fault === 'o2' ? { captureBytes: 1024 * 1024 } : {}), + }; + clientId = ( + await json('/session', { sessionId, sessionScope: 'thread', ...profile }) + ).clientId; + for (const field of ['command', 'description']) { + for (const value of ['\ud800', '\udc00', '😀\ud800', '\udc00\ud800']) { + scenario = { field, value }; + const before = controls.length; + const publicationBefore = publications.length; + await prompt(); + const newControls = controls.slice(before); + assert( + newControls.every((route) => route.endsWith('/runtimes:warm')), + JSON.stringify(newControls), + ); + assert.equal( + publications.length, + publicationBefore, + JSON.stringify(publications), + ); + await assert.rejects( + access(path.join(session.directory, 'invalid.txt')), + ); + await assert.rejects( + access(path.join(session.directory, 'sibling.txt')), + ); + } + } + scenario = undefined; + await json('/session/' + sessionId + '/detach', {}, 204); + clientId = (await json('/session/' + sessionId + '/load', profile)) + .clientId; + await prompt(); + scenario = { field: 'command', value: '\ud800' }; + repairInTurn = true; + await prompt(); + scenario = undefined; + repairInTurn = false; + assert.equal( + await readFile(path.join(session.directory, 'unicode-ok.txt'), 'utf8'), + validText, + ); + await json('/session/' + sessionId + '/detach', {}, 204); + clientId = (await json('/session/' + sessionId + '/load', profile)) + .clientId; + await prompt(); + await json('/session/' + sessionId + '/detach', {}, 204); + refused.clear(); + { + sessionId = config.secondarySessionId; + clientId = ''; + clientId = ( + await json('/session', { + sessionId, + sessionScope: 'thread', + ...profile, + }) + ).clientId; + await assert.rejects( + access(path.join(session.directory, 'unicode-secondary-ok.txt')), + ); + corrected = true; + await prompt(); + corrected = false; + assert.equal( + await readFile( + path.join(session.directory, 'unicode-secondary-ok.txt'), + 'utf8', + ), + validText, + ); + await json('/session/' + sessionId + '/detach', {}, 204); + } + } + console.log( + 'HOSTED_SHELL_UNICODE_OK: ' + + config.sessions.map((session) => session.fault).join(',') + + ', 9 refusals per capture, durable reload, Unicode correction, second Session', + ); +} finally { + await cli.close(); + await model.close(); + proxy.closeAllConnections(); + await new Promise((resolve, reject) => + proxy.close((error) => (error ? reject(error) : resolve())), + ); +} diff --git a/packages/cli/src/serve/hosted-hook-session.test.ts b/packages/cli/src/serve/hosted-hook-session.test.ts index 221cecb315d..a6c8399e1bf 100644 --- a/packages/cli/src/serve/hosted-hook-session.test.ts +++ b/packages/cli/src/serve/hosted-hook-session.test.ts @@ -1342,13 +1342,16 @@ it('applies failClosed when a lost reply is settled through status after reconst }); }); +function heldHookExecution() { + return new Promise<(result: ManagedHookResult) => void>((started) => { + execute = () => new Promise((finish) => started(finish)); + }); +} + it('waits for Runtime admission before acknowledging an async Hook', async () => { catalog = { ...catalog, hooks: [{ ...catalog.hooks[0], async: true }] }; + const admitted = heldHookExecution(); let finish: ((result: ManagedHookResult) => void) | undefined; - execute = () => - new Promise((resolve) => { - finish = resolve; - }); let completed = false; const firing = hooks .fire(HookEventName.PreToolUse, 'call-1', {}, signal()) @@ -1356,13 +1359,21 @@ it('waits for Runtime admission before acknowledging an async Hook', async () => completed = true; return result; }); - await vi.waitFor(() => expect(finish).toBeDefined(), { - timeout: isSlowTestHost() ? 30_000 : 1_000, - }); - expect(completed).toBe(false); - finish!({ success: true, outcome: 'success', duration: 0 }); - await firing; - expect(completed).toBe(true); + try { + finish = await Promise.race([ + admitted, + firing.then(() => { + throw new Error('Hook completed before Runtime admission.'); + }), + ]); + expect(completed).toBe(false); + finish!({ success: true, outcome: 'success', duration: 0 }); + await firing; + expect(completed).toBe(true); + } finally { + finish?.({ success: false, outcome: 'cancelled', duration: 0 }); + await Promise.allSettled([firing]); + } }); it('rejects async prompt Hooks instead of leaving a detached model request', async () => { @@ -1383,32 +1394,37 @@ it('rejects async prompt Hooks instead of leaving a detached model request', asy }); it('rejects a changed input while the same occurrence is still running', async () => { + const admitted = heldHookExecution(); let finish: ((result: ManagedHookResult) => void) | undefined; - execute = () => - new Promise((resolve) => { - finish = resolve; - }); const firing = hooks.fire( HookEventName.PreToolUse, 'call-1', { tool_name: 'read_file' }, signal(), ); - await vi.waitFor(() => expect(finish).toBeDefined(), { - timeout: isSlowTestHost() ? 30_000 : 1_000, - }); - await expect( - hooks.fire( - HookEventName.PreToolUse, - 'call-1', - { tool_name: 'write_file' }, - signal(), - ), - ).rejects.toThrow(HostedHookInputConflictError); - expect(hooks.hasPendingOperations).toBe(true); - finish!({ success: true, outcome: 'success', duration: 0 }); - await firing; - expect(hooks.hasPendingOperations).toBe(false); + try { + finish = await Promise.race([ + admitted, + firing.then(() => { + throw new Error('Hook completed before Runtime admission.'); + }), + ]); + await expect( + hooks.fire( + HookEventName.PreToolUse, + 'call-1', + { tool_name: 'write_file' }, + signal(), + ), + ).rejects.toThrow(HostedHookInputConflictError); + expect(hooks.hasPendingOperations).toBe(true); + finish!({ success: true, outcome: 'success', duration: 0 }); + await firing; + expect(hooks.hasPendingOperations).toBe(false); + } finally { + finish?.({ success: false, outcome: 'cancelled', duration: 0 }); + await Promise.allSettled([firing]); + } }); it('rejects another tool input order while the same occurrence is still running', async () => { @@ -1448,24 +1464,29 @@ it('cancels an active occurrence and does not dispatch later sequential hooks', { ...catalog.hooks[0], sequential: true, hookId: 'second' }, ], }; + const admitted = heldHookExecution(); let finish: ((result: ManagedHookResult) => void) | undefined; - execute = () => - new Promise((resolve) => { - finish = resolve; - }); const firing = hooks.fire(HookEventName.PreToolUse, 'call-1', {}, signal()); - await vi.waitFor(() => expect(finish).toBeDefined(), { - timeout: isSlowTestHost() ? 30_000 : 1_000, - }); - const cancelled = await hooks.status('call-1', true); - expect(cancelled.cancelRequested).toBe(true); - finish!({ success: false, outcome: 'cancelled', duration: 0 }); - expect((await firing)?.decision).toBe('block'); - expect( - requests.filter((request) => request.kind === 'hook-execute'), - ).toHaveLength(1); - expect((await hooks.status('call-1')).run.state).toBe('cancelled'); - expect(hooks.hasPendingOperations).toBe(false); + try { + finish = await Promise.race([ + admitted, + firing.then(() => { + throw new Error('Hook completed before Runtime admission.'); + }), + ]); + const cancelled = await hooks.status('call-1', true); + expect(cancelled.cancelRequested).toBe(true); + finish!({ success: false, outcome: 'cancelled', duration: 0 }); + expect((await firing)?.decision).toBe('block'); + expect( + requests.filter((request) => request.kind === 'hook-execute'), + ).toHaveLength(1); + expect((await hooks.status('call-1')).run.state).toBe('cancelled'); + expect(hooks.hasPendingOperations).toBe(false); + } finally { + finish?.({ success: false, outcome: 'cancelled', duration: 0 }); + await Promise.allSettled([firing]); + } }); it('refuses an ambiguous external occurrence id', async () => { @@ -3398,6 +3419,113 @@ function loseFinalMarkerWrite() { }); } +it.each([ + ...[false, true].flatMap((merge) => + [false, true].map((loseMarker) => ({ + merge, + loseMarker, + event: HookEventName.PreToolUse, + padding: 0, + })), + ), + ...[false, true].flatMap((loseMarker) => + [HookEventName.PreToolUse, HookEventName.PermissionRequest].map( + (event) => ({ + merge: event === HookEventName.PreToolUse, + loseMarker, + event, + padding: 40 * 1024, + }), + ), + ), + ...[false, true].map((loseMarker) => ({ + merge: false, + loseMarker, + event: HookEventName.PermissionRequest, + padding: 0, + })), +])( + 'stops malformed sequential Shell input and reconstructs its terminal prefix ($event, $merge, $loseMarker, $padding)', + async ({ merge, loseMarker, event, padding }) => { + enforceHostedResourceLimit(); + const malformed = 'x'.repeat(padding) + 'echo 😀\ud800'; + catalog = { + ...catalog, + hooks: ['first', 'repair'].map((hookId) => ({ + ...catalog.hooks[0], + hookId, + eventName: event, + sequential: true, + onceKey: hookId === 'repair' ? 'repair-once' : null, + })), + }; + execute = async (operation) => ({ + success: true, + outcome: 'success', + duration: 0, + output: { + hookSpecificOutput: + event === HookEventName.PermissionRequest + ? { + decision: { + behavior: 'allow', + updatedInput: { + command: + operation.hookId === 'first' + ? malformed + : 'echo repaired', + }, + }, + } + : { + [merge ? 'tool_input' : 'updatedInput']: { + command: + operation.hookId === 'first' ? malformed : 'echo repaired', + }, + }, + }, + }); + const input = { + tool_name: 'run_shell_command', + tool_input: { command: 'echo valid' }, + }; + const expected = { + hookSpecificOutput: + event === HookEventName.PermissionRequest + ? { decision: { updatedInput: { command: malformed } } } + : { updatedInput: { command: malformed } }, + }; + const publish = vi.spyOn(session.resources, 'publish'); + const lost = loseMarker ? loseFinalMarkerWrite() : undefined; + const firing = hooks.fire(event, 'unicode', input, signal()); + if (lost) { + await expect(firing).rejects.toThrow('lost final marker write'); + lost.mockRestore(); + } else await expect(firing).resolves.toMatchObject(expected); + const restored = new HostedHookSession(options, session, pin); + expect((await restored.status('unicode')).run.state).toBe('settled'); + expect( + await restored.fire(event, 'unicode', input, signal()), + ).toMatchObject(expected); + expect(restored.hasPendingOperations).toBe(false); + for (const [kind, bytes] of publish.mock.calls) + if (kind === 'managed-hook-result') + expect(bytes.length).toBeLessThanOrEqual(60 * 1024); + expect( + requests + .filter((request) => request.kind === 'hook-execute') + .map((request) => request.hookId), + ).toEqual(['first']); + expect( + session.authority + .extensionRecordsInDomain('hook_execution') + .some( + (entry) => parseHookExecution(entry.record).onceKey === 'repair-once', + ), + ).toBe(false); + }, +); + it.each([ [HookEventName.PreToolUse, false], [HookEventName.PreToolUse, true], @@ -3595,6 +3723,74 @@ it.each([false, true])( }, ); +it.each([false, true])( + 'bounds assembled Unicode refusal metadata (lost marker: %s)', + async (loseMarker) => { + enforceHostedResourceLimit(); + catalog = { + ...catalog, + hooks: ['first', 'repair'].map((hookId) => ({ + ...catalog.hooks[0], + hookId, + sequential: true, + })), + }; + execute = async () => ({ + success: true, + outcome: 'success', + duration: 0, + output: { + hookSpecificOutput: { + tool_input: { description: '\ud800' }, + padding: 'x'.repeat(40 * 1024), + }, + }, + }); + const input = { + tool_name: 'run_shell_command', + tool_input: { command: 'x'.repeat(40 * 1024) }, + }; + const lost = loseMarker ? loseFinalMarkerWrite() : undefined; + const firing = hooks.fire( + HookEventName.PreToolUse, + 'assembled-unicode', + input, + signal(), + ); + if (lost) { + await expect(firing).rejects.toThrow('lost final marker write'); + lost.mockRestore(); + } else + await expect(firing).resolves.toMatchObject({ + continue: false, + decision: 'block', + }); + const restored = new HostedHookSession(options, session, pin); + expect((await restored.status('assembled-unicode')).run.state).toBe( + 'settled', + ); + expect( + await restored.fire( + HookEventName.PreToolUse, + 'assembled-unicode', + input, + signal(), + ), + ).toMatchObject({ + decision: 'block', + reason: expect.stringContaining('60 KiB'), + stopReason: + 'Hosted Shell description contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.', + }); + expect(restored.hasPendingOperations).toBe(false); + expect( + requests + .filter((request) => request.kind === 'hook-execute') + .map((request) => request.hookId), + ).toEqual(['first']); + }, +); + it.each(['none', 'hook', 'borrowed'] as const)( 'releases only acquired Hook owners after actual HTTP warm (%s)', async (mode) => { diff --git a/packages/cli/src/serve/hosted-hook-session.ts b/packages/cli/src/serve/hosted-hook-session.ts index 7321ef2d17d..70705265016 100644 --- a/packages/cli/src/serve/hosted-hook-session.ts +++ b/packages/cli/src/serve/hosted-hook-session.ts @@ -58,6 +58,7 @@ import { } from './hosted-workspace-broker.js'; import type { HostedWorkspaceBrokerOptions } from './hosted-workspace-broker.js'; import { waitForTurn } from './hosted-turn-wait.js'; +import { hostedShellInputError } from './hosted-shell-input.js'; export class HostedHookRecoveryRequiredError extends Error { constructor() { @@ -124,13 +125,18 @@ function sequentialInput( if (!result.success || !result.output) return input; const effective = applyHookOutputToInput(input, result.output, event); const output = result.output.hookSpecificOutput; + const decision = + event === HookEventName.PermissionRequest + ? (output?.['decision'] as { updatedInput?: unknown } | undefined) + : undefined; + const updatedInput = decision?.updatedInput ?? output?.['updatedInput']; return { ...effective, // PreToolUse replacements are validated by the shared helper above. ...(event !== HookEventName.PreToolUse && - output?.['updatedInput'] && - typeof output['updatedInput'] === 'object' - ? { tool_input: output['updatedInput'] } + updatedInput && + typeof updatedInput === 'object' + ? { tool_input: updatedInput } : {}), ...(typeof output?.['updatedPrompt'] === 'string' ? { prompt: output['updatedPrompt'] } @@ -138,6 +144,57 @@ function sequentialInput( }; } +function malformedShellInput(input: HookInput, event: HookEventName): boolean { + return ( + (event === HookEventName.PreToolUse || + event === HookEventName.PermissionRequest) && + 'tool_name' in input && + 'tool_input' in input && + hostedShellInputError(input['tool_name'], input['tool_input']) !== undefined + ); +} + +function shellRefusalOutput( + output: HookOutput | undefined, + input: HookInput, + event: HookEventName, +): HookOutput | undefined { + if ( + !('tool_name' in input) || + !('tool_input' in input) || + !malformedShellInput(input, event) + ) + return output; + const specific = { ...output?.hookSpecificOutput }; + delete specific['tool_input']; + delete specific['updatedInput']; + const refusal = { + ...output, + hookSpecificOutput: { + ...specific, + ...(event === HookEventName.PermissionRequest + ? { + decision: { + ...(specific?.['decision'] as + | Record + | undefined), + updatedInput: input['tool_input'], + }, + } + : { updatedInput: input['tool_input'] }), + }, + }; + return exceedsHookResourceLimit({ output: refusal }) + ? { + ...byteLimitOutput(event), + stopReason: hostedShellInputError( + input['tool_name'], + input['tool_input'], + ), + } + : refusal; +} + function semanticInput(value: object): Record { const fields = { ...value } as Record; for (const key of [ @@ -1152,6 +1209,7 @@ export class HostedHookSession { const result = await execute(hook, index); results.push(result); effectiveInput = sequentialInput(effectiveInput, result, event); + if (malformedShellInput(effectiveInput, event)) break; } } else { const settled = await Promise.allSettled(plan.hooks.map(execute)); @@ -1166,7 +1224,11 @@ export class HostedHookSession { ? cancelledResult().output : inputLimitExceeded ? byteLimitOutput(event) - : this.aggregateOutput(results, event); + : shellRefusalOutput( + this.aggregateOutput(results, event), + effectiveInput, + event, + ); const resultRef = await this.publish('managed-hook-result', { ...(output ? { output: createHookOutput(event, output) } : {}), }); @@ -1818,8 +1880,10 @@ export class HostedHookSession { eventName: record.eventName as HookEventName, ...(result.error ? { error: new Error(result.error) } : {}), } as HookExecutionResult); - if (plan.sequential) + if (plan.sequential) { effectiveInput = sequentialInput(effectiveInput, result, event); + if (malformedShellInput(effectiveInput, event)) break; + } } if (record.run.execution === 'intent') { await this.commit('hook_execution', record.hookExecutionId, (latest) => @@ -1837,7 +1901,11 @@ export class HostedHookSession { } const output = inputLimitExceeded ? byteLimitOutput(event) - : this.aggregateOutput(results, event); + : shellRefusalOutput( + this.aggregateOutput(results, event), + effectiveInput, + event, + ); const resultRef = await this.publish('managed-hook-result', { ...(output ? { output } : {}), }); diff --git a/packages/cli/src/serve/hosted-shell-input.test.ts b/packages/cli/src/serve/hosted-shell-input.test.ts new file mode 100644 index 00000000000..6bab24a2de4 --- /dev/null +++ b/packages/cli/src/serve/hosted-shell-input.test.ts @@ -0,0 +1,86 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { expect, it } from 'vitest'; +import { managedToolDigest } from '@qwen-code/qwen-code-core/tools/managed-tool-protocol.js'; +import { + hostedShellHookInputError, + hostedShellInputError, +} from './hosted-shell-input.js'; + +it.each(['command', 'description'])( + 'rejects unpaired code units in %s without echoing input', + (field) => { + for (const value of [ + '\ud800', + '\udc00', + '\ud800a', + '\udc00\ud800', + '\ud800\ud800', + '\udc00\udc00', + '😀\ud800', + ]) { + const error = hostedShellInputError('run_shell_command', { + command: 'pwd', + [field]: value, + }); + expect(error).toBe( + 'Hosted Shell ' + + field + + ' contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.', + ); + } + }, +); + +it.each([ + 'ASCII', + '中文', + '😀', + 'é', + '\\ud800', + '\x1b\x01\x7f\u2028\u2029', + '', +])('preserves valid text and canonical digest (%j)', (value) => { + const input = { command: 'printf ' + value, description: value }; + const encoded = JSON.stringify(input); + const digest = managedToolDigest(input); + expect(hostedShellInputError('run_shell_command', input)).toBeUndefined(); + expect(JSON.stringify(input)).toBe(encoded); + expect(managedToolDigest(input)).toBe(digest); +}); + +it('leaves other tools and argument shapes to their existing validators', () => { + expect( + hostedShellInputError('monitor', { command: '\ud800' }), + ).toBeUndefined(); + for (const input of [ + undefined, + null, + [], + { command: 7 }, + { command: 'pwd', description: 7 }, + { command: 'pwd' }, + ]) + expect(hostedShellInputError('run_shell_command', input)).toBeUndefined(); +}); + +it('recognizes a bounded Unicode refusal without accepting unrelated stop reasons', () => { + const error = hostedShellInputError('run_shell_command', { + description: '\ud800', + })!; + const output = { continue: false, stopReason: error }; + expect(hostedShellHookInputError('run_shell_command', output)).toBe(error); + expect(hostedShellHookInputError('read_file', output)).toBeUndefined(); + expect( + hostedShellHookInputError('run_shell_command', { + stopReason: 'policy stop', + }), + ).toBeUndefined(); + expect( + hostedShellHookInputError('run_shell_command', undefined), + ).toBeUndefined(); +}); diff --git a/packages/cli/src/serve/hosted-shell-input.ts b/packages/cli/src/serve/hosted-shell-input.ts new file mode 100644 index 00000000000..5b07f09043e --- /dev/null +++ b/packages/cli/src/serve/hosted-shell-input.ts @@ -0,0 +1,42 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { HookOutput } from '@qwen-code/qwen-code-core/hooks/types.js'; + +function unicodeError(field: string): string { + return ( + 'Hosted Shell ' + + field + + ' contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.' + ); +} + +export function hostedShellHookInputError( + toolName: unknown, + output: HookOutput | undefined, +): string | undefined { + if (toolName !== 'run_shell_command') return undefined; + return ['command', 'description'].some( + (field) => output?.stopReason === unicodeError(field), + ) + ? output?.stopReason + : undefined; +} + +export function hostedShellInputError( + toolName: unknown, + input: unknown, +): string | undefined { + if (toolName !== 'run_shell_command' || !input || typeof input !== 'object') + return undefined; + const args = input as Record; + for (const field of ['command', 'description']) { + const value = args[field]; + if (typeof value === 'string' && /[\uD800-\uDFFF]/u.test(value)) + return unicodeError(field); + } + return undefined; +} diff --git a/packages/cli/src/serve/hosted-workspace-tool-turn.test.ts b/packages/cli/src/serve/hosted-workspace-tool-turn.test.ts index 2bd7edab89d..c3bbc6c851b 100644 --- a/packages/cli/src/serve/hosted-workspace-tool-turn.test.ts +++ b/packages/cli/src/serve/hosted-workspace-tool-turn.test.ts @@ -4907,6 +4907,1159 @@ function hookSession(fire: HostedHookSession['fire']): HostedHookSession { } as unknown as HostedHookSession; } +it('executes corrected Unicode after a refused batch with exact command bytes', async () => { + turn = createTurn(true); + const invalid = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + const run = (call: ToolCallRequestInfo) => + turn.execute( + [call], + [{ functionCall: { id: call.callId, name: call.name, args: call.args } }], + 'model', + new AbortController().signal, + ); + await run(invalid); + await turn.consumeResults(); + broker.execute.mockResolvedValue({ + executionStatus: 'not_started', + responseParts: [], + error: { message: 'fixture stop' }, + capture: null, + }); + const args = { + command: ' printf 中文😀é\\ud800\x1b\x01\x7f\u2028\u2029 ', + description: '中文😀', + }; + await run({ ...invalid, callId: 'corrected-unicode', args }); + const input = JSON.parse(broker.execute.mock.calls[0][1]).input; + expect(input).toEqual({ ...args, is_background: false }); + expect(broker.execute).toHaveBeenCalledOnce(); + await turn.consumeResults(); + await turn.finish(); + expect(broker.release).toHaveBeenCalledOnce(); +}); + +it('refuses an oversized assistant record on the early Unicode refusal path', async () => { + turn = createTurn(true); + const call = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + messageFitsInline.mockImplementation((type) => type !== 'assistant'); + await expect( + turn.execute( + [call], + [{ functionCall: { id: call.callId, name: call.name, args: call.args } }], + 'model', + new AbortController().signal, + ), + ).rejects.toThrow( + 'Hosted assistant record exceeds the inline Session Store limit.', + ); + expect(await session.sink.project()).toEqual([]); +}); + +it('refuses an oversized tool refusal on the early Unicode refusal path', async () => { + turn = createTurn(true); + const call = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + messageFitsInline.mockImplementation((type) => type !== 'tool_result'); + await expect( + turn.execute( + [call], + [{ functionCall: { id: call.callId, name: call.name, args: call.args } }], + 'model', + new AbortController().signal, + ), + ).rejects.toThrow( + 'Hosted tool refusal exceeds the inline Session Store limit.', + ); + expect(await session.sink.project()).toEqual([]); +}); + +it.each([false, true])( + 'preserves an acquired owner for a later Unicode refusal (commit failure: %s)', + async (failCommit) => { + const original = commit; + let resultCommits = 0; + commit = async (...args) => { + if (args[0] === 'tool_result' && ++resultCommits === 2 && failCommit) + throw new Error('refusal commit failed'); + return original(...args); + }; + turn = createTurn(true); + await turn.execute( + [calls[0]], + [parts[0]], + 'model', + new AbortController().signal, + ); + await turn.consumeResults(); + const call = { + ...calls[0], + callId: 'later-invalid', + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + const execution = turn.execute( + [call], + [{ functionCall: { id: call.callId, name: call.name, args: call.args } }], + 'model', + new AbortController().signal, + ); + if (failCommit) + await expect(execution).rejects.toBeInstanceOf( + HostedToolRecoveryRequiredError, + ); + else { + const response = await execution; + expect(response[0].functionResponse?.response?.['error']).toContain( + 'unpaired UTF-16 surrogate', + ); + await turn.consumeResults(); + } + expect(broker.acquire).toHaveBeenCalledOnce(); + expect(broker.execute).toHaveBeenCalledOnce(); + expect(broker.release).not.toHaveBeenCalled(); + if (failCommit) + await expect(turn.finish()).rejects.toBeInstanceOf( + HostedToolRecoveryRequiredError, + ); + else await turn.finish(); + expect(broker.release).toHaveBeenCalledTimes(failCommit ? 0 : 1); + }, +); + +it.each( + [false, true].flatMap((publication) => + [1, 2].flatMap((version) => + ['command', 'description'].flatMap((field) => + ['\ud800', '\udc00', '😀\ud800', '\udc00\ud800'].flatMap((value) => + [false, true].map((invalidLast) => ({ + publication, + version, + field, + value, + invalidLast, + })), + ), + ), + ), + ), +)( + 'durably refuses malformed Shell Unicode before Hooks and acquisition ($publication/$version/$field/$value/last:$invalidLast)', + async ({ publication, version, field, value, invalidLast }) => { + const owner = { + owner: vi.fn(), + request: vi.fn(), + rememberAdmission: vi.fn(), + } as unknown as HttpToolPublicationOwner; + const fire = vi.fn(); + const makeTurn = () => + new HostedWorkspaceToolTurn( + { baseUrl: 'http://127.0.0.1:1', token: 'test' }, + session, + harness, + 'prompt', + commit, + messageFitsInline, + publication + ? { owner, captureBytes: 1024 } + : { + resources: session.resources, + assertWritable: async () => undefined, + }, + undefined, + { settings: { mode: 'default', timeoutMs: 100 }, waiters }, + { + hooks: hookSession(fire), + profile: 'hosted-workspace-shell/' + version, + }, + ); + turn = makeTurn(); + const invalid = { + ...calls[0], + name: 'run_shell_command', + args: { command: 'printf valid', [field]: value }, + }; + const siblingShell = { + ...invalid, + callId: 'valid-shell', + args: { command: 'pwd' }, + }; + const siblingWrite = { + ...calls[1], + name: 'write_file', + args: { file_path: 'sibling.txt', content: 'unexecuted' }, + }; + const batch = invalidLast + ? [siblingShell, siblingWrite, invalid] + : [invalid, siblingShell, siblingWrite]; + const invalidIndex = invalidLast ? 2 : 0; + const input = batch.map((call) => ({ + functionCall: { id: call.callId, name: call.name, args: call.args }, + })); + const responses = await turn.execute( + batch, + input, + 'model', + new AbortController().signal, + ); + expect(responses.map((part) => part.functionResponse?.id)).toEqual( + batch.map((call) => call.callId), + ); + expect(responses[invalidIndex].functionResponse?.response?.['error']).toBe( + 'Hosted Shell ' + + field + + ' contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.', + ); + for (const [index, response] of responses.entries()) + if (index !== invalidIndex) + expect(response.functionResponse?.response?.['error']).toContain( + 'not executed', + ); + expect(fire).not.toHaveBeenCalled(); + expect(broker.acquire).not.toHaveBeenCalled(); + expect(broker.registerPublisher).not.toHaveBeenCalled(); + expect(broker.prepare).not.toHaveBeenCalled(); + expect(broker.prepareV3).not.toHaveBeenCalled(); + expect(broker.execute).not.toHaveBeenCalled(); + expect(owner.request).not.toHaveBeenCalled(); + const history = await session.sink.project(); + expect(history.map((entry) => entry.type)).toEqual([ + 'assistant', + 'tool_result', + ]); + expect(history[0].message?.parts).toEqual(input); + expect(history[1].message?.parts).toEqual( + JSON.parse(JSON.stringify(responses)), + ); + const restored = makeTurn(); + expect( + await restored.resumeHookResults( + responses, + 'model', + new AbortController().signal, + ), + ).toEqual(responses); + expect(await session.sink.project()).toEqual(history); + await expect( + restored.resumeHookResults( + responses.slice(0, 1), + 'model', + new AbortController().signal, + ), + ).rejects.toBeInstanceOf(HostedToolRecoveryRequiredError); + expect(fire).not.toHaveBeenCalled(); + await turn.finish(); + await restored.close(); + }, +); + +it.each([ + { + name: 'run_shell_command', + args: { command: 'pwd', timeout: 0 }, + error: 'Hosted Shell timeout must be an integer from 1 to 600000 ms.', + }, + { + name: 'run_shell_command', + args: { command: 'pwd', description: 7 }, + error: 'Hosted Shell description must be a string.', + }, +])( + 'preserves the $name validation error beside malformed Unicode', + async ({ name, args, error }) => { + turn = createTurn(true); + const batch = [ + { ...calls[0], name, args }, + { ...calls[1], name: 'run_shell_command', args: { command: '\ud800' } }, + ]; + const responses = await turn.execute( + batch, + batch.map((call) => ({ + functionCall: { id: call.callId, name: call.name, args: call.args }, + })), + 'model', + new AbortController().signal, + ); + expect(responses[0].functionResponse?.response?.['error']).toContain(error); + expect(responses[1].functionResponse?.response?.['error']).toContain( + 'unpaired UTF-16 surrogate', + ); + expect((await session.sink.project())[1].message?.parts).toEqual( + JSON.parse(JSON.stringify(responses)), + ); + expect(broker.acquire).not.toHaveBeenCalled(); + expect(broker.execute).not.toHaveBeenCalled(); + }, +); + +it.each([false, true])( + 'replays a Unicode refusal with optional sibling args (omitted: %s)', + async (omitted) => { + const fire = vi.fn(); + const makeTurn = () => + createTurn(true, { mode: 'default' }, hookSession(fire)); + turn = makeTurn(); + const batch = [ + { ...calls[0], name: 'run_shell_command', args: { command: '\ud800' } }, + { ...calls[1], name: 'read_file', args: {} }, + ]; + const input = batch.map((call, index) => ({ + functionCall: { + id: call.callId, + name: call.name, + ...(omitted && index === 1 ? {} : { args: call.args }), + }, + })); + const responses = await turn.execute( + batch, + input, + 'model', + new AbortController().signal, + ); + expect(responses[0].functionResponse?.response?.['error']).toContain( + 'unpaired UTF-16 surrogate', + ); + expect(responses[1].functionResponse?.response?.['error']).toContain( + 'require file_path', + ); + const restored = makeTurn(); + expect( + await restored.resumeHookResults( + responses, + 'model', + new AbortController().signal, + ), + ).toEqual(responses); + expect((await session.sink.project())[0].message?.parts).toEqual(input); + expect(fire).not.toHaveBeenCalled(); + expect(broker.acquire).not.toHaveBeenCalled(); + expect(broker.execute).not.toHaveBeenCalled(); + await turn.finish(); + await restored.close(); + }, +); + +it.each([false, true])( + 'checks malformed Shell before permission Hooks on an MCP turn (declared: %s)', + async (declared) => { + const fire = vi.fn(); + const mcp = { + broker: { ...broker, runtimeSessionId: 'prompt' }, + ensureReady: async () => undefined, + refresh: vi.fn(async () => undefined), + tools: () => [], + toolInput: () => undefined, + }; + turn = new HostedWorkspaceToolTurn( + { baseUrl: 'http://127.0.0.1:1', token: 'test' }, + session, + harness, + 'prompt', + commit, + messageFitsInline, + declared + ? { + resources: session.resources, + assertWritable: async () => undefined, + } + : undefined, + undefined, + { settings: { mode: 'default', timeoutMs: 100 }, waiters }, + { + profile: declared + ? 'hosted-workspace-shell/1' + : 'hosted-workspace-files/1', + hooks: hookSession(fire), + mcp: mcp as unknown as import('./hosted-mcp-session.js').HostedMcpSession, + }, + ); + const call = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + const batch = declared ? [call, calls[1]] : [call]; + const execution = turn.execute( + batch, + batch.map((item) => ({ + functionCall: { id: item.callId, name: item.name, args: item.args }, + })), + 'model', + new AbortController().signal, + ); + if (declared) { + const responses = await execution; + expect(responses[0].functionResponse?.response?.['error']).toContain( + 'unpaired UTF-16 surrogate', + ); + expect(responses[1].functionResponse?.response?.['error']).toContain( + 'not executed', + ); + expect((await session.sink.project()).map((entry) => entry.type)).toEqual( + ['assistant', 'tool_result'], + ); + await turn.finish(); + } else { + await expect(execution).rejects.toThrow( + 'Hosted Workspace profile refused a tool call.', + ); + expect(await session.sink.project()).toEqual([]); + } + expect(mcp.refresh).toHaveBeenCalled(); + expect(fire).not.toHaveBeenCalled(); + expect(broker.acquire).not.toHaveBeenCalled(); + expect(broker.prepare).not.toHaveBeenCalled(); + expect(broker.execute).not.toHaveBeenCalled(); + expect(broker.release).not.toHaveBeenCalled(); + }, +); + +it.each( + ['assistant', 'tool_result'].flatMap((failedType) => + [false, true].map((lostReply) => ({ failedType, lostReply })), + ), +)( + 'keeps malformed Shell Unicode recovery blocked after $failedType commit failure (lost reply: $lostReply)', + async ({ failedType, lostReply }) => { + const original = commit; + commit = async (...args) => { + if (args[0] === failedType) { + if (lostReply) await original(...args); + throw new Error('lost Unicode refusal commit'); + } + return original(...args); + }; + turn = createTurn(true); + const call = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + await expect( + turn.execute( + [call], + [ + { + functionCall: { id: call.callId, name: call.name, args: call.args }, + }, + ], + 'model', + new AbortController().signal, + ), + ).rejects.toBeInstanceOf(HostedToolRecoveryRequiredError); + await expect(turn.finish()).rejects.toBeInstanceOf( + HostedToolRecoveryRequiredError, + ); + expect(broker.acquire).not.toHaveBeenCalled(); + expect(broker.execute).not.toHaveBeenCalled(); + const history = await session.sink.project(); + expect(history.map((entry) => entry.type)).toEqual( + failedType === 'assistant' + ? lostReply + ? ['assistant'] + : [] + : lostReply + ? ['assistant', 'tool_result'] + : ['assistant'], + ); + }, +); + +it.each<{ + event: HookEventName; + deny: boolean; + denySibling?: HookEventName; + failResult?: boolean; + bothEvents?: boolean; + unstartedSibling?: boolean; + cancelledSizePlan?: 'not_started_proven' | 'settled'; + bounded?: boolean; +}>([ + { event: HookEventName.PermissionRequest, deny: false }, + { + event: HookEventName.PermissionRequest, + deny: false, + unstartedSibling: true, + }, + { event: HookEventName.PreToolUse, deny: false, bounded: true }, + { event: HookEventName.PermissionRequest, deny: false, bounded: true }, + ...(['not_started_proven', 'settled'] as const).map((cancelledSizePlan) => ({ + event: HookEventName.PermissionRequest, + deny: false, + cancelledSizePlan, + })), + { event: HookEventName.PreToolUse, deny: false }, + { event: HookEventName.PreToolUse, deny: true }, + { + event: HookEventName.PreToolUse, + deny: false, + denySibling: HookEventName.PermissionRequest, + }, + { + event: HookEventName.PreToolUse, + deny: false, + denySibling: HookEventName.PreToolUse, + }, + { event: HookEventName.PreToolUse, deny: false, failResult: true }, + { event: HookEventName.PermissionRequest, deny: false, bothEvents: true }, +])( + 'refuses a durable malformed $event rewrite for the entire batch (deny: $deny, sibling: $denySibling, fault: $failResult, concurrent marker reads: $bothEvents, unstarted sibling: $unstartedSibling, cancelled size plan: $cancelledSizePlan, bounded: $bounded) across reconstruction', + async ({ + event, + deny, + denySibling, + failResult, + bothEvents, + unstartedSibling, + cancelledSizePlan, + bounded, + }) => { + const pin = { + catalogId: 'unicode', + catalogRevision: 1, + definitionDigest: 'a'.repeat(64), + }; + const malformed = { command: 'echo \ud800' }; + const hookControl = vi.fn( + async (operation) => ({ + operationId: operation.operationId, + state: 'settled', + ...(operation.kind === 'hook-catalog' + ? { + catalog: { + ...pin, + hooks: [ + ...new Set([ + event, + ...(denySibling ? [denySibling] : []), + ...(bothEvents + ? [ + HookEventName.PreToolUse, + HookEventName.PermissionRequest, + ] + : []), + HookEventName.PostToolBatch, + ]), + ].flatMap((eventName) => { + const hook = { + hookId: eventName, + eventName, + matcher: + bothEvents || unstartedSibling || cancelledSizePlan + ? undefined + : 'run_shell_command', + sequential: bounded ?? false, + async: false, + failClosed: true, + onceKey: null, + config: { type: 'command' as const }, + }; + return bounded && event === HookEventName.PermissionRequest + ? [{ ...hook, hookId: `${eventName}:metadata` }, hook] + : [hook]; + }), + }, + } + : { + result: { + success: true, + outcome: 'success', + duration: 0, + output: + bounded && event === HookEventName.PermissionRequest + ? { + hookSpecificOutput: + operation.kind === 'hook-execute' && + operation.hookId.endsWith(':metadata') + ? { + decision: { + behavior: 'allow', + message: 'x'.repeat(40 * 1024), + }, + } + : { + updatedInput: { + command: 'x'.repeat(40 * 1024), + description: '\ud800', + }, + }, + } + : bounded + ? { + hookSpecificOutput: { + tool_input: { description: '\ud800' }, + padding: 'x'.repeat(40 * 1024), + }, + } + : { + hookSpecificOutput: + operation.kind === 'hook-execute' && + operation.hookId === HookEventName.PermissionRequest + ? { + decision: { + behavior: + denySibling && + operation.kind === 'hook-execute' && + 'tool_use_id' in operation.input && + operation.input.tool_use_id === + 'denied-sibling' + ? 'deny' + : 'allow', + ...(event === + HookEventName.PermissionRequest && + 'tool_name' in operation.input && + operation.input.tool_name === + 'run_shell_command' + ? { updatedInput: malformed } + : {}), + }, + } + : { + ...(denySibling && + operation.kind === 'hook-execute' && + 'tool_use_id' in operation.input && + operation.input.tool_use_id === + 'denied-sibling' + ? denySibling === HookEventName.PreToolUse + ? { + permissionDecision: 'deny', + permissionDecisionReason: + 'recorded policy denial', + } + : {} + : event === HookEventName.PreToolUse + ? { + updatedInput: malformed, + ...(deny + ? { permissionDecision: 'deny' } + : {}), + } + : {}), + }, + }, + }, + }), + }), + ); + const hookBroker = { + ...broker, + runtimeSessionId: 'prompt', + runtime: { + bindingId: 'binding', + generation: '1', + workspaceGeneration: '1', + }, + hookControl, + } as unknown as HostedWorkspaceBroker; + const makeHooks = () => + new HostedHookSession( + { baseUrl: 'http://127.0.0.1:1', token: 'test' }, + session, + pin, + hookBroker, + ); + const approval = { + mode: + event === HookEventName.PermissionRequest || + denySibling === HookEventName.PermissionRequest || + bothEvents + ? ('default' as const) + : ('yolo' as const), + }; + const hooks = makeHooks(); + turn = createTurn(true, approval, hooks); + const shellCall = { + ...calls[0], + name: 'run_shell_command', + args: { command: bounded ? 'x'.repeat(40 * 1024) : 'echo valid' }, + }; + const batch = denySibling + ? [{ ...shellCall, callId: 'denied-sibling' }, shellCall] + : [ + shellCall, + cancelledSizePlan + ? { + ...calls[1], + args: { ...calls[1].args, new_string: 'x'.repeat(60 * 1024) }, + } + : calls[1], + ]; + if (unstartedSibling || cancelledSizePlan) { + await hooks.ensureReady(); + const sibling = batch[1]; + const occurrenceId = hostedHookOccurrenceId( + HookEventName.PermissionRequest, + `prompt:${sibling.callId}`, + ); + const original = session.authority.commitExtensionRecord.bind( + session.authority, + ); + const crash = vi + .spyOn(session.authority, 'commitExtensionRecord') + .mockImplementation(async (...args) => { + const result = await original(...args); + const record = args[1].record as { + hookExecutionId?: string; + hookId?: string; + run: { execution: string }; + }; + if ( + record.hookId === '__plan__' && + record.hookExecutionId === occurrenceId && + record.run.execution === + (cancelledSizePlan === 'settled' ? 'dispatch_started' : 'intent') + ) + throw new Error('crash after plan commit'); + return result; + }); + try { + await expect( + hooks.fire( + HookEventName.PermissionRequest, + `prompt:${sibling.callId}`, + { + tool_name: sibling.name, + tool_input: sibling.args, + tool_use_id: sibling.callId, + prompt_id: 'prompt', + }, + new AbortController().signal, + undefined, + undefined, + HOSTED_TOOL_INPUT_ORDER, + ), + ).rejects.toThrow('crash after plan'); + } finally { + crash.mockRestore(); + } + const marker = await hooks.status(occurrenceId, true); + if (cancelledSizePlan === 'settled') + expect(marker.resultRef).not.toBeNull(); + else expect(marker.resultRef).toBeNull(); + expect(marker.run).toMatchObject({ + state: 'cancelled', + execution: cancelledSizePlan ?? 'not_started_proven', + }); + if (cancelledSizePlan) { + const plan = JSON.parse( + (await session.resources.read(marker.planRef)).toString(), + ); + expect(plan).toMatchObject({ + refusedInputDigest: expect.stringMatching(/^[a-f0-9]{64}$/), + hooks: [], + }); + expect(plan.input.tool_use_id).toBeUndefined(); + } + } + const publish = bounded + ? vi.spyOn(session.resources, 'publish') + : undefined; + const originalCommit = commit; + if (failResult) + commit = async (...args) => { + if (args[0] === 'tool_result') + throw new Error('store unavailable while refusing rewrite'); + return originalCommit(...args); + }; + if (failResult) turn = createTurn(true, approval, hooks); + const settle = vi.spyOn(harness, 'settleConsumedRuntimeContinuation'); + const execution = turn.execute( + batch, + batch.map((call) => ({ + functionCall: { id: call.callId, name: call.name, args: call.args }, + })), + 'model', + new AbortController().signal, + ); + if (failResult) { + const error = await execution.catch((cause: unknown) => cause); + expect(error).toBeInstanceOf(HostedToolRecoveryRequiredError); + expect((error as HostedToolRecoveryRequiredError).cause).toEqual( + new Error('store unavailable while refusing rewrite'), + ); + await expect(turn.finish()).rejects.toBeInstanceOf( + HostedToolRecoveryRequiredError, + ); + expect(settle).not.toHaveBeenCalled(); + expect(hooks.mountHeld).toBe(true); + expect((await session.sink.project()).map((entry) => entry.type)).toEqual( + ['assistant'], + ); + expect(broker.execute).not.toHaveBeenCalled(); + return; + } + const responses = await execution; + expect( + responses[denySibling ? 1 : 0].functionResponse?.response?.['error'], + ).toContain('unpaired UTF-16 surrogate'); + if (denySibling) + expect(responses[0].functionResponse?.response?.['error']).toBe( + denySibling === HookEventName.PermissionRequest + ? 'PermissionRequest Hook denied the call.' + : 'recorded policy denial', + ); + else + expect(responses[1].functionResponse?.response?.['error']).toContain( + cancelledSizePlan + ? 'Hook data exceeds the 60 KiB' + : unstartedSibling + ? 'PermissionRequest Hook denied the call.' + : 'not executed', + ); + if (bounded) { + for (const [kind, bytes] of publish!.mock.calls) + if (kind === 'managed-hook-result') + expect(bytes.length).toBeLessThanOrEqual(60 * 1024); + const marker = session.authority.extensionRecord( + 'hook_execution', + hostedHookOccurrenceId(event, `prompt:${shellCall.callId}`), + )!.record as { resultRef: ManagedSessionDurableRef }; + const saved = JSON.parse( + (await session.resources.read(marker.resultRef)).toString(), + ); + expect(saved.output).toMatchObject({ + reason: expect.stringContaining('60 KiB'), + stopReason: + 'Hosted Shell description contains an unpaired UTF-16 surrogate. Provide valid Unicode and retry.', + }); + if (event === HookEventName.PermissionRequest) { + expect(saved.output.hookSpecificOutput).toEqual({ + decision: { behavior: 'deny', interrupt: true }, + }); + expect(broker.acquire).toHaveBeenCalledOnce(); + } else expect(saved.output.hookSpecificOutput).toBeUndefined(); + } + expect(broker.prepare).not.toHaveBeenCalled(); + expect(broker.execute).not.toHaveBeenCalled(); + const history = await session.sink.project(); + expect(history.map((entry) => entry.type)).toEqual([ + 'assistant', + 'tool_result', + ]); + const executed = hookControl.mock.calls.filter( + ([operation]) => operation.kind === 'hook-execute', + ); + expect( + executed.map( + ([operation]) => operation.kind === 'hook-execute' && operation.hookId, + ), + ).toEqual( + denySibling === HookEventName.PermissionRequest + ? [ + HookEventName.PreToolUse, + HookEventName.PreToolUse, + HookEventName.PermissionRequest, + ] + : denySibling + ? [event, event] + : bothEvents + ? [ + HookEventName.PreToolUse, + HookEventName.PreToolUse, + HookEventName.PermissionRequest, + HookEventName.PermissionRequest, + ] + : bounded && event === HookEventName.PermissionRequest + ? [`${event}:metadata`, event] + : [event], + ); + const restored = createTurn(true, approval, makeHooks()); + const count = hookControl.mock.calls.length; + const markerRefs = bothEvents + ? new Set( + [HookEventName.PermissionRequest, HookEventName.PreToolUse].flatMap( + (eventName) => { + const record = session.authority.extensionRecord( + 'hook_execution', + hostedHookOccurrenceId(eventName, `prompt:${shellCall.callId}`), + )!.record as { + inputRef: ManagedSessionDurableRef; + resultRef: ManagedSessionDurableRef; + }; + return [record.inputRef, record.resultRef].map( + (ref) => ref.resourceId, + ); + }, + ), + ) + : undefined; + const read = session.resources.read.bind(session.resources); + let unblock = () => {}; + const gate = new Promise((resolve) => { + unblock = resolve; + }); + const reads: string[] = []; + const spy = bothEvents + ? vi.spyOn(session.resources, 'read').mockImplementation(async (ref) => { + if (markerRefs!.has(ref.resourceId)) { + reads.push(ref.resourceId); + await gate; + } + return read(ref); + }) + : undefined; + const resuming = restored.resumeHookResults( + responses, + 'model', + new AbortController().signal, + ); + const drained = Promise.allSettled([resuming]); + try { + if (bothEvents) + await vi.waitFor(() => expect(reads).toHaveLength(4), { + timeout: 10_000, + }); + } finally { + unblock(); + await drained; + spy?.mockRestore(); + } + expect(await resuming).toEqual(responses); + expect( + await restored.resumeHookResults( + responses, + 'model', + new AbortController().signal, + ), + ).toEqual(responses); + expect(hookControl).toHaveBeenCalledTimes(count); + expect(await session.sink.project()).toEqual(history); + await turn.finish(); + if (event === HookEventName.PreToolUse) { + expect(settle).toHaveBeenCalledOnce(); + expect((await harness.ensureRunnable()).continuation.phase).toBe( + 'before_model', + ); + expect(hooks.mountHeld).toBe(true); + } + if (unstartedSibling) { + const originalView = session.authority.extensionRecord.bind( + session.authority, + ); + const blocked = vi + .spyOn(session.authority, 'extensionRecord') + .mockImplementation((domain, id) => { + const entry = originalView(domain, id); + if ( + entry && + domain === 'hook_execution' && + id === + hostedHookOccurrenceId( + HookEventName.PermissionRequest, + `prompt:${batch[1].callId}`, + ) + ) { + const record = entry.record as { run: Record }; + return { + ...entry, + record: { + ...record, + run: { + ...record.run, + state: 'recovery_blocked', + reason: 'handler_unavailable', + }, + }, + }; + } + return entry; + }); + try { + await expect( + createTurn(true, approval, makeHooks()).resumeHookResults( + responses, + 'model', + new AbortController().signal, + ), + ).rejects.toBeInstanceOf(HostedToolRecoveryRequiredError); + } finally { + blocked.mockRestore(); + } + } + await restored.close(); + }, +); + +it.each(['live arguments', 'persisted error text'] as const)( + 'rejects contradictory Unicode refusal evidence: %s', + async (mismatch) => { + const hooks = hookSession(vi.fn()); + const call = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + turn = createTurn(true, undefined, hooks); + const assistant = [ + { + functionCall: { + id: call.callId, + name: call.name, + args: + mismatch === 'live arguments' + ? { command: 'different\ud800' } + : call.args, + }, + }, + ]; + if (mismatch === 'live arguments') { + await expect( + turn.execute([call], assistant, 'model', new AbortController().signal), + ).rejects.toBeInstanceOf(HostedToolRecoveryRequiredError); + } else { + const responses = await turn.execute( + [call], + assistant, + 'model', + new AbortController().signal, + ); + const tampered = structuredClone(responses); + tampered[0].functionResponse!.response!['error'] = 'different error'; + await expect( + createTurn(true, undefined, hooks).resumeHookResults( + tampered, + 'model', + new AbortController().signal, + ), + ).rejects.toBeInstanceOf(HostedToolRecoveryRequiredError); + await expect( + createTurn(true, undefined, hooks).resumeHookResults( + responses, + 'model', + new AbortController().signal, + ), + ).resolves.toEqual(responses); + } + expect((await session.sink.project()).map((entry) => entry.type)).toEqual([ + 'assistant', + 'tool_result', + ]); + expect(broker.execute).not.toHaveBeenCalled(); + expect(hooks.fire).not.toHaveBeenCalled(); + }, +); + +it('keeps Unicode refusal replay blocked by a real unresolved sibling Hook', async () => { + const pin = { + catalogId: 'unresolved', + catalogRevision: 1, + definitionDigest: 'a'.repeat(64), + }; + const hookControl = vi.fn( + async (operation) => ({ + operationId: operation.operationId, + ...(operation.kind === 'hook-catalog' + ? { + state: 'settled' as const, + catalog: { + ...pin, + hooks: [ + { + hookId: 'unresolved', + eventName: HookEventName.PermissionRequest, + sequential: false, + async: false, + failClosed: true, + onceKey: null, + config: { type: 'command' as const }, + }, + ], + }, + } + : { state: 'outcome_unknown' as const }), + }), + ); + const hookBroker = { + ...broker, + runtimeSessionId: 'prompt', + hookControl, + runtime: { + bindingId: 'binding', + generation: '1', + workspaceGeneration: '1', + }, + } as unknown as HostedWorkspaceBroker; + const makeHooks = () => + new HostedHookSession( + { baseUrl: 'http://127.0.0.1:1', token: 'test' }, + session, + pin, + hookBroker, + ); + const hooks = makeHooks(); + const malformed = { + ...calls[0], + name: 'run_shell_command', + args: { command: '\ud800' }, + }; + const batch = [malformed, calls[1]]; + turn = createTurn(true, undefined, hooks); + const responses = await turn.execute( + batch, + batch.map((call) => ({ + functionCall: { + id: call.callId, + name: call.name, + args: call.args, + }, + })), + 'model', + new AbortController().signal, + ); + const sibling = batch[1]; + await expect( + hooks.fire( + HookEventName.PermissionRequest, + `prompt:${sibling.callId}`, + { + tool_name: sibling.name, + tool_input: sibling.args, + tool_use_id: sibling.callId, + prompt_id: 'prompt', + }, + new AbortController().signal, + ), + ).rejects.toThrow('Hosted Hook requires reconciliation'); + const child = session.authority + .extensionRecordsInDomain('hook_execution') + .map( + (entry) => + entry.record as { hookId: string; run: Record }, + ) + .find((record) => record.hookId === 'unresolved')!; + expect(child.run).toMatchObject({ + state: 'recovery_blocked', + execution: 'outcome_unknown', + }); + const marker = await hooks.status( + hostedHookOccurrenceId( + HookEventName.PermissionRequest, + `prompt:${sibling.callId}`, + ), + true, + ); + expect(marker.resultRef).toBeNull(); + expect(marker.run.execution).toBe('dispatch_started'); + await expect( + createTurn(true, undefined, makeHooks()).resumeHookResults( + responses, + 'model', + new AbortController().signal, + ), + ).rejects.toBeInstanceOf(HostedToolRecoveryRequiredError); + expect(broker.execute).not.toHaveBeenCalled(); +}); + it.each(['assistant-commit', 'second-call'] as const)( 'settles every tool refusal when cancelled before a new Hook plan (%s)', async (window) => { @@ -5206,14 +6359,19 @@ it.each(['allow', 'deny'] as const)( }, ); -it.each([ +it.each<{ + decision: 'allow' | 'deny' | 'ask'; + oversized: boolean; + ordered?: boolean; +}>([ { decision: 'allow', oversized: false }, { decision: 'deny', oversized: false }, { decision: 'ask', oversized: false }, { decision: 'allow', oversized: true }, -] as const)( - 'restores PreToolUse context from its durable receipt without duplicate effects ($decision, oversized: $oversized)', - async ({ decision, oversized }) => { + { decision: 'allow', oversized: false, ordered: true }, +])( + 'restores PreToolUse context from its durable receipt without duplicate effects ($decision, oversized: $oversized, ordered: $ordered)', + async ({ decision, oversized, ordered }) => { if (oversized) { const remote = createHttpManagedSessionStores({ baseUrl: 'http://127.0.0.1:1', @@ -5246,6 +6404,7 @@ it.each([ catalog: { ...pin, hooks: [ + ...(ordered ? [HookEventName.PermissionRequest] : []), HookEventName.PreToolUse, HookEventName.PostToolUse, ].map((eventName) => ({ @@ -5267,18 +6426,37 @@ it.each([ output: { hookSpecificOutput: operation.kind === 'hook-execute' && - operation.hookId === HookEventName.PreToolUse + operation.hookId === HookEventName.PermissionRequest ? { - permissionDecision: decision, - additionalContext: oversized - ? 'x'.repeat(36 * 1024) - : 'before ', + decision: { + behavior: 'allow', + updatedInput: { + file_path: 'permission.txt', + content: 'fixture', + }, + }, } - : { - additionalContext: oversized - ? 'y'.repeat(36 * 1024) - : 'after tool', - }, + : operation.kind === 'hook-execute' && + operation.hookId === HookEventName.PreToolUse + ? { + permissionDecision: decision, + ...(ordered + ? { + updatedInput: { + file_path: 'pre.txt', + content: 'fixture', + }, + } + : {}), + additionalContext: oversized + ? 'x'.repeat(36 * 1024) + : 'before ', + } + : { + additionalContext: oversized + ? 'y'.repeat(36 * 1024) + : 'after tool', + }, }, }, }), @@ -5301,10 +6479,18 @@ it.each([ pin, hookBroker, ); - turn = createTurn(false, { mode: 'yolo' }, createHooks()); + const mode = ordered ? 'default' : 'yolo'; + turn = createTurn(false, { mode }, createHooks()); + const call = ordered + ? { + ...calls[0], + name: 'write_file', + args: { file_path: 'original.txt', content: 'fixture' }, + } + : calls[0]; const running = turn.execute( - [calls[0]], - [parts[0]], + [call], + [{ functionCall: { id: call.callId, name: call.name, args: call.args } }], 'model', new AbortController().signal, ); @@ -5316,6 +6502,48 @@ it.each([ answer('deny'), ); const responses = await running; + if (ordered) { + expect( + hookControl.mock.calls.find( + ([operation]) => + operation.kind === 'hook-execute' && + operation.hookId === HookEventName.PreToolUse, + )?.[0], + ).toMatchObject({ + input: { tool_input: { file_path: 'original.txt' } }, + }); + expect( + hookControl.mock.calls.find( + ([operation]) => + operation.kind === 'hook-execute' && + operation.hookId === HookEventName.PermissionRequest, + )?.[0], + ).toMatchObject({ input: { tool_input: { file_path: 'pre.txt' } } }); + expect( + hookControl.mock.calls + .filter(([operation]) => operation.kind === 'hook-execute') + .map( + ([operation]) => + operation.kind === 'hook-execute' && operation.hookId, + ), + ).toEqual([ + HookEventName.PreToolUse, + HookEventName.PermissionRequest, + HookEventName.PostToolUse, + ]); + expect(JSON.parse(broker.execute.mock.calls[0][1])).toMatchObject({ + input: { file_path: 'permission.txt' }, + }); + expect( + hookControl.mock.calls.find( + ([operation]) => + operation.kind === 'hook-execute' && + operation.hookId === HookEventName.PostToolUse, + )?.[0], + ).toMatchObject({ + input: { tool_input: { file_path: 'permission.txt' } }, + }); + } const contexts = responses.filter((part) => typeof part.text === 'string'); expect(contexts).toEqual( decision === 'ask' || oversized @@ -5337,7 +6565,7 @@ it.each([ } const history = await session.sink.project(); const controls = hookControl.mock.calls.length; - const restored = createTurn(false, { mode: 'yolo' }, createHooks()); + const restored = createTurn(false, { mode }, createHooks()); expect( await restored.resumeHookResults( responses, @@ -7810,6 +9038,7 @@ function backgroundTurnRig( closeResults?: ReadonlyArray<'NOT_STARTED' | 'MISSING'>; executionCallIds?: readonly string[]; hooks?: HostedHookSession; + approval?: 'default'; } = {}, ) { enablement.childRun = true; @@ -7955,12 +9184,14 @@ function backgroundTurnRig( (type: string) => options.fitsInline?.(type) ?? true, { owner, captureBytes: 1024 * 1024 }, undefined, - undefined, + options.approval + ? { settings: { mode: options.approval, timeoutMs: 100 }, waiters } + : undefined, { + hooks: options.hooks, childRuns: orchestrator as never, backgroundLane: options.lane, foregroundOnly: options.foregroundOnly, - hooks: options.hooks, }, ); return { @@ -10470,3 +11701,39 @@ it('keeps the deliberate refusal while child_run stays disabled', async () => { expect(broker.executeV3).not.toHaveBeenCalled(); expect(broker.prepareV3).not.toHaveBeenCalled(); }); + +it('refuses a malformed permission rewrite on an admitted background Shell', async () => { + const fire = vi + .fn() + .mockImplementation(async (event) => + event === HookEventName.PermissionRequest + ? { + hookSpecificOutput: { + decision: { + behavior: 'allow', + updatedInput: { command: '\ud800', is_background: true }, + }, + }, + } + : undefined, + ); + const rig = backgroundTurnRig( + { executionStatus: 'success', responseParts: [], capture: null }, + { hooks: hookSession(fire), approval: 'default' }, + ); + turn = rig.turn; + const { call, parts: input } = backgroundCall(); + const responses = await turn.execute( + [call], + input, + 'model', + new AbortController().signal, + ); + expect(responses[0].functionResponse?.response?.['error']).toContain( + 'unpaired UTF-16 surrogate', + ); + expect(rig.orchestrator.calls).toHaveLength(0); + expect(broker.prepareV3).not.toHaveBeenCalled(); + expect(broker.executeV3).not.toHaveBeenCalled(); + expect(broker.acquire).toHaveBeenCalledOnce(); +}); diff --git a/packages/cli/src/serve/hosted-workspace-tool-turn.ts b/packages/cli/src/serve/hosted-workspace-tool-turn.ts index 807560660b9..ee5b27221ed 100644 --- a/packages/cli/src/serve/hosted-workspace-tool-turn.ts +++ b/packages/cli/src/serve/hosted-workspace-tool-turn.ts @@ -108,6 +108,10 @@ import { type HostedWorkspaceBrokerOptions, } from './hosted-workspace-broker.js'; import { HostedShellPublisher } from './hosted-shell-publisher.js'; +import { + hostedShellHookInputError, + hostedShellInputError, +} from './hosted-shell-input.js'; import { MANAGED_WORKSPACE_CONTEXT_FILES } from './managed-runtime-provider-protocol.js'; import type { HostedMcpSession } from './hosted-mcp-session.js'; import type { @@ -2638,6 +2642,139 @@ export class HostedWorkspaceToolTurn { entry.payload['batchId'] === assistant?.uuid, ); const preToolOutputs = new Map(); + let unicodeRefused = calls.some((call) => + hostedShellInputError(call.name, call.args), + ); + let completeEvidence = true; + await Promise.all( + calls.map(async (call) => { + let args = call.args; + const records = await Promise.all( + [HookEventName.PermissionRequest, HookEventName.PreToolUse].map( + async (event) => { + const marker = this.session.authority.extensionRecord( + 'hook_execution', + hostedHookOccurrenceId( + event, + `${this.promptId}:${call.callId}`, + ), + ); + if (!marker) return undefined; + const record = parseHookExecution(marker.record); + const [inputBytes, resultBytes] = await Promise.all([ + this.session.resources.read(record.inputRef), + record.resultRef + ? this.session.resources.read(record.resultRef) + : undefined, + ]); + const original = JSON.parse(inputBytes.toString()) as { + tool_input?: Record; + tool_use_id?: string; + tool_name?: string; + }; + const saved = resultBytes + ? (JSON.parse(resultBytes.toString()) as { + output?: HookOutput; + }) + : undefined; + let matches = + original.tool_use_id === call.callId && + original.tool_name === call.name; + if (!matches) { + const plan = JSON.parse( + ( + await this.session.resources.read(record.planRef) + ).toString(), + ) as { + refusedInputDigest?: string; + hooks?: unknown[]; + input?: unknown; + }; + matches = + typeof plan.refusedInputDigest === 'string' && + /^[a-f0-9]{64}$/.test(plan.refusedInputDigest) && + plan.hooks?.length === 0 && + isDeepStrictEqual(plan.input, original) && + (record.resultRef !== null || + record.run.execution === 'not_started_proven'); + } + if ( + (!record.resultRef && + record.run.execution !== 'not_started_proven') || + record.run.state === 'recovery_blocked' || + !matches + ) + completeEvidence = false; + return { event, original, saved }; + }, + ), + ); + for (const record of records) { + if (!record) continue; + const { event, original, saved } = record; + const specific = saved?.output?.hookSpecificOutput; + const decision = specific?.['decision'] as + | { updatedInput?: Record } + | undefined; + args = + (event === HookEventName.PermissionRequest + ? decision?.updatedInput + : (specific?.['updatedInput'] as + | Record + | undefined)) ?? + original.tool_input ?? + args; + if ( + hostedShellInputError(call.name, args) || + hostedShellHookInputError(call.name, saved?.output) + ) + unicodeRefused = true; + if (event === HookEventName.PreToolUse && saved?.output) + preToolOutputs.set( + call.callId, + new PreToolUseHookOutput(saved.output), + ); + } + }), + ); + if (unicodeRefused) { + const originalCalls = assistant?.message?.parts?.flatMap((part) => + part.functionCall ? [part.functionCall] : [], + ); + const persisted = history + .slice(assistant ? history.indexOf(assistant) + 1 : history.length) + .find((entry) => entry.type === 'tool_result')?.message?.parts; + if ( + !completeEvidence || + intents.length || + !isDeepStrictEqual( + originalCalls?.map((call) => ({ + id: call.id, + name: call.name, + args: call.args ?? {}, + })), + calls.map((call) => ({ + id: call.callId, + name: call.name, + args: call.args, + })), + ) || + responses.length !== calls.length || + !responses.every( + (part, index) => + part.functionResponse?.id === calls[index].callId && + part.functionResponse?.name === calls[index].name && + typeof part.functionResponse?.response?.['error'] === 'string', + ) || + !isDeepStrictEqual(persisted, JSON.parse(JSON.stringify(responses))) + ) { + this.uncertain = true; + throw new HostedToolRecoveryRequiredError( + 'Shell Unicode refusal evidence is incomplete or contradictory.', + ); + } + return responses; + } // Each call with the input its Hook decisions left. Records without an // order are the earlier one's, where PreToolUse ran on what // PermissionRequest left: there `earlier` is how an earlier release @@ -3041,6 +3178,43 @@ export class HostedWorkspaceToolTurn { this.promptHookRunner = runner; } + private async refuseBatch( + calls: ToolCallRequestInfo[], + errors: Array, + parts: Part[] | undefined, + model: string, + skillOrigin: + | Pick + | undefined, + ): Promise { + if (parts && !this.messageFitsInline('assistant', parts, model)) + throw new Error( + 'Hosted assistant record exceeds the inline Session Store limit.', + ); + const responses = calls.flatMap((call, index) => + convertToFunctionErrorResponse( + call.name, + call.callId, + [], + errors[index] ?? + 'This tool was not executed because another call in the batch has invalid arguments. Retry the batch with corrected arguments.', + ), + ); + if (!this.messageFitsInline('tool_result', responses, model)) + throw new Error( + 'Hosted tool refusal exceeds the inline Session Store limit.', + ); + this.uncertain = true; + try { + if (parts) await this.commit('assistant', parts, model, skillOrigin); + await this.commit('tool_result', responses, model); + this.uncertain = false; + return responses; + } catch (cause) { + throw new HostedToolRecoveryRequiredError(cause); + } + } + setToolOutputBudget(budget: number): void { this.toolOutputBudget = budget; } @@ -3518,6 +3692,8 @@ export class HostedWorkspaceToolTurn { ) { validationError = 'Hosted Shell requires one foreground command.'; } + validationError = + hostedShellInputError(call.name, args) ?? validationError; input = backgroundAdmitted ? { ...args, is_background: true } : this.publication @@ -4054,7 +4230,6 @@ export class HostedWorkspaceToolTurn { }; }); }; - let requests = restored?.requests ?? prepareRequests(calls); const skillOrigin = calls.every( (call) => call.name === 'skill' && call.isClientInitiated, ) @@ -4064,6 +4239,23 @@ export class HostedWorkspaceToolTurn { clientInitiatedSkills: true as const, } : undefined; + const unicodeErrors = calls.map((call) => + hostedShellInputError(call.name, call.args), + ); + if (unicodeErrors.some((error) => error !== undefined)) { + // Profile, duplicate-id and truncated-input hard refusals still win. + const requests = prepareRequests(calls); + return this.refuseBatch( + calls, + requests.map( + (request, index) => unicodeErrors[index] ?? request.validationError, + ), + parts, + model, + skillOrigin, + ); + } + let requests = restored?.requests ?? prepareRequests(calls); if (!this.messageFitsInline('assistant', parts, model)) throw new Error( 'Hosted assistant record exceeds the inline Session Store limit.', @@ -4088,28 +4280,13 @@ export class HostedWorkspaceToolTurn { ); signal.throwIfAborted(); if (requests.some((request) => request.validationError)) { - const responses = requests.flatMap((request) => - convertToFunctionErrorResponse( - request.call.name, - request.call.callId, - [], - request.validationError ?? - 'This tool was not executed because another call in the batch has invalid arguments. Retry the batch with corrected arguments.', - ), + return this.refuseBatch( + calls, + requests.map((request) => request.validationError), + parts, + model, + skillOrigin, ); - if (!this.messageFitsInline('tool_result', responses, model)) - throw new Error( - 'Hosted tool refusal exceeds the inline Session Store limit.', - ); - this.uncertain = true; - try { - await this.commit('assistant', parts, model, skillOrigin); - await this.commit('tool_result', responses, model); - this.uncertain = false; - return responses; - } catch (cause) { - throw new HostedToolRecoveryRequiredError(cause); - } } if (this.hooks || this.mcp || requests.some((request) => !request.skill)) await waitForTurn(this.warmed, signal); @@ -4246,14 +4423,25 @@ export class HostedWorkspaceToolTurn { const asks = new Set(); if (this.hooks) { const decided = this.nativeBatch.refusals; - requests = await this.decideHookInputs( + const decidedInputs = await this.decideHookInputs( requests, (inputs) => prepareRequests(inputs, true), decided, asks, signal, ); + requests = decidedInputs.requests; this.nativeBatch.requests = requests; + if (decidedInputs.unicode.some((error) => error !== undefined)) + return this.refuseBatch( + requests.map((request) => request.call), + requests.map( + (_, index) => decidedInputs.unicode[index] ?? decided[index], + ), + undefined, + model, + skillOrigin, + ); await startPublisher( requests.filter((_, index) => decided[index] === undefined), ); @@ -4278,6 +4466,7 @@ export class HostedWorkspaceToolTurn { Math.max(0, requests.length - 1), ); } catch (cause) { + if (cause instanceof HostedToolRecoveryRequiredError) throw cause; throw new HostedToolRecoveryRequiredError(cause); } const refusal = (index: number): Part[] | undefined => { @@ -7681,8 +7870,12 @@ export class HostedWorkspaceToolTurn { refusals: Array, asks: Set, signal: AbortSignal, - ): Promise { + ): Promise<{ + requests: HostedPreparedRequest[]; + unicode: Array; + }> { const inputs = requests.map((request) => request.call); + const unicode: Array = []; for (const [index, request] of requests.entries()) { const occurrence = `${this.promptId}:${request.call.callId}`; const output = await this.hooks!.fire( @@ -7698,6 +7891,12 @@ export class HostedWorkspaceToolTurn { undefined, HOSTED_TOOL_INPUT_ORDER, ); + unicode[index] = + hostedShellInputError( + request.call.name, + output?.hookSpecificOutput?.['updatedInput'], + ) ?? hostedShellHookInputError(request.call.name, output); + if (unicode[index] !== undefined) continue; refusals[index] = preToolUseRefusal(output, request.skill); if (refusals[index] !== undefined) continue; const replacement = preToolUseReplacement(output, request.skill); @@ -7714,6 +7913,7 @@ export class HostedWorkspaceToolTurn { let permissionRan = false; for (const [index, request] of requests.entries()) { if ( + unicode[index] !== undefined || refusals[index] !== undefined || !this.approval || request.skill || @@ -7725,17 +7925,23 @@ export class HostedWorkspaceToolTurn { ) continue; permissionRan = true; - const decision = permissionRequestDecision( - await this.hooks!.fire( - HookEventName.PermissionRequest, - `${this.promptId}:${request.call.callId}`, - this.permissionRequestFields(request.call), - signal, - this.promptHookRunner, - undefined, - HOSTED_TOOL_INPUT_ORDER, - ), + const output = await this.hooks!.fire( + HookEventName.PermissionRequest, + `${this.promptId}:${request.call.callId}`, + this.permissionRequestFields(request.call), + signal, + this.promptHookRunner, + undefined, + HOSTED_TOOL_INPUT_ORDER, ); + const decision = permissionRequestDecision(output); + const unicodeError = + hostedShellInputError(request.call.name, decision.input) ?? + hostedShellHookInputError(request.call.name, output); + if (unicodeError !== undefined) { + unicode[index] = unicodeError; + continue; + } refusals[index] = decision.refusal; if (decision.allowed) this.hookAllowed.add(request.call.callId); if (decision.input) @@ -7746,7 +7952,7 @@ export class HostedWorkspaceToolTurn { for (const index of asks) refusals[index] ??= 'Hook requires approval, but no approval policy is available.'; - return requests; + return { requests, unicode }; } /** diff --git a/packages/managed-agent-server/src/test/java/com/alibaba/qwen/code/managedagent/HostedWorkspaceToolTurnIT.java b/packages/managed-agent-server/src/test/java/com/alibaba/qwen/code/managedagent/HostedWorkspaceToolTurnIT.java index 05e4626bd44..67793da4205 100644 --- a/packages/managed-agent-server/src/test/java/com/alibaba/qwen/code/managedagent/HostedWorkspaceToolTurnIT.java +++ b/packages/managed-agent-server/src/test/java/com/alibaba/qwen/code/managedagent/HostedWorkspaceToolTurnIT.java @@ -1,13 +1,21 @@ package com.alibaba.qwen.code.managedagent; +import com.alibaba.qwen.code.managedagent.api.ToolPublicationController; +import com.alibaba.qwen.code.managedagent.artifact.ToolPublicationObjectStore; +import com.alibaba.qwen.code.managedagent.config.ManagedAgentProperties; import com.alibaba.qwen.code.managedagent.persistence.ManagedAgentStore; import com.alibaba.qwen.code.managedagent.persistence.ManagedSessionStore; +import com.alibaba.qwen.code.managedagent.persistence.ToolPublicationAdmissionStore; +import com.alibaba.qwen.code.managedagent.persistence.ToolPublicationDataStore; +import com.alibaba.qwen.code.managedagent.persistence.ToolPublicationStore; import com.alibaba.qwen.code.managedagent.persistence.WorkspaceStorageGuard; import com.alibaba.qwen.code.managedagent.session.HarnessEventProjector; import com.alibaba.qwen.code.managedagent.workspace.WorkspaceSelection; import com.alibaba.qwen.code.managedagent.workspace.runtime.EmbeddedRuntimeBroker; +import com.alibaba.qwen.code.runtimebroker.RuntimeBindingRepository; import com.alibaba.qwen.code.runtimebroker.RuntimeSession; import com.alibaba.qwen.code.runtimebroker.RuntimeTransport; +import com.alibaba.qwen.code.runtimebroker.ToolExecutionRepository; import com.alibaba.qwen.code.runtimebroker.WorkspaceExecutionProfile; import com.alibaba.qwen.code.runtimebroker.managedschema.ManagedSchemaProjection; import com.fasterxml.jackson.databind.JsonNode; @@ -17,13 +25,19 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.system.CapturedOutput; import org.springframework.boot.test.system.OutputCaptureExtension; import org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext; +import org.springframework.context.support.GenericApplicationContext; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.transaction.PlatformTransactionManager; +import java.io.ByteArrayInputStream; +import java.io.InputStream; import java.lang.reflect.Proxy; import java.net.InetSocketAddress; import java.nio.charset.StandardCharsets; @@ -43,6 +57,7 @@ import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -67,6 +82,13 @@ class HostedWorkspaceToolTurnIT { } } + @ParameterizedTest + @ValueSource(strings = {"local", "o2"}) + @Timeout(360) + void malformedShellUnicodeIsCorrectableBeforeAcquisition(String capture) throws Exception { + runDriver(List.of(capture), "shell-unicode"); + } + @Test @Timeout(360) void operatorRecoveryReleasesOnlyTheOriginalQuiescentShellOnMySql() throws Exception { @@ -175,10 +197,53 @@ class HostedWorkspaceToolTurnIT { runDriver(cases, "provider-control"); } + private static void registerUnicodePublication(GenericApplicationContext context) { + var objects = new ConcurrentHashMap(); + context.registerBean("unicodeObjects", ToolPublicationObjectStore.class, () -> new ToolPublicationObjectStore() { + @Override + public void putIfAbsent(String key, byte[] bytes) { + byte[] previous = objects.putIfAbsent(key, bytes.clone()); + if (previous != null && !java.util.Arrays.equals(previous, bytes)) { + throw new IllegalStateException("Immutable fixture object changed"); + } + } + @Override + public InputStream open(String key) { + byte[] bytes = objects.get(key); + if (bytes == null) { + throw new IllegalStateException("Missing fixture object"); + } + return new ByteArrayInputStream(bytes); + } + @Override + public void validateConfiguration() {} + }); + context.registerBean("unicodeGrants", ToolPublicationStore.class, () -> new ToolPublicationStore( + context.getBean(JdbcTemplate.class), context.getBean(PlatformTransactionManager.class), + context.getBean(ManagedSessionStore.class), context.getBean(ToolExecutionRepository.class), + context.getBean(RuntimeBindingRepository.class), + new ToolPublicationStore.Capacity(1024 * 1024, 16 * 1024 * 1024, 64 * 1024 * 1024, 4), false)); + context.registerBean("unicodeData", ToolPublicationDataStore.class, () -> new ToolPublicationDataStore( + context.getBean(JdbcTemplate.class), context.getBean(PlatformTransactionManager.class), + context.getBean(ToolPublicationStore.class), context.getBean(ManagedSessionStore.class), + context.getBean(ToolPublicationObjectStore.class), Duration.ofSeconds(30), Duration.ofSeconds(5), + new ToolPublicationDataStore.VerificationBudget(1024 * 1024, Duration.ofMinutes(25)))); + context.registerBean("unicodePublicationController", ToolPublicationController.class, () -> { + var properties = context.getBean(ManagedAgentProperties.class); + properties.getToolPublication().setEntryConcurrency(4); + return new ToolPublicationController(context.getBean(ToolPublicationStore.class), + context.getBean(ToolPublicationDataStore.class), + new ToolPublicationAdmissionStore(context.getBean(JdbcTemplate.class), + context.getBean(PlatformTransactionManager.class), context.getBean(ManagedSessionStore.class), + context.getBean(ToolPublicationDataStore.class)), properties); + }); + } + private void runDriver(List cases, String driverName) throws Exception { boolean latency = driverName.equals("latency"); + boolean unicode = driverName.equals("shell-unicode"); boolean operatorRecovery = driverName.equals("operator-recovery"); - boolean faults = !driverName.equals("workspace-tool-turn") && !latency && !operatorRecovery; + boolean faults = !driverName.equals("workspace-tool-turn") && !latency && !unicode && !operatorRecovery; boolean storeFaults = driverName.equals("store-failure"); boolean cancellations = driverName.equals("cancellation"); boolean sseGaps = driverName.equals("sse-gap"); @@ -253,6 +318,9 @@ class HostedWorkspaceToolTurnIT { boolean verifiedRecovery = List.of("Linux", "Mac OS X").contains(System.getProperty("os.name")); arguments.add("--qwen.managed-agent.runtime-broker.verified-workspace-recovery-enabled=" + verifiedRecovery); var application = new SpringApplicationBuilder(ManagedAgentServerApplication.class); + if (unicode) { + application.initializers(context -> registerUnicodePublication((GenericApplicationContext) context)); + } if (sseGaps) { // Keep SQL polling outside the 10s receive window to require live hub delivery. arguments.add("--qwen.managed-agent.events.poll-interval=60s"); @@ -262,6 +330,10 @@ class HostedWorkspaceToolTurnIT { } try (var spring = (ServletWebServerApplicationContext) application.run(arguments.toArray(String[]::new))) { JdbcTemplate jdbc = spring.getBean(JdbcTemplate.class); + if (unicode) { + spring.getBean(ManagedAgentProperties.class).getToolPublication() + .setServiceBaseUrl("http://127.0.0.1:" + spring.getWebServer().getPort()); + } if (faults) { var metadata = jdbc.queryForMap("SELECT VERSION() AS version, @@version_comment AS engine"); System.out.println((shellOutput || providerControl ? "FG6F_DATABASE " : sseGaps ? "FG6E_DATABASE " : cancellations ? "FG6D_DATABASE " : storeFaults ? "FG6B_DATABASE " : "FG6A_DATABASE ") + metadata); @@ -284,7 +356,7 @@ class HostedWorkspaceToolTurnIT { "sha256:" + "a".repeat(64), "qwen-code", null, null, List.of(), null, new WorkspaceSelection(workspaceId, "child")); String profile = operatorRecovery ? (index < 2 ? "hosted-workspace-shell/1" : "hosted-workspace-files/1") - : !shellOutput && (faults || index < 2) ? "hosted-workspace-files/1" : "hosted-workspace-shell/1"; + : !unicode && !shellOutput && (faults || index < 2) ? "hosted-workspace-files/1" : "hosted-workspace-shell/1"; String secondary = operatorRecovery ? store.insertWorkspaceSessionCommand(tenant, "actor", "secondary-" + index, "sha256:" + "a".repeat(64), "qwen-code", null, null, List.of(), null, new WorkspaceSelection(workspaceId, "child")).sessionId() : ""; @@ -376,7 +448,8 @@ class HostedWorkspaceToolTurnIT { assertThat(driver.waitFor(faults || latency ? 130 : 270, TimeUnit.SECONDS)).as("Driver timeout: %s", Files.readString(log)).isTrue(); assertThat(driver.exitValue()).as("Driver output: %s", Files.readString(log)).isZero(); System.out.println(Files.readString(log)); - assertThat(Files.readString(log)).contains(operatorRecovery ? "HOSTED_OPERATOR_RECOVERY_OK" : latency ? "HOSTED_LATENCY_OK" + assertThat(Files.readString(log)).contains(unicode ? "HOSTED_SHELL_UNICODE_OK: " + String.join(",", cases) + : operatorRecovery ? "HOSTED_OPERATOR_RECOVERY_OK" : latency ? "HOSTED_LATENCY_OK" : providerControl ? "HOSTED_PROVIDER_FAULTS_OK" : shellOutput ? "HOSTED_SHELL_OUTPUT_FAULTS_OK" : sseGaps ? "HOSTED_SSE_GAP_OK" @@ -393,7 +466,35 @@ class HostedWorkspaceToolTurnIT { if (operatorRecovery) { continue; } - if (providerControl) { + if (unicode) { + assertThat(workspace.resolve("child/invalid.txt")).doesNotExist(); + assertThat(workspace.resolve("child/sibling.txt")).doesNotExist(); + assertThat(Files.readString(workspace.resolve("child/unicode-ok.txt"))) + .isEqualTo("中文😀é\\ud800"); + assertThat(Files.readString(workspace.resolve("child/unicode-secondary-ok.txt"))) + .isEqualTo("中文😀é\\ud800"); + for (String sessionId : List.of(sessions.get(index).get("sessionId").toString(), secondarySessionId)) { + var publications = jdbc.queryForList("SELECT state, producer_phase," + + " CASE WHEN accepted_complete THEN 1 ELSE 0 END AS complete" + + " FROM ma_output_capture WHERE tenant_id = ? AND session_id = ?", tenant, sessionId); + if (cases.contains("o2")) { + assertThat(publications).hasSize(1); + var publication = publications.getFirst(); + assertThat(publication.get("state")).isIn("OPEN", "FENCED"); + assertThat(publication.get("producer_phase")).isEqualTo("REFERENCED"); + assertThat(((Number) publication.get("complete")).intValue()).isEqualTo(1); + } else { + assertThat(publications).isEmpty(); + } + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM ma_tool_execution" + + " WHERE harness_session_id = ?", Integer.class, sessionId)).isEqualTo(1); + var execution = jdbc.queryForMap("SELECT dispatch_generation, execution_state, execution_status" + + " FROM ma_tool_execution WHERE harness_session_id = ?", sessionId); + assertThat(((Number) execution.get("dispatch_generation")).longValue()).isEqualTo(1); + assertThat(execution.get("execution_state")).isEqualTo("SETTLED"); + assertThat(execution.get("execution_status")).isEqualTo("success"); + } + } else if (providerControl) { providerProbe.assertReport(sessions.get(index), reports.get(index)); } else if (latency) { boolean tool = cases.get(index).equals("tool"); @@ -439,7 +540,7 @@ class HostedWorkspaceToolTurnIT { driver.destroyForcibly(); } } - if (!faults && !latency && !operatorRecovery) { + if (!faults && !latency && !unicode && !operatorRecovery) { Object service = ReflectionTestUtils.getField(broker, "service"); Object provisioner = ReflectionTestUtils.getField(service, "provisioner"); Object local = ReflectionTestUtils.getField(provisioner, "delegate");