Repository navigation
feat(managed-agent): Serve durable permission Actions (D6b) - #13101
Conversation
|
E2E and precommit verification report (macOS, Java 21):
Not established by this report: Windows/Linux, Oracle MySQL, or real-process deny/auto-edit variants. Question Actions, votes, approval UI and pending-approval restart recovery remain outside D6b. |
|
Fixed the CI failure in 373b93a. The asynchronous coordinator could call the shared Mockito mock between The test fixture now installs its default answer during bean creation and dispatches by unique Action ID through a concurrent map, without runtime stubbing or Mockito resets. Production code and behavioral assertions are unchanged. Verification: full server suite 207/207 passed; focused Actions 8/8 passed; Checkstyle passed. Concurrent stress with 10,000 responses and 100,000 background calls passed. Audit rounds 5 and 6 found no Critical. The new SDK Java CI run completed successfully, including the previously failing Hosted process fault gates / MySQL 8.4 / Java 21 check. |
|
Merged current main in c11c1bd and resolved the bilingual design conflict while preserving both changes. Moved the Actions migration to V23 because tool publication recovery now occupies V22. Validation passed: build, typecheck, bundle, Checkstyle, all 239 managed-agent-server unit tests, and the real Hosted Public/WebShell owner approval E2E. Two additional open-ended/reverse audit rounds found no Critical defects. |
|
Two things from the WebShell side (#12867 D6): 1. Migration version collision after merging 2. The WebShell approval UI is #13107 (draft). It only touches 中文说明WebShell 侧（#12867 的 D6）有两点： 1. 合入 2. WebShell 审批界面在 #13107（draft）。 它只改动 |
…e contract to 1.25 Merging main brought in #12946's V23__managed_mcp_records.sql next to this PR's V23__managed_actions.sql; Flyway refuses two migrations with the same version, so the server would not start. The Actions migration becomes V24 with its content unchanged. The contract takes 1.25.0 on top of main's 1.24.0 (#12946), with a v1.25 note for the Actions routes this PR serves.
Picks up #13101's renumbering of the Actions migration to V24. The branch still carried V23__managed_actions.sql, which collides with main's V23__managed_mcp_records.sql in CI's merge with main.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Read the D6b path end to end at 358212a8 — admission → durable operation → worker → Harness resolve → journal projection → completion — plus the authorization surface, the V24 migration, and the generated client.
What I verified: regenerating managed-agent-api.ts from this PR's OpenAPI (npm run generate:managed-agent-api) produces a clean diff, so the generated types match the contract. CI is green on this head (Java 11/17/21, Hosted MySQL 8.4, the MariaDB job, Checkstyle, Lint & Static). I couldn't run the Java suite locally (no JDK/Maven on this machine), so everything below is a source read at the pinned SHA.
Both of your points from the 12:29 comment are addressed at the head: the Actions migration is now V24__managed_actions.sql and the contract is 1.25.0. The migration content itself looks right.
The main flow reads as designed, and the owner gate plus the replay/conflict handling in ActionResponseCoordinator hold up — I found no injection, replay-flip, or re-decision path. Two things I'd tighten before merge:
ManagedActionsTestnever actually runs against MariaDB — its datasource keys offd6b.mysql.*, which nothing in the repo sets, so the DB matrix claimed in the design doc isn't exercised by CI.QwenHostedHarnessConnector's 3-arg constructor silently disables the approval-mode pin and its confirmation guard.
Everything else is non-blocking and inline. One inline note is a question rather than a finding: the projection's duplicate-record guard, where I couldn't construct a reachable path but the sibling projection on the same call site behaves differently.
|
|
||
| @SpringBootTest( | ||
| properties = { | ||
| "spring.datasource.url=${d6b.mysql.url:jdbc:h2:mem:managed-actions;MODE=MySQL;DB_CLOSE_DELAY=-1;DATABASE_TO_LOWER=TRUE}", |
There was a problem hiding this comment.
Non-blocking, but the claim needs fixing. This datasource only leaves H2 if d6b.mysql.url is set, and nothing in the repo ever sets it. Every other MySQL IT in this module keys off mysql.url (HostedHarnessMySqlIT:94, ToolPublicationRecoveryMySqlIT:30), and the mysql-integration job passes only -Dmysql.url / -Dmysql.user / -Dmysql.password — so this test takes the H2 branch even in that job, and design §7's "projection and route tests on H2 and MariaDB (MySQL driver)" isn't reproduced by CI.
The V24 DDL does still run against real MySQL 8.4 through the Hosted ITs, so this is a vacuous verification claim rather than an unvalidated migration. The module convention (${mysql.url:jdbc:h2:...} + driver-class-name) would let the MariaDB leg actually be switched on, and asserting the database product would make a mis-wire fail loudly instead of silently falling back.
There was a problem hiding this comment.
Confirmed at 358212a: the CI matrix passes mysql.* and this class reads d6b.mysql., so a green MariaDB job must not be counted as this Action suite running on MariaDB. The earlier six-test MariaDB result was a separate manually configured local run, as qualified in the PR report; the final strict-validation evidence is H2. Deferred: wire this suite into the shared mysql. matrix and assert the database product. Per the established after-round-5 policy, this non-blocking coverage improvement is a follow-up rather than a change to this PR.
There was a problem hiding this comment.
I did not change this one, on purpose. Wiring it to the shared mysql.url would point this test at the same managed_agent_test database the rest of the mysql-integration job uses, and the H2 default is an isolated in-memory DB — two Spring contexts with different datasource properties against one schema is exactly the kind of interference I cannot check without a JVM. Dropping the forced org.h2.Driver default is also required before -Dmysql.url could work at all, since Boot only infers the driver when driver-class-name is absent. Leaving the call to you: either keep the opt-in property and soften the design §7 wording to say the MariaDB leg is manual, or wire it into a job with its own database.
|
|
||
| public QwenHostedHarnessConnector(ManagedAgentProperties properties, | ||
| AgentStateStore sessions, WorkspaceExecutionStore workspaceExecution) { | ||
| this(properties, sessions, workspaceExecution, null); |
There was a problem hiding this comment.
The 3-arg constructor now passes null for the new actions dependency, and every new use site tolerates that by degrading silently: create() then sends the deployment default mode instead of the mode pinned on the Session row (253-259), and the confirmation guard at 118 is skipped entirely. So a Session pinned default/auto-edit can be created with yolo and never verified against what the Harness echoed back — the exact case design §6.4 declares fail-closed. No error, no log.
Today only the 4-arg constructor is used in production wiring (HarnessConfiguration:26), so this is latent rather than live. Might be worth dropping the null-tolerant overload, or failing loudly for a Workspace-bound Session, so an un-wired store can't quietly disable the pin.
There was a problem hiding this comment.
Confirmed the nullable overload is unsafe if used for a Workspace-bound Session. I checked all constructor call sites at 358212a: production HarnessConfiguration uses the four-argument constructor with ManagedActionStore; the three-argument overload is used only in tests. The live path reads the pinned mode and verifies the Harness echo, so I found no currently reachable production bypass. Deferred: remove the nullable overload or reject Workspace use without the store. This remains a non-blocking wiring improvement under the after-round-5 Critical-only policy.
There was a problem hiding this comment.
Fixed in 7ab2952: createOrLoad now throws when a Workspace-bound Session has no Managed Action store, so neither the mode pin nor the Harness confirmation can be skipped. With that guard in place the two actions != null checks were dead, so they are gone. Both tests that use the 3-arg constructor build non-Workspace sessions (SessionRecord compact constructor → workspace = null), so they are unaffected.
|
|
||
| private boolean hasActions(SessionRecord session) { | ||
| return session.workspace() != null | ||
| && actions != null |
There was a problem hiding this comment.
hasActions issues its own SELECT approval_mode for every Session it renders, and it's called from both publicSession (503) and webShellSession (525) — so the Session list endpoints now add N queries for a column of the row the store already loaded. Worth carrying approval_mode in the session projection and deriving the capability from that.
There was a problem hiding this comment.
Confirmed: hasActions adds an approval_mode lookup per Workspace Session rendered. This is a non-blocking query-efficiency suggestion. Deferred: carry the pinned mode through the existing Session projection and derive the capability without the extra lookup. Recorded here for follow-up under the after-round-5 Critical-only policy.
| previous == null | ||
| ? "requested".equals(state) | ||
| : previous.options().equals(options) | ||
| && "requested".equals(previous.state()) |
There was a problem hiding this comment.
Question rather than a finding — the projection isn't idempotent: a requested record re-applied for an action that already exists fails this guard, and since require (538) throws a 400 and apply runs before the journal_tx insert (ManagedSessionStore:348 vs :361), the whole commit is rejected — and per design §5.4 a failed journal write also blocks the Session's later writes.
I could not construct a reachable producer for that branch: a retry on the same command key short-circuits in replay() (ManagedSessionStore:315), a different command key carrying the same content fails validateHead first, and the harness mints a fresh tool_approval_<32hex> per request (hosted-workspace-tool-turn.ts:1378). So it may be a purely defensive assertion. But the sibling Stage H projection on the same call site does tolerate a re-applied record (ManagedExtensionRecordStore:98, the applied/shaped bookkeeping), so the asymmetry is worth a second look. Could you confirm whether a duplicate action.changed record can reach apply? If it can, this should be a silent no-op instead of a 400.
There was a problem hiding this comment.
Checked the current commit end to end. An identical command retry finds its existing transaction and returns replay before actions.apply. A different command replaying the old sequence fails validateHead before projection. A new requested event reusing an existing Action ID is not a valid producer transition: Hosted creates a fresh ID per request, and the Action lifecycle is requested followed by one terminal transition. I found no reachable valid duplicate that should be accepted here. Keeping the guard prevents a malformed new transaction from resetting an existing Action; no change needed for this question.
| id); | ||
| } | ||
| if (jdbc.queryForObject( | ||
| "SELECT COUNT(*) FROM managed_agent_session WHERE tenant_id = ? AND" |
There was a problem hiding this comment.
This COUNT(*) runs after the INSERT/UPDATE above and silently skips the action.updated event when the count isn't 1, so that path leaves a persisted Action row with no public event and no log. The caller has already validated the head row for this session (commitJournal → validateHead), so the query is redundant on the happy path and only guards a half-applied state on the other one. Dropping it (or failing loudly instead of skipping) would remove both.
There was a problem hiding this comment.
Confirmed the conditional event append and the redundant query on the normal product Session path. I found no demonstrated live product path that loses an Action event: projection is inside the journal commit transaction, and normal Workspace Actions have the product Session row. Deferred: clarify the journal-only case and replace the conditional guard with an explicit invariant where appropriate. This is a non-blocking simplification under the after-round-5 Critical-only policy.
There was a problem hiding this comment.
Withdrawing this one — I was wrong about the caller. appendPublicEventIfAbsent (ManagedAgentStore:1643) starts with requireSessionForUpdate, which throws when there is no managed_agent_session row, and that projection table is not the qwen_managed_session_journal_head row the caller validated. So without the COUNT(*) a journal commit for a Session that is not projected yet would fail the whole commit instead of skipping the event. The guard is load-bearing; no change made.
| sessionId); | ||
| } | ||
|
|
||
| public void requireOwner(String tenantId, String sessionId, String actorId) { |
There was a problem hiding this comment.
requireOwner calls actorKey without the IllegalArgumentException catch that the sibling call sites use to answer 403 actor_scope_mismatch (ManagedWorkspaceRegistry:36-39 and 141-145), so an actor scope that WorkspaceActor rejects would surface here as an unmapped 500-class error instead of the contract's 403. Fail-closed either way, so not a blocker — but the neighbouring routes are consistent about mapping it and this one isn't.
There was a problem hiding this comment.
Confirmed the exception mapping differs from the neighbouring registry calls. It does not grant access or bypass the owner check, but the error response should be made consistent. Deferred: map an invalid actor scope to the declared 403 and cover that path. Recorded as a non-blocking follow-up under the after-round-5 Critical-only policy.
There was a problem hiding this comment.
Fixed in 7ab2952: the actor key is resolved first, and an IllegalArgumentException now answers 403 actor_scope_mismatch, matching ManagedWorkspaceRegistry:141-145. An unknown or absent creator still answers action_forbidden.
| List.of( | ||
| "inputRevision", | ||
| "policyRevision", | ||
| "createdAt", |
There was a problem hiding this comment.
createdAt/expiresAt are copied straight from the Harness optionsRef, i.e. epoch milliseconds, while the public Session representation divides by 1000 (ManagedAgentService:487-488) and the WebShell Session view does not (517). The public API therefore mixes seconds and milliseconds across resources: a client that follows the Session convention to compute a countdown from expires_at is off by 1000×, and expiry is exactly what a responder consults. Worth pinning the unit in the contract (or naming the fields ..._ms) and asserting it in the contract test.
There was a problem hiding this comment.
Confirmed: these Action fields expose the original Harness epoch milliseconds on both surfaces, while public Session timestamps use seconds. The unit is insufficiently explicit in the public contract. I found no current Action UI/countdown consumer in this slice. Deferred: explicitly pin the Action timestamp unit in the contract and assert it, considering compatibility before renaming or converting fields. Recorded under the after-round-5 Critical-only policy; this PR does not silently change existing units.
| private ManagedActionService actions; | ||
|
|
||
| @Autowired | ||
| void setActions(ManagedActionService actions) { |
There was a problem hiding this comment.
The ACTION_RESPONSE operation envelope is built here through a setter-injected back-reference (117-131) while ManagedActionService also constructs its own PublicCommandOperation/WebShellCommandOperation (130-172). So the wire shape and the web ? "x_y" : "xy" naming rule now live in two places, and a SessionLifecycleService constructed without the setter NPEs on the ACTION_RESPONSE path instead of failing at wiring time. Keeping a single projection builder would remove both.
There was a problem hiding this comment.
I checked the current code: SessionLifecycleService delegates ACTION_RESPONSE to ManagedActionService.publicOperation/webOperation; the Action wire envelope itself is built there, rather than duplicated in the lifecycle service. The remaining concern is valid: constructing the lifecycle service manually without its setter leaves that branch unwired. Production Spring wiring supplies the dependency. Deferred: simplify dependency wiring while retaining a single Action projection builder. No live production defect found; retained as a non-blocking follow-up under the after-round-5 policy.
| .header(TENANT, tenant).principal(actor).header(IDEMPOTENCY_KEY, "contract-answer"), body); | ||
| exchange(drift, "respondWebShellAction", 202, post(WEB_SHELL + "/actions/respond").header(TENANT, tenant).principal(actor), | ||
| "{\"sessionId\":\"%s\",\"actionId\":\"%s\",\"requestId\":\"action-trace\",\"idempotencyKey\":\"contract-answer\",\"response\":{\"kind\":\"permission\",\"inputRevision\":1,\"policyRevision\":\"hosted-tool-approval/1\",\"optionId\":\"allow\"}}".formatted(session, journal.id)); | ||
| journal.change("cancelled", null); |
There was a problem hiding this comment.
action_cancelled is produced here but never asserted. endedCode maps cancelled → action_cancelled (ManagedActionStore:470-475), and the suite only checks action_expired (ManagedActionsTest:367, :417) and action_already_resolved (:309, :475). The D6a path cancels an Action when the Turn is aborted, so a response racing that cancellation is reachable; one assertion that it completes with failure_code/failureCode action_cancelled and returns no resolution would cover the third terminal state.
There was a problem hiding this comment.
Confirmed the cancellation fixture does not assert the resulting failure envelope. The implementation maps cancelled to action_cancelled; the comment identifies missing regression coverage rather than a demonstrated incorrect result. Deferred: assert failure_code/failureCode=action_cancelled and no resolution on both surfaces. Recorded as a non-blocking test follow-up under the after-round-5 Critical-only policy.
There was a problem hiding this comment.
Added in 7ab2952 as cancelledActionsFailTheResponseWithActionCancelled, mirroring the expiry test: delivery keeps failing, the journal then records cancelled, and the operation must end FAILED with failure_code action_cancelled and no action_resolution.
| `action.updated` Session event. The table | ||
| needs a Flyway migration; open pull requests hold V19 to V21, so its number is | ||
| settled at merge. | ||
| uses Flyway migration V23; tool publication uses V20–V22. |
There was a problem hiding this comment.
This still says the Actions table "uses Flyway migration V23" (same at ...zh-CN.md:121), and the PR body says "Flyway V23 adds the Action projection" — but after the renumber the migration is V24__managed_actions.sql, because V23 is held by #12946's managed_mcp_records. Small drift, but this is the section a future reader uses to reason about the migration chain.
There was a problem hiding this comment.
Confirmed: the current migration is V24__managed_actions.sql, with V23 occupied by managed_mcp_records. I corrected the PR description to V24. The stale V23 sentence in both linked design documents is recorded here as a deferred documentation correction under the after-round-5 Critical-only policy; the migration chain itself is already correct.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed 358212a8b2ac661bdfa960533e4de395fb05e7a4 across all 32 changed files and the downstream Hosted authority, Workspace admission, journal store, operation readers and delivery workers. No blocking correctness or security defects found after the initial review and independent reverse passes.
Verified the creator-only response gate and tenant/read isolation, strict original-revision/option validation, actor-scoped cross-surface idempotency, transactional Action/event projection, deterministic decision reconciliation after lost replies, competing responses, lease-generation fencing, and approval-mode pinning with fail-closed Harness confirmation. V24 and OpenAPI 1.25.0 address the earlier migration/version collision.
Fresh validation at this commit:
- 92 Java tests passed, with zero failures, errors or skipped tests: 19 Hosted SDK tests, 17 focused Action/contract/config/connector tests, 55 related lifecycle/Workspace/server regression tests, and one real Hosted integration test. The SDK and Runtime Broker artifacts were rebuilt from this commit.
- The real Hosted Harness and Runtime worker completed four owner approvals across public and WebShell APIs, rejected a reader with 403, replayed the original operation, reconciled committed decisions, performed the expected file operations, and avoided duplicate physical execution.
npm run build,npm run bundleandnpm run typecheckpassed; the tracked checkout remained clean.
I also rechecked the newly posted review threads. The nullable connector overload is currently used only by tests; production wiring supplies the Action store and enforces the pin. Same-command journal retries return through the replay path before Action projection; I found no reachable valid duplicate-request transition that needs to be accepted. The existing test-coverage, wiring, query and documentation suggestions remain non-blocking and are not duplicated here.
Local database evidence is H2 with a deterministic local model, not fresh MySQL/MariaDB verification. In particular, the focused Action test's d6b.mysql.* properties are not driven by the current MariaDB CI job; its green result must not be counted as that test running on MariaDB. Pending-approval restart recovery, questions/votes and the WebShell approval UI remain the documented follow-up scope. CI has no failed checks at this review; WebShell smoke and automated review were still running.
|
@qwen-code /triage |
A Workspace Session whose connector was built without the Managed Action store silently fell back to the deployment approval mode and skipped the Harness mode confirmation -- the case the design declares fail-closed. createOrLoad now refuses it, so the two remaining null checks in the connector are gone and an un-wired store fails loudly at admission. requireOwner let an IllegalArgumentException from actorKey escape as an unmapped 500. It now answers 403 actor_scope_mismatch, matching the neighbouring workspace routes. endedCode maps cancelled to action_cancelled but no test asserted it: a cancelled Action now fails its response operation with that code and returns no resolution. The design docs still named the Actions migration V23 after it moved to V24; V23 belongs to the Session MCP catalog (#12946).
|
Pushed
I also withdrew one of my own findings inline: the Verification caveat: there is no JDK or Maven on the machine I reviewed from, so none of this was compiled or run locally. The changes are small and mirror existing patterns, but the Java matrix is the only thing that can confirm them — worth watching before merge. Left untouched, with reasons in the inline threads: the |
|
@qwen-code /triage |
|
Follow-up to my review, after reading your ten replies — three corrections and one piece of context. Context on Corrections to my own review:
Closed by your analysis: the projection's duplicate-record guard. Your producer walk matches what I could not construct either — same command key returns at Still open as follow-ups, per your notes: the |
Real-environment verification at
|
| Area | Result |
|---|---|
| Reviewer Test Plan, Session created through the public API | 41/41 |
| Reviewer Test Plan, Session created through WebShell | 41/41 |
| Deny, mixed batch, allow-vs-deny races, 20 identical requests, request validation | 44/44 |
| Expiry with an 8 s timeout: nobody answers, three calls in a row, answer delivered late | 12/12 |
auto-edit and yolo file Turns |
8/8 and 8/8 |
Delivery faults: lost reply, four failed deliveries, 503 twice, 400 |
15/15 |
action.updated on the public and WebShell event streams |
5/5 |
Database created by main 3a8fd11711, upgraded V23 → V24, then the test plan |
8/8 and 41/41 |
| Startup validation, 13 mode and timeout combinations | as documented |
| Real model: allow through the public API, deny through WebShell | 4/4 |
| 1,626 recorded live responses against the shipped OpenAPI 1.25.0 document | all valid |
| Unit suite on JDK 21 | 251/251 |
HostedPublicWorkspaceIT on local MySQL 8.4.7 |
2/2 |
The description lists deny, auto-edit, Oracle MySQL and restart recovery as not validated. The first three are covered above with real processes (MySQL 8.4.7 here, 8.4.6 in CI). Restarts are F1.
What the test plan asks for, as measured:
- Both surfaces. The pending approval has the same id on the public API and WebShell, and the Session reports
capabilities.actions: trueon both. Nothing runs while it waits (file absent, 0 tool executions, Turnrunning). - Who may answer.
bobandcarolget403 action_forbiddenon both surfaces.mallory, a request without an actor and another tenant get404 session_not_found. None of them creates an operation. - Idempotency. The same key returns the same operation on the same surface and on the other one. The same key with
denyinstead ofallowanswers409 idempotency_conflict. 20 concurrent identical requests produce one operation. - Completion. The operation completes with
action_resolution.outcome: decidedand a decision receipt, the Turn finishes, and the file holds the edited content after 3 tool executions and 4 model calls. - Terminal state. The decided Action stays readable with its receipt and leaves the pending list. A new answer gets
409 action_already_resolved. The original key still returns the original operation. - Lost reply. The proxy let the Harness record the decision and dropped its
200. The operation completeddecidedfrom the committed journal record, with one call to the Harness and no retry. - Temporary failures. Four dropped deliveries were retried after about 2, 2, 4 and 8 s. The Session stayed
ACTIVEand the TurnRUNNINGthroughout, then the operation completed.
Beyond the test plan:
- Deny. The write never runs, the model gets "The Session owner denied this tool call" and the Turn completes. With the real model the deny produced one approval and no retry.
- Mixed batch. Two calls in one assistant message are asked one at a time. Allowing the first and denying the second runs exactly one.
- Races. Allow and deny sent together under different keys: one is recorded, the other operation ends
failedwithaction_already_resolved, and the file system matches the winner in 3 of 3 runs. - Expiry. The Turn ends by itself 9.9 s after creation with an 8 s timeout. With three sequential writes only the first is asked about and the Turn ends after one timeout. An answer admitted in time but delivered 10 s late ends
failedwithaction_expired, and the call does not run. - Migration. At
c11c1bdf3cI hit the V23 collision independently: git merged cleanly and the merged server refused to start withFound more than one migration with version 23. The rename in57b9960ca2fixes it. A fresh database goes 22 → 23 → 24, and a database created bymainupgrades in place with its Sessions left atyolo. 7ab2952507. This commit was pushed from a machine without a JDK, so I built it. It compiles, the unit suite is 251/251 including the newcancelledActionsFailTheResponseWithActionCancelled, the Hosted IT is 2/2 and the whole real-stack sequence passes on it. The firstubuntu-latest / Java 11run was red because of the runner:maven-javadoc-plugin … zip END header not foundin "Build release artifacts". That step passes locally on JDK 11.0.32, and the re-run is green.
F1: a Java server restart strands a pending approval
This is the "restart recovery of pending approvals" that the description lists as not validated. I restarted only the Java server and left the Harness process running.
After the restart the approval is still listed on both surfaces and the owner's allow is accepted with 202. It is never delivered. The connector has to re-attach with POST /session/:id/load, and the Harness that still holds the Session answers 409 hosted_session_already_attached every time. The resolve route is never reached. A Session created after the restart on another Workspace is asked, answered and completed by the same Harness.
| During the outage | What happens | Runs |
|---|---|---|
| The Harness writes nothing to the Store (outages of 4 to 19 s) | The Harness expires the Action at its expiry time. The operation ends failed with action_expired at its next retry, 5 to 26 s later. |
5 |
| A Store write from the Harness fails (outages of 23 to 55 s) | The Session becomes recovery-blocked. The Action stays requested and listed, and the operation stays running. |
5 |
Both outcomes fail closed: the allowed call never ran in any run. In both, the Java Turn stays running and new Sessions on that Workspace fail with hosted_turn_failed.
With the default 10 minute timeout, the answer was accepted 4 s after the approval appeared and the operation ended failed with action_expired 608 s later, after 15 attempts.
This is inherited. On main 3a8fd11711 in yolo, a Turn that is waiting for the model when the server restarts ends the same way: load answers 409, the Java Turn stays RUNNING and the Workspace stays busy. What D6b changes is the exposure. A Turn now waits for a person for up to the approval timeout, so a deploy is far more likely to land inside one.
Not a blocker. Suggestions:
- Section 8 of the design names only a Harness restart. Add the Java server restart, with the advice to drain pending approvals before a deploy and keep the timeout short.
- Track re-attaching after a server restart under Stage G (feat(managed-agent): Stage G authoritative Session history, writer fencing and takeover #12952).
F2: the owner cannot see what the call will do
The Action carries the tool name and the function call id. No client-visible surface shows the arguments, before or after the answer. I checked the Action detail on both surfaces, the Session Items, the events, the WebShell transcript and the Turn detail. While the Turn waits, Items hold one entry, the user message.
The design says the assistant message is committed first "so a client can show the pending calls from the Session's Items" (5.2) and "Arguments come from Items" (6.3). The message is in the journal, but the Java projection does not expose tool calls. So today an owner approves write_file without knowing the path or the content.
Not a blocker for the API in this PR. It matters before default is offered to people, and for #13107, whose card takes the arguments from the tool row. Either project the pending call into Items, or correct the two design sentences until that lands.
Smaller notes
- Harness that does not report
approvalMode. With a Harness bundle from before D6a, or a proxy that strips the field, every Workspace Session fails after about 35 s withhosted_harness_unavailable. Nothing runs, which is the intended fail-closed behaviour fordefault. It also happens inyolo, where the old Harness would behave correctly, so the Harness has to be upgraded before or with the Java server. An optional 6-line candidate accepts a missing mode only for a Session pinned toyolo. With it ayoloSession completes on the old Harness in 1.8 s,defaultstill refuses, the test plan is 41/41 and the unit suite is 251/251. - Workspace held while waiting. A second Session on the same Workspace is accepted with
202and its Turn fails 1 to 2 s later withhosted_turn_failed. The Workspace serves new Sessions again once the first Turn ends. This is the documented trade-off, with a number on it. - No other way out of a waiting Turn. WebShell
turns/cancel, a second message, close and delete all answer409 workspace_unavailablefor a bound Session. The owner's answer or the expiry ends the wait, soaction_cancelledcannot be produced through Java today. - Timestamps. Live data confirm the open thread: Action
created_atandexpires_atare milliseconds, thecreated_atof the same Session's public events is seconds.
Tests
39 single-edit mutants of the PR's Java code:
| Layer | Killed |
|---|---|
| Unit suite | 21 |
HostedPublicWorkspaceIT, on the survivors |
3 more |
| 8 candidate unit tests | 10 more |
| Left | 5, two of them equivalent |
Survivors of the unit suite that the candidate tests cover:
- the
action.updatedevent is appended - a
questionresponse is refused - an ended Action cannot change again, and a known Action's options cannot change
- an expired worker lease is reclaimed
- a Harness
400ends the operation - an exactly full last page reports no more
- the lifecycle worker never sees Action responses
The last one guards real behaviour. The lifecycle worker closes the Session of every operation it claims, and only the operation_kind <> 'ACTION_RESPONSE' filter keeps it away. No test in the PR fails when that filter is removed.
The candidate class passes Checkstyle and runs 8/8 at 7ab2952507. I offer it as a follow-up, not a condition for merging.
Not covered
- A real-stack run on Linux or Windows. CI runs the Hosted IT on ubuntu with MySQL 8.4.6.
- MariaDB under the real stack.
- The WebShell approval UI (feat(web-shell): show and answer Hosted tool approvals in the Managed panel #13107).
- Approvals for the shell profile, which Java does not select.
- More than one Java server.
- Question Actions.
Evidence
Probe scripts, transcripts for all three heads, the mutation runs and the candidates are in pr13101/ at 2f14a1eaa7.
1. Reviewer Test Plan, 41/41 on each surface
2. Deny, mixed batch, races, validation
3. Expiry, auto-edit, yolo, startup validation
4. Delivery faults, event streams, busy Workspace
5. F1: Java server restart with a pending approval
6. Migration collision at the earlier head, upgrade from main at this head
7. Suites, contract conformance, mutation testing
8. F2: visibility of the call; Harness without approvalMode
9. Real model round trip
中文版本
真实环境验证（head 7ab2952507）
结论：可以合并。 PR 描述里的每条主张都在真实栈上复现，单元测试与 Hosted 测试在本地和 CI 均通过，没有发现回归。在对真实用户开启 default 或 auto-edit 之前，有两个限制值得知道（下文 F1、F2）。两者要么继承自 main，要么属于后续工作，而默认模式仍是 yolo。
验证期间 head 动了两次（c11c1bdf3c → 358212a8b2 → 7ab2952507），每个 head 我都把整套流程重跑了一遍。下面的数字除特别标注外都来自 7ab2952507。
跑了什么
- 由该 head 构建的服务端 fat jar（JDK 21.0.12），内嵌 Runtime Broker，
loader.path上挂一个可信 actor 适配器 - 打包后的 Hosted Harness（
dist/cli.js serve --profile hosted-harness，Node 24.18.1）和真实 Runtime worker，来自同一棵源码树 - MySQL 8.4.7（原生、UTC），macOS arm64
- 脚本化的 OpenAI 兼容模型，用于产生确定的工具调用；另有一轮使用真实模型（qwen3.8-max）
- 服务端与 Harness 之间的记录代理，可丢弃、延迟或替换某次调用
- 四个 actor：
alice创建 Session，bob只读，carol是同一 Workspace 的另一个创建者，mallory无权限
结果
| 范围 | 结果 |
|---|---|
| Reviewer Test Plan，经公共 API 创建 Session | 41/41 |
| Reviewer Test Plan，经 WebShell 创建 Session | 41/41 |
| 拒绝、混合批次、allow 与 deny 竞争、20 个相同请求、请求校验 | 44/44 |
| 8 秒超时下的过期：无人回答、连续三次调用、回答迟到 | 12/12 |
auto-edit 与 yolo 的文件 Turn |
8/8、8/8 |
投递故障：应答丢失、四次投递失败、两次 503、一次 400 |
15/15 |
公共与 WebShell 事件流上的 action.updated |
5/5 |
由 main 3a8fd11711 建库，升级 V23 → V24，再跑测试计划 |
8/8、41/41 |
| 启动校验，13 种模式与超时组合 | 与文档一致 |
| 真实模型：公共 API 回答 allow，WebShell 回答 deny | 4/4 |
| 1,626 条真实响应对照随包发布的 OpenAPI 1.25.0 文档 | 全部合规 |
| 单元测试，JDK 21 | 251/251 |
HostedPublicWorkspaceIT，本地 MySQL 8.4.7 |
2/2 |
PR 描述把 deny、auto-edit、Oracle MySQL 和重启恢复列为未验证。前三项已在上表用真实进程覆盖（本地 MySQL 8.4.7，CI 为 8.4.6）。重启见 F1。
测试计划要求的各项，实测如下：
- 两个入口。 待审批请求在公共 API 与 WebShell 上 id 相同，Session 在两边都报告
capabilities.actions: true。等待期间没有任何执行（文件不存在，工具执行 0 次，Turn 为running）。 - 谁能回答。
bob和carol在两个入口都得到403 action_forbidden。mallory、不带 actor 的请求、其他租户得到404 session_not_found。这些请求都没有产生 operation。 - 幂等。 同一个键在同一入口、换一个入口都返回同一个 operation。同一个键把
allow换成deny得到409 idempotency_conflict。20 个并发相同请求只产生一个 operation。 - 完成。 operation 以
action_resolution.outcome: decided和决定回执完成，Turn 结束，文件是编辑后的内容，共 3 次工具执行、4 次模型调用。 - 终态。 已决定的 Action 仍可读取并带回执，同时从待审批列表消失。新的回答得到
409 action_already_resolved。原来的键仍返回原 operation。 - 应答丢失。 代理让 Harness 记录了决定，再丢掉它的
200。operation 依据已提交的 journal 记录以decided完成，只调用了 Harness 一次，没有重试。 - 临时失败。 四次被丢弃的投递分别在约 2、2、4、8 秒后重试。期间 Session 一直是
ACTIVE，Turn 一直是RUNNING，之后 operation 完成。
测试计划之外：
- 拒绝。 写入没有执行，模型收到"The Session owner denied this tool call"，Turn 完成。真实模型被拒绝后只产生一次审批，没有重试。
- 混合批次。 同一条 assistant 消息里的两个调用逐个询问。允许第一个、拒绝第二个，只执行了一个。
- 竞争。 用不同的键同时发 allow 和 deny：只记录一个，另一个 operation 以
failed、action_already_resolved结束，文件系统状态与胜者一致，3 次都如此。 - 过期。 8 秒超时下，Turn 在创建后 9.9 秒自行结束。连续三次写入只询问第一次，Turn 在一个超时后结束。及时受理但迟到 10 秒才送达的回答以
failed、action_expired结束，调用不执行。 - 迁移。 在
c11c1bdf3c上我独立撞到了 V23 冲突：git 合并无冲突，合并后的服务端拒绝启动，报Found more than one migration with version 23。57b9960ca2的改名修复了它。新库按 22 → 23 → 24 迁移，由main建的库可原地升级，已有 Session 保持yolo。 7ab2952507。 这个提交是从一台没有 JDK 的机器推送的，所以我构建了它。它能编译，单元测试 251/251（包含新增的cancelledActionsFailTheResponseWithActionCancelled），Hosted IT 2/2，整套真实栈流程在它上面通过。ubuntu-latest / Java 11第一次变红是 runner 的问题："Build release artifacts" 步骤里maven-javadoc-plugin … zip END header not found。同一步骤在本地 JDK 11.0.32 上通过，重跑已变绿。
F1：重启 Java 服务端会搁浅待审批请求
这就是 PR 描述里列为未验证的"待审批请求的重启恢复"。我只重启 Java 服务端，Harness 进程保持运行。
重启后，审批仍在两个入口的列表里，owner 的 allow 也被 202 受理，但永远送不到。connector 必须用 POST /session/:id/load 重新挂接，而仍持有该 Session 的 Harness 每次都回答 409 hosted_session_already_attached，resolve 路由从未被调用。重启后在另一个 Workspace 上新建的 Session，由同一个 Harness 正常询问、回答并完成。
| 停机期间 | 结果 | 次数 |
|---|---|---|
| Harness 没有向 Store 写入（停机 4 到 19 秒） | Harness 在过期时间把 Action 置为过期。operation 在下一次重试时以 failed、action_expired 结束，晚 5 到 26 秒。 |
5 |
| Harness 的一次 Store 写入失败（停机 23 到 55 秒） | Session 进入 recovery-blocked。Action 保持 requested 并留在列表里，operation 保持 running。 |
5 |
两种结果都是失败关闭：所有运行里，被允许的调用都没有执行。两种情况下 Java 侧的 Turn 都停在 running，该 Workspace 上的新 Session 以 hosted_turn_failed 失败。
在默认的 10 分钟超时下，回答在审批出现 4 秒后被受理，operation 在 608 秒后以 failed、action_expired 结束，共尝试 15 次。
这是继承来的问题。在 main 3a8fd11711 的 yolo 模式下，服务端重启时正在等模型的 Turn 结局相同：load 回答 409，Java 侧 Turn 停在 RUNNING，Workspace 一直被占用。D6b 改变的是暴露面：Turn 现在会等人，最长等到审批超时，一次发布落在 Turn 中间的概率大了很多。
不阻塞合并。建议：
- 设计文档第 8 节只写了 Harness 重启。补上 Java 服务端重启，并建议发布前先清空待审批请求、超时设短一些。
- 服务端重启后的重新挂接放到 Stage G（feat(managed-agent): Stage G authoritative Session history, writer fencing and takeover #12952）跟踪。
F2：owner 看不到这次调用要做什么
Action 带有工具名和 function call id。没有任何客户端可见的入口展示参数，回答前后都是如此。我检查了两个入口的 Action 详情、Session Items、事件、WebShell transcript 和 Turn 详情。Turn 等待期间 Items 只有一条，即用户消息。
设计文档说先提交 assistant 消息，"这样客户端可以从 Session 的 Items 展示待处理的调用"（5.2），以及"参数来自 Items"（6.3）。消息确实在 journal 里，但 Java 的投影没有把工具调用暴露出来。所以现在 owner 批准 write_file 时，并不知道路径和内容。
对本 PR 的 API 不构成阻塞。在把 default 提供给用户之前需要解决，也影响 #13107，它的卡片从工具行取参数。要么把待处理的调用投影进 Items，要么在那之前先修正设计文档里的这两句话。
其他说明
- 不回报
approvalMode的 Harness。 换成 D6a 之前构建的 Harness，或用代理去掉该字段，所有 Workspace Session 都在约 35 秒后以hosted_harness_unavailable失败。没有任何执行，这对default是预期的失败关闭。但yolo下也会如此，而旧 Harness 在yolo下的行为本来是正确的，所以 Harness 必须先于 Java 服务端升级，或与它一起升级。附了一个可选的 6 行候选：只对固定为yolo的 Session 接受缺失的模式。打上之后，yoloSession 在旧 Harness 上 1.8 秒完成，default仍然拒绝，测试计划 41/41，单元测试 251/251。 - 等待期间 Workspace 被占用。 同一 Workspace 上的第二个 Session 被
202受理，1 到 2 秒后它的 Turn 以hosted_turn_failed失败。第一个 Turn 结束后，该 Workspace 又能服务新 Session。这是文档里已说明的取舍，这里给出了实测数字。 - 等待中的 Turn 没有别的出口。 对绑定 Workspace 的 Session，WebShell
turns/cancel、第二条消息、close、delete 都回答409 workspace_unavailable。只有 owner 的回答或过期能结束等待，所以目前经 Java 产生不了action_cancelled。 - 时间戳。 实测数据印证了那条未关闭的评审意见：Action 的
created_at、expires_at是毫秒，同一 Session 公共事件的created_at是秒。
测试
对 PR 的 Java 代码做了 39 个单点变异：
| 层 | 杀死 |
|---|---|
| 单元测试 | 21 |
HostedPublicWorkspaceIT（针对存活者） |
再 3 个 |
| 8 个候选单元测试 | 再 10 个 |
| 剩余 | 5 个，其中 2 个等价 |
候选测试覆盖的单元测试存活变异：
- 追加了
action.updated事件 question类回答被拒绝- 已结束的 Action 不能再变，已知 Action 的选项不能变
- 过期的 worker 租约会被回收
- Harness 返回
400会结束 operation - 恰好满页的最后一页报告没有更多
- 生命周期 worker 永远看不到 Action 回答
最后一条守护的是真实行为。生命周期 worker 会关闭它认领的每个 operation 所属的 Session，只有 operation_kind <> 'ACTION_RESPONSE' 这个过滤条件把它挡在外面。去掉这个过滤后，PR 里没有任何测试失败。
候选测试类通过 Checkstyle，在 7ab2952507 上 8/8 通过。我把它作为后续项提供，不作为合并条件。
未覆盖
- Linux 或 Windows 上的真实栈运行。CI 在 ubuntu + MySQL 8.4.6 上跑了 Hosted IT。
- 真实栈下的 MariaDB。
- WebShell 审批界面（feat(web-shell): show and answer Hosted tool approvals in the Managed panel #13107）。
- Shell profile 的审批，Java 目前不会选择该 profile。
- 多个 Java 服务端。
- 问题类 Action。
证据
探针脚本、三个 head 的运行记录、变异测试记录和候选补丁都在 2f14a1eaa7 的 pr13101/ 目录下。截图见上方英文部分。
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Reviewed at 7ab2952. No provable blocking correctness, security, data-loss, regression, or compatibility defect. What I checked myself at this head, rather than inheriting from the existing threads:
The two items previously graded "tighten before merge" are resolved or non-blocking in the current code. QwenHostedHarnessConnector now fails closed instead of degrading: a Workspace-bound Session constructed without a ManagedActionStore throws IllegalStateException at lines 108-109, and the pinned-mode confirmation at lines 121-123 throws when the Harness echoes a different approval mode, so the "pinned default/auto-edit created as yolo and never verified" path no longer exists. Production wiring is unaffected either way — HarnessConfiguration:26 builds the connector with the four-argument constructor that supplies the store, and createOrLoad selects the pinned mode through actions.approvalMode(...) at lines 256-260. ManagedActionStore.requireOwner (lines 58-65) now answers an invalid actor scope with actor_scope_mismatch rather than letting it escape as an unmapped 500, and it is called on the response path at line 336 before any mutation, with every read keyed by tenantId plus sessionId.
The earlier migration/version collision is genuinely closed. There is exactly one V24__managed_actions.sql in db/migration at this head, so the renumber off #12946's V23__managed_mcp_records leaves no duplicate Flyway version — the failure mode that would have broken store startup is not present.
Remaining unresolved threads are non-blocking by their own grading and are agreed follow-ups. The Action test datasource keying off d6b.mysql.* weakens a coverage claim rather than a gate: the V24 DDL is still exercised against real MySQL 8.4 through the Hosted integration tests, and the MariaDB and MySQL fault-gate lanes are green on this head, so no unvalidated migration ships. The per-Session approval_mode lookup is query efficiency, the lifecycle-service setter wiring is a construction-time concern that Spring wiring satisfies, and the projection's duplicate-record guard was closed as correct by both sides. The Action timestamp unit is the only contract-shaped item, and it affects a resource this PR introduces rather than changing any existing field's meaning, so it is not a compatibility regression; leaving it aligned with the expires_at consumer landing separately is a reasonable sequencing choice. None of these is a defect I can prove at this head, and per the review scope I am not restating them as suggestions.
CI is green on this head across the Java 11/17/21, MariaDB, MySQL 8.4 fault-gate, real-daemon E2E, Lint & Static, and WebShell lanes; only review-pr is still pending, which is not treated as a gate.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approving at 7ab2952507. CI is green across the Java 11/17/21, MariaDB, MySQL 8.4 fault-gate, real-daemon E2E, Lint and WebShell lanes; the Flyway collision is closed (single V24__managed_actions.sql after #12946's V23), and the real-stack verification above covers both surfaces, permissions, idempotency, expiry, delivery faults and the V23 → V24 upgrade. The open threads are non-blocking follow-ups. F1 (a Java server restart strands a pending approval) and F2 (arguments not visible to the approver) must be closed before default or auto-edit is offered to users; they are tracked with Stage G (#12952) and #12867.
Main's permission Actions work (#13101) and this branch's Workspace-bound later Turns both extended WebShellSessionCapabilities with a second field, so the record, its single construction site, the generated WebShell types and the cross-tenant contract assertion each need the union rather than one side. - ApiModels: the record is now (tasks, actions, workspaceTurns). - ManagedAgentService: pass both hasActions(session) and maySubmitWorkspaceTurn(session, actorId). - managed-agent-api.ts: emit actions then workspaceTurns, matching the contract's property order now that neither field is planned; actions stays required and workspaceTurns optional per the schema's required list. - ManagedAgentApiContractTest: the foreign-tenant session asserts both capabilities as false. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-conflict/jmuob3c85i1
The deferral resolved: #13101 merged and took 1.25.0, and #13117 took 1.26.0. Record this PR's contract change per the version-history convention: creator later-Turn admission and the workspaceTurns capability. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmuoob5v90j
… panel (QwenLM#13107) * feat(managed-agent): Serve durable permission Actions (D6b) * test(managed-agent): Avoid concurrent Action mock stubbing * feat(web-shell): show and answer Hosted tool approvals in the Managed panel The Managed panel passed pendingApproval={null}, so a Hosted Session waiting on a D6a approval showed nothing to answer. Build on the D6b Actions contract (QwenLM#13101): - The provider gains an optional Actions reader. The Java provider lists requested permission Actions through actions/query and answers through actions/respond with the Action's inputRevision and policyRevision and a per-Action, per-option idempotency key. The Session summary reports the actions capability only when the service does. - action.updated projects to action_updated. It carries no Turn, so the transcript skips it instead of settling the Turn being streamed. - useManagedActions re-reads pending approvals on action_updated, on a stream gap, after an expiry and after an answer. It hides an answered approval and brings it back if the answer fails. - The page maps the pending Action onto the shared approval card, with allow/deny as allow_once/reject_once so labels are localized, and attaches it to the tool row `${turnId}:${functionCallId}` so the row expands and shows its arguments. Not built, type-checked, linted or tested locally. * feat(web-shell): export the pending Action type for custom Managed providers * fix(managed-agent): renumber the Actions migration to V24 and bump the contract to 1.25 Merging main brought in QwenLM#12946's V23__managed_mcp_records.sql next to this PR's V23__managed_actions.sql; Flyway refuses two migrations with the same version, so the server would not start. The Actions migration becomes V24 with its content unchanged. The contract takes 1.25.0 on top of main's 1.24.0 (QwenLM#12946), with a v1.25 note for the Actions routes this PR serves. * fix(web-shell): memoize pending actions to keep the respond callback stable The conditional selected a fresh empty array on every render, so the useCallback dependency changed identity each time and ESLint's react-hooks/exhaustive-deps warning failed the lint gate. * style(web-shell): format the Managed approvals page test with Prettier * fix(web-shell): retry failed approval reads and name them as such A transient failure of actions/query on the first read left a pending approval invisible until a manual Refresh: summary polls do not re-read Actions, and without an Action there is no expiry timer. The page then showed "The approval answer could not be sent" although nothing was sent. - useManagedActions reports loadError and answerError separately, and retries a failed read after 2, 5 and 10 seconds before giving up. retry() reads again on demand and restarts the bound. - The page shows "Pending approvals could not be loaded." with a Retry button for a read failure, and keeps the answer message for a failed answer. - Tests cover the automatic recovery, the retry bound and the manual retry. * fix(web-shell): recover failed approval answers and bound expiry reads * fix(web-shell): Show available managed approval arguments * fix(web-shell): keep the approval card when an answer was not applied * fix(web-shell): keep Managed approvals stable across reloads and ended Actions Treat an unknown session summary as unknown rather than as no Actions capability, so a reload keeps the shown approval answerable. Drop an approval the service reports as ended instead of offering a retry, scope answer failures to the approval that failed, stop retrying client errors, restart the retry ladder whenever reads resume, and carry the arguments-unavailable notice inside the approval card. * fix(web-shell): land only the Critical R1-1 fix for Managed approvals The previous commit also carried the review's Suggestions. The PR is past its review-round budget, so only R1-1 stays here: an unknown session summary keeps the shown approval instead of reading as no Actions capability. The Suggestions move to the follow-up recorded in QwenLM#12867. * fix(web-shell): match Managed approvals to itemId-keyed tool rows Since QwenLM#13037, Managed tool rows are keyed ${turnId}:${itemId} and Java tool Items always carry an itemId, so findManagedApprovalTool no longer found the row for a pending Action: the card could not show that row's arguments or expand it. Keep the producer's call ID on the row and match on it, still within the Action's Turn. Candidate patch from wenshao's round-4 real-stack verification (R4-1). * test(web-shell): pin the page half of the Managed approval reload fix Reverting the page gate back to detail.summary?.capabilities.actions === true kept every test green (G9). Hold a reload's Session summary and assert the shown approval stays and no extra Actions read happens. Candidate test from wenshao's round-3 real-stack verification. * test(web-shell): spell canCancel in the Managed approvals fixtures The three approvals fixtures handed to `summary()` omitted `capabilities.canCancel`, which is a required member, so each one was a TS2741 that no gate in this package reports (both tsconfigs exclude `client/**/*.test.tsx` and vitest does not typecheck). They also modelled a summary the real mapper cannot emit, which always sets both flags. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmupe138l16 * fix(web-shell): keep the Harness call title on Managed approval cards `toManagedPermissionRequest` reads `tool.title` for the approval `title`, but no producer ever set it: `managedEventsToMessages` only assigned args/status/times/rawOutput, so the left branch was dead and the card description degenerated into a restatement of the heading while the Harness`s own per-call title was dropped. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmupe138l16 * fix(web-shell): stop Managed approval state outliving its Action Two lifecycle leaks in `useManagedActions`: - a successful re-read cleared `loadError` but never `answerError`, so "the answer could not be confirmed" stayed on screen after the Harness ended the Action, and then labelled the next, unrelated card. The warning now carries the Action it belongs to and is dropped once that Action is no longer pending. - a withdrawn reader cleared `pending` without resetting the retry budget, so a reader restored after the ladder was exhausted got a single attempt with no retry scheduled, contradicting the hook`s own comment. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmupe138l16 * fix(web-shell): stop retrying approval reads the service already answered R1-5: a 4xx other than 408/429 is the service's answer, not a hiccup, so a deleted Session no longer costs four guaranteed-failing /actions/query requests and a Retry that can never succeed. The classification the submit path already used moves to managed-request-error.ts and is reused, so both failure paths of the feature agree. R1-4: the hook reports whether the pending approvals were read for this Session, so a failed background re-read is named as a refresh instead of claiming nothing could be loaded next to the loaded, actionable card. R1-8: the "arguments are unavailable" caveat renders as a sibling of the approval dialog, so ToolApproval takes an extra description id and the panel describes the caveat too — otherwise a screen-reader user confirms a Hosted tool call hearing only the tool name. Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmupe138l16 * test(web-shell): pin four Managed approval paths that no test observed R1-10, four sites the review mutation-tested as unpinned: 1. the `stream_gap` arm of the re-read trigger — the durable transcript is the only route that reports a gap here, since use-managed-session breaks the live loop on a gap before merging it; the hook doc now says so. 2. the session-switch reset effect and the cross-session mask — the new test switches Session while the previous card is displayed and its failed answer is still flagged, so deleting the effect leaves the warning behind and reverting the mask leaves the stale card answerable. 3. the `pendingApproval` wiring into MessageList — the page test's mock now captures the prop, which is what keeps the turn owning the pending call from being folded away. 4. the `setAnswerError(undefined)` clear on a successful answer — the retry test now ends on the alert being gone, not just on the card. Every assertion was checked to fail with the production line it covers reverted (8/8 mutations RED). Co-authored-by: Qwen-Coder <[email protected]> Patrol-Run: qwen-pr-closeout/jmupe138l16 * fix(web-shell): isolate late Managed approval replies Ignore answer settlements once their Session or pending Action has changed, and clear or display warnings only for the Action they belong to. Pin late success and failure after Session switches and Action replacement. * fix(web-shell): drop a Managed approval the service reports as ended Answering an Action that already expired, was cancelled or was answered elsewhere returns 409 with a code the contract defines as ended, and no retry can succeed. Keep the card hidden, clear the warning and read the list again instead of offering a retry. A read that still lists the Action shows it again, so a wrong report cannot hide an approvable Action. The 403 actor_scope_mismatch is not an ended Action and keeps its handling. --------- Co-authored-by: Shaojin Wen <[email protected]> Co-authored-by: yiliang114 <[email protected]> Co-authored-by: Shaojin Wen <[email protected]> Co-authored-by: qwen-code-dev-bot <[email protected]> Co-authored-by: yiliang114 <[email protected]> Co-authored-by: Qwen-Coder <[email protected]>
4 participants
What this PR does
Implements D6b permission Actions on the public API and WebShell: list pending approvals, inspect any approval, and submit an owner response as a durable operation. Both surfaces share creator authorization, idempotency, and retries; completion follows the committed Hosted decision, including reconciliation after a lost reply. Workspace sessions can use default or auto-edit approval mode, with a pinned mode and a bounded deployment timeout.
Why it's needed
D6a can pause Hosted tool calls for approval, but the Java server had no way for a session owner to answer them. D6b completes that path so an authorized response lets the turn continue while other actors receive 403 and duplicate responses return the original operation.
Reviewer Test Plan
How to verify
Create a Workspace session with default approval mode and request a file write. Confirm that its pending approval is available through both surfaces and that the session reports Actions support. A reader who did not create the session must receive action_forbidden when answering. The creator's allow response should complete with a decided receipt and let the tool turn finish. Repeating the same response key, including through the other surface, should return the same operation; changing its content must conflict. Verify terminal detail remains readable, terminal approvals disappear from the pending list, and temporary delivery failures retry without changing session lifecycle state.
Evidence (Before & After)
Before: the six Java Action operations were planned and Workspace file execution required yolo. After: 40 focused Java tests passed, including a packaged Hosted Harness and real Runtime worker exercise covering approvals on both surfaces, reader denial, owner admission, replay, decided outcome and completed turns. Eight focused Action tests cover malformed journals, original decision validation, and cross-surface pagination. Build, typecheck, bundle and Checkstyle passed. Earlier isolated MariaDB compatibility verification passed six Action tests and the Hosted process test; final journal validation changes were verified on H2.
Tested on
Environment (optional)
Java 21, Node.js, H2 in MySQL mode, and an isolated MariaDB 10.11.19 instance using the MySQL JDBC driver. The MariaDB process and temporary data directory were removed after testing.
Risk & Scope
The English design and Chinese design are synchronized.
Linked Issues
Implements the D6b slice of #12867. Question Actions and votes remain planned.
中文说明
本 PR 的改动
在公共 API 与 WebShell 实现 D6b 权限类 Action：列出待审批请求、读取任意状态的审批，并将 owner 的回答受理为持久 operation。两个入口共享创建者授权、幂等与重试；完成结果以已提交的 Hosted 决定为准，也能在回答丢失后对账。Workspace Session 可使用 default 或 auto-edit 审批模式，模式在创建时固定，部署级超时有明确范围。
为什么需要
D6a 能暂停 Hosted 工具调用并等待审批，但 Java 服务端没有入口供 Session owner 回答。D6b 补齐这条链路，让有权的回答继续 Turn，其他 actor 得到 403，重复回答返回原 operation。
Reviewer Test Plan
如何验证
使用 default 审批模式创建 Workspace Session 并请求写入文件。确认两个入口都能读取待审批 Action，Session 也报告 Actions 能力。非创建者的 reader 回答时必须得到 action_forbidden。创建者回答 allow 后，operation 应携带 decided 回执完成，工具 Turn 随后完成。使用同一回答键重复请求，包括切换入口，应返回相同 operation；修改内容必须冲突。确认终态详情仍可读取，终态审批从待审批列表消失，临时投递失败持续重试且不改变 Session 生命周期状态。
前后证据
之前：六个 Java Action 操作均为 planned，Workspace 文件执行要求 yolo。之后：40 个针对性的 Java 测试通过，包括使用打包 Hosted Harness 与真实 Runtime worker 的测试，覆盖双入口审批、reader 拒绝、owner 受理、重放、decided 结果及 Turn 完成。8 个 Action 测试覆盖畸形 journal、原始决定校验与跨入口分页。构建、typecheck、bundle 与 Checkstyle 均通过。较早的隔离 MariaDB 兼容性验证通过了 6 个 Action 测试和 Hosted 进程测试；最终 journal 校验修改在 H2 上验证。
测试平台
环境
Java 21、Node.js、MySQL 模式下的 H2，以及使用 MySQL JDBC 驱动的隔离 MariaDB 10.11.19 实例。测试后已清理 MariaDB 进程与临时数据目录。
风险与范围
英文设计与中文设计已同步。
关联 Issue
实现 #12867 的 D6b 切片。问题类 Action 与投票仍为 planned。