From c421d09340cef2ee30bb766700a03e95cdb8069a Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Fri, 18 Sep 2026 19:49:27 +0300 Subject: [PATCH 01/10] docs(specs): chat message reliability spec, plan and tasks (CLEAN-102) Spec 015: lost, duplicated and misordered chat messages, delivery state, timestamps, scroll on send, agent status consistency. Research records the causes found in code and the four defects reproduced against the local hub with the scripted socket probe. Co-Authored-By: Claude Fable 5.1 --- .specify/feature.json | 2 +- .../checklists/requirements.md | 43 ++ .../contracts/bridle-socket.md | 117 +++++ .../data-model.md | 104 +++++ specs/015-chat-message-reliability/plan.md | 232 ++++++++++ specs/015-chat-message-reliability/probe.mjs | 144 ++++++ .../quickstart.md | 139 ++++++ .../015-chat-message-reliability/research.md | 405 ++++++++++++++++ specs/015-chat-message-reliability/spec.md | 433 ++++++++++++++++++ specs/015-chat-message-reliability/tasks.md | 255 +++++++++++ 10 files changed, 1873 insertions(+), 1 deletion(-) create mode 100644 specs/015-chat-message-reliability/checklists/requirements.md create mode 100644 specs/015-chat-message-reliability/contracts/bridle-socket.md create mode 100644 specs/015-chat-message-reliability/data-model.md create mode 100644 specs/015-chat-message-reliability/plan.md create mode 100644 specs/015-chat-message-reliability/probe.mjs create mode 100644 specs/015-chat-message-reliability/quickstart.md create mode 100644 specs/015-chat-message-reliability/research.md create mode 100644 specs/015-chat-message-reliability/spec.md create mode 100644 specs/015-chat-message-reliability/tasks.md diff --git a/.specify/feature.json b/.specify/feature.json index 35781f75..1ce695e1 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1,3 +1,3 @@ { - "feature_directory": "specs/014-a2a-protocol-upgrade" + "feature_directory": "specs/015-chat-message-reliability" } diff --git a/specs/015-chat-message-reliability/checklists/requirements.md b/specs/015-chat-message-reliability/checklists/requirements.md new file mode 100644 index 00000000..89669ca3 --- /dev/null +++ b/specs/015-chat-message-reliability/checklists/requirements.md @@ -0,0 +1,43 @@ +# Specification Quality Checklist: Chat message reliability + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-09-18 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Validated in one pass on 2026-09-18; no items failed. +- No [NEEDS CLARIFICATION] markers were used. The choices that could have been questions + were settled as documented defaults under Assumptions and are the ones most worth a + second look in `/speckit-clarify`: the slow / not-delivered thresholds (5 s / 30 s), + unsent messages kept per device with manual resend only, and no retroactive repair of + conversations already saved with fused agent messages. +- The spec names product surfaces (admin panel, app console, home page) and refers to + the screenshots; these are scope and evidence, not implementation. +- Every reported failure is intermittent and none has been reproduced yet. The + Assumptions section says so; reproduction is the first job of `/speckit-plan`. diff --git a/specs/015-chat-message-reliability/contracts/bridle-socket.md b/specs/015-chat-message-reliability/contracts/bridle-socket.md new file mode 100644 index 00000000..af0c144b --- /dev/null +++ b/specs/015-chat-message-reliability/contracts/bridle-socket.md @@ -0,0 +1,117 @@ +# Contract: browser ↔ hub socket protocol (additive changes) + +Namespace and events as handled by +`api/src/slices/bridle/handlers/bridleClientWs.handler.ts`. **Every change is optional +and additive**: a client that sends none of the new fields and passes no ack callback — +the embed widget, an old cached bundle — gets exactly today's behaviour. + +## Handshake `auth` + +| Field | Type | Status | Meaning | +|-------|------|--------|---------| +| `token` / share fields / `capabilities` / `prompt` | — | unchanged | | +| `lastSeq` | number | **new, optional** | Highest hub `seq` this client has applied for this agent. When present, the hub replays buffered events with `seq > lastSeq`, in order, immediately after `welcome` and before any live event. Absent or `0`: no replay (first connect). | + +## `welcome` (hub → browser) + +`{ clientId }` → `{ clientId, seq }` where `seq` is the hub's current sequence for this +client. **New, optional to read.** A client whose `lastSeq` is greater than `seq` (the +hub restarted and lost its buffer) resets its `lastSeq` to `seq` and runs the transcript +reconcile instead of waiting for a replay. + +## `message` (browser → hub) + +```ts +{ + text?: string + parts?: BridlePart[] + images?: Array<{ base64: string; mediaType: string }> + attachmentIds?: string[] + clientMessageId?: string // NEW, optional — UUID minted by the browser +} +``` + +**Acknowledgement (NEW, optional)** — when the browser passes a socket.io ack callback, +the hub calls it exactly once: + +```ts +| { status: 'accepted'; messageId: string; ts: number; duplicate?: true } +| { status: 'rejected'; code: 'AGENT_OFFLINE' | 'ATTACHMENT_FAILED' | 'SHARE_REJECTED' | 'EMPTY'; message?: string } +``` + +Rules: + +1. `accepted` is sent after the message has been handed to a connected agent socket. + `messageId` equals `clientMessageId` when one was supplied, otherwise the id the hub + minted. `ts` is the hub's clock at acceptance. +2. The hub forwards `clientMessageId` to the agent as the message's `messageId` + (today: a hub-minted UUID). +3. **Idempotency**: a `clientMessageId` already accepted from the same client within the + last 10 minutes is answered `accepted` with `duplicate: true` and is **not** forwarded + again. +4. **Agent offline**: with an ack callback → `rejected / AGENT_OFFLINE`, and the + synthetic "Agent is not connected. Please try again later." agent message is **not** + sent. Without an ack callback → today's synthetic message, unchanged. +5. **Attachment failure**: with an ack callback → `rejected / ATTACHMENT_FAILED` *and* + the existing `message_error` event (kept for the composer-level notice). Without → + `message_error` only, unchanged. +6. **Share link rejected**: `rejected / SHARE_REJECTED`, then the existing + `bridle_error` + disconnect, unchanged. +7. The browser treats a missing ack after 30 s as `failed / TIMEOUT`. A late ack for a + message already marked failed moves it to delivered. + +## Events hub → browser (`message`, `stream`, `stream_end`, `typing`, `thinking`, `debug`, `agent_status`, `message_error`) + +Each payload gains **`seq: number`** (new; per client, strictly increasing, assigned by +the hub when it routes the event). Payloads are otherwise unchanged. Old clients ignore +it. + +Browser rules: + +- Remember the highest `seq` applied per conversation (`lastSeq`), in memory and, for + the app, alongside the persisted conversation. +- Ignore an event whose `seq` is not greater than `lastSeq` (replay overlap is harmless). +- Applying a replayed `stream` / `stream_end` for a `messageId` already on screen updates + that bubble; it never adds a second one (unchanged rule, now relied upon). + +## Several sockets per identity (NEW behaviour) + +Today the hub holds one socket per `clientId:agentId` and a new connection silently +replaces the previous one (every Owner/Admin has `clientId = "admin"`). New rule: + +- Any number of sockets may be registered for one identity. Every hub → browser event for + that identity is sent to **all** of them, with the same `seq`. +- Each socket reports its own `lastSeq` and gets its own replay. +- Disconnecting one socket does not affect the others; per-identity turn tracking is + cleared when the last socket leaves. +- **`user_message` (hub → browser, NEW)**: when a socket's `message` is accepted, the hub + sends `{ type: 'user_message', messageId, text, attachments?, ts, seq }` to the + identity's **other** sockets, so a second view shows the question and not only the + answer. Clients ignore a `user_message` whose `messageId` they already hold. Old + clients ignore the unknown event. + +## Buffering guarantees (hub) + +- Events addressed to a client are buffered even while no socket is registered for it. +- Bounds: last 500 events or 10 minutes per client; whichever is hit first evicts the + oldest. Buffer and sequence are dropped after 10 minutes without a registered socket. +- In-memory only: a hub restart loses buffers; clients detect it via `welcome.seq` and + fall back to the transcript. + +## HTTP + +No new routes. `GET /api/agent/:agentId/transcript` (existing, used by the admin) is +additionally called by the app console for the watchdog safety net, through a new method +on the app's bridle gateway using the generated SDK. Response shape unchanged; equal +`ts` values are returned in file order. + +## Agent ↔ hub (informative — runtime is outside this repository) + +Required of the runtime for spec US3 and for exact de-duplication: + +- Persist the incoming `messageId` as the `id` of the transcript's `user` event. +- Persist **one `assistant` event per emitted message**, with the wire `messageId` as + its `id`, instead of one event per turn with concatenated text. + +Desirable: a `persisted` acknowledgement to the hub once the user event is written, so +"delivered" can mean "saved" rather than "handed over". diff --git a/specs/015-chat-message-reliability/data-model.md b/specs/015-chat-message-reliability/data-model.md new file mode 100644 index 00000000..7ab3ca9e --- /dev/null +++ b/specs/015-chat-message-reliability/data-model.md @@ -0,0 +1,104 @@ +# Data Model: Chat message reliability + +No database entity changes. Everything below is client-side view state, browser storage, +or in-memory hub state. Names are the intended TypeScript shapes; the app uses the +`IBridle*` naming of `app/slices/bridle/domain/bridle.types.ts`, the admin mirrors them +in its store file. + +## Message (client) + +Extends today's `IBridleMessage` / `IBridleMessageData`. New fields are optional so that +conversations already stored in `localStorage` still load. + +| Field | Type | Notes | +|-------|------|-------| +| `id` | string | Person's message: the `clientMessageId` (UUID) generated at send. Agent message: the wire `messageId`. Replayed message: the transcript event id. One id per message end to end (research F7). | +| `role` | user \| agent | unchanged | +| `text`, `attachments` / `parts` | — | unchanged | +| `ts` | number (epoch ms) | **Display only.** Local send time until the ack returns the hub's `ts`, then that value. Never used for ordering. | +| `seq` | number | Per-conversation arrival sequence, assigned by the store on append. The only ordering key. | +| `delivery` | `sending` \| `slow` \| `delivered` \| `failed` | Person's messages only. Absent means delivered (legacy and replayed messages). | +| `failureCode` | string? | `AGENT_OFFLINE`, `TIMEOUT`, `ATTACHMENT_FAILED`, `SHARE_REJECTED`, `OFFLINE`. Drives the wording under the bubble. | +| `streaming` | boolean? | unchanged; never persisted | + +**Validation**: `seq` is strictly increasing within a conversation. A message with +`delivery` other than `delivered` always has `role = user`. + +**Migration of stored conversations (app)**: on hydrate, messages without `seq` are +numbered in stored array order; messages without `delivery` are treated as delivered. + +## Delivery state machine + +```text + send() 5 s without ack 30 s without ack + (none) ──────────► sending ───────────────────► slow ─────────────────────► failed + │ │ │ ▲ + │ ack accepted │ ack accepted │ │ resend() (same id) + ▼ ▼ ▼ │ + delivered ◄────────────────────┘ sending + ▲ + │ ack rejected / message_error ──► failed (with failureCode) +``` + +- A page load turns every persisted `sending` / `slow` into `failed` (`TIMEOUT`): the + ack can no longer arrive on a socket that no longer exists. +- `discard()` removes a `failed` message from the conversation and the outbox. +- Thresholds (`SLOW_MS = 5000`, `FAILED_MS = 30000`) live in `utils/delivery.ts`. + +## Thinking block (client) + +Unchanged except: gains `seq` (assigned when the segment opens) and is ordered by it. +The `ts` anchor workaround `Math.max(e.ts, lastTs + 1)` is removed. + +## Flow item (view) + +What the template iterates: `{ key, seq, kind: 'message' | 'block' | 'day', … }`, +produced by the pure `buildChatFlow(messages, blocks, locale)` in `utils/chatFlow.ts`, +sorted by `seq`. A `day` separator is inserted where the calendar day of `ts` changes +between two consecutive messages (it takes the `seq` of the message it precedes). + +## Outbox (browser storage) + +- **App**: no separate structure — the persisted conversation + (`bridle:conversation:`) already holds the person's messages; `delivery` and + `failureCode` ride along. +- **Admin**: `localStorage["bridle:outbox::"]` = array of person's + messages whose `delivery !== 'delivered'`, without image bytes (attachment references + only). Merged under the loaded transcript; an entry is dropped once its id is present + in the transcript (interim fallback: same text within ±2 min — research D3). + +## Conversation (admin store) + +The admin store's single `messages` / `thinkingBlocks` / socket become a record keyed by +conversation (`:`), mirroring the app store: + +| State | Scope | +|-------|-------| +| `messages`, `thinkingBlocks`, `closedTurns`, `nextSeq`, `lastHubSeq` | per conversation | +| `isTyping`, `isConnected`, `isAgentConnected`, transcript cursor | per conversation | +| socket | per conversation | +| `markdownEnabled`, panel open/closed | global (unchanged) | + +## Channel ownership (app store) + +`channels: Map`. `acquire(conv)` +increments (and cancels a pending close); `release(conv)` decrements and, at zero, +schedules the close after a short grace delay so a route change reuses the socket. + +## Hub state (API, in memory) + +| Structure | Key | Contents | Bounds | +|-----------|-----|----------|--------| +| `outSeq` | clientKey (`clientId:agentId`) | last sequence number issued | — | +| `replay` | clientKey | ring buffer of `{ seq, event }` routed to that client, kept whether or not a socket is registered | 500 events **or** 10 min, evicted with the client's other state after 10 min idle | +| `seenMessageIds` | clientKey | `clientMessageId → acceptedAt` | 10 min TTL, max 200 per client | + +Registering a client no longer resets these; unregistering keeps them for the idle +window so a reconnect can catch up. + +## Transcript message (API read model) + +`TranscriptMessage` is unchanged in shape. Behavioural changes: equal `ts` values keep +file (append) order; one `assistant` event per emitted message is the expected runtime +format going forward (research D6). Older single-event turns keep rendering as one +bubble. diff --git a/specs/015-chat-message-reliability/plan.md b/specs/015-chat-message-reliability/plan.md new file mode 100644 index 00000000..65fde545 --- /dev/null +++ b/specs/015-chat-message-reliability/plan.md @@ -0,0 +1,232 @@ +# Implementation Plan: Chat message reliability + +**Branch**: `fix/CLEAN-102-chat-message-reliability` (spec dir `015-chat-message-reliability`) | **Date**: 2026-09-18 | **Spec**: [spec.md](./spec.md) + +**Input**: Feature specification from `/specs/015-chat-message-reliability/spec.md`, plus +the planning request: "что насчёт сообщений где порядок не правильный? и при отправке +сообщения нужно скроллить вниз если мы где-то наверху, это обычное поведение". + +**Note**: `setup_plan.py` was not run — Python is not installed on this machine. The +plan file and feature directory were set up by hand; `.specify/feature.json` points at +this feature. + +## Summary + +The chat loses, doubles and reorders messages because it has no delivery confirmation, +no stable message identity, no recovery of missed events, and orders the timeline by +timestamps taken from two different clocks. The plan fixes these at their source rather +than per symptom: + +1. **Order by arrival sequence, not by time** (answers the ordering question directly — + see below). Time becomes display-only. +2. **Acknowledged sends** with a client-generated message id, an idempotent hub, and a + per-device outbox — gives "sending / slow / delivered / not delivered", Resend, and + survival across reloads. +3. **Hub replay buffer** so events that arrive while a browser is reconnecting are + delivered afterwards — fixes hung answers and the landing → agent handoff. +4. **Conversation-scoped chat state** in the admin store and reference-counted channel + ownership in the app store — stops two chat views from clobbering each other. +5. **Timestamps and delivery state under each message**, and one scroll rule: sending + always scrolls to the bottom, incoming content only follows a reader already there. +6. **Message boundaries in the transcript** — a change in the sibling repository + `CleanSlice/runtime` (see Dependencies). +7. **Several sockets per identity** *(added after the local experiments)* — the hub keeps + one socket per identity and every Owner/Admin shares the identity `admin`, so the last + view to connect takes all events and earlier views go silent. Reproduced (research + E3/F10). The hub fans out to every socket of the identity. +8. **Single source of truth for agents** *(added on request; approach set by the + request: SSOT, so the class of defect does not come back)* — the list row stays on + "Deploying" while the detail shows "Failed" because the same agent exists as several + independent copies (research F11). An agent lives once in `agentStore`; every fetch + upserts into it, the status stream patches it, components render from it by id — in + both `admin/` and `app/` (research D12). + +**Testing approach** (per the request: test whichever way is convenient; the goal is to +find the cause and fix the defect): no test-first mandate. Hub behaviour is checked with +the existing jest specs and the scripted socket probe; ordering, delivery and +reconciliation logic as pure functions with `bun test`; browser-only behaviour with a +headless Playwright script where that is quicker than doing it by hand. + +**What has been reproduced** (local stack, scripted socket client — research "Update: +experiments"): no delivery ack (E1), three unrelated ids per message (E2), a second view +stealing the first one's events (E3), an answer lost across a reconnect while the +transcript has it (E4). **Not yet observed in a browser**: the ordering bug (needs clock +skew), scrolling, the landing-page handoff (F5) and the admin store clobbering (F3). + +### The ordering question, answered + +The wrong order has a concrete cause, confirmed in the source (research F1). Both chat +views sort the timeline with `items.sort((a, b) => a.ts - b.ts)`. Your message carries +the **browser's** clock; the agent's messages carry the **agent's** clock. When the +browser clock is ahead by more than the agent's response time, the reply sorts above +your question — so your message "drops below" it. It is intermittent because it depends +on the skew and on how quickly the first agent message arrives (a greeting or a short +acknowledgement arrives in milliseconds). + +Fix (research D1): every item gets a per-conversation sequence number when it is +appended, and the view orders by that. Arrival order is the order you experienced, and +clock skew cannot affect it. This is the first implementation slice because it is small, +self-contained and removes the most visible artifact. + +The duplicate in the same screenshot is a different defect (research F5/F2 — the send +that silently went nowhere and was sent again); it is covered by items 2–4. + +## Technical Context + +**Language/Version**: TypeScript 5.x throughout. API: NestJS on Bun/Node. Clients: Nuxt 4 +/ Vue 3 (`app/` console with Pinia setup stores and i18n; `admin/` with a Pinia options +store, English only). + +**Primary Dependencies**: socket.io (server + client) for the hub; Pinia; generated +OpenAPI SDK (`openapi-ts`) for HTTP. No new runtime dependency is planned. + +**Storage**: No database change. Browser `localStorage` (app: existing +`bridle:conversation:` gains fields; admin: new `bridle:outbox::`). +Hub state stays in memory (new bounded replay buffer and seen-id cache). Transcript JSONL +on S3 is written by the agent runtime and only read here. + +**Testing**: API — jest (`cd api && bun run test`), extending the existing specs. +`app/` and `admin/` have no test runner; pure logic goes under each slice's `utils/` and +is covered with `bun test` (no new dependency). UI behaviour is validated via +[quickstart.md](./quickstart.md). Type safety: `npx nuxt typecheck` in `app/` and +`admin/` (not `bun run typecheck` in `app/` — it regenerates the SDK). + +**Target Platform**: Browsers (desktop first) against the API in the k3s cluster; local +stack via the repo's Makefile. + +**Project Type**: Web application — one API, two web clients sharing a wire protocol. + +**Performance Goals**: Delivery state visible within 5 s (slow) / 30 s (failed). A +finished answer visible in an open chat within 10 s in ≥ 99 % of turns (SC-005). Replay +after reconnect completes before live traffic resumes. + +**Constraints**: The `message` wire contract is shared with the embed widget — every +protocol change must be additive and optional. The hub is single-instance in memory +(already true today). New user-visible strings in `app/` follow `docs/i18n.md`. The +agent runtime's source is outside this repository. + +**Scale/Scope**: Two chat surfaces (three entry points in the app: landing hero, agent +page, share page; two in the admin: Rancher panel, agent chat tab), one hub, one +transcript reader. Roughly: API ~4 files + specs, app ~8 files, admin ~5 files, where +the admin store refactor (singleton → conversation-scoped) is the largest piece. + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +`.specify/memory/constitution.md` is still the unfilled template — it defines no +principles, so there are no constitutional gates to evaluate. The gates applied instead +are the repository's own rules (`CLAUDE.md`, `.cursor/rules/project.mdc`): + +| Gate | Status | +|------|--------| +| Work tracked under a `CLEAN-` id, branch from `origin/main` | Pass — CLEAN-102, `fix/CLEAN-102-chat-message-reliability` | +| No hand-written types where OpenAPI generates them | Pass — HTTP surface unchanged except reuse of the existing transcript route; socket payload types are not OpenAPI-generated | +| `app/` strings via `en.json` + `i18n:sync`; `admin/` English only | Pass — planned in D9 | +| Slice structure respected (`data/`, `domain/`, `stores/`, `components/`, `utils/`) | Pass — see Project Structure | + +**Post-design re-check**: unchanged. No violations, Complexity Tracking left empty. + +## Project Structure + +### Documentation (this feature) + +```text +specs/015-chat-message-reliability/ +├── spec.md +├── plan.md # this file +├── research.md # Phase 0 — findings F1–F9, decisions D1–D10, open items +├── data-model.md # Phase 1 — client-side entities and state machines +├── quickstart.md # Phase 1 — validation scenarios +├── contracts/ +│ └── bridle-socket.md # Phase 1 — additive changes to the browser ↔ hub protocol +├── checklists/requirements.md +└── tasks.md # Phase 2 (/speckit-tasks — not created here) +``` + +### Source Code (repository root) + +```text +api/src/slices/bridle/ +├── handlers/bridleClientWs.handler.ts # ack callback, clientMessageId, lastSeq handshake +├── data/bridle.gateway.ts # forward client id, seen-id cache, per-client seq + replay buffer +├── domain/bridle.types.ts # additive payload types +├── domain/bridle.gateway.ts # interface additions +└── (specs next to each file) +api/src/slices/agent/file/domain/ +└── transcriptReader.service.ts # file-order tie-break, per-message assistant events + +app/slices/bridle/ +├── domain/bridle.types.ts # seq, delivery, clientMessageId on IBridleMessage +├── domain/bridle.gateway.ts # send() returns an ack promise; transcript tail +├── data/bridle.gateway.ts # emit with ack + timeout, lastSeq in handshake +├── stores/bridle.ts # seq, delivery machine, outbox, acquire/release, replay +├── utils/chatFlow.ts # pure: ordering + day separators (new, tested) +├── utils/delivery.ts # pure: state machine + thresholds (new, tested) +├── components/bridle/chat/Provider.vue # order by seq, scroll rule +├── components/bridle/chat/Message.vue # time, delivery state, Resend / Discard +└── i18n/locales/en.json # new keys → `bun run i18n:sync` (never hand-write ru.json) + +admin/slices/bridle/ +├── stores/bridle.ts # conversation-scoped state, ack, outbox, replay, seq +├── utils/chatFlow.ts, utils/delivery.ts # same pure logic as the app (new, tested) +├── components/bridle/Provider.vue # order by seq, scroll on send +└── components/bridle/Message.vue # time, delivery state, Resend / Discard +admin/slices/rancher/components/rancher/Provider.vue # passes its own conversation key +``` + +**Structure Decision**: No new slice and no shared package. `app/` and `admin/` are +separate Nuxt projects with no shared workspace library today, so the two small pure +modules (`chatFlow`, `delivery`) are duplicated rather than introducing cross-project +packaging for this fix; the duplication is deliberate and noted for a later cleanup. + +## Implementation slices (order) + +Each slice is independently shippable; the order follows the spec's priorities and puts +the cheapest high-value fix first. + +1. **Ordering by sequence + scroll rule** (both clients, no API change) — spec US2, + FR-006/007/024/025. Smallest change, removes the most visible artifact. +2. **Timestamps under messages** (both clients) — US5 part 1, FR-017/018. +3. **Acknowledged send + idempotent hub + delivery states + outbox** (API + both + clients) — US1, US5 part 2, FR-001–005, 019–021. +4. **Channel ownership (app) and conversation-scoped store (admin)** — US1/US2, + FR-001/008; removes the handoff and cross-chat clobbering. +5. **Hub replay buffer + reconnect catch-up + transcript safety net** — US4, + FR-012/014–016. +6. **Transcript boundaries** — US3, FR-010–013. API reader part here; the runtime part is + a separate ticket and PR in `CleanSlice/runtime`. +7. **Agent status consistency (admin)** — US6, FR-027. Independent of everything above; + small; can ship as its own PR at any point. + +**Fan-out to several sockets per identity** (FR-026, research D11) is part of slice 3's +hub work: the ack, the seen-id cache, the sequence and the replay buffer are all keyed by +identity, so the registry has to become multi-socket before they are built on it. It is +the change most likely to remove "the answer never arrived" reports on its own. + +Slices 3 and 4 touch the same admin store; 4 is sequenced after 3 only for the app — +for the admin the refactor in 4 should land **before** 3 to avoid writing the outbox +twice. `/speckit-tasks` should order them that way. + +## Dependencies and risks + +- **Agent runtime — `CleanSlice/runtime`, sibling checkout `../runtime`** — needed for + slice 6 (one transcript event per emitted message with the wire `messageId` as its id; + persist the forwarded user `messageId`) and for closing the delivered-vs-saved gap in + D2. Confirmed in its source: `loop.service.ts` `sendFinalResponse` writes one + `assistant` event per turn with the accumulated text and a random id. Without that + change US3 cannot be met for the admin and the chat history pages; everything else can + ship. It needs its own ticket, branch and PR in that repository. +- **Local validation gap** — on this Windows machine the runtime's session file lands in + an NTFS alternate data stream and the API's transcript route returns nothing, so + reload-from-transcript scenarios must be validated on the cluster. +- **Two hypotheses (F3, F5)** are not reproduced. The fixes in slice 4 are sound + independently, but the claim that they explain the vanished landing-page question must + be confirmed in a running stack. +- **Embed widget compatibility** — all wire changes are optional fields and an optional + ack; a client that sends neither behaves as today. +- **Single API instance** — the replay buffer and seen-id cache are in memory. + +## Complexity Tracking + +No constitution violations to justify. diff --git a/specs/015-chat-message-reliability/probe.mjs b/specs/015-chat-message-reliability/probe.mjs new file mode 100644 index 00000000..3df9db62 --- /dev/null +++ b/specs/015-chat-message-reliability/probe.mjs @@ -0,0 +1,144 @@ +// Probe the local bridle hub the way the browser does. Prints no secrets. +import { createRequire } from 'node:module'; +import { readFileSync } from 'node:fs'; + +const REPO = 'C:/Users/maxim/orca/workspaces/ranch/chat-issues'; +const require = createRequire(REPO + '/package.json'); +const { io } = require('socket.io-client'); + +const API = 'http://localhost:3333'; +const AGENT = 'agent-bb620efe-abb5-4123-8ace-6d9b963387c7'; +const MODE = process.argv[2] || 'normal'; + +const env = {}; +for (const line of readFileSync(REPO + '/.env.project', 'utf8').split(/\r?\n/)) { + const i = line.indexOf('='); + if (i > 0) env[line.slice(0, i).trim()] = line.slice(i + 1).trim(); +} + +const t0 = Date.now(); +const rel = () => String(Date.now() - t0).padStart(6) + 'ms'; +const log = (...a) => console.log(rel(), ...a); +const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + +async function login() { + const res = await fetch(API + '/auth/login', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ email: env.RANCH_LOGIN, password: env.RANCH_PASS }), + }); + const json = await res.json().catch(() => ({})); + const data = json.data ?? json; + const token = data.accessToken ?? data.access_token ?? data.token; + log('login', res.status, 'keys:', Object.keys(data).join(','), 'token:', token ? 'yes' : 'NO'); + if (!token) throw new Error('no token'); + return token; +} + +function connect(token) { + const socket = io(API + '/ws/client', { + transports: ['websocket'], + reconnection: false, + auth: { agentId: AGENT, capabilities: ['thinking'], token }, + }); + const seen = []; + const short = (s) => (s ?? '').replace(/\s+/g, ' ').slice(0, 70); + socket.on('connect', () => log('connect', socket.id)); + socket.on('disconnect', (r) => log('disconnect', r)); + socket.on('connect_error', (e) => log('connect_error', e.message)); + socket.on('welcome', (d) => log('welcome', JSON.stringify(d))); + socket.on('bridle_error', (d) => log('bridle_error', JSON.stringify(d))); + socket.on('message_error', (d) => log('message_error', JSON.stringify(d))); + socket.on('agent_status', (d) => log('agent_status', JSON.stringify(d))); + socket.on('typing', () => { seen.push('typing'); log('typing'); }); + socket.on('thinking', (e) => { + seen.push('thinking'); + log('thinking', 'turn=' + String(e.turnId).slice(0, 8), e.done ? 'DONE' : 'step=' + (e.step?.label ?? e.step?.id ?? ''), 'skew=' + (e.ts - Date.now()) + 'ms'); + }); + let streamChunks = 0; + socket.on('stream', (d) => { + streamChunks++; + if (streamChunks === 1 || streamChunks % 25 === 0) log('stream #' + streamChunks, 'id=' + String(d.messageId).slice(0, 8), 'len=' + (d.text ?? '').length); + seen.push('stream'); + }); + socket.on('stream_end', (d) => { + seen.push('stream_end'); + log('stream_end', 'id=' + String(d.messageId).slice(0, 8), 'skew=' + ((d.ts ?? NaN) - Date.now()) + 'ms', 'seq=' + d.seq, '"' + short(d.text) + '"'); + streamChunks = 0; + }); + socket.on('message', (d) => { + seen.push('message'); + log('message', 'id=' + String(d.messageId).slice(0, 8), 'skew=' + ((d.ts ?? NaN) - Date.now()) + 'ms', '"' + short(d.text) + '"'); + }); + return { socket, seen }; +} + +async function transcriptTail(token, n) { + const res = await fetch(`${API}/api/agent/${AGENT}/transcript`, { + headers: { Authorization: 'Bearer ' + token }, + }); + const json = await res.json().catch(() => ({})); + const data = json.data ?? json; + const msgs = data.messages ?? []; + log('transcript', res.status, 'count=' + msgs.length, 'hasMore=' + data.hasMore); + for (const m of msgs.slice(-n)) { + console.log(' ', m.role.padEnd(9), 'id=' + String(m.id).slice(0, 8), new Date(m.ts).toISOString().slice(11, 23), '"' + (m.text ?? '').replace(/\s+/g, ' ').slice(0, 90) + '"'); + } + return msgs; +} + +const token = await login(); + +if (MODE === 'normal') { + const { socket, seen } = connect(token); + await sleep(1500); + let acked = false; + const clientMessageId = 'probe-' + Date.now(); + log('SEND (with ack callback + clientMessageId=' + clientMessageId + ')'); + socket.emit('message', { text: 'Привет! Это тест чата. Ответь одним коротким предложением.', clientMessageId }, (a) => { + acked = true; + log('ACK', JSON.stringify(a)); + }); + await sleep(45000); + log('ack callback called:', acked, '| events:', [...new Set(seen)].join(',')); + socket.close(); + await sleep(2000); + await transcriptTail(token, 6); +} + +if (MODE === 'gap') { + const a = connect(token); + await sleep(1500); + log('SEND long question'); + a.socket.emit('message', { text: 'Тест обрыва связи. Перечисли пять фактов о лошадях, по одному предложению на факт.' }); + // Drop the socket as soon as the agent shows any sign of life. + while (!a.seen.length && Date.now() - t0 < 30000) await sleep(100); + log('>>> dropping socket mid-turn (events so far: ' + a.seen.join(',') + ')'); + a.socket.close(); + await sleep(25000); + log('>>> reconnecting'); + const b = connect(token); + await sleep(25000); + log('events after reconnect:', b.seen.length ? [...new Set(b.seen)].join(',') : 'NONE'); + b.socket.close(); + await sleep(1500); + await transcriptTail(token, 4); +} + +if (MODE === 'steal') { + // Two "tabs" of the same admin user. A connects first and sends; B connects second. + const a = connect(token); + await sleep(1200); + const b = connect(token); + await sleep(1200); + log('SEND from tab A (the one that connected FIRST)'); + a.socket.emit('message', { text: 'Тест двух вкладок. Ответь одним словом: ок.' }); + await sleep(20000); + log('tab A received:', a.seen.length ? [...new Set(a.seen)].join(',') : 'NOTHING'); + log('tab B received:', b.seen.length ? [...new Set(b.seen)].join(',') : 'NOTHING'); + a.socket.close(); + b.socket.close(); + await sleep(1000); +} + +process.exit(0); diff --git a/specs/015-chat-message-reliability/quickstart.md b/specs/015-chat-message-reliability/quickstart.md new file mode 100644 index 00000000..d0aa9e8d --- /dev/null +++ b/specs/015-chat-message-reliability/quickstart.md @@ -0,0 +1,139 @@ +# Quickstart: validating chat message reliability + +How to prove the feature works end to end. Shapes and rules are in +[data-model.md](./data-model.md) and [contracts/bridle-socket.md](./contracts/bridle-socket.md); +this file only says what to run and what to expect. + +## Prerequisites + +- Local stack: `make setup` once, then `make dev` (API :3000, app :3001, admin :3002, + k3d). At least one agent in `running` state; for the delegation-style multi-message + turns, Rancher plus one peer. +- An admin login and an app console login. +- Browser DevTools (Network → throttling / Offline, Application → Local Storage). + +## Step 0 — Before any fix: capture the evidence (research open items 1 and 3) + +1. Pick one conversation that showed fused bubbles after reload. In the admin, open the + agent's Files → `data/sessions/.jsonl` and look at one multi-message turn. + **Record**: is there one `assistant` event for the turn or one per message? Does the + `user` event's `id` equal the `messageId` the hub forwarded (API log line for the + send)? This settles research F6 / F7 and the de-duplication rule in D3. +2. Reproduce the ordering bug on purpose: set the OS clock **2 minutes ahead**, send + "привет" to an agent that replies quickly. **Expected today**: the reply renders + above the question. Keep the clock skewed for scenario 1 below. +3. Reproduce the handoff: ask a question in the landing-page chat, immediately click + through to that agent's page. **Record** whether the chat shows "Reconnecting…", and + whether the answer arrives (research F5). +4. Admin: open the Rancher panel and an agent's Chat tab, start a turn in one, open the + other. **Record** whether the first chat's messages change (research F3). + +## Automated checks + +```bash +cd api && bun run test -- bridle transcriptReader # hub ack, idempotency, replay buffer, reader order +cd app && bun test slices/bridle/utils # chatFlow ordering, delivery state machine +cd admin && bun test slices/bridle/utils +cd app && npx nuxt typecheck # not `bun run typecheck` — it regenerates the SDK +cd admin && npx nuxt typecheck +bun run i18n:sync # after adding keys to app/slices/bridle/i18n/locales/en.json +``` + +## Manual scenarios + +Run each in **both** surfaces unless marked. + +### 1 — Order survives clock skew (US2, FR-006/007) + +With the OS clock 2 minutes ahead: send a question, wait for a multi-message answer, +send a follow-up while the agent is still working. +**Expect**: question above its answer; follow-up exactly where it was sent; identical +order after a reload. Reset the clock afterwards. + +### 2 — Scroll (FR-024/025) + +Fill the chat past one screen. Scroll to the top. (a) Wait for an incoming agent message +→ the view **does not move**. (b) Send a message → the view **scrolls to the bottom** +and the new message is visible. (c) At the bottom during a streamed answer → the view +follows the stream. + +### 3 — Timestamps (US5, FR-017/018) + +Every bubble shows a time of day; hovering shows full date and time; a separator appears +between messages from different days. Reload: every time is unchanged. App in Russian: +separators and states are in Russian; admin is English. + +### 4 — Delivery states (US5, FR-019–021) + +- Normal network: the message shows no warning state (delivered). +- DevTools throttling "Slow 3G", or pause the API process for ~8 s: a loading state + appears under the message after ~5 s and clears on delivery. +- Stop the agent (scale to 0 / stop from the admin), send: "not delivered" appears under + the message with Resend and Discard. **No** "Agent is not connected" agent bubble. + +### 5 — Nothing is lost across reload (US1, FR-002–005) + +- Send, reload within one second → the message is there once, either delivered or not + delivered — never missing, never twice. +- With the agent stopped: send, reload → the message is still there, marked not + delivered. Start the agent, press Resend → it becomes delivered, the agent answers + **once**, and a further reload shows one copy. +- Discard a not-delivered message, reload → it stays gone. +- Idempotency (a resend whose original did arrive) is not practical to stage by hand; it + is covered by the hub's jest spec for a repeated `clientMessageId` in the automated + checks above. + +### 6 — Landing page → agent page (US1 scenario 1, app only) + +Ask a question in the landing hero chat, click through to the agent page while the agent +is still answering. +**Expect**: the question is the first item, shown once; no "Reconnecting…"; the answer +continues in place; nothing to retype. + +### 7 — Two chats at once (admin only) + +Rancher panel open with a turn running; open an agent's Chat tab and send there. +**Expect**: each chat keeps its own messages and its own thinking timeline. + +### 8 — Answer arrives after a connection gap (US4, FR-014–016) + +Start a long turn. DevTools → Offline for ~10 s across the moment the agent finishes +(watch the API log for the `stream_end`), then back Online. +**Expect**: the answer appears without a reload within 10 s of reconnecting; the +thinking indicator ends; no duplicate bubbles. Then repeat, but restart the API during +the gap (buffer lost): the client falls back to the transcript and still shows the +answer. + +### 10 — Two views of one conversation (FR-026) + +Open the same agent's chat in two tabs (or the admin panel and the app console) under the +same login. Send from the tab that was opened **first**. +**Expect**: both tabs show the question once and the answer once. *Before the fix +(reproduced 2026-09-18 with a scripted client): the sending tab receives nothing and the +other tab receives the answer.* + +### 11 — Agent status is the same everywhere (US6, admin only) + +Agents screen, an agent selected. Trigger a deploy that ends in `failed` (or `running`). +**Expect**: list row, header pill and Overview card change together within 10 s, no +reload; the failure reason is reachable from the list row. Repeat with a **different** +agent selected: the changing agent's row still updates. + +### 9 — Reload looks like live (US3, FR-010–013) — *needs the runtime change* + +Agent answers in ≥ 3 messages with a list, inline code and bold text. Screenshot, reload +(admin) and open the same conversation under `/chats` (app), compare. +**Expect**: same number of bubbles, same boundaries, same formatting, no glued sentences. +Until the runtime persists one event per message this scenario is expected to **fail** +for the admin and the history pages, and must be reported as such. + +## Success criteria mapping + +| Criterion | Scenario | +|-----------|----------| +| SC-001, SC-002 | 5, 6, 8 (scripted ×100 for the numbers) | +| SC-003 | 9 | +| SC-004 | 1, 6 | +| SC-005 | 8 | +| SC-006 | 3 | +| SC-007 | 4 | diff --git a/specs/015-chat-message-reliability/research.md b/specs/015-chat-message-reliability/research.md new file mode 100644 index 00000000..4fd44730 --- /dev/null +++ b/specs/015-chat-message-reliability/research.md @@ -0,0 +1,405 @@ +# Research: Chat message reliability (CLEAN-102) + +**Date**: 2026-09-18 · **Method**: static reading of the chat code in `app/`, `admin/` +and `api/` on `origin/main` @ `71f8351`. Nothing here was reproduced in a running stack +yet. Each finding is marked **Confirmed in code** (the defect is visible in the source) +or **Hypothesis** (a plausible cause that needs a reproduction before it is trusted). + +## How the chat works today + +- One in-memory hub in the API (`api/src/slices/bridle/data/bridle.gateway.ts`) relays + between browser sockets and the agent runtime's socket. It stores nothing. +- The **app console** keeps each conversation in `localStorage` + (`bridle:conversation:`, `app/slices/bridle/stores/bridle.ts`). A reload replays + that, never the server transcript. The landing-page hero chat and the agent page mount + the same `BridleChatProvider` with the same key (`agentId`). +- The **admin panel** keeps nothing locally. On mount it calls `clearMessages()`, loads + the transcript over HTTP, then opens the socket + (`admin/slices/bridle/components/bridle/Provider.vue:316-331`). +- The **transcript** is an append-only JSONL written by the agent runtime. The runtime's + source is **not in this repository**; the API only reads the file + (`api/src/slices/agent/file/domain/transcriptReader.service.ts`). + +## Findings + +### F1 — Wrong order: the timeline is sorted by timestamps from two different clocks — *Confirmed in code* + +Both surfaces build the visible flow by sorting messages and thinking blocks by `ts`: +`app/slices/bridle/components/bridle/chat/Provider.vue:116` and +`admin/slices/bridle/components/bridle/Provider.vue:85` +(`items.sort((a, b) => a.ts - b.ts)`). + +The person's message is stamped with the **browser** clock (`ts: Date.now()` in both +`sendMessage`s). Agent messages are stamped with the **agent's** clock +(`ts: reply.ts ?? Date.now()`). The code already knows this — the thinking-block anchor +carries the comment "wire ts is agent-clock" and compensates with +`Math.max(e.ts, lastTs + 1)` — but messages get no such treatment. + +So whenever the browser clock runs ahead of the agent's by more than the agent's +response time, the reply sorts **above** the question. That is exactly "сначала пишу я, +а потом ответ агента, но моё сообщение опускается". It is intermittent because it +depends on clock skew and on how fast the first agent message arrives (a greeting or a +short acknowledgement arrives within milliseconds, a long answer does not). + +It also explains order changing after a reload in the admin: live order uses mixed +clocks, replayed order uses only runtime timestamps. + +### F2 — Sent messages are fire-and-forget; nothing confirms delivery — *Confirmed in code* + +`socket.emit('message', …)` has no acknowledgement in either client +(`app/slices/bridle/data/bridle.gateway.ts:140`, `admin/…/stores/bridle.ts:777`). The +hub's `handleMessage` returns nothing. The local bubble is added before the emit and +looks identical whether or not anyone received it. + +When the agent's socket is not connected, the hub does not reject the message: it sends +back a fake **agent** message, "Agent is not connected. Please try again later." +(`bridle.gateway.ts:190-207`), and drops the person's text. In the admin, a reload then +shows the transcript, which never contained that message — "сообщение проглатывалось +после перезагрузки". In the app the bubble survives in `localStorage` but is +indistinguishable from a delivered one. + +### F3 — Admin: the transcript load replaces the message list — *Confirmed in code* + +`loadTranscript` does `this.messages = page.messages.map(…)` +(`admin/…/stores/bridle.ts:1066`). Anything appended locally before it resolves is +wiped. The mount sequence awaits it before connecting, which protects the first mount, +but the store is a **singleton shared by every provider** (the comment at +`Provider.vue:314` says so): the Rancher panel and an agent's chat tab write into the +same `messages`, `thinkingBlocks` and socket. Opening one while the other is mid-turn +clears and replaces the other's list. *Hypothesis*: this is a source of vanished and +mixed-up messages in the admin; needs a reproduction with both chats open. + +### F4 — Events sent while the browser is not registered are dropped — *Confirmed in code* + +`handleAgentEvent` looks the client up and, if it is not registered at that instant, +discards the event (`bridle.gateway.ts:254-257`). There is no buffer and no replay. Both +clients, on `disconnect`, set typing/pending to false and close all turns. After the +socket reconnects nothing re-requests what was missed. + +Consequence: a `stream_end` / `message` that lands during a reconnect gap never reaches +the browser. The agent's logs show the answer; the chat shows a frozen or vanished +thinking indicator until a reload — "зависает ответ, хотя в логах он уже отображается". +In the app a reload does not help either, because the app replays `localStorage`, which +never received the answer. + +### F5 — Landing page → agent page handoff tears down the shared channel — *Hypothesis* + +The hero chat and the agent page use the same conversation key. `connect()` is +idempotent per key (`if (channels.has(key)) return`), and each provider's watcher +registers `onCleanup(() => disconnect(conversation))`. The agent page has a top-level +`await useAsyncData`, so it resolves under Suspense: its provider can mount (and find +the channel already open, so it opens none) **before** the landing page unmounts and +closes that channel. The agent page is then left with no channel: "Reconnecting…" +forever, the in-flight answer is lost (F4), and the next send takes the offline path, +which hands the text back to the composer as a draft — sending it again produces the +second copy seen in screenshot 4. + +This fits "задал вопрос на главной, перешёл в агента — сообщения нет / ответ не пришёл" +and the duplicate, but the mount/unmount order must be observed in a running app before +it is treated as the cause. The fix (reference-counted channel ownership) is correct +regardless of whether this is the only cause. + +### F6 — Separate agent messages fuse into one bubble after reload — *Cause outside this repo; to verify* + +Live, each agent message arrives as its own `message` / `stream_end` with its own +`messageId`, and renders as its own bubble. After a reload the admin shows what the +transcript holds. The reader emits one message per JSONL event and the admin maps them +one to one — nothing in this repository joins messages. The fused text has no separator +at all ("Проверю:Ха!", "о себе.Skyhunter"), which is what plain concatenation of text +blocks produces. + +Working conclusion: **the runtime persists one `assistant` event per turn, with the +turn's text blocks concatenated**, while it emits them on the wire one by one. This must +be confirmed by reading a real session JSONL for one of the screenshot conversations +(first task). The app's own reload is not affected (it replays its local copy), but the +app's chat history pages (`/chats/:id`) read the same transcript and are. + +### F7 — No stable message identity across live and replay — *Confirmed in code* + +The local echo id is `u-` (app) or a random UUID (admin). The hub mints a +different `messageId: randomUUID()` when forwarding to the agent +(`bridle.gateway.ts:235`). The transcript's user event has its own `id`. Three ids for +one message: a local copy can never be matched with its saved copy, which is the +precondition for both "show it once" (FR-009) and "keep the unsent one after reload". +Whether the runtime stores the hub's `messageId` as the event `id` is unknown — same +JSONL inspection as F6. + +### F8 — Scrolling — *Confirmed in code* + +- Admin follows new content only when the reader is within 80 px of the bottom + (`Provider.vue:286-298`). Sending is treated like any other change, so sending while + scrolled up leaves the view where it was. This is the behaviour the request calls out. +- App scrolls to the bottom on **every** change, including each streamed chunk + (`chat/Provider.vue:144-156`), which pulls a reader back down while they are reading + earlier messages. + +### F9 — Timestamps exist in the data and are not rendered — *Confirmed in code* + +Every message already has `ts` in both stores and in the transcript DTO. Neither +`Message.vue` displays it. + +## Decisions + +### D1 — Order by sequence, display by time + +- **Decision**: The visible flow is ordered by a per-conversation **arrival sequence** + (`seq`) assigned by the store when an item is appended: messages and thinking blocks + alike. `ts` is kept for display only and never used for ordering. Replayed history is + taken in transcript (file) order and numbered from there. A thinking block is anchored + by `seq`, replacing the `Math.max(e.ts, lastTs + 1)` workaround. +- **Rationale**: Arrival order at the client *is* the order the person experienced, it is + immune to clock skew, and it needs no server change. Removes F1 completely. +- **Alternatives considered**: (a) normalise agent timestamps by an estimated clock + offset — fragile, still wrong for sub-second gaps; (b) have the hub stamp every event + with its own clock and sort by that — still loses to the browser-stamped local echo + unless the echo is re-stamped from the ack, and adds nothing `seq` does not give. +- **Display time**: a person's message shows its local send time until the hub's ack + returns the server time, then shows that (so live and replay agree, FR-018). + +### D2 — Acknowledged sends with a client-generated id + +- **Decision**: The client generates `clientMessageId` (UUID) per message and sends it + with the `message` event using a socket.io acknowledgement callback. The hub replies + `{ status: 'accepted', messageId, ts }` once it has handed the message to a connected + agent socket, or `{ status: 'rejected', code }` (`AGENT_OFFLINE`, + `ATTACHMENT_FAILED`, `SHARE_REJECTED`). The hub forwards the **client's** id to the + agent as `messageId` instead of minting its own. +- **Backward compatibility**: a client that passes no ack callback (embed widget, older + bundles) keeps today's behaviour, including the "Agent is not connected" message. New + clients get the rejection instead and never see that fake agent message. +- **Rationale**: Smallest change that gives a truthful delivery state (F2) and one id + end to end (F7). +- **Limit, stated plainly**: "accepted" means *handed to the agent's socket*, not + *written to the transcript*. The spec's definition of delivered is the latter. Closing + that gap needs the runtime to acknowledge persistence — see D6 / Open items. Until + then a message can in rare cases be "delivered" and still be missing after a reload + (agent crash between receive and write). The plan treats hub acceptance as delivered + and records this as a known gap rather than hiding it. +- **Alternatives considered**: HTTP `POST /send` for delivery + socket for replies — a + bigger change to two clients and the embed contract, for the same guarantee. + +### D3 — Delivery state machine and a per-device outbox + +- **Decision**: States `sending → slow (5 s) → delivered | failed (30 s or rejected)`. + Messages that are not `delivered` are persisted per device: the app already persists + the conversation and gains a `delivery` field; the admin gains a small outbox in + `localStorage` keyed by agent + channel, merged under the transcript on load. On + reload, anything still `sending`/`slow` is shown as `failed` with Resend / Discard. +- **Resend** re-emits with the **same** `clientMessageId` (see D4). No automatic resend, + per the spec. +- **De-duplication on load (admin)**: an outbox entry whose id appears in the transcript + is dropped from the outbox. If the JSONL inspection shows the runtime does *not* + persist the forwarded id, the interim rule is: drop an outbox entry when a transcript + user message with identical text exists within ±2 minutes of its send time. That + heuristic is a stop-gap and is called out as such in tasks. + +### D4 — Idempotent receive in the hub + +- **Decision**: The hub remembers recently seen `clientMessageId`s per client + (in-memory, 10-minute TTL, bounded). A repeat is acknowledged + `{ status: 'accepted', duplicate: true }` and **not** forwarded again. +- **Rationale**: Makes Resend safe when the original did arrive but its ack was lost + (FR-005). + +### D5 — Replay buffer for missed events, transcript as the safety net + +- **Decision**: The hub stamps every event it routes to a browser client with a + monotonically increasing per-client `seq` and keeps a bounded ring buffer of recent + events per client (last 500 events or 10 minutes, whichever is smaller). It buffers + even when the client is not currently registered. On (re)connect the client sends + `lastSeq` in the handshake; the hub replays everything after it before live traffic. + Clients stop force-closing turns on a transient disconnect; they close them only when + the watchdog expires or the replay says the turn ended. +- **Safety net**: when the watchdog expires with a turn still open, the client fetches + the transcript tail and reconciles (admin already has the route; the app gains a + gateway method for the existing `GET /api/agent/:id/transcript`). +- **Rationale**: Fixes F4 for both surfaces and for the landing → agent handoff, without + making the transcript a live data path. +- **Constraint**: the hub is in-memory, so this assumes a single API instance — the same + assumption the hub's client and agent registries already make. Noted in the plan. +- **Alternatives considered**: transcript polling only — slow, and useless for the app's + local-first history; persistent event store — out of proportion for this problem. + +### D6 — Message boundaries in the transcript + +- **Decision**: In this repository: the reader gains support for per-message assistant + events (it already emits one bubble per event, so this is mostly tests and a + tie-break on file order instead of `ts`). The **runtime** must persist one assistant + event per emitted message, with the wire `messageId` as the event id, and persist the + forwarded user `messageId` too. That change lives outside this repo and is tracked as + a dependency. +- **Interim**: none that is honest. Text blocks concatenated without a separator cannot + be split back reliably. Existing fused conversations stay fused (already an assumption + in the spec). + +### D7 — Channel ownership by reference count (app), conversation-scoped state (admin) + +- **Decision (app)**: `connect`/`disconnect` become acquire/release with a reference + count per conversation key; the socket closes when the last holder releases, after a + short grace delay so a page-to-page handoff reuses the live socket. In-flight turn + state survives the handoff. +- **Decision (admin)**: the singleton store's chat state becomes keyed by conversation + (agent + channel), mirroring the app store, so the Rancher panel and an agent tab no + longer share one message list and one socket. +- **Rationale**: F5 and F3. The admin change is the largest single piece of work here; + it is also the precondition for the outbox and sequence numbers being per + conversation. + +### D8 — Scrolling + +- **Decision**: One rule in both surfaces: *sending* always scrolls to the bottom; + *incoming* content follows only a reader who is within 80 px of the bottom. The app's + unconditional scroll-on-every-chunk is replaced by the admin's near-bottom rule; the + admin gains the scroll-on-send. + +### D9 — Timestamps + +- **Decision**: Time of day under each message, a date separator between days, full + date-time in the `title` tooltip. Formatting via `Intl.DateTimeFormat` with the active + locale in the app (no new strings for the time itself); the admin uses `en`. New + wording (delivery states, Resend, Discard, date separators "Today"/"Yesterday") goes + through `docs/i18n.md`: keys in the slice's `en.json`, `bun run i18n:sync` for `ru`. + +### D10 — Testing + +- **Decision**: API changes are covered in the existing jest specs + (`bridleClientWs.handler.spec.ts`, `bridle.gateway.spec.ts`, + `transcriptReader.service.spec.ts`). `app/` and `admin/` have no test runner today + (`"test": "echo 'no tests yet'"`), so the logic that matters — sequencing, the + delivery state machine, outbox/transcript reconciliation, replay application — is + written as pure functions under each slice's `utils/` and covered with `bun test`, + which needs no new dependency (Bun is already the runtime). UI behaviour (scroll, + labels) is validated through `quickstart.md`. + +## Update 2026-09-18 — runtime source located + +The runtime is the sibling repository `CleanSlice/runtime` (the API already looks for it +at `../runtime`, see `paddockRunner.resolveRuntimeRoot`). Read at `da3e74f`, not run: + +- **F6 confirmed in source.** `src/slices/runtime/loop/domain/loop.service.ts` + (`sendFinalResponse`) appends **one** `assistant` event per turn with + `data: { text: fullText }` — the text accumulated over the whole turn — while the + channel receives the pieces as separate messages. That is the fused bubble. +- **F7 answered.** That event's `id` is a fresh `randomUUID()`, not the wire + `messageId`. Whether the `user` event keeps the forwarded `messageId` was not checked. +- Open items 1 and 2 below are therefore mostly closed: slice 6 is a change in + `CleanSlice/runtime` (its own ticket/PR), and D3's interim text-match rule is needed + until that lands. + +## Update 2026-09-18 — experiments against the local stack + +Stack: API :3333, app :3000, admin :3001, local `CleanSlice/runtime` connected to the hub +as `agent-bb620efe-…`. Method: a Node script using `socket.io-client`, logging in through +`POST /auth/login` and connecting to `/ws/client` exactly as the browser does. The UI +itself was not driven — ordering, scrolling, the landing handoff (F5) and the admin +singleton (F3) are still unobserved in a browser. + +| # | Experiment | Result | +|---|-----------|--------| +| E1 | Send with a socket.io ack callback and a `clientMessageId` | Reply arrived in 1.2 s. **Ack callback never called** (waited 45 s). F2 reproduced. | +| E2 | Compare ids for one exchange | Wire agent `messageId` `38379ecb…`; transcript `assistant` event id `f421a550…`; transcript `user` event id `51a748fc…`, its `data` holds only `text, from` — no message id at all. **Three unrelated ids. F7 reproduced**, and the runtime does not persist the forwarded id. | +| E3 | Two connections of the same login; the **first** one sends | First connection received **nothing**. The second, which sent nothing, received `typing` and the answer. **New finding F10, reproduced.** | +| E4 | Send, drop the socket 100 ms after `typing`, reconnect 25 s later, wait 25 s | **No events after reconnect.** The runtime's transcript contains the full 306-character answer. **F4 reproduced.** | +| — | Clock skew on this machine | `ts` on agent events was within 1 ms of the local clock (same host), so F1 cannot show up locally without skewing the clock; it needs the browser scenario in quickstart. | + +### F10 — One registration per identity: a second view steals the first one's events — *Reproduced (E3)* + +The hub keeps exactly one socket per `clientId:agentId` +(`BridleGateway.clients`, `registerClient` overwrites). And +`clientIdFromJwtPayload` gives **every Owner and Admin the same `clientId`: `admin`** +("they share one channel so their history lives in one place", +`api/src/slices/bridle/domain/chatIdentity.ts:93`); other users get their `sub`. + +So the last view to connect receives everything and every earlier view silently receives +nothing: a second tab, the admin panel next to the app console, a colleague who is also +an admin opening the same agent, or a page that reconnected. For the view that lost, the +turn looks exactly like the reports: the question is sent, the indicator spins, no answer +arrives although the agent's log has it; the *other* view shows an answer to a question +nobody asked there. This is very likely the most frequent cause of "зависает ответ" and +of messages turning up in the wrong place, and it makes F4 worse (any reconnect +elsewhere cuts this view off). + +Shared history for admins is a product decision and stays. What has to change is the +delivery: fan out to **all** sockets registered for the identity. + +### D11 — Several sockets per identity (decision) + +- **Decision**: `clients` becomes `clientKey → Set`. Events for a + client are sent to every registered socket. The sequence and replay buffer (D5) stay + per `clientKey`; each socket reports its own `lastSeq`. `unregisterClient` removes one + socket; turn tracking is cleared when the last one leaves. +- **Consequence for the person's own messages**: a message sent from view A is echoed by + the hub to the identity's *other* sockets as a new `user_message` event + (`{ messageId, text, attachments, ts, seq }`), so view B shows the question as well as + the answer instead of an orphan answer. View A ignores it by id. +- **Rationale**: Required by FR-008 / FR-026; without it the ack and replay work would + still leave multi-view use broken. + +### F11 — Agent status disagrees between the list and the detail — *Confirmed in code* + +In the admin's agents screen the list (`workspace/Rail.vue`) renders the array loaded +once by `useAsyncData('admin-agents', () => agentStore.fetchAll())` in +`workspace/Provider.vue:12`. It is refreshed only after a delete; nothing polls it (the +30-second timer there refreshes capacity only). The detail pane (`workspace/Main.vue`) +loads its own copy with `fetchById`, refreshes it through `useAgentLifecycle` and also +reads the live status stream (`useAgentStatusStore().agents[id]`). `fetchById` does not +write its result back into `agentStore.agents`. So the list keeps whatever status it saw +at load time — "Deploying" — while the detail moves on to "Failed". + +### D12 — Single source of truth (SSOT) for agents (decision, revised on request) + +*Direction from the request: "используй SSOT для таких проблем чтобы не возвращаться к +ним" — fix the class of defect, not the one screen.* + +The defect is not "the list is not refreshed"; it is that **the same agent exists as +several independent copies**: the list array from `fetchAll`, a `useAsyncData` copy per +detail component from `fetchById` (`workspace/Main.vue`, `edit/Provider.vue`, +`agentFile/Provider.vue`; in the app `agent/Provider.vue`, `agent/chat/Provider.vue`), +the live copy in `agentStatusStore.agents`, and optimistic edits applied to a local ref +in `useAgentLifecycle`. Any of them can move on without the others. + +- **Decision**: An agent lives **once**, in `agentStore.agents`. Rules, in both `admin/` + and `app/`: + 1. Every read that returns an agent writes it into the store: `fetchAll` replaces the + collection, `fetchById` / create / update / restart **upsert** by id. + 2. Components render from the store by id (`agentStore.byId(id)`), never from the + value a fetch returned. `useAsyncData` is kept for SSR and `pending`/`error`, not + as the render source. + 3. Pushes write into the same record: the status stream's `applyMessage` patches + `agentStore` instead of holding a parallel `agents` map. + 4. Optimistic changes go through one store action (`patch(id, partial)`) with rollback, + not through a component-local ref. +- **Result**: the list row, the header and the Overview card are the same object; they + cannot disagree, and a future screen that shows an agent gets this for free. +- **Also to check**: `useAgentLifecycle` notes that the backend reconciles status on each + `fetchById` ("Backend syncStatus runs on each fetchById"). If the list endpoint does + not reconcile, a non-selected agent's row depends on the stream alone — verify, and + make sure the stream covers every listed agent. +- **Written down so it sticks**: a short rule in `docs/` linked from `AGENTS.md`. +- **Alternatives considered**: polling `fetchAll`, or overlaying the stream on the list + in the rail only — both patch this screen and leave the duplicated state that caused + it. +- The chat work follows the same principle: one conversation record per key in the store + (D7), one id per message (D2), one ordering key (D1). +- **Scope note**: this is unrelated to the chat code. It is included in this feature on + request; it is a small, separate slice and could equally ship as its own PR. + +### Local-environment note + +On this Windows machine the runtime writes the session to `data/sessions/bridle:admin.jsonl`; +NTFS treats the colon as an alternate data stream, so the file appears as an empty +`bridle` with a stream `admin.jsonl`. The API's transcript route reads through the file +gateway and returned 0 messages locally. **Reload-from-transcript scenarios (US3, and the +admin's reload in general) cannot be validated on this local setup as it stands**; they +need the cluster or a non-Windows runtime. + +## Open items + +1. **Read a real session JSONL** for one of the screenshot conversations to confirm F6 + and answer F7 (does the runtime store the forwarded `messageId`?). Blocks the final + shape of D3's de-duplication and D6. +2. **Where the agent runtime's source lives and who changes it** — needed for D6 and for + a persistence-level delivery ack (D2's stated gap). Not discoverable from this repo. +3. **Reproduce F5 and F3** in a running stack before and after the fix; they are the two + hypotheses in this document. +4. **API replica count in production** — D5 assumes one instance. diff --git a/specs/015-chat-message-reliability/spec.md b/specs/015-chat-message-reliability/spec.md new file mode 100644 index 00000000..fa24b939 --- /dev/null +++ b/specs/015-chat-message-reliability/spec.md @@ -0,0 +1,433 @@ +# Feature Specification: Chat message reliability — nothing lost, nothing doubled, same after reload + +**Feature Branch**: `015-chat-message-reliability` (git: `fix/CLEAN-102-chat-message-reliability`, Jira: CLEAN-102) + +**Created**: 2026-09-18 + +**Status**: Draft + +**Input**: User description: "Необходимо решить проблему с чатом, где возникают артефакты в виде неправильной группировки сообщений, иногда они просто исчезают, когда задал вопрос на главной странице, перешел в агента и этого сообщения нет, иногда возникало такое что сообщение просто проглатывалось после перезагрузки страницы и все, приходится писать заново. Иногда сообщения дублируются. Нужно отображать время когда было отправлено сообщение. Если оно было отправлено но не доставлено, из-за чего исчезнет после обновления страницы, это нужно помечать под самим письмом, загрузку, если слишком долго доставляется. Иногда зависает ответ, хотя в логах он уже отображается. Агент может писать разными сообщениями, а после перезагрузки ломается форматирование и все в один пузырь накладывается. Сообщение почему-то мое опускается, хотя сначала пишу я, а потом ответ агента." + +## Evidence from the screenshots *(context, supplied with the request 2026-09-18)* + +Four screenshots came with the request. What they show, as observed: + +- **Live turn, correct** (admin, "Chat with Rancher"): the agent answers in several + separate bubbles, each followed by its own "Thought for a moment" step, with the + person's question sitting between them where it was asked. +- **Same kind of turn after a reload, broken** (admin, two screenshots): the agent's + separate messages are fused into one bubble with no break between them — "Проверю:Ха! + **Работает!**", "Попробую с токеном:✅ **Хост доступен**", "…с полной информацией о + себе.Skyhunter сообщает…". The thinking steps that separated them are gone. +- **Wrong order and a duplicate** (app console, conversation started from the home + page): the person's question "Спроси у Elderly Care Match: assisted living near + Seattle, WA" appears first, then the agent's greeting "Access granted! Send me a + message." and its first reply, and then **the same question again**, below the reply + it caused. + +Reported without a screenshot: a question asked on the home page is missing once the +agent chat opens; a sent message is gone after a reload and has to be typed again; the +reply stays on "thinking" although the agent's logs already contain the answer. None of +these has been reproduced on demand yet — all are described as "sometimes". + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - What I sent is never silently lost (Priority: P1) + +A person types a question — on the home page or in an agent's chat — and sends it. From +that moment the message is theirs to keep: it is visible in the conversation it belongs +to, it survives moving from the home page into the agent's chat, and it survives a page +reload. If the agent never received it, the message is still there, says so, and can be +sent again without retyping. + +**Why this priority**: Losing what a person wrote is the most damaging failure in a +chat: it costs the work of writing it again and removes any reason to trust the +conversation. Every other story assumes the message exists. + +**Independent Test**: Send a question from the home page and confirm it is the first +message in the agent chat that opens. Send a message in an agent chat, reload +immediately, and confirm it is still there. Send a message while the agent is +unreachable, reload, and confirm the text is still there, marked as not delivered, with +a way to resend. + +**Acceptance Scenarios**: + +1. **Given** a person on the home page, **When** they send a question and are taken to + the agent's chat, **Then** that question is shown in the chat exactly once, before + any agent reply to it. +2. **Given** a message that the agent has received, **When** the person reloads the page + at any moment afterwards — including while the agent is still answering —, **Then** + the message is shown in the same place in the conversation. +3. **Given** a message that was sent but never reached the agent, **When** the person + reloads the page on the same device, **Then** the text is not lost: it is shown as not + delivered and can be resent in one action, without retyping. +4. **Given** a not-delivered message, **When** the person resends it and it is received, + **Then** it becomes an ordinary delivered message and appears once. +5. **Given** a not-delivered message, **When** the person chooses to discard it, + **Then** it is removed and does not come back after a reload. + +--- + +### User Story 2 - The conversation reads in the order it happened, each message once (Priority: P1) + +A person reads a conversation top to bottom and it matches what happened: their question +comes before the reply it caused, a greeting the agent sent before the question comes +before the question, and no message — theirs or the agent's — appears twice. + +**Why this priority**: A conversation in the wrong order or with repeats cannot be read +or quoted, and a doubled question looks like it was sent twice (and may make the person +wonder whether the agent acted on it twice). + +**Independent Test**: Run a scripted conversation — greeting, question from the home +page, multi-message reply, follow-up question sent while the agent is still answering — +and compare the displayed sequence with the sequence of events, live and after a reload. + +**Acceptance Scenarios**: + +1. **Given** a person sends a question and the agent replies, **When** the conversation + is displayed, **Then** the question is above the reply, live and after a reload. +2. **Given** a conversation opened from the home page where the agent also sends a + greeting, **When** it is displayed, **Then** greeting, question and reply each appear + once, in the order they actually happened. +3. **Given** a connection that drops and recovers in the middle of a turn, **When** the + chat catches up, **Then** no message already on screen is shown a second time. +4. **Given** the same conversation open in two tabs, **When** a message is sent from + one, **Then** each tab shows it once. +5. **Given** a person sends a follow-up while the agent is still answering, **When** the + turn completes, **Then** the follow-up stays where it was sent relative to the agent + messages that came before and after it. +6. **Given** a person has scrolled up to re-read earlier messages, **When** they send a + message, **Then** the view scrolls to the bottom so the message they just sent is + visible — the usual chat behaviour. +7. **Given** a person has scrolled up and has not sent anything, **When** the agent's + messages or thinking steps arrive, **Then** the view stays where the person left it. + +--- + +### User Story 3 - A reloaded conversation looks like the one I watched (Priority: P2) + +A person watches an agent work through a task: several separate messages, with thinking +steps between them. After a reload, or when opening the conversation later from the chat +history, they see the same thing — the same number of agent messages, split in the same +places, with the same formatting. + +**Why this priority**: History is how people return to an answer and how operators +review what an agent did. Today a reload glues messages together and destroys the +paragraph breaks, so the saved conversation is harder to read than the live one and +looks like a different answer. + +**Independent Test**: Have an agent answer in at least three separate messages with +formatted content (list, inline code, bold). Record the live view, reload, open the same +conversation from history, and compare message count, boundaries and formatting. + +**Acceptance Scenarios**: + +1. **Given** an agent turn that produced N separate messages, **When** the conversation + is reloaded or opened from history, **Then** it shows N separate agent messages with + the same text in each. +2. **Given** an agent message containing lists, inline code, bold text or line breaks, + **When** the conversation is reloaded, **Then** the formatting is the same as it was + live, and no two sentences that were in different messages run together. +3. **Given** a turn where thinking steps were shown between agent messages, **When** the + conversation is reloaded, **Then** the messages stay separate where those steps were; + whether the steps themselves are shown again is covered under Assumptions. +4. **Given** a conversation reloaded while the agent is still answering, **When** the + rest of the answer arrives, **Then** it continues as further messages in the same + turn rather than restarting, doubling or merging into an earlier bubble. + +--- + +### User Story 4 - The answer shows up when the agent has answered (Priority: P2) + +When the agent has finished answering, the person sees the answer. The chat does not +stay on "thinking" after the agent is done, and when something really has gone wrong the +person is told, instead of watching an indicator that will never finish. + +**Why this priority**: A reply that exists but is not shown is indistinguishable from an +agent that failed; people reload, resend and create the duplicates above. It ranks below +the first two stories because a reload currently recovers the answer, whereas lost and +doubled messages are not recoverable by the person. + +**Independent Test**: Trigger turns of different lengths, including one where the +connection is interrupted just before the agent finishes, and check that the answer +appears without a manual reload and the thinking indicator ends. + +**Acceptance Scenarios**: + +1. **Given** the agent has completed its answer, **When** the completion was missed by + the open chat (for example because the connection dropped for a moment), **Then** the + chat recovers the answer on its own and shows it without a manual reload. +2. **Given** a turn that ends in an error on the agent's side, **When** this happens, + **Then** the thinking indicator stops and the person sees that the turn failed. +3. **Given** a turn that is legitimately long, **When** the person waits, **Then** the + chat keeps showing that the agent is working and does not declare a failure while + progress is still arriving. + +--- + +### User Story 5 - I can see when a message was sent and whether it got through (Priority: P2) + +Every message shows the time it was sent. Under their own messages a person can tell +the state at a glance: still sending, taking unusually long, delivered, or not delivered. +Delivered is the quiet default; the states that need attention are the ones that stand +out. + +**Why this priority**: This is the new capability in the request and it is what turns +the failures above from silent into visible. It comes after the fixes because marking a +lost message is a poor substitute for not losing it, but it is what lets a person act +when delivery does fail. + +**Independent Test**: Send messages under normal conditions, under a slow connection and +with the agent unreachable, and check the time and the state shown under each, live and +after a reload. + +**Acceptance Scenarios**: + +1. **Given** any message in a conversation, **When** it is displayed, **Then** the time + it was sent is visible with it, in the viewer's local time, and the date is + recognisable for messages from earlier days. +2. **Given** a message the person has just sent, **When** delivery takes longer than the + "slow" threshold, **Then** a loading state appears under that message until it is + delivered or marked not delivered. +3. **Given** a message that did not reach the agent within the "failed" threshold or was + rejected, **When** this is determined, **Then** "not delivered" is shown under that + message together with the way to resend it. +4. **Given** a delivered message, **When** the conversation is reloaded, **Then** its + time is unchanged — the time shown is when it was sent, not when the page was loaded. +5. **Given** the app console in Russian, **When** times and states are shown, **Then** + the wording is localised; the admin panel stays in English. + +--- + +### User Story 6 - An agent shows the same status everywhere (Priority: P3) + +*Added 2026-09-18 on request, with a screenshot: the admin's agent list shows Rancher as +"Deploying" while the same agent's header and Overview, on the same screen, show +"Failed — startup did not produce a running agent within 5 minutes".* + +An operator looking at the agents screen sees one status per agent. When an agent's +status changes — deploying to running, deploying to failed, running to stopped — every +place on screen that shows that agent's status changes with it, without a reload. + +**Why this priority**: It is not a chat defect, but it undermines the same trust: an +operator watching the list waits for a deploy that has already failed. It is ranked +last because the correct status is visible one click away and nothing is lost. + +**Independent Test**: Start a deploy that will fail (or succeed) and keep the agents +screen open with that agent selected. Compare the status in the list row, the header and +the Overview card at the moment the status changes and one minute later. + +**Acceptance Scenarios**: + +1. **Given** the agents screen open with an agent in "Deploying", **When** the deploy + fails or completes, **Then** the list row, the header and the Overview card show the + new status within the same few seconds, without a reload. +2. **Given** an agent that is not the selected one, **When** its status changes, + **Then** its row in the list reflects the change without the operator opening it. +3. **Given** a status with a reason (for example the start-up timeout), **When** it is + shown in the list, **Then** the reason is available there too. + +--- + +### Edge Cases + +- The person sends a question from the home page, and the agent is starting up or + unreachable: the question must be waiting in the agent chat, marked according to its + real state, not dropped. +- The person sends, then reloads before any confirmation arrives: after the reload the + message is either shown as delivered (it did arrive) or as not delivered — never both, + never neither. +- A resend races with a late confirmation of the original: the agent must not receive or + act on the same message twice, and the conversation shows it once. +- The device clock is wrong or differs from the agent's: order must still follow what + actually happened, not the device clock. +- Two messages are sent within the same second: both are kept, in send order. +- The agent sends an empty or whitespace-only message between two real ones: it must not + create an empty bubble, and must not cause its neighbours to merge. +- A message carries attachments: the delivery state and the resend action cover the + message together with its files. +- A very long conversation is loaded in pages: grouping and order stay correct across + the page boundary, and a message on the boundary is not duplicated. +- A not-delivered message is kept on one device and the person opens the conversation on + another: the other device shows only what the agent received (see Assumptions). +- Conversations saved before this change, where messages were already fused: see + Assumptions. + +## Requirements *(mandatory)* + +### Functional Requirements + +**Keeping what was sent** + +- **FR-001**: A message sent from the home page MUST appear in the agent chat that opens + for it, exactly once, positioned before any agent reply to it. +- **FR-002**: A message the agent has received MUST be shown in the conversation after + any page reload, including a reload during the agent's answer. +- **FR-003**: A message that was sent but has not been confirmed as received MUST be + retained on the sending device across reloads until it is delivered or the person + discards it. +- **FR-004**: A person MUST be able to resend a not-delivered message in one action, + with its text and attachments intact, and MUST be able to discard it. +- **FR-005**: Resending MUST NOT result in the agent handling the same message twice, + even if the original turns out to have been received. + +**Order and uniqueness** + +- **FR-006**: Messages MUST be displayed in the order the events happened in the + conversation, and this order MUST be the same live and after a reload. +- **FR-007**: The displayed order MUST NOT depend on the clock of the viewer's device. +- **FR-008**: Each message MUST be displayed at most once per conversation view, + including after reconnects, catch-up after a dropped connection, paged loading of + older history, and when the conversation is open in several tabs. +- **FR-009**: A message the person sent MUST be recognised as the same message when the + saved conversation is loaded, so that the local copy and the saved copy never appear + side by side. + +**Same conversation after reload** + +- **FR-010**: Separate agent messages within one turn MUST remain separate when the + conversation is reloaded or opened from history, with the same boundaries as live. +- **FR-011**: The text and formatting of each message MUST be the same live and after a + reload; text from different messages MUST NOT be joined without a break. +- **FR-012**: When a conversation is reloaded during an agent's answer, the remainder of + the answer MUST continue the same turn without duplicating or merging with messages + already shown. +- **FR-013**: Empty agent messages MUST NOT be rendered and MUST NOT change how + neighbouring messages are grouped. + +**Answers that arrive** + +- **FR-014**: When the agent has completed an answer and the open chat has not shown it, + the chat MUST obtain and display it without the person reloading, within the recovery + time in SC-005. +- **FR-015**: The thinking indicator MUST end when the turn ends — by completion, by + failure or by cancellation — and a failed turn MUST be shown to the person as failed. +- **FR-016**: A long-running turn MUST NOT be reported as failed while progress from the + agent is still arriving. + +**Time and delivery state** + +- **FR-017**: Every message, from the person and from the agent, MUST show the time it + was sent, in the viewer's local time; messages from earlier days MUST be attributable + to their date. +- **FR-018**: The time shown for a message MUST be stable: the same value live, after a + reload and in the chat history. +- **FR-019**: Under each of the person's own messages the chat MUST be able to show one + of: sending, slow (loading shown because delivery is taking longer than usual), + delivered, not delivered. "Delivered" MAY be shown unobtrusively or implied by the + absence of any other state. +- **FR-020**: The loading state MUST appear once delivery has taken longer than the + "slow" threshold, and the message MUST be marked not delivered once the "failed" + threshold passes or delivery is rejected (defaults under Assumptions). +- **FR-021**: The not-delivered mark MUST be placed under the message it refers to, and + MUST make clear that the message will not be kept by the agent unless resent. + +**Scrolling** + +- **FR-024**: Sending a message MUST scroll the conversation to the bottom, wherever the + person was scrolled to, so that the message they sent is in view. +- **FR-025**: Incoming content (agent messages, streamed text, thinking steps) MUST keep + the view pinned to the bottom only for a person who is already at the bottom; it MUST + NOT pull back a person who scrolled up to read. This holds in both surfaces. + +**Several views of one conversation** + +- **FR-026**: When the same person has the same conversation open in more than one + place — two tabs, the admin panel and the app console, a second device — every open + view MUST receive the agent's messages. Opening a second view MUST NOT stop the first + one from receiving them. + +**Agent status consistency (admin)** + +- **FR-027**: Every place in the admin that shows an agent's status MUST show the same + value, and MUST reflect a status change within 10 seconds without a reload — including + list rows for agents other than the selected one. + +**Scope** + +- **FR-022**: All of the above MUST hold in every place the product shows a live agent + chat: the admin panel's chats with Rancher and with agents, and the app console's + agent chat including conversations started from the home page. +- **FR-023**: New user-visible wording in the app console MUST be available in every + language the console supports; the admin panel remains English-only. + +### Key Entities + +- **Conversation**: one continuous exchange between a person and an agent; has an + ordered sequence of messages and may be viewed live or from history. +- **Message**: one bubble's worth of content from the person or the agent. Has a stable + identity that is the same live and in the saved conversation, an author, content with + formatting, optional attachments, the time it was sent, and a position in the + conversation's order. +- **Turn**: the person's message plus everything the agent produced in response — + possibly several agent messages with thinking steps between them. Has a state: + running, completed, failed or cancelled. +- **Delivery state**: for a person's message — sending, slow, delivered, not delivered. +- **Unsent message**: a person's message retained on their device because delivery was + never confirmed; can be resent or discarded. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: In 100 scripted sends across the covered chats — including sends from the + home page and sends followed by an immediate reload — zero messages are lost: each one + is visible afterwards either as delivered or as not delivered with its text intact. +- **SC-002**: In the same run, and in runs with the connection interrupted mid-turn, + zero messages are displayed twice. +- **SC-003**: For 20 scripted conversations including multi-message agent turns, the + sequence of messages shown after a reload is identical to the sequence shown live: + same count, same order, same boundaries, same formatting. +- **SC-004**: A person's question is never displayed below the agent reply it caused — + zero occurrences in the scripted conversations, including those started from the home + page. +- **SC-005**: When an agent has finished answering, the answer is visible in an open + chat within 10 seconds in at least 99% of turns, and in 100% of turns without a manual + reload. +- **SC-006**: Every message in the covered chats shows its sent time, and that time is + identical before and after a reload. +- **SC-007**: When a message cannot be delivered, the person sees the not-delivered mark + under it within 35 seconds of sending, and can resend it in one action without + retyping. +- **SC-009**: With one conversation open in two views, 100% of agent messages appear in + both views. +- **SC-010**: After an agent's status changes, no two places on the admin's agents screen + disagree about it for longer than 10 seconds. +- **SC-008**: People no longer need to retype a message because the chat lost it: zero + such reports in the two weeks after release, against the recurring reports that + prompted this work. + +## Assumptions + +- **Both surfaces are in scope.** The screenshots show the admin panel ("Chat with + Rancher") and the app console (Russian "Немного подумал", home-page flow), and they + share the same chat behaviour, so the fix covers both. Read-only transcript viewers in + the chat history are in scope only for User Story 3 and the sent time. +- **Thresholds.** "Slow" is 5 seconds without confirmation; "not delivered" is 30 + seconds without confirmation, or an explicit rejection. These are starting values to + be tuned during planning, not contractual numbers beyond SC-007. +- **"Delivered" means received by the agent's side and saved to the conversation** — the + point after which a reload will show the message. It does not mean the agent has read + or answered it. +- **Unsent messages are kept per device.** A message that never reached the agent exists + only on the device that wrote it; another device shows the conversation as the agent + knows it. Syncing unsent messages between devices is out of scope. +- **No automatic resend.** A not-delivered message is resent by the person, not silently + by the chat, so that an agent never acts on something the person has given up on. + Re-establishing the connection and catching up on the agent's answer is automatic. +- **Thinking steps after reload.** Whether the collapsed "Thought for a moment" steps + are shown again in a reloaded conversation is existing behaviour and is not changed + here; what this work guarantees is that messages stay split where those steps were. +- **Time format.** Time of day under each message, with a date separator between days; + the full date and time are available on demand (for example on hover). Relative times + ("5 min ago") are not required. +- **Existing conversations.** Conversations saved before this change whose agent + messages were already stored fused together are not repaired retroactively unless the + original boundaries can be recovered at no extra cost; the guarantees apply to + conversations and turns created after release. +- **Other channels are out of scope.** Telegram and other messenger channels have their + own delivery model (see `003-telegram-restart-recovery`) and are not changed here. +- **Root causes are not yet established.** All failures are reported as intermittent. + Planning starts with reproducing each one; if a reported symptom turns out to have a + cause outside the chat (for example agent start-up, covered by + `001-stabilize-agent-startup`), it is tracked there and referenced from this feature. diff --git a/specs/015-chat-message-reliability/tasks.md b/specs/015-chat-message-reliability/tasks.md new file mode 100644 index 00000000..37d1c653 --- /dev/null +++ b/specs/015-chat-message-reliability/tasks.md @@ -0,0 +1,255 @@ +--- + +description: "Task list for chat message reliability (CLEAN-102)" +--- + +# Tasks: Chat message reliability — nothing lost, nothing doubled, same after reload + +**Input**: Design documents from `/specs/015-chat-message-reliability/` + +**Prerequisites**: [plan.md](./plan.md), [spec.md](./spec.md), [research.md](./research.md), +[data-model.md](./data-model.md), [contracts/bridle-socket.md](./contracts/bridle-socket.md), +[quickstart.md](./quickstart.md) + +**Branch / ticket**: `fix/CLEAN-102-chat-message-reliability` · CLEAN-102 + +**Tests**: No test-first mandate. Direction from the request: *"тести как удобно, лишь +выявить причину и исправить дефект"*. Each story ends with a **Verify** task that uses +whatever is quickest for that defect — the scripted socket probe, a jest spec next to the +hub code, `bun test` for a pure function, or a headless browser script. A defect counts as +fixed only when its verify task was run and the result written into `research.md`. + +**SSOT**: Direction from the request — fix duplicated state at the source so the class of +defect does not return (research D12). It shapes Phase 2 (one conversation record per key) +and Phase 8 (one agent record per id), and is written down as a project rule in T053. + +**Organization**: Grouped by user story. US2 comes before US1 although both are P1: it has +no API dependency, is the smallest change, and removes the artifact asked about first +(wrong order). + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: can run in parallel (different files, no dependency on an unfinished task) +- **[Story]**: US1–US6 from spec.md +- Paths are relative to the repository root + +## Local stack (for every Verify task) + +API `http://localhost:3333`, app `:3000`, admin `:3001`, local `CleanSlice/runtime` +connected as `agent-bb620efe-abb5-4123-8ace-6d9b963387c7`; login `RANCH_LOGIN` / +`RANCH_PASS` from `.env.project` (never print them). Every Owner/Admin login shares the +chat identity `admin`, so a probe run steals events from an open admin tab until T010 +lands — reload that tab afterwards. + +--- + +## Phase 1: Setup + +**Purpose**: Tools to reproduce and verify; nothing user-visible. + +- [ ] T001 Save the scripted socket client used for research E1–E4 as `specs/015-chat-message-reliability/probe.mjs`: modes `normal` (send with ack callback + `clientMessageId`, log every event with `messageId`, `seq`, clock skew), `steal` (two sockets, first one sends, report what each received), `gap` (drop the socket after the first `typing`, reconnect after 25 s, report events), new mode `offline` (send while the agent is disconnected, print ack and any synthetic agent message). Read `API_URL`, `AGENT_ID` from env with the local defaults above; read credentials from `.env.project`; resolve `socket.io-client` via `createRequire` from the repo root; print no secrets. +- [ ] T002 [P] Replace the placeholder test scripts with `"test": "bun test slices"` in `app/package.json` and `admin/package.json` so pure-function tests under `slices/**/utils/*.test.ts` run with `cd app && bun test` / `cd admin && bun test`. +- [ ] T003 [P] Create a headless browser harness `specs/015-chat-message-reliability/e2e/chat.e2e.mjs` run with `bunx playwright` (install browsers on first run; do **not** add Playwright to any `package.json`): helpers to log in to the app (`:3000`) and the admin (`:3001`), open an agent chat, send a message, read the rendered bubbles in DOM order (role + text + time), read scroll position, and start a page with a skewed clock (`addInitScript` overriding `Date.now` by +120 000 ms). If Playwright cannot be installed on this machine, record that in `research.md` and fall back to the manual steps in `quickstart.md`. +- [ ] T004 With T003 (or by hand), observe the four defects not yet seen in a browser and write what happened under a new "Observed in browser" heading in `specs/015-chat-message-reliability/research.md`: F1 (reply above question with the clock +2 min), F8 (no scroll on send in admin; forced scroll on every chunk in app), F5 (landing hero chat → agent page: does the chat stay connected, does the answer arrive), F3 (admin: Rancher panel + agent Chat tab open together). Mark each hypothesis confirmed or refuted; if F5 or F3 is refuted, note it on T028 / T009 before doing them. + +--- + +## Phase 2: Foundational (blocking prerequisites) + +**Purpose**: Shared shapes, the pure logic both clients use, one conversation record per +key in the admin store (SSOT), and a hub that can talk to more than one socket per +identity. **No user story work starts before this phase is done.** + +- [ ] T005 [P] Extend the message and thinking-block types in `app/slices/bridle/domain/bridle.types.ts` with optional `seq: number`, `delivery: 'sending' | 'slow' | 'delivered' | 'failed'`, `failureCode?: string`, and add `BridleDeliveryStates` / `IBridleSendAck` (`accepted` with `messageId`, `ts`, `duplicate?` | `rejected` with `code`, `message?`) exactly as in data-model.md and contracts/bridle-socket.md; keep every new field optional so stored conversations still load. +- [ ] T006 [P] Create `app/slices/bridle/utils/chatFlow.ts`: pure `buildChatFlow(messages, blocks, { locale, now })` returning flow items `{ key, seq, kind: 'message' | 'block' | 'day', ... }` ordered **only by `seq`**, inserting a `day` item where the calendar day of `ts` changes between consecutive messages, plus `nextSeq(items)` and `numberLegacy(messages)` (assign `seq` in array order to items that lack one). Cover in `app/slices/bridle/utils/chatFlow.test.ts`: reply stamped 2 minutes earlier than the question still renders after it; equal `ts`; day boundary; legacy messages without `seq`. +- [ ] T007 [P] Create `app/slices/bridle/utils/delivery.ts`: constants `SLOW_MS = 5000`, `FAILED_MS = 30000`, pure reducer `nextDelivery(state, event)` for events `sent | tick(elapsedMs) | ackAccepted | ackRejected(code) | pageLoad | resend`, implementing the state machine in data-model.md (a late `ackAccepted` moves `failed` to `delivered`; `pageLoad` turns `sending`/`slow` into `failed` with `TIMEOUT`). Cover every transition in `app/slices/bridle/utils/delivery.test.ts`. +- [ ] T008 [P] Mirror T006 and T007 for the admin as `admin/slices/bridle/utils/chatFlow.ts`, `admin/slices/bridle/utils/delivery.ts` with their `.test.ts` files, adapted to the admin's `IBridleMessageData` / `IThinkingBlock` (status `'thinking' | 'done'`) shapes; add a header comment in all four files naming the twin file so they are changed together. +- [ ] T009 SSOT for admin chat state — refactor `admin/slices/bridle/stores/bridle.ts` from one global `messages` / `thinkingBlocks` / socket / `isTyping` / `isConnected` / `isAgentConnected` / transcript cursor into a record keyed by conversation `":"` (mirroring `app/slices/bridle/stores/bridle.ts`): per-key state object with `messages`, `thinkingBlocks`, `closedTurns`, `nextSeq`, `lastHubSeq`, flags, cursor and its own socket; actions take the key (`connect(apiUrl, agentId, channel)`, `sendMessage(key, …)`, `loadTranscript`, `loadOlderTranscript`, `clearMessages(key)`, `resetTranscript`); keep `markdownEnabled`, panel open/close, debug storage global. Update every consumer to pass and read its own key: `admin/slices/bridle/components/bridle/Provider.vue`, `admin/slices/bridle/components/bridle/Input.vue`, `admin/slices/rancher/components/rancher/Provider.vue`, `admin/slices/agent/agent/composables/useAgentLifecycle.ts`. Remove the mount-time `store.clearMessages()` "previous agent leaks through" workaround in `Provider.vue` — it exists only because of the singleton. +- [ ] T010 Several sockets per identity in the hub — in `api/src/slices/bridle/data/bridle.gateway.ts` change `clients` from one registration per `clientKey` to `Map>`: `registerClient` adds, `unregisterClient(clientId, agentId, socketId)` removes that socket only and clears `activeTurns` for the key when the last one leaves, `sendToClient` / `handleAgentEvent` / `broadcastAgentStatus` / `handleDebugEvent` send to **every** socket of the key, `prompt` / `capabilities` forwarded to the agent come from the sending socket, `health` / `agentHealth` / `listAgents` count sockets. Update the abstract class in `api/src/slices/bridle/domain/bridle.gateway.ts` and the callers in `api/src/slices/bridle/handlers/bridleClientWs.handler.ts`. Add cases to `api/src/slices/bridle/data/bridle.gateway.spec.ts`: two sockets of one identity both receive an agent event; disconnecting one leaves the other receiving. +- [ ] T011 Verify Phase 2: `cd api && bun run test -- bridle`, `cd app && bun test`, `cd admin && bun test`, `cd admin && npx nuxt typecheck`; run `node specs/015-chat-message-reliability/probe.mjs steal` and confirm **both** sockets now receive `typing` and the answer (before: only the second one did — research E3). Record the output in `research.md`. + +**Checkpoint**: the hub fans out to every view; the admin keeps one conversation record per key; ordering and delivery logic exist as tested pure functions. + +--- + +## Phase 3: User Story 2 — The conversation reads in the order it happened, each message once (P1) 🎯 MVP + +**Goal**: Question above its answer regardless of clock skew; no message twice; sending +scrolls to the bottom, incoming content never pulls a reader back down. + +**Independent Test**: quickstart scenarios 1, 2 and 10 — clock +2 min, send, multi-message +answer, follow-up mid-turn, reload; scroll up and send; two tabs. + +- [ ] T012 [US2] In `app/slices/bridle/stores/bridle.ts` assign `seq` from a per-conversation counter on every append: `appendMessage`, the new-bubble branch of `onStream`, `onMessage`, and when `onThinking` opens a block (replace `ts: Math.max(e.ts, lastTs + 1)` with `ts: e.ts` plus `seq`); in `hydrate` run `numberLegacy` over stored messages and restore the counter; keep `ts` untouched for display. +- [ ] T013 [US2] In `app/slices/bridle/components/bridle/chat/Provider.vue` replace the `chatFlow` computed (currently `items.sort((a, b) => a.ts - b.ts)`) with `buildChatFlow` from `#bridle/utils/chatFlow` (use the slice's existing alias), keying rows by the item `key`. +- [ ] T014 [US2] Scroll rule in `app/slices/bridle/components/bridle/chat/Provider.vue`: measure "near bottom" (within 80 px) **before** the DOM grows and follow incoming changes only then; in `onSend` always call `scrollToBottom()` after `nextTick`; keep the instant jump on mount. +- [ ] T015 [US2] In `admin/slices/bridle/stores/bridle.ts` assign `seq` per conversation on every push (`message`, `stream` new bubble, `stream_end` new bubble, `sendMessage` echo, thinking block open — drop the `Math.max(... lastTs + 1)` anchor); `loadTranscript` numbers messages in returned order starting at 1 and sets the counter; `loadOlderTranscript` numbers the prepended page with values **below** the current minimum so existing items keep their `seq`. +- [ ] T016 [US2] In `admin/slices/bridle/components/bridle/Provider.vue` build the flow with `buildChatFlow` from `admin/slices/bridle/utils/chatFlow.ts` instead of the `ts` sort, and scroll to the bottom on send regardless of position (keep the near-bottom rule for incoming content). +- [ ] T017 [US2] Echo the sender's message to the identity's other sockets: in `api/src/slices/bridle/data/bridle.gateway.ts` `sendToAgent`, after handing the message to the agent, send `{ type: 'user_message', messageId, text, attachments?, ts }` to every socket of the key **except** the sending one (pass the sending `socketId` in from `api/src/slices/bridle/handlers/bridleClientWs.handler.ts`); add the type to `api/src/slices/bridle/domain/bridle.types.ts`; cover in `bridle.gateway.spec.ts`. +- [ ] T018 [P] [US2] Handle `user_message` in the app: `socket.on('user_message')` in `app/slices/bridle/data/bridle.gateway.ts` (+ `onUserMessage` in `app/slices/bridle/domain/bridle.gateway.ts` events), and in `app/slices/bridle/stores/bridle.ts` append it as a delivered user message unless a message with that `id` already exists. +- [ ] T019 [P] [US2] Handle `user_message` in `admin/slices/bridle/stores/bridle.ts` the same way (skip when the id is already in that conversation). +- [ ] T020 [US2] Verify US2: `bun test` for chatFlow in both clients; with T003 run the skewed-clock scenario in app and admin and assert DOM order question → answer live and after reload; assert scroll-on-send from the top and no movement on incoming content while scrolled up; two pages of one login, send from the first, assert both show question once and answer once. Write results into `research.md` ("Observed in browser"). + +**Checkpoint**: wrong order and the scroll complaint are fixed and demonstrable on their own. + +--- + +## Phase 4: User Story 1 — What I sent is never silently lost (P1) + +**Goal**: Every send is acknowledged or visibly fails; failed messages survive a reload and +can be resent once without retyping; the landing → agent page handoff keeps the chat alive. + +**Independent Test**: quickstart scenarios 5 and 6, plus `probe.mjs normal` / `offline`. + +- [ ] T021 [US1] Wire contract types in `api/src/slices/bridle/domain/bridle.types.ts`: optional `clientMessageId` on the browser → hub `message` payload and the `BridleSendAck` union from contracts/bridle-socket.md. +- [ ] T022 [US1] In `api/src/slices/bridle/data/bridle.gateway.ts` make `sendToAgent` take an optional `clientMessageId` and a `withAck` flag and **return** `{ status: 'accepted', messageId, ts, duplicate? } | { status: 'rejected', code: 'AGENT_OFFLINE' }`: forward `clientMessageId` to the agent as `messageId` when present (else mint one as today); keep a per-`clientKey` seen-id cache (10-minute TTL, max 200) and answer a repeat with `duplicate: true` **without** forwarding; when the agent is offline return the rejection and send the synthetic "Agent is not connected" agent message **only** when `withAck` is false. Update the abstract signature in `api/src/slices/bridle/domain/bridle.gateway.ts` and the HTTP callers (`bridle.controller.ts`, `domain/bridleSync.service.ts`) to the new return type without changing their behaviour. +- [ ] T023 [US1] In `api/src/slices/bridle/handlers/bridleClientWs.handler.ts` `handleMessage`: accept the socket.io ack callback, call it exactly once on every path — `rejected/EMPTY` (nothing to send), `rejected/SHARE_REJECTED` (before the existing `bridle_error` + disconnect), `rejected/ATTACHMENT_FAILED` (alongside the existing `message_error`), or the result of `sendToAgent`; behave exactly as today when no callback is passed (embed widget). +- [ ] T024 [P] [US1] Add cases to `api/src/slices/bridle/handlers/bridleClientWs.handler.spec.ts` and `api/src/slices/bridle/data/bridle.gateway.spec.ts`: accepted ack carries the client's id; agent offline → rejection and **no** synthetic message with ack, synthetic message without ack; same `clientMessageId` twice → second is `duplicate` and the agent receives one message; attachment failure → rejection plus `message_error`. +- [ ] T025 [US1] App transport: in `app/slices/bridle/domain/bridle.gateway.ts` change `IBridleChannel.send` to `send(text, attachmentIds, clientMessageId): Promise`; in `app/slices/bridle/data/bridle.gateway.ts` emit with `socket.timeout(30_000).emit('message', { text, attachmentIds?, clientMessageId }, cb)` and resolve a timeout as `{ status: 'rejected', code: 'TIMEOUT' }`. +- [ ] T026 [US1] App store delivery in `app/slices/bridle/stores/bridle.ts`: `sendMessage` uses `crypto.randomUUID()` as the message `id` and `clientMessageId`, appends with `delivery: 'sending'`, starts a 5 s timer to `slow`, applies the ack through `nextDelivery` (on accept replace `ts` with the ack's `ts`), persists `delivery` with the conversation; `hydrate` applies `pageLoad` to every stored `sending`/`slow` message; new actions `resend(conv, id)` (same id, same text and attachment ids) and `discard(conv, id)`; when not connected, append the message as `failed` with `failureCode: 'OFFLINE'` instead of handing the text back to the composer — keep the CLEAN-72 draft hand-back only for the session-ended path in `onRejected`; `pending` no longer blocks a second send (FR: follow-up while the agent answers) — gate the composer on uploads only. +- [ ] T027 [US1] Admin store delivery in `admin/slices/bridle/stores/bridle.ts`: same rules as T026 on the per-conversation record — uuid as `id` + `clientMessageId`, emit with ack + 30 s timeout, never emit into a null or disconnected socket (mark `failed/OFFLINE` instead), `resend` / `discard` actions; persist non-delivered messages (text + attachment references, no image bytes) under `localStorage["bridle:outbox::"]`; after `loadTranscript` merge the outbox under the transcript, dropping an entry whose id is in the transcript **or** (interim, until the runtime persists ids — research E2) whose exact text appears in a transcript user message within ±2 minutes of its send time; mark that fallback with a comment pointing at T044. +- [ ] T028 [US1] Channel ownership in the app (research D7/F5): in `app/slices/bridle/stores/bridle.ts` turn `connect` / `disconnect` into reference-counted `acquire(conv)` / `release(conv)` — `channels: Map`, `release` at zero schedules the close after 3 s and `acquire` cancels it; a release with holders left must **not** touch `pending`, thinking blocks or the connection state. Switch the watcher in `app/slices/bridle/components/bridle/chat/Provider.vue` to acquire/release. Check the other mount points still behave: `app/slices/common/components/landing/hero/Provider.vue`, `app/slices/agent/components/agent/chat/Provider.vue`, `app/slices/share/components/share/page/Provider.vue`. +- [ ] T029 [US1] Verify US1: `probe.mjs normal` prints an `accepted` ack with the client's id; `probe.mjs offline` (stop the local runtime first) prints `rejected/AGENT_OFFLINE` and no agent bubble; jest specs from T024 pass; with T003: send → reload within 1 s → message present once; runtime stopped → send → reload → message present, not delivered → start runtime → Resend → delivered, one answer, one copy after another reload; Discard → gone after reload; landing hero question → click through to the agent page mid-answer → question once, no "Reconnecting…", answer continues. Record results in `research.md`. + +**Checkpoint**: nothing a person sends can disappear without a visible state. + +--- + +## Phase 5: User Story 5 — I can see when a message was sent and whether it got through (P2) + +**Goal**: Time under every message, date separators, delivery state with Resend / Discard +under the person's own messages. (Placed before US3/US4 because it is the visible half of +US1 and shares its code.) + +**Independent Test**: quickstart scenarios 3 and 4. + +- [ ] T030 [US5] Add keys to `app/slices/bridle/i18n/locales/en.json` — delivery (`sending`, `slow`, `not_delivered`, `not_delivered_hint` "The agent did not receive this message — it will be gone unless you resend it", per-code variants for `AGENT_OFFLINE` / `OFFLINE` / `TIMEOUT` / `ATTACHMENT_FAILED`), actions (`resend`, `discard`), day separators (`today`, `yesterday`) — following `docs/i18n.md`; then run `bun run i18n:sync` from the repo root to generate `ru.json`. Do not hand-write `ru.json`. +- [ ] T031 [US5] In `app/slices/bridle/components/bridle/chat/Message.vue` render under each bubble the time of day via `Intl.DateTimeFormat(locale, { hour: '2-digit', minute: '2-digit' })` with the full date-time in `title`; for the person's messages render the delivery line from `message.delivery` (nothing extra when delivered, spinner + `slow` text, `not_delivered` + hint + Resend / Discard buttons emitting `resend` / `discard`); copy decided in script travels as an i18n key, templates use `$t`. +- [ ] T032 [US5] In `app/slices/bridle/components/bridle/chat/Provider.vue` render `day` flow items as a centred separator (`today` / `yesterday` keys, otherwise a localised long date) and wire `resend` / `discard` from `Message.vue` to the store actions from T026. +- [ ] T033 [P] [US5] Admin equivalents in English only: time + tooltip + delivery line + Resend / Discard in `admin/slices/bridle/components/bridle/Message.vue`; day separators and action wiring in `admin/slices/bridle/components/bridle/Provider.vue`. +- [ ] T034 [US5] Verify US5 with T003: every bubble shows a time; times identical before and after reload; throttled network shows the loading state after ~5 s; runtime stopped shows "not delivered" under the message within 35 s; app in `ru` shows Russian wording, admin English. + +--- + +## Phase 6: User Story 4 — The answer shows up when the agent has answered (P2) + +**Goal**: Events that arrive while a browser is reconnecting are replayed; a transient +disconnect no longer kills the turn; a dead turn says so. + +**Independent Test**: quickstart scenario 8, `probe.mjs gap`. + +- [ ] T035 [US4] Sequence + replay buffer in `api/src/slices/bridle/data/bridle.gateway.ts`: per-`clientKey` counter stamped as `seq` on **every** event routed to a browser (`handleAgentEvent`, `sendToClient`, `user_message`, status, debug); per-key ring buffer (last 500 events or 10 minutes) filled **whether or not a socket is registered**; idle eviction of counter + buffer 10 minutes after the key's last socket left; `replaySince(clientKey, lastSeq)` returning buffered events in order; `currentSeq(clientKey)`. Add to the abstract class in `api/src/slices/bridle/domain/bridle.gateway.ts`. +- [ ] T036 [US4] In `api/src/slices/bridle/handlers/bridleClientWs.handler.ts` `handleConnection`: read optional numeric `lastSeq` from the handshake `auth`, emit `welcome` as `{ clientId, seq: currentSeq }`, then emit `replaySince(lastSeq)` to **this** socket before it is added to the fan-out set, so replay and live traffic cannot interleave out of order. +- [ ] T037 [P] [US4] Spec cases in `api/src/slices/bridle/data/bridle.gateway.spec.ts` and `bridleClientWs.handler.spec.ts`: event routed with no socket registered is replayed on reconnect with `lastSeq`; events at or below `lastSeq` are not replayed; buffer bounds and idle eviction; `welcome.seq` lower than the client's `lastSeq` (hub restarted). +- [ ] T038 [US4] App client in `app/slices/bridle/data/bridle.gateway.ts` + `app/slices/bridle/stores/bridle.ts`: pass `lastSeq` from the store in the `auth` function (called on every reconnect); store `lastHubSeq` per conversation and persist it with the conversation; ignore any event whose `seq` ≤ `lastHubSeq`; on `welcome` with a lower `seq` reset `lastHubSeq` and run the transcript reconcile (T040); in `onDisconnected` stop calling `closeAllTurns` and stop clearing `pending` — leave the turn open and let the watchdog decide. +- [ ] T039 [US4] Same in `admin/slices/bridle/stores/bridle.ts`: `lastSeq` in the socket `auth` callback, per-conversation `lastHubSeq`, drop stale `seq`, no `_closeAllTurns()` / `isTyping = false` on a transient `disconnect`, `welcome.seq` fallback to `loadTranscript` merge. +- [ ] T040 [US4] Safety net + visible failure: add `transcriptTail(agentId, channel)` to `app/slices/bridle/domain/bridle.gateway.ts` / `app/slices/bridle/data/bridle.gateway.ts` using the generated SDK call for `GET /api/agent/:agentId/transcript` (if the SDK lacks it, regenerate with `cd app && bun run build:api`, do not hand-write types); when the watchdog in `app/slices/bridle/stores/bridle.ts` expires with a turn still open, fetch the tail, append assistant messages newer than the last one on screen, and if there are none show a "the agent did not finish this turn" notice (new key in `en.json`, then `i18n:sync`). Same behaviour in `admin/slices/bridle/stores/bridle.ts` using its existing `fetchTranscriptPage`. +- [ ] T041 [US4] Verify US4: `probe.mjs gap` extended to send `lastSeq` on reconnect now reports the missed `stream_end` after reconnecting (before: none — research E4); jest specs from T037 pass; with T003: go offline across the end of a turn, come back, answer visible within 10 s with no duplicate bubble; restart the API during the gap and confirm the transcript fallback path runs (on this Windows machine the transcript route returns nothing — see research "Local-environment note" — so assert the *notice*, and validate the recovery itself on the cluster). + +--- + +## Phase 7: User Story 3 — A reloaded conversation looks like the one I watched (P2) + +**Goal**: One bubble per agent message after reload, same boundaries and formatting. +**The decisive change is in `CleanSlice/runtime`, not in this repository.** + +**Independent Test**: quickstart scenario 9, on the cluster. + +- [ ] T042 [US3] Add cases to `api/src/slices/agent/file/domain/transcriptReader.service.spec.ts` proving the reader already does the right thing once the runtime writes per-message events: several consecutive `assistant` events in one turn come back as separate messages in file order; events with equal `ts` keep file order; `transient` partial chunks are still dropped. Fix `transcriptReader.service.ts` only if a case fails. +- [ ] T043 [US3] **Separate repository — confirm with the requester before starting; needs its own CLEAN ticket, branch and PR in `CleanSlice/runtime` (`E:/code/dream/cleanslice/runtime`).** In `src/slices/runtime/loop/domain/loop.service.ts` persist one `assistant` event per message emitted to the channel, with the wire `messageId` as the event `id`, instead of one event per turn with the accumulated `fullText` in `sendFinalResponse`; persist the incoming `messageId` as the `id` of the `user` event. Keep the model-facing history equivalent (the next LLM call must still see the whole turn). Desirable: emit a `persisted` acknowledgement to the hub after the user event is written. +- [ ] T044 [US3] After T043 ships: remove the interim text-match fallback from T027 (match by id only) and treat a `persisted` ack, if implemented, as the `delivered` signal in `api/src/slices/bridle/data/bridle.gateway.ts`; update contracts/bridle-socket.md accordingly. +- [ ] T045 [US3] Verify US3 on the cluster: agent answers in ≥ 3 messages with list, inline code and bold; compare live view, admin after reload and the app's `/chats/:id` page — same count, boundaries and formatting, no glued sentences. Until T043 ships this is expected to **fail** and must be reported as failing, not skipped. + +--- + +## Phase 8: User Story 6 — An agent shows the same status everywhere (P3) — SSOT + +**Goal**: One agent record per id; every screen renders from it; fetches upsert into it; +pushes patch it. Independent of Phases 3–7 — can ship as its own PR. + +**Independent Test**: quickstart scenario 11. + +- [ ] T046 [US6] SSOT in `admin/slices/agent/agent/stores/agent.ts`: add `byId(id)` (computed lookup into `agents`), `upsert(agent)` (replace by id or append), `patch(id, partial)` returning a rollback function; make `fetchById` upsert its result before returning, `fetchAdmin` likewise; keep `fetchAll` as the collection load; route the existing optimistic status flips in `update` / restart / stop through `patch`. +- [ ] T047 [US6] In `admin/slices/agent/agent/stores/agentStatus.ts` `applyMessage`: write `status.agent` into the agent store with `useAgentStore().patch(status.agent.id, status.agent)` (upsert when the id is unknown), remove the record on `deleted`, and make the store's own `agents` map a derived view of the agent store (or delete it and update its readers) so there is no second copy. +- [ ] T048 [US6] Render from the store: in `admin/slices/agent/agent/components/agent/workspace/Main.vue` keep `useAsyncData(() => agentStore.fetchById(id))` for loading/error only and pass `computed(() => agentStore.byId(props.id))` to the template and to `useAgentLifecycle`; in `admin/slices/agent/agent/composables/useAgentLifecycle.ts` replace every `agent.value = { ...agent.value, status }` with `agentStore.patch(agentId, { status })` + rollback, and drop `liveAgent` merging that T047 makes redundant; do the same for `admin/slices/agent/agent/components/agent/edit/Provider.vue` and the two `fetchById` calls in `admin/slices/agent/file/components/agentFile/Provider.vue`. +- [ ] T049 [US6] In `admin/slices/agent/agent/components/agent/workspace/Provider.vue` pass `agentStore.agents` (via `storeToRefs`) to `AgentWorkspaceRail` instead of the `useAsyncData` `data` ref, and make sure `RailItem.vue` shows `statusReason` for the row; check the other list consumers read the store too: `admin/slices/agent/agent/pages/agents/index.vue`, `admin/slices/chat/components/chat/list/Provider.vue`, `admin/slices/paddock/components/paddock/evaluation/list/Provider.vue`, `admin/slices/llm/composables/useLlmUsageOverview.ts`. +- [ ] T050 [P] [US6] Same SSOT in the app: `app/slices/agent/stores/agent.ts` (`byId`, `upsert`, `patch`; `fetchById`, `create`, `update`, `restart` upsert; `fetchPublic` stays separate — public cards are a different projection), and render from the store in `app/slices/agent/components/agent/Provider.vue`, `app/slices/agent/components/agent/chat/Provider.vue` (its optimistic `agent.value = { ...status: 'deploying' }` becomes `patch` + rollback) and `app/slices/agent/components/agent/workspace/Provider.vue`. +- [ ] T051 [US6] Check the server side of the same defect: `useAgentLifecycle` says "Backend syncStatus runs on each fetchById". In `api/src/slices/agent/agent/` find where status is reconciled and whether the list endpoint (`findAll`) and the `/agents/status/stream` SSE reconcile too; if a non-selected agent's status can stay stale in the database until someone opens it, make the periodic reconciler / stream cover every agent, and add a spec next to the service. Record the finding in `research.md` either way. +- [ ] T052 [US6] Verify US6: on the agents screen trigger a deploy that ends in `failed` — list row, header pill and Overview card change together within 10 s without reload and the reason is on the row; repeat with a different agent selected; `cd admin && npx nuxt typecheck`, `cd app && npx nuxt typecheck`. + +--- + +## Phase 9: Polish & cross-cutting + +- [ ] T053 [P] Write the SSOT rule down so the defect class does not return: `docs/state.md` — "an entity lives once in its Pinia store; fetches upsert; pushes patch; components render by id; `useAsyncData` is for loading state, not a render source; optimistic changes go through a store action with rollback" — with the agent store and the chat conversation record as the two worked examples; link it from `AGENTS.md` and `CLAUDE.md`. +- [ ] T054 [P] Update `README.md` / `docs/operations/` where they describe the chat socket protocol (ack, `clientMessageId`, `seq` / `lastSeq`, `user_message`, several sockets per identity) — point to `specs/015-chat-message-reliability/contracts/bridle-socket.md` rather than duplicating it. +- [ ] T055 Full gate: `cd api && bun run lint && bun run test`; `cd app && bun test && npx nuxt typecheck`; `cd admin && bun test && npx nuxt typecheck` (not `bun run typecheck` in `app/` — it regenerates the SDK); confirm no generated file under `*/slices/setup/api/data/repositories/api/` is staged unless T040 required a regeneration. +- [ ] T056 Run every scenario in `specs/015-chat-message-reliability/quickstart.md`, fill the "Observed" results into `research.md`, and update `spec.md` success-criteria status honestly — including anything that still fails (US3 until T043). +- [ ] T057 Delivery cycle: CLEAN-102 checkpoint comments after each phase, Conventional Commits with `(CLEAN-102)` — `fix(api):`, `fix(app):`, `fix(admin):` — PR into `main` with the ticket linked, PR URL on the ticket, move to In Testing; a separate ticket + PR for T043 in `CleanSlice/runtime`; if Phase 8 ships on its own, give it its own PR from the same ticket or a new one, as the requester prefers. + +--- + +## Dependencies & execution order + +### Phase dependencies + +- **Phase 1** → no dependencies. T004 informs T009 and T028 (it confirms or refutes F3 / F5). +- **Phase 2** → blocks Phases 3–7. T009 blocks every admin chat task; T010 blocks T017, T022, T035. +- **Phase 3 (US2)** → after Phase 2. No API dependency except T017–T019. +- **Phase 4 (US1)** → after Phase 2; T026/T027 build on the `seq` work of T012/T015 (same files — do not run in parallel with Phase 3). +- **Phase 5 (US5)** → after Phase 4 (renders what US1 stores). +- **Phase 6 (US4)** → after Phase 2; touches the same store files as Phases 3–4, so sequence it after them. +- **Phase 7 (US3)** → T042 any time; T043 is external; T044–T045 after T043. +- **Phase 8 (US6)** → independent of all chat phases; can run in parallel with them (different slices) or ship first. +- **Phase 9** → last. + +### Same-file serialisation + +`app/slices/bridle/stores/bridle.ts`: T012 → T018 → T026 → T028 → T038 → T040. +`admin/slices/bridle/stores/bridle.ts`: T009 → T015 → T019 → T027 → T039 → T040. +`api/src/slices/bridle/data/bridle.gateway.ts`: T010 → T017 → T022 → T035. +`api/src/slices/bridle/handlers/bridleClientWs.handler.ts`: T010 → T017 → T023 → T036. + +### Parallel opportunities + +- Phase 1: T002, T003 alongside T001. +- Phase 2: T005, T006, T007, T008 together; T009 (admin) and T010 (API) are different projects and can run side by side. +- Phase 3: T018 (app) and T019 (admin) together after T017. +- Phase 4: T024 (specs) alongside T025–T027; T026 (app) and T027 (admin) are different projects. +- Phase 5: T033 (admin) alongside T030–T032 (app). +- Phase 6: T037 alongside T038–T039; T038 (app) and T039 (admin) side by side. +- Phase 8 as a whole, and T050 (app) alongside T046–T049 (admin). + +## Parallel example: Phase 2 + +```text +Task: "T006 chatFlow.ts + tests in app/slices/bridle/utils/" +Task: "T007 delivery.ts + tests in app/slices/bridle/utils/" +Task: "T008 admin twins in admin/slices/bridle/utils/" +Task: "T010 several sockets per identity in api/src/slices/bridle/data/bridle.gateway.ts" +# then, on its own: T009 admin store refactor +``` + +## Implementation strategy + +### MVP + +Phase 1 → Phase 2 → **Phase 3 (US2)**. That alone fixes the wrong order, the scroll +complaint and — through T010 — the reproduced "the tab that sent gets nothing" defect, +which is the likeliest cause of hung answers. Stop, verify with T011 and T020, demo. + +### Incremental delivery + +1. MVP above → PR 1 (or keep on the branch and continue). +2. Phase 4 + Phase 5 (US1 + US5): acknowledged sends, outbox, time and delivery state. +3. Phase 6 (US4): replay after reconnect. +4. Phase 8 (US6, SSOT for agents): independent; ship whenever convenient. +5. Phase 7 (US3): gated on the runtime change (T043) — schedule it with whoever owns `CleanSlice/runtime`. + +## Notes + +- Verify tasks are the definition of done for a story; "should work" is not a result — + write the observed output into `research.md`. +- Every wire change is additive and optional (embed widget compatibility). +- The hub's new state is in memory; the plan assumes one API instance, as the hub already does. +- Commit after each task or logical group; never commit `.env.project` or regenerated SDK files by accident. From 93443c6005e7db89139802c35b22aa6bfc0cb54a Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Fri, 18 Sep 2026 20:04:28 +0300 Subject: [PATCH 02/10] fix(api): hub delivers to every socket, acknowledges sends, replays missed events (CLEAN-102) The hub kept one socket per identity and every Owner/Admin chats as `admin`, so the last view to connect took every event: the tab that asked got nothing and another tab showed the answer. A conversation now holds all its sockets and fans out to them; the sender's other views get a `user_message` echo. Sends are acknowledged through the socket.io ack when the browser mints a `clientMessageId`: accepted / rejected (AGENT_OFFLINE, ATTACHMENT_FAILED, SHARE_REJECTED, EMPTY), the id travels to the agent as the message id, and a resend of an id already handed over is not forwarded twice. Callers that send no id (embed widget) keep the synthetic "Agent is not connected" reply. Routed events carry a per-conversation `seq` and are kept in a bounded buffer, also while no socket is connected; a reconnect with `lastSeq` gets what it missed before live traffic. Verified against the local stack with specs/015-chat-message-reliability/probe.mjs (steal, gap). Co-Authored-By: Claude Fable 5.1 --- .../data/bridle.gateway.channels.spec.ts | 228 +++++++++++++++ api/src/slices/bridle/data/bridle.gateway.ts | 273 ++++++++++++++---- .../slices/bridle/domain/bridle.gateway.ts | 31 +- api/src/slices/bridle/domain/bridle.types.ts | 59 ++++ .../bridle/domain/bridleSync.service.ts | 6 +- .../handlers/bridleClientWs.ack.spec.ts | 113 ++++++++ .../handlers/bridleClientWs.handler.spec.ts | 8 +- .../bridle/handlers/bridleClientWs.handler.ts | 50 +++- specs/015-chat-message-reliability/probe.mjs | 181 ++++++------ 9 files changed, 802 insertions(+), 147 deletions(-) create mode 100644 api/src/slices/bridle/data/bridle.gateway.channels.spec.ts create mode 100644 api/src/slices/bridle/handlers/bridleClientWs.ack.spec.ts diff --git a/api/src/slices/bridle/data/bridle.gateway.channels.spec.ts b/api/src/slices/bridle/data/bridle.gateway.channels.spec.ts new file mode 100644 index 00000000..774f00ca --- /dev/null +++ b/api/src/slices/bridle/data/bridle.gateway.channels.spec.ts @@ -0,0 +1,228 @@ +import { BridleGateway } from './bridle.gateway'; + +type Event = Record; + +function collector(into: Event[]) { + return (data: unknown) => { + into.push(data as Event); + }; +} + +/** + * One identity, several places (CLEAN-102). Every Owner/Admin chats as + * `admin`, and a conversation is routinely open in two tabs or in the admin + * panel next to the console. With one slot per identity the last socket to + * connect took every event and the tab that asked the question got nothing. + */ +describe('BridleGateway — several sockets on one conversation', () => { + function twoTabs() { + const gateway = new BridleGateway(); + const toAgent: Event[] = []; + const tabA: Event[] = []; + const tabB: Event[] = []; + gateway.registerAgent('agent-1', 'agent-socket', collector(toAgent)); + gateway.registerClient('admin', 'agent-1', 'tab-a', collector(tabA), true); + gateway.registerClient('admin', 'agent-1', 'tab-b', collector(tabB), true); + return { gateway, toAgent, tabA, tabB }; + } + + it('delivers the answer to the tab that asked, not only the newest one', () => { + const { gateway, tabA, tabB } = twoTabs(); + + gateway.handleAgentEvent('agent-1', { + type: 'message', + clientId: 'admin', + text: 'hi', + messageId: 'm1', + }); + + expect(tabA.map((e) => e.type)).toEqual(['message']); + expect(tabB.map((e) => e.type)).toEqual(['message']); + expect(tabA[0].seq).toBe(tabB[0].seq); + }); + + it('keeps the other tab receiving when one of them leaves', () => { + const { gateway, tabA, tabB } = twoTabs(); + + gateway.unregisterClient('admin', 'agent-1', 'tab-b'); + gateway.handleAgentEvent('agent-1', { + type: 'message', + clientId: 'admin', + text: 'hi', + messageId: 'm1', + }); + + expect(tabA).toHaveLength(1); + expect(tabB).toHaveLength(0); + }); + + it('shows the question in the other tab, not in the one that sent it', () => { + const { gateway, tabA, tabB } = twoTabs(); + + gateway.sendToAgent('admin', 'agent-1', 'model text', [], undefined, { + socketId: 'tab-a', + clientMessageId: 'c1', + displayText: 'typed text', + }); + + expect(tabA).toHaveLength(0); + expect(tabB).toHaveLength(1); + expect(tabB[0]).toMatchObject({ + type: 'user_message', + messageId: 'c1', + text: 'typed text', + }); + }); + + it('counts sockets, not identities, in health', () => { + const { gateway } = twoTabs(); + + expect(gateway.agentHealth('agent-1').browserClients).toBe(2); + }); +}); + +describe('BridleGateway — acknowledged sends', () => { + function setup(agentOnline = true) { + const gateway = new BridleGateway(); + const toAgent: Event[] = []; + const toBrowser: Event[] = []; + if (agentOnline) { + gateway.registerAgent('agent-1', 'agent-socket', collector(toAgent)); + } + gateway.registerClient( + 'admin', + 'agent-1', + 'tab-a', + collector(toBrowser), + true, + ); + return { gateway, toAgent, toBrowser }; + } + const withAck = { socketId: 'tab-a', clientMessageId: 'c1', withAck: true }; + + it('forwards the id the browser minted and reports it accepted', () => { + const { gateway, toAgent } = setup(); + + const result = gateway.sendToAgent( + 'admin', + 'agent-1', + 'hi', + [], + undefined, + withAck, + ); + + expect(result).toMatchObject({ status: 'accepted', messageId: 'c1' }); + expect(toAgent).toHaveLength(1); + expect(toAgent[0].messageId).toBe('c1'); + }); + + it('hands a resend to the agent only once', () => { + const { gateway, toAgent } = setup(); + + gateway.sendToAgent('admin', 'agent-1', 'hi', [], undefined, withAck); + const again = gateway.sendToAgent( + 'admin', + 'agent-1', + 'hi', + [], + undefined, + withAck, + ); + + expect(again).toMatchObject({ status: 'accepted', duplicate: true }); + expect(toAgent).toHaveLength(1); + }); + + it('rejects instead of faking an agent reply when the caller wants an ack', () => { + const { gateway, toBrowser } = setup(false); + + const result = gateway.sendToAgent( + 'admin', + 'agent-1', + 'hi', + [], + undefined, + withAck, + ); + + expect(result).toEqual({ status: 'rejected', code: 'AGENT_OFFLINE' }); + expect(toBrowser).toHaveLength(0); + }); + + it('keeps the synthetic reply for callers that send no id (embed widget)', () => { + const { gateway, toBrowser } = setup(false); + + const result = gateway.sendToAgent('admin', 'agent-1', 'hi', []); + + expect(result).toEqual({ status: 'rejected', code: 'AGENT_OFFLINE' }); + expect(toBrowser).toHaveLength(1); + expect(toBrowser[0]).toMatchObject({ type: 'message' }); + }); +}); + +/** + * The answer that landed while the browser was reconnecting used to be gone + * for good: the agent's log had it, the chat kept spinning (CLEAN-102). + */ +describe('BridleGateway — catching up after a reconnect', () => { + function emit( + gateway: BridleGateway, + messageId: string, + type: 'message' | 'stream' | 'stream_end' = 'message', + ) { + gateway.handleAgentEvent('agent-1', { + type, + clientId: 'admin', + text: messageId, + messageId, + }); + } + + it('keeps events that arrive while no socket is connected', () => { + const gateway = new BridleGateway(); + const first: Event[] = []; + gateway.registerClient('admin', 'agent-1', 'tab-a', collector(first), true); + emit(gateway, 'm1'); + const lastSeq = first[0].seq as number; + + gateway.unregisterClient('admin', 'agent-1', 'tab-a'); + emit(gateway, 'm2'); + + const missed = gateway.replaySince('admin', 'agent-1', lastSeq) as Event[]; + expect(missed.map((e) => e.messageId)).toEqual(['m2']); + expect(missed[0].seq as number).toBeGreaterThan(lastSeq); + }); + + it('replays nothing that the browser already has', () => { + const gateway = new BridleGateway(); + gateway.registerClient('admin', 'agent-1', 'tab-a', () => undefined, true); + emit(gateway, 'm1'); + + const upToDate = gateway.currentSeq('admin', 'agent-1'); + + expect(gateway.replaySince('admin', 'agent-1', upToDate)).toEqual([]); + }); + + it('replays only the newest frame of a streamed message', () => { + const gateway = new BridleGateway(); + gateway.registerClient('admin', 'agent-1', 'tab-a', () => undefined, true); + const before = gateway.currentSeq('admin', 'agent-1'); + + emit(gateway, 'm1', 'stream'); + emit(gateway, 'm1', 'stream'); + emit(gateway, 'm1', 'stream_end'); + + const missed = gateway.replaySince('admin', 'agent-1', before) as Event[]; + expect(missed.map((e) => e.type)).toEqual(['stream', 'stream_end']); + }); + + it('knows nothing about an identity that never connected', () => { + const gateway = new BridleGateway(); + + emit(gateway, 'm1'); + + expect(gateway.currentSeq('admin', 'agent-1')).toBe(0); + expect(gateway.replaySince('admin', 'agent-1', 0)).toEqual([]); + }); +}); diff --git a/api/src/slices/bridle/data/bridle.gateway.ts b/api/src/slices/bridle/data/bridle.gateway.ts index e913123c..34cfa2bd 100644 --- a/api/src/slices/bridle/data/bridle.gateway.ts +++ b/api/src/slices/bridle/data/bridle.gateway.ts @@ -15,6 +15,8 @@ import type { BridlePart, IBridleAttachment, IActiveTurn, + IBridleSendOptions, + BridleSendResult, } from '../domain/bridle.types'; import { randomUUID } from 'crypto'; @@ -27,6 +29,31 @@ interface IPendingSync { const DEFAULT_SYNC_TIMEOUT_MS = 15_000; +/** How much a reconnecting browser can catch up on — whichever runs out first. */ +const REPLAY_MAX_EVENTS = 500; +const REPLAY_MAX_AGE_MS = 10 * 60_000; +/** A conversation nobody is connected to is forgotten after this long. */ +const CHANNEL_IDLE_MS = REPLAY_MAX_AGE_MS; +/** How long a message id is remembered, so a resend is not delivered twice. */ +const SEEN_TTL_MS = 10 * 60_000; +const SEEN_MAX = 200; + +interface IBufferedEvent { + at: number; + event: Record & { seq: number }; +} + +interface IClientChannel { + /** Every socket open on this conversation, by socket id. */ + sockets: Map; + /** Last number issued; strictly increasing for the conversation's life. */ + seq: number; + buffer: IBufferedEvent[]; + /** clientMessageId → when it was accepted. */ + seen: Map; + idleTimer?: NodeJS.Timeout; +} + /** * Hub implementation — manages per-agent connections and per-agent browser * client connections. Routes messages between them scoped by agentId. @@ -49,17 +76,97 @@ export class BridleGateway extends IBridleGateway { /** * Browser clients keyed by `${clientId}\u0000${agentId}`. Keying by the pair * (not clientId alone) lets ONE user hold several concurrent conversations — - * e.g. a multi-slot dashboard chatting with N agents on N sockets — without - * later sockets overwriting earlier ones (they share clientId='admin'/sub). + * e.g. a multi-slot dashboard chatting with N agents. + * + * A conversation holds EVERY socket open on it, not one (CLEAN-102). An + * identity is routinely open in several places at once — two tabs, the admin + * panel beside the console, a colleague (every Owner/Admin shares + * clientId='admin'), an HTTP sendAndAwait — and with a single slot the last + * one to connect took every event while the others sat on a spinner for an + * answer that was delivered somewhere else. + * + * It also outlives its sockets for a while: events are numbered and kept in + * a bounded buffer so a browser that was mid-reconnect when the answer + * landed can ask for what it missed instead of never seeing it. */ - private clients = new Map(); + private channels = new Map(); private clientKey(clientId: string, agentId: string): string { return `${clientId}\u0000${agentId}`; } + private *sockets(): IterableIterator { + for (const channel of this.channels.values()) { + yield* channel.sockets.values(); + } + } + + private channelFor(clientId: string, agentId: string): IClientChannel { + const key = this.clientKey(clientId, agentId); + let channel = this.channels.get(key); + if (!channel) { + channel = { + sockets: new Map(), + // Seeded from the clock, not zero: after an API restart the new + // numbers are still above any `lastSeq` a browser kept, so its + // catch-up request returns the new buffer instead of skipping it. + seq: Date.now(), + buffer: [], + seen: new Map(), + }; + this.channels.set(key, channel); + } + return channel; + } + /** - * Turns in flight, keyed like `clients`. Written from the thinking events + * Number an event, remember it, and hand it to every socket on the + * conversation — minus the one that caused it, for `user_message`. + */ + private route( + channel: IClientChannel, + data: Record, + exceptSocketId?: string, + ): void { + const now = Date.now(); + const event: IBufferedEvent['event'] = { ...data, seq: ++channel.seq }; + // A `stream` frame carries the whole text so far, so only the newest one + // per message is worth replaying — keeping them all would fill the buffer + // with every intermediate state of one long answer. + if (data.type === 'stream' && typeof data.messageId === 'string') { + channel.buffer = channel.buffer.filter( + (b) => + !(b.event.type === 'stream' && b.event.messageId === data.messageId), + ); + } + channel.buffer.push({ at: now, event }); + while ( + channel.buffer.length > REPLAY_MAX_EVENTS || + (channel.buffer.length > 0 && + now - channel.buffer[0].at > REPLAY_MAX_AGE_MS) + ) { + channel.buffer.shift(); + } + for (const socket of channel.sockets.values()) { + if (socket.socketId !== exceptSocketId) socket.send(event); + } + } + + replaySince(clientId: string, agentId: string, lastSeq: number): unknown[] { + const channel = this.channels.get(this.clientKey(clientId, agentId)); + if (!channel) return []; + const cutoff = Date.now() - REPLAY_MAX_AGE_MS; + return channel.buffer + .filter((b) => b.at >= cutoff && b.event.seq > lastSeq) + .map((b) => b.event); + } + + currentSeq(clientId: string, agentId: string): number { + return this.channels.get(this.clientKey(clientId, agentId))?.seq ?? 0; + } + + /** + * Turns in flight, keyed like `channels`. Written from the thinking events * the hub already relays, so the API can drop a step of its own into the * timeline a person is watching (CLEAN-74) instead of minting a turnId that * would close the runtime's own block in every console. @@ -126,7 +233,7 @@ export class BridleGateway extends IBridleGateway { * agent connected) vs orange (one side down) without polling. */ private broadcastAgentStatus(agentId: string, connected: boolean): void { - for (const client of this.clients.values()) { + for (const client of this.sockets()) { if (client.agentId !== agentId) continue; client.send({ type: 'agent_status', agentId, connected }); } @@ -145,7 +252,12 @@ export class BridleGateway extends IBridleGateway { prompt?: string, capabilities?: string[], ): void { - this.clients.set(this.clientKey(clientId, agentId), { + const channel = this.channelFor(clientId, agentId); + if (channel.idleTimer) { + clearTimeout(channel.idleTimer); + channel.idleTimer = undefined; + } + channel.sockets.set(socketId, { clientId, agentId, socketId, @@ -155,29 +267,28 @@ export class BridleGateway extends IBridleGateway { ...(capabilities && capabilities.length ? { capabilities } : {}), }); this.logger.log( - `Browser client registered: ${clientId} agentId=${agentId} socket=${socketId} admin=${isAdmin}${capabilities?.length ? ` caps=[${capabilities.join(',')}]` : ''} (total: ${this.clients.size})`, + `Browser client registered: ${clientId} agentId=${agentId} socket=${socketId} admin=${isAdmin}${capabilities?.length ? ` caps=[${capabilities.join(',')}]` : ''} (sockets on this conversation: ${channel.sockets.size})`, ); } unregisterClient(clientId: string, agentId: string, socketId: string): void { const key = this.clientKey(clientId, agentId); - const current = this.clients.get(key); - if (!current) return; - if (current.socketId !== socketId) { - // A stale/blackholed connection (detected late via ping timeout) is - // disconnecting after a reconnect already took over this clientId — - // the live registration must survive, or the browser silently stops - // receiving stream/message events until the page is reloaded. - this.logger.log( - `Ignoring stale disconnect for client=${clientId} agentId=${agentId}: socket=${socketId} is not the current owner (${current.socketId})`, - ); - return; - } - this.clients.delete(key); - this.activeTurns.delete(key); + const channel = this.channels.get(key); + // Removing by socket id is what makes a stale/blackholed connection's late + // disconnect harmless: it can only ever remove itself. + if (!channel?.sockets.delete(socketId)) return; this.logger.log( - `Browser client unregistered: ${clientId} agentId=${agentId} (total: ${this.clients.size})`, + `Browser client unregistered: ${clientId} agentId=${agentId} socket=${socketId} (sockets left: ${channel.sockets.size})`, ); + if (channel.sockets.size > 0) return; + + this.activeTurns.delete(key); + // Keep the numbering and the buffer for a while — the usual reason for an + // empty conversation is a page that is about to reconnect. + channel.idleTimer = setTimeout(() => { + if (channel.sockets.size === 0) this.channels.delete(key); + }, CHANNEL_IDLE_MS); + channel.idleTimer.unref?.(); } sendToAgent( @@ -186,28 +297,58 @@ export class BridleGateway extends IBridleGateway { text: string, parts: BridlePart[], attachments?: IBridleAttachment[], - ): void { + options: IBridleSendOptions = {}, + ): BridleSendResult { + const channel = this.channelFor(clientId, agentId); + const now = Date.now(); + + // A resend of something already handed over (the ack was lost, not the + // message) must not make the agent answer twice. + const { clientMessageId } = options; + if (clientMessageId) { + const acceptedAt = channel.seen.get(clientMessageId); + if (acceptedAt !== undefined && now - acceptedAt < SEEN_TTL_MS) { + return { + status: 'accepted', + messageId: clientMessageId, + ts: acceptedAt, + duplicate: true, + }; + } + } + const agentSend = this.agents.get(agentId)?.send; if (!agentSend) { this.logger.warn( `Cannot send to agent — not connected (agentId=${agentId})`, ); - this.sendToClient(clientId, agentId, { - type: 'message', - text: 'Agent is not connected. Please try again later.', - parts: [ - { - type: 'text', - text: 'Agent is not connected. Please try again later.', - }, - ], - messageId: randomUUID(), - ts: Date.now(), - }); - return; + // A caller that asked for an acknowledgement is told the truth and shows + // it under the person's own message. Everyone else (embed widget, older + // bundles) keeps the sentence they have always rendered as a reply. + if (!options.withAck) { + this.sendToClient(clientId, agentId, { + type: 'message', + text: 'Agent is not connected. Please try again later.', + parts: [ + { + type: 'text', + text: 'Agent is not connected. Please try again later.', + }, + ], + messageId: randomUUID(), + ts: now, + }); + } + return { status: 'rejected', code: 'AGENT_OFFLINE' }; } - const client = this.clients.get(this.clientKey(clientId, agentId)); + // The browser's own id travels end to end when it sent one, so the bubble + // on screen, the message the agent gets and (once the runtime stores it) + // the transcript entry are one and the same message. + const messageId = clientMessageId ?? randomUUID(); + const client = + (options.socketId && channel.sockets.get(options.socketId)) || + channel.sockets.values().next().value; agentSend({ type: 'message', clientId, @@ -232,15 +373,47 @@ export class BridleGateway extends IBridleGateway { })), } : {}), - messageId: randomUUID(), + messageId, }); + + if (clientMessageId) { + channel.seen.set(clientMessageId, now); + for (const [id, at] of channel.seen) { + if (channel.seen.size <= SEEN_MAX && now - at < SEEN_TTL_MS) break; + channel.seen.delete(id); + } + } + + // The other places this conversation is open get the question too — + // otherwise they would show an answer to something nobody asked there. + this.route( + channel, + { + type: 'user_message', + messageId, + text: options.displayText ?? text, + ...(attachments?.length + ? { + attachments: attachments.map((a) => ({ + id: a.id, + name: a.name, + mimeType: a.mimeType, + size: a.size, + kind: a.kind, + })), + } + : {}), + ts: now, + }, + options.socketId, + ); + + return { status: 'accepted', messageId, ts: now }; } sendToClient(clientId: string, agentId: string, data: unknown): void { - const client = this.clients.get(this.clientKey(clientId, agentId)); - if (client) { - client.send(data); - } + const channel = this.channels.get(this.clientKey(clientId, agentId)); + if (channel) this.route(channel, data as Record); } handleAgentEvent(agentId: string, data: IBridleOutgoingEvent): void { @@ -251,10 +424,10 @@ export class BridleGateway extends IBridleGateway { this.trackTurn(agentId, clientId, data); } - const client = this.clients.get(this.clientKey(clientId, agentId)); - if (client) { - client.send(data); - } + // No socket right now is not a reason to drop the event: `route` keeps it + // for the reconnect. An identity the hub has never seen gets nothing. + const channel = this.channels.get(this.clientKey(clientId, agentId)); + if (channel) this.route(channel, { ...data }); } /** A step opens or refreshes the turn; the terminal `done` closes it. */ @@ -331,7 +504,7 @@ export class BridleGateway extends IBridleGateway { // only knows the immediate sender, but multiple admins may be observing // the same agent and they all want to see prompt traces. let delivered = 0; - for (const client of this.clients.values()) { + for (const client of this.sockets()) { if (client.agentId !== agentId) continue; if (!client.isAdmin) continue; client.send(data); @@ -348,13 +521,13 @@ export class BridleGateway extends IBridleGateway { return { ok: true, agentConnected: this.agents.size > 0, - browserClients: this.clients.size, + browserClients: [...this.sockets()].length, }; } agentHealth(agentId: string): IBridleAgentHealthData { let clientCount = 0; - for (const client of this.clients.values()) { + for (const client of this.sockets()) { if (client.agentId === agentId) clientCount++; } return { @@ -409,7 +582,7 @@ export class BridleGateway extends IBridleGateway { const result: Array<{ agentId: string; clients: number }> = []; for (const agentId of this.agents.keys()) { let clients = 0; - for (const c of this.clients.values()) { + for (const c of this.sockets()) { if (c.agentId === agentId) clients++; } result.push({ agentId, clients }); diff --git a/api/src/slices/bridle/domain/bridle.gateway.ts b/api/src/slices/bridle/domain/bridle.gateway.ts index 1367fe67..4509df0b 100644 --- a/api/src/slices/bridle/domain/bridle.gateway.ts +++ b/api/src/slices/bridle/domain/bridle.gateway.ts @@ -8,6 +8,8 @@ import type { BridlePart, IBridleAttachment, IActiveTurn, + IBridleSendOptions, + BridleSendResult, } from './bridle.types'; export interface ISyncAgentResult { @@ -35,12 +37,26 @@ export abstract class IBridleGateway { text: string, parts: BridlePart[], attachments?: IBridleAttachment[], - ): void; - /** Send an event to a specific browser client (scoped to clientId + agentId) */ + options?: IBridleSendOptions, + ): BridleSendResult; + /** Send an event to every socket open on this conversation (clientId + + * agentId), numbered and kept for replay like the agent's own events. */ abstract sendToClient(clientId: string, agentId: string, data: unknown): void; - /** Register a browser client for a specific agent. `socketId` marks the - * owning socket so a stale connection's late disconnect can't wipe a newer - * registration for the same clientId+agentId (mirrors registerAgent). */ + /** + * Events routed to this conversation after `lastSeq`, oldest first — what a + * browser missed while it was reconnecting. Empty when nothing is buffered + * or the conversation is unknown. + */ + abstract replaySince( + clientId: string, + agentId: string, + lastSeq: number, + ): unknown[]; + /** Last sequence number issued for this conversation; 0 when unknown. */ + abstract currentSeq(clientId: string, agentId: string): number; + /** Register a browser socket on a conversation. Several sockets may share + * one clientId+agentId — they all receive its events; `socketId` tells them + * apart so each one can only ever unregister itself. */ abstract registerClient( clientId: string, agentId: string, @@ -54,8 +70,9 @@ export abstract class IBridleGateway { * every message so runtimes can gate `thinking`/`ui` emission. */ capabilities?: string[], ): void; - /** Unregister a browser client — no-op unless `socketId` still owns the - * current registration for clientId+agentId. */ + /** Unregister one browser socket. The conversation's other sockets are + * untouched; its numbering and replay buffer outlive the last one for a + * while so a reconnect can catch up. */ abstract unregisterClient( clientId: string, agentId: string, diff --git a/api/src/slices/bridle/domain/bridle.types.ts b/api/src/slices/bridle/domain/bridle.types.ts index b75b5dc9..84710c8b 100644 --- a/api/src/slices/bridle/domain/bridle.types.ts +++ b/api/src/slices/bridle/domain/bridle.types.ts @@ -101,6 +101,65 @@ export interface IBridleIncomingMessage { capabilities?: string[]; } +/** + * What a browser is told about a message it sent, through the socket.io + * acknowledgement (CLEAN-102). `accepted` means "handed to a connected agent", + * not "answered": it is what lets the chat show delivered / not delivered + * under a person's own message instead of a bubble that looks the same either + * way. Only callers that pass an ack callback get one — the embed widget and + * older bundles send none and behave exactly as before. + */ +export type BridleSendAck = + | { + status: 'accepted'; + /** The browser's own `clientMessageId` when it sent one. */ + messageId: string; + /** Hub clock at acceptance — the time both live and replayed views show. */ + ts: number; + /** A resend of something already handed over; not forwarded again. */ + duplicate?: true; + } + | { + status: 'rejected'; + code: 'AGENT_OFFLINE' | 'ATTACHMENT_FAILED' | 'SHARE_REJECTED' | 'EMPTY'; + message?: string; + }; + +/** The subset of {@link BridleSendAck} the hub itself can produce. */ +export type BridleSendResult = + | Extract + | { status: 'rejected'; code: 'AGENT_OFFLINE' }; + +export interface IBridleSendOptions { + /** Id minted by the browser; becomes the message id end to end. */ + clientMessageId?: string; + /** The sending socket: its prompt/capabilities go to the agent, and it is + * the one socket that does NOT get the `user_message` echo. */ + socketId?: string; + /** The caller renders the outcome itself, so no synthetic "Agent is not + * connected" reply is sent on its behalf. */ + withAck?: boolean; + /** What the person typed, without the attachment blocks inlined for the + * model — that is what the other open views should show. */ + displayText?: string; +} + +/** + * Hub → browser: a message sent from ANOTHER socket of the same conversation + * (second tab, the admin next to the console). Without it that view would + * show an answer to a question nobody asked there. + */ +export interface IBridleUserMessageEvent { + type: 'user_message'; + messageId: string; + text: string; + attachments?: Array< + Pick + >; + ts: number; + seq: number; +} + /** Agent → Hub: events routed to browser clients */ export interface IBridleOutgoingEvent { type: diff --git a/api/src/slices/bridle/domain/bridleSync.service.ts b/api/src/slices/bridle/domain/bridleSync.service.ts index 30455377..7891b2fb 100644 --- a/api/src/slices/bridle/domain/bridleSync.service.ts +++ b/api/src/slices/bridle/domain/bridleSync.service.ts @@ -111,7 +111,11 @@ export class BridleSyncService { capabilities, ); - this.hub.sendToAgent(clientId, agentId, text, parts, attachments); + // `socketId` makes this call's own capabilities travel with the message + // now that a conversation can hold several sockets at once. + this.hub.sendToAgent(clientId, agentId, text, parts, attachments, { + socketId, + }); }); } } diff --git a/api/src/slices/bridle/handlers/bridleClientWs.ack.spec.ts b/api/src/slices/bridle/handlers/bridleClientWs.ack.spec.ts new file mode 100644 index 00000000..c6da1492 --- /dev/null +++ b/api/src/slices/bridle/handlers/bridleClientWs.ack.spec.ts @@ -0,0 +1,113 @@ +import type { Socket } from 'socket.io'; +import { BridleClientWsHandler } from './bridleClientWs.handler'; +import type { BridleAttachmentService, IBridleSendOptions } from '../domain'; + +/** + * What the browser is told about a message it sent (CLEAN-102). The value + * `handleMessage` returns is the socket.io acknowledgement: Nest passes it to + * the callback the browser supplied. Before this, a send was fire-and-forget + * and a bubble looked the same whether or not anyone received it. + */ +function makeHandler(expand: BridleAttachmentService['expand']) { + const forwarded: Array = []; + const hub = { + sendToAgent: ( + _clientId: string, + _agentId: string, + _text: string, + _parts: unknown[], + _attachments: unknown, + options?: IBridleSendOptions, + ) => { + forwarded.push(options); + return { status: 'accepted', messageId: 'from-hub', ts: 42 }; + }, + }; + const handler = new BridleClientWsHandler( + hub as never, + { expand } as BridleAttachmentService, + {} as never, + {} as never, + {} as never, + ); + const emitted: string[] = []; + const client = { + id: 'socket-1', + data: { clientId: 'admin', agentId: 'agent-1' }, + emit: (event: string) => { + emitted.push(event); + return true; + }, + disconnect: () => undefined, + } as unknown as Socket; + return { handler, client, forwarded, emitted }; +} + +const passThrough: BridleAttachmentService['expand'] = async (_a, text) => ({ + text, + parts: [], + attachments: [], +}); + +describe('BridleClientWsHandler — acknowledging a sent message', () => { + it('answers with what the hub decided', async () => { + const { handler, client } = makeHandler(passThrough); + + const ack = await handler.handleMessage(client, { + text: 'hello', + clientMessageId: 'c1', + }); + + expect(ack).toEqual({ status: 'accepted', messageId: 'from-hub', ts: 42 }); + }); + + it('asks the hub for an honest outcome only when the browser minted an id', async () => { + const { handler, client, forwarded } = makeHandler(passThrough); + + await handler.handleMessage(client, { + text: 'hello', + clientMessageId: 'c1', + }); + await handler.handleMessage(client, { text: 'from the embed widget' }); + + expect(forwarded[0]).toMatchObject({ + socketId: 'socket-1', + clientMessageId: 'c1', + withAck: true, + displayText: 'hello', + }); + expect(forwarded[1]?.withAck).toBeUndefined(); + expect(forwarded[1]?.clientMessageId).toBeUndefined(); + }); + + it('rejects a message with nothing in it', async () => { + const { handler, client, forwarded } = makeHandler(passThrough); + + const ack = await handler.handleMessage(client, { + text: '', + clientMessageId: 'c1', + }); + + expect(ack).toEqual({ status: 'rejected', code: 'EMPTY' }); + expect(forwarded).toHaveLength(0); + }); + + it('rejects when an attachment cannot be read, and still raises the notice', async () => { + const { handler, client, emitted } = makeHandler(async () => { + throw new Error('file is gone'); + }); + + const ack = await handler.handleMessage(client, { + text: 'look', + attachmentIds: ['a1'], + clientMessageId: 'c1', + }); + + expect(ack).toEqual({ + status: 'rejected', + code: 'ATTACHMENT_FAILED', + message: 'file is gone', + }); + expect(emitted).toEqual(['message_error']); + }); +}); diff --git a/api/src/slices/bridle/handlers/bridleClientWs.handler.spec.ts b/api/src/slices/bridle/handlers/bridleClientWs.handler.spec.ts index c8716f28..d63efb8a 100644 --- a/api/src/slices/bridle/handlers/bridleClientWs.handler.spec.ts +++ b/api/src/slices/bridle/handlers/bridleClientWs.handler.spec.ts @@ -51,6 +51,7 @@ function makeHandler( attachments?: IBridleAttachment[], ) => { sent.push({ clientId, agentId, text, parts, attachments }); + return { status: 'accepted', messageId: 'm1', ts: 1 }; }, }; const attachments = { expand } as BridleAttachmentService; @@ -213,6 +214,8 @@ function makeConnection(options: IConnectOptions) { registered.push({ clientId, agentId }); }, isAgentConnected: () => true, + currentSeq: () => 0, + replaySince: () => [], }; const shareLinks = { authorizeChat: options.authorizeChat ?? rejectEveryShare, @@ -477,7 +480,10 @@ describe('BridleClientWsHandler — share-link handshake', () => { share: { token: 'sl_good', visitorId: 'v7' }, }); expect(registered).toEqual([{ clientId: 'share-v7', agentId: 'agent-1' }]); - expect(emitted[0]).toEqual({ event: 'welcome', payload: { clientId: 'share-v7' } }); + expect(emitted[0]).toEqual({ + event: 'welcome', + payload: { clientId: 'share-v7', seq: 0 }, + }); }); it('rejects a dead link with the service code and never registers the socket', async () => { diff --git a/api/src/slices/bridle/handlers/bridleClientWs.handler.ts b/api/src/slices/bridle/handlers/bridleClientWs.handler.ts index adf02829..7c7d978e 100644 --- a/api/src/slices/bridle/handlers/bridleClientWs.handler.ts +++ b/api/src/slices/bridle/handlers/bridleClientWs.handler.ts @@ -18,6 +18,7 @@ import { IBridleGateway, BridleAttachmentService, type BridlePart, + type BridleSendAck, type ChatRequesterKinds, buildParts, clientIdFromJwtPayload, @@ -100,6 +101,7 @@ export class BridleClientWsHandler capabilities?: unknown; shareToken?: unknown; shareVisitor?: unknown; + lastSeq?: unknown; }; // Offered AT ALL, like `hasShareToken` on the HTTP side: an empty token // must reach `authorizeChat` and come back rejected, never slip past into @@ -289,6 +291,22 @@ export class BridleClientWsHandler client.emit(event, data); }; + // A reconnecting browser says how far it got (CLEAN-102). What it missed + // goes out before the socket joins the conversation — all in one tick, so + // nothing live can slip in between and arrive ahead of older events. + const lastSeq = + typeof auth.lastSeq === 'number' && Number.isFinite(auth.lastSeq) + ? auth.lastSeq + : 0; + client.emit('welcome', { + clientId, + seq: this.hub.currentSeq(clientId, agentId), + }); + if (lastSeq > 0) { + for (const missed of this.hub.replaySince(clientId, agentId, lastSeq)) { + send(missed); + } + } this.hub.registerClient( clientId, agentId, @@ -298,7 +316,6 @@ export class BridleClientWsHandler prompt, capabilities, ); - client.emit('welcome', { clientId }); // Tell the new client whether the agent runtime is currently online so the // chat header can render the right indicator color before any subsequent // register/unregister broadcasts. @@ -333,12 +350,26 @@ export class BridleClientWsHandler parts?: BridlePart[]; images?: Array<{ base64: string; mediaType: string }>; attachmentIds?: string[]; + clientMessageId?: string; }, - ) { + ): Promise { const clientId = client.data?.clientId as string; const agentId = client.data?.agentId as string; if (!clientId || !agentId) return; + // The return value is the socket.io acknowledgement (CLEAN-102): Nest hands + // it to the callback the browser passed, and drops it when there is none. + // A browser that mints a `clientMessageId` is one that renders the outcome + // under the person's message, so it is also the one that must not get the + // hub's synthetic "Agent is not connected" reply. The embed widget and + // older bundles send no id and keep today's behaviour byte for byte. + const clientMessageId = + typeof data.clientMessageId === 'string' && + data.clientMessageId.length > 0 && + data.clientMessageId.length <= 100 + ? data.clientMessageId + : undefined; + // A share link is re-validated per message, as the HTTP routes do: a // revoked link must stop a socket that is already open, not only the next // page load. The rejection goes out as `bridle_error` so the share page @@ -358,7 +389,7 @@ export class BridleClientWsHandler ); client.emit('bridle_error', { code, agentId }); client.disconnect(); - return; + return { status: 'rejected', code: 'SHARE_REJECTED', message: code }; } } @@ -392,18 +423,25 @@ export class BridleClientWsHandler `Attachment expansion failed: clientId=${clientId} agentId=${agentId}: ${message}`, ); client.emit('message_error', { message }); - return; + return { status: 'rejected', code: 'ATTACHMENT_FAILED', message }; } const parts = [...base, ...expanded.parts]; - if (!expanded.text && parts.length === 0) return; + if (!expanded.text && parts.length === 0) { + return { status: 'rejected', code: 'EMPTY' }; + } - this.hub.sendToAgent( + return this.hub.sendToAgent( clientId, agentId, expanded.text, parts, expanded.attachments, + { + socketId: client.id, + displayText: text, + ...(clientMessageId ? { clientMessageId, withAck: true } : {}), + }, ); } diff --git a/specs/015-chat-message-reliability/probe.mjs b/specs/015-chat-message-reliability/probe.mjs index 3df9db62..45d75bf6 100644 --- a/specs/015-chat-message-reliability/probe.mjs +++ b/specs/015-chat-message-reliability/probe.mjs @@ -1,17 +1,29 @@ -// Probe the local bridle hub the way the browser does. Prints no secrets. +// Probe the bridle hub the way a browser does (CLEAN-102). Prints no secrets. +// +// node specs/015-chat-message-reliability/probe.mjs +// +// normal one turn; shows the delivery ack and every event with its seq +// steal two sockets of one login, the FIRST one sends — who gets the answer? +// gap drop the socket mid-turn, reconnect with lastSeq — is the answer replayed? +// offline send while the agent is disconnected — rejection, or a fake agent reply? +// +// API_URL / AGENT_ID override the local defaults. Credentials come from +// RANCH_LOGIN / RANCH_PASS in .env.project at the repo root. import { createRequire } from 'node:module'; import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; -const REPO = 'C:/Users/maxim/orca/workspaces/ranch/chat-issues'; -const require = createRequire(REPO + '/package.json'); +const REPO = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); +const require = createRequire(resolve(REPO, 'package.json')); const { io } = require('socket.io-client'); -const API = 'http://localhost:3333'; -const AGENT = 'agent-bb620efe-abb5-4123-8ace-6d9b963387c7'; +const API = process.env.API_URL || 'http://localhost:3333'; +const AGENT = process.env.AGENT_ID || 'agent-bb620efe-abb5-4123-8ace-6d9b963387c7'; const MODE = process.argv[2] || 'normal'; const env = {}; -for (const line of readFileSync(REPO + '/.env.project', 'utf8').split(/\r?\n/)) { +for (const line of readFileSync(resolve(REPO, '.env.project'), 'utf8').split(/\r?\n/)) { const i = line.indexOf('='); if (i > 0) env[line.slice(0, i).trim()] = line.slice(i + 1).trim(); } @@ -20,6 +32,8 @@ const t0 = Date.now(); const rel = () => String(Date.now() - t0).padStart(6) + 'ms'; const log = (...a) => console.log(rel(), ...a); const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); +const short = (s) => (s ?? '').replace(/\s+/g, ' ').slice(0, 70); +const id8 = (s) => String(s).slice(0, 8); async function login() { const res = await fetch(API + '/auth/login', { @@ -28,117 +42,120 @@ async function login() { body: JSON.stringify({ email: env.RANCH_LOGIN, password: env.RANCH_PASS }), }); const json = await res.json().catch(() => ({})); - const data = json.data ?? json; - const token = data.accessToken ?? data.access_token ?? data.token; - log('login', res.status, 'keys:', Object.keys(data).join(','), 'token:', token ? 'yes' : 'NO'); - if (!token) throw new Error('no token'); + const token = (json.data ?? json).accessToken; + log('login', res.status, 'token:', token ? 'yes' : 'NO'); + if (!token) throw new Error('login failed'); return token; } -function connect(token) { +function connect(token, name, lastSeq = 0) { const socket = io(API + '/ws/client', { transports: ['websocket'], reconnection: false, - auth: { agentId: AGENT, capabilities: ['thinking'], token }, + auth: { agentId: AGENT, capabilities: ['thinking'], token, lastSeq }, }); - const seen = []; - const short = (s) => (s ?? '').replace(/\s+/g, ' ').slice(0, 70); - socket.on('connect', () => log('connect', socket.id)); - socket.on('disconnect', (r) => log('disconnect', r)); - socket.on('connect_error', (e) => log('connect_error', e.message)); - socket.on('welcome', (d) => log('welcome', JSON.stringify(d))); - socket.on('bridle_error', (d) => log('bridle_error', JSON.stringify(d))); - socket.on('message_error', (d) => log('message_error', JSON.stringify(d))); - socket.on('agent_status', (d) => log('agent_status', JSON.stringify(d))); - socket.on('typing', () => { seen.push('typing'); log('typing'); }); + const state = { socket, seen: [], lastSeq }; + const note = (type, seq) => { + state.seen.push(type); + if (typeof seq === 'number') state.lastSeq = Math.max(state.lastSeq, seq); + }; + const tag = `[${name}]`; + socket.on('connect', () => log(tag, 'connect')); + socket.on('disconnect', (r) => log(tag, 'disconnect', r)); + socket.on('connect_error', (e) => log(tag, 'connect_error', e.message)); + socket.on('welcome', (d) => log(tag, 'welcome', JSON.stringify(d))); + socket.on('bridle_error', (d) => log(tag, 'bridle_error', JSON.stringify(d))); + socket.on('message_error', (d) => log(tag, 'message_error', JSON.stringify(d))); + socket.on('agent_status', (d) => log(tag, 'agent_status connected=' + d.connected)); + socket.on('typing', (d) => { note('typing', d?.seq); log(tag, 'typing', 'seq=' + d?.seq); }); socket.on('thinking', (e) => { - seen.push('thinking'); - log('thinking', 'turn=' + String(e.turnId).slice(0, 8), e.done ? 'DONE' : 'step=' + (e.step?.label ?? e.step?.id ?? ''), 'skew=' + (e.ts - Date.now()) + 'ms'); + note('thinking', e.seq); + log(tag, 'thinking', e.done ? 'DONE' : 'step', 'seq=' + e.seq); }); - let streamChunks = 0; + let chunks = 0; socket.on('stream', (d) => { - streamChunks++; - if (streamChunks === 1 || streamChunks % 25 === 0) log('stream #' + streamChunks, 'id=' + String(d.messageId).slice(0, 8), 'len=' + (d.text ?? '').length); - seen.push('stream'); + note('stream', d.seq); + if (++chunks === 1) log(tag, 'stream (first chunk)', 'id=' + id8(d.messageId), 'seq=' + d.seq); }); socket.on('stream_end', (d) => { - seen.push('stream_end'); - log('stream_end', 'id=' + String(d.messageId).slice(0, 8), 'skew=' + ((d.ts ?? NaN) - Date.now()) + 'ms', 'seq=' + d.seq, '"' + short(d.text) + '"'); - streamChunks = 0; + note('stream_end', d.seq); + chunks = 0; + log(tag, 'stream_end', 'id=' + id8(d.messageId), 'seq=' + d.seq, 'skew=' + ((d.ts ?? NaN) - Date.now()) + 'ms', '"' + short(d.text) + '"'); }); socket.on('message', (d) => { - seen.push('message'); - log('message', 'id=' + String(d.messageId).slice(0, 8), 'skew=' + ((d.ts ?? NaN) - Date.now()) + 'ms', '"' + short(d.text) + '"'); + note('message', d.seq); + log(tag, 'message', 'id=' + id8(d.messageId), 'seq=' + d.seq, '"' + short(d.text) + '"'); }); - return { socket, seen }; + socket.on('user_message', (d) => { + note('user_message', d.seq); + log(tag, 'user_message', 'id=' + id8(d.messageId), 'seq=' + d.seq, '"' + short(d.text) + '"'); + }); + return state; } -async function transcriptTail(token, n) { - const res = await fetch(`${API}/api/agent/${AGENT}/transcript`, { - headers: { Authorization: 'Bearer ' + token }, +function send(state, text, clientMessageId = 'probe-' + Date.now()) { + return new Promise((done) => { + const timer = setTimeout(() => { log('ACK: none within 30s'); done(null); }, 30000); + state.socket.emit('message', { text, clientMessageId }, (ack) => { + clearTimeout(timer); + log('ACK', JSON.stringify(ack)); + done(ack); + }); }); - const json = await res.json().catch(() => ({})); - const data = json.data ?? json; - const msgs = data.messages ?? []; - log('transcript', res.status, 'count=' + msgs.length, 'hasMore=' + data.hasMore); - for (const m of msgs.slice(-n)) { - console.log(' ', m.role.padEnd(9), 'id=' + String(m.id).slice(0, 8), new Date(m.ts).toISOString().slice(11, 23), '"' + (m.text ?? '').replace(/\s+/g, ' ').slice(0, 90) + '"'); - } - return msgs; } +const summary = (s) => (s.seen.length ? [...new Set(s.seen)].join(',') : 'NOTHING'); const token = await login(); if (MODE === 'normal') { - const { socket, seen } = connect(token); + const a = connect(token, 'A'); await sleep(1500); - let acked = false; - const clientMessageId = 'probe-' + Date.now(); - log('SEND (with ack callback + clientMessageId=' + clientMessageId + ')'); - socket.emit('message', { text: 'Привет! Это тест чата. Ответь одним коротким предложением.', clientMessageId }, (a) => { - acked = true; - log('ACK', JSON.stringify(a)); - }); - await sleep(45000); - log('ack callback called:', acked, '| events:', [...new Set(seen)].join(',')); - socket.close(); - await sleep(2000); - await transcriptTail(token, 6); + log('SEND'); + await send(a, 'Привет! Это тест чата. Ответь одним коротким предложением.'); + await sleep(15000); + log('received:', summary(a)); + a.socket.close(); +} + +if (MODE === 'steal') { + const a = connect(token, 'A'); + await sleep(1200); + const b = connect(token, 'B'); + await sleep(1200); + log('SEND from A — the socket that connected FIRST'); + await send(a, 'Тест двух вкладок. Ответь одним словом: ок.'); + await sleep(15000); + log('A received:', summary(a)); + log('B received:', summary(b)); + a.socket.close(); + b.socket.close(); } if (MODE === 'gap') { - const a = connect(token); + const a = connect(token, 'A'); await sleep(1500); log('SEND long question'); - a.socket.emit('message', { text: 'Тест обрыва связи. Перечисли пять фактов о лошадях, по одному предложению на факт.' }); - // Drop the socket as soon as the agent shows any sign of life. - while (!a.seen.length && Date.now() - t0 < 30000) await sleep(100); - log('>>> dropping socket mid-turn (events so far: ' + a.seen.join(',') + ')'); + void send(a, 'Тест обрыва связи. Перечисли пять фактов о лошадях, по одному предложению на факт.'); + while (!a.seen.includes('typing') && Date.now() - t0 < 30000) await sleep(50); + log('>>> dropping the socket mid-turn; lastSeq=' + a.lastSeq); a.socket.close(); await sleep(25000); - log('>>> reconnecting'); - const b = connect(token); - await sleep(25000); - log('events after reconnect:', b.seen.length ? [...new Set(b.seen)].join(',') : 'NONE'); + log('>>> reconnecting with lastSeq=' + a.lastSeq); + const b = connect(token, 'A2', a.lastSeq); + await sleep(8000); + log('after reconnect received:', summary(b)); b.socket.close(); - await sleep(1500); - await transcriptTail(token, 4); } -if (MODE === 'steal') { - // Two "tabs" of the same admin user. A connects first and sends; B connects second. - const a = connect(token); - await sleep(1200); - const b = connect(token); - await sleep(1200); - log('SEND from tab A (the one that connected FIRST)'); - a.socket.emit('message', { text: 'Тест двух вкладок. Ответь одним словом: ок.' }); - await sleep(20000); - log('tab A received:', a.seen.length ? [...new Set(a.seen)].join(',') : 'NOTHING'); - log('tab B received:', b.seen.length ? [...new Set(b.seen)].join(',') : 'NOTHING'); +if (MODE === 'offline') { + const a = connect(token, 'A'); + await sleep(1500); + log('SEND (expecting the agent to be disconnected)'); + await send(a, 'Это сообщение не должно потеряться молча.'); + await sleep(3000); + log('received:', summary(a), '— a "message" here means the hub faked an agent reply'); a.socket.close(); - b.socket.close(); - await sleep(1000); } +await sleep(500); process.exit(0); From d38e851e0ec5e4825d770a7949b8391d30593932 Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Fri, 18 Sep 2026 20:12:35 +0300 Subject: [PATCH 03/10] fix(api): replay a streamed turn as the bubbles it was shown as (CLEAN-102) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime stores one assistant event per turn; its text is every bubble of the turn concatenated, so a reload showed "Проверю:Ха! Работает!" in a single bubble. When the event carries the bubbles (`data.messages`, written by the runtime from CLEAN-102 on), the reader replays those under their wire ids. Older events and any malformed `messages` array fall back to the whole text. Co-Authored-By: Claude Fable 5.1 --- .../domain/transcriptReader.bubbles.spec.ts | 107 ++++++++++++++++++ .../file/domain/transcriptReader.service.ts | 42 +++++++ 2 files changed, 149 insertions(+) create mode 100644 api/src/slices/agent/file/domain/transcriptReader.bubbles.spec.ts diff --git a/api/src/slices/agent/file/domain/transcriptReader.bubbles.spec.ts b/api/src/slices/agent/file/domain/transcriptReader.bubbles.spec.ts new file mode 100644 index 00000000..c432b50e --- /dev/null +++ b/api/src/slices/agent/file/domain/transcriptReader.bubbles.spec.ts @@ -0,0 +1,107 @@ +import { TranscriptReaderService } from './transcriptReader.service'; +import type { IFileGateway } from './file.gateway'; +import type { IFileChunk } from './file.types'; + +const PATH = 'data/sessions/bridle:admin.jsonl'; + +function fakeFiles(content: string): IFileGateway { + return { + readRange: async (): Promise => ({ + path: PATH, + content, + size: Buffer.byteLength(content), + totalSize: Buffer.byteLength(content), + offset: 0, + nextOffset: null, + hasMore: false, + updatedAt: new Date(0), + }), + } as unknown as IFileGateway; +} + +const read = (events: Array>) => + new TranscriptReaderService( + fakeFiles(events.map((e) => JSON.stringify(e)).join('\n')), + ).read('agent-1', PATH); + +/** + * A streamed turn is watched as several bubbles and stored as one event. The + * replay used to show that event's whole text — "Проверю:Ха! Работает!" — in + * a single bubble (CLEAN-102). The runtime now records the bubbles on the + * event; the reader plays those back. + */ +describe('TranscriptReaderService — bubbles of one turn', () => { + const question = { + id: 'q1', + type: 'user', + ts: 1000, + data: { text: 'работает?' }, + }; + + it('replays the bubbles the person saw, under their wire ids', async () => { + const messages = await read([ + question, + { + id: 'turn-1', + type: 'assistant', + ts: 5000, + data: { + text: 'Проверю:Ха! Работает!', + messages: [ + { id: 'wire-1', text: 'Проверю:', ts: 2000 }, + { id: 'wire-2', text: 'Ха! Работает!', ts: 4000 }, + ], + }, + }, + ]); + + expect(messages.map((m) => [m.id, m.role, m.text])).toEqual([ + ['q1', 'user', 'работает?'], + ['wire-1', 'assistant', 'Проверю:'], + ['wire-2', 'assistant', 'Ха! Работает!'], + ]); + }); + + it('keeps showing turns stored before bubbles were recorded', async () => { + const messages = await read([ + question, + { id: 'turn-1', type: 'assistant', ts: 5000, data: { text: 'Да.' } }, + ]); + + expect(messages.map((m) => [m.id, m.text])).toEqual([ + ['q1', 'работает?'], + ['turn-1', 'Да.'], + ]); + }); + + it('falls back to the whole text rather than replay only some of it', async () => { + const messages = await read([ + { + id: 'turn-1', + type: 'assistant', + ts: 5000, + data: { + text: 'Проверю:Ха! Работает!', + messages: [ + { id: 'wire-1', text: 'Проверю:', ts: 2000 }, + { id: 'wire-2', text: 'Ха! Работает!' }, + ], + }, + }, + ]); + + expect(messages.map((m) => [m.id, m.text])).toEqual([ + ['turn-1', 'Проверю:Ха! Работает!'], + ]); + }); + + it('keeps the order of the file for messages written in the same millisecond', async () => { + const messages = await read([ + { id: 'a', type: 'user', ts: 1000, data: { text: 'first' } }, + { id: 'b', type: 'assistant', ts: 1000, data: { text: 'second' } }, + { id: 'c', type: 'user', ts: 1000, data: { text: 'third' } }, + ]); + + expect(messages.map((m) => m.id)).toEqual(['a', 'b', 'c']); + }); +}); diff --git a/api/src/slices/agent/file/domain/transcriptReader.service.ts b/api/src/slices/agent/file/domain/transcriptReader.service.ts index fc4f7610..b03eb7bf 100644 --- a/api/src/slices/agent/file/domain/transcriptReader.service.ts +++ b/api/src/slices/agent/file/domain/transcriptReader.service.ts @@ -75,11 +75,41 @@ interface RawEvent { params?: unknown; result?: unknown; attachments?: unknown; + /** `assistant` only: the bubbles the turn was shown as (runtime ≥ CLEAN-102). */ + messages?: unknown; }; } const ATTACHMENT_KINDS = new Set(['image', 'text', 'binary']); +/** + * Same stance as {@link sanitizeAttachments}: runtime-authored, still external + * input. All or nothing — one malformed entry and the caller falls back to + * the event's whole text, because replaying only the valid bubbles would + * silently drop part of what the agent said. + */ +function sanitizeBubbles( + raw: unknown, +): Array> { + if (!Array.isArray(raw)) return []; + const out: Array> = []; + for (const entry of raw) { + const b = entry as Record; + if ( + typeof b?.id !== 'string' || + !b.id || + typeof b?.text !== 'string' || + !b.text.trim() || + typeof b?.ts !== 'number' || + !Number.isFinite(b.ts) + ) { + return []; + } + out.push({ id: b.id, text: b.text, ts: b.ts }); + } + return out; +} + /** * The JSONL is runtime-authored but still external input to this API — * filter each entry down to the exact replayable shape and drop anything @@ -159,6 +189,18 @@ export class TranscriptReaderService { // split result proves the turn happened even when the runtime stored // no attachment metadata (legacy records). if (rendered === null && !attachments.length) return; + // A streamed turn reached the person as several bubbles but is stored + // as one event (its `text` is what the model sees). When the runtime + // recorded the bubbles, replay THOSE — same ids, same boundaries — + // instead of one paragraph with the sentences glued together (CLEAN-102). + const bubbles = + evt.type === 'assistant' ? sanitizeBubbles(evt.data?.messages) : []; + if (bubbles.length) { + for (const bubble of bubbles) { + messages.push({ ...bubble, role: 'assistant' }); + } + return; + } messages.push({ id: evt.id, role: evt.type as TranscriptMessage['role'], From 569b5578c308aceb96888b0878ae35ef027ebaf8 Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Fri, 18 Sep 2026 20:13:40 +0300 Subject: [PATCH 04/10] fix(api): push every agent status change to the status stream (CLEAN-102) The drift sweep moves agents to `failed` / `unreachable` with no pod or hub event behind the change (the 5-minute startup timeout), and GET /agents/:id reconciles on read. Neither reached `/agents/status/stream`, so an open agents list kept showing `deploying` until somebody opened that agent. Every status write in AgentStatusService now goes through one method that also emits a `modified` frame; the controller's sync-on-read notifies the same way. docs/state.md records the client-side rule this belongs to: one record per entity in its store, fetches upsert, pushes patch, components render by id. Co-Authored-By: Claude Fable 5.1 --- .../slices/agent/agent/agent.controller.ts | 3 + .../agent/domain/agentStatus.service.spec.ts | 64 ++++++++++++- .../agent/agent/domain/agentStatus.service.ts | 74 ++++++++++++--- docs/state.md | 92 +++++++++++++++++++ 4 files changed, 221 insertions(+), 12 deletions(-) create mode 100644 docs/state.md diff --git a/api/src/slices/agent/agent/agent.controller.ts b/api/src/slices/agent/agent/agent.controller.ts index 47df8fc0..f61444bd 100644 --- a/api/src/slices/agent/agent/agent.controller.ts +++ b/api/src/slices/agent/agent/agent.controller.ts @@ -118,6 +118,9 @@ export class AgentController { agent.workflowId, `deploy workflow ${phase.toLowerCase()}`, ); + // Only the caller of this GET would learn about the flip otherwise — + // push it to every open status stream (lists, other operators). + this.agentStatusService.notifyStatusChanged(agentId); return this.agentGateway.findById(agentId); } } catch { diff --git a/api/src/slices/agent/agent/domain/agentStatus.service.spec.ts b/api/src/slices/agent/agent/domain/agentStatus.service.spec.ts index 5c34113a..91ddeb05 100644 --- a/api/src/slices/agent/agent/domain/agentStatus.service.spec.ts +++ b/api/src/slices/agent/agent/domain/agentStatus.service.spec.ts @@ -1,5 +1,8 @@ import { Subject } from 'rxjs'; -import { AgentStatusService } from './agentStatus.service'; +import { + AgentStatusService, + AgentStatusStreamMessage, +} from './agentStatus.service'; import { IAgentData } from './agent.types'; import { IAgentPodStatus } from '#/agent/pod/domain/pod.types'; @@ -217,6 +220,65 @@ describe('AgentStatusService bridle connectivity', () => { ); }); + // The sweep's DB-only transitions have no pod or hub event behind them. If + // the stream stays silent, a list keeps showing 'deploying' for an agent + // nobody has open while its detail view (which polls) already says 'failed'. + test('startup timeout on an agent nobody has open reaches the status stream', async () => { + const agents = [ + makeAgent({ id: 'a1' }), + makeAgent({ id: 'a2', name: 'Stuck', status: 'deploying' }), + ]; + const bed = createTestBed(agents, [makePod({ agentId: 'a1' })]); + bed.bridleGateway.isAgentConnected.mockImplementation( + (id: string) => id === 'a1', + ); + bed.agentGateway.updateStatus.mockImplementation( + (id: string, status: IAgentData['status'], _wf, reason?: string) => { + const row = agents.find((a) => a.id === id); + if (row) Object.assign(row, { status, statusReason: reason ?? null }); + return Promise.resolve(row); + }, + ); + + const frames: AgentStatusStreamMessage[] = []; + const sub = bed.service.stream$().subscribe((msg) => frames.push(msg)); + await sweep(bed.service); + await new Promise((resolve) => setImmediate(resolve)); + sub.unsubscribe(); + + const events = frames.filter((f) => f.type === 'event'); + expect(events).toHaveLength(1); + expect(events[0].payload).toMatchObject({ + eventType: 'modified', + status: { + agent: { + id: 'a2', + status: 'failed', + statusReason: + 'startup did not produce a running agent within 5 minutes', + }, + pod: null, + bridleConnected: false, + }, + }); + }); + + test('notifyStatusChanged pushes the current row for writes made elsewhere', async () => { + const bed = createTestBed([makeAgent({ status: 'failed' })], []); + + const frames: AgentStatusStreamMessage[] = []; + const sub = bed.service.stream$().subscribe((msg) => frames.push(msg)); + bed.service.notifyStatusChanged('a1'); + await new Promise((resolve) => setImmediate(resolve)); + sub.unsubscribe(); + + const events = frames.filter((f) => f.type === 'event'); + expect(events).toHaveLength(1); + expect(events[0].payload).toMatchObject({ + status: { agent: { id: 'a1', status: 'failed' } }, + }); + }); + test('snapshot carries live bridleConnected per agent', async () => { const bed = createTestBed( [makeAgent({ id: 'a1' }), makeAgent({ id: 'a2', name: 'Second' })], diff --git a/api/src/slices/agent/agent/domain/agentStatus.service.ts b/api/src/slices/agent/agent/domain/agentStatus.service.ts index 95f3ae86..6a86fb8c 100644 --- a/api/src/slices/agent/agent/domain/agentStatus.service.ts +++ b/api/src/slices/agent/agent/domain/agentStatus.service.ts @@ -8,6 +8,7 @@ import { } from '@nestjs/common'; import { Observable, + Subject, Subscription, defer, filter, @@ -89,6 +90,11 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { // First-observed timestamps of "running pod, no hub connection" per agent. // In-memory on purpose: resets on API restart, which IS the restart grace. private readonly bridleDownSince = new Map(); + // Ids of agents whose DB status was just written. The drift sweep changes + // rows with no pod or hub event behind it (startup timeout → 'failed', + // 'unreachable'), so without this the SSE stream stayed silent and every + // list kept showing 'deploying' until someone opened that agent. + private readonly statusWrites$ = new Subject(); constructor( private readonly agentGateway: IAgentGateway, @@ -193,11 +199,7 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { this.logger.log( `Reconciling agent ${agentId}: bridle runtime registered — marking running`, ); - await this.agentGateway.updateStatus( - agentId, - 'running', - agent.workflowId ?? undefined, - ); + await this.writeStatus(agentId, 'running', agent.workflowId ?? undefined); // Sync-conflict marker (CLEAN-50): a not-yet-running agent that just // registered on bridle is a fresh boot, and the runtime pulls its S3 // working copy at boot, right before this connect. Deliberately NOT set @@ -208,6 +210,21 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { this.deployTracker.clear(agentId); } + // Every status write in this service goes through here so SSE consumers + // hear about it — whatever triggered the write. + private async writeStatus( + ...args: Parameters + ): Promise { + await this.agentGateway.updateStatus(...args); + this.statusWrites$.next(args[0]); + } + + // For status writes made outside this service (GET /agents/:id syncStatus): + // pushes the agent's current row to every open status stream. + notifyStatusChanged(agentId: string): void { + this.statusWrites$.next(agentId); + } + async snapshot(): Promise { const [agents, pods] = await Promise.all([ this.agentGateway.findAll(), @@ -309,7 +326,42 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { filter((msg): msg is AgentStatusStreamMessage => msg !== null), ); - return merge(initial$, updates$, bridleUpdates$); + // DB-only transitions (see statusWrites$) — no pod or hub event carries + // them, so they get a frame of their own. + const statusUpdates$ = this.statusWrites$.pipe( + mergeMap((agentId) => + from( + Promise.all([ + // A failed lookup must cost this one frame, not the whole stream: + // an error here would end the SSE connection of every listener. + this.agentGateway.findById(agentId).catch(() => null), + Promise.resolve() + .then(() => this.podGateway.list()) + .catch(() => []), + ]), + ).pipe( + map(([agent, pods]): AgentStatusStreamMessage | null => + agent + ? { + type: 'event', + payload: { + eventType: 'modified', + status: { + agent, + pod: pods.find((p) => p.agentId === agentId) ?? null, + bridleConnected: + this.bridleGateway.isAgentConnected(agentId), + }, + }, + } + : null, + ), + ), + ), + filter((msg): msg is AgentStatusStreamMessage => msg !== null), + ); + + return merge(initial$, updates$, bridleUpdates$, statusUpdates$); } private async detectDrift(reason: 'startup' | 'periodic'): Promise { @@ -352,7 +404,7 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { this.logger.log( `Drift: agent ${agent.id} (${agent.name}) is ${agent.status} in DB but bridle has it registered — marking running`, ); - await this.agentGateway.updateStatus( + await this.writeStatus( agent.id, 'running', agent.workflowId ?? undefined, @@ -393,7 +445,7 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { this.logger.warn( `Drift: agent ${agent.id} (${agent.name}) is ${agent.status} in DB but no pod exists — marking failed (${reason})`, ); - await this.agentGateway.updateStatus( + await this.writeStatus( agent.id, 'failed', agent.workflowId ?? undefined, @@ -434,7 +486,7 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { this.logger.warn( `Drift: agent ${agent.id} (${agent.name}) pod is Running+Ready but the runtime never registered on the bridle hub — marking unreachable`, ); - await this.agentGateway.updateStatus( + await this.writeStatus( agent.id, 'unreachable', agent.workflowId ?? undefined, @@ -543,7 +595,7 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { : '') + ` — marking failed`, ); - await this.agentGateway.updateStatus( + await this.writeStatus( agent.id, 'failed', agent.workflowId ?? undefined, @@ -575,7 +627,7 @@ export class AgentStatusService implements OnModuleInit, OnModuleDestroy { this.logger.log( `Reconciling agent ${agent.id}: pod ${podStatus.podName} is Running+Ready — marking running`, ); - await this.agentGateway.updateStatus( + await this.writeStatus( agent.id, 'running', agent.workflowId ?? undefined, diff --git a/docs/state.md b/docs/state.md new file mode 100644 index 00000000..9430b88e --- /dev/null +++ b/docs/state.md @@ -0,0 +1,92 @@ +# Client state: one record per entity + +Applies to `admin/` and `app/`. + +## The one rule that explains the rest + +**An entity lives once, in its Pinia store. Everything on screen is a view of +that record.** + +The defect this rule exists for: the agents screen showed one agent as +"Deploying" in the left list and "Failed" in its own header, on the same page. +Nothing was wrong with either request. The list rendered the array one +`useAsyncData` call returned, the header rendered the object another one +returned, a status stream fed a third copy, and an optimistic flip edited a +fourth. Four copies of one agent, each free to move on without the others. +Refreshing one of them more often does not fix that; having one does. + +## The rules + +1. **Fetches upsert.** A list fetch replaces the store's collection; any call + that returns a single entity (`fetchById`, create, update, restart, …) + upserts it by id before returning. No store action hands back an entity it + did not also store. +2. **Pushes patch the same record.** A stream, socket or poll writes into the + store's record. It never keeps a map of its own "fresher" copies for + components to prefer — that is a second source of truth with a nicer name. +3. **Components render by id.** `computed(() => store.byId(id))`, or the + store's collection through `storeToRefs`. Never the value a fetch returned. +4. **`useAsyncData` is for loading state, not a render source.** Keep it for + `pending` / `error` / `refresh`; ignore its `data`. +5. **Optimistic changes go through a store action with rollback.** + `const rollback = store.patch(id, { … })`, and `rollback()` when the call + fails. Never `localRef.value = { ...localRef.value, … }` in a component or + composable. +6. **One fact, one derivation.** If a label is computed from the record (a + display status, a tone), compute it the same way everywhere or in one shared + place. A row that overlays pod state on the status and a header that does + not will disagree even when they read the same record. + +## Worked example: the agent store + +`admin/slices/agent/agent/stores/agent.ts` (the app's +`app/slices/agent/stores/agent.ts` has the same three functions): + +```ts +byId(id): IAgentData | undefined // lookup; reactive inside a computed +upsert(agent): IAgentData // full record in, replaces or appends +patch(id, partial): () => void // partial change, returns a rollback +``` + +- `fetchAll` replaces `agents`; `fetchById`, `fetchAdmin`, `create`, `update`, + `restart`, `stop`, `start`, `demoteAdmin` upsert. +- `restart` / `stop` / `start` flip the status with `patch` before the request + and roll back if it throws — callers (`useAgentLifecycle`, the app's chat + header) do not flip anything themselves. The rollback restores a field only + if it still holds the optimistic value, so a fresher server write that + landed in between survives. +- The status stream (`stores/agentStatus.ts`) upserts the agent row each frame + carries into this store and keeps only what exists nowhere else: pod state + and hub connectivity. Its frames carry the full `AgentDto` and are decoded + by the same `AgentMapper` as REST responses, so a frame can never thin out a + record. +- The rail (`workspace/Provider.vue`), the header (`workspace/Main.vue`), the + Overview card, the edit page and the Files tab all read `agentStore.agents` + / `agentStore.byId(id)`. + +In code: + +```ts +// loading state from the request, content from the store +const { pending, refresh } = useAsyncData(`admin-agent-${props.id}`, () => + agentStore.fetchById(props.id), +); +const agent = computed(() => agentStore.byId(props.id)); +``` + +A projection for a different audience is a different collection, not a second +copy: the app's `publicAgents` (landing-page cards from `GET /agents/public`) +stays separate from `agents` on purpose. + +The chat conversation record in the bridle stores follows the same rule: one +conversation per key in the store, written to by the socket and rendered by +key. + +## Review checklist + +- Does the template read `data` from `useAsyncData` / `useFetch`? Render from + the store instead. +- Does a store action return an entity without storing it? +- Is there a `live…` / `fresh…` / `current…` ref next to a store that already + holds that entity? +- Does anything assign to a local copy of an entity to "update" it? From 7ed09081c8f1a068c680be5c8cf8739a22c5eb9d Mon Sep 17 00:00:00 2001 From: "Maksym Hryzodub [DREAM]" Date: Fri, 18 Sep 2026 20:13:41 +0300 Subject: [PATCH 05/10] fix(app): one agent record per id in the agent store (CLEAN-102) The agents list, the agent page and the chat page each held their own copy of an agent, so a status could differ between them. The store is now the single source: fetchById/create/update/restart upsert into `agents`, components render `byId(id)`, and the optimistic restart flip goes through `patch()` with a rollback. A failed poll no longer wipes the record the open page renders. Co-Authored-By: Claude Fable 5.1 --- .../agent/components/agent/Provider.vue | 5 +- .../agent/components/agent/chat/Provider.vue | 13 +-- .../components/agent/workspace/Provider.vue | 6 +- app/slices/agent/pages/agents/index.vue | 11 +-- app/slices/agent/stores/agent.ts | 83 +++++++++++++++---- 5 files changed, 89 insertions(+), 29 deletions(-) diff --git a/app/slices/agent/components/agent/Provider.vue b/app/slices/agent/components/agent/Provider.vue index 771bb236..b788aa79 100644 --- a/app/slices/agent/components/agent/Provider.vue +++ b/app/slices/agent/components/agent/Provider.vue @@ -10,8 +10,11 @@ const route = useRoute(); const agentStore = useAgentStore(); -const { data: agent, pending, error } = await useAsyncData( +// The request is for `pending` / `error`; what renders is the store's record +// (docs/state.md), which `fetchById` upserts into. +const { pending, error } = await useAsyncData( `agent-${route.params.id}`, () => agentStore.fetchById(route.params.id as string), ); +const agent = computed(() => agentStore.byId(route.params.id as string)); diff --git a/app/slices/agent/components/agent/chat/Provider.vue b/app/slices/agent/components/agent/chat/Provider.vue index 65748b49..677f5348 100644 --- a/app/slices/agent/components/agent/chat/Provider.vue +++ b/app/slices/agent/components/agent/chat/Provider.vue @@ -4,10 +4,14 @@ const props = defineProps<{ id: string }>(); const agentStore = useAgentStore(); const authStore = useAuthStore(); -const { data: agent, pending, error, refresh } = await useAsyncData( +// The request is for `pending` / `error` / `refresh`; what renders is the +// store's record (docs/state.md) — the same object the rail row shows, so the +// header pill and the row cannot disagree. `fetchById` upserts into it. +const { pending, error, refresh } = await useAsyncData( `agent-${props.id}`, () => agentStore.fetchById(props.id), ); +const agent = computed(() => agentStore.byId(props.id)); const canManage = computed(() => authStore.hasRole(UserRoleTypes.Owner, UserRoleTypes.Admin), @@ -32,14 +36,13 @@ async function onRestart() { restartError.value = null; restartFailed.value = false; restartStartedAt.value = Date.now(); - const previous = agent.value.status; - // Optimistic flip — overlay appears immediately, status pill animates. - agent.value = { ...agent.value, status: 'deploying' }; + // The optimistic 'deploying' flip — overlay appears immediately, status + // pill animates — and its rollback happen inside `agentStore.restart`, on + // the one store record, so the rail row flips with this header. try { await agentStore.restart(agent.value.id); await refresh(); } catch (err) { - if (agent.value) agent.value = { ...agent.value, status: previous }; restartFailed.value = true; // A 401 the api plugin could not recover from is the session-ended // dialog's story to tell; the banner falls back to `chat.restart_failed` diff --git a/app/slices/agent/components/agent/workspace/Provider.vue b/app/slices/agent/components/agent/workspace/Provider.vue index 8b9ba211..facaa0f4 100644 --- a/app/slices/agent/components/agent/workspace/Provider.vue +++ b/app/slices/agent/components/agent/workspace/Provider.vue @@ -11,9 +11,13 @@ const canCreate = computed(() => // One list request for the whole workspace, shared with the resolver page by // its key so landing here does not re-fetch what it already read. -const { data: agents, pending, refresh } = await useAsyncData('agents', () => +// +// The request gives `pending` and `refresh`; the rail renders the store's +// collection (docs/state.md), the same records the open chat reads by id. +const { pending, refresh } = await useAsyncData('agents', () => agentStore.fetchAll(), ); +const { agents } = storeToRefs(agentStore); // The rail shows every agent's runtime state, so it has to keep up with // agents other people start and stop. The app console has no status stream diff --git a/app/slices/agent/pages/agents/index.vue b/app/slices/agent/pages/agents/index.vue index 9a937cbe..57f2cd70 100644 --- a/app/slices/agent/pages/agents/index.vue +++ b/app/slices/agent/pages/agents/index.vue @@ -11,14 +11,15 @@ const canCreate = computed(() => authStore.hasRole(UserRoleTypes.Owner, UserRoleTypes.Admin), ); -const { data: agents, pending } = await useAsyncData('agents', () => - agentStore.fetchAll(), -); +// Awaited for its loading state; the list itself is read from the store like +// everywhere else (docs/state.md). +const { pending } = await useAsyncData('agents', () => agentStore.fetchAll()); +const { agents } = storeToRefs(agentStore); // Remembered agent → first running → first in the list. The remembered id is // only honoured while it is still in this user's visible list, so a deleted // or newly-hidden agent is a non-event rather than a dead landing. -const landing = computed(() => resolveLanding(agents.value ?? [])); +const landing = computed(() => resolveLanding(agents.value)); watchEffect(() => { if (landing.value) { @@ -28,7 +29,7 @@ watchEffect(() => {