diff --git a/CHANGELOG.md b/CHANGELOG.md index 637c3473..58522f0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ All notable changes to this project are documented in this file. Format follows For narrative release notes written for operators and product owners, see [RELEASE_NOTES.md](RELEASE_NOTES.md). +## [1.19.1] - 2026-09-07 + +A patch release. Two MCP Apps defects made an App look broken while the tool behind it had really run: an app-initiated `tools/call` relayed an empty result back to the iframe, and a call made between turns hit a torn-down MCP session and came back as a 502. Both are fixed at the dispatch boundary. The artifact library listing now serves from `UserArtifactsIndex` instead of the base table, which retires the ~3x read amplification and the per-request in-memory sort. **No CDK deploy and no infrastructure change** — but the index that 1.19.0 shipped as groundwork is now on the read path, so its backfill has moved from optional to **required before deploying**. + +### ⚡ Performance + +- **The artifact library listing reads `UserArtifactsIndex`.** `list_for_user` queries the index (`GSI2PK=USER#{uid}`, `GSI2SK` descending) rather than the base table. HEAD and version rows share a base partition, so the old Query scanned roughly 3x the rows it returned and then date-sorted them in memory; only HEAD rows carry the GSI2 keys, so the index holds one row per artifact already newest-first. Ordering now comes from the store instead of being recomputed per request. The response still returns the whole library in one payload, paging the index internally (#989) + +### 🐛 Fixed + +- **App-initiated `tools/call` returned empty content to the iframe.** `_serialize_content` read the result with `getattr`, but Strands' `MCPToolResult` extends `ToolResult`, a `TypedDict` — so `call_tool_sync` returns a plain dict at runtime and the attribute lookup found nothing. The failure was silent end to end: app-api returned 200, inference-api returned 200, and the MCP server had really run the tool, so a write took effect while the App received nothing to render. Any MCP App that re-reads state after an edit appeared frozen (#993) +- **App-initiated tool calls between turns failed with an intermittent 502.** A call arriving after a turn ended resolved to a cached agent whose MCP client sessions Strands had already torn down, raising `MCPClientInitializationError` — which surfaced as `AppToolCallError(502)` in the App. It looked intermittent because a call made while the turn was still streaming found the session alive. The client is now reconnected for the duration of the call and left as it was found; a session already live belongs to an in-flight turn and is used as-is, never stopped, and overlapping calls against the same client share one revived session through a refcount (#994) + +### ⚠️ Changed + +- **`backfill_artifact_user_index_keys.py` now stamps undated rows instead of reporting them.** A HEAD row with no `updated_at` was previously counted and named but left unstamped, on the grounds that a fabricated timestamp would sort wrongly forever. With the index on the read path that choice would drop the artifact from a sparse index — and from its owner's library — silently and permanently. Such a row is now stamped with an empty timestamp segment (`ARTIFACT##{aid}`), which sorts below every real timestamp and so reads last, exactly where the previous in-memory sort put it. **Re-run the script if you ran the 1.19.0 version and it reported any undated rows** (#989) + ## [1.19.0] - 2026-09-06 A correctness release for interrupted turns, plus the share inbox coming out of the dark. Two separate defects made a conversation misreport its own history: a completed response could be labelled **"Response interrupted"** with a Continue button, and an interrupted one could show the model-directed `` in the user's own chat bubble — permanently. Both are fixed at the source rather than patched at the render. The artifact **"Shared with you" inbox now ships on by default** with a kill switch, so a fork gets the finished feature instead of having to discover a variable. Infrastructure adds `UserArtifactsIndex` to the existing `{prefix}-user-artifacts` table (one GSI operation) with a backfill for rows that predate it; nothing reads the index yet. **Requires a CDK deploy**, and two one-shot scripts are available post-deploy. diff --git a/README.md b/README.md index d74ef909..70720035 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ **An open-source, production-ready Generative AI platform for institutions** *Built by Boise State University, designed for everyone.* -[![Release](https://img.shields.io/badge/Release-v1.19.0-6366f1?style=flat&logo=github&logoColor=white)](RELEASE_NOTES.md) +[![Release](https://img.shields.io/badge/Release-v1.19.1-6366f1?style=flat&logo=github&logoColor=white)](RELEASE_NOTES.md) [![Nightly](https://github.com/Boise-State-Development/agentcore-public-stack/actions/workflows/nightly.yml/badge.svg)](https://github.com/Boise-State-Development/agentcore-public-stack/actions/workflows/nightly.yml) ![Python](https://img.shields.io/badge/Python-3.13+-3776AB?style=flat&logo=python&logoColor=white) @@ -296,7 +296,7 @@ agentcore-public-stack/ See [RELEASE_NOTES.md](RELEASE_NOTES.md) for the full changelog, including new features, bug fixes, platform upgrades, and deployment notes for each release. -**Current release:** v1.19.0 +**Current release:** v1.19.1 --- diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 3bf62395..64c2bd30 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,3 +1,62 @@ +# Release Notes — v1.19.1 + +**Release Date:** September 7, 2026 +**Previous Release:** v1.19.0 (September 6, 2026) + +--- + +> 🏗️ **No CDK deploy required.** No infrastructure changed in this release — `infrastructure/gsi-inventory.json` is byte-identical to `main`, and no table gains or loses an index. +> +> ⚠️ **One prerequisite carried forward from 1.19.0, now mandatory.** `UserArtifactsIndex` shipped in 1.19.0 with nothing reading it. This release puts it on the artifact library's read path. Before deploying, the index must report `ACTIVE` **and** `backfill_artifact_user_index_keys.py` must have been applied — an unstamped row is absent from a sparse index, which means an artifact missing from its owner's library. See Deployment notes. + +--- + +## Highlights + +A patch release with two MCP Apps fixes and one performance change. Both defects had the same shape — the tool behind an App really ran, and the App showed nothing for it. One relayed an empty result to the iframe because the MCP client returns a dict where the code expected an object; the other hit a torn-down MCP session on any call made between turns and surfaced as an intermittent 502. Separately, the artifact library listing moves off the base table onto `UserArtifactsIndex`, retiring the read amplification and the per-request sort that the index was added to remove. + +## 🐛 Bug fixes + +- **An MCP App could issue a tool call, have it succeed, and render nothing.** `_serialize_content` read the tool result's content with `getattr`, but Strands' `MCPToolResult` extends `ToolResult` — a `TypedDict`, so what `call_tool_sync` returns at runtime is a plain dict and the attribute lookup found nothing. Every app-initiated `tools/call` therefore relayed `content: []`. Nothing in the chain reported a problem: app-api returned 200, inference-api returned 200, and the MCP server had genuinely run the tool, so a write took effect while the App received nothing to show for it — any App that re-reads state after an edit simply appeared frozen. The dict shape is now handled alongside the attribute one, mirroring the branch `_is_error` already had. The existing dispatch-test fakes were objects carrying a `.content` attribute, which is exactly why the attribute-only path looked correct; the new test uses the dict shape the client really returns (#993) +- **App-initiated tool calls between turns failed with a 502 that looked intermittent.** Such a call arrives after the turn that built the agent has ended. `routes.py` rebuilds the conversation's agent, but with `cache_write=False` it reads a *cached* agent — and Strands tears that agent's MCP client sessions down when its turn ends. `_resolve_client` then handed back a client whose session was no longer running, `call_tool_sync` raised `MCPClientInitializationError`, and that became an `AppToolCallError(502)` reaching the App as a Bad Gateway. The intermittence was the tell: a call made while the turn was still streaming found the session alive and worked. The call is now wrapped so the client is reconnected for its duration and left as it was found. A session already live belongs to an in-flight turn and is used as-is, never stopped here, and overlapping app calls against the same client share one revived session through a refcount, so no call has the connection closed underneath it. The fix is deliberately kept at the dispatch boundary — resolving the live client from the freshly built agent is the deeper fix, but it reaches into how tool providers are held and cached (#994) + +## ⚡ Performance + +**The artifact library listing now serves from `UserArtifactsIndex`.** 1.19.0 added the index and deliberately left it unread; this release switches the read over. `list_for_user` queries `GSI2PK=USER#{uid}` with `GSI2SK` descending instead of querying the base table. + +The base partition holds both HEAD and version rows, so the old Query spanned roughly three times the rows it returned and then date-sorted them in memory on every request. Only HEAD rows carry the GSI2 keys, so the index holds one row per artifact and already in newest-first order — both the amplification and the sort go away, and the ordering comes from the store rather than being recomputed per call. + +The endpoint still returns the whole library in one response, paging the index internally. Exposing pagination is a larger change than it looks: search and the type filter live in the SPA today, and a filter that can only see the loaded page is worse than no filter because it looks authoritative — both would have to move server-side in the same change. The index makes that possible whenever it is wanted. + +### Two things this turned up + +**The library tests were passing against the old code path.** The test fixture declared no `GlobalSecondaryIndexes` at all, so a suite that should have required an index went green without one. The fixture now declares it, which makes moto raise `ResourceNotFoundException` if the query ever stops using the index — the tests exercise the index rather than silently falling back. + +**Undated rows would have vanished.** A sparse index omits any HEAD row without `GSI2PK`, permanently and silently. Rows predating `updated_at` cannot carry a real timestamp, and the original backfill reported them rather than stamping them — correct while nothing read the index, and a silent data-loss path the moment something did. They are now stamped with an empty timestamp segment (`ARTIFACT##{aid}`): not a fabricated time, but a key that sorts below every digit and so reads last when the index is read descending — exactly where the old in-memory sort put it. Neither dev nor prod holds such a row today; this is the defensive branch, and it preserves a contract `test_undated_legacy_rows_are_returned_and_sort_last` already asserted. + +### Test Coverage + +Backend suites for app_api, architecture and the artifact writer pass at 942 tests, with the library fixture now index-backed. The MCP Apps fixes add dispatch tests using the dict result shape the MCP client actually returns and covering session revival, reuse of an already-live session, and refcounted overlap. + +## 🚀 Deployment notes + +**No CDK deploy is required** — this release changes no infrastructure. `backend.yml` and `frontend-deploy.yml` are sufficient. + +**Before deploying, confirm the 1.19.0 index groundwork is complete.** The artifact library now reads `UserArtifactsIndex`, so two things that were optional last release are prerequisites now: + +1. The index reports `ACTIVE` — `UPDATE_COMPLETE` on the stack is not the same thing: + + ```bash + aws dynamodb describe-table --table-name -user-artifacts \ + --query 'Table.GlobalSecondaryIndexes[].{Name:IndexName,Status:IndexStatus}' + ``` + +2. `backend/scripts/backfill_artifact_user_index_keys.py` has been applied. The index is sparse: an unstamped HEAD row is *absent* from it, not stale, so an artifact written before 2026-09-04 and never backfilled disappears from its owner's library after this deploy. The script is dry-run unless given `--apply`. + +**Re-run the backfill if the 1.19.0 version reported any undated rows.** That version counted rows with no `updated_at` and left them unstamped; this version stamps them with an empty timestamp segment so they sort last instead of dropping out. A re-run is idempotent and skips rows already stamped. + +--- + # Release Notes — v1.19.0 **Release Date:** September 6, 2026 diff --git a/VERSION b/VERSION index 815d5ca0..66e2ae6c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.19.0 +1.19.1 diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 3add5959..f85da038 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "agentcore-stack" -version = "1.19.0" +version = "1.19.1" requires-python = ">=3.10" description = "Multi-agent conversational AI system with AWS Bedrock AgentCore" readme = "README.md" diff --git a/backend/scripts/backfill_artifact_user_index_keys.py b/backend/scripts/backfill_artifact_user_index_keys.py index 2b843ecb..6f72950b 100644 --- a/backend/scripts/backfill_artifact_user_index_keys.py +++ b/backend/scripts/backfill_artifact_user_index_keys.py @@ -43,9 +43,12 @@ left alone. * **Never resurrects a deleted row.** ``attribute_exists(SK)`` on every update, matching the writer's own write-back rule. -* **Reports what it cannot fix** rather than guessing — a HEAD row with - no ``updated_at`` is counted and named, not stamped with a fabricated - timestamp that would sort wrongly forever. +* **Encodes a missing ``updated_at`` rather than inventing one.** Such a + row is stamped with an empty timestamp segment + (``ARTIFACT##{aid}``), which sorts below every real timestamp and so + reads last — where the previous in-memory sort put it. Leaving it + unstamped would drop the artifact from a sparse index, and from its + owner's library, silently. Run against dev first, then prod:: @@ -113,16 +116,27 @@ def plan_row(item: Dict[str, Any]) -> Dict[str, Any] | None: return {"skip": f"unexpected PK {pk!r}"} if not artifact_id: return {"skip": "no artifact_id attribute"} - if not updated_at: - # Deliberately not falling back to created_at or "now": GSI2SK is - # the sort key the library orders by, and a fabricated timestamp - # would order this artifact wrongly for the rest of its life. - # Better to name it and let a human decide. - return {"skip": "no updated_at attribute"} - + # A row with no `updated_at` is stamped with an EMPTY timestamp + # segment, not a fabricated one. Two things make that the right + # answer rather than a fudge: + # + # * Leaving it unstamped would drop the artifact out of a sparse + # index — and so out of its owner's library — permanently and + # silently. Dropping somebody's oldest artifacts is worse than + # showing them undated. + # * "ARTIFACT##{aid}" sorts BELOW every real timestamp ("#" < any + # digit), so read descending it lands last — exactly where the + # old in-memory sort put undated rows. It encodes "no timestamp" + # honestly instead of inventing one that would sort wrongly + # forever. + # + # Neither dev nor prod had such a row when this was written; this is + # the defensive branch, and it preserves a contract the library's + # tests already assert. return { "gsi2pk": pk, # GSI2PK is exactly the base PK — USER#{user_id} "gsi2sk": f"ARTIFACT#{updated_at}#{artifact_id}", + "undated": not updated_at, } @@ -145,7 +159,11 @@ def backfill(table: Any, apply: bool) -> Dict[str, int]: continue logger.info( - "stamp %s %s -> GSI2SK=%s", item.get("PK"), sk, plan["gsi2sk"] + "stamp %s %s -> GSI2SK=%s%s", + item.get("PK"), + sk, + plan["gsi2sk"], + " (no updated_at — sorts last)" if plan.get("undated") else "", ) if not apply: stats["stamped"] += 1 diff --git a/backend/src/apis/app_api/artifacts/service.py b/backend/src/apis/app_api/artifacts/service.py index ee078b42..e858897b 100644 --- a/backend/src/apis/app_api/artifacts/service.py +++ b/backend/src/apis/app_api/artifacts/service.py @@ -398,6 +398,9 @@ def get_render_token_service() -> RenderTokenService: # Frozen contract — the HEAD row + SessionIndex keys the artifact writer # (backend/src/agents/builtin_tools/artifacts/service.py) emits. _SESSION_INDEX = "SessionIndex" +# Sparse index over HEAD rows only — read the block comment in +# `list_for_user` before assuming a missing artifact is a query bug. +_USER_INDEX = "UserArtifactsIndex" class ArtifactListService: @@ -532,41 +535,73 @@ def heads_for_session( def list_for_user(self, *, user_id: str) -> list[dict]: """Every artifact the user owns, at HEAD, newest-first. - One base-table Query, no index. The table is already partitioned - by user (`PK=USER#{uid}`), so ownership is enforced by the key - rather than re-checked per row the way `list_for_session` has to - be — a user-wide list is the query this schema was already - shaped for. - - Deliberately not using a GSI. `SessionIndex` is partitioned by - session, not user, so it cannot serve this at all; the sparse - user index the writer stamps keys for (`GSI2PK`/`GSI2SK`) does - not exist yet, and is not needed while the heaviest partition - sits far under a 1MB page. See the writer's module docstring. - - Two consequences of reading the base table, both deliberate: - - * The Query spans version rows as well as HEAD rows, so it reads - roughly 3x what it returns. Filtering happens here rather than - in a FilterExpression because the obvious server-side - discriminator (`attribute_exists(GSI1PK)`) would couple "is - HEAD" to "is session-indexed" — two facts that only happen to - coincide today — and a FilterExpression saves payload, not - read capacity, so it buys nothing worth that coupling. - * The base table sorts by artifact id (a random uuid4), not by - time, so recency ordering is applied here in memory. This is - the part that would move server-side behind the user index. + Served from `UserArtifactsIndex` (GSI2PK=USER#{uid}, + GSI2SK=ARTIFACT#{updated_at}#{aid}) with + `ScanIndexForward=False`. + + This was a base-table Query on the same partition until the + index existed. That worked, but read badly: HEAD and version + rows share the partition, so it spanned roughly 3x the rows it + returned and then date-sorted them in memory. Only HEAD rows + carry the GSI2 keys, so the index holds one row per artifact + already in newest-first order — the amplification and the sort + both go away, and the ordering comes from the store instead of + being recomputed per request. + + ############################################################ + # This index is SPARSE. A HEAD row without GSI2PK is not stale + # in it, it is ABSENT from it — and silently, surfacing as a + # library that lists fewer artifacts than the user made. + # + # Two things keep it complete, and both must stay true: + # * the writer stamps GSI2PK/GSI2SK on BOTH of its write + # paths, and + # * rows predating that (2026-09-04) were stamped by + # `scripts/backfill_artifact_user_index_keys.py`. + # + # If an environment is ever found listing fewer artifacts than + # its table holds, re-run that script before looking anywhere + # else. It is idempotent. + ############################################################ + + Still returns the whole library in one response, paging the + index internally. Exposing pagination is a bigger change than it + looks: search and the type filter are applied in the SPA today, + and a filter that sees only the loaded page is worse than no + filter because it looks authoritative — both would have to move + server-side in the same change. The index makes that possible + whenever it is wanted; it is not wanted yet. """ table = _table() - items: list[dict] = [] + rows: list[dict] = [] kwargs: dict = { - "KeyConditionExpression": Key("PK").eq(f"USER#{user_id}") - & Key("SK").begins_with("ARTIFACT#"), + "IndexName": _USER_INDEX, + "KeyConditionExpression": Key("GSI2PK").eq(f"USER#{user_id}"), + # GSI2SK leads with updated_at, so descending IS newest-first. + "ScanIndexForward": False, } try: while True: resp = table.query(**kwargs) - items.extend(resp.get("Items", [])) + for item in resp.get("Items", []): + if not item.get("artifact_id"): + continue + rows.append( + { + "artifact_id": item.get("artifact_id", ""), + "version": int(item.get("version", 0)), + "title": item.get("title", ""), + "content_type": item.get( + "content_type", "text/html; charset=utf-8" + ), + # Rows written before these attributes existed + # degrade to an empty string rather than + # dropping out of the library. + "created_at": item.get("created_at") or "", + "updated_at": item.get("updated_at") or "", + "session_id": item.get("session_id") or "", + } + ) last = resp.get("LastEvaluatedKey") if not last: break @@ -574,33 +609,8 @@ def list_for_user(self, *, user_id: str) -> list[dict]: except ClientError as exc: raise ArtifactQueryError("artifact library query failed") from exc - heads = [ - item for item in items - if str(item.get("SK", "")).endswith("#HEAD") - ] - rows = [ - { - "artifact_id": item.get("artifact_id", ""), - "version": int(item.get("version", 0)), - "title": item.get("title", ""), - "content_type": item.get( - "content_type", "text/html; charset=utf-8" - ), - # Rows written before these attributes existed degrade to - # an empty string rather than dropping out of the library. - "created_at": item.get("created_at") or "", - "updated_at": item.get("updated_at") or "", - "session_id": item.get("session_id") or "", - } - for item in heads - if item.get("artifact_id") - ] - # Newest-first. Undated legacy rows sort last rather than first, - # which an empty-string key would otherwise do. - rows.sort( - key=lambda row: (bool(row["updated_at"]), row["updated_at"]), - reverse=True, - ) + # No sort here on purpose — the index supplied the order. Adding + # one back would silently mask a broken sort key. return rows @staticmethod diff --git a/backend/src/apis/inference_api/chat/app_tool_dispatch.py b/backend/src/apis/inference_api/chat/app_tool_dispatch.py index 3530f019..b5a8a894 100644 --- a/backend/src/apis/inference_api/chat/app_tool_dispatch.py +++ b/backend/src/apis/inference_api/chat/app_tool_dispatch.py @@ -24,7 +24,9 @@ from __future__ import annotations import asyncio +import contextlib import logging +import threading import uuid from typing import Any, Dict, List, Optional @@ -56,6 +58,12 @@ def _serialize_content(result: Any) -> List[Dict[str, Any]]: block so a quirky server response still round-trips. """ content = getattr(result, "content", None) + if content is None and isinstance(result, dict): + # Strands' MCPToolResult extends ToolResult, a TypedDict — so a result + # is a plain dict at runtime and `getattr` finds nothing. Without this + # every app-initiated tools/call returned `content: []`, leaving the + # App with no data to render. Mirrors `_is_error`'s dict handling. + content = result.get("content") blocks: List[Dict[str, Any]] = [] if isinstance(content, list): for item in content: @@ -81,6 +89,77 @@ def _is_error(result: Any) -> bool: return bool(val) +# Guards the start/stop refcount below. Sessions are cheap to hold but must +# not be torn down under a concurrent call, so overlapping app calls against +# the same client share one revived session. +_revive_lock = threading.Lock() +_revived_users: Dict[int, int] = {} + + +def _session_is_active(client: Any) -> bool: + """Whether `client` currently has a live MCP session. + + Strands exposes this only as a private predicate; treat an unexpected + client shape as "active" so we never start a session we cannot own. + """ + probe = getattr(client, "_is_session_active", None) + if not callable(probe): + return True + try: + return bool(probe()) + except Exception: # noqa: BLE001 - a broken probe must not block the call + return True + + +@contextlib.contextmanager +def _active_session(client: Any): + """Ensure `client` can serve one out-of-band tools/call, then restore it. + + An app-initiated call arrives *between* turns: the agent it belongs to is + served from cache, and Strands tore that agent's MCP client sessions down + when the turn that built them ended. The catalog still holds the client + object, so calling straight through raises + `MCPClientInitializationError("the client session is not running")` and the + App sees a 502. + + Reconnect for the duration of the call and leave the client as we found + it. A session that is already live — the mid-stream case, where the turn + is still running — is used as-is and never stopped here, because it + belongs to that turn. + """ + key = id(client) + started_here = False + + with _revive_lock: + if _revived_users.get(key): + # Another app call already revived it; join that session. + _revived_users[key] += 1 + elif _session_is_active(client): + pass # Live session owned by an in-flight turn — use, don't touch. + else: + client.start() + _revived_users[key] = 1 + started_here = True + + try: + yield client + finally: + if started_here or _revived_users.get(key): + with _revive_lock: + remaining = _revived_users.get(key, 0) - 1 + if remaining > 0: + _revived_users[key] = remaining + else: + _revived_users.pop(key, None) + try: + client.stop(None, None, None) + except Exception: # noqa: BLE001 - best-effort teardown + logger.warning( + "failed to stop a revived MCP client session", + exc_info=True, + ) + + def _resolve_client(agent: Any, tool_name: str): """The MCP client that surfaced `tool_name`. @@ -136,10 +215,14 @@ async def dispatch_app_tool_call( synth_id = f"app-{tool_use_id}-{uuid.uuid4().hex[:8]}" args = dict(arguments or {}) + def _invoke() -> Any: + # `start()` blocks on the handshake, so revive inside the worker + # thread rather than on the event loop. + with _active_session(client): + return client.call_tool_sync(synth_id, tool_name, args) + try: - result = await asyncio.to_thread( - client.call_tool_sync, synth_id, tool_name, args - ) + result = await asyncio.to_thread(_invoke) except Exception as exc: # noqa: BLE001 - surfaced to the App as an error logger.warning( "app tools/call dispatch failed (tool=%s session=%s): %s", diff --git a/backend/tests/apis/app_api/artifacts/test_artifact_library.py b/backend/tests/apis/app_api/artifacts/test_artifact_library.py index e7b651c4..2c684a18 100644 --- a/backend/tests/apis/app_api/artifacts/test_artifact_library.py +++ b/backend/tests/apis/app_api/artifacts/test_artifact_library.py @@ -53,8 +53,24 @@ def client(monkeypatch: pytest.MonkeyPatch): AttributeDefinitions=[ {"AttributeName": "PK", "AttributeType": "S"}, {"AttributeName": "SK", "AttributeType": "S"}, + {"AttributeName": "GSI2PK", "AttributeType": "S"}, + {"AttributeName": "GSI2SK", "AttributeType": "S"}, ], BillingMode="PAY_PER_REQUEST", + # The library endpoint reads this index, so the fixture must + # have it. moto raises ResourceNotFoundException without it — + # correct, and worth keeping: it is what makes these tests + # exercise the index instead of a base-table read. + GlobalSecondaryIndexes=[ + { + "IndexName": "UserArtifactsIndex", + "KeySchema": [ + {"AttributeName": "GSI2PK", "KeyType": "HASH"}, + {"AttributeName": "GSI2SK", "KeyType": "RANGE"}, + ], + "Projection": {"ProjectionType": "ALL"}, + } + ], ) monkeypatch.setenv("DYNAMODB_ARTIFACTS_TABLE_NAME", TABLE) @@ -116,8 +132,12 @@ def _put_artifact( if updated_at is not None: head["updated_at"] = updated_at head["GSI1SK"] = f"ARTIFACT#{updated_at}#{artifact}" - head["GSI2PK"] = f"USER#{user_id}" - head["GSI2SK"] = f"ARTIFACT#{updated_at}#{artifact}" + # GSI2 keys are stamped either way — the post-backfill state of the + # table. An undated row carries an empty timestamp segment, which + # sorts below every real one, so it reads last instead of dropping + # out of the sparse index entirely. + head["GSI2PK"] = f"USER#{user_id}" + head["GSI2SK"] = f"ARTIFACT#{updated_at or ''}#{artifact}" table.put_item(Item=head) @@ -242,7 +262,13 @@ def test_library_route_is_not_shadowed_by_the_artifact_id_route(client) -> None: def test_paginates_a_partition_larger_than_one_page(client) -> None: """The Query loop must drain `LastEvaluatedKey`. Asserted with a stub rather than 1MB of fixture rows, since moto pages on real byte - size and a realistic partition is far under the limit.""" + size and a realistic partition is far under the limit. + + Page 1 carries the NEWER row, because that is what the index does: + GSI2SK leads with `updated_at` and is read descending. So the + service must preserve page order rather than re-sort — a + client-side sort would pass here either way and hide a broken sort + key.""" calls: list[dict] = [] class Paged: @@ -253,14 +279,14 @@ def query(self, **kwargs): "Items": [ { "PK": f"USER#{USER_ID}", - "SK": "ARTIFACT#a1#HEAD", - "artifact_id": "a1", + "SK": "ARTIFACT#a2#HEAD", + "artifact_id": "a2", "version": 1, - "title": "One", + "title": "Two", "content_type": "text/markdown", - "created_at": "2026-05-01T09:00:00+00:00", - "updated_at": "2026-05-01T09:00:00+00:00", - "session_id": "s1", + "created_at": "2026-05-02T09:00:00+00:00", + "updated_at": "2026-05-02T09:00:00+00:00", + "session_id": "s2", } ], "LastEvaluatedKey": {"PK": "x", "SK": "y"}, @@ -269,14 +295,14 @@ def query(self, **kwargs): "Items": [ { "PK": f"USER#{USER_ID}", - "SK": "ARTIFACT#a2#HEAD", - "artifact_id": "a2", + "SK": "ARTIFACT#a1#HEAD", + "artifact_id": "a1", "version": 1, - "title": "Two", + "title": "One", "content_type": "text/markdown", - "created_at": "2026-05-02T09:00:00+00:00", - "updated_at": "2026-05-02T09:00:00+00:00", - "session_id": "s2", + "created_at": "2026-05-01T09:00:00+00:00", + "updated_at": "2026-05-01T09:00:00+00:00", + "session_id": "s1", } ] } diff --git a/backend/tests/apis/inference_api/test_app_tool_dispatch.py b/backend/tests/apis/inference_api/test_app_tool_dispatch.py index d270b52c..7d4dbeda 100644 --- a/backend/tests/apis/inference_api/test_app_tool_dispatch.py +++ b/backend/tests/apis/inference_api/test_app_tool_dispatch.py @@ -165,3 +165,99 @@ async def test_success_returns_result_and_publishes_thread_events(monkeypatch): assert events[0]["data"]["tool_use"]["name"] == "widget_tool" assert events[0]["data"]["tool_use"]["origin"] == "mcp_app" assert events[1]["data"]["tool_result"]["status"] == "success" + + +@pytest.mark.asyncio +async def test_dict_shaped_result_keeps_its_content(monkeypatch): + """A dict-shaped tool result must round-trip its content blocks. + + Strands' `MCPToolResult` extends `ToolResult`, a TypedDict — so what + `call_tool_sync` returns is a plain dict at runtime, and the `getattr` + lookup in `_serialize_content` finds nothing on it. That regression sent + `content: []` back for every app-initiated tools/call, leaving embedded + Apps with no data to render while every layer still reported 200/success. + + The other fakes in this module are objects with a `.content` attribute, + which is why the attribute path alone looked correct. + """ + result = { + "toolUseId": "mcp-1", + "status": "success", + # Strands content blocks are untagged: `text`/`json`, no `type`. + "content": [{"text": '{"lists": []}'}], + } + client = _FakeClient(result) + _patch(monkeypatch, enabled=True, meta=_ui(["model", "app"]), client=client) + + payload = await _call(session_id="disp-dict") + + assert payload["result"]["content"] == [{"text": '{"lists": []}'}] + assert payload["result"]["isError"] is False + + +class _SessionClient(_FakeClient): + """Client that tracks its MCP session the way Strands' MCPClient does. + + `start()` raises if the session is already running, matching upstream, so + a test fails loudly if the dispatch tries to revive a live session. + """ + + def __init__(self, active: bool, result=None) -> None: + super().__init__(result) + self.active = active + self.starts = 0 + self.stops = 0 + self.active_during_call: bool | None = None + + def _is_session_active(self) -> bool: + return self.active + + def start(self): + if self.active: + raise AssertionError("start() on an already-running session") + self.active = True + self.starts += 1 + return self + + def stop(self, exc_type, exc_val, exc_tb) -> None: + self.active = False + self.stops += 1 + + def call_tool_sync(self, tool_use_id, name, arguments=None): + self.active_during_call = self.active + return super().call_tool_sync(tool_use_id, name, arguments) + + +@pytest.mark.asyncio +async def test_revives_a_torn_down_client_session(monkeypatch): + """An app call between turns must reconnect rather than 502. + + The agent is served from cache, so Strands has already torn down its MCP + client sessions; the catalog still holds the client. Calling straight + through raised MCPClientInitializationError, which surfaced to the App as + a 502 Bad Gateway. + """ + client = _SessionClient(active=False) + _patch(monkeypatch, enabled=True, meta=_ui(["model", "app"]), client=client) + + payload = await _call(session_id="disp-revive") + + assert payload["result"]["isError"] is False + assert client.starts == 1 + assert client.active_during_call is True + # Restored to how we found it — the revival is scoped to this one call. + assert client.stops == 1 + assert client.active is False + + +@pytest.mark.asyncio +async def test_leaves_a_live_client_session_alone(monkeypatch): + """Mid-stream the session belongs to the running turn — don't touch it.""" + client = _SessionClient(active=True) + _patch(monkeypatch, enabled=True, meta=_ui(["model", "app"]), client=client) + + await _call(session_id="disp-live") + + assert client.starts == 0 + assert client.stops == 0 + assert client.active is True diff --git a/backend/tests/test_backfill_artifact_user_index_keys.py b/backend/tests/test_backfill_artifact_user_index_keys.py index 6035f2ac..9a9692a3 100644 --- a/backend/tests/test_backfill_artifact_user_index_keys.py +++ b/backend/tests/test_backfill_artifact_user_index_keys.py @@ -165,18 +165,24 @@ def test_spans_every_user(table): # ------------------------------------------------------------------ -def test_refuses_to_fabricate_a_sort_key(table): - """A HEAD row with no `updated_at` is reported, never guessed at. - - GSI2SK is what the library orders by; inventing a timestamp would - sort that artifact wrongly for the rest of its life, invisibly.""" +def test_encodes_a_missing_timestamp_instead_of_inventing_one(table): + """A HEAD row with no `updated_at` is still indexed, with an EMPTY + timestamp segment. + + Leaving it unstamped would drop the artifact out of a sparse index — + and out of its owner's library — silently. An empty segment sorts + below every real timestamp, so descending it reads last, exactly + where the previous in-memory sort put undated rows.""" put_head(table, updated_at=None) stats = backfill_mod.backfill(table, apply=True) - assert stats["skipped"] == 1 - assert stats["stamped"] == 0 - assert "GSI2PK" not in row(table) + assert stats["stamped"] == 1 + assert stats["skipped"] == 0 + item = row(table) + assert item["GSI2SK"] == "ARTIFACT##a1" + # "#" is below every digit, so this sorts under any real timestamp. + assert item["GSI2SK"] < "ARTIFACT#2026-01-01T00:00:00+00:00#a1" def test_dry_run_writes_nothing(table): diff --git a/backend/uv.lock b/backend/uv.lock index c995f68c..5f2c7f6b 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -12,7 +12,7 @@ resolution-markers = [ [[package]] name = "agentcore-stack" -version = "1.19.0" +version = "1.19.1" source = { editable = "." } dependencies = [ { name = "aiofiles" }, diff --git a/frontend/ai.client/package-lock.json b/frontend/ai.client/package-lock.json index 3fd84055..c8a2b8b8 100644 --- a/frontend/ai.client/package-lock.json +++ b/frontend/ai.client/package-lock.json @@ -1,12 +1,12 @@ { "name": "ai.client", - "version": "1.19.0", + "version": "1.19.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ai.client", - "version": "1.19.0", + "version": "1.19.1", "dependencies": { "@angular/cdk": "21.2.14", "@angular/common": "21.2.19", diff --git a/frontend/ai.client/package.json b/frontend/ai.client/package.json index 188c8a95..276bb989 100644 --- a/frontend/ai.client/package.json +++ b/frontend/ai.client/package.json @@ -1,6 +1,6 @@ { "name": "ai.client", - "version": "1.19.0", + "version": "1.19.1", "scripts": { "ng": "ng", "prestart": "tsx scripts/branding/generate-brand-theme.ts && tsx scripts/branding/generate-surface-theme.ts && tsx scripts/branding/generate-surface-colors.ts && tsx scripts/branding/generate-favicons.ts", diff --git a/infrastructure/package-lock.json b/infrastructure/package-lock.json index f883411b..2d0c39c1 100644 --- a/infrastructure/package-lock.json +++ b/infrastructure/package-lock.json @@ -1,12 +1,12 @@ { "name": "infrastructure", - "version": "1.19.0", + "version": "1.19.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "infrastructure", - "version": "1.19.0", + "version": "1.19.1", "dependencies": { "aws-cdk-lib": "2.265.0", "constructs": "10.6.0" diff --git a/infrastructure/package.json b/infrastructure/package.json index 5e4e28bb..7899cf9b 100644 --- a/infrastructure/package.json +++ b/infrastructure/package.json @@ -1,6 +1,6 @@ { "name": "infrastructure", - "version": "1.19.0", + "version": "1.19.1", "bin": { "infrastructure": "bin/infrastructure.js" }, diff --git a/tui/pyproject.toml b/tui/pyproject.toml index c5652247..859f222e 100644 --- a/tui/pyproject.toml +++ b/tui/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "agentcore-tui" -version = "1.19.0" +version = "1.19.1" requires-python = ">=3.11" description = "Terminal client for the AgentCore Public Stack — streaming AI chat in your terminal" readme = "README.md" diff --git a/tui/src/agentcore_tui/__init__.py b/tui/src/agentcore_tui/__init__.py index bff8d598..70e17e45 100644 --- a/tui/src/agentcore_tui/__init__.py +++ b/tui/src/agentcore_tui/__init__.py @@ -7,6 +7,6 @@ from __future__ import annotations -__version__ = "1.19.0" +__version__ = "1.19.1" __all__ = ["__version__"] diff --git a/tui/uv.lock b/tui/uv.lock index bb665a8d..8b06354f 100644 --- a/tui/uv.lock +++ b/tui/uv.lock @@ -8,7 +8,7 @@ resolution-markers = [ [[package]] name = "agentcore-tui" -version = "1.19.0" +version = "1.19.1" source = { editable = "." } dependencies = [ { name = "httpx" },