diff --git a/docs/memory/architecture.md b/docs/memory/architecture.md index 6a081d8f2..01d14e364 100644 --- a/docs/memory/architecture.md +++ b/docs/memory/architecture.md @@ -650,7 +650,13 @@ directly). Agents call the MCP `report` tool, which POSTs to `POST /api/agents/{ (`offset`/`limit`, true `total`); the UI fetches tabular reports through it, so expanding a 1.2 MB report transfers ~8 KB. Storage stays a single TEXT blob — no migration — and the slice is Python-side, so it bounds the response, not the read; off-row storage waits - on a payload distribution that justifies it. + on a payload distribution that justifies it. The **portal** detail route carries the same + window as two optional query params (`rows_offset`/`rows_limit`, #2162) rather than a second + route: `/rows` is `Depends(get_current_user)`, which a portal principal cannot satisfy, and + a clone on a client-facing prefix would need its own copy of the uniform-404 contract. There + the *server* decides tabularity from the real payload (a non-tabular one returns whole with + no `row_meta`, never a 400), and because paging re-reads the blob per request it is + rate-limited per (client, agent). - **Export** (#1536): `GET /api/reports/{id}/export?format=xlsx|pdf` → `services/report_export.py` (pure builders, lazily-imported `openpyxl`/`reportlab`, both pure-Python wheels). Shape mismatch degrades to a sensible sheet or JSON rather than @@ -661,7 +667,24 @@ directly). Agents call the MCP `report` tool, which POSTs to `POST /api/agents/{ `GET /api/reports/{id}` returns the full payload, lazy-loaded when a card expands. - **Fleet access**: `GET /api/reports` + `GET /api/reports/stats` filter via `accessible_agent_names` + `_narrow_to_agent` (admin = all). Renderers (`components/reports/`) - pick by `display_hint` → `report_type` prefix → JSON, with shape-validation fallback to JSON. + pick by `display_hint` → `report_type` prefix → fallback, with a shape check per hint. +- **Three renderer surfaces, a per-surface fallback** (#2162): Agent Detail, the Operations fleet tab, + and the **Workspace agent page** all mount the same `ReportRenderer`. The third was added + after it shipped `JSON.stringify(payload)` to external clients — a disclosure defect + (`payload` is free-form agent JSON of the class `client_portal/agent_page.py` refuses to + expose for an ask's `context`, canary G-04), which a typed renderer narrows because it + reads only the keys its hint declares. This matters to the CI pin above: `test_1535` + regexes `payload.X` out of `ReportRenderer.vue`, so a third consumer widens that drift + guard's blast radius and **`shapeOk` must stay in that file** — extracting it is the + natural refactor and it empties the pinned set. The fallback is **per-surface**: the default + stays `ReportJson`, so both operator surfaces render exactly what they always did, and only the + Workspace passes `:fallback-component="ReportSummary"` (bounded, humanised, credential-shaped + tokens redacted at value level, no raw payload reachable behind it). AC #2 asks for a client + fallback "deliberately stricter than the operator side", so the split IS the design — a global + summary would erase it, and a raw dump is a FEATURE when you are debugging an agent's own + output. The override deliberately catches an agent-chosen `display_hint: "json"` as well as a + shape mismatch, since `json` is a valid enum value and replacing only the mismatch path would + leave an agent able to request a dump in front of a client. - **Agent read-back** (#1538): `list_reports` / `get_report` MCP tools over the existing access-controlled REST endpoints — no new endpoint, no new tenant-boundary logic. The MCP layer adds the narrowing the backend cannot do (agent key → owner scope → `{self} ∪ diff --git a/docs/memory/feature-flows/agent-reports.md b/docs/memory/feature-flows/agent-reports.md index 9d4b42b2a..ca6b01864 100644 --- a/docs/memory/feature-flows/agent-reports.md +++ b/docs/memory/feature-flows/agent-reports.md @@ -87,12 +87,19 @@ calling agent. `setActive` gate so a WS trigger only refetches while the panel is mounted). Wired into `utils/websocket.js` `agent_report` dispatch. - **Renderers** `components/reports/` — `ReportRenderer.vue` picks by `display_hint` → - `report_type` prefix → JSON, validating payload shape and falling back to `ReportJson` on - mismatch (Codex #10). Typed renderers: `ReportTable`, `ReportKpiTiles`, `ReportMarkdown` - (DOMPurify via `utils/markdown.js`), `ReportTimeline`, `ReportJson`. -- **Panels** — `ReportsPanel.vue` (Agent Detail "Reports" tab) and `ReportsPanelFleet.vue` - (Operations → Reports tab; agent/type/time/search filters + KPI tiles from - `GET /api/reports/stats`). Lists show metadata; full payload lazy-loads on expand. + `report_type` prefix → fallback, validating payload shape and falling back on mismatch + (Codex #10). Typed renderers: `ReportTable`, `ReportKpiTiles`, `ReportMarkdown` + (DOMPurify via `utils/markdown.js`), `ReportTimeline`. The fallback is a **prop**, not a + fixture (#2162): `fallbackComponent` defaults to `ReportJson`, which is what both operator + panels still render, and only the client-facing Workspace passes `ReportSummary` — there is + no disclosure behind it, because a raw payload must be unreachable on that surface. +- **Panels — THREE consumers, not two** (#2162) — `ReportsPanel.vue` (Agent Detail "Reports" + tab), `ReportsPanelFleet.vue` (Operations → Reports tab; agent/type/time/search filters + + KPI tiles from `GET /api/reports/stats`), and the **Workspace agent page's Reports tab** + (`components/portal/PortalAgentPage.vue`, client-facing — see + [workspace-agent-page.md](workspace-agent-page.md)). Lists show metadata; full payload + lazy-loads on expand. Enumerating them is load-bearing: this list said "two" while the + third shipped a raw JSON dump to external clients. ### Renderer payload contracts | hint | expected payload shape | @@ -101,7 +108,18 @@ calling agent. | `kpi` | `{ tiles: Array<{label, value, unit?}> }` | | `markdown` | `{ markdown: string }` | | `timeline` | `{ events: Array<{ts?, label, detail?}> }` | -| `json` (or anything malformed) | rendered as a pretty-printed JSON viewer | +| `json` (or anything malformed) | `ReportJson` by default; the overriding surface's `fallbackComponent` (#2162) | + +**The fallback is per-surface, since #2162.** `ReportJson` — a pretty-printed dump — stays the +default, and both operator panels keep it: it is the useful answer when you are debugging an +agent's own output. It is unacceptable for an external client, so `ReportRenderer` takes a +`fallbackComponent` prop and the Workspace passes `ReportSummary`, a bounded summary (≤40 entries +with a counted remainder, ~200-char values, depth 1 — a nested value is described as "12 items", +never serialised) with credential-shaped tokens redacted at **value** level, mirroring canary +G-04's prefix set, and no raw payload behind it. AC #2 asks for a client fallback "deliberately +stricter than the operator side", so the split is the design. The override covers an +agent-chosen `json` hint as well as a shape mismatch — replacing only the mismatch path would +leave an agent able to request a dump in front of a client. ## Retention `cleanup_service._sweep_retention_772` prunes `agent_reports` older than @@ -203,6 +221,17 @@ is needed to know whether to page. Non-tabular payloads answer 400 on the rows r than being given an invented row axis, and no-access answers 404 exactly like `GET /reports/{id}`, so the sibling route cannot be used to probe an id. +**The client-facing surface windows differently, on purpose (#2162).** This route is +`Depends(get_current_user)`, which a portal principal — a verified email with no `users` row — +structurally cannot satisfy, so the Workspace could not reuse it and a clone on a +client-facing prefix would have needed its own copy of the uniform-404 contract. Its detail +route took two optional query params instead (`rows_offset`/`rows_limit`), and the **server** +decides tabularity from the real payload rather than trusting `display_hint`: a non-tabular +payload comes back whole with no `row_meta` instead of a 400, which removes the +predict-then-recover round trip the operator client needs. Same honest residual as below, plus +one more — paging re-reads the blob per request, so on a prefix a client can loop it is +rate-limited per (client, agent). + **Honest residual.** The slice happens in Python after the whole blob is read from the column, so it bounds the RESPONSE, not the read. Moving the slice into SQL needs the rows off-row; the trigger for that work should be a measured payload distribution approaching the diff --git a/docs/memory/feature-flows/workspace-agent-page.md b/docs/memory/feature-flows/workspace-agent-page.md index 3cdfcb224..f7d40a22e 100644 --- a/docs/memory/feature-flows/workspace-agent-page.md +++ b/docs/memory/feature-flows/workspace-agent-page.md @@ -88,6 +88,97 @@ mechanism anywhere in Trinity — no table, no column, no endpoint. It has no da source, so it was omitted rather than invented. A number a user reads as "how well is this agent doing" has to come from something real. +## The Reports tab (#2162) + +The tab shipped `
{{ JSON.stringify(payload, null, 2) }}
`. Read beside +this document's own thesis, that is not a cosmetic gap: the section above refuses +to expose an ask's `context` at all because free-form agent JSON has been a +credential-leak surface (canary G-04) — and `agent_reports.payload` is the *same +category*, filed by the same agents, and was being dumped key-for-key to an +external client. Routing it through the shared `components/reports/` renderer set +**narrows** what crosses, because a typed renderer reads only the keys its hint +declares (`tiles`, `columns`+`rows`, `markdown`, `events`) and never the rest of +the payload. + +**Rendering is presentation, and this does not move the exclusion boundary.** +Everything the page must not show is still dropped in `client_portal/agent_page.py` +before the payload exists; nothing is filtered in the Vue component. What changed +is how the payload that legitimately crosses is *presented*. The payload itself +remains agent-authored untrusted content of the same class as `asks.title` and +the schedule name — bounded and escaped, never trusted. + +**The fallback is the one place this surface deliberately differs from the +operator ones.** The shared set's fallback is the raw JSON viewer, so reuse alone +could not satisfy "never a raw dump to a client" — and AC #2 asks for a fallback +*"deliberately stricter than the operator side, because the audience is an +external client"*, i.e. it asks for a SPLIT, not for a stricter default +everywhere. So `ReportRenderer` gained a `fallbackComponent` override defaulting +to `ReportJson` — every operator call site passes nothing and renders exactly what +it always did, because a raw payload is the useful answer when you are debugging +an agent's own output — and this page passes `ReportSummary`: a bounded key-value +view (≤40 entries with a counted remainder, ~200-char values, depth 1, so a +nested value is described as "12 items" and never serialised) with +credential-shaped tokens redacted at **value** level, and no raw payload +reachable behind it at all. A key-name allow-list was rejected twice over: the +fallback fires precisely on payloads nobody has seen, so an allow-list blanks +nearly all of them, and an allowed key's value carries the secret anyway +(`{"status": "failed: sk-…"}`). + +The override deliberately catches an agent-chosen `display_hint: "json"` +(`src/mcp-server/src/tools/reports.ts`) as well as a shape mismatch — replacing +only the *mismatch* path would leave an agent able to put a raw dump in front of +a client by asking for one. + +**Honest residual.** A key-value summary still names every top-level key. It +bounds and humanises; it does not eliminate the class, and a well-shaped +`markdown` or `table` report still renders its values as authored. The general +fix is a G-04-style scrub at the portal read boundary — a security change to a +shipping read path, which deserves its own review rather than riding a UI fix. + +**Row windowing without a second route.** AC #3 wants #1537's windowed-rows +pattern, and the operator reader `GET /api/reports/{id}/rows` is +`Depends(get_current_user)` — which a portal principal (a verified email with no +`users` row) structurally cannot satisfy, the same fact #2128 hit with +feature-flags. Rather than clone it onto a client-facing prefix, the **existing** +detail route took two optional params: + +``` +GET .../agents/{name}/reports/{id}?rows_offset=&rows_limit= + tabular payload -> payload {columns, rows: window} + row_meta {total, offset, limit} + anything else -> payload whole, no row_meta (rows_limit ignored) +``` + +The **server** decides tabularity from the real payload, so the client never +predicts a shape from an agent-authored `display_hint` that can disagree with what +was filed — which deletes the 400-and-recover branch a client-side prediction +would need. `rows_limit` absent is byte-identical to before. No new route means no +second gate and no second copy of the 404-uniformity contract: a foreign report id +and a missing one stay indistinguishable through the windowed path too. + +**Read amplification is real and mitigated, not hidden.** The slice happens in +Python after the whole (≤5 MiB) blob is read out of the column, so paging +*multiplies* reads — the route that exists to cut transfer raises them. Acceptable +behind an operator JWT; on a prefix a client can loop it is an amplification +primitive, so the route is rate-limited per (client, agent) after the roster gate. +A report whose total fits one page costs exactly **one** request and shows no +footer, so only genuinely large tables page at all. + +**Bounded by rows, not by a nested scroll region.** The page has one scroll axis +(#2101, and the asks list above made the same call); a 100-row window plus +`ReportTable`'s stated total and an explicit "Load more" satisfies "contained with +a stated total" without a second scroll axis. + +**The store owns the state, and every await is generation-guarded.** A reset +cannot cancel a promise already in flight, so the `reportsLoaded` flag that +contract #15 requires (an empty state must gate on a *succeeded* fetch, never on +list length) would otherwise have turned a transient wrong-render into a permanent +one: switch agents mid-fetch, the old agent's list lands in the cleared state, and +the new agent is marked loaded-with-the-wrong-data for the life of the mount. +Every report request captures a generation counter before its first await and +discards its result if a reset bumped it. The component additionally gates each +read on the state belonging to the agent on screen — the store is a singleton that +outlives it, and a fresh **mount** for a different agent fires no props watcher. + ## The UX repairs (#2161) The page shipped with four defects, and fixing them forced two of ent#360's own @@ -252,7 +343,15 @@ destination" is finally true. | UI | `components/StackedBarChart.vue` | optional `labels` prop (#2161) | | UI | `utils/executionBuckets.js` | **new** — `BUCKET_COLORS` + chart helpers, shared with `OverviewPanel.vue` (#2161) | | UI | `views/Portal.vue`, `router/index.js` | `/workspace/a/:agentName`; shared stage escape (#2161) | -| Store | `stores/clientPortal.js` | `fetchAgentPage`, `fetchAgentReports`, `fetchAgentReport` | +| Store | `stores/clientPortal.js` | `fetchAgentPage`, `fetchAgentReports`, `fetchAgentReport`; the whole Reports orchestration + generation guard (#2162) | +| Service | `client_portal/agent_page.py` | `_window_rows` + `report_detail(rows_offset, rows_limit)` (#2162) | +| Router | `client_portal/router.py` | `rows_offset`/`rows_limit` on the existing detail route, rate-limited (#2162) | +| UI | `components/reports/ReportSummary.vue` | **new** — the CLIENT-FACING human-readable fallback; no raw escape hatch (#2162) | +| UI | `components/reports/reportSummary.js` | **new** — the bounded, redacting summariser (pure) (#2162) | +| UI | `components/reports/ReportRenderer.vue` | `fallbackComponent` override, default `ReportJson` (operator unchanged); `shapeOk` untouched (#2162) | +| UI | `components/reports/{ReportTable,ReportKpiTiles}.vue` | dark ink pair on meta text — AC #4 (#2162) | +| UI | `components/reports/ReportTimeline.vue` | `bg-blue-500` → `bg-status-info-500` (#2162) | +| UI | `utils/reportPaging.js` | **new** — the one frontend page-size constant, shared with `stores/reports.js` (#2162) | ## Tests @@ -283,6 +382,37 @@ param-enumerating guard cannot have and the reason this bug shipped twice. Also pins that the chart is stacked by untranslated buckets while the labels ride the separate prop — the mistake that renders a blank chart. +`tests/unit/test_2162_portal_report_window.py` — the row window: `rows_limit` +absent returns today's payload unchanged, a tabular payload windows with a TRUE +total, a non-tabular one comes back whole with no `row_meta` (never a 400), the +offset/limit clamps, and the inherited gate — foreign and missing ids stay +indistinguishable 404s through the windowed path. Plus the route wiring: both +params optional, bounded by the shared `REPORT_ROWS_PAGE_MAX`, rate-limited +*after* the roster gate, and resolvable by FastAPI under postponed annotations. + +`src/frontend/tests/unit/reportSummary.spec.js` — the fallback summariser. The +two that define it are negatives: no output path ever serialises the payload +(behaviourally, and by scanning the module source), and a credential-shaped token +is redacted **at value level** — as a whole value, embedded mid-string under an +innocuous key, and past the truncation point. Redaction runs before both +truncation and key humanisation; the humanisation ordering was caught by its own +test, since rewriting `_` to a space destroys the very shape every pattern keys +on. + +`src/frontend/tests/unit/portalReportsStore.spec.js` — the store contract against +a mocked axios (`fleetGridFailuresFetch.spec.js` shape). The agent-switch race is +the one that matters: agent A's list, failure, payload and load-more page each +resolve *after* a switch to B and must all be discarded, with B left NOT marked +loaded. Plus the load-more terminal guard, windowed-vs-whole (`row_meta` present +is the only paging signal), and that a failed fetch never lands in the payload map. + +`src/frontend/tests/unit/portalReportsRendering.spec.js` — the wiring no unit test +can reach: the tab holds no `
` and no serialiser, mounts the shared
+`ReportRenderer`, passes `:fallback-component`, and adds no second scroll axis.
+Guards the **mechanism** rather than the spelling — a prop declared and never used
+would pass a call-site scan — and re-asserts the five CI-pinned `payload.X` keys
+are still inside `ReportRenderer.vue`, the file `test_1535` regexes them out of.
+
 `src/frontend/tests/unit/workspaceRoomsGate.spec.js` — F24 was **rewritten**, not
 deleted. It used to require each exit function to contain `route.params.roomId`;
 after #2161 that would mandate the enumeration that *was* the defect, so it now
diff --git a/docs/memory/requirements/core-agent.md b/docs/memory/requirements/core-agent.md
index 2eda82426..0cbd5c70c 100644
--- a/docs/memory/requirements/core-agent.md
+++ b/docs/memory/requirements/core-agent.md
@@ -437,6 +437,20 @@
   id misses by construction; a failing read costs the labels, not the rows. It is
   **not assumed to be human-written** — schedule creation is `AuthorizedAgent`, so
   an agent-scoped key can author it — and is therefore capped and escaped.
+- **Reports are rendered, never dumped (#2162)**: the Reports tab drives the shared
+  `components/reports/` renderer set (`display_hint` → `report_type` prefix → shape check),
+  the same dispatch Agent Detail uses — reused, not forked, because those renderer keys are
+  CI-pinned as the canonical contract (`test_1535_report_prompt_guidance.py`). It shipped
+  dumping `JSON.stringify(payload)` at an external client, which is the *same* disclosure this
+  section already refuses for an ask's `context`: a typed renderer reads only the keys its hint
+  declares, so this strictly narrows what crosses. The one deliberate divergence from the
+  operator surfaces is the fallback: they keep the raw JSON viewer (useful when you are
+  debugging an agent's own output), while this surface passes `:fallback-component` and an
+  unrecognised payload gets a bounded, humanised key-value summary with credential-shaped tokens
+  redacted and no raw payload reachable behind it. Honest limit — a summary still names every top-level
+  key; it bounds and humanises the residual rather than removing it. A `table` payload is
+  fetched a window at a time (`rows_offset`/`rows_limit`) so a large report never transfers
+  whole, and the tab grows by an explicit "Load more" rather than a nested scroll region.
 - **Key Features**:
   - Header: avatar, name, description, health, last active
   - Stats strip: tasks in window, completed rate, first-try rate, window selector
@@ -449,7 +463,9 @@
     health `unknown` (monitoring is default-OFF, so "unhealthy" would be a lie),
     empty sections, and a failing data source degrades that section only
   - Endpoints: `GET /agents/{name}/page?window=`, `.../reports`,
-    `.../reports/{id}` under the client-portal prefix, all roster-gated
+    `.../reports/{id}` (optional `rows_offset`/`rows_limit` window a tabular payload,
+    #2162 — two query params on the existing route, not a second route) under the
+    client-portal prefix, all roster-gated
 - **Not met**: the AC's **rating tally**. There is no rating, thumbs or feedback
   mechanism anywhere in Trinity, so it has no data source and was omitted rather
   than invented — a number a user reads as "how well is this agent doing" has to
diff --git a/docs/memory/requirements/lifecycle-observability.md b/docs/memory/requirements/lifecycle-observability.md
index 40eec3e99..e4b62128b 100644
--- a/docs/memory/requirements/lifecycle-observability.md
+++ b/docs/memory/requirements/lifecycle-observability.md
@@ -311,10 +311,43 @@ endpoint — reports flow agent → MCP → backend.
 - **FR-4 — Real-time**: a **thin** `agent_report` WebSocket trigger (agent_name, report_id,
   report_type, created_at — never title/payload, since `/ws` is unfiltered SCOPE_ALL); the
   frontend refetches via the access-controlled REST endpoints.
-- **FR-5 — Frontend**: Agent Detail "Reports" tab + Operations → "Reports" fleet tab. Generic
-  + typed renderers (table / KPI tiles / markdown / timeline / JSON) chosen by `display_hint`,
-  then `report_type` prefix, then JSON; each renderer validates payload shape and falls back to
-  the JSON viewer on mismatch. List shows metadata; full payload lazy-loads on expand.
+- **FR-5 — Frontend**: **three** surfaces share one renderer set — Agent Detail "Reports" tab,
+  Operations → "Reports" fleet tab, and the **Workspace agent page's Reports tab** (§5.11 of
+  `core-agent.md`). Typed renderers (table / KPI tiles / markdown / timeline) chosen by
+  `display_hint`, then `report_type` prefix; each validates payload shape and falls back on
+  mismatch. List shows metadata; full payload lazy-loads on expand.
+
+  **The fallback is per-surface, and the client-facing one is stricter (#2162).** The shared
+  default stays the JSON viewer: an operator reading a malformed report is debugging an agent's
+  own output, where the raw payload is the useful answer. For an external client it is not —
+  `payload` is free-form agent-authored JSON of the same class as an ask's `context`, which the
+  Workspace refuses to expose at all (a known credential-leak surface, canary G-04). So
+  `ReportRenderer` takes a `fallbackComponent` override (default `ReportJson`, so every operator
+  call site is untouched) and the Workspace passes `ReportSummary`: a bounded, humanised key-value
+  view (≤40 entries, truncated values, depth 1, credential-shaped tokens redacted) with **no raw
+  payload reachable behind it at all**. The override covers an agent-chosen
+  `display_hint: "json"` as well as a shape mismatch — `json` is a valid value in the MCP tool's
+  enum, so replacing only the mismatch path would leave an agent able to dump on request. A typed
+  renderer reads only the keys its hint declares, so routing a client through the shared set
+  **strictly reduces** what is exposed; the summary bounds and humanises the residual rather than
+  eliminating it (a boundary-side scrub is the general fix).
+
+  **Enumerating the surfaces here is load-bearing**: FR-5 said "two" while a third shipped a raw
+  dump to clients for two releases, which is how #2162 happened. A fourth consumer belongs in
+  this list before it ships.
+- **FR-5a — Client-facing row windowing (#2162)**: the Workspace cannot use FR-9's
+  `GET /api/reports/{id}/rows` — that route is `Depends(get_current_user)` and a portal principal
+  is a verified email with no `users` row (the #2128 structural fact). Rather than clone the
+  route on a client-facing prefix, the **existing** portal detail route takes two optional query
+  params, `rows_offset` / `rows_limit`, and windows `payload["rows"]` **only when the payload
+  really is `{columns, rows}`**, attaching `row_meta {total, offset, limit}` when it does.
+  `rows_limit` absent → byte-identical to before, so the change is purely additive; a non-tabular
+  payload with `rows_limit` set returns whole with no `row_meta` — never a 400, because the
+  server holds the payload and the client should not have to guess its shape from an
+  agent-authored `display_hint` that can disagree with it. Inherits the detail route's roster
+  gate and its uniform 404 rather than re-deriving either. Same honest limit as FR-9: the slice
+  is Python-side after the whole blob is read, so it bounds the **response**, not the read — and
+  because it is client-reachable and loopable it is **rate-limited** per (client, agent).
 - **FR-6 — Retention**: cleanup sweep deletes `agent_reports` older than
   `agent_reports_retention_days` (default 90; `0` disables), chunked like the #772 sweeps.
 - **FR-8 — Agent read-back** (#1538, epic #1534): MCP `list_reports` (metadata; filters
@@ -384,7 +417,10 @@ endpoint — reports flow agent → MCP → backend.
   something only agents whose own CLAUDE.md mentions it ever do. Documents the call, when to
   reach for it (results a human re-reads: scheduled-run findings, batch summaries, KPI
   snapshots), the payload shape per `display_hint` — the shapes FR-5's renderers dispatch on,
-  since a mismatch fails silently as a raw-JSON fallback — the aggregate-before-publishing
+  since a mismatch fails silently into the fallback (a bounded key-value summary since #2162,
+  and on the client-facing Workspace surface that is *all* the reader gets, with no raw payload
+  behind it — so a wrong shape costs the intended presentation outright) — the
+  aggregate-before-publishing
   expectation given the FR-3 cap, and the reports-are-one-way boundary against §26's operator
   queue. Runtime-aware for free via `_adapt_instructions_for_runtime` (#1187: Codex gets bare
   `report`, not `mcp__trinity__report`), and the Codex orientation note lists the tool.
diff --git a/src/backend/client_portal/agent_page.py b/src/backend/client_portal/agent_page.py
index 1ad186841..9455b5907 100644
--- a/src/backend/client_portal/agent_page.py
+++ b/src/backend/client_portal/agent_page.py
@@ -46,6 +46,7 @@
 from typing import Optional
 
 from database import db
+from models import REPORT_ROWS_PAGE_MAX
 
 from . import db as portal_db
 
@@ -242,7 +243,8 @@ def reports(agent_name: str, limit: int = 20, offset: int = 0) -> list[dict]:
     """Metadata for the Reports tab (payload fetched separately when expanded).
 
     Reuses the existing report surface (#918) exactly as the Technical Notes
-    ask. Metadata only: a payload is up to 256 KB and belongs on expansion.
+    ask. Metadata only: a payload is up to 5 MiB (`REPORT_PAYLOAD_MAX_BYTES`,
+    raised from 256 KB in #1537) and belongs on expansion.
     """
     try:
         rows = db.get_reports_for_agent(agent_name, limit=limit, offset=offset)
@@ -303,13 +305,70 @@ def _last_active(agent_name: str) -> Optional[str]:
     return rows[0].get("started_at") if rows else None
 
 
-def report_detail(agent_name: str, report_id: str) -> Optional[dict]:
+def _window_rows(payload, offset: int, limit: int):
+    """Slice a tabular payload's `rows`, or leave it alone (#2162).
+
+    Returns `(payload, row_meta)` — `row_meta` is None whenever no window was
+    applied, which is how the caller (and, downstream, the renderer's footer)
+    tells a windowed table from a whole document.
+
+    **The server decides tabularity, from the real payload.** `display_hint` is
+    agent-authored and can disagree with what was actually filed, so a client
+    that predicted the shape would need a 400 and a recovery re-fetch for the
+    disagreement. Answering "here it is whole, and no, that wasn't a table"
+    removes that branch: one request, always. Every other display_hint is a
+    bounded document (a KPI tile set, a markdown body) with no row axis to
+    slice, so there is nothing to answer 400 about.
+
+    Subtractive on `rows` only: sibling keys are returned exactly as filed, the
+    same as the unwindowed path. The copy is shallow so the source row is never
+    truncated in place.
+    """
+    if not isinstance(payload, dict):
+        return payload, None
+    columns = payload.get("columns")
+    rows = payload.get("rows")
+    if not isinstance(columns, list) or not isinstance(rows, list):
+        return payload, None
+
+    # A negative offset would silently serve rows counted from the END under an
+    # honest-looking total; an unbounded limit would defeat the point of the
+    # window. The route validates both (422), but the clamp is what actually
+    # bounds the response and must not depend on one caller doing so.
+    offset = max(0, int(offset))
+    limit = max(1, min(int(limit), REPORT_ROWS_PAGE_MAX))
+
+    windowed = dict(payload)
+    windowed["rows"] = rows[offset:offset + limit]
+    # `total` is the TRUE row count, not the window — it is what "Showing 100 of
+    # 12,431" reads, and a windowed total would hide the rest behind a footer
+    # that never appears.
+    return windowed, {"total": len(rows), "offset": offset, "limit": limit}
+
+
+def report_detail(agent_name: str, report_id: str, *,
+                  rows_offset: int = 0,
+                  rows_limit: Optional[int] = None) -> Optional[dict]:
     """One report's full payload, scoped to the agent whose page is open.
 
     The agent check is the point: report ids are global, and without it any
     rostered agent's page would read any report in the install. A foreign id
     returns None → the router's 404, identical to a nonexistent one, so this
     cannot be used to test whether a report exists (invariant #8).
+
+    `rows_limit` (#2162) windows a tabular payload's rows, so a large table is
+    never shipped whole to a client — the #1537 pattern, reached here through
+    two optional params rather than a second route. The operator row reader is
+    `Depends(get_current_user)`, which a portal principal (a verified email with
+    no `users` row) structurally cannot satisfy; cloning it on a client-facing
+    prefix would mean a second hand-written gate beside the one above, and that
+    is how a 404-uniformity contract drifts. Absent `rows_limit`, this returns
+    byte-for-byte what it returned before the parameter existed.
+
+    Honest limit, same as the operator route's: the slice happens in Python
+    after the whole blob is read out of the column, so this bounds the RESPONSE,
+    not the read — and therefore paging MULTIPLIES reads. That is why the route
+    rate-limits (a client can loop it; an operator on a JWT is a different risk).
     """
     try:
         row = db.get_report(report_id)
@@ -318,14 +377,26 @@ def report_detail(agent_name: str, report_id: str) -> Optional[dict]:
         return None
     if not row or row.get("agent_name") != agent_name:
         return None
-    return {
+
+    payload = row.get("payload")
+    row_meta = None
+    if rows_limit is not None:
+        payload, row_meta = _window_rows(payload, rows_offset, rows_limit)
+
+    detail = {
         "id": row.get("id"),
         "agent_name": row.get("agent_name"),
         "report_type": row.get("report_type"),
         "title": row.get("title"),
         "display_hint": row.get("display_hint"),
-        "payload": row.get("payload"),
+        "payload": payload,
         "period_start": row.get("period_start"),
         "period_end": row.get("period_end"),
         "created_at": row.get("created_at"),
     }
+    # Present ONLY when a window was actually applied: the client keys "is this
+    # paged?" off its presence, so an always-present key with null fields would
+    # make every bounded document render a Load-more footer it can never satisfy.
+    if row_meta is not None:
+        detail["row_meta"] = row_meta
+    return detail
diff --git a/src/backend/client_portal/router.py b/src/backend/client_portal/router.py
index 3a28e7638..2301e2e1e 100644
--- a/src/backend/client_portal/router.py
+++ b/src/backend/client_portal/router.py
@@ -15,6 +15,7 @@
 import json
 import logging
 import os
+from typing import Optional
 
 import httpx
 from fastapi import (
@@ -30,8 +31,7 @@
     reject_agent_principal,
     require_admin,
 )
-from models import User
-
+from models import REPORT_ROWS_PAGE_MAX, User
 from services.agent_auth import agent_httpx_client
 from services.docker_service import get_agent_container
 from services.platform_audit_service import AuditEventType, platform_audit_service
@@ -568,8 +568,8 @@ def portal_agent_reports(
     principal: PortalPrincipal = Depends(get_portal_principal),
 ):
     """Report metadata for the page's Reports tab. Payloads are fetched per
-    report on expansion — one is capped at 256 KB and a list of them is not a
-    list view."""
+    report on expansion — one is capped at 5 MiB (`REPORT_PAYLOAD_MAX_BYTES`,
+    raised from 256 KB in #1537) and a list of them is not a list view."""
     _require_roster(agent_name, principal.email, principal.is_platform)
     return {
         "agent_name": agent_name,
@@ -583,13 +583,39 @@ def portal_agent_reports(
 def portal_agent_report_detail(
     agent_name: str,
     report_id: str,
+    rows_offset: int = Query(0, ge=0),
+    rows_limit: Optional[int] = Query(None, ge=1, le=REPORT_ROWS_PAGE_MAX),
     principal: PortalPrincipal = Depends(get_portal_principal),
 ):
     """One report's payload. The report must belong to the path agent — a report
     id from another agent returns the same 404 as one that does not exist, so
-    this is not a cross-agent read oracle."""
-    _require_roster(agent_name, principal.email, principal.is_platform)
-    report = agent_page.report_detail(agent_name, report_id)
+    this is not a cross-agent read oracle.
+
+    `rows_offset`/`rows_limit` (#2162) window a **tabular** payload's rows, so a
+    large table is not shipped whole to a client — #1537's pattern, without
+    #1537's route: the operator row reader is JWT-gated and a portal principal
+    cannot reach it, and a second route here would need a second copy of the
+    uniform-404 contract above. Both params are optional and default to today's
+    behaviour; a non-tabular payload with `rows_limit` set comes back whole with
+    no `row_meta` rather than a 400, because the server holds the payload and the
+    client should not have to guess its shape from an agent-authored hint.
+
+    Bounds come from `models.REPORT_ROWS_PAGE_MAX`, the same constant the
+    operator reader uses — imported, never re-typed, so the two page sizes cannot
+    drift apart with each side's tests pinning its own version.
+    """
+    email = principal.email
+    _require_roster(agent_name, email, principal.is_platform)
+    from services import rate_limiter
+
+    # Paging re-reads and re-parses the whole (≤5 MiB) blob per request, so the
+    # route that exists to cut TRANSFER raises READS — fine behind an operator
+    # JWT, an amplification primitive on a prefix a client can loop. Keyed after
+    # the roster gate so an unreachable agent can never mint limiter keys.
+    rate_limiter.enforce(f"portal_report_detail:{email}:{agent_name}", 60, 60)
+    report = agent_page.report_detail(
+        agent_name, report_id, rows_offset=rows_offset, rows_limit=rows_limit,
+    )
     if report is None:
         raise HTTPException(status_code=404, detail="Report not found")
     return report
diff --git a/src/frontend/src/components/portal/PortalAgentPage.vue b/src/frontend/src/components/portal/PortalAgentPage.vue
index ef04f5247..7646ccc94 100644
--- a/src/frontend/src/components/portal/PortalAgentPage.vue
+++ b/src/frontend/src/components/portal/PortalAgentPage.vue
@@ -190,8 +190,29 @@
       
 
       
+      
       
@@ -282,6 +323,10 @@ import { ref, computed, watch, onMounted } from 'vue'
 import { useClientPortalStore } from '@/stores/clientPortal'
 import { BUCKET_COLORS, bucketsForChart, hasChartActivity } from '@/utils/executionBuckets'
 import StackedBarChart from '@/components/StackedBarChart.vue'
+import InlineError from '@/components/InlineError.vue'
+import LoadFailed from '@/components/LoadFailed.vue'
+import ReportRenderer from '@/components/reports/ReportRenderer.vue'
+import ReportSummary from '@/components/reports/ReportSummary.vue'
 import PortalAvatar from './PortalAvatar.vue'
 import { PORTAL_BUCKET_LABELS } from './portalUtils'
 
@@ -321,9 +366,25 @@ const timeWindow = ref('7d')
 const page = ref(null)
 const loading = ref(false)
 const error = ref(null)
-const reports = ref([])
+// Only the "which card is open" bit is local (#2162). Everything else about
+// reports — the list, its loaded/failed flags, per-report payloads, row meta,
+// per-report errors — lives in the store (design contract #21), which is also
+// what makes the agent-switch race testable: a reset there invalidates requests
+// already in flight, which clearing a ref here cannot do.
 const openReport = ref(null)
-const reportPayloads = ref({})
+// The store is a singleton and outlives this component, so a fresh MOUNT for a
+// different agent would otherwise read the previous one's reports as "already
+// loaded" and never refetch — the props watcher below only fires on a change
+// within one instance, not on a remount. Every read is therefore gated on the
+// state actually belonging to the agent on screen; the store's generation
+// counter covers the in-flight half, this covers the at-rest half.
+const reportsMine = computed(() => store.reportsAgent === props.agentName)
+const reports = computed(() => (reportsMine.value ? store.reports : []))
+const reportsLoaded = computed(() => reportsMine.value && store.reportsLoaded)
+const reportsError = computed(() => (reportsMine.value ? store.reportsError : null))
+const reportPayloads = computed(() => (reportsMine.value ? store.reportPayloads : {}))
+const reportRowMeta = computed(() => (reportsMine.value ? store.reportRowMeta : {}))
+const reportErrors = computed(() => (reportsMine.value ? store.reportErrors : {}))
 const documents = ref([])
 const uploads = ref([])
 
@@ -387,8 +448,11 @@ async function load() {
 // Files are separate surfaces that most visits never look at.
 watch(tab, async (t) => {
   try {
-    if (t === 'reports' && !reports.value.length) {
-      reports.value = await store.fetchAgentReports(props.agentName)
+    // Gated on the LOADED FLAG, not on list length: an agent with genuinely
+    // zero reports would otherwise refetch on every entry to the tab, and a
+    // failed fetch would look identical to an empty one (contract #15).
+    if (t === 'reports' && !reportsLoaded.value) {
+      await loadReports()
     } else if (t === 'files' && !documents.value.length && !uploads.value.length) {
       const [d, u] = await Promise.all([
         store.fetchDocuments(props.agentName).catch(() => []),
@@ -406,8 +470,11 @@ watch(tab, async (t) => {
 watch(() => props.agentName, () => {
   pageCache.clear()   // #2160: keyed by name, but never serve one agent's page for another
   page.value = null
-  reports.value = []
-  reportPayloads.value = {}
+  // #2162: the store owns report state AND the generation counter, so this also
+  // invalidates any report request already in flight for the previous agent —
+  // the half a plain ref-clear cannot do, and the reason `reportsLoaded` is safe
+  // to add at all (it would otherwise make a transient wrong-render permanent).
+  store.resetAgentReports(props.agentName)
   openReport.value = null
   documents.value = []
   uploads.value = []
@@ -418,20 +485,32 @@ watch(() => props.agentName, () => {
 watch(timeWindow, load)
 onMounted(load)
 
+function loadReports() {
+  return store.loadAgentReports(props.agentName)
+}
+
 async function toggleReport(id) {
   if (openReport.value === id) { openReport.value = null; return }
   openReport.value = id
-  if (reportPayloads.value[id]) return
-  try {
-    const r = await store.fetchAgentReport(props.agentName, id)
-    reportPayloads.value = { ...reportPayloads.value, [id]: r?.payload ?? {} }
-  } catch {
-    reportPayloads.value = { ...reportPayloads.value, [id]: { error: 'Could not load this report.' } }
-  }
+  // The store owns the already-loaded / already-in-flight guards, so a rapid
+  // expand-collapse-expand cannot fire duplicate requests — which matters here
+  // because each one re-reads the whole blob server-side.
+  await store.loadAgentReport(props.agentName, id)
+}
+
+function retryReport(id) {
+  return store.loadAgentReport(props.agentName, id)
+}
+
+function dismissReportError(id) {
+  store.clearReportError(id)
+}
+
+function loadMoreRows(id) {
+  return store.loadMoreReportRows(props.agentName, id)
 }
 
 const pct = (v) => (v === null || v === undefined ? '—' : `${Math.round(v * 100)}%`)
-const pretty = (v) => { try { return JSON.stringify(v, null, 2) } catch { return String(v) } }
 const size = (b) => (b === null || b === undefined ? '' : b > 1048576 ? `${(b / 1048576).toFixed(1)} MB` : `${Math.max(1, Math.round(b / 1024))} KB`)
 const duration = (ms) => (!ms ? '' : ms < 1000 ? `${ms}ms` : ms < 60000 ? `${(ms / 1000).toFixed(1)}s` : `${Math.round(ms / 60000)}m`)
 
diff --git a/src/frontend/src/components/reports/ReportKpiTiles.vue b/src/frontend/src/components/reports/ReportKpiTiles.vue
index 35195ea5e..da8faf1f1 100644
--- a/src/frontend/src/components/reports/ReportKpiTiles.vue
+++ b/src/frontend/src/components/reports/ReportKpiTiles.vue
@@ -5,7 +5,7 @@
       :key="idx"
       class="bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 rounded-lg px-3 py-2"
     >
-      

{{ tile.label }}

+

{{ tile.label }}

{{ tile.value }}{{ tile.unit }}

diff --git a/src/frontend/src/components/reports/ReportRenderer.vue b/src/frontend/src/components/reports/ReportRenderer.vue index 51d3ce9dc..e6a138234 100644 --- a/src/frontend/src/components/reports/ReportRenderer.vue +++ b/src/frontend/src/components/reports/ReportRenderer.vue @@ -1,5 +1,11 @@ diff --git a/src/frontend/src/components/reports/ReportSummary.vue b/src/frontend/src/components/reports/ReportSummary.vue new file mode 100644 index 000000000..07c6f33fe --- /dev/null +++ b/src/frontend/src/components/reports/ReportSummary.vue @@ -0,0 +1,79 @@ + + + diff --git a/src/frontend/src/components/reports/ReportTable.vue b/src/frontend/src/components/reports/ReportTable.vue index 4c6cae6ae..425a0a20f 100644 --- a/src/frontend/src/components/reports/ReportTable.vue +++ b/src/frontend/src/components/reports/ReportTable.vue @@ -2,7 +2,7 @@
- + @@ -26,7 +26,7 @@
- + Showing {{ rows.length.toLocaleString() }} of {{ meta.total.toLocaleString() }} rows
{{ col }}