feat(web-shell): show and answer Hosted tool approvals in the Managed panel - #13107

Real-stack verification — b784842050

Verdict. On a real stack the approval card does what this PR says: it appears, the owner answers it, "Yes, allow once" lets the Turn continue and "Reject" reaches the model as a refusal. That holds with a scripted model and with qwen3.8-max. I found nothing in this PR's own code that should block merging it after #13101.

One thing to decide before it is marked ready (F1): the card never shows what is being approved. The description says the card attaches to the tool row, which supplies the call's arguments. On the real stack that row does not exist, so the owner sees only WriteFile / write_file. The cause is outside this PR, but the description promises otherwise.

Setup

MySQL 8.4.7, the Spring server jar with the embedded Runtime Broker (JDK 21), the packaged Hosted Harness (dist/cli.js), approval mode default, Workspace files on. The Managed panel is the real ManagedAgentWebShell served by vite from the PR sources and driven by headless Chromium. The base arm is the same page from #13101's head 358212a8b2, against the same server.

The head moved twice while I was working (badf94c4ae → cd7d6c61ab → b784842050). Every number below is from b784842050. Java and the Harness were built from cd7d6c61ab; the two heads differ only in two files under packages/web-shell/client (0 diff lines elsewhere).

Before / after

One Session waiting on a write_file approval, opened by both pages:

What holds

107 of 118 checks pass. The 11 misses are F1 (10) and N3 (1).

F1 — the owner answers without seeing the call

• The rig database holds 30 Sessions and 55 approvals from these runs (scripted and real model). Its 58 Items are all of type message: 0 tool_call Items and 0 item.tool_call.updated events, while an approval is pending and after the Turn finishes.
• Why: the Hosted Harness only emits agent_message_chunk updates (hosted-harness-session.ts:348), and Java creates tool Items only from tool_call / tool_call_update Harness events (HarnessEventProjector.java:55). The D6 design says "Arguments come from Items", but Hosted Turns put no function call into Items.
• Effect here: findManagedApprovalTool never finds a row and rawInput is never set. The "main risk" in the description (the Action's functionCallId equals the tool Item's toolCallId; "both hold on the feat(managed-agent): Serve durable permission Actions (D6b) #13101 head") cannot be confirmed or refuted by real data, because there is no tool Item to compare with. Only unit fixtures exercise that matching.
• A batch makes it concrete: three write_file calls in one model message produce three identical cards, so the owner cannot tell which file each answer is for.

Ways forward, for the maintainers to pick:

1. Merge this as the answering plumbing and track argument display on feat(managed-agent): Stage D follow-ups for durable lifecycle, Turns, Actions, durable admission and AgentDefinition #12867 before default / auto-edit is offered to users.
2. Hold it until the Harness publishes the function call when it commits the assistant message (before it asks), so Java projects a tool Item and this PR's matching has something to match.
3. Either way, correct the description, and consider a line on the card saying the arguments are not available.

I would not block the merge on F1, but I would not call the approval flow usable until it is closed: approving a write or a command the owner cannot read defeats the purpose of asking.

Smaller notes

• N1 — a reader gets a card they cannot use. An actor who may read the Session but did not create it sees the same card. Answering returns 403 action_forbidden, which the page renders as "The approval answer could not be sent. Try again." Retrying can never work for that viewer.
• N2 — lost reply, then the other option. If the reply to "allow" is lost after the service committed it and delivery to the Harness is slow (8 s injected), the card returns with the same "could not be sent" text. Clicking "Reject" is then accepted (202), fails server-side with action_already_resolved, the tool runs, and the page shows nothing. Narrow, and the first answer was a real one; it is the wording that misleads.
• N3 — browser clock ahead of the expiry. The expiry re-read uses the browser's Date.now(). With the browser clock past expiresAt, the page calls actions/query about once a second for the whole wait (11 calls in 12 s, against 0 with the clock in step).
• N4 — test gaps. 31 source mutants of the new client code: the PR's tests kill 21. Not pinned: the re-read after expiry, hiding an answered approval while it settles, state leaking across a Session switch, reporting a failed list read, matching the Action's own Turn when a later Turn reuses the call ID, and rendering the failure text. candidate-tests.patch adds five tests (+127 lines, tests only; eslint, prettier and tsc clean, 91/91) and brings that to 28. The three left are the stream_gap trigger, a constant inputRevision (it is always 1 today), and passing pendingApproval to the mocked MessageList.
• N5 — the test plan cannot be followed from the panel. "Create a Workspace-bound Session, and ask for a file write" is not possible in the Web Shell: the Workspace creator sends input: [] and the composer is hidden on Workspace Sessions. I created the Sessions through sessions/create with input and workspace. Those Sessions also show "Message execution is not available in this service yet" while a Turn is executing. Both predate this PR.

Gates

• At b784842050, in a clean worktree: eslint --max-warnings 0, prettier, tsc --noEmit and the web-shell build all exit 0; unit tests 86/86 in client/components/managed and 10,443/10,443 in packages/web-shell.
• The earlier red CI is resolved. On badf94c4ae I reproduced the Lint & Static failure (react-hooks/exhaustive-deps at use-managed-actions.ts:85) and found a Prettier failure in ManagedSessionsPage.test.tsx that the failed ESLint step was hiding; 89a14fd38c and b784842050 fix both. The two red Java jobs were the Flyway V23 collision from feat(managed-agent): Serve durable permission Actions (D6b) #13101, fixed there by V24.
• CI on this head: 19 checks pass, web-shell E2E Smoke was still pending when I posted this.

Not covered

• Shell approvals. The rig's Harness offered read_file, write_file and edit only, so the exec-style card was not exercised.
• auto-edit mode, question Actions, and the stream-gap path.
• Linux and Windows. Java and the Harness were not rebuilt at b784842050 (see Setup).

Unrelated to this PR, seen on the way: one model message with 22 tool calls leaves the Turn recovery-blocked with commit.commandId exceeds 512 UTF-8 bytes. It happens in yolo mode too, so it has nothing to do with approvals; 8 calls work.

Evidence (figures, scenario logs, rig scripts, probes): assets-pr13107 @ 81aeca90

Review follow-up at 4dc1472d2b (built on the concurrent 4fbe5025 read-recovery fix).

• Fixed: unchanged expiries now cause one re-read rather than recurring one-second queries; immediately rejected answers re-arm the existing approval button; HTTP 202 responses with public status: failed propagate their failureCode as an error and restore the card. Creator-only 403 and uncertain answer outcomes have accurate messages.
• Tests strengthened: a post-answer read still returning the requested Action keeps the card hidden, so the answered filter is now exercised. The page covers a failed initial read, its direct retry, creator-only rejection and retrying the same option. The bounded retry test advances each interval across a React commit.
• F1 / N5 narrowed: the card says when arguments are unavailable, and the description no longer claims Hosted tool Items exist. The reviewer plan creates a bound Session with initial input through the public API. Producing tool Items and adding a prompt-capable bound composer remain follow-ups.
• N2 follow-up: detecting an already-failed response is covered; following a previously admitted operation to a later terminal result remains outside this PR. A requested Action is not automatically unhidden during settlement, since that would invite a second answer before the first settles.
• N4 / visual evidence follow-up: focused regressions were added. The broader mutation matrix and extra screenshot scenarios remain follow-ups. The existing real-stack report supplies the backend/browser evidence; this round adds frontend integration checks with mocked HTTP responses.

Validation on macOS: repository npm run build and npm run typecheck, changed-file ESLint/Prettier, and all 92 Managed tests in 11 files passed. Two full diff audit passes were clean. Independent integration checks confirmed: clock-ahead queries 5 → 2 over about 4.4 seconds; initial-503 retry changes only Actions queries 1 → 2 while Session/transcript/stream remain 1 and submissions/answers 0; an already-failed 202 now rejects, restores the card and sets the answer error. Regression cases were red before their fixes.

Scope ledger: two substantive fix rounds (4fbe5025, then this commit). Against the inherited D6b base, implementation remains 10 files, 319 → 394 changed lines; tests remain 6 files, 441 → 630 changed lines; the full PR-owned delta still has 16 WebShell files. Test growth triggered a scope audit: this round retains distinct approval regressions in existing test files and keeps operation tracking, producer changes, questions/votes and broader test matrices as follow-ups. Landing order remains #13101, then #13107 after merging main.

Current bounded pass at e572cd72: partial review5375002256 reports one Critical and12 Suggestions. Independent actual-page/Java HTTP observation reproduced R1-1: Refresh removes the live card, and a15-second summary-only outage leaves it absent although the real Actions endpoint still returns the requested Action. Initial unknown, explicit no-capability and selected-Session isolation controls were checked. This pass fixes the selected-session capability lifetime and its direct regressions; the12 Suggestions are classified and recorded separately under the repository’s roughly-five-round scope rule. No producer, API or operation-tracking work is included.

本轮仅处理已由独立真实页面/Java HTTP 探测复现的 R1-1：刷新与摘要接口故障错误撤下已确认可读的审批。保留初始未知、明确无能力与切换 Session 的隔离边界；其余12条建议依约五轮 review 后的范围规则另行记录。

@qwen-code /verify

Please verify the current head independently, including the central behavior and load-bearing negative controls. Keep the report tied to the exact tested commit.

@qwen-code /verify

@qwen-code /verify

Current-head E2E acceptance at 105b272: 79 browser assertions passed, 0 failed, using actual MySQL, Java/Broker, packaged Hosted Harness and Chromium. The model and actor identity use local test adapters. Full report, raw results, network records, hashes and probe scripts.

The parameter probe reproduced a remaining defect at 756e8d9: a valid write_file Item suppressed the unavailable notice but its write content did not appear on the approval card. Commit 105b272 renders the already-received arguments through the shared card. Two actual-card DOM regressions failed before the fix and pass afterward. Separately, 94 Managed tests, repository build/typecheck, changed-file lint/formatting and two full-diff self-audits passed.

Before, at 756e8d9:

After, at 105b272:

The actual Hosted producer still emits no tool Items, and that real path retains the explicit arguments-unavailable notice. This report does not establish upstream Item production, production identity/model integration, or tracking an admitted operation through a later asynchronous failure. The still-settling answer/list-requested branch is covered by a focused hook test rather than a forced browser run. This macOS local-process run is separate from the CI container/VM verification; current-head CI and independent verification are still pending.

Qwen Code review request accepted. Review is queued for an available runner; follow the workflow run for progress. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes.

Real-stack verification, round 5 — 72608f2c03

Since round 4, this branch has gained:

• both round-4 candidates (R4-1 in 19c606bd5f, the G9 page test in 7e4b802227);
• a clean merge of main e083d6a6b8 (feat(managed-agent): Add hosted file history and undo #13110, feat(managed-agent): Host Managed sessions in a private ACP child (M2) #13131). Those change Java and the Harness, and nothing in web-shell;
• fixes for R1-2 … R1-9;
• late-reply isolation (a90b7966e4, 72608f2c03).

The head moved from 9fedb263a0 to 72608f2c03 while I was verifying. I re-ran everything below on 72608f2c03; the 9fedb263a0 logs are kept in the evidence. I rebuilt the Java server and the Hosted Harness from the head. "Before" in the comparisons is the round-4 head 24b26f107d, served side by side against the same server.

Verdict.

• The build and the merges are sound. The main merge needed no hand resolution. The one hand-resolved conflict, in 72608f2c03 (late replies vs ended Actions), keeps both sides. eslint, prettier, tsc and the build all exit 0. Unit tests pass 198/198 in managed and 10,602/10,602 in packages/web-shell. The PR's test files now type-check (R1-9); the one remaining error is on a main line and exists on main too.
• Nothing regressed on the real stack. 166 of 169 checks pass; the 3 misses are the known R2-1 probe. This covers allow/deny, a real hosted model, expiry, viewers, faults, sequences, clock skew, R1-1 and R4-1.
• R1-2, R1-3, R1-4, R1-5, R1-7, R1-8 and R4-1 are fixed on the real page. So is late-reply isolation. Each was shown against the round-4 head (figure 3):
  • A stale tab's Reject gets a real 409 action_already_resolved, and the card now leaves without a warning.
  • A lagging tab answers just after expiresAt and gets a real 409 action_expired. No warning is left on an empty panel.
  • After a Session switch or a replaced approval, a late reply (503, success or 409) no longer warns beside the wrong card. None of the 4 variants warns at the head; 3 of them did before.
  • A 404 is read once instead of four times.
  • A failed refresh now says "could not be refreshed".
  • The Harness's call title reaches the card.
  • Chromium's computed accessible description of the dialog now includes the arguments caveat, with no dangling IDREF.
• The "a wrong report cannot hide a live Action" property holds. I replayed Java's real 409 body for an Action that was still live. The card left, came back after the next read, and the real Allow was then applied once.
• R1-6 is only half fixed. This is the one open behaviour item; details and a candidate are below. It is Suggestion-level: the alert's Retry button works throughout.
• There are three test gaps. Mutation testing kills 64 of 77 mutants. Deleting action_already_resolved from the ended codes keeps every test green, and that is the code Java actually returns in R1-2. A +55-line test-only candidate closes this gap and two others (below).

R1-6 — Refresh after the retry ladder is spent still gets no retry

The R1-6 finding named the page's Refresh as the path that strands the ladder. It also said the bug survives the R1-1 fix. 95fbc3ca96 resets loadFailures only in the !reader || !sessionId branch, but since R1-1, Refresh never reaches that branch:

1. Refresh drops the summary, so enabled becomes undefined.
2. The reader is kept, and the effect returns at if (enabled === undefined) return undefined; without resetting the budget.
3. When the summary comes back, one read runs with the counter still at 4. If that read fails, LOAD_RETRY_DELAYS_MS[4] is undefined, so no retry is scheduled.

The new test restores the retry budget when the reader is withdrawn and back drives enabled: false → true, which the page does not do on Refresh.

On the real page: actions/query returned 503 four times, then the user clicked Refresh, the next read failed once more, and the service was healthy after that.

• At the head, the page made 5 reads and showed no card for 15 s.
• The round-4 head behaves the same.
• With the candidate, the page made 6 reads and the card came back after 2 s.

candidate-r1-6-reload-budget.patch (+36/−1):

• It resets the budget in the enabled === undefined branch as well.
• It adds the same test as the author's, driven through undefined. That test fails on the head (expected "spy" to be called 6 times, but got 5 times) and passes with the patch.
• managed passes 199/199, and eslint, prettier and tsc are clean.
• The ladder stays bounded: stops retrying after a bound still passes, because only a reload — a user action — resets the budget.

Test gaps the mutants found

Mutation testing kills 64 of 77 mutants at the head. Three of the survivors are real gaps:

• N1. Deleting 'action_already_resolved' from ENDED_ACTION_CODES keeps every test green. The tests cover action_expired and action_cancelled, but R1-2 on the real stack returns action_already_resolved.
• A3 and A8. The Action-Turn guard in findManagedApprovalTool is unpinned for both row kinds. The tests always put the Action's Turn last, so "match any Turn's call with that ID" gives the same answer.

candidate-r5-tests.patch adds +55 lines of tests only. It applies to the head and passes 200/200, and it kills all three mutants.

Other survivors are not gaps. The late-reply commits added a render-time check, answerError shown only for the displayed Action, which overlaps the existing state clearing. Each survives alone, and the pairs L3+N11 and L3+H10 are killed together. The rest change nothing a test or the page observes, or are old (H15, P5, N7).

Before and after

Not covered

• Shell approvals, auto-edit, question Actions, Linux and Windows.
• The feat(managed-agent): Add hosted file history and undo #13110 file-history UI, which is not part of this PR.

CI on this head: 8 checks pass; Capture web-shell visuals and web-shell E2E Smoke were still pending when I posted this.

Evidence (figures, raw screenshots, scenario logs for both heads, probes, mutation runs, both candidate patches): assets-pr13107 @ b2276304 / pr13107/r5

@qwen-code /triage

@qwen-code /triage

Post-merge review — reviewed at head 9fedb263a0d9a95d766ae92a2238f8a081b69bd6 (the last pre-merge commit fully read), base e083d6a6b8cc728d12133cbd098e8a9f8176c546. The PR merged to 72608f2c while the review was running; findings are against 9fedb263.

Summary

No blocking finding. One deferred Minor the author already acknowledged. Three positive verifications of the highest-risk claims.

Verdict if pre-merge: Approve with the Minor noted below.

Minor finding — R2-1

packages/web-shell/client/components/managed/ManagedSessionsPage.tsx:586 (managed.approval.forbidden branch)

When respond returns action_forbidden (HTTP 403), the ToolApproval card is restored to the screen with no disabled prop and no visual indication that the current user cannot answer. The "You don't have permission" message (managed.approval.forbidden) is rendered as a sibling <p> below the card, but both buttons remain clickable. A non-creator viewer who sees this message can keep clicking Allow or Deny, generating one 403 per click indefinitely until the Action expires.

The author explicitly deferred this in the R1-2 thread: "I am leaving this open rather than guessing the contract; it needs a product call." Recording it here so it travels with the shipped code and is findable in the follow-up ledger.

action_forbidden is intentionally absent from ENDED_ACTION_CODES (use-managed-actions.ts:17-21), so it does not take the silent-hide path the new commit added for action_expired / action_cancelled / action_already_resolved. That distinction is correct (a forbidden viewer is different from an ended action), but the card still needs either a disabled prop or a product-defined hide policy.

Verified claims

Class 10 — stated intent: "An unchanged expiry does not re-arm a one-second query loop" (use-managed-actions.ts:161-171)

Verified correct. earliestExpiry is computed as Math.min(...actions.map(a => a.expiresAt)) — a stable number whenever the action set is unchanged. React's useEffect compares dependencies by ===; a numeric value that does not change means the expiry effect does not re-run and no new setTimeout is set. The timer fires exactly once per distinct earliestExpiry value.

Class 4 — authorization: action_forbidden detection (ManagedSessionsPage.tsx:180-184)

Verified correctly wired end-to-end. JavaManagedAgentClient.toHttpError reads payload.error.code into JavaManagedAgentHttpError.code (java-managed-agent-client.ts:461-479). The page checks 'code' in approvalCause && approvalCause.code === 'action_forbidden', which matches that property. Creator-only enforcement is server-side (HTTP 403 on respondWebShellAction); the frontend correctly shows a distinct message rather than leaking any privilege.

Class 8 — replay divergence: still-settling answered actions (use-managed-actions.ts:179)

Verified correct. answered is a ReadonlySet<string> populated optimistically on click and cleared only on success, failure, or session change. action = actions.find(entry => !answered.has(entry.actionId)) skips any action in the set, so a re-read that returns a just-answered action as requested does not flash the card back.

R1-2 (definitively-ended actions) — addressed by commit 9fedb263

The new commit adds ENDED_ACTION_CODES = { action_expired, action_cancelled, action_already_resolved }, an endedAction(failure) helper, and endedAnswers ref. When respond throws one of these codes the card is kept in answered (hidden), endedAnswers records it, no error is shown, and a re-read is triggered. On the subsequent read, if the server still lists the action (race / stale report), stillListed removes it from answered so the card reappears — a clean safety net. endedAnswers.current.clear() in the session-reset effect (line 95) prevents the ref from leaking across sessions.

Scope and unreviewed dimensions

Reviewed: use-managed-actions.ts, managed-approval.ts, managed-request-error.ts, ManagedSessionsPage.tsx, java-managed-agent-provider.ts, managed-agent-provider.ts, java-managed-agent-client.ts, ToolApproval.tsx, generated schema types. Examined patches for all 19 changed files.

Not covered: local execution unavailable — rungs 1-3 not run; daemon provider (no actions property, no delta); test files reviewed for relevance but not executed.

Reviewed with AI assistance.