From 33b852f66b90e89571c097cc88dbfa038b04f0fc Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sun, 6 Sep 2026 11:21:10 +0100 Subject: [PATCH] fix(assistant): preserve tool evidence identity Concurrent calls with the same tool name could collapse pending activity or remove a sibling approval. Deep transcript reconciliation also mutated shared tool objects when status rows disappeared, replacing earlier evidence with a later result. Match supplied invocation IDs strictly and key immutable message rows without reconciling their nested evidence. Add regressions for both causes and record desktop, intermediate and narrow browser replay of all 23 captured tool results. Record the healthy and dependency model qualification limits separately from renderer and fixture-cleanup proof. --- .../PATROL_ASSISTANT_CUSTOMER_JOURNEY.md | 82 +++++++++++- docs/release-control/v6/internal/status.json | 2 +- .../v6/internal/subsystems/ai-runtime.md | 6 + frontend-modern/browser-verification.json | 41 ++---- .../src/components/AI/Chat/ChatMessages.tsx | 122 +++++++++--------- .../AI/Chat/__tests__/ChatMessages.test.tsx | 73 +++++++++++ .../AI/Chat/__tests__/useChat.test.ts | 90 +++++++++++++ .../src/components/AI/Chat/hooks/useChat.ts | 15 +-- 8 files changed, 328 insertions(+), 103 deletions(-) diff --git a/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md b/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md index 833d1fb99..fdcf448c3 100644 --- a/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md +++ b/docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md @@ -62,7 +62,7 @@ reproduction evidence, not a representative customer success rate. | Step | Work | Acceptance | Current state | |---|---|---|---| | 1. Product contract and baseline | Map the current loop and sources of judgment. Record telemetry populations and gaps. | Every identified decision has an owner. Activity is not labelled usefulness. | Complete for this redesign scope. Contract, ownership decisions and baseline limits are recorded. | -| 2. Shared evidence | Preserve canonical risk reasons and SMART counters, source/time semantics and history across tools/turns. | Regression tests preserve unknown versus zero and all canonical evidence. Real responses can inspect the same facts as the product. | Implemented and qualified for the named shared-evidence defects. Canonical disk detail, risk and cadence pass real data-path proof. Affected package and concurrency checks pass. Integrated CI later exposed remaining query and allocation regressions. The final bounded query-reuse correction passes complete selected exact-base worker comparisons and full metrics/database and focused race checks. Final landing CI remains open. Real-model interpretation failures remain tracked in step 5. | +| 2. Shared evidence | Preserve canonical risk reasons and SMART counters, source/time semantics and history across tools/turns. | Regression tests preserve unknown versus zero and all canonical evidence. Real responses can inspect the same facts as the product. | Implemented and qualified for the named shared-evidence defects. Canonical disk detail, risk and cadence pass real data-path proof. Affected package and concurrency checks pass. Integrated CI later exposed remaining query and allocation regressions. The final bounded query-reuse correction passes complete selected exact-base worker comparisons and full metrics/database and focused race checks. Final landing CI passed and PRs #1928 and #1929 merged. Real-model interpretation failures remain tracked in step 5. | | 3. Diagnostic orchestration | Correct proposal-as-proof. Audit triage budgets, unmatched-signal evaluation, assessment completion and investigation cutoffs. | No code-written causal conclusion. No quality inferred from tool, flag or finding counts. Each retained pass has an objective reason. Safety boundaries and incomplete outcomes remain explicit. | Proposal promotion and capture inference were removed in c5d2f56dda. Commit 668af3fe6b removes investigation success-call floors, checkpoint instructions and generic call-count wrap-up rules. The detection slice removes contextless follow-up passes, flag/report-count policy and first-finding completion modes. Full chat and AI suites, focused API and conversation race tests pass. Real-model/action outcome qualification remains open. | | 4. Issue through verified outcome | Follow existing issue/investigation/action records into Assistant, approval, execution and independent readback. | Accepted proposal is visibly distinct from execution and verification. Rejected or unsupported actions do not become success. Uncertainty can survive an action proposal. | Existing foundation, full journey qualification pending. | | 5. Ground-truth qualification and landing | Extend existing qualification tooling only where necessary. Exercise healthy/unhealthy, dependency, missing-access, storage/backup and approved/rejected action cases. Inspect the final browser journey at desktop and narrow widths. | Record exact source/model/permissions, evidence, decisions, faults/misses, latency and verification. Fix in-scope failures, pass appropriate proofs and land scoped commits. | Pending. | @@ -1328,3 +1328,83 @@ diagnostic-record commit `ba69933da352` passed its two-file staged hook, all 163 tests in 127.527s with unchanged hashes. The current runtime change still requires its final staged hook and exact-head remote CI. Overall diagnostic and action-outcome qualification remains open. + + +## Healthy and dependency Assistant qualification, 2026-09-06 + +Two ordinary read-only Assistant requests used the configured +`claude-subscription:claude-opus-5` route with explicit `autonomous_mode=false`. +These are single-case observations, not an autonomous Patrol pass or an estimate +of customer success, false-alarm or missed-problem rates. The earlier policy +refusal remains in force. No paid-provider request was made. + +The unchanged Docker dependency manifest ran through the existing `DockerLab` +on the monitored Tower host. Only run-owned Alpine containers and their private +network were created. The existing production container was untouched. Independent +Docker observations and Pulse resource convergence established healthy client and +dependency, then a stopped dependency and running-but-unhealthy client. Fault +injection used a deliberate stop with a five-second grace period. Its exit 137 +therefore does not establish OOM. + +| Case | Observed result | Limit | +|---|---|---| +| Healthy client | 82.835s, seven tool calls, no failed tools. Assistant correctly recommended no action. Independent observations before and after the request retained both healthy containers and unchanged identities. | The answer incorrectly inferred no contribution to or effect from host storage pressure from empty mounts and zero sampled writes. The primary decision passes this case, but the full explanation does not. | +| Stopped dependency | 204.384s, sixteen tool calls, three failed reads. Assistant identified the stopped sibling and treated the dependency explanation as a hypothesis. It preserved the command-access limit and did not claim exit 137 proved OOM. | It excluded storage causality from zero sampled I/O, overstated what `OOMKilled=false` establishes, recommended restarting the client without first establishing that need, and claimed continued failure after dependency recovery must be a client-healthcheck fault. Those claims exceed the observations. This is partial diagnostic evidence, not a qualification pass. | + +The fault case also received two `app-container not found` responses from config +reads despite successful canonical get responses. That tool capability/error +contract remains an in-scope follow-up. No post-answer Docker fault observation or +explicit recovery phase was recorded before the fixture's 25-minute deadline. +Deadline cleanup completed at 08:16:40.820Z, removed both run-owned containers and +their network, then passed a second no-op cleanup with unchanged original +inventory. The helper exited on its deadline. This proves cleanup, not a completed +recovery or action-outcome journey. Neither Assistant request mutated the lab. + +Private evidence root: +`/Volumes/Development/pulse/tmp/patrol-assistant-lab-readiness/runs/asst-20260906075136356-380d4d/`. +It contains independent `oracle/` receipts, healthy/fault Pulse convergence, +complete SSE streams, persisted sessions, requests, screenshots and before/after +source/binary bindings. Both requests used unchanged runtime binary SHA256 +`ebd0c2c74e4dbee330284a12a99137cfcecaecfe1ec2545c25432f06183a10c1`. +HTTP windows were 07:53:58.742Z to 07:55:21.577Z and 07:58:04.234Z to +08:01:28.618Z. Displayed token counts are not complete context or billable-cost +measurements. Browser interception blocked unrelated non-GET requests, including +route checks, producing an artificial selected-route warning. It did not test +route recovery or retry the autonomous refusal. + +### Tool evidence identity correction + +The captured browser responses exposed a separate reproducible evidence defect. +Concurrent starts/progress used name fallback even when invocation IDs differed. +Completing one same-name action could remove another invocation's approval card. +The shared reducer now treats supplied IDs as authoritative through start, +progress, cancellation, completion and approval cleanup. Older ID-less name +matching remains the existing compatibility path. + +A second reproduction showed deep Solid store reconciliation mutating objects +shared by `toolCalls` and `streamEvents`. Removing a workflow row could change the +first query's input and output into a later alerts call. Message rendering now +keys rows by message ID and reads the immutable message through an accessor. +It keeps DOM stability without copying or mutating the evidence graph. Regression +proof includes the actual status-row-removal trigger, concurrent same-name calls, +sibling approval retention and cancellation. The affected test files pass all +167 tests, including existing message mount-stability checks. + +Browser matrix: `/patrol` at 1440x1000, 900x1000 and 390x1000. Captured-response +replay checks all seven healthy and sixteen dependency tool inputs and outputs +against their exact terminal SSE records. It also exercises hover/focus, keyboard +expansion/collapse, output scrolling and restored persisted sessions. A controlled +stream fixture exercises concurrent pending tools, repeated starts, progress, +out-of-order completion, independent approval cards, cancellation and failed +completion. These are renderer/identity checks, not model or action qualification. +Private replay and state receipts live under +`/Volumes/Development/pulse/tmp/patrol-assistant-lab-readiness/identity-*`. + +PR #1928 merged its earlier scope at `6d2d188867430f653e1bf9ada634fd2b90440786`. +The later diagnosis record and route-label performance correction merged through +PR #1929 at `cf98358c0eb46987a82def5776fa41db5f54210a`. Its backend, frontend, +benchmarks, governance, CodeQL and eight Core E2E shards passed. The identity +correction above is a separate scoped change and requires its own landing checks. +The redesign remains open for reliable interpretation, the config-read contract, +storage/backup, approved/rejected action outcomes and supported autonomous Patrol +qualification. Wider customer readiness still requires independent Pro environments. diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index 84edfe899..bf404a895 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -10201,7 +10201,7 @@ }, { "id": "patrol-assistant-customer-outcome-qualification", - "summary": "The explicit Patrol/Assistant redesign goal, product contract and ordered source-bound proof remain in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Observations, hypotheses, proposals, executions and independently verified outcomes remain distinct. Model judgment owns diagnosis. Assistant continues the same issue and governed action records. The recorded 2026-09-05 baseline has 127 paid installs, 71 with Patrol enabled and 23 with Assistant calls. Fourteen verified resolutions came from one install, and schema 17 outcome/provider/cost fields had no adoption. Usage is not evidence of useful linked tasks. Implemented corrections preserve canonical risk, source/time/history evidence, failed-read status and diagnostic continuity while removing proposal-as-root-cause promotion, duplicate causal inference, contextless evaluations, quotas and call-count completion proxies. Named evidence and linked-issue browser matrices pass. The latest ordinary model diagnosis still makes unsupported temporal claims and remains unqualified. Claude Max explicitly refused autonomous readiness. Cached refusal and API409 enforcement remain intact. Ordinary Assistant success is not autonomy qualification. Alternate paid-provider approval remains pending, with no paid request or production mutation. The service-storage fault/oracle independently verifies bounded ENOSPC, filesystem exhaustion, unhealthy service with healthy control, recovery, refused overwrite/symlink writes and unchanged pre-existing inventory. This qualifies the oracle, not diagnosis. All twelve manifests and five full-schema regression tests pass, and the published qualification guide passes desktop/narrow browser proof. Missing-access, broader storage/backup and approved/rejected action outcomes still require full real-model qualification. Integrated CI exposed retained-query performance and disk-probe completion defects. The disk race fix b964eea767 and named-binding fix 1f41fa174d are committed and pushed to open PR #1928, each after a 163-test exact staged hook. Full affected packages and focused race proofs pass. Ten paired final-source worker benchmark rounds pass the unchanged time/bytes/allocation checker against exact base 3347, including history/chart APIs and a memory-fallback control. The 500-node read has +0.26% allocations and -1.68% bytes with no significant runtime difference. Remote CI and landing remain open. Current missing-access work reproduces topology loss with zero command connections and preserves known resource kind, parent and routing without dispatch. Reads and file mutations use one failed NO_AGENT envelope instead of installation speculation or successful write results. No-target cardinality and hostname collision boundaries have regression coverage. Full tools proof passes on Go 1.26.8 in 59.437s. The exact five-test race proof passes in 1.051s and existing monitoring-token/WebSocket execution-scope tests pass in 0.122s. The final 18-case scripted browser matrix passes at /patrol, 1440/900/390x1000, including current handler errors, investigation expansion, linked Assistant details, dismissal and reload without resubmission. This is rendering/access-boundary proof, not model diagnosis. Missing-access commit58caeda69ba3 is pushed after the exact eight-file staged hook passed 163 tests in 127.706s with unchanged hashes. On head1f41, governance, all eight Core E2E shards and CodeQL pass, but Build and Test run34015148620 is pending with no jobs. Missing PR checks are not evidence of success. Existing diagnostic permission controls and advertised hypervisor action authority remain distinct. Exact receipts and historical failed/discarded runs are retained in the plan. Independent volunteered Pro environments remain a separate wider-readiness gate. The opt-in live dependency/restart oracle now passes all four checked-in dependency and approved/rejected/autonomous service fault contracts in 65.080s on Go1.26.8. It proves injected state, unchanged fault after refused duplicate injection, explicit fixture recovery and restored baseline. All four two-pass cleanups preserve pre-existing Docker inventory. It makes no Pulse/model request and does not qualify approval, rejection or action outcomes. The new oracle test source and raw receipts are recorded in the plan. Ordinary qualification and CLI package proof passes in 4.224s and 0.008s with the live environment unset and unchanged test hash. Oracle commit173d74a8e422 is pushed after its exact four-file staged hook passed 163 tests in 129.364s with unchanged hashes. The current real ordinary Assistant request takes 216.283s with 15 tool calls and one failed read. Known identity and the access limit survive correctly, but the answer contradicts explicit timestamp semantics, mislabels mixed retained points as hourly and misplaces a memory extremum. This remains failed diagnostic qualification. Source/binary-bound raw evidence and the browser-interception limitation are recorded in the plan. No autonomous retry or paid-provider request occurred. Exact-head remote CI and landing remain pending. Run34017211910 completed with every Build and Test job except Benchmarks passing. A same-host comparison reproduced tiny route-normalisation timing regressions despite identical source and normalized instructions. The canonical ASCII-only classifiers now avoid unnecessary rune decoding while preserving label behavior. The full HTTP-metrics test file passes under race in 1.057s. Ten alternating exact-base3347 pairs pass the unchanged benchmark checker with segment times 32-39% below base and unchanged allocations, including adjacent full-route/middleware controls. Exact source hashes, failed comparisons and measurement limits are in the plan. Final staged-hook and new-head remote landing checks remain required.", + "summary": "The explicit Patrol/Assistant redesign goal, contract, execution plan and source-bound evidence remain in docs/qualification/PATROL_ASSISTANT_CUSTOMER_JOURNEY.md. Model judgment owns diagnosis. Observations, hypotheses, proposals, executions and independently verified outcomes remain distinct. Assistant continues the same issue and governed action records. The recorded 2026-09-05 baseline has 127 paid installations, 71 with Patrol enabled and 23 with Assistant calls. Fourteen verified resolutions came from one installation. Schema 17 outcome/provider/cost fields had no adoption. Usage does not prove useful linked tasks or representative false-alarm, missed-problem or success rates. Shared risk, provenance, history, missing-access and diagnostic-continuity corrections have regression and named browser proof. Proposal promotion, duplicate causal inference, contextless evaluations and count-based diagnostic completion policy were removed. Independent Docker fault/oracle contracts qualify reproducible injection, negative controls and cleanup, not model competence or governed action outcomes. Integrated CI exposed retained-query performance, disk-probe ordering and route-label timing regressions. Their scoped corrections and qualification records landed through PR1928 and PR1929. PR1929 merged at cf98358c0eb46987a82def5776fa41db5f54210a with backend, frontend, benchmark, governance, CodeQL and all eight Core E2E shards passing. The current ordinary retained-history diagnosis still contradicts explicit temporal semantics and remains unqualified. Two additional read-only Assistant requests used claude-subscription:claude-opus-5 against run-owned containers on the monitored Tower host. The healthy request took 82.835s and seven tools, correctly recommending no action, but overstated absence of storage impact. The dependency request took 204.384s and sixteen tools with three failed reads. It identified the stopped dependency, preserved the missing command access and causal uncertainty, but overstated storage exclusion and recovery implications. Config reads incorrectly reported app-container not found after successful canonical gets. These single cases remain partial diagnosis evidence, not a qualification pass. The fixture deadline performed two-pass cleanup with unchanged original inventory. Post-answer fault readback and explicit recovery were not completed, so no action outcome is claimed. Captured responses exposed concurrent tool-ID merging, sibling approval removal and renderer mutation of shared evidence. The current scoped correction keeps supplied invocation IDs authoritative and stable message rows without deep transcript reconciliation. All 167 affected frontend tests pass. Final-source browser replay at /patrol, 1440/900/390x1000, preserves all seven and sixteen exact tool inputs/outputs. Controlled stream states verify concurrent progress, cancellation, failed completion and sibling approval retention without provider or infrastructure actions. Exact captures, failed reproductions, hashes and remaining limits are in the plan. This correction still requires its own scoped landing checks. Claude Max explicitly refused autonomous Patrol readiness. Cached refusal and API409 enforcement remain intact, with no bypass or repeated retry. Ordinary Assistant is not autonomous qualification. Approval for an alternate separately billed provider remains pending, and no paid request occurred. Reliable interpretation, the config-read contract, broader storage/backup and approved/rejected action outcomes remain required local work. Independent volunteered Pro environments remain a separate wider-readiness gate.", "owner": "project-owner", "status": "planned", "recorded_at": "2026-09-05", diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 3b1e59cd2..a8001aa9b 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -2414,6 +2414,12 @@ deriving an older display status from `workflowStatusHistory`. normalized tool name when an older server omits IDs, upsert one pending tool row and collapse stale duplicate pending rows instead of replaying several near-identical steps in the transcript. + A supplied invocation ID is authoritative. Distinct IDs must not merge by + tool name during start, progress, cancellation, completion or approval + cleanup. Completing one invocation must leave its sibling approvals intact. + Message rendering keys rows by message ID and reads immutable transcript + values. It must not deeply reconcile shared tool objects: removing a status + row must never mutate evidence retained in another event or in toolCalls. `frontend-modern/src/api/aiChat.ts` owns the Assistant predicate: token content and hidden reasoning may continue to opt out of those checkpoints so answer streaming remains fast, while session, workflow, model-switch, tool, diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index df645715b..a942fbaa7 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,36 +1,20 @@ { "version": 1, - "base_sha": "61607333cc9e2fe1c1712b94ce458ac45abdabd1", - "verified_at": "2026-09-06T03:17:28.710286Z", + "base_sha": "9b4fa52d84e6196108e961ccba2b317df497268b", + "verified_at": "2026-09-06T10:15:18.709765Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/features/alerts/AlertDeliveryHealthCard.tsx", - "frontend-modern/src/features/alerts/OverviewTab.tsx", - "frontend-modern/src/features/alerts/useNotificationDeliveryHealth.ts" + "frontend-modern/src/components/AI/Chat/ChatMessages.tsx", + "frontend-modern/src/components/AI/Chat/hooks/useChat.ts" ], "content_sha256": { - "frontend-modern/src/features/alerts/AlertDeliveryHealthCard.tsx": "dad60838804e575c0314398b304bda6d2fbd842cb2f693b3f33d168a4bce5dc1", - "frontend-modern/src/features/alerts/OverviewTab.tsx": "8f7fdc04bd0546f86152c8bb392ff3e3fb755f1f0a1c28d1ee85da3e9fe23582", - "frontend-modern/src/features/alerts/useNotificationDeliveryHealth.ts": "1eadf6df30b4f1130f868b8f139561ea60fdb87e1823deeeaaf2f8fbc699a00c" + "frontend-modern/src/components/AI/Chat/ChatMessages.tsx": "9672f7608d1e3a531c73cba20fd4a78752316783212afd0c292ddfd11d2bf371", + "frontend-modern/src/components/AI/Chat/hooks/useChat.ts": "0b56b7a56e35d51ca96f0e126dd493b3164aa9e0ad4d8ae24bcf3af7a574b97c" }, "routes": [ - "/qualification (actual delivery-health and Overview components in isolated Solid Router fixtures, scripted API promises)", - "/patrol", - "/alerts" + "/patrol" ], "viewports": [ - { - "width": 1440, - "height": 900 - }, - { - "width": 900, - "height": 900 - }, - { - "width": 390, - "height": 900 - }, { "width": 1440, "height": 1000 @@ -45,13 +29,12 @@ } ], "states": [ - "Delivery ordering: 18 scripted cases at three widths. Overview refresh: 12 scripted cases across light/dark themes and three widths. Unavailable health, retained Retry/Dismiss actions, pending disabled Refresh and healthy recovery. These fixtures do not qualify installed backend delivery or recipient outcomes.", - "Final card pixels inspected at desktop, intermediate and narrow widths in both themes. Heading is readable and actions wrap within the card. This is a focused card check, not a full dark-theme shell audit.", - "Ordinary and alert-mirrored Patrol findings preserve unknown cause and failed-read evidence in the investigation transcript and linked Assistant. Scripted responses qualify presentation and context preservation, not diagnostic competence." + "Captured healthy and dependency Assistant responses replayed without provider calls. All 7 and 16 terminal tool records retain exact inputs, outputs and distinct rows. Success, failed NO_AGENT and unsupported config results inspected.", + "Controlled stream: concurrent same-name calls, repeated starts, progress, two separate approval cards, sibling completion, canceled call and failed completion. No real action was attempted.", + "Restored persisted healthy and dependency sessions retain original prompts and evidence. Artificial route warning reflects blocked non-GET readiness checks, not route qualification." ], "interactions": [ - "Retry and Dismiss followed by failed health read, manual Refresh while pending, healthy recovery and overlapping old/new response ordering. Geometry assertions check heading overflow and control bounds.", - "Selected issue, evidence expansion, keyboard Review and Explain, nested transcript expansion/collapse, Assistant streaming, preserved draft, reload, error/retry, menu Escape and outside dismissal at 1440/900/390 widths.", - "Private receipts: /Volumes/Development/pulse/tmp/patrol-merge-browser/final-health-ordering and final-overview-refresh, and tmp/patrol-assistant-journey/result.json. No infrastructure mutations or autonomous provider calls." + "Hover, focus, Enter/Space expand and collapse, scroll expanded output to its end, Escape, reload and select the original persisted session. Tool details and wrapping inspected at desktop, intermediate and narrow widths.", + "Private evidence: /Volumes/Development/pulse/tmp/patrol-assistant-lab-readiness/identity-verified-healthy, identity-verified-fault and identity-states. Baseline reproductions retained in identity-before-fault and failed focused regressions." ] } diff --git a/frontend-modern/src/components/AI/Chat/ChatMessages.tsx b/frontend-modern/src/components/AI/Chat/ChatMessages.tsx index e6f3adf91..b5be6a9a8 100644 --- a/frontend-modern/src/components/AI/Chat/ChatMessages.tsx +++ b/frontend-modern/src/components/AI/Chat/ChatMessages.tsx @@ -1,5 +1,4 @@ import { Component, Show, For, createEffect, createMemo, createSignal } from 'solid-js'; -import { createStore, reconcile } from 'solid-js/store'; import ArrowDownIcon from 'lucide-solid/icons/arrow-down'; import { Button } from '@/components/shared/Button'; import { MessageItem } from './MessageItem'; @@ -66,22 +65,13 @@ export const ChatMessages: Component = (props) => { let containerRef: HTMLDivElement | undefined; const [isPinnedToBottom, setIsPinnedToBottom] = createSignal(true); - // useChat hands us a fresh, immutably-rebuilt message array on every stream - // event (each content chunk, workflow-status change, tool update spreads a new - // message object). Rendering that array directly through , which keys by - // object reference, tears down and recreates the whole MessageItem on every - // event — the visible flashing / rows popping in and out and the transcript - // jumping up and down during a turn. - // - // Reconcile the incoming array into a keyed store mirror so each message keeps - // a stable identity across updates (matched by id). MessageItem already reads - // every field through `() => props.message.x` accessors, so once it stops - // re-mounting, only the genuinely changed text/rows update in place. This keeps - // the streaming transcript stable the way OpenCode's timeline is. - const [mirroredMessages, setMirroredMessages] = createStore([]); - createEffect(() => { - setMirroredMessages(reconcile(props.messages, { key: 'id', merge: false })); - }); + // Key only the message rows. Deep store reconciliation mutates objects shared + // by toolCalls and streamEvents when a status row is removed, corrupting the + // source transcript. Read each immutable message through its stable ID instead. + const messagesById = createMemo( + () => new Map(props.messages.map((message) => [message.id, message])), + ); + const messageIds = createMemo(() => [...messagesById().keys()]); // Regenerate re-runs the LAST turn via session undo, so only the final // assistant answer qualifies, and only once it has settled with nothing @@ -342,54 +332,58 @@ export const ChatMessages: Component = (props) => { {/* Messages */} - - {(message) => { - const queuedMeta = createMemo(() => queuedFollowUpMetaByMessageId().get(message.id)); + + {(messageId) => { + const queuedMeta = createMemo(() => queuedFollowUpMetaByMessageId().get(messageId)); return ( - props.onApprove(message.id, approval)} - onSkip={(toolId) => props.onSkip(message.id, toolId)} - onAnswerQuestion={(question, answers) => - props.onAnswerQuestion(message.id, question, answers) - } - onSkipQuestion={(questionId) => props.onSkipQuestion(message.id, questionId)} - onRetry={props.onRetry} - onRegenerate={ - message.id === regenerableMessageId() - ? () => props.onRegenerate?.(message.id) - : undefined - } - onEditPrompt={ - message.id === editablePromptMessageId() - ? () => props.onEditPrompt?.(message.id) - : undefined - } - onChangeModel={props.onChangeModel} - getModelRouteLabel={props.getModelRouteLabel} - modelRouteAlternative={props.getModelRouteAlternative?.(message)} - onUseModelRoute={props.onUseModelRoute} - queuedPosition={queuedMeta()?.position} - queuedCount={queuedMeta()?.count} - queuedPaused={queuedMeta()?.paused} - queuedSteering={queuedMeta()?.steering} - onEditQueued={ - queuedMeta() && !queuedMeta()?.steering && props.onEditQueuedFollowUp - ? () => { - const meta = queuedMeta(); - if (meta) props.onEditQueuedFollowUp?.(meta.id); - } - : undefined - } - onCancelQueued={ - queuedMeta() && !queuedMeta()?.steering && props.onCancelQueuedFollowUp - ? () => { - const meta = queuedMeta(); - if (meta) props.onCancelQueuedFollowUp?.(meta.id); - } - : undefined - } - /> + + {(message) => ( + props.onApprove(messageId, approval)} + onSkip={(toolId) => props.onSkip(messageId, toolId)} + onAnswerQuestion={(question, answers) => + props.onAnswerQuestion(messageId, question, answers) + } + onSkipQuestion={(questionId) => props.onSkipQuestion(messageId, questionId)} + onRetry={props.onRetry} + onRegenerate={ + messageId === regenerableMessageId() + ? () => props.onRegenerate?.(messageId) + : undefined + } + onEditPrompt={ + messageId === editablePromptMessageId() + ? () => props.onEditPrompt?.(messageId) + : undefined + } + onChangeModel={props.onChangeModel} + getModelRouteLabel={props.getModelRouteLabel} + modelRouteAlternative={props.getModelRouteAlternative?.(message())} + onUseModelRoute={props.onUseModelRoute} + queuedPosition={queuedMeta()?.position} + queuedCount={queuedMeta()?.count} + queuedPaused={queuedMeta()?.paused} + queuedSteering={queuedMeta()?.steering} + onEditQueued={ + queuedMeta() && !queuedMeta()?.steering && props.onEditQueuedFollowUp + ? () => { + const meta = queuedMeta(); + if (meta) props.onEditQueuedFollowUp?.(meta.id); + } + : undefined + } + onCancelQueued={ + queuedMeta() && !queuedMeta()?.steering && props.onCancelQueuedFollowUp + ? () => { + const meta = queuedMeta(); + if (meta) props.onCancelQueuedFollowUp?.(meta.id); + } + : undefined + } + /> + )} + ); }} diff --git a/frontend-modern/src/components/AI/Chat/__tests__/ChatMessages.test.tsx b/frontend-modern/src/components/AI/Chat/__tests__/ChatMessages.test.tsx index 25d40557f..09518c33c 100644 --- a/frontend-modern/src/components/AI/Chat/__tests__/ChatMessages.test.tsx +++ b/frontend-modern/src/components/AI/Chat/__tests__/ChatMessages.test.tsx @@ -115,6 +115,79 @@ function setScrollMetrics( } describe('ChatMessages', () => { + it('does not mutate shared tool evidence when completed status rows are removed', () => { + const a = { name: 'pulse_query', input: 'client', output: 'client evidence', success: true }; + const b = { name: 'pulse_alerts', input: 'alerts', output: 'alert evidence', success: true }; + const pendingA = { id: 'a', name: a.name, input: a.input }; + const pendingB = { id: 'b', name: b.name, input: b.input }; + const [messages, setMessages] = createSignal([ + makeMessage({ + role: 'assistant', + toolCalls: [], + streamEvents: [ + { + type: 'workflow_status', + workflowStatus: { phase: 'provider_start', message: 'Starting' }, + }, + { type: 'pending_tool', toolId: 'a', pendingTool: pendingA }, + { type: 'pending_tool', toolId: 'b', pendingTool: pendingB }, + ], + pendingTools: [pendingA, pendingB], + }), + ]); + render(() => ); + setMessages([ + makeMessage({ + role: 'assistant', + toolCalls: [a], + streamEvents: [ + { + type: 'workflow_status', + workflowStatus: { phase: 'provider_start', message: 'Starting' }, + }, + { type: 'tool', toolId: 'a', tool: a }, + { type: 'pending_tool', toolId: 'b', pendingTool: pendingB }, + ], + pendingTools: [pendingB], + }), + ]); + setMessages([ + makeMessage({ + role: 'assistant', + toolCalls: [a, b], + streamEvents: [ + { + type: 'workflow_status', + workflowStatus: { phase: 'provider_start', message: 'Starting' }, + }, + { type: 'tool', toolId: 'a', tool: a }, + { type: 'tool', toolId: 'b', tool: b }, + ], + pendingTools: [], + }), + ]); + setMessages([ + makeMessage({ + role: 'assistant', + toolCalls: [a, b], + streamEvents: [ + { type: 'tool', toolId: 'a', tool: a }, + { type: 'tool', toolId: 'b', tool: b }, + ], + pendingTools: [], + }), + ]); + expect(a).toEqual({ + name: 'pulse_query', + input: 'client', + output: 'client evidence', + success: true, + }); + expect( + capturedMessageItemProps[0].message.streamEvents?.map((event) => event.tool?.output), + ).toEqual(['client evidence', 'alert evidence']); + }); + describe('empty transcript', () => { it('keeps the transcript blank when there are no messages or resume actions', () => { const { container } = render(() => ); diff --git a/frontend-modern/src/components/AI/Chat/__tests__/useChat.test.ts b/frontend-modern/src/components/AI/Chat/__tests__/useChat.test.ts index 13e1ec523..74f4cf2fb 100644 --- a/frontend-modern/src/components/AI/Chat/__tests__/useChat.test.ts +++ b/frontend-modern/src/components/AI/Chat/__tests__/useChat.test.ts @@ -2219,6 +2219,96 @@ describe('useChat', () => { dispose(); }); + it('preserves distinct invocation IDs across concurrent same-name tool updates', async () => { + const { getFireEvent } = setupWithEventCapture(); + const { value: chat, dispose } = withRoot(() => useChat({ sessionId: 's' })); + await chat.sendMessage('hi'); + const fire = getFireEvent(); + const inputA = '{"action":"search","query":"client"}'; + const inputB = '{"action":"search","query":"Tower"}'; + const assistant = () => chat.messages().find((m) => m.role === 'assistant')!; + fire({ type: 'tool_start', data: { id: 'a', name: 'pulse_query', input: inputA } }); + fire({ type: 'tool_start', data: { id: 'b', name: 'pulse_query', input: inputB } }); + expect(assistant().pendingTools?.map((tool) => [tool.id, tool.input])).toEqual([ + ['a', inputA], + ['b', inputB], + ]); + fire({ + type: 'tool_progress', + data: { id: 'a', name: 'pulse_query', message: 'Reading client' }, + }); + fire({ type: 'tool_start', data: { id: 'a', name: 'pulse_query', input: inputA } }); + expect(assistant().pendingTools?.map((tool) => tool.id)).toEqual(['a', 'b']); + expect(assistant().pendingTools?.[1].progress).toBeUndefined(); + fire({ + type: 'tool_end', + data: { id: 'a', name: 'pulse_query', output: 'client evidence', success: true }, + }); + expect( + assistant() + .streamEvents?.filter((event) => event.type === 'tool' || event.type === 'pending_tool') + .map((event) => [ + event.type, + event.toolId, + event.tool?.input || event.pendingTool?.input, + ]), + ).toEqual([ + ['tool', 'a', inputA], + ['pending_tool', 'b', inputB], + ]); + fire({ + type: 'tool_end', + data: { id: 'b', name: 'pulse_query', output: 'host unavailable', success: false }, + }); + expect(assistant().pendingTools).toEqual([]); + expect(assistant().toolCalls?.map((tool) => [tool.input, tool.output, tool.success])).toEqual( + [ + [inputA, 'client evidence', true], + [inputB, 'host unavailable', false], + ], + ); + dispose(); + }); + + it('does not cancel or approve a sibling invocation with the same name', async () => { + const { getFireEvent } = setupWithEventCapture(); + const { value: chat, dispose } = withRoot(() => useChat({ sessionId: 's' })); + await chat.sendMessage('hi'); + const fire = getFireEvent(); + const assistant = () => chat.messages().find((m) => m.role === 'assistant')!; + for (const id of ['a', 'b']) { + fire({ + type: 'tool_start', + data: { id, name: 'pulse_control', input: JSON.stringify({ resource_id: id }) }, + }); + fire({ + type: 'approval_needed', + data: { + tool_id: id, + tool_name: 'pulse_control', + approval_id: `approval-${id}`, + command: id, + }, + }); + } + fire({ type: 'tool_end', data: { id: 'a', name: 'pulse_control', output: 'completed a' } }); + expect(assistant().pendingApprovals?.map((approval) => approval.toolId)).toEqual(['b']); + expect( + assistant() + .streamEvents?.filter((event) => event.type === 'approval') + .map((event) => event.approval?.toolId), + ).toEqual(['b']); + fire({ type: 'tool_start', data: { id: 'c', name: 'pulse_control', input: '{}' } }); + fire({ type: 'tool_cancel', data: { id: 'c', name: 'pulse_control', reason: 'Skipped' } }); + expect(assistant().pendingTools?.map((tool) => tool.id)).toEqual(['b']); + expect( + assistant() + .streamEvents?.filter((event) => event.type === 'tool_cancel') + .map((event) => event.toolId), + ).toEqual(['c']); + dispose(); + }); + it('processes tool_start events', async () => { const { getFireEvent } = setupWithEventCapture(); const { value: chat, dispose } = withRoot(() => useChat({ sessionId: 's' })); diff --git a/frontend-modern/src/components/AI/Chat/hooks/useChat.ts b/frontend-modern/src/components/AI/Chat/hooks/useChat.ts index f49d8f4b3..eb96406a2 100644 --- a/frontend-modern/src/components/AI/Chat/hooks/useChat.ts +++ b/frontend-modern/src/components/AI/Chat/hooks/useChat.ts @@ -821,8 +821,8 @@ export function useChat(options: UseChatOptions = {}) { const matchesTool = (tool?: PendingTool, toolId?: string) => { if (!tool) return false; - if (data.id && toolId === data.id) return true; - if (data.id && tool.id === data.id) return true; + // A supplied invocation ID must never fall back to a sibling tool name. + if (data.id) return toolId === data.id || tool.id === data.id; return normalizedName !== '' && normalizeChatToolName(tool.name) === normalizedName; }; @@ -903,8 +903,8 @@ export function useChat(options: UseChatOptions = {}) { const matchesTool = (tool?: PendingTool, toolId?: string) => { if (!tool) return false; - if (data.id && toolId === data.id) return true; - if (data.id && tool.id === data.id) return true; + // A supplied invocation ID must never fall back to a sibling tool name. + if (data.id) return toolId === data.id || tool.id === data.id; return normalizedName !== '' && normalizeChatToolName(tool.name) === normalizedName; }; @@ -1519,8 +1519,8 @@ export function useChat(options: UseChatOptions = {}) { const normalizedName = normalizeChatToolName(data.name || ''); const matchesTool = (tool?: PendingTool, toolId?: string) => { if (!tool) return false; - if (data.id && toolId === data.id) return true; - if (data.id && tool.id === data.id) return true; + // A supplied invocation ID must never fall back to a sibling tool name. + if (data.id) return toolId === data.id || tool.id === data.id; return normalizedName !== '' && normalizeChatToolName(tool.name) === normalizedName; }; const pendingTools = msg.pendingTools || []; @@ -1588,8 +1588,7 @@ export function useChat(options: UseChatOptions = {}) { data.input && data.input.trim() ? data.input : resolvedPendingTool?.input || '{}'; const completedRawInput = data.raw_input ?? resolvedPendingTool?.rawInput; const matchesCompletedTool = (toolId?: string, toolName?: string) => { - if (data.id && toolId === data.id) return true; - if (completedToolId && toolId === completedToolId) return true; + if (completedToolId) return toolId === completedToolId; return ( normalizedEndName !== '' && normalizeChatToolName(toolName || '') === normalizedEndName