diff --git a/.github/workflows/app-api.yml b/.github/workflows/app-api.yml index 566880f9..60e71890 100644 --- a/.github/workflows/app-api.yml +++ b/.github/workflows/app-api.yml @@ -51,6 +51,9 @@ on: required: false default: 'false' +permissions: + contents: read + env: # Build and Deploy Configuration (non-sensitive, repository-level) CDK_REQUIRE_APPROVAL: never @@ -128,6 +131,7 @@ jobs: name: Build Docker Image runs-on: ubuntu-24.04 needs: [install, check-stack-dependencies] + if: github.event_name != 'pull_request' # Select environment to get project prefix for image naming environment: ${{ github.event.inputs.environment || (github.ref == 'refs/heads/main' && 'production') || (github.ref == 'refs/heads/develop' && 'development') || 'development' }} diff --git a/.github/workflows/bootstrap-data-seeding.yml b/.github/workflows/bootstrap-data-seeding.yml index 70db6a78..13a97795 100644 --- a/.github/workflows/bootstrap-data-seeding.yml +++ b/.github/workflows/bootstrap-data-seeding.yml @@ -1,5 +1,8 @@ name: "6. Seed Bootstrap Data" +permissions: + contents: read + on: workflow_dispatch: inputs: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 9f84748a..0a7735a3 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -1,5 +1,8 @@ name: "CodeQL Advanced" +permissions: + contents: read + on: push: branches: [ "develop" ] diff --git a/.github/workflows/frontend.yml b/.github/workflows/frontend.yml index 7e69bd6d..2a8590ac 100644 --- a/.github/workflows/frontend.yml +++ b/.github/workflows/frontend.yml @@ -45,10 +45,13 @@ on: required: false default: 'false' +permissions: + contents: read + env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true # Build and Deploy Configuration (non-sensitive, repository-level) - BUILD_CONFIG: production + BUILD_CONFIG: ${{ github.event.inputs.environment || (github.ref == 'refs/heads/main' && 'production') || 'development' }} CDK_REQUIRE_APPROVAL: never WAIT_FOR_INVALIDATION: false # Suppress repeated config dumps from load-env.sh in CI logs @@ -120,6 +123,7 @@ jobs: name: Build Frontend runs-on: ubuntu-24.04 needs: [install, check-stack-dependencies] + if: github.event_name != 'pull_request' steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -152,6 +156,7 @@ jobs: name: Build CDK runs-on: ubuntu-24.04 needs: [install, check-stack-dependencies] + if: github.event_name != 'pull_request' outputs: app-version: ${{ steps.read-version.outputs.APP_VERSION }} steps: diff --git a/.github/workflows/gateway.yml b/.github/workflows/gateway.yml index 0265940c..883bd0ac 100644 --- a/.github/workflows/gateway.yml +++ b/.github/workflows/gateway.yml @@ -39,6 +39,9 @@ on: required: false default: 'false' +permissions: + contents: read + env: # Build and Deploy Configuration (non-sensitive, repository-level) CDK_REQUIRE_APPROVAL: never @@ -55,6 +58,7 @@ jobs: check-stack-dependencies: name: Check Stack Dependencies runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -66,6 +70,7 @@ jobs: install: name: Install Dependencies runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' # Select environment based on trigger # Manual: workflow_dispatch input @@ -151,6 +156,7 @@ jobs: name: Synthesize CloudFormation runs-on: ubuntu-24.04 needs: build-cdk + if: github.event_name != 'pull_request' # Select environment based on trigger # Manual: workflow_dispatch input diff --git a/.github/workflows/inference-api.yml b/.github/workflows/inference-api.yml index 3e323660..fdad6232 100644 --- a/.github/workflows/inference-api.yml +++ b/.github/workflows/inference-api.yml @@ -49,6 +49,9 @@ on: required: false default: 'false' +permissions: + contents: read + env: # Build and Deploy Configuration (non-sensitive, repository-level) CDK_REQUIRE_APPROVAL: never @@ -126,6 +129,7 @@ jobs: name: Build Docker Image runs-on: ubuntu-24.04-arm needs: [install, check-stack-dependencies] + if: github.event_name != 'pull_request' # Select environment to get project prefix for image naming environment: ${{ github.event.inputs.environment || (github.ref == 'refs/heads/main' && 'production') || (github.ref == 'refs/heads/develop' && 'development') || 'development' }} diff --git a/.github/workflows/infrastructure.yml b/.github/workflows/infrastructure.yml index 9ffa3956..b4120fe8 100644 --- a/.github/workflows/infrastructure.yml +++ b/.github/workflows/infrastructure.yml @@ -47,6 +47,9 @@ on: required: false default: 'false' +permissions: + contents: read + env: # Build and Deploy Configuration (non-sensitive, repository-level) CDK_REQUIRE_APPROVAL: never @@ -136,6 +139,7 @@ jobs: name: Synthesize CloudFormation Template runs-on: ubuntu-24.04 needs: build + if: github.event_name != 'pull_request' # Select environment based on trigger # Manual: workflow_dispatch input diff --git a/.github/workflows/nightly-deploy-pipeline.yml b/.github/workflows/nightly-deploy-pipeline.yml index 1e6a1c33..ade4a818 100644 --- a/.github/workflows/nightly-deploy-pipeline.yml +++ b/.github/workflows/nightly-deploy-pipeline.yml @@ -35,6 +35,9 @@ on: default: '' type: string +permissions: + contents: read + env: CDK_REQUIRE_APPROVAL: never FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e17fafab..ebca1c7c 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -47,6 +47,9 @@ on: - 'true' - 'false' +permissions: + contents: read + env: CDK_REQUIRE_APPROVAL: never FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true diff --git a/.github/workflows/rag-ingestion.yml b/.github/workflows/rag-ingestion.yml index cd3af356..3d4e89af 100644 --- a/.github/workflows/rag-ingestion.yml +++ b/.github/workflows/rag-ingestion.yml @@ -40,6 +40,9 @@ on: required: false default: 'false' +permissions: + contents: read + env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true # Build and Deploy Configuration (non-sensitive, repository-level) @@ -56,6 +59,7 @@ jobs: check-stack-dependencies: name: Check Stack Dependencies runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -67,6 +71,7 @@ jobs: install: name: Install Dependencies runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -99,6 +104,7 @@ jobs: name: Build Docker Image runs-on: ubuntu-24.04-arm # Use ARM64-native runner for ARM64 Lambda builds needs: [install, check-stack-dependencies] + if: github.event_name != 'pull_request' # Select environment to get project prefix for image naming environment: ${{ github.event.inputs.environment || (github.ref == 'refs/heads/main' && 'production') || (github.ref == 'refs/heads/develop' && 'development') || 'development' }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 50501bcc..44597031 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,5 +1,8 @@ name: "Release" +permissions: + contents: read + on: push: branches: diff --git a/.github/workflows/sagemaker-fine-tuning.yml b/.github/workflows/sagemaker-fine-tuning.yml index 08bea585..21999115 100644 --- a/.github/workflows/sagemaker-fine-tuning.yml +++ b/.github/workflows/sagemaker-fine-tuning.yml @@ -47,6 +47,9 @@ on: required: false default: 'false' +permissions: + contents: read + env: # Build and Deploy Configuration (non-sensitive, repository-level) CDK_REQUIRE_APPROVAL: never @@ -63,6 +66,7 @@ jobs: check-stack-dependencies: name: Check Stack Dependencies runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -74,6 +78,7 @@ jobs: install: name: Install Dependencies runs-on: ubuntu-24.04 + if: github.event_name != 'pull_request' steps: - name: Checkout code uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -124,6 +129,7 @@ jobs: name: Synthesize CloudFormation Template runs-on: ubuntu-24.04 needs: build + if: github.event_name != 'pull_request' outputs: enabled: ${{ steps.check.outputs.enabled }} diff --git a/.github/workflows/version-check.yml b/.github/workflows/version-check.yml index 625a94e7..4045d47b 100644 --- a/.github/workflows/version-check.yml +++ b/.github/workflows/version-check.yml @@ -1,5 +1,8 @@ name: "Version Check" +permissions: + contents: read + env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true # Suppress repeated config dumps from load-env.sh in CI logs diff --git a/CODE_REVIEW_TOKEN_STORAGE.md b/CODE_REVIEW_TOKEN_STORAGE.md new file mode 100644 index 00000000..3b3ee2e7 --- /dev/null +++ b/CODE_REVIEW_TOKEN_STORAGE.md @@ -0,0 +1,216 @@ +# Code Review: Token Storage Security + +**Date:** 2025-03-24 +**Scope:** `frontend/ai.client/src/app/auth/auth.service.ts`, `auth.interceptor.ts`, `auth.guard.ts` +**Severity Scale:** ๐Ÿ”ด Critical ยท ๐ŸŸ  High ยท ๐ŸŸก Medium ยท ๐ŸŸข Low + +--- + +## Executive Summary + +The application currently stores OAuth access tokens, refresh tokens, and token expiry timestamps in `localStorage`. While the PKCE implementation and CSRF state handling are solid (using `sessionStorage` correctly for ephemeral flow data), the long-lived token storage strategy exposes the application to well-documented attack vectors. This review identifies specific vulnerabilities and proposes a phased migration path. + +--- + +## Current Implementation + +### What's Done Well + +- **PKCE with S256**: The `login()` flow correctly generates a cryptographic code verifier and SHA-256 challenge. State and code verifier are stored in `sessionStorage` (ephemeral, tab-scoped) โ€” this is correct. +- **CSRF state validation**: `handleCallback()` verifies the `state` parameter before exchanging the authorization code. +- **Token expiry buffer**: `isTokenExpired()` uses a 60-second buffer to preemptively refresh, avoiding edge-case 401s. +- **Interceptor retry logic**: The HTTP interceptor handles expired tokens gracefully with a single retry after refresh. +- **No implicit flow**: The app uses Authorization Code + PKCE exclusively, which aligns with current IETF best practice (draft-ietf-oauth-browser-based-apps-26). + +### What's Stored Where + +| Data | Storage | Concern | +|------|---------|---------| +| `access_token` | `localStorage` | ๐Ÿ”ด Accessible to any JS in the origin | +| `refresh_token` | `localStorage` | ๐Ÿ”ด Long-lived credential exposed to XSS | +| `token_expiry` | `localStorage` | ๐ŸŸก Leaks session timing info | +| `auth_provider_id` | `localStorage` | ๐ŸŸข Non-sensitive display data | +| `auth_state` | `sessionStorage` | โœ… Correct โ€” ephemeral CSRF token | +| `auth_code_verifier` | `sessionStorage` | โœ… Correct โ€” ephemeral PKCE data | +| `auth_return_url` | `sessionStorage` | โœ… Correct โ€” ephemeral navigation state | + +--- + +## Findings + +### ๐Ÿ”ด F-01: Refresh Token in localStorage + +**File:** `auth.service.ts:286` +```typescript +localStorage.setItem(this.refreshTokenKey, response.refresh_token); +``` + +**Risk:** A single XSS vulnerability anywhere in the origin gives an attacker the refresh token. Unlike access tokens (short-lived), a stolen refresh token grants the attacker the ability to mint new access tokens independently, potentially for hours or days, even after the user closes their browser. + +**References:** +- OWASP HTML5 Security Cheat Sheet: *"Do not store session identifiers in local storage as the data is always accessible by JavaScript. Cookies can mitigate this risk using the httpOnly flag."* +- IETF draft-ietf-oauth-browser-based-apps-26, Section 8.5: *"localStorage persists between page reloads as well as is shared across all tabs... localStorage does not protect against unauthorized access from malicious JavaScript."* +- Auth0 Refresh Token Best Practices: Refresh tokens in SPAs should use rotation with automatic reuse detection to limit blast radius. + +**Impact:** An attacker with XSS can silently exfiltrate the refresh token and use it from their own machine to generate access tokens. The user would have no indication of compromise. The attacker maintains access until the refresh token expires or is revoked. + +--- + +### ๐Ÿ”ด F-02: Access Token in localStorage + +**File:** `auth.service.ts:284` +```typescript +localStorage.setItem(this.tokenKey, response.access_token); +``` + +**Risk:** The access token is readable by any script running in the same origin. While access tokens are shorter-lived than refresh tokens, `localStorage` persists across tabs and browser restarts, meaning a token could remain available long after the user thinks they've left the application. + +**References:** +- IETF draft-ietf-oauth-browser-based-apps-26, Section 8.4: In-memory storage is preferred over persistent storage for access tokens, as it *"limits the exposure of the tokens to the current execution context only."* +- OWASP JWT Cheat Sheet, Token Sidejacking section: Recommends binding tokens to a browser fingerprint via HttpOnly cookies to prevent XSS-based theft. + +--- + +### ๐ŸŸ  F-03: No Token Binding / Sidejacking Protection + +**Risk:** The tokens are pure bearer tokens with no binding to the browser session. If exfiltrated, they work from any HTTP client. The OWASP JWT Cheat Sheet recommends a "user context" fingerprint โ€” a random value sent as an HttpOnly cookie with a SHA-256 hash embedded in the token โ€” so that a stolen token is useless without the corresponding cookie. + +**Current state:** The application has no mechanism to detect or prevent token replay from a different client/device. + +--- + +### ๐ŸŸ  F-04: Token Expiry Stored as Plaintext Timestamp + +**File:** `auth.service.ts:289` +```typescript +const expiryTime = Date.now() + response.expires_in * 1000; +localStorage.setItem(this.tokenExpiryKey, expiryTime.toString()); +``` + +**Risk:** An attacker (or malicious browser extension) can trivially modify this value to extend the perceived validity of a stolen token, bypassing the client-side expiry check. The server still validates expiry, but the client will continue sending the expired token without attempting a refresh, which could cause confusing UX failures. + +**Note:** This is a medium-severity issue because server-side validation is the real enforcement boundary. However, client-side expiry should not be trivially tamperable. + +--- + +### ๐ŸŸก F-05: localStorage Persists After Logout Intent + +**File:** `auth.service.ts:298-307` + +The `clearTokens()` method does remove tokens from `localStorage`, but if the browser crashes, the tab is force-closed, or the user simply closes the browser without clicking "logout," the tokens remain in `localStorage` indefinitely (until they expire server-side). `sessionStorage` would at least clear on tab close. + +--- + +### ๐ŸŸก F-06: id_token Received but Not Stored or Validated + +**File:** `auth.service.ts:7` +```typescript +id_token?: string; +``` + +The `TokenRefreshResponse` interface includes `id_token`, but the `storeTokens()` method silently discards it. If the ID token is being used elsewhere (e.g., decoded for user profile info), it should be validated (signature, issuer, audience, expiry). If it's not needed, the interface should document why it's ignored. + +--- + +### ๐ŸŸก F-07: Cross-Tab Token Synchronization Gap + +The `storeTokens()` method dispatches a custom `token-stored` event for same-tab notification, but `localStorage` changes in other tabs are only detectable via the native `storage` event. There's no listener for the `storage` event, meaning if a user has multiple tabs open and one tab refreshes the token, other tabs may continue using the old (now potentially rotated/invalid) token until they independently detect expiry. + +--- + +## Recommended Changes + +### Option A: Backend-For-Frontend (BFF) Pattern โ€” Recommended + +This is the strongest option and is explicitly recommended by the IETF draft for *"business applications, sensitive applications, and applications that handle personal data."* AgentCore handles student data at an institution โ€” this qualifies. + +**How it works:** +1. The App API (FastAPI) handles the entire OAuth code exchange server-side +2. Tokens are stored server-side, associated with a session ID +3. The browser receives only an HttpOnly, Secure, SameSite=Strict cookie containing the session ID +4. The App API proxies requests to the Inference API / resource servers, attaching the access token server-side +5. The frontend never sees or stores any OAuth tokens + +**What changes:** + +| Component | Change | +|-----------|--------| +| `auth.service.ts` | Remove all `localStorage` token operations. Login redirects to App API `/auth/login` endpoint. Session state tracked via cookie presence. | +| `auth.interceptor.ts` | Remove token attachment logic. Cookies are sent automatically. Handle 401 by redirecting to login. | +| `auth.guard.ts` | Check session via a lightweight `/auth/session` API call instead of inspecting local tokens. | +| App API | New `/auth/login`, `/auth/callback`, `/auth/session`, `/auth/logout` endpoints. Server-side token storage (DynamoDB or in-memory with Redis). | +| Cookie config | `HttpOnly`, `Secure`, `SameSite=Strict`, `__Host-` prefix, `Path=/` | + +**Mitigates:** F-01, F-02, F-03, F-04, F-05, F-07 + +**Trade-offs:** +- All API traffic to external resource servers must proxy through the BFF (adds latency, increases backend load) +- Requires server-side session storage infrastructure +- More complex deployment + +--- + +### Option B: Web Worker Token Isolation โ€” Moderate Improvement + +If a full BFF is too large a change, isolating the refresh token in a Web Worker is the next best option. This is explicitly called out in the IETF draft (Section 8.3) as a practical pattern. + +**How it works:** +1. A dedicated Web Worker handles all token exchange and refresh operations +2. The refresh token never leaves the Web Worker's memory +3. The Web Worker provides the access token to the main thread on request +4. Access tokens are held in-memory only (closure variable), never in `localStorage` + +**What changes:** + +| Component | Change | +|-----------|--------| +| New `token-worker.ts` | Web Worker that performs code exchange, stores refresh token in its own scope, handles refresh flows | +| `auth.service.ts` | Communicates with the worker via `postMessage`. No `localStorage` for tokens. Access token held in a private variable. | +| `auth.interceptor.ts` | Gets token from `AuthService` in-memory variable instead of `localStorage` | + +**Mitigates:** F-01 (fully), F-02 (partially โ€” access token still in main thread memory but not persisted), F-05 + +**Trade-offs:** +- Access token is still in-memory in the main thread (vulnerable to sophisticated XSS, but not to simple `localStorage` reads) +- Page refresh requires re-obtaining the access token from the worker (or re-triggering a silent auth flow) +- Does not protect against the "Acquisition and Extraction of New Tokens" attack (Section 5.1.3 of the IETF draft) โ€” an attacker with XSS can still run their own OAuth flow + +--- + +### Option C: Minimum Viable Hardening โ€” Quick Wins + +If neither Option A nor B is feasible short-term, these changes reduce risk with minimal refactoring: + +| # | Change | Addresses | +|---|--------|-----------| +| 1 | **Move tokens from `localStorage` to `sessionStorage`**. Tokens are scoped to the tab and cleared on browser close. This is a one-line change per `getItem`/`setItem` call. | F-05 | +| 2 | **Move refresh token to in-memory only** (private class variable). Accept that page refresh requires a silent re-auth or new login. Keep access token in `sessionStorage` for tab persistence. | F-01 | +| 3 | **Add `storage` event listener** to sync token changes across tabs, or invalidate stale tabs. | F-07 | +| 4 | **Validate or discard `id_token`** explicitly. If used, validate signature/claims. If not, remove from interface or add a comment. | F-06 | +| 5 | **Derive expiry from the token itself** (decode the JWT `exp` claim) rather than storing a separate tamperable timestamp. | F-04 | +| 6 | **Configure Cognito refresh token rotation** if not already enabled. Ensures a stolen refresh token is invalidated after one use. | F-01 | +| 7 | **Shorten access token lifetime** to 5-15 minutes (Cognito default is 60 min). Reduces the window of exploitation for stolen access tokens. | F-02 | + +--- + +## Prioritized Action Plan + +| Priority | Action | Effort | Impact | +|----------|--------|--------|--------| +| P0 | Move refresh token to in-memory storage (Option C, item 2) | Small | Eliminates the highest-risk finding | +| P0 | Enable Cognito refresh token rotation (Option C, item 6) | Config change | Defense-in-depth for refresh token theft | +| P1 | Move access token from `localStorage` to `sessionStorage` (Option C, item 1) | Small | Reduces persistence and cross-tab exposure | +| P1 | Shorten access token lifetime to 5-15 min (Option C, item 7) | Config change | Limits blast radius of stolen access tokens | +| P2 | Derive expiry from JWT `exp` claim (Option C, item 5) | Small | Removes tamperable client-side state | +| P2 | Add cross-tab token sync via `storage` event (Option C, item 3) | Small | Prevents stale token usage | +| P3 | Evaluate and plan BFF migration (Option A) | Large | Comprehensive long-term fix | + +--- + +## References + +1. [IETF draft-ietf-oauth-browser-based-apps-26](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-browser-based-apps) โ€” OAuth 2.0 for Browser-Based Applications (December 2025) +2. [OWASP HTML5 Security Cheat Sheet โ€” Local Storage](https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#local-storage) +3. [OWASP JWT Cheat Sheet โ€” Token Sidejacking](https://cheatsheetseries.owasp.org/cheatsheets/JSON_Web_Token_for_Java_Cheat_Sheet.html#token-sidejacking) +4. [OWASP Session Management Cheat Sheet](https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html) +5. [Auth0 โ€” Refresh Tokens: What They Are and When to Use Them](https://auth0.com/blog/refresh-tokens-what-are-they-and-when-to-use-them/) diff --git a/README.md b/README.md index d93e741a..8e9ce824 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.0.0--beta.18-6366f1?style=flat&logo=github&logoColor=white)](RELEASE_NOTES.md) +[![Release](https://img.shields.io/badge/Release-v1.0.0--beta.19-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) @@ -90,6 +90,22 @@ Users can upload images (PNG, JPEG, GIF, WebP) and documents (PDF, CSV, DOCX) di Train and run inference on open-source models directly from the platform. Users with admin-granted access can upload datasets, launch **SageMaker training jobs** on models like BERT, RoBERTa, GPT-2, and more, then run **batch inference** on trained models โ€” all with real-time progress tracking, quota enforcement, and automatic 30-day artifact retention. No ML infrastructure setup required. +### ๐Ÿ”‘ API Keys + +Enable programmatic access to the platform's AI models via REST API. Users generate personal API keys from the Settings page and use them to call the `/chat/api-converse` endpoint โ€” the same models, quotas, and RBAC permissions as the web UI, accessible from any language or tool. + +**How it works:** + +- **One key per user** โ€” creating a new key automatically revokes the previous one +- **90-day expiration** โ€” keys expire automatically; create a new one to renew +- **Secure by design** โ€” keys are SHA-256 hashed at rest; the raw key is shown only once at creation +- **Rate limited** โ€” 60 requests per minute per key +- **Streaming support** โ€” both SSE streaming and standard JSON responses + +**Authentication:** Include the key in the `X-API-Key` header with each request. The platform resolves the user, checks quotas, enforces RBAC model access, and tracks costs โ€” identical to browser-based usage. + +**Built-in code examples:** The API Keys settings page includes a code generator with cURL, Python, and JavaScript examples, a model selector, and configurable parameters (temperature, max tokens, system prompt) so users can start integrating immediately. + ### ๐Ÿง  Memory and Context A two-tier memory system combines **short-term session history** with **long-term user preferences**. The agent remembers coding style preferences, language choices, and learned facts across sessions โ€” personalization without compromising privacy. @@ -241,7 +257,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.0.0-beta.18 +**Current release:** v1.0.0-beta.19 --- diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 6e83e360..e97a3627 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,3 +1,140 @@ +# Release Notes โ€” v1.0.0-beta.19 + +**Release Date:** March 25, 2026 +**Previous Release:** v1.0.0-beta.18 (March 24, 2026) + +--- + +## Highlights + +This release introduces **Conversation Sharing** โ€” a full-stack feature that lets users share point-in-time snapshots of conversations via URL, with public or email-restricted access controls. Alongside that, **session compaction** has been refactored and enabled by default to automatically manage context window size in long conversations, **fine-tuning** gains drag-and-drop dataset uploads and custom HuggingFace model support, and a round of **security hardening** resolves all remaining CodeQL clear-text logging alerts. The frontend production build is now fully optimized (4.96 MB initial, down from 8.85 MB), and PR workflows have been slimmed down to only run build and test steps. + +--- + +## New Feature: Conversation Sharing + +Users can now share conversations with others via shareable URLs. Shares are point-in-time snapshots โ€” the shared view captures the conversation as it existed at the moment of sharing, so subsequent messages don't leak into shared links. + +### How It Works + +- **Share modal** accessible from the session UI lets users create a share with either `public` (anyone with the link) or `specific` (restricted to a list of email addresses) access +- **Manage shares dialog** on the session management page shows all active shares with options to update access levels or revoke +- **Read-only shared view** at `/shared/:shareId` renders the conversation with full markdown formatting, no authentication required for public shares +- **Export support** for downloading shared conversations + +### Backend + +Three new API routers handle the sharing lifecycle: + +- `POST /conversations/{session_id}/share` โ€” Create a share snapshot +- `GET /conversations/{session_id}/shares` โ€” List shares for a session +- `PUT /shares/{share_id}` โ€” Update access level or allowed emails +- `DELETE /shares/{share_id}` โ€” Revoke a share +- `GET /shares/{share_id}/export` โ€” Export shared conversation +- `GET /shared/{share_id}` โ€” Public read-only retrieval + +### Infrastructure + +A new `shared-conversations` DynamoDB table is provisioned in the Infrastructure stack with two GSIs: + +- `SessionShareIndex` โ€” Lookup shares by original session ID +- `OwnerShareIndex` โ€” List shares by owner, sorted by creation time + +The table name and ARN are exported via SSM parameters and imported by the App API stack, which grants full CRUD permissions to the Fargate task role. + +### Test Coverage + +1,300+ lines of new tests across three test files covering share CRUD operations, access control enforcement, export functionality, and property validation. + +--- + +## Session Compaction โ€” Enabled by Default + +The session compaction system has been refactored and is now **enabled by default** for all conversations. Compaction automatically manages context window size by summarizing older turns when the token count exceeds the threshold, keeping conversations responsive without manual intervention. + +- **Default configuration:** enabled, 100K token threshold, 3 protected recent turns, 500-char max tool content length +- **Turn-based session manager** rewritten with cleaner separation of concerns (870-line net reduction) +- **Expanded test suite** with 481+ new lines of test coverage for compaction behavior + +--- + +## Fine-Tuning Enhancements + +### Drag-and-Drop Dataset Upload + +The training job creation page now supports drag-and-drop file upload with visual feedback, replacing the basic file picker. Upload instructions have been updated to guide users through dataset formatting requirements. + +### Custom HuggingFace Model Support + +Users are no longer limited to the preset model list. The training job form now includes a searchable model selector that accepts any valid HuggingFace model identifier. The backend validates and passes custom model IDs through to SageMaker. Frontend tests cover the custom model selection and submission flow. + +--- + +## Security Hardening + +### Clear-Text Logging Remediation + +All remaining CodeQL clear-text logging alerts have been resolved: + +- **`seed_auth_provider`** โ€” Client IDs masked to first 8 characters, Secrets Manager ARNs fully redacted from output +- **`seed_bootstrap_data`** โ€” Full exception objects replaced with error codes in log messages +- **`external_mcp_client`** โ€” Server URLs removed from logs, MCP client configuration logging downgraded from info to debug +- **`oauth_tool_service`** โ€” Decrypted tokens isolated into `_try_get_token()` to prevent taint propagation, lazy log formatting applied +- **`config.ts`** โ€” AWS account IDs and CORS origins removed from CDK config log output + +### OAuth Redirect Validation + +The OAuth callback endpoint now validates redirect URLs to prevent open redirect vulnerabilities. + +### Workflow Permissions + +All 13 GitHub Actions workflows now declare explicit `permissions: contents: read`, implementing the principle of least privilege instead of relying on default token permissions. + +--- + +## Frontend Production Optimization + +The Angular production build is now fully optimized: + +- Removed `optimization: false` override from base build options that was blocking the production configuration +- Production config now enables full optimization, disables source maps, and extracts licenses +- `anyComponentStyle` budget increased from 4 kB to 200 kB to accommodate Tailwind CSS +- **Result:** 4.96 MB initial bundle (871 KB gzipped), down from 8.85 MB unoptimized +- `BUILD_CONFIG` is now branch-aware: `main` โ†’ production, `develop` โ†’ development, manual dispatch โ†’ user input + +### Google Fonts Fix + +Google Fonts `@import` statements moved from component CSS to `index.html` `` tags, fixing a CI build failure where the CSS optimizer couldn't resolve external font URLs. + +--- + +## CI/CD Improvements + +### Lighter PR Workflows + +Pull request workflow runs have been significantly trimmed across all 7 deployment workflows. PRs now only run: + +- Dependency installation and caching +- Stack dependency validation +- CDK TypeScript compilation (catches build errors) +- Python tests (app-api, inference-api) +- Frontend tests (Vitest) + +Skipped on PRs: Docker image builds, Docker image tests, CDK synthesis, CDK validation, ECR push, and deployment. This reduces PR CI time and eliminates the need for AWS credentials on pull requests. + +--- + +## Bug Fixes + +- **Bedrock prompt caching** โ€” Caching configuration commented out in model config due to current Bedrock limitations. Tests updated to reflect the change. + +--- + +## Deployment Notes + +This release adds a new DynamoDB table (`shared-conversations`) to the Infrastructure stack. Deploy the Infrastructure stack first, then the App API stack. If deploying all stacks simultaneously, the App API deployment may fail on first run due to the SSM parameter dependency โ€” just rerun it after Infrastructure completes. + +--- # Release Notes โ€” v1.0.0-beta.18 **Release Date:** March 24, 2026 diff --git a/VERSION b/VERSION index b031bbaa..3282efda 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.0.0-beta.18 +1.0.0-beta.19 diff --git a/backend/pyproject.toml b/backend/pyproject.toml index cad261c1..137b65b7 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "agentcore-stack" -version = "1.0.0-beta.18" +version = "1.0.0-beta.19" requires-python = ">=3.10" description = "Multi-agent conversational AI system with AWS Bedrock AgentCore" readme = "README.md" diff --git a/backend/scripts/seed_auth_provider.py b/backend/scripts/seed_auth_provider.py index 0e6a3f09..ec569e8b 100755 --- a/backend/scripts/seed_auth_provider.py +++ b/backend/scripts/seed_auth_provider.py @@ -334,7 +334,7 @@ def main() -> None: print(f" Display Name: {item['displayName']}") print(f" Enabled: {item['enabled']}") print(f" Issuer URL: {item['issuerUrl']}") - print(f" Client ID: {item['clientId']}") + print(f" Client ID: {item['clientId'][:8]}...") print(f" Client Secret: {'*' * min(len(args.client_secret), 20)}") print(f" Scopes: {item['scopes']}") print(f" PKCE: {item['pkceEnabled']}") @@ -352,7 +352,7 @@ def main() -> None: print(f" Button Color: {item.get('buttonColor')}") print() print(f" DynamoDB Table: {args.table_name}") - print(f" Secrets ARN: {args.secrets_arn}") + print(f" Secrets ARN: {'configured' if args.secrets_arn else '(not set)'}") print(f" Region: {args.region}") if args.profile: print(f" AWS Profile: {args.profile}") diff --git a/backend/scripts/seed_bootstrap_data.py b/backend/scripts/seed_bootstrap_data.py index 0f20bd8e..d93d7818 100644 --- a/backend/scripts/seed_bootstrap_data.py +++ b/backend/scripts/seed_bootstrap_data.py @@ -96,7 +96,8 @@ def seed_auth_provider( result.details.append(msg) return result except ClientError as e: - msg = f"Failed to check existing auth provider '{provider_id}': {e}" + error_code = e.response["Error"]["Code"] + msg = f"Failed to check existing auth provider '{provider_id}': {error_code}" logger.error(msg) result.failed = 1 result.details.append(msg) @@ -164,7 +165,8 @@ def seed_auth_provider( table.put_item(Item=item) logger.info("Auth provider '%s' written to DynamoDB", provider_id) except ClientError as e: - msg = f"Failed to write auth provider '{provider_id}' to DynamoDB: {e}" + error_code = e.response["Error"]["Code"] + msg = f"Failed to write auth provider '{provider_id}' to DynamoDB: {error_code}" logger.error(msg) result.failed = 1 result.details.append(msg) @@ -193,7 +195,8 @@ def seed_auth_provider( else: logger.info("Client secret for '%s' already in Secrets Manager โ€” kept existing", provider_id) except ClientError as e: - msg = f"Failed to write secret for '{provider_id}': {e}" + error_code = e.response["Error"]["Code"] + msg = f"Failed to write secret for '{provider_id}': {error_code}" logger.error(msg) result.failed = 1 result.details.append(msg) @@ -869,12 +872,14 @@ def main() -> None: results: list[SeedResult] = [] # --- Auth provider seeding --- - required_auth_vars = { - "SEED_AUTH_ISSUER_URL": auth_issuer_url, - "SEED_AUTH_CLIENT_ID": auth_client_id, - "SEED_AUTH_CLIENT_SECRET": auth_client_secret, - } - missing_auth = [k for k, v in required_auth_vars.items() if not v] + required_auth_var_names = [ + "SEED_AUTH_ISSUER_URL", + "SEED_AUTH_CLIENT_ID", + "SEED_AUTH_CLIENT_SECRET", + ] + missing_auth = [ + name for name in required_auth_var_names if not os.environ.get(name, "") + ] if missing_auth: logger.warning( diff --git a/backend/src/.env.example b/backend/src/.env.example index 230ad27d..6bea8a6e 100644 --- a/backend/src/.env.example +++ b/backend/src/.env.example @@ -198,6 +198,17 @@ DYNAMODB_APP_ROLES_TABLE_NAME= # Example: bsu-agentcore-user-files-dev DYNAMODB_USER_FILES_TABLE_NAME=bsu-agentcore-user-files +# ============================================================================= +# SHARED CONVERSATIONS (OPTIONAL) +# ============================================================================= +# DynamoDB table for shared conversation snapshots +# Purpose: Store point-in-time snapshots of shared conversations +# Local Development: Leave empty to disable sharing feature +# Production: Set to your DynamoDB table name from CDK deployment +# CDK Deployment: Created by InfrastructureStack +# Example: dev-boisestateai-v2-shared-conversations +SHARED_CONVERSATIONS_TABLE_NAME= + # ============================================================================= # OAUTH PROVIDER MANAGEMENT (OPTIONAL) # ============================================================================= diff --git a/backend/src/agents/main_agent/core/model_config.py b/backend/src/agents/main_agent/core/model_config.py index ee7b7cff..7313ed23 100644 --- a/backend/src/agents/main_agent/core/model_config.py +++ b/backend/src/agents/main_agent/core/model_config.py @@ -114,11 +114,14 @@ def to_bedrock_config(self) -> Dict[str, Any]: "temperature": self.temperature } - # Use CacheConfig with strategy="auto" for automatic prompt caching - # This automatically injects cache points at the end of the last assistant message + # TODO: Re-enable once Bedrock supports cachePoint blocks alongside + # non-PDF document blocks (.md, .docx, etc.). Currently causes: + # ValidationException: messages.N.content.M.type: Field required + # because Bedrock can't translate cachePoint after document blocks + # to the Anthropic format. # See: https://github.com/strands-agents/sdk-python/pull/1438 - if self.caching_enabled: - config["cache_config"] = CacheConfig(strategy="auto") + # if self.caching_enabled: + # config["cache_config"] = CacheConfig(strategy="auto") # Configure botocore-level retries and timeouts for Bedrock API calls # This is the first retry layer (HTTP-level), fires before Strands SDK retries diff --git a/backend/src/agents/main_agent/integrations/external_mcp_client.py b/backend/src/agents/main_agent/integrations/external_mcp_client.py index 238254d0..161f8721 100644 --- a/backend/src/agents/main_agent/integrations/external_mcp_client.py +++ b/backend/src/agents/main_agent/integrations/external_mcp_client.py @@ -121,13 +121,13 @@ def create_external_mcp_client( tool_id = tool_definition.tool_id if tool_definition else "unknown" requires_oauth = tool_definition.requires_oauth_provider if tool_definition else None + has_token = bool(oauth_token) logger.info(f"Creating external MCP client for tool: {tool_id}") - logger.info(f" Server URL: {config.server_url}") - logger.info(f" Transport: {config.transport}") - logger.info(f" Auth Type: {config.auth_type}") + logger.debug(f" Transport: {config.transport}") + logger.debug(f" Auth Type: {config.auth_type}") if requires_oauth: - logger.info(f" Requires OAuth Provider: {requires_oauth}") - logger.info(f" OAuth Token Provided: {bool(oauth_token)}") + logger.debug(" Requires OAuth Provider: yes") + logger.debug(f" OAuth Token Provided: {has_token}") try: # Build list of auth handlers (may combine multiple) @@ -140,7 +140,7 @@ def create_external_mcp_client( if oauth_token: oauth_auth = create_oauth_bearer_auth(token=oauth_token) auth_handlers.append(oauth_auth) - logger.info(" Using OAuth Bearer token auth (skipping SigV4)") + logger.debug(" Using OAuth Bearer token auth (skipping SigV4)") # AWS IAM SigV4 authentication (for Lambda/API Gateway without OAuth) elif config.auth_type == MCPAuthType.AWS_IAM or config.auth_type == "aws-iam": @@ -156,7 +156,7 @@ def create_external_mcp_client( sigv4_auth = get_sigv4_auth(service=service, region=region) auth_handlers.append(sigv4_auth) - logger.info(f" Using AWS IAM SigV4 auth for service: {service}, region: {region}") + logger.debug(f" Using AWS IAM SigV4 auth for service: {service}, region: {region}") elif config.auth_type == MCPAuthType.API_KEY or config.auth_type == "api-key": # API key authentication would be handled via headers @@ -174,7 +174,7 @@ def create_external_mcp_client( auth = auth_handlers[0] elif len(auth_handlers) > 1: auth = CompositeAuth(*auth_handlers) - logger.info(f" Using composite auth with {len(auth_handlers)} handlers") + logger.debug(f" Using composite auth with {len(auth_handlers)} handlers") # Create the MCP client based on transport type transport = config.transport diff --git a/backend/src/agents/main_agent/session/compaction_models.py b/backend/src/agents/main_agent/session/compaction_models.py index ccd47a6c..35fd018c 100644 --- a/backend/src/agents/main_agent/session/compaction_models.py +++ b/backend/src/agents/main_agent/session/compaction_models.py @@ -54,17 +54,17 @@ class CompactionConfig: Can be loaded from environment variables or passed directly. """ - enabled: bool = False + enabled: bool = True token_threshold: int = 100_000 # Trigger checkpoint when exceeded - protected_turns: int = 2 # Recent turns to protect from truncation + protected_turns: int = 3 # Recent turns to protect from truncation max_tool_content_length: int = 500 # Max chars before truncating tool output @classmethod def from_env(cls) -> "CompactionConfig": """Load configuration from environment variables.""" return cls( - enabled=os.environ.get("AGENTCORE_MEMORY_COMPACTION_ENABLED", "false").lower() == "true", + enabled=os.environ.get("AGENTCORE_MEMORY_COMPACTION_ENABLED", "true").lower() == "true", token_threshold=int(os.environ.get("AGENTCORE_MEMORY_COMPACTION_TOKEN_THRESHOLD", "100000")), - protected_turns=int(os.environ.get("AGENTCORE_MEMORY_COMPACTION_PROTECTED_TURNS", "2")), + protected_turns=int(os.environ.get("AGENTCORE_MEMORY_COMPACTION_PROTECTED_TURNS", "3")), max_tool_content_length=int(os.environ.get("AGENTCORE_MEMORY_COMPACTION_MAX_TOOL_CONTENT_LENGTH", "500")), ) diff --git a/backend/src/agents/main_agent/session/turn_based_session_manager.py b/backend/src/agents/main_agent/session/turn_based_session_manager.py index 6cf8fb45..5241301f 100644 --- a/backend/src/agents/main_agent/session/turn_based_session_manager.py +++ b/backend/src/agents/main_agent/session/turn_based_session_manager.py @@ -1,17 +1,16 @@ """ -Session Manager with Context Compaction for AgentCore Memory +Compacting Session Manager for AgentCore Memory -This session manager provides: -1. Message count tracking (avoids eventual consistency issues with AgentCore Memory) -2. Proper hook registration (ensures our callbacks are used, not the base manager's) -3. Session cancellation support -4. Automatic context window compaction (truncation + checkpointing) +Extends AgentCoreMemorySessionManager (subclass, not wrapper) so the SDK's +standard hook wiring (register_hooks) handles message persistence, sync, and +LTM retrieval correctly. We only override initialize() to layer on compaction +after the SDK finishes its standard session restore. Compaction Strategy (two-feature approach): -- Stage 1: Tool content truncation - Applied every turn, reduces verbose tool I/O -- Stage 2: Checkpoint + Summary - Triggered when token threshold exceeded +- Stage 1: Tool content truncation โ€” applied every turn, reduces verbose tool I/O +- Stage 2: Checkpoint + Summary โ€” triggered when token threshold exceeded -Based on: https://medium.com/@tonypeng_30327/part-2-building-agent-memory-system-bedrock-agentcore-context-compaction-82917f4c2ba0 +Based on: https://github.com/aws-samples/sample-strands-agent-with-agentcore """ import copy @@ -32,25 +31,22 @@ logger = logging.getLogger(__name__) -class TurnBasedSessionManager: +class TurnBasedSessionManager(AgentCoreMemorySessionManager): """ - Session manager with built-in context compaction for AgentCore Memory. + Session manager with token-based context compaction. + + Inherits from AgentCoreMemorySessionManager so the SDK's register_hooks() + handles all hook wiring: append_message, sync_agent, retrieve_customer_context. + We override initialize() to apply compaction after the SDK restores the session. Features: - - Message count tracking (initialized once at startup to avoid eventual consistency) - - Proper hook registration (intercepts MessageAddedEvent to track count) - - Session cancellation support - - Two-feature compaction: - 1. Tool content truncation (every turn) - 2. Checkpoint + summary (when token threshold exceeded) - - The compaction state is stored as a nested attribute in the DynamoDB session - record, not as a separate item. This ensures atomic updates and simplifies - the storage layer. + - Checkpoint-based message loading (skip old messages, prepend summary) + - Tool content truncation (reduce verbose tool I/O in older turns) + - Session cancellation support (via cancelled flag) + - Compaction state persisted in DynamoDB session metadata """ # Class-level DynamoDB table reference for compaction state - # Uses DYNAMODB_SESSIONS_METADATA_TABLE_NAME (same as session metadata) _dynamodb_table = None _dynamodb_table_name: Optional[str] = None @@ -61,6 +57,7 @@ def __init__( compaction_config: Optional[CompactionConfig] = None, user_id: Optional[str] = None, summarization_strategy_id: Optional[str] = None, + **kwargs: Any, ): """ Initialize session manager with optional compaction. @@ -72,14 +69,14 @@ def __init__( user_id: User ID for DynamoDB session lookup summarization_strategy_id: Strategy ID for LTM summary retrieval """ - self.base_manager = AgentCoreMemorySessionManager( + super().__init__( agentcore_memory_config=agentcore_memory_config, - region_name=region_name + region_name=region_name, + **kwargs, ) - self.config = agentcore_memory_config - self.region_name = region_name self.user_id = user_id + self.region_name = region_name self.summarization_strategy_id = summarization_strategy_id # Compaction config (None means disabled) @@ -96,73 +93,191 @@ def __init__( # Session control self.cancelled = False - # Message count tracking - self.message_count: int = self._initialize_message_count() + # Message count tracking (for stream_coordinator compatibility) + self.message_count: int = 0 # Log initialization if compaction_config and compaction_config.enabled: logger.info( - f"โœ… TurnBasedSessionManager initialized with compaction " + f"TurnBasedSessionManager initialized with compaction " f"(threshold={compaction_config.token_threshold:,}, " - f"protected_turns={compaction_config.protected_turns}, " - f"initial_message_count={self.message_count})" + f"protected_turns={compaction_config.protected_turns})" ) else: - logger.info( - f"โœ… TurnBasedSessionManager initialized " - f"(compaction disabled, initial_message_count={self.message_count})" - ) + logger.info("TurnBasedSessionManager initialized (compaction disabled)") - def _get_dynamodb_table(self): + # ========================================================================= + # SDK Overrides โ€” minimal surface area + # ========================================================================= + + def append_message(self, message: Dict, agent: "Agent", **kwargs: Any) -> None: + """Append message with empty-content filtering and cancellation check.""" + if self.cancelled: + logger.warning("Session cancelled, ignoring message") + return + + # Filter out empty content blocks before saving + filtered_message = self._filter_empty_text(message) + content = filtered_message.get("content", []) + if not content or (isinstance(content, list) and len(content) == 0): + logger.debug("Skipping message with empty content") + return + + super().append_message(filtered_message, agent, **kwargs) + self.message_count += 1 + + def initialize(self, agent: "Agent", **kwargs: Any) -> None: + """ + Initialize agent with two-feature compaction. + + Flow: + 1. Capture whether this is a new session (before SDK resets the flag) + 2. Let the SDK restore agent state and load messages from AgentCore Memory + 3. Apply compaction (checkpoint + truncation) on the loaded messages + """ + is_new_session = self._is_new_session + + logger.info(f"TurnBasedSessionManager.initialize() called for agent_id={agent.agent_id}") + + # Let the SDK handle all session restore logic: + # - read/create agent in session repository + # - restore agent state, internal state, conversation manager state + # - load messages from AgentCore Memory + # - fix broken tool-use histories + super().initialize(agent, **kwargs) + + self._total_message_count_at_init = len(agent.messages) + self.message_count = self._total_message_count_at_init + + # Initialize compaction defaults + self.compaction_state = CompactionState() + self._valid_cutoff_indices = [] + self._all_messages_for_summary = [] + + # Apply compaction for existing sessions with compaction enabled + if is_new_session: + return + + if not self.compaction_config or not self.compaction_config.enabled: + return + + try: + self._apply_compaction(agent) + except Exception as e: + logger.error(f"Compaction failed, using full history: {e}", exc_info=True) + self.compaction_state = CompactionState() + self._valid_cutoff_indices = [] + self._all_messages_for_summary = [] + + # ========================================================================= + # Compaction โ€” applied after SDK session restore + # ========================================================================= + + def _apply_compaction(self, agent: "Agent") -> None: """ - Lazy initialization of DynamoDB table for compaction state. + Apply compaction to agent.messages after SDK session restore. - Uses DYNAMODB_SESSIONS_METADATA_TABLE_NAME - the same table that stores - session metadata (title, preferences, etc). Compaction state is stored - as a nested 'compaction' attribute on the session record. + Modifies agent.messages in-place to: + 1. Skip old messages (checkpoint-based) + 2. Prepend conversation summary + 3. Truncate verbose tool content in older turns """ + all_messages = agent.messages + + logger.info( + f"Compaction decision: config={self.compaction_config}, " + f"enabled={self.compaction_config.enabled}, " + f"messages_loaded={len(all_messages)}" + ) + + # Load compaction state from DynamoDB + self.compaction_state = self._load_compaction_state() + + # Cache valid cutoff indices (user text messages, not tool results) + self._valid_cutoff_indices = self._find_valid_cutoff_indices(all_messages) + + # Store messages for summary generation (shallow โ€” only deep-copied + # if update_after_turn actually advances the checkpoint) + self._all_messages_for_summary = all_messages[:] + + # Apply checkpoint: skip old messages, prepend summary + checkpoint = self.compaction_state.checkpoint + stage = "none" + + if checkpoint > 0 and checkpoint < len(all_messages): + messages_to_process = all_messages[checkpoint:] + + summary = self.compaction_state.summary + if summary and messages_to_process: + messages_to_process = self._prepend_summary_to_first_message( + messages_to_process, summary + ) + stage = "checkpoint" + else: + messages_to_process = all_messages + + # Apply truncation (always when compaction enabled) + protected_indices = self._find_protected_indices( + messages_to_process, self.compaction_config.protected_turns + ) + truncated_messages, truncation_count, _ = self._truncate_tool_contents( + messages_to_process, protected_indices=protected_indices + ) + + if truncation_count > 0: + stage = "checkpoint+truncation" if stage == "checkpoint" else "truncation" + + agent.messages = truncated_messages + + logger.info( + f"Compaction initialized: stage={stage}, " + f"original={self._total_message_count_at_init}, " + f"final={len(agent.messages)}, " + f"truncations={truncation_count}" + ) + + # ========================================================================= + # Compaction State Persistence + # ========================================================================= + + def _get_dynamodb_table(self): + """Lazy initialization of DynamoDB table for compaction state.""" if TurnBasedSessionManager._dynamodb_table is None: - table_name = os.environ.get('DYNAMODB_SESSIONS_METADATA_TABLE_NAME') + table_name = os.environ.get("DYNAMODB_SESSIONS_METADATA_TABLE_NAME") if not table_name: - logger.warning("DYNAMODB_SESSIONS_METADATA_TABLE_NAME not configured, compaction state will not persist") + logger.warning( + "DYNAMODB_SESSIONS_METADATA_TABLE_NAME not configured, " + "compaction state will not persist" + ) return None import boto3 + TurnBasedSessionManager._dynamodb_table_name = table_name - dynamodb = boto3.resource('dynamodb', region_name=self.region_name) + dynamodb = boto3.resource("dynamodb", region_name=self.region_name) TurnBasedSessionManager._dynamodb_table = dynamodb.Table(table_name) logger.debug(f"Initialized DynamoDB table for compaction: {table_name}") return TurnBasedSessionManager._dynamodb_table def _get_session_via_gsi(self, table) -> Optional[Dict]: - """ - Look up session record using GSI (SessionLookupIndex). - - With the SK pattern S#ACTIVE#{last_message_at}#{session_id}, we can't - use get_item directly. The GSI allows lookup by session_id alone. - - Returns: - Session item dict including PK/SK if found, None otherwise - """ + """Look up session record using GSI (SessionLookupIndex).""" try: from boto3.dynamodb.conditions import Key response = table.query( - IndexName='SessionLookupIndex', + IndexName="SessionLookupIndex", KeyConditionExpression=( - Key('GSI_PK').eq(f'SESSION#{self.config.session_id}') & - Key('GSI_SK').eq('META') - ) + Key("GSI_PK").eq(f"SESSION#{self.config.session_id}") + & Key("GSI_SK").eq("META") + ), ) - items = response.get('Items', []) + items = response.get("Items", []) if not items: return None item = items[0] - - # Verify user ownership - if item.get('userId') != self.user_id: + if item.get("userId") != self.user_id: logger.warning(f"Session {self.config.session_id} belongs to different user") return None @@ -172,70 +287,29 @@ def _get_session_via_gsi(self, table) -> Optional[Dict]: logger.debug(f"GSI lookup failed: {e}") return None - def _initialize_message_count(self) -> int: - """ - Initialize message count by querying AgentCore Memory once at startup. - - Returns: - Initial message count (0 if session is new or if query fails) - """ - try: - messages = self.base_manager.list_messages( - self.config.session_id, - "default" # agent_id - ) - initial_count = len(messages) if messages else 0 - logger.info(f"๐Ÿ“Š Initialized message count from AgentCore Memory: {initial_count}") - return initial_count - except Exception as e: - logger.warning(f"Failed to initialize message count: {e}, defaulting to 0") - return 0 - - # ========================================================================= - # Compaction State Persistence - # ========================================================================= - def _load_compaction_state(self) -> CompactionState: - """ - Load compaction state from DynamoDB session metadata. - - The session record uses SK pattern: S#ACTIVE#{last_message_at}#{session_id} - We use the GSI (SessionLookupIndex) to find it by session_id alone. - """ - if not self.user_id: - logger.debug("_load_compaction_state: No user_id, returning default state") - return CompactionState() - if not self.compaction_config: - logger.debug("_load_compaction_state: No compaction_config, returning default state") - return CompactionState() - if not self.compaction_config.enabled: - logger.debug("_load_compaction_state: Compaction disabled, returning default state") + """Load compaction state from DynamoDB session metadata.""" + if not self.user_id or not self.compaction_config or not self.compaction_config.enabled: return CompactionState() try: table = self._get_dynamodb_table() if not table: - logger.warning("_load_compaction_state: No DynamoDB table available") return CompactionState() - # Look up session via GSI since we don't know the exact SK session_item = self._get_session_via_gsi(table) if not session_item: - logger.debug(f"_load_compaction_state: No session record found for session {self.config.session_id}") return CompactionState() - # Extract compaction state from session record - compaction_data = session_item.get('compaction') + compaction_data = session_item.get("compaction") if compaction_data: state = CompactionState.from_dict(compaction_data) logger.info( - f"๐Ÿ“ Loaded compaction state: checkpoint={state.checkpoint}, " + f"Loaded compaction state: checkpoint={state.checkpoint}, " f"summary_len={len(state.summary) if state.summary else 0}, " f"last_tokens={state.last_input_tokens}" ) return state - else: - logger.debug(f"_load_compaction_state: Session record found but no 'compaction' attribute") return CompactionState() @@ -244,11 +318,7 @@ def _load_compaction_state(self) -> CompactionState: return CompactionState() def _save_compaction_state(self, state: CompactionState) -> None: - """ - Save compaction state to DynamoDB session metadata. - - Uses GSI to find the session record, then updates it with the compaction state. - """ + """Save compaction state to DynamoDB session metadata.""" if not self.user_id or not self.compaction_config or not self.compaction_config.enabled: return @@ -257,28 +327,23 @@ def _save_compaction_state(self, state: CompactionState) -> None: if not table: return - # Look up session via GSI to get the actual PK/SK session_item = self._get_session_via_gsi(table) if not session_item: - logger.warning(f"Session record not found, cannot save compaction state") + logger.warning("Session record not found, cannot save compaction state") return - pk = session_item.get('PK') - sk = session_item.get('SK') + pk = session_item.get("PK") + sk = session_item.get("SK") if not pk or not sk: - logger.warning(f"Session record missing PK/SK, cannot save compaction state") return - # Update with compaction state state.updated_at = datetime.now(timezone.utc).isoformat() table.update_item( - Key={'PK': pk, 'SK': sk}, - UpdateExpression='SET compaction = :state', - ExpressionAttributeValues={ - ':state': state.to_dict() - } + Key={"PK": pk, "SK": sk}, + UpdateExpression="SET compaction = :state", + ExpressionAttributeValues={":state": state.to_dict()}, ) - logger.debug(f"๐Ÿ’พ Saved compaction state: checkpoint={state.checkpoint}") + logger.debug(f"Saved compaction state: checkpoint={state.checkpoint}") except Exception as e: logger.error(f"Error saving compaction state: {e}") @@ -292,21 +357,17 @@ def _get_summarization_strategy_id(self) -> Optional[str]: return self.summarization_strategy_id try: - # Try to discover from memory configuration - response = self.base_manager.memory_client.gmcp_client.get_memory( + response = self.memory_client.gmcp_client.get_memory( memoryId=self.config.memory_id ) - strategies = response.get('memory', {}).get('strategies', []) - + strategies = response.get("memory", {}).get("strategies", []) for strategy in strategies: - if strategy.get('type') == 'SUMMARIZATION': - strategy_id = strategy.get('strategyId', '') + if strategy.get("type") == "SUMMARIZATION": + strategy_id = strategy.get("strategyId", "") self.summarization_strategy_id = strategy_id logger.debug(f"Discovered SUMMARIZATION strategy: {strategy_id}") return strategy_id - return None - except Exception as e: logger.warning(f"Failed to get SUMMARIZATION strategy ID: {e}") return None @@ -326,16 +387,15 @@ def _retrieve_session_summaries(self) -> List[str]: f"/sessions/{self.config.session_id}" ) - client = boto3.client('bedrock-agentcore', region_name=self.region_name) + client = boto3.client("bedrock-agentcore", region_name=self.region_name) response = client.list_memory_records( memoryId=self.config.memory_id, namespace=namespace, - maxResults=100 + maxResults=100, ) - records = response.get('memoryRecordSummaries', []) + records = response.get("memoryRecordSummaries", []) summaries = [] - for record in records: content = record.get("content", {}) if isinstance(content, dict): @@ -344,8 +404,7 @@ def _retrieve_session_summaries(self) -> List[str]: summaries.append(text) if summaries: - logger.info(f"๐Ÿ“‹ Retrieved {len(summaries)} summaries from LTM") - + logger.info(f"Retrieved {len(summaries)} summaries from LTM") return summaries except Exception as e: @@ -353,59 +412,161 @@ def _retrieve_session_summaries(self) -> List[str]: return [] def _generate_fallback_summary(self, messages: List[Dict]) -> Optional[str]: - """Generate a fallback summary when LTM summaries unavailable.""" + """Generate a fallback summary when LTM summaries are unavailable.""" if not messages: return None try: key_points = [] + tools_used = set() for msg in messages: - role = msg.get('role', '') - content = msg.get('content', []) + role = msg.get("role", "") + content = msg.get("content", []) + if not isinstance(content, list): + continue - if role == 'user' and isinstance(content, list): + if role == "user": for block in content: - if isinstance(block, dict) and 'text' in block: - text = block['text'] - # Skip tool results - if 'toolResult' not in block: - first_line = text.split('\n')[0][:100] - if first_line and not first_line.startswith('<'): - key_points.append(f"- User asked about: {first_line}") + if isinstance(block, dict) and "text" in block: + text = block["text"] + first_line = text.split("\n")[0][:150] + if first_line and not first_line.startswith("<"): + key_points.append(f"- User: {first_line}") break + elif role == "assistant": + for block in content: + if isinstance(block, dict): + if "toolUse" in block: + tool_name = block["toolUse"].get("name", "") + if tool_name: + tools_used.add(tool_name) + elif "text" in block: + text = block["text"] + first_line = text.split("\n")[0][:150] + if first_line and not first_line.startswith("<"): + key_points.append(f"- Assistant: {first_line}") + break if key_points: - summary = "Previous conversation topics:\n" + "\n".join(key_points[-10:]) - logger.debug(f"๐Ÿ“ Generated fallback summary with {len(key_points)} points") - return summary + parts = ["Previous conversation:"] + parts.append("\n".join(key_points[-15:])) + if tools_used: + parts.append(f"\nTools used: {', '.join(sorted(tools_used))}") + return "\n".join(parts) except Exception as e: logger.warning(f"Failed to generate fallback summary: {e}") return None + # ========================================================================= + # Post-Turn Compaction (Stage 2) + # ========================================================================= + + async def update_after_turn(self, input_tokens: int) -> None: + """ + Update compaction state after a turn completes. + + Called by StreamCoordinator with input token count from model response. + Triggers checkpoint creation when token threshold exceeded. + """ + if not self.compaction_config or not self.compaction_config.enabled: + return + + if self.compaction_state is None: + self.compaction_state = CompactionState() + + self.compaction_state.last_input_tokens = input_tokens + + if input_tokens <= self.compaction_config.token_threshold: + self._save_compaction_state(self.compaction_state) + return + + logger.info( + f"Threshold exceeded: {input_tokens:,} > " + f"{self.compaction_config.token_threshold:,}" + ) + + if not self._valid_cutoff_indices: + logger.info("No valid cutoff points cached, skipping checkpoint update") + self._save_compaction_state(self.compaction_state) + return + + total_turns = len(self._valid_cutoff_indices) + protected_turns = self.compaction_config.protected_turns + + if total_turns <= protected_turns: + logger.debug( + f"Only {total_turns} turns available (need > {protected_turns}), " + f"keeping all messages" + ) + self._save_compaction_state(self.compaction_state) + return + + new_checkpoint = self._valid_cutoff_indices[-protected_turns] + current_checkpoint = self.compaction_state.checkpoint + + if new_checkpoint <= current_checkpoint: + self._save_compaction_state(self.compaction_state) + return + + logger.info(f"Checkpoint update: {current_checkpoint} -> {new_checkpoint}") + + # Retrieve or generate summary for compacted messages + summaries = self._retrieve_session_summaries() + if summaries: + summary = "\n\n".join(summaries) + else: + messages_to_summarize = self._all_messages_for_summary[:new_checkpoint] + summary = self._generate_fallback_summary(messages_to_summarize) + + self.compaction_state.checkpoint = new_checkpoint + self.compaction_state.summary = summary + self._save_compaction_state(self.compaction_state) + + logger.info( + f"Compaction checkpoint set: {new_checkpoint}, " + f"summary_length={len(summary) if summary else 0}" + ) + # ========================================================================= # Message Processing Helpers # ========================================================================= + @staticmethod + def _filter_empty_text(message: dict) -> dict: + """Filter out empty or invalid content blocks from a message.""" + if "content" not in message: + return message + content = message.get("content", []) + if not isinstance(content, list): + return message + + def is_valid_block(block): + if not isinstance(block, dict): + return False + if "text" in block: + text = block.get("text", "") + return isinstance(text, str) and text.strip() != "" + return any(key in block for key in ("toolUse", "toolResult", "image", "document")) + + filtered = [block for block in content if is_valid_block(block)] + return {**message, "content": filtered} + def _has_tool_result(self, message: Dict) -> bool: """Check if message contains toolResult block.""" - content = message.get('content', []) + content = message.get("content", []) if isinstance(content, list): for block in content: - if isinstance(block, dict) and 'toolResult' in block: + if isinstance(block, dict) and "toolResult" in block: return True return False def _find_valid_cutoff_indices(self, messages: List[Dict]) -> List[int]: - """ - Find valid cutoff points (user message indices that start turns). - - A valid cutoff is a user message that is NOT a tool result. - """ + """Find valid cutoff points (user text message indices, not tool results).""" valid_indices = [] for i, msg in enumerate(messages): - if msg.get('role') == 'user' and not self._has_tool_result(msg): + if msg.get("role") == "user" and not self._has_tool_result(msg): valid_indices.append(i) return valid_indices @@ -415,13 +576,11 @@ def _find_protected_indices(self, messages: List[Dict], protected_turns: int) -> return set() turn_start_indices = self._find_valid_cutoff_indices(messages) - if not turn_start_indices: return set() turns_to_protect = min(protected_turns, len(turn_start_indices)) protected_start_idx = turn_start_indices[-turns_to_protect] - return set(range(protected_start_idx, len(messages))) # ========================================================================= @@ -437,7 +596,7 @@ def _truncate_text(self, text: str, max_length: int) -> str: def _truncate_tool_contents( self, messages: List[Dict], - protected_indices: Optional[set] = None + protected_indices: Optional[set] = None, ) -> tuple: """ Stage 1 Compaction: Truncate long tool inputs/results and replace images. @@ -460,7 +619,7 @@ def _truncate_tool_contents( if msg_idx in protected_indices: continue - content = msg.get('content', []) + content = msg.get("content", []) if not isinstance(content, list): continue @@ -468,86 +627,83 @@ def _truncate_tool_contents( if not isinstance(block, dict): continue - # Handle image blocks - replace with placeholder - if 'image' in block: - image_data = block['image'] - image_format = image_data.get('format', 'unknown') - source = image_data.get('source', {}) - original_bytes = source.get('bytes', b'') + # Replace image blocks with placeholder + if "image" in block: + image_data = block["image"] + image_format = image_data.get("format", "unknown") + source = image_data.get("source", {}) + original_bytes = source.get("bytes", b"") original_size = len(original_bytes) if isinstance(original_bytes, bytes) else 0 content[block_idx] = { - 'text': f'[Image placeholder: format={image_format}, original_size={original_size} bytes]' + "text": f"[Image placeholder: format={image_format}, original_size={original_size} bytes]" } truncation_count += 1 total_chars_saved += original_size - # Handle toolUse input - elif 'toolUse' in block: - tool_use = block['toolUse'] - tool_input = tool_use.get('input', {}) + # Truncate toolUse input + elif "toolUse" in block: + tool_use = block["toolUse"] + tool_input = tool_use.get("input", {}) if isinstance(tool_input, dict): input_str = json.dumps(tool_input, ensure_ascii=False) if len(input_str) > max_len: original_len = len(input_str) - tool_use['input'] = { - "_truncated": self._truncate_text(input_str, max_len) - } + tool_use["input"] = {"_truncated": self._truncate_text(input_str, max_len)} truncation_count += 1 total_chars_saved += original_len - max_len elif isinstance(tool_input, str) and len(tool_input) > max_len: original_len = len(tool_input) - tool_use['input'] = self._truncate_text(tool_input, max_len) + tool_use["input"] = self._truncate_text(tool_input, max_len) truncation_count += 1 total_chars_saved += original_len - max_len - # Handle toolResult content - elif 'toolResult' in block: - tool_result = block['toolResult'] - result_content = tool_result.get('content', []) + # Truncate toolResult content + elif "toolResult" in block: + tool_result = block["toolResult"] + result_content = tool_result.get("content", []) if isinstance(result_content, list): for result_idx, result_block in enumerate(result_content): if not isinstance(result_block, dict): continue - # Replace images with placeholder - if 'image' in result_block: - image_data = result_block['image'] - image_format = image_data.get('format', 'unknown') - source = image_data.get('source', {}) - original_bytes = source.get('bytes', b'') - original_size = len(original_bytes) if isinstance(original_bytes, bytes) else 0 + if "image" in result_block: + image_data = result_block["image"] + image_format = image_data.get("format", "unknown") + source = image_data.get("source", {}) + original_bytes = source.get("bytes", b"") + original_size = ( + len(original_bytes) if isinstance(original_bytes, bytes) else 0 + ) result_content[result_idx] = { - 'text': f'[Image placeholder: format={image_format}, original_size={original_size} bytes]' + "text": f"[Image placeholder: format={image_format}, original_size={original_size} bytes]" } truncation_count += 1 total_chars_saved += original_size - # Truncate text - elif 'text' in result_block: - text = result_block['text'] + elif "text" in result_block: + text = result_block["text"] if len(text) > max_len: original_len = len(text) - result_block['text'] = self._truncate_text(text, max_len) + result_block["text"] = self._truncate_text(text, max_len) truncation_count += 1 total_chars_saved += original_len - max_len - # Truncate JSON - elif 'json' in result_block: - json_content = result_block['json'] + elif "json" in result_block: + json_content = result_block["json"] json_str = json.dumps(json_content, ensure_ascii=False) if len(json_str) > max_len: original_len = len(json_str) - result_block.pop('json') - result_block['text'] = self._truncate_text(json_str, max_len) + result_block.pop("json") + result_block["text"] = self._truncate_text(json_str, max_len) truncation_count += 1 total_chars_saved += original_len - max_len if truncation_count > 0: - logger.info(f"โœ‚๏ธ Truncated {truncation_count} items, saved ~{total_chars_saved:,} chars") + logger.info(f"Truncated {truncation_count} items, saved ~{total_chars_saved:,} chars") return modified_messages, truncation_count, total_chars_saved @@ -558,7 +714,7 @@ def _truncate_tool_contents( def _prepend_summary_to_first_message( self, messages: List[Dict], - summary: str + summary: str, ) -> List[Dict]: """Prepend summary to the first user message's text content.""" if not messages or not summary: @@ -567,7 +723,7 @@ def _prepend_summary_to_first_message( modified_messages = copy.deepcopy(messages) first_msg = modified_messages[0] - if first_msg.get('role') != 'user': + if first_msg.get("role") != "user": return messages summary_prefix = ( @@ -578,323 +734,25 @@ def _prepend_summary_to_first_message( "\n\n" ) - content = first_msg.get('content', []) + content = first_msg.get("content", []) if isinstance(content, list) and len(content) > 0: for block in content: - if isinstance(block, dict) and 'text' in block: - block['text'] = summary_prefix + block['text'] + if isinstance(block, dict) and "text" in block: + block["text"] = summary_prefix + block["text"] return modified_messages # No text block found, insert one - content.insert(0, {'text': summary_prefix.rstrip()}) - first_msg['content'] = content + content.insert(0, {"text": summary_prefix.rstrip()}) + first_msg["content"] = content return modified_messages # ========================================================================= - # Initialization with Compaction - # ========================================================================= - - def initialize(self, agent: "Agent") -> None: - """ - Initialize agent with two-feature compaction. - - This method: - 1. Delegates to base manager for basic initialization - 2. Loads compaction state from DynamoDB - 3. Applies checkpoint (skips old messages, prepends summary) - 4. Applies truncation to tool contents - """ - # First, let base manager do its initialization - self.base_manager.initialize(agent) - - # If compaction is disabled, we're done - if not self.compaction_config or not self.compaction_config.enabled: - return - - # Get messages that base manager loaded - all_messages = agent.messages or [] - self._total_message_count_at_init = len(all_messages) - - if not all_messages: - self.compaction_state = CompactionState() - self._valid_cutoff_indices = [] - self._all_messages_for_summary = [] - return - - # Cache for checkpoint calculation later - self._all_messages_for_summary = [copy.deepcopy(m) for m in all_messages] - self._valid_cutoff_indices = self._find_valid_cutoff_indices(all_messages) - - # Load compaction state - self.compaction_state = self._load_compaction_state() - checkpoint = self.compaction_state.checkpoint - summary = self.compaction_state.summary - - logger.info( - f"๐Ÿ“ Compaction init: checkpoint={checkpoint}, " - f"total_messages={len(all_messages)}, " - f"has_summary={summary is not None}" - ) - - # Track compaction stage for logging - stage = "none" - messages_to_process = all_messages - - # Apply checkpoint if set - if checkpoint > 0 and checkpoint < len(all_messages): - messages_to_process = all_messages[checkpoint:] - logger.info( - f"๐Ÿ“ Checkpoint applied: loading {len(messages_to_process)} " - f"of {len(all_messages)} messages (checkpoint={checkpoint})" - ) - - # Prepend summary if available - if summary and messages_to_process: - messages_to_process = self._prepend_summary_to_first_message( - messages_to_process, summary - ) - logger.info(f"๐Ÿ“‹ Prepended summary ({len(summary)} chars)") - - stage = "checkpoint" - - # Apply truncation (always when compaction enabled) - protected_indices = self._find_protected_indices( - messages_to_process, - self.compaction_config.protected_turns - ) - - truncated_messages, truncation_count, _ = self._truncate_tool_contents( - messages_to_process, - protected_indices=protected_indices - ) - - if truncation_count > 0: - stage = "checkpoint+truncation" if stage == "checkpoint" else "truncation" - - # Set processed messages on agent - agent.messages = truncated_messages - - logger.info( - f"โœ… Compaction initialized: stage={stage}, " - f"original={self._total_message_count_at_init}, " - f"final={len(truncated_messages)}, " - f"truncations={truncation_count}" - ) - - # ========================================================================= - # Post-Turn Update (Stage 2 Compaction Trigger) - # ========================================================================= - - async def update_after_turn(self, input_tokens: int) -> None: - """ - Update compaction state after a turn completes. - - Called by StreamCoordinator with input token count from model response. - Triggers checkpoint creation when token threshold exceeded. - - Args: - input_tokens: Total input token count from this turn - """ - if not self.compaction_config or not self.compaction_config.enabled: - return - - if self.compaction_state is None: - self.compaction_state = CompactionState() - - # Always update token count - self.compaction_state.last_input_tokens = input_tokens - - # Check if threshold exceeded - if input_tokens <= self.compaction_config.token_threshold: - self._save_compaction_state(self.compaction_state) - return - - logger.info( - f"๐Ÿ” Threshold exceeded: {input_tokens:,} > " - f"{self.compaction_config.token_threshold:,}" - ) - - # Fetch fresh messages from AgentCore Memory - # The cached indices from initialize() are stale - new messages were added - try: - raw_messages = self.base_manager.list_messages( - self.config.session_id, - "default" # agent_id - ) - if not raw_messages: - logger.info("โš ๏ธ No messages in session, skipping checkpoint") - self._save_compaction_state(self.compaction_state) - return - - # Convert SessionMessage objects to dicts - all_messages = [] - for msg in raw_messages: - if hasattr(msg, 'message'): - # Wrapped format: {'message': {...}, 'message_id': ...} - msg_data = msg.message if hasattr(msg.message, '__dict__') else msg.message - if hasattr(msg_data, 'model_dump'): - all_messages.append(msg_data.model_dump()) - elif isinstance(msg_data, dict): - all_messages.append(msg_data) - elif hasattr(msg, 'model_dump'): - all_messages.append(msg.model_dump()) - elif isinstance(msg, dict): - all_messages.append(msg) - - logger.info(f"๐Ÿ“ฅ Fetched {len(all_messages)} messages for compaction calculation") - - except Exception as e: - logger.warning(f"Failed to fetch messages for compaction: {e}") - self._save_compaction_state(self.compaction_state) - return - - # Recalculate valid cutoff indices from fresh messages - valid_cutoff_indices = self._find_valid_cutoff_indices(all_messages) - - if not valid_cutoff_indices: - logger.info("โš ๏ธ No valid cutoff points found in messages") - self._save_compaction_state(self.compaction_state) - return - - total_turns = len(valid_cutoff_indices) - protected_turns = self.compaction_config.protected_turns - - logger.info(f"๐Ÿ“Š Found {total_turns} turns, protecting last {protected_turns}") - - if total_turns <= protected_turns: - logger.info( - f"โš ๏ธ Only {total_turns} turns (need > {protected_turns}), " - f"keeping all messages" - ) - self._save_compaction_state(self.compaction_state) - return - - # Calculate new checkpoint at the oldest protected turn boundary - new_checkpoint = valid_cutoff_indices[-protected_turns] - current_checkpoint = self.compaction_state.checkpoint - - if new_checkpoint <= current_checkpoint: - logger.debug(f"Checkpoint unchanged: {current_checkpoint}") - self._save_compaction_state(self.compaction_state) - return - - logger.info( - f"๐Ÿ”„ Checkpoint update: {current_checkpoint} โ†’ {new_checkpoint}" - ) - - # Update cached data for summary generation - self._valid_cutoff_indices = valid_cutoff_indices - self._all_messages_for_summary = all_messages - - # Generate summary for compacted messages - messages_to_summarize = ( - self._all_messages_for_summary[:new_checkpoint] - if self._all_messages_for_summary else [] - ) - - # Try LTM summaries first, fall back to simple summary - summaries = self._retrieve_session_summaries() - if summaries: - summary = "\n\n".join(summaries) - else: - summary = self._generate_fallback_summary(messages_to_summarize) - - # Update state - self.compaction_state.checkpoint = new_checkpoint - self.compaction_state.summary = summary - - # Persist - self._save_compaction_state(self.compaction_state) - - logger.info( - f"โœ… Compaction checkpoint set: {new_checkpoint}, " - f"summary_length={len(summary) if summary else 0}" - ) - - # ========================================================================= - # Session Manager Interface + # Convenience โ€” flush is a no-op (SDK handles persistence via hooks) # ========================================================================= def flush(self) -> Optional[int]: - """ - Flush is now a no-op since we don't buffer messages. - - Returns: - Message ID of the last message, or None if no messages - """ + """Return the last message index. SDK handles actual persistence via hooks.""" if self.message_count > 0: return self.message_count - 1 return None - - def append_message(self, message, agent, **kwargs): - """ - Pass message through to base manager and track message count. - - Args: - message: Message from Strands framework - agent: Agent instance - **kwargs: Additional arguments - """ - if self.cancelled: - logger.warning(f"๐Ÿšซ Session cancelled, ignoring message") - return - - # Delegate to base manager - self.base_manager.append_message(message, agent, **kwargs) - - # Track message count - self.message_count += 1 - - role = message.get("role", "unknown") - logger.debug(f"๐Ÿ“ Message persisted (role={role}, count={self.message_count})") - - def register_hooks(self, registry, **kwargs): - """ - Register hooks with the Strands Agent framework. - - CRITICAL: This method MUST be defined here to prevent the base manager - from registering its own hooks. We register OUR methods as callbacks. - """ - from strands.hooks import ( - AgentInitializedEvent, - MessageAddedEvent, - AfterInvocationEvent - ) - - logger.info("๐Ÿ”— Registering hooks (with compaction support)") - - # Register initialization hook - use OUR initialize (with compaction) - registry.add_callback( - AgentInitializedEvent, - lambda event: self.initialize(event.agent) - ) - - # Register message added hook - use OUR append_message - registry.add_callback( - MessageAddedEvent, - lambda event: self.append_message(event.message, event.agent) - ) - - # Register sync hooks - delegate to base manager - registry.add_callback( - MessageAddedEvent, - lambda event: self.base_manager.sync_agent(event.agent) - ) - - registry.add_callback( - AfterInvocationEvent, - lambda event: self.base_manager.sync_agent(event.agent) - ) - - # Register LTM retrieval hook - registry.add_callback( - MessageAddedEvent, - lambda event: self.base_manager.retrieve_customer_context(event) - ) - - logger.info("โœ… Hooks registered (including LTM retrieval)") - - def __getattr__(self, name): - """Delegate unknown methods to base AgentCore session manager.""" - return getattr(self.base_manager, name) diff --git a/backend/src/agents/main_agent/tools/oauth_tool_service.py b/backend/src/agents/main_agent/tools/oauth_tool_service.py index 0c06f834..0e6e5ab5 100644 --- a/backend/src/agents/main_agent/tools/oauth_tool_service.py +++ b/backend/src/agents/main_agent/tools/oauth_tool_service.py @@ -151,20 +151,10 @@ async def get_token_for_tool( provider = await provider_repo.get_provider(provider_id) provider_name = provider.display_name if provider else provider_id - # Try to get the token - token = await oauth_service.get_decrypted_token( - user_id=user_id, - provider_id=provider_id - ) - - if token: - logger.info(f"Retrieved OAuth token for user {user_id}, provider {provider_id}") - return OAuthTokenResult( - connected=True, - access_token=token, - provider_id=provider_id, - provider_name=provider_name, - ) + # Try to get the token โ€” isolated to limit taint scope + result = await self._try_get_token(oauth_service, user_id, provider_id, provider_name) + if result: + return result # Check if user has a connection but needs re-auth from apis.shared.oauth.token_repository import get_token_repository @@ -172,17 +162,18 @@ async def get_token_for_tool( user_token = await token_repo.get_user_token(user_id, provider_id) if user_token and user_token.status in ("expired", "needs_reauth", "revoked"): - logger.info(f"User {user_id} needs re-auth for provider {provider_id}") + token_status = user_token.status + logger.debug("User needs re-auth for an OAuth provider") return OAuthTokenResult( connected=False, provider_id=provider_id, provider_name=provider_name, needs_reauth=True, - error=f"Token {user_token.status}", + error=f"Token {token_status}", ) # User not connected - logger.info(f"User {user_id} not connected to provider {provider_id}") + logger.debug("User not connected to an OAuth provider") return OAuthTokenResult( connected=False, provider_id=provider_id, @@ -190,7 +181,7 @@ async def get_token_for_tool( ) except Exception as e: - logger.error(f"Error getting OAuth token: {e}", exc_info=True) + logger.error("Error checking OAuth connection: %s", type(e).__name__, exc_info=True) return OAuthTokenResult( connected=False, provider_id=provider_id, @@ -198,6 +189,28 @@ async def get_token_for_tool( error=str(e), ) + @staticmethod + async def _try_get_token( + oauth_service: "OAuthService", + user_id: str, + provider_id: str, + provider_name: str, + ) -> Optional[OAuthTokenResult]: + """Fetch decrypted token in isolated scope to avoid taint leaking to callers.""" + token = await oauth_service.get_decrypted_token( + user_id=user_id, + provider_id=provider_id, + ) + if token: + logger.debug("OAuth token successfully retrieved") + return OAuthTokenResult( + connected=True, + access_token=token, + provider_id=provider_id, + provider_name=provider_name, + ) + return None + async def check_connection( self, user_id: str, diff --git a/backend/src/apis/app_api/fine_tuning/job_models.py b/backend/src/apis/app_api/fine_tuning/job_models.py index ca3473ed..a80b84d2 100644 --- a/backend/src/apis/app_api/fine_tuning/job_models.py +++ b/backend/src/apis/app_api/fine_tuning/job_models.py @@ -243,6 +243,7 @@ class CreateJobRequest(BaseModel): instance_type: Optional[str] = None hyperparameters: Optional[Dict[str, str]] = None max_runtime_seconds: int = Field(default=86400, le=432000, gt=0) + custom_huggingface_model_id: Optional[str] = None class JobResponse(BaseModel): diff --git a/backend/src/apis/app_api/fine_tuning/routes.py b/backend/src/apis/app_api/fine_tuning/routes.py index 9447909e..87dfe87c 100644 --- a/backend/src/apis/app_api/fine_tuning/routes.py +++ b/backend/src/apis/app_api/fine_tuning/routes.py @@ -7,7 +7,8 @@ from datetime import datetime, timezone, timedelta from typing import Optional -from fastapi import APIRouter, Depends, HTTPException, status +import httpx +from fastapi import APIRouter, Depends, HTTPException, Query, status from apis.shared.auth import User from apis.shared.auth.dependencies import get_current_user @@ -83,6 +84,101 @@ async def list_models( return [m.model_dump() for m in AVAILABLE_MODELS] +# ========================================================================= +# HuggingFace Model Search (proxy) +# ========================================================================= + +# Pipeline tags compatible with AutoModelForSequenceClassification +COMPATIBLE_PIPELINE_TAGS = [ + "fill-mask", + "text-classification", + "feature-extraction", + "token-classification", + "text-generation", +] + + +@router.get("/huggingface-models") +async def search_huggingface_models( + search: str = Query(..., min_length=2, max_length=200), + compatible_only: bool = Query(True), + grant: dict = Depends(require_fine_tuning_access), +): + """Search HuggingFace Hub models. Proxied to avoid CORS issues. + + When compatible_only=True (default), makes parallel requests for each + compatible pipeline_tag and merges results sorted by downloads. + """ + try: + async with httpx.AsyncClient(timeout=10.0) as client: + if compatible_only: + # Query each compatible pipeline_tag in parallel + import asyncio + + async def _fetch_tag(tag: str): + resp = await client.get( + "https://huggingface.co/api/models", + params={ + "search": search, + "pipeline_tag": tag, + "library": "transformers", + "limit": 5, + "sort": "downloads", + "direction": "-1", + }, + ) + resp.raise_for_status() + return resp.json() + + results = await asyncio.gather( + *[_fetch_tag(tag) for tag in COMPATIBLE_PIPELINE_TAGS], + return_exceptions=True, + ) + + # Merge, deduplicate, and sort by downloads + seen = set() + models = [] + for result in results: + if isinstance(result, Exception): + continue + for m in result: + mid = m.get("id", "") + if mid and mid not in seen: + seen.add(mid) + models.append(m) + models.sort(key=lambda m: m.get("downloads", 0), reverse=True) + models = models[:15] + else: + response = await client.get( + "https://huggingface.co/api/models", + params={ + "search": search, + "limit": 15, + "sort": "downloads", + "direction": "-1", + }, + ) + response.raise_for_status() + models = response.json() + + return [ + { + "id": m.get("id", ""), + "downloads": m.get("downloads", 0), + "likes": m.get("likes", 0), + "pipeline_tag": m.get("pipeline_tag"), + "library_name": m.get("library_name"), + "author": m.get("author"), + "model_type": (m.get("config") or {}).get("model_type"), + } + for m in models + if m.get("id") + ] + except httpx.HTTPError as e: + logger.warning(f"HuggingFace API search failed: {e}") + raise HTTPException(status_code=502, detail="Failed to search HuggingFace models") + + # ========================================================================= # Presigned URL # ========================================================================= @@ -131,11 +227,17 @@ async def create_job( script_service: ScriptPackagingService = Depends(get_script_packaging_service), ): """Create a new fine-tuning training job.""" - # Validate model + # Validate model โ€” either from catalog or custom HuggingFace model model = MODEL_CATALOG.get(request.model_id) - if not model: + if not model and not request.custom_huggingface_model_id: raise HTTPException(status_code=400, detail=f"Unknown model_id: {request.model_id}") + if request.custom_huggingface_model_id: + # Validate the custom HuggingFace model ID format (org/model or just model) + hf_id = request.custom_huggingface_model_id.strip() + if not hf_id or len(hf_id) > 200: + raise HTTPException(status_code=400, detail="Invalid HuggingFace model ID.") + # Verify dataset exists in S3 if not s3_service.check_object_exists(request.dataset_s3_key): raise HTTPException(status_code=400, detail="Dataset not found in S3. Upload your dataset first.") @@ -149,11 +251,29 @@ async def create_job( ) # Resolve instance type and hyperparameters - instance_type = request.instance_type or model.default_instance_type - hyperparameters = {**model.default_hyperparameters} + if model: + instance_type = request.instance_type or model.default_instance_type + hyperparameters = {**model.default_hyperparameters} + model_name = model.model_name + huggingface_id = model.huggingface_model_id + else: + # Custom HuggingFace model โ€” use sensible defaults + instance_type = request.instance_type or "ml.g5.xlarge" + hyperparameters = { + "epochs": "3", + "per_device_train_batch_size": "8", + "learning_rate": "2e-5", + "weight_decay": "0.01", + "split_ratio": "0.8", + "seed": "42", + "context_length": "512", + } + huggingface_id = request.custom_huggingface_model_id.strip() + model_name = huggingface_id + if request.hyperparameters: hyperparameters.update(request.hyperparameters) - hyperparameters["model_name_or_path"] = model.huggingface_model_id + hyperparameters["model_name_or_path"] = huggingface_id # Generate identifiers job_id = uuid.uuid4().hex @@ -182,7 +302,7 @@ async def create_job( email=user.email, job_id=job_id, model_id=request.model_id, - model_name=model.model_name, + model_name=model_name, dataset_s3_key=request.dataset_s3_key, instance_type=instance_type, hyperparameters=hyperparameters, diff --git a/backend/src/apis/app_api/main.py b/backend/src/apis/app_api/main.py index 8b3a2f61..97f6efc3 100644 --- a/backend/src/apis/app_api/main.py +++ b/backend/src/apis/app_api/main.py @@ -113,6 +113,7 @@ async def lifespan(app: FastAPI): from apis.app_api.users.routes import router as users_router from apis.app_api.user_settings.routes import router as user_settings_router from apis.shared.oauth.routes import router as oauth_router +from apis.app_api.shares.routes import conversations_share_router, shares_router, shared_view_router # Include routers app.include_router(health_router) @@ -132,6 +133,9 @@ async def lifespan(app: FastAPI): app.include_router(tools_router) # Tool discovery and permissions app.include_router(files_router) # File upload via pre-signed URLs app.include_router(oauth_router) # OAuth provider connections +app.include_router(conversations_share_router) # Share conversations endpoints +app.include_router(shares_router) # Share management (update, revoke, export) +app.include_router(shared_view_router) # Shared conversation read-only view # Conditionally register fine-tuning routes if os.environ.get("FINE_TUNING_ENABLED", "false").lower() == "true": diff --git a/backend/src/apis/app_api/sessions/services/metadata.py b/backend/src/apis/app_api/sessions/services/metadata.py index cf1806c5..c180db83 100644 --- a/backend/src/apis/app_api/sessions/services/metadata.py +++ b/backend/src/apis/app_api/sessions/services/metadata.py @@ -988,9 +988,14 @@ async def _list_user_sessions_cloud( # No limit trimming needed either - we use exact Limit in query params - # Generate next_token from LastEvaluatedKey if present + # Generate next_token from LastEvaluatedKey if present. + # Only return a next_token when we actually filled the requested page. + # DynamoDB's Limit caps items *evaluated*, so filtered-out items + # (parse failures) can cause fewer results than the limit while still + # producing a LastEvaluatedKey. Returning that token to the client + # makes it show a "Load More" button with nothing left to load. next_page_token = None - if 'LastEvaluatedKey' in response: + if 'LastEvaluatedKey' in response and (not limit or len(sessions) >= limit): next_page_token = base64.b64encode( json.dumps(response['LastEvaluatedKey']).encode('utf-8') ).decode('utf-8') diff --git a/backend/src/apis/app_api/shares/__init__.py b/backend/src/apis/app_api/shares/__init__.py new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/backend/src/apis/app_api/shares/__init__.py @@ -0,0 +1 @@ + diff --git a/backend/src/apis/app_api/shares/models.py b/backend/src/apis/app_api/shares/models.py new file mode 100644 index 00000000..423ebf3a --- /dev/null +++ b/backend/src/apis/app_api/shares/models.py @@ -0,0 +1,104 @@ +"""Share API request/response models + +This module contains all share-related data models including: +- CreateShareRequest / UpdateShareRequest for share operations +- ShareResponse for share metadata +- SharedConversationResponse for full shared conversation data +""" + +from typing import List, Literal, Optional + +from pydantic import BaseModel, ConfigDict, Field, model_validator + +from apis.shared.sessions.models import MessageResponse + + +class CreateShareRequest(BaseModel): + """Request body for creating a share""" + + model_config = ConfigDict(populate_by_name=True) + + access_level: Literal["public", "specific"] = Field( + ..., alias="accessLevel", description="Access level for the share" + ) + allowed_emails: Optional[List[str]] = Field( + default=None, + alias="allowedEmails", + description="Email addresses allowed to view (required when accessLevel is 'specific')", + ) + + @model_validator(mode="after") + def validate_allowed_emails(self) -> "CreateShareRequest": + if self.access_level == "specific" and ( + not self.allowed_emails or len(self.allowed_emails) == 0 + ): + raise ValueError( + "allowed_emails is required when access_level is 'specific'" + ) + return self + + +class UpdateShareRequest(BaseModel): + """Request body for updating share settings""" + + model_config = ConfigDict(populate_by_name=True) + + access_level: Optional[Literal["public", "specific"]] = Field( + default=None, alias="accessLevel", description="New access level for the share" + ) + allowed_emails: Optional[List[str]] = Field( + default=None, + alias="allowedEmails", + description="Updated email addresses allowed to view", + ) + + @model_validator(mode="after") + def validate_allowed_emails(self) -> "UpdateShareRequest": + if self.access_level == "specific" and ( + not self.allowed_emails or len(self.allowed_emails) == 0 + ): + raise ValueError( + "allowed_emails is required when access_level is 'specific'" + ) + return self + + +class ShareResponse(BaseModel): + """Response model for share operations""" + + model_config = ConfigDict(populate_by_name=True) + + share_id: str = Field(..., alias="shareId", description="Unique share identifier") + session_id: str = Field(..., alias="sessionId", description="Original session identifier") + owner_id: str = Field(..., alias="ownerId", description="User ID of the share creator") + access_level: Literal["public", "specific"] = Field( + ..., alias="accessLevel", description="Access level for the share" + ) + allowed_emails: Optional[List[str]] = Field( + default=None, alias="allowedEmails", description="Allowed email addresses" + ) + created_at: str = Field(..., alias="createdAt", description="ISO 8601 timestamp of share creation") + share_url: str = Field(..., alias="shareUrl", description="Shareable URL for the conversation") + + +class ShareListResponse(BaseModel): + """Response model for listing all shares for a session""" + + model_config = ConfigDict(populate_by_name=True) + + shares: List[ShareResponse] = Field(..., description="List of shares for the session") + + +class SharedConversationResponse(BaseModel): + """Response model for retrieving a shared conversation""" + + model_config = ConfigDict(populate_by_name=True) + + share_id: str = Field(..., alias="shareId", description="Unique share identifier") + title: str = Field(..., description="Conversation title") + access_level: Literal["public", "specific"] = Field( + ..., alias="accessLevel", description="Access level for the share" + ) + created_at: str = Field(..., alias="createdAt", description="ISO 8601 timestamp of share creation") + owner_id: str = Field(..., alias="ownerId", description="User ID of the share creator") + messages: List[MessageResponse] = Field(..., description="Snapshot of conversation messages") diff --git a/backend/src/apis/app_api/shares/routes.py b/backend/src/apis/app_api/shares/routes.py new file mode 100644 index 00000000..2613744c --- /dev/null +++ b/backend/src/apis/app_api/shares/routes.py @@ -0,0 +1,207 @@ +"""Share API routes + +Provides endpoints for sharing conversations via shareable URLs. +Three routers: + - conversations_share_router: mounted at /conversations (create, list shares) + - shares_router: mounted at /shares (update, revoke, export individual shares) + - shared_view_router: mounted at /shared (read-only retrieval) +""" + +import logging + +from fastapi import APIRouter, Depends, HTTPException, Response + +from apis.shared.auth.dependencies import get_current_user +from apis.shared.auth.models import User + +from .models import ( + CreateShareRequest, + ShareListResponse, + ShareResponse, + SharedConversationResponse, + UpdateShareRequest, +) +from .service import ( + AccessDeniedError, + NotOwnerError, + SessionNotFoundError, + ShareNotFoundError, + ShareTableNotFoundError, + get_share_service, +) + +logger = logging.getLogger(__name__) + +# Router for /conversations/{session_id}/share endpoints +conversations_share_router = APIRouter(prefix="/conversations", tags=["shares"]) + +# Router for /shares/{share_id} endpoints (update, revoke, export) +shares_router = APIRouter(prefix="/shares", tags=["shares"]) + +# Router for /shared/{share_id} endpoint (read-only view) +shared_view_router = APIRouter(prefix="/shared", tags=["shares"]) + + +# ------------------------------------------------------------------ +# Conversation-scoped endpoints +# ------------------------------------------------------------------ + + +@conversations_share_router.post( + "/{session_id}/share", + response_model=ShareResponse, + response_model_by_alias=True, + status_code=201, +) +async def create_share( + session_id: str, + request: CreateShareRequest, + current_user: User = Depends(get_current_user), +): + """Create a point-in-time share snapshot for a conversation.""" + try: + return await get_share_service().create_share( + session_id=session_id, + user=current_user, + request=request, + ) + except SessionNotFoundError: + raise HTTPException(status_code=404, detail=f"Session not found: {session_id}") + except NotOwnerError: + raise HTTPException(status_code=403, detail="You do not have permission to share this session") + except ShareTableNotFoundError: + raise HTTPException(status_code=503, detail="Share feature unavailable - table not deployed") + except Exception as e: + safe_session_id = session_id.replace("\r", "").replace("\n", "") + logger.error(f"Error creating share for session {safe_session_id}: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Failed to create share") + + +@conversations_share_router.get( + "/{session_id}/shares", + response_model=ShareListResponse, + response_model_by_alias=True, +) +async def list_shares_for_session( + session_id: str, + current_user: User = Depends(get_current_user), +): + """List all shares for a session (owner only).""" + try: + return await get_share_service().get_shares_for_session(session_id, current_user.user_id) + except ShareTableNotFoundError: + raise HTTPException(status_code=503, detail="Share feature unavailable - table not deployed") + except Exception as e: + sanitized_session_id = session_id.replace("\r", "").replace("\n", "") + logger.error(f"Error listing shares for session {sanitized_session_id}: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Failed to list shares") + + +# ------------------------------------------------------------------ +# Share-scoped endpoints (by share_id) +# ------------------------------------------------------------------ + + +@shares_router.patch( + "/{share_id}", + response_model=ShareResponse, + response_model_by_alias=True, +) +async def update_share( + share_id: str, + request: UpdateShareRequest, + current_user: User = Depends(get_current_user), +): + """Update access level or allowed emails on an existing share.""" + try: + return await get_share_service().update_share( + share_id=share_id, + user=current_user, + request=request, + ) + except ShareNotFoundError: + raise HTTPException(status_code=404, detail="Share not found") + except NotOwnerError: + raise HTTPException(status_code=403, detail="You do not have permission to update this share") + except ShareTableNotFoundError: + raise HTTPException(status_code=503, detail="Share feature unavailable - table not deployed") + except Exception as e: + sanitized_share_id = share_id.replace("\r", "").replace("\n", "") + logger.error(f"Error updating share {sanitized_share_id}: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Failed to update share") + + +@shares_router.delete("/{share_id}", status_code=204) +async def revoke_share( + share_id: str, + current_user: User = Depends(get_current_user), +): + """Revoke (delete) a specific share.""" + try: + await get_share_service().revoke_share(share_id=share_id, user=current_user) + return Response(status_code=204) + except ShareNotFoundError: + raise HTTPException(status_code=404, detail="Share not found") + except NotOwnerError: + raise HTTPException(status_code=403, detail="You do not have permission to revoke this share") + except ShareTableNotFoundError: + raise HTTPException(status_code=503, detail="Share feature unavailable - table not deployed") + except Exception as e: + sanitized_share_id = share_id.replace("\r", "").replace("\n", "") + logger.error(f"Error revoking share {sanitized_share_id}: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Failed to revoke share") + + +@shares_router.post("/{share_id}/export", status_code=201) +async def export_shared_conversation( + share_id: str, + current_user: User = Depends(get_current_user), +): + """Export a shared conversation into a new session for the current user.""" + try: + return await get_share_service().export_shared_conversation( + share_id=share_id, + requester=current_user, + ) + except ShareNotFoundError: + raise HTTPException(status_code=404, detail="Share not found") + except AccessDeniedError: + raise HTTPException(status_code=403, detail="Access denied") + except ShareTableNotFoundError: + raise HTTPException(status_code=503, detail="Share feature unavailable - table not deployed") + except Exception as e: + sanitized_share_id = share_id.replace("\r", "").replace("\n", "") + logger.error(f"Error exporting share {sanitized_share_id}: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Failed to export shared conversation") + + +# ------------------------------------------------------------------ +# Shared view endpoint (public read-only) +# ------------------------------------------------------------------ + + +@shared_view_router.get( + "/{share_id}", + response_model=SharedConversationResponse, + response_model_by_alias=True, +) +async def get_shared_conversation( + share_id: str, + current_user: User = Depends(get_current_user), +): + """Retrieve a shared conversation snapshot (access-controlled).""" + try: + return await get_share_service().get_shared_conversation( + share_id=share_id, + requester=current_user, + ) + except ShareNotFoundError: + raise HTTPException(status_code=404, detail="Share not found") + except AccessDeniedError: + raise HTTPException(status_code=403, detail="Access denied") + except ShareTableNotFoundError: + raise HTTPException(status_code=503, detail="Share feature unavailable - table not deployed") + except Exception as e: + sanitized_share_id = share_id.replace("\r", "").replace("\n", "") + logger.error(f"Error retrieving shared conversation {sanitized_share_id}: {e}", exc_info=True) + raise HTTPException(status_code=500, detail="Failed to retrieve shared conversation") diff --git a/backend/src/apis/app_api/shares/service.py b/backend/src/apis/app_api/shares/service.py new file mode 100644 index 00000000..0d12c7ca --- /dev/null +++ b/backend/src/apis/app_api/shares/service.py @@ -0,0 +1,528 @@ +"""Share service layer + +Business logic for creating, retrieving, updating, and revoking +conversation share snapshots. Supports multiple shares per session. +""" + +import json +import logging +import os +import re +import uuid +from decimal import Decimal +from datetime import datetime, timezone +from typing import Any, List, Optional + +import boto3 +from boto3.dynamodb.conditions import Key +from botocore.exceptions import ClientError + +from apis.shared.auth.models import User +from apis.shared.sessions.messages import get_messages +from apis.shared.sessions.metadata import get_session_metadata, store_session_metadata + +from .models import ( + CreateShareRequest, + ShareListResponse, + ShareResponse, + SharedConversationResponse, + UpdateShareRequest, +) + +logger = logging.getLogger(__name__) + + +class ShareService: + """Handles share CRUD operations against the shared-conversations DynamoDB table.""" + + def __init__(self) -> None: + table_name = os.environ.get("SHARED_CONVERSATIONS_TABLE_NAME", "") + self._table_name = table_name + self._enabled = bool(table_name) + + if self._enabled: + self._dynamodb = boto3.resource("dynamodb") + self._table = self._dynamodb.Table(table_name) + logger.info(f"ShareService initialized with table: {table_name}") + else: + self._dynamodb = None + self._table = None + logger.warning("ShareService disabled - SHARED_CONVERSATIONS_TABLE_NAME not set") + + # ------------------------------------------------------------------ + # Public API + # ------------------------------------------------------------------ + + async def create_share( + self, + session_id: str, + user: User, + request: CreateShareRequest, + ) -> ShareResponse: + """Create a new share snapshot for a session. + + Multiple shares can exist per session (e.g. after continuing a conversation). + """ + self._ensure_enabled() + + # Verify session ownership + metadata = await get_session_metadata(session_id=session_id, user_id=user.user_id) + if not metadata: + raise SessionNotFoundError(session_id) + + # Snapshot messages + messages_response = await get_messages(session_id=session_id, user_id=user.user_id) + messages_snapshot = [ + msg.model_dump(by_alias=True, exclude_none=True) + for msg in messages_response.messages + ] + + metadata_snapshot = metadata.model_dump(by_alias=True, exclude_none=True) + + # Convert floats to Decimal for DynamoDB compatibility + messages_snapshot = self._convert_floats_to_decimal(messages_snapshot) + metadata_snapshot = self._convert_floats_to_decimal(metadata_snapshot) + + # Build item + share_id = str(uuid.uuid4()) + now = datetime.now(timezone.utc).isoformat() + allowed_emails = self._resolve_allowed_emails( + request.access_level, request.allowed_emails, user.email + ) + + item = { + "share_id": share_id, + "session_id": session_id, + "owner_id": user.user_id, + "owner_email": user.email, + "access_level": request.access_level, + "created_at": now, + "metadata": metadata_snapshot, + "messages": messages_snapshot, + } + if allowed_emails is not None: + item["allowed_emails"] = allowed_emails + + self._table.put_item(Item=item) + logger.info(f"Created share {self._sanitize_id(share_id)} for session {self._sanitize_id(session_id)}") + + return self._build_share_response(item) + + async def get_shared_conversation( + self, + share_id: str, + requester: User, + ) -> SharedConversationResponse: + """Retrieve a shared conversation snapshot, enforcing access control.""" + self._ensure_enabled() + + item = self._get_share_item(share_id) + if not item: + raise ShareNotFoundError() + + self._check_access(item, requester) + + return self._build_shared_conversation_response(item) + + async def update_share( + self, + share_id: str, + user: User, + request: UpdateShareRequest, + ) -> ShareResponse: + """Update access level / allowed emails on an existing share.""" + self._ensure_enabled() + + item = self._get_share_item(share_id) + if not item: + raise ShareNotFoundError() + + if item["owner_id"] != user.user_id: + raise NotOwnerError() + + update_expr_parts: list[str] = [] + attr_values: dict = {} + remove_parts: list[str] = [] + + new_access = request.access_level or item.get("access_level") + + if request.access_level is not None: + update_expr_parts.append("access_level = :al") + attr_values[":al"] = request.access_level + + # Resolve allowed_emails + if new_access == "specific": + emails = request.allowed_emails or item.get("allowed_emails", []) + resolved = self._resolve_allowed_emails(new_access, emails, user.email) + update_expr_parts.append("allowed_emails = :ae") + attr_values[":ae"] = resolved + elif request.access_level is not None: + # Switching to public โ†’ clear allowed_emails + remove_parts.append("allowed_emails") + + if not update_expr_parts and not remove_parts: + return self._build_share_response(item) + + update_expr = "" + if update_expr_parts: + update_expr += "SET " + ", ".join(update_expr_parts) + if remove_parts: + update_expr += " REMOVE " + ", ".join(remove_parts) + + kwargs = { + "Key": {"share_id": item["share_id"]}, + "UpdateExpression": update_expr, + "ReturnValues": "ALL_NEW", + } + if attr_values: + kwargs["ExpressionAttributeValues"] = attr_values + + result = self._table.update_item(**kwargs) + updated = result.get("Attributes", item) + logger.info(f"Updated share {item['share_id']}") + + return self._build_share_response(updated) + + async def revoke_share(self, share_id: str, user: User) -> None: + """Delete a specific share by share_id.""" + self._ensure_enabled() + + item = self._get_share_item(share_id) + if not item: + raise ShareNotFoundError() + + if item["owner_id"] != user.user_id: + raise NotOwnerError() + + self._table.delete_item(Key={"share_id": item["share_id"]}) + logger.info(f"Revoked share {item['share_id']}") + + async def get_shares_for_session(self, session_id: str, user_id: str) -> ShareListResponse: + """Return all shares for a session owned by the user.""" + self._ensure_enabled() + + items = self._find_shares_by_session(session_id) + shares = [ + self._build_share_response(item) + for item in items + if item["owner_id"] == user_id + ] + return ShareListResponse(shares=shares) + + async def export_shared_conversation( + self, + share_id: str, + requester: User, + ) -> dict: + """Export a shared conversation as a new session for the requester. + + Creates a new session with the snapshot messages copied into AgentCore + Memory, producing a full fork of the shared conversation. + """ + self._ensure_enabled() + + item = self._get_share_item(share_id) + if not item: + raise ShareNotFoundError() + + self._check_access(item, requester) + + snapshot_messages = item.get("messages", []) + metadata = item.get("metadata", {}) + original_title = metadata.get("title", "Untitled Conversation") + new_title = f"{original_title} (shared)" + + new_session_id = str(uuid.uuid4()) + now = datetime.now(timezone.utc).isoformat() + + # Copy snapshot messages into AgentCore Memory for the new session + message_count = await self._copy_messages_to_memory( + new_session_id, requester.user_id, snapshot_messages + ) + + from apis.shared.sessions.models import SessionMetadata + + session_meta = SessionMetadata( + session_id=new_session_id, + user_id=requester.user_id, + title=new_title, + status="active", + created_at=now, + last_message_at=now, + message_count=message_count, + ) + + await store_session_metadata( + session_id=new_session_id, + user_id=requester.user_id, + session_metadata=session_meta, + ) + + logger.info( + f"Exported share {self._sanitize_id(share_id)} to new session {self._sanitize_id(new_session_id)} " + f"for user {self._sanitize_id(requester.user_id)} ({message_count} messages copied)" + ) + + return {"sessionId": new_session_id, "title": new_title} + + # ------------------------------------------------------------------ + # Internal helpers + # ------------------------------------------------------------------ + + # ------------------------------------------------------------------ + # Message copying helpers + + async def _copy_messages_to_memory( + self, + session_id: str, + user_id: str, + snapshot_messages: list, + ) -> int: + """Write snapshot messages into AgentCore Memory for a new session. + + Converts each MessageResponse dict back to Bedrock Converse format + and appends it via AgentCoreMemorySessionManager. + + Returns: + Number of messages successfully written. + """ + if not snapshot_messages: + return 0 + + import asyncio + + try: + from bedrock_agentcore.memory.integrations.strands.config import ( + AgentCoreMemoryConfig, + ) + from bedrock_agentcore.memory.integrations.strands.session_manager import ( + AgentCoreMemorySessionManager, + ) + except ImportError: + logger.error("AgentCore Memory SDK not available โ€” cannot copy messages") + return 0 + + memory_id = os.environ.get("AGENTCORE_MEMORY_ID") + aws_region = os.environ.get("AWS_REGION", "us-west-2") + if not memory_id: + logger.error("AGENTCORE_MEMORY_ID not set โ€” cannot copy messages") + return 0 + + config = AgentCoreMemoryConfig( + memory_id=memory_id, + session_id=session_id, + actor_id=user_id, + enable_prompt_caching=False, + ) + mgr = AgentCoreMemorySessionManager( + agentcore_memory_config=config, region_name=aws_region + ) + + count = 0 + for msg_dict in snapshot_messages: + converse_msg = self._snapshot_msg_to_converse(msg_dict) + if converse_msg is None: + continue + try: + await asyncio.to_thread(mgr.append_message, converse_msg, None) + count += 1 + except Exception as e: + logger.warning(f"Failed to copy message {count}: {e}") + + logger.info(f"Copied {count}/{len(snapshot_messages)} messages to AgentCore Memory") + return count + + @staticmethod + def _snapshot_msg_to_converse(msg: dict) -> Optional[dict]: + """Convert a snapshot MessageResponse dict to Bedrock Converse format. + + Snapshot format (MessageResponse): + {"id": "...", "role": "user", "content": [{"type": "text", "text": "hi"}, ...], ...} + + Converse format (Strands/Bedrock): + {"role": "user", "content": [{"text": "hi"}, ...]} + """ + role = msg.get("role") + if role not in ("user", "assistant"): + return None + + raw_content = msg.get("content", []) + converse_content = [] + + for block in raw_content: + block_type = block.get("type") if isinstance(block, dict) else None + if block_type == "text" and block.get("text"): + converse_content.append({"text": block["text"]}) + elif block_type == "toolUse" and block.get("toolUse"): + converse_content.append({"toolUse": block["toolUse"]}) + elif block_type == "toolResult" and block.get("toolResult"): + converse_content.append({"toolResult": block["toolResult"]}) + elif block_type == "image" and block.get("image"): + converse_content.append({"image": block["image"]}) + elif block_type == "document" and block.get("document"): + converse_content.append({"document": block["document"]}) + elif block_type == "reasoningContent" and block.get("reasoningContent"): + converse_content.append({"reasoningContent": block["reasoningContent"]}) + # Skip unknown/empty blocks + + if not converse_content: + return None + + return {"role": role, "content": converse_content} + + @staticmethod + def _convert_floats_to_decimal(obj: Any) -> Any: + """Recursively convert float values to Decimal for DynamoDB compatibility. + + DynamoDB's boto3 resource doesn't accept Python floats directly. + This converts all floats in nested dicts/lists to Decimal. + """ + if isinstance(obj, float): + # Use string conversion to preserve precision + return Decimal(str(obj)) + elif isinstance(obj, dict): + return {k: ShareService._convert_floats_to_decimal(v) for k, v in obj.items()} + elif isinstance(obj, list): + return [ShareService._convert_floats_to_decimal(item) for item in obj] + return obj + + @staticmethod + def _sanitize_id(value: str, max_length: int = 128) -> str: + """Return a log-safe version of an ID string. + + Strips anything that isn't an alphanumeric character or a hyphen/underscore, + then truncates to ``max_length``. This prevents log-injection attacks where + a crafted ID embeds newlines or ANSI escape sequences. + """ + sanitized = re.sub(r"[^a-zA-Z0-9\-_]", "", value) + return sanitized[:max_length] + + def _ensure_enabled(self) -> None: + if not self._enabled: + raise ShareTableNotFoundError() + + def _get_share_item(self, share_id: str) -> Optional[dict]: + try: + resp = self._table.get_item(Key={"share_id": share_id}) + return resp.get("Item") + except ClientError as e: + if e.response["Error"]["Code"] == "ResourceNotFoundException": + logger.error(f"Shared conversations table '{self._table_name}' not found - has CDK been deployed?") + raise ShareTableNotFoundError() + raise + + def _find_shares_by_session(self, session_id: str) -> List[dict]: + """Return all shares for a given session_id.""" + try: + resp = self._table.query( + IndexName="SessionShareIndex", + KeyConditionExpression=Key("session_id").eq(session_id), + ) + return resp.get("Items", []) + except ClientError as e: + if e.response["Error"]["Code"] == "ResourceNotFoundException": + logger.error(f"Shared conversations table '{self._table_name}' not found - has CDK been deployed?") + raise ShareTableNotFoundError() + raise + + @staticmethod + def _resolve_allowed_emails( + access_level: str, + allowed_emails: Optional[List[str]], + owner_email: str, + ) -> Optional[List[str]]: + if access_level != "specific": + return None + emails = list(allowed_emails or []) + if owner_email.lower() not in [e.lower() for e in emails]: + emails.insert(0, owner_email) + return emails + + def _check_access(self, item: dict, requester: User) -> None: + access_level = item.get("access_level", "specific") + + # Owner always has access + if requester.user_id == item["owner_id"]: + return + + if access_level == "public": + return + + if access_level == "specific": + allowed = [e.lower() for e in item.get("allowed_emails", [])] + if requester.email.lower() in allowed: + return + + raise AccessDeniedError() + + def _build_share_response(self, item: dict) -> ShareResponse: + return ShareResponse( + share_id=item["share_id"], + session_id=item["session_id"], + owner_id=item["owner_id"], + access_level=item["access_level"], + allowed_emails=item.get("allowed_emails"), + created_at=item["created_at"], + share_url=f"/shared/{item['share_id']}", + ) + + def _build_shared_conversation_response(self, item: dict) -> SharedConversationResponse: + from apis.shared.sessions.models import MessageResponse + + metadata = item.get("metadata", {}) + raw_messages = item.get("messages", []) + + messages = [] + for msg_data in raw_messages: + try: + messages.append(MessageResponse.model_validate(msg_data)) + except Exception as e: + logger.warning(f"Skipping malformed message in share {item['share_id']}: {e}") + + return SharedConversationResponse( + share_id=item["share_id"], + title=metadata.get("title", "Untitled Conversation"), + access_level=item["access_level"], + created_at=item["created_at"], + owner_id=item["owner_id"], + messages=messages, + ) + + +# ------------------------------------------------------------------ +# Domain exceptions +# ------------------------------------------------------------------ + +class SessionNotFoundError(Exception): + def __init__(self, session_id: str): + self.session_id = session_id + super().__init__(f"Session not found: {session_id}") + + +class ShareNotFoundError(Exception): + pass + + +class NotOwnerError(Exception): + pass + + +class AccessDeniedError(Exception): + pass + + +class ShareTableNotFoundError(Exception): + """Raised when the DynamoDB table does not exist (CDK not deployed).""" + pass + + +# Global service instance (singleton) +_service_instance: Optional[ShareService] = None + + +def get_share_service() -> ShareService: + """Get or create the global ShareService instance.""" + global _service_instance + if _service_instance is None: + _service_instance = ShareService() + return _service_instance diff --git a/backend/src/apis/shared/oauth/routes.py b/backend/src/apis/shared/oauth/routes.py index c4a99ece..1c4de22e 100644 --- a/backend/src/apis/shared/oauth/routes.py +++ b/backend/src/apis/shared/oauth/routes.py @@ -3,7 +3,7 @@ import logging import os from typing import Optional -from urllib.parse import urlencode +from urllib.parse import urlencode, urlparse from fastapi import APIRouter, Depends, HTTPException, Query, status from fastapi.responses import RedirectResponse @@ -202,8 +202,18 @@ async def oauth_callback( state=state, ) - # Build redirect URL - redirect_base = frontend_redirect or f"{frontend_url}{callback_path}" + # Build redirect URL โ€” validate that frontend_redirect is same-origin + # to prevent open redirect attacks via manipulated OAuth state + redirect_base = f"{frontend_url}{callback_path}" + if frontend_redirect: + parsed = urlparse(frontend_redirect) + parsed_frontend = urlparse(frontend_url) + if parsed.scheme == parsed_frontend.scheme and parsed.netloc == parsed_frontend.netloc: + redirect_base = frontend_redirect + else: + logger.warning( + f"OAuth callback redirect blocked โ€” origin mismatch: {parsed.netloc} != {parsed_frontend.netloc}" + ) if callback_error: params = urlencode({"error": callback_error, "provider": provider_id}) diff --git a/backend/src/apis/shared/sessions/metadata.py b/backend/src/apis/shared/sessions/metadata.py index ed9cf2e9..21e83a85 100644 --- a/backend/src/apis/shared/sessions/metadata.py +++ b/backend/src/apis/shared/sessions/metadata.py @@ -1004,47 +1004,58 @@ async def _list_user_sessions_cloud( query_params['ExclusiveStartKey'] = exclusive_start_key if limit: - # With new schema, we can use exact limit - no over-fetching needed query_params['Limit'] = limit - # Execute query - response = table.query(**query_params) + # Pagination loop: DynamoDB's Limit caps items *evaluated*, not items + # *returned* after application-level filtering (preview sessions, parse + # failures). A single query may return fewer valid sessions than the + # requested limit while still having more data in the partition. We keep + # querying until we fill the page or exhaust the partition. + sessions: list[SessionMetadata] = [] + last_evaluated_key = None - # Parse items - filter out preview sessions - sessions = [] - for item in response['Items']: - try: - # Convert Decimal to float - item = _convert_decimal_to_float(item) + while True: + response = table.query(**query_params) + + for item in response['Items']: + try: + item = _convert_decimal_to_float(item) - # Remove DynamoDB keys - for key in ['PK', 'SK', 'GSI_PK', 'GSI_SK']: - item.pop(key, None) + for key in ['PK', 'SK', 'GSI_PK', 'GSI_SK']: + item.pop(key, None) - # Skip preview sessions - they should not appear in user's session list - session_id = item.get('sessionId', '') - if is_preview_session(session_id): + # Skip preview sessions - they should not appear in user's session list + session_id = item.get('sessionId', '') + if is_preview_session(session_id): + continue + + metadata = SessionMetadata.model_validate(item) + sessions.append(metadata) + + # Stop collecting once we have enough + if limit and len(sessions) >= limit: + break + except Exception as e: + # JUSTIFICATION: When listing sessions from DynamoDB, individual session parsing + # failures should not break the entire list operation. We skip corrupted sessions + # and continue processing others. This provides better UX than failing completely. + logger.warning(f"Failed to parse session item: {e}") continue - metadata = SessionMetadata.model_validate(item) - sessions.append(metadata) - except Exception as e: - # JUSTIFICATION: When listing sessions from DynamoDB, individual session parsing - # failures should not break the entire list operation. We skip corrupted sessions - # and continue processing others. This provides better UX than failing completely. - logger.warning(f"Failed to parse session item: {e}") - continue + last_evaluated_key = response.get('LastEvaluatedKey') - # No in-memory sorting needed! With new SK pattern S#ACTIVE#{last_message_at}#{session_id}, - # DynamoDB returns items already sorted by timestamp (via ScanIndexForward=False) + # Stop if we've filled the page or there's no more data + if (limit and len(sessions) >= limit) or not last_evaluated_key: + break - # No limit trimming needed either - we use exact Limit in query params + # Continue querying from where DynamoDB left off + query_params['ExclusiveStartKey'] = last_evaluated_key - # Generate next_token from LastEvaluatedKey if present + # Generate next_token only when there is genuinely more data to fetch next_page_token = None - if 'LastEvaluatedKey' in response: + if last_evaluated_key and limit and len(sessions) >= limit: next_page_token = base64.b64encode( - json.dumps(response['LastEvaluatedKey']).encode('utf-8') + json.dumps(last_evaluated_key).encode('utf-8') ).decode('utf-8') logger.info(f"Listed {len(sessions)} sessions for user {user_id} from DynamoDB") diff --git a/backend/tests/agents/main_agent/core/test_model_config.py b/backend/tests/agents/main_agent/core/test_model_config.py index 3ba0cf7d..7aac7789 100644 --- a/backend/tests/agents/main_agent/core/test_model_config.py +++ b/backend/tests/agents/main_agent/core/test_model_config.py @@ -5,8 +5,6 @@ provider-specific config dicts, to_dict, from_params with defaults and invalid provider. """ -from unittest.mock import MagicMock, patch - import pytest from agents.main_agent.core.model_config import ModelConfig, ModelProvider, RetryConfig @@ -102,22 +100,15 @@ def test_explicit_gemini_overrides_gpt_model_id(self): class TestToBedrockConfig: """Validates: Requirements 1.6, 1.7""" - @patch("agents.main_agent.core.model_config.CacheConfig", create=True) - def test_bedrock_config_with_caching(self, mock_cache_cls): - """Req 1.6 โ€” caching enabled โ†’ dict has model_id, temperature, cache_config.""" - mock_cache_instance = MagicMock() - mock_cache_cls.return_value = mock_cache_instance - - with patch.dict( - "sys.modules", - {"strands.models": MagicMock(CacheConfig=mock_cache_cls)}, - ): - cfg = ModelConfig(caching_enabled=True) - result = cfg.to_bedrock_config() + def test_bedrock_config_with_caching_disabled_due_to_bedrock_limitation(self): + """Req 1.6 โ€” caching_enabled=True but cache_config omitted due to + Bedrock limitation with non-PDF document blocks. See model_config.py TODO.""" + cfg = ModelConfig(caching_enabled=True) + result = cfg.to_bedrock_config() assert result["model_id"] == cfg.model_id assert result["temperature"] == cfg.temperature - assert "cache_config" in result + assert "cache_config" not in result def test_bedrock_config_without_caching(self): """Req 1.6 (negative) โ€” caching disabled โ†’ no cache_config key.""" diff --git a/backend/tests/agents/main_agent/session/conftest.py b/backend/tests/agents/main_agent/session/conftest.py index b0727ee7..25e12614 100644 --- a/backend/tests/agents/main_agent/session/conftest.py +++ b/backend/tests/agents/main_agent/session/conftest.py @@ -6,6 +6,7 @@ """ import json +import threading from unittest.mock import MagicMock, patch import boto3 @@ -109,6 +110,8 @@ def mock_agentcore_config(): config.session_id = TEST_SESSION_ID config.memory_id = TEST_MEMORY_ID config.actor_id = TEST_ACTOR_ID + config.batch_size = 1 + config.flush_interval_seconds = None return config @@ -121,7 +124,7 @@ def compaction_config() -> CompactionConfig: return CompactionConfig( enabled=True, token_threshold=1000, - protected_turns=2, + protected_turns=3, max_tool_content_length=50, ) @@ -183,6 +186,35 @@ def seed_session_record(table, session_id: str, user_id: str, compaction=None) - return item +# --------------------------------------------------------------------------- +# Mock parent __init__ โ€” sets required attributes without AWS calls +# --------------------------------------------------------------------------- + +def _mock_parent_init(config): + """Return a replacement __init__ for AgentCoreMemorySessionManager.""" + def _init(self, agentcore_memory_config=None, region_name=None, **kwargs): + cfg = agentcore_memory_config or config + self.config = cfg + self.memory_client = MagicMock() + self.session_repository = self + self.session_id = cfg.session_id + self._is_new_session = True + self.session = MagicMock() + self._latest_agent_message = {} + self._last_synced_internal_state = {} + self.has_existing_agent = False + self.converter = MagicMock() + self._message_buffer = [] + self._message_lock = threading.Lock() + self._agent_state_buffer = [] + self._agent_state_lock = threading.Lock() + self._agent_created_at_cache = {} + self._flush_timer = None + self._timer_lock = threading.Lock() + self._shutdown = False + return _init + + # --------------------------------------------------------------------------- # TurnBasedSessionManager factory fixture # --------------------------------------------------------------------------- @@ -191,22 +223,26 @@ def seed_session_record(table, session_id: str, user_id: str, compaction=None) - def make_session_manager(mock_agentcore_config): """ Factory fixture โ€” returns a callable that creates a TurnBasedSessionManager - with the AgentCoreMemorySessionManager mocked out. + with AgentCoreMemorySessionManager.__init__ mocked out so no AWS calls are made. + + The resulting manager inherits all TurnBasedSessionManager methods and has + the parent's required attributes set via the mock __init__. """ def _factory(compaction_config=None, user_id=TEST_USER_ID, **kwargs): - with patch( - "agents.main_agent.session.turn_based_session_manager.AgentCoreMemorySessionManager" - ) as MockBaseManager: - mock_base = MagicMock() - mock_base.list_messages.return_value = [] - MockBaseManager.return_value = mock_base - - from agents.main_agent.session.turn_based_session_manager import TurnBasedSessionManager + from bedrock_agentcore.memory.integrations.strands.session_manager import ( + AgentCoreMemorySessionManager, + ) + from agents.main_agent.session.turn_based_session_manager import TurnBasedSessionManager - # Reset class-level state between tests - TurnBasedSessionManager._dynamodb_table = None - TurnBasedSessionManager._dynamodb_table_name = None + # Reset class-level state between tests + TurnBasedSessionManager._dynamodb_table = None + TurnBasedSessionManager._dynamodb_table_name = None + with patch.object( + AgentCoreMemorySessionManager, + "__init__", + _mock_parent_init(mock_agentcore_config), + ): mgr = TurnBasedSessionManager( agentcore_memory_config=mock_agentcore_config, region_name=REGION, @@ -214,7 +250,14 @@ def _factory(compaction_config=None, user_id=TEST_USER_ID, **kwargs): user_id=user_id, **kwargs, ) - mgr._mock_base = mock_base - return mgr + + # Set up mock methods for session repository operations + # (These are inherited from AgentCoreMemorySessionManager and called via self) + mgr.read_agent = MagicMock(return_value=None) + mgr.list_messages = MagicMock(return_value=[]) + mgr.create_agent = MagicMock() + mgr.create_message = MagicMock() + + return mgr return _factory diff --git a/backend/tests/agents/main_agent/session/test_compaction_models.py b/backend/tests/agents/main_agent/session/test_compaction_models.py index 4d426bfc..af885472 100644 --- a/backend/tests/agents/main_agent/session/test_compaction_models.py +++ b/backend/tests/agents/main_agent/session/test_compaction_models.py @@ -152,7 +152,7 @@ def test_from_env_defaults_when_no_vars(self, monkeypatch): monkeypatch.delenv("AGENTCORE_MEMORY_COMPACTION_MAX_TOOL_CONTENT_LENGTH", raising=False) config = CompactionConfig.from_env() - assert config.enabled is False + assert config.enabled is True assert config.token_threshold == 100_000 - assert config.protected_turns == 2 + assert config.protected_turns == 3 assert config.max_tool_content_length == 500 diff --git a/backend/tests/agents/main_agent/session/test_turn_based_session_manager.py b/backend/tests/agents/main_agent/session/test_turn_based_session_manager.py index 3d824ba9..82c833d3 100644 --- a/backend/tests/agents/main_agent/session/test_turn_based_session_manager.py +++ b/backend/tests/agents/main_agent/session/test_turn_based_session_manager.py @@ -4,6 +4,10 @@ Covers: initialization, message helpers, truncation (Stage 1), summary injection, DynamoDB state persistence, LTM retrieval, initialization flow, post-turn update (Stage 2), session interface, and property-based tests. + +Architecture: TurnBasedSessionManager inherits from AgentCoreMemorySessionManager. +Tests mock the parent's __init__ to avoid AWS calls and set up required attributes +via the make_session_manager fixture in conftest.py. """ import copy @@ -60,6 +64,17 @@ def test_message_builders(self): conv = make_conversation(3) assert len(conv) == 6 + def test_has_required_parent_attributes(self, make_session_manager): + """Verify the mock parent __init__ sets up all required attributes.""" + mgr = make_session_manager() + assert hasattr(mgr, "config") + assert hasattr(mgr, "memory_client") + assert hasattr(mgr, "session_id") + assert hasattr(mgr, "_latest_agent_message") + assert hasattr(mgr, "_is_new_session") + assert hasattr(mgr, "has_existing_agent") + assert mgr.session_id == TEST_SESSION_ID + # =========================================================================== # Task 2 โ€” Message processing helpers @@ -413,7 +428,7 @@ def test_returns_cached_id(self, make_session_manager): def test_discovers_from_memory_config(self, make_session_manager): mgr = make_session_manager() - mgr.base_manager.memory_client.gmcp_client.get_memory.return_value = { + mgr.memory_client.gmcp_client.get_memory.return_value = { "memory": { "strategies": [ {"type": "EXTRACTION", "strategyId": "ext-1"}, @@ -427,14 +442,14 @@ def test_discovers_from_memory_config(self, make_session_manager): def test_returns_none_when_no_summarization_strategy(self, make_session_manager): mgr = make_session_manager() - mgr.base_manager.memory_client.gmcp_client.get_memory.return_value = { + mgr.memory_client.gmcp_client.get_memory.return_value = { "memory": {"strategies": [{"type": "EXTRACTION", "strategyId": "ext-1"}]} } assert mgr._get_summarization_strategy_id() is None def test_returns_none_on_error(self, make_session_manager): mgr = make_session_manager() - mgr.base_manager.memory_client.gmcp_client.get_memory.side_effect = Exception("fail") + mgr.memory_client.gmcp_client.get_memory.side_effect = Exception("fail") assert mgr._get_summarization_strategy_id() is None @@ -442,7 +457,7 @@ class TestRetrieveSessionSummaries: def test_returns_empty_when_no_strategy(self, make_session_manager): mgr = make_session_manager() - mgr.base_manager.memory_client.gmcp_client.get_memory.return_value = { + mgr.memory_client.gmcp_client.get_memory.return_value = { "memory": {"strategies": []} } assert mgr._retrieve_session_summaries() == [] @@ -503,74 +518,196 @@ def test_empty_messages_returns_none(self, make_session_manager): mgr = make_session_manager() assert mgr._generate_fallback_summary([]) is None - def test_limits_to_10_points(self, make_session_manager): + def test_limits_to_15_points(self, make_session_manager): mgr = make_session_manager() messages = [make_user_message(f"Topic {i}") for i in range(20)] summary = mgr._generate_fallback_summary(messages) - assert summary.count("- User asked about:") == 10 + # Fallback summary limits key_points to last 15 + lines = [line for line in summary.split("\n") if line.startswith("- User:")] + assert len(lines) == 15 # =========================================================================== -# Task 7 โ€” Initialization flow +# Task 7 โ€” Initialization flow (SDK override) # =========================================================================== class TestInitialize: + """Test the initialize() override which handles compaction on session restore.""" - def test_compaction_disabled_delegates_only(self, make_session_manager): - mgr = make_session_manager() + def _make_mock_agent(self, messages=None): + """Create a mock agent for initialize() tests.""" agent = MagicMock() - agent.messages = [make_user_message("hi")] - mgr.initialize(agent) - mgr._mock_base.initialize.assert_called_once_with(agent) - - def test_compaction_enabled_empty_messages(self, make_session_manager, compaction_config): + agent.agent_id = "default" + agent.messages = messages or [] + agent.state = MagicMock() + agent.conversation_manager.restore_from_session.return_value = [] + agent.conversation_manager.removed_message_count = 0 + return agent + + def _make_mock_session_agent(self): + """Create a mock SessionAgent for the existing-agent path.""" + session_agent = MagicMock() + session_agent.state = {} + session_agent.conversation_manager_state = {} + return session_agent + + def _make_mock_session_messages(self, messages): + """Convert raw message dicts to mock SessionMessage objects.""" + session_messages = [] + for msg in messages: + sm = MagicMock() + sm.to_message.return_value = msg + session_messages.append(sm) + return session_messages + + def test_new_agent_creates_session(self, make_session_manager, compaction_config): + """When session_agent is None, should create agent and set empty compaction state.""" mgr = make_session_manager(compaction_config=compaction_config) - agent = MagicMock() - agent.messages = [] + mgr.read_agent = MagicMock(return_value=None) + + agent = self._make_mock_agent() mgr.initialize(agent) + + mgr.create_agent.assert_called_once() assert mgr.compaction_state is not None assert mgr.compaction_state.checkpoint == 0 + assert mgr._valid_cutoff_indices == [] + + def test_existing_agent_no_compaction_loads_all_messages(self, make_session_manager): + """Without compaction config, should load all messages from session.""" + mgr = make_session_manager() # no compaction_config + messages = make_conversation(3) # 6 messages + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False + + agent = self._make_mock_agent() + mgr.initialize(agent) + + assert len(agent.messages) == 6 + assert mgr.message_count == 6 - def test_compaction_no_checkpoint_applies_truncation(self, make_session_manager, compaction_config): + def test_existing_agent_compaction_no_checkpoint(self, make_session_manager, compaction_config): + """Compaction enabled with checkpoint=0 should load all messages with truncation.""" mgr = make_session_manager(compaction_config=compaction_config) - # Patch _load_compaction_state to return default (no checkpoint) - mgr._load_compaction_state = lambda: CompactionState() - agent = MagicMock() - agent.messages = [ + mgr._load_compaction_state = MagicMock(return_value=CompactionState()) + + messages = [ make_user_message("q1"), - make_tool_result_message("t1", "x" * 200), + make_assistant_message("a1"), + make_user_message("q2"), + make_tool_result_message("t1", "x" * 200), # will be truncated ] + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False + + agent = self._make_mock_agent() mgr.initialize(agent) - # Messages should still be length 2 (truncation doesn't remove messages) - assert len(agent.messages) == 2 - def test_compaction_with_checkpoint_slices_messages(self, make_session_manager, compaction_config): + # All 4 messages kept (checkpoint=0), but truncation applied + assert len(agent.messages) == 4 + # Valid cutoffs cached for user text messages (indices 0, 2) + assert mgr._valid_cutoff_indices == [0, 2] + + def test_existing_agent_compaction_with_checkpoint_slices_messages(self, make_session_manager, compaction_config): + """Checkpoint > 0 should skip old messages and prepend summary.""" mgr = make_session_manager(compaction_config=compaction_config) - mgr._load_compaction_state = lambda: CompactionState(checkpoint=4, summary="old context") - agent = MagicMock() - agent.messages = make_conversation(4) # 8 messages + mgr._load_compaction_state = MagicMock( + return_value=CompactionState(checkpoint=4, summary="old context") + ) + + messages = make_conversation(4) # 8 messages + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False + + agent = self._make_mock_agent() mgr.initialize(agent) - # Should have sliced from index 4 onward = 4 messages + + # Should slice from index 4: 4 messages remain assert len(agent.messages) == 4 - # Summary should be prepended to first message + # Summary prepended to first user message first_text = agent.messages[0]["content"][0]["text"] assert "old context" in first_text + assert "" in first_text - def test_compaction_checkpoint_plus_truncation(self, make_session_manager, compaction_config): + def test_existing_agent_compaction_checkpoint_plus_truncation(self, make_session_manager, compaction_config): + """Both checkpoint slicing and truncation should apply.""" mgr = make_session_manager(compaction_config=compaction_config) - mgr._load_compaction_state = lambda: CompactionState(checkpoint=2) - agent = MagicMock() - # Build messages where post-checkpoint messages have truncatable content - agent.messages = [ + mgr._load_compaction_state = MagicMock( + return_value=CompactionState(checkpoint=2) + ) + + messages = [ make_user_message("old1"), make_assistant_message("old2"), make_user_message("new1"), - make_tool_result_message("t1", "r" * 200), + make_tool_result_message("t1", "r" * 200), # truncatable ] + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False + + agent = self._make_mock_agent() mgr.initialize(agent) + # Sliced from index 2: 2 messages remain assert len(agent.messages) == 2 + def test_duplicate_agent_id_raises(self, make_session_manager): + """Second initialize with same agent_id should raise SessionException.""" + from strands.types.exceptions import SessionException + + mgr = make_session_manager() + mgr.read_agent = MagicMock(return_value=None) + agent = self._make_mock_agent() + + mgr.initialize(agent) + + with pytest.raises(SessionException, match="unique"): + mgr.initialize(agent) + + def test_sets_has_existing_agent_flag(self, make_session_manager): + """initialize() should mark has_existing_agent = True.""" + mgr = make_session_manager() + mgr.read_agent = MagicMock(return_value=None) + agent = self._make_mock_agent() + + assert mgr.has_existing_agent is False + mgr.initialize(agent) + assert mgr.has_existing_agent is True + + def test_sets_is_new_session_false(self, make_session_manager): + """initialize() should mark _is_new_session = False.""" + mgr = make_session_manager() + mgr.read_agent = MagicMock(return_value=None) + agent = self._make_mock_agent() + + mgr.initialize(agent) + assert mgr._is_new_session is False + + def test_caches_all_messages_for_summary(self, make_session_manager, compaction_config): + """Compaction path should cache shallow copies of all messages for summary generation.""" + mgr = make_session_manager(compaction_config=compaction_config) + mgr._load_compaction_state = MagicMock(return_value=CompactionState()) + + messages = make_conversation(3) + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False + + agent = self._make_mock_agent() + mgr.initialize(agent) + + # Should have cached 6 messages for summary generation + assert len(mgr._all_messages_for_summary) == 6 + # =========================================================================== # Task 8 โ€” Post-turn update (Stage 2, async) @@ -601,14 +738,15 @@ async def test_above_threshold_creates_checkpoint(self, make_session_manager, co mgr._save_compaction_state = MagicMock() mgr._retrieve_session_summaries = MagicMock(return_value=[]) - # Return 5 turns of messages (10 messages) as dicts + # Simulate 5 turns cached during initialize messages = make_conversation(5) - mgr._mock_base.list_messages.return_value = messages + mgr._valid_cutoff_indices = [0, 2, 4, 6, 8] # 5 user message indices + mgr._all_messages_for_summary = messages await mgr.update_after_turn(2000) # above 1000 threshold - # With 5 turns and protected_turns=2, checkpoint should be at turn 3 start (index 6) - assert mgr.compaction_state.checkpoint == 6 + # With 5 turns and protected_turns=3, checkpoint should be at turn 2 start (index 4) + assert mgr.compaction_state.checkpoint == 4 assert mgr.compaction_state.last_input_tokens == 2000 @pytest.mark.asyncio @@ -617,72 +755,79 @@ async def test_not_enough_turns_keeps_all(self, make_session_manager, compaction mgr.compaction_state = CompactionState() mgr._save_compaction_state = MagicMock() - # Only 2 turns = protected_turns, so no compaction possible - mgr._mock_base.list_messages.return_value = make_conversation(2) + # Only 3 turns = protected_turns, so no compaction possible + mgr._valid_cutoff_indices = [0, 2, 4] await mgr.update_after_turn(2000) assert mgr.compaction_state.checkpoint == 0 @pytest.mark.asyncio - async def test_checkpoint_unchanged_no_update(self, make_session_manager, compaction_config): + async def test_empty_cutoff_indices_skips_checkpoint(self, make_session_manager, compaction_config): mgr = make_session_manager(compaction_config=compaction_config) - mgr.compaction_state = CompactionState(checkpoint=6) + mgr.compaction_state = CompactionState() mgr._save_compaction_state = MagicMock() - # Same 5 turns โ€” checkpoint would be 6 again, same as current - mgr._mock_base.list_messages.return_value = make_conversation(5) + mgr._valid_cutoff_indices = [] # no valid cutoffs (e.g., new session) await mgr.update_after_turn(2000) - # Checkpoint should remain 6 (no update) - assert mgr.compaction_state.checkpoint == 6 + assert mgr.compaction_state.checkpoint == 0 + mgr._save_compaction_state.assert_called_once() @pytest.mark.asyncio - async def test_uses_ltm_summaries_when_available(self, make_session_manager, compaction_config): + async def test_checkpoint_unchanged_no_update(self, make_session_manager, compaction_config): mgr = make_session_manager(compaction_config=compaction_config) - mgr.compaction_state = CompactionState() + mgr.compaction_state = CompactionState(checkpoint=4) mgr._save_compaction_state = MagicMock() - mgr._retrieve_session_summaries = MagicMock(return_value=["LTM summary 1", "LTM summary 2"]) - mgr._mock_base.list_messages.return_value = make_conversation(5) + # 5 turns โ€” checkpoint would be at index 4 again, same as current + mgr._valid_cutoff_indices = [0, 2, 4, 6, 8] await mgr.update_after_turn(2000) - assert "LTM summary 1" in mgr.compaction_state.summary - assert "LTM summary 2" in mgr.compaction_state.summary + # Checkpoint should remain 4 (no update) + assert mgr.compaction_state.checkpoint == 4 @pytest.mark.asyncio - async def test_falls_back_to_generated_summary(self, make_session_manager, compaction_config): + async def test_checkpoint_advances_when_more_turns(self, make_session_manager, compaction_config): + """New turns should advance the checkpoint.""" mgr = make_session_manager(compaction_config=compaction_config) - mgr.compaction_state = CompactionState() + mgr.compaction_state = CompactionState(checkpoint=4) mgr._save_compaction_state = MagicMock() - mgr._retrieve_session_summaries = MagicMock(return_value=[]) + mgr._retrieve_session_summaries = MagicMock(return_value=["Updated summary"]) - mgr._mock_base.list_messages.return_value = make_conversation(5) + # 6 turns now โ€” checkpoint should advance + mgr._valid_cutoff_indices = [0, 2, 4, 6, 8, 10] + mgr._all_messages_for_summary = make_conversation(6) await mgr.update_after_turn(2000) - assert mgr.compaction_state.summary is not None - assert "Previous conversation topics" in mgr.compaction_state.summary + assert mgr.compaction_state.checkpoint == 6 # [-3] = index 6 @pytest.mark.asyncio - async def test_message_fetch_failure_graceful(self, make_session_manager, compaction_config): + async def test_uses_ltm_summaries_when_available(self, make_session_manager, compaction_config): mgr = make_session_manager(compaction_config=compaction_config) mgr.compaction_state = CompactionState() mgr._save_compaction_state = MagicMock() - mgr._mock_base.list_messages.side_effect = Exception("network error") + mgr._retrieve_session_summaries = MagicMock(return_value=["LTM summary 1", "LTM summary 2"]) + + mgr._valid_cutoff_indices = [0, 2, 4, 6, 8] + mgr._all_messages_for_summary = make_conversation(5) await mgr.update_after_turn(2000) - # Should save state and not raise - mgr._save_compaction_state.assert_called_once() - assert mgr.compaction_state.checkpoint == 0 + assert "LTM summary 1" in mgr.compaction_state.summary + assert "LTM summary 2" in mgr.compaction_state.summary @pytest.mark.asyncio - async def test_no_messages_skips_checkpoint(self, make_session_manager, compaction_config): + async def test_falls_back_to_generated_summary(self, make_session_manager, compaction_config): mgr = make_session_manager(compaction_config=compaction_config) mgr.compaction_state = CompactionState() mgr._save_compaction_state = MagicMock() - mgr._mock_base.list_messages.return_value = [] + mgr._retrieve_session_summaries = MagicMock(return_value=[]) + + mgr._valid_cutoff_indices = [0, 2, 4, 6, 8] + mgr._all_messages_for_summary = make_conversation(5) await mgr.update_after_turn(2000) - assert mgr.compaction_state.checkpoint == 0 + assert mgr.compaction_state.summary is not None + assert "Previous conversation" in mgr.compaction_state.summary @pytest.mark.asyncio async def test_initializes_compaction_state_if_none(self, make_session_manager, compaction_config): @@ -713,94 +858,192 @@ def test_returns_none_when_empty(self, make_session_manager): class TestAppendMessage: - def test_delegates_and_increments(self, make_session_manager): + def test_increments_message_count(self, make_session_manager): mgr = make_session_manager() agent = MagicMock() msg = {"role": "user", "content": [{"text": "hi"}]} - mgr.append_message(msg, agent) - mgr._mock_base.append_message.assert_called_once_with(msg, agent) + with patch( + "agents.main_agent.session.turn_based_session_manager.AgentCoreMemorySessionManager.append_message" + ): + mgr.append_message(msg, agent) assert mgr.message_count == 1 - def test_cancelled_skips_delegation(self, make_session_manager): + def test_cancelled_skips(self, make_session_manager): mgr = make_session_manager() mgr.cancelled = True agent = MagicMock() msg = {"role": "user", "content": [{"text": "hi"}]} mgr.append_message(msg, agent) - mgr._mock_base.append_message.assert_not_called() assert mgr.message_count == 0 def test_increments_multiple_times(self, make_session_manager): mgr = make_session_manager() agent = MagicMock() - for i in range(3): - mgr.append_message({"role": "user", "content": [{"text": f"m{i}"}]}, agent) + with patch( + "agents.main_agent.session.turn_based_session_manager.AgentCoreMemorySessionManager.append_message" + ): + for i in range(3): + mgr.append_message({"role": "user", "content": [{"text": f"m{i}"}]}, agent) assert mgr.message_count == 3 + def test_filters_empty_text_blocks(self, make_session_manager): + mgr = make_session_manager() + agent = MagicMock() + msg = {"role": "user", "content": [{"text": " "}]} # whitespace-only + mgr.append_message(msg, agent) + # Empty after filtering โ€” should not increment + assert mgr.message_count == 0 -class TestRegisterHooks: + def test_keeps_tool_use_blocks(self, make_session_manager): + mgr = make_session_manager() + agent = MagicMock() + msg = make_tool_use_message("t1", "calc", {"x": 1}) + with patch( + "agents.main_agent.session.turn_based_session_manager.AgentCoreMemorySessionManager.append_message" + ): + mgr.append_message(msg, agent) + assert mgr.message_count == 1 + + +class TestFilterEmptyText: - def test_registers_all_event_types(self, make_session_manager): + def test_removes_empty_text_blocks(self, make_session_manager): mgr = make_session_manager() - registry = MagicMock() - mgr.register_hooks(registry) + msg = {"role": "user", "content": [{"text": ""}, {"text": "keep"}]} + result = mgr._filter_empty_text(msg) + assert len(result["content"]) == 1 + assert result["content"][0]["text"] == "keep" - # Should have 5 add_callback calls: - # AgentInitializedEvent, MessageAddedEvent (append), MessageAddedEvent (sync), - # AfterInvocationEvent (sync), MessageAddedEvent (LTM) - assert registry.add_callback.call_count == 5 + def test_removes_whitespace_only_text(self, make_session_manager): + mgr = make_session_manager() + msg = {"role": "user", "content": [{"text": " \n "}]} + result = mgr._filter_empty_text(msg) + assert len(result["content"]) == 0 + + def test_keeps_non_text_blocks(self, make_session_manager): + mgr = make_session_manager() + msg = {"role": "assistant", "content": [ + {"text": ""}, + {"toolUse": {"toolUseId": "t1", "name": "calc", "input": {}}}, + ]} + result = mgr._filter_empty_text(msg) + assert len(result["content"]) == 1 + assert "toolUse" in result["content"][0] + + def test_no_content_key_unchanged(self, make_session_manager): + mgr = make_session_manager() + msg = {"role": "user"} + assert mgr._filter_empty_text(msg) == msg - def test_init_hook_calls_our_initialize(self, make_session_manager): + def test_non_list_content_unchanged(self, make_session_manager): mgr = make_session_manager() - registry = MagicMock() - mgr.register_hooks(registry) + msg = {"role": "user", "content": "string"} + assert mgr._filter_empty_text(msg) == msg - # First callback registered is for AgentInitializedEvent - first_call = registry.add_callback.call_args_list[0] - callback = first_call[0][1] - # Simulate the event - event = MagicMock() - event.agent = MagicMock() - event.agent.messages = [] +# =========================================================================== +# Task 10 โ€” End-to-end compaction lifecycle +# =========================================================================== - with patch.object(mgr, "initialize") as mock_init: - callback(event) - mock_init.assert_called_once_with(event.agent) +class TestCompactionLifecycle: + """Integration-style tests that exercise the full init โ†’ stream โ†’ update cycle.""" - def test_message_hook_calls_our_append(self, make_session_manager): - mgr = make_session_manager() - registry = MagicMock() - mgr.register_hooks(registry) + def _make_mock_agent(self, messages=None): + agent = MagicMock() + agent.agent_id = "default" + agent.messages = messages or [] + agent.state = MagicMock() + agent.conversation_manager.restore_from_session.return_value = [] + agent.conversation_manager.removed_message_count = 0 + return agent + + def _make_mock_session_messages(self, messages): + session_messages = [] + for msg in messages: + sm = MagicMock() + sm.to_message.return_value = msg + session_messages.append(sm) + return session_messages + + def _make_mock_session_agent(self): + session_agent = MagicMock() + session_agent.state = {} + session_agent.conversation_manager_state = {} + return session_agent - # Second callback is MessageAddedEvent -> append_message - second_call = registry.add_callback.call_args_list[1] - callback = second_call[0][1] + @pytest.mark.asyncio + async def test_turn1_new_session_no_checkpoint(self, make_session_manager, compaction_config): + """Turn 1: New session โ€” no messages, no checkpoint.""" + mgr = make_session_manager(compaction_config=compaction_config) + mgr.read_agent = MagicMock(return_value=None) - event = MagicMock() - event.message = {"role": "user", "content": [{"text": "hi"}]} - event.agent = MagicMock() + agent = self._make_mock_agent() + mgr.initialize(agent) - with patch.object(mgr, "append_message") as mock_append: - callback(event) - mock_append.assert_called_once_with(event.message, event.agent) + assert mgr._valid_cutoff_indices == [] + assert mgr.compaction_state.checkpoint == 0 + # Simulate under-threshold turn + mgr._save_compaction_state = MagicMock() + await mgr.update_after_turn(500) + assert mgr.compaction_state.checkpoint == 0 -class TestGetattr: + @pytest.mark.asyncio + async def test_turn6_creates_checkpoint(self, make_session_manager, compaction_config): + """Turn 6: Enough turns to create a checkpoint (>3 protected).""" + mgr = make_session_manager(compaction_config=compaction_config) + mgr._load_compaction_state = MagicMock(return_value=CompactionState()) + mgr._retrieve_session_summaries = MagicMock(return_value=["Session summary"]) - def test_delegates_unknown_attributes(self, make_session_manager): - mgr = make_session_manager() - mgr._mock_base.some_method.return_value = "delegated" - assert mgr.some_method() == "delegated" + # Simulate 5 prior turns of history + messages = make_conversation(5) # 10 messages + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False - def test_delegates_unknown_property(self, make_session_manager): - mgr = make_session_manager() - mgr._mock_base.some_prop = 42 - assert mgr.some_prop == 42 + agent = self._make_mock_agent() + mgr.initialize(agent) + + # Verify cutoff indices cached + assert mgr._valid_cutoff_indices == [0, 2, 4, 6, 8] + + # Simulate above-threshold turn + mgr._save_compaction_state = MagicMock() + await mgr.update_after_turn(2000) + + # 5 cutoffs, protected=3 โ†’ checkpoint at [-3] = index 4 + assert mgr.compaction_state.checkpoint == 4 + assert "Session summary" in mgr.compaction_state.summary + + @pytest.mark.asyncio + async def test_turn5_applies_checkpoint(self, make_session_manager, compaction_config): + """Turn 5: Checkpoint was set โ€” messages should be sliced.""" + mgr = make_session_manager(compaction_config=compaction_config) + mgr._load_compaction_state = MagicMock( + return_value=CompactionState(checkpoint=4, summary="Prior discussion summary") + ) + + messages = make_conversation(5) # 10 messages + session_agent = self._make_mock_session_agent() + mgr.read_agent = MagicMock(return_value=session_agent) + mgr.list_messages = MagicMock(return_value=self._make_mock_session_messages(messages)) + mgr._is_new_session = False + + agent = self._make_mock_agent() + mgr.initialize(agent) + + # Messages 0-3 skipped (checkpoint=4), messages 4-9 kept = 6 messages + assert len(agent.messages) == 6 + # Summary prepended to first message + first_text = agent.messages[0]["content"][0]["text"] + assert "Prior discussion summary" in first_text + # original=10, final=6 (compaction working!) + assert mgr._total_message_count_at_init == 10 # =========================================================================== -# Task 10 โ€” Property-based tests +# Task 11 โ€” Property-based tests # =========================================================================== try: diff --git a/backend/tests/routes/test_share_export.py b/backend/tests/routes/test_share_export.py new file mode 100644 index 00000000..3073c5c9 --- /dev/null +++ b/backend/tests/routes/test_share_export.py @@ -0,0 +1,316 @@ +"""Tests for share export (conversation fork) logic. + +Tests the service-level methods that copy snapshot messages into +AgentCore Memory when a user exports a shared conversation. +""" + +import os + +os.environ.setdefault("AWS_REGION", "us-east-1") +os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1") + +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from apis.app_api.shares.service import ShareService + + +# --------------------------------------------------------------------------- +# _snapshot_msg_to_converse โ€” pure function, no mocking needed +# --------------------------------------------------------------------------- + + +class TestSnapshotMsgToConverse: + """Unit tests for the MessageResponseโ†’Converse format converter.""" + + def test_text_message(self): + msg = { + "id": "msg-sess-0", + "role": "user", + "content": [{"type": "text", "text": "Hello"}], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert result == {"role": "user", "content": [{"text": "Hello"}]} + + def test_assistant_with_tool_use(self): + tool_use_payload = {"toolUseId": "t1", "name": "search", "input": {"q": "test"}} + msg = { + "id": "msg-sess-1", + "role": "assistant", + "content": [ + {"type": "text", "text": "Let me search."}, + {"type": "toolUse", "toolUse": tool_use_payload}, + ], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert result["role"] == "assistant" + assert len(result["content"]) == 2 + assert result["content"][0] == {"text": "Let me search."} + assert result["content"][1] == {"toolUse": tool_use_payload} + + def test_tool_result_block(self): + tool_result_payload = {"toolUseId": "t1", "content": [{"text": "result"}]} + msg = { + "id": "msg-sess-2", + "role": "user", + "content": [{"type": "toolResult", "toolResult": tool_result_payload}], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert result["content"][0] == {"toolResult": tool_result_payload} + + def test_image_block(self): + image_payload = {"format": "png", "source": {"bytes": "base64data"}} + msg = { + "id": "msg-sess-3", + "role": "user", + "content": [{"type": "image", "image": image_payload}], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert result["content"][0] == {"image": image_payload} + + def test_document_block(self): + doc_payload = {"format": "pdf", "name": "report", "source": {"bytes": "base64data"}} + msg = { + "id": "msg-sess-4", + "role": "user", + "content": [{"type": "document", "document": doc_payload}], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert result["content"][0] == {"document": doc_payload} + + def test_reasoning_content_block(self): + reasoning_payload = {"reasoningText": {"text": "thinking..."}} + msg = { + "id": "msg-sess-5", + "role": "assistant", + "content": [{"type": "reasoningContent", "reasoningContent": reasoning_payload}], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert result["content"][0] == {"reasoningContent": reasoning_payload} + + def test_skips_system_role(self): + msg = { + "id": "msg-sess-6", + "role": "system", + "content": [{"type": "text", "text": "system prompt"}], + "createdAt": "2025-06-01T00:00:00Z", + } + assert ShareService._snapshot_msg_to_converse(msg) is None + + def test_skips_empty_content(self): + msg = {"id": "msg-sess-7", "role": "user", "content": [], "createdAt": "2025-06-01T00:00:00Z"} + assert ShareService._snapshot_msg_to_converse(msg) is None + + def test_skips_unknown_block_types(self): + msg = { + "id": "msg-sess-8", + "role": "user", + "content": [{"type": "unknown", "data": "stuff"}], + "createdAt": "2025-06-01T00:00:00Z", + } + assert ShareService._snapshot_msg_to_converse(msg) is None + + def test_mixed_content_skips_empty_text(self): + """A text block with empty string should be skipped.""" + msg = { + "id": "msg-sess-9", + "role": "assistant", + "content": [ + {"type": "text", "text": ""}, + {"type": "text", "text": "real content"}, + ], + "createdAt": "2025-06-01T00:00:00Z", + } + result = ShareService._snapshot_msg_to_converse(msg) + assert len(result["content"]) == 1 + assert result["content"][0] == {"text": "real content"} + + def test_strips_metadata_and_id(self): + """Output should only contain role and content โ€” no id, createdAt, metadata.""" + msg = { + "id": "msg-sess-10", + "role": "user", + "content": [{"type": "text", "text": "hi"}], + "createdAt": "2025-06-01T00:00:00Z", + "metadata": {"tokenUsage": {"inputTokens": 10}}, + "citations": [{"url": "https://example.com"}], + } + result = ShareService._snapshot_msg_to_converse(msg) + assert set(result.keys()) == {"role", "content"} + + +# --------------------------------------------------------------------------- +# _copy_messages_to_memory โ€” needs AgentCore Memory mocked +# --------------------------------------------------------------------------- + + +class TestCopyMessagesToMemory: + """Tests for writing snapshot messages into AgentCore Memory.""" + + @pytest.fixture + def service(self): + """ShareService with DynamoDB disabled (we only test memory copying).""" + with patch.dict(os.environ, {"SHARED_CONVERSATIONS_TABLE_NAME": ""}): + svc = ShareService() + return svc + + @pytest.mark.asyncio + async def test_empty_snapshot_returns_zero(self, service): + count = await service._copy_messages_to_memory("sess-new", "user-1", []) + assert count == 0 + + @pytest.mark.asyncio + async def test_copies_valid_messages(self, service): + snapshot = [ + {"id": "msg-0", "role": "user", "content": [{"type": "text", "text": "Hello"}], "createdAt": "2025-06-01T00:00:00Z"}, + {"id": "msg-1", "role": "assistant", "content": [{"type": "text", "text": "Hi there"}], "createdAt": "2025-06-01T00:00:01Z"}, + ] + + mock_mgr = MagicMock() + mock_mgr.append_message = MagicMock() + + with patch.dict(os.environ, {"AGENTCORE_MEMORY_ID": "mem-123", "AWS_REGION": "us-east-1"}), \ + patch("bedrock_agentcore.memory.integrations.strands.session_manager.AgentCoreMemorySessionManager", return_value=mock_mgr), \ + patch("bedrock_agentcore.memory.integrations.strands.config.AgentCoreMemoryConfig"): + count = await service._copy_messages_to_memory("sess-new", "user-1", snapshot) + + assert count == 2 + assert mock_mgr.append_message.call_count == 2 + + # Verify first call was the user message in Converse format + first_call_msg = mock_mgr.append_message.call_args_list[0][0][0] + assert first_call_msg == {"role": "user", "content": [{"text": "Hello"}]} + + # Verify second call was the assistant message + second_call_msg = mock_mgr.append_message.call_args_list[1][0][0] + assert second_call_msg == {"role": "assistant", "content": [{"text": "Hi there"}]} + + @pytest.mark.asyncio + async def test_skips_unconvertible_messages(self, service): + snapshot = [ + {"id": "msg-0", "role": "system", "content": [{"type": "text", "text": "system"}], "createdAt": "2025-06-01T00:00:00Z"}, + {"id": "msg-1", "role": "user", "content": [{"type": "text", "text": "Hello"}], "createdAt": "2025-06-01T00:00:01Z"}, + ] + + mock_mgr = MagicMock() + mock_mgr.append_message = MagicMock() + + with patch.dict(os.environ, {"AGENTCORE_MEMORY_ID": "mem-123", "AWS_REGION": "us-east-1"}), \ + patch("bedrock_agentcore.memory.integrations.strands.session_manager.AgentCoreMemorySessionManager", return_value=mock_mgr), \ + patch("bedrock_agentcore.memory.integrations.strands.config.AgentCoreMemoryConfig"): + count = await service._copy_messages_to_memory("sess-new", "user-1", snapshot) + + assert count == 1 + assert mock_mgr.append_message.call_count == 1 + + @pytest.mark.asyncio + async def test_continues_on_individual_message_failure(self, service): + snapshot = [ + {"id": "msg-0", "role": "user", "content": [{"type": "text", "text": "First"}], "createdAt": "2025-06-01T00:00:00Z"}, + {"id": "msg-1", "role": "user", "content": [{"type": "text", "text": "Second"}], "createdAt": "2025-06-01T00:00:01Z"}, + {"id": "msg-2", "role": "assistant", "content": [{"type": "text", "text": "Third"}], "createdAt": "2025-06-01T00:00:02Z"}, + ] + + mock_mgr = MagicMock() + # Second call raises, first and third succeed + mock_mgr.append_message = MagicMock(side_effect=[None, RuntimeError("boom"), None]) + + with patch.dict(os.environ, {"AGENTCORE_MEMORY_ID": "mem-123", "AWS_REGION": "us-east-1"}), \ + patch("bedrock_agentcore.memory.integrations.strands.session_manager.AgentCoreMemorySessionManager", return_value=mock_mgr), \ + patch("bedrock_agentcore.memory.integrations.strands.config.AgentCoreMemoryConfig"): + count = await service._copy_messages_to_memory("sess-new", "user-1", snapshot) + + assert count == 2 # 1st and 3rd succeeded + assert mock_mgr.append_message.call_count == 3 + + @pytest.mark.asyncio + async def test_returns_zero_when_memory_id_missing(self, service): + snapshot = [ + {"id": "msg-0", "role": "user", "content": [{"type": "text", "text": "Hello"}], "createdAt": "2025-06-01T00:00:00Z"}, + ] + + with patch.dict(os.environ, {"AGENTCORE_MEMORY_ID": "", "AWS_REGION": "us-east-1"}): + count = await service._copy_messages_to_memory("sess-new", "user-1", snapshot) + + assert count == 0 + + +# --------------------------------------------------------------------------- +# export_shared_conversation โ€” integration of the above +# --------------------------------------------------------------------------- + + +class TestExportSharedConversation: + """Tests that export creates a session with copied messages.""" + + @pytest.fixture + def service(self): + with patch.dict(os.environ, {"SHARED_CONVERSATIONS_TABLE_NAME": "shares-table"}): + with patch("boto3.resource"): + svc = ShareService() + return svc + + def _make_share_item(self, messages=None): + return { + "share_id": "share-001", + "session_id": "orig-sess", + "owner_id": "owner-001", + "owner_email": "owner@example.com", + "access_level": "public", + "created_at": "2025-06-01T00:00:00Z", + "metadata": {"title": "My Chat"}, + "messages": messages or [ + {"id": "msg-0", "role": "user", "content": [{"type": "text", "text": "Hello"}], "createdAt": "2025-06-01T00:00:00Z"}, + {"id": "msg-1", "role": "assistant", "content": [{"type": "text", "text": "Hi"}], "createdAt": "2025-06-01T00:00:01Z"}, + ], + } + + @pytest.mark.asyncio + async def test_export_copies_messages_and_sets_count(self, service): + from apis.shared.auth.models import User + + requester = User(email="viewer@example.com", user_id="viewer-001", name="Viewer", roles=["User"]) + share_item = self._make_share_item() + + with patch.object(service, "_get_share_item", return_value=share_item), \ + patch.object(service, "_check_access"), \ + patch.object(service, "_copy_messages_to_memory", new_callable=AsyncMock, return_value=2) as mock_copy, \ + patch("apis.app_api.shares.service.store_session_metadata", new_callable=AsyncMock) as mock_store: + result = await service.export_shared_conversation("share-001", requester) + + assert "sessionId" in result + assert result["title"] == "My Chat (shared)" + + # Verify messages were passed to copy + mock_copy.assert_called_once() + call_args = mock_copy.call_args + assert call_args[0][1] == "viewer-001" # user_id + assert len(call_args[0][2]) == 2 # 2 snapshot messages + + # Verify session metadata has correct message_count + mock_store.assert_called_once() + stored_meta = mock_store.call_args[1]["session_metadata"] + assert stored_meta.message_count == 2 + + @pytest.mark.asyncio + async def test_export_empty_snapshot_creates_session_with_zero_messages(self, service): + from apis.shared.auth.models import User + + requester = User(email="viewer@example.com", user_id="viewer-001", name="Viewer", roles=["User"]) + share_item = self._make_share_item(messages=[]) + + with patch.object(service, "_get_share_item", return_value=share_item), \ + patch.object(service, "_check_access"), \ + patch.object(service, "_copy_messages_to_memory", new_callable=AsyncMock, return_value=0) as mock_copy, \ + patch("apis.app_api.shares.service.store_session_metadata", new_callable=AsyncMock): + result = await service.export_shared_conversation("share-001", requester) + + assert result["title"] == "My Chat (shared)" + mock_copy.assert_called_once() diff --git a/backend/tests/routes/test_share_properties.py b/backend/tests/routes/test_share_properties.py new file mode 100644 index 00000000..80c459da --- /dev/null +++ b/backend/tests/routes/test_share_properties.py @@ -0,0 +1,468 @@ +"""Property-based tests for the share conversations feature. + +Uses Hypothesis to verify invariants across randomly generated inputs. +Each test maps to a design property from the share-conversations spec. + +Feature: share-conversations +""" + +from datetime import datetime, timezone +from typing import Optional +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from hypothesis import given, settings, HealthCheck, assume +from hypothesis import strategies as st + +from apis.app_api.shares.models import ( + CreateShareRequest, + ShareResponse, + UpdateShareRequest, +) +from apis.app_api.shares.routes import conversations_share_router, shares_router +from apis.app_api.shares.service import ( + AccessDeniedError, + NotOwnerError, + ShareNotFoundError, + ShareService, +) +from apis.shared.auth.models import User +from apis.shared.sessions.models import MessageResponse + +from tests.routes.conftest import mock_auth_user + + +# --------------------------------------------------------------------------- +# Hypothesis strategies +# --------------------------------------------------------------------------- + +st_email = st.emails() + +st_user_id = st.uuids().map(str) + +st_share_id = st.uuids().map(str) + +st_session_id = st.uuids().map(str) + +st_access_level = st.sampled_from(["public", "specific"]) + +st_iso_timestamp = st.datetimes( + min_value=datetime(2020, 1, 1), + max_value=datetime(2030, 1, 1), +).map(lambda dt: dt.replace(tzinfo=timezone.utc).isoformat()) + +st_title = st.text( + alphabet=st.characters(whitelist_categories=("L", "N", "Z")), + min_size=1, + max_size=100, +) + +st_message_content = st.fixed_dictionaries({ + "type": st.just("text"), + "text": st.text(min_size=1, max_size=200), +}) + +st_message = st.fixed_dictionaries({ + "id": st.uuids().map(str), + "role": st.sampled_from(["user", "assistant"]), + "content": st.lists(st_message_content, min_size=1, max_size=3), + "createdAt": st_iso_timestamp, +}) + +st_email_list = st.lists(st_email, min_size=1, max_size=10) + + +@st.composite +def st_share_response(draw): + """Generate a random valid ShareResponse.""" + access = draw(st_access_level) + emails = draw(st_email_list) if access == "specific" else None + return ShareResponse( + share_id=draw(st_share_id), + session_id=draw(st_session_id), + owner_id=draw(st_user_id), + access_level=access, + allowed_emails=emails, + created_at=draw(st_iso_timestamp), + share_url=f"/shared/{draw(st_share_id)}", + ) + + +# --------------------------------------------------------------------------- +# Property 2: Owner email auto-inclusion invariant +# Validates: Requirements 1.2, 4.2, 4.5 +# --------------------------------------------------------------------------- + +class TestOwnerEmailAutoInclusion: + """Property 2: Owner email auto-inclusion invariant. + + For any create or update where access_level is 'specific', + the resolved allowed_emails always contains the owner's email. + """ + + @given( + owner_email=st_email, + input_emails=st.lists(st_email, min_size=0, max_size=10), + ) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_owner_always_in_allowed_emails(self, owner_email, input_emails): + """Feature: share-conversations, Property 2: Owner email auto-inclusion invariant + + Validates: Requirements 1.2, 4.2, 4.5 + """ + result = ShareService._resolve_allowed_emails( + access_level="specific", + allowed_emails=input_emails, + owner_email=owner_email, + ) + + assert result is not None + assert owner_email.lower() in [e.lower() for e in result] + + @given(owner_email=st_email) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_owner_included_even_when_emails_empty(self, owner_email): + """Feature: share-conversations, Property 2: Owner email auto-inclusion (empty list) + + Validates: Requirements 1.2, 4.2, 4.5 + """ + result = ShareService._resolve_allowed_emails( + access_level="specific", + allowed_emails=[], + owner_email=owner_email, + ) + + assert result is not None + assert owner_email in result + + @given(owner_email=st_email) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_owner_included_when_emails_none(self, owner_email): + """Feature: share-conversations, Property 2: Owner email auto-inclusion (None) + + Validates: Requirements 1.2, 4.2, 4.5 + """ + result = ShareService._resolve_allowed_emails( + access_level="specific", + allowed_emails=None, + owner_email=owner_email, + ) + + assert result is not None + assert owner_email in result + + +# --------------------------------------------------------------------------- +# Property 3: "Specific" access requires non-empty allowed_emails +# Validates: Requirements 1.3, 4.3 +# --------------------------------------------------------------------------- + +class TestSpecificRequiresEmails: + """Property 3: 'Specific' access requires non-empty allowed_emails. + + For any create/update with access_level 'specific' and empty/missing + allowed_emails, the Pydantic model raises a validation error. + """ + + @given(data=st.data()) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_create_specific_empty_emails_raises(self, data): + """Feature: share-conversations, Property 3: Specific access requires non-empty allowed_emails (create) + + Validates: Requirements 1.3, 4.3 + """ + empty_emails = data.draw(st.sampled_from([[], None])) + + with pytest.raises(ValueError, match="allowed_emails is required"): + CreateShareRequest( + access_level="specific", + allowed_emails=empty_emails, + ) + + @given(data=st.data()) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_update_specific_empty_emails_raises(self, data): + """Feature: share-conversations, Property 3: Specific access requires non-empty allowed_emails (update) + + Validates: Requirements 1.3, 4.3 + """ + empty_emails = data.draw(st.sampled_from([[], None])) + + with pytest.raises(ValueError, match="allowed_emails is required"): + UpdateShareRequest( + access_level="specific", + allowed_emails=empty_emails, + ) + + +# --------------------------------------------------------------------------- +# Property 5: Access control matrix +# Validates: Requirements 2.1, 2.2, 2.3, 2.4, 2.5 +# --------------------------------------------------------------------------- + +class TestAccessControlMatrix: + """Property 5: Access control matrix. + + For any shared conversation and any authenticated requesting user, + access is granted/denied according to the access_level rules. + """ + + @given( + owner_id=st_user_id, + requester_id=st_user_id, + requester_email=st_email, + access_level=st_access_level, + allowed_emails=st.lists(st_email, min_size=0, max_size=5), + ) + @settings( + max_examples=200, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_access_control_matches_matrix( + self, owner_id, requester_id, requester_email, access_level, allowed_emails + ): + """Feature: share-conversations, Property 5: Access control matrix + + Validates: Requirements 2.1, 2.2, 2.3, 2.4, 2.5 + """ + service = ShareService.__new__(ShareService) + + item = { + "share_id": "test-share", + "owner_id": owner_id, + "access_level": access_level, + "allowed_emails": allowed_emails, + } + + requester = User( + email=requester_email, + user_id=requester_id, + name="Test", + roles=["User"], + ) + + is_owner = requester_id == owner_id + email_in_allowed = requester_email.lower() in [e.lower() for e in allowed_emails] + + # Determine expected outcome + if is_owner: + should_allow = True + elif access_level == "public": + should_allow = True + elif access_level == "specific": + should_allow = email_in_allowed + else: + should_allow = False + + if should_allow: + # Should not raise + service._check_access(item, requester) + else: + with pytest.raises(AccessDeniedError): + service._check_access(item, requester) + + +# --------------------------------------------------------------------------- +# Property 8: Non-specific access levels clear allowed_emails +# Validates: Requirements 4.1, 4.4 +# --------------------------------------------------------------------------- + +class TestNonSpecificClearsEmails: + """Property 8: Non-specific access levels clear allowed_emails. + + For any access_level that is not 'specific', _resolve_allowed_emails + returns None. + """ + + @given( + access_level=st.just("public"), + emails=st.lists(st_email, min_size=0, max_size=5), + owner_email=st_email, + ) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_non_specific_returns_none(self, access_level, emails, owner_email): + """Feature: share-conversations, Property 8: Non-specific access levels clear allowed_emails + + Validates: Requirements 4.1, 4.4 + """ + result = ShareService._resolve_allowed_emails( + access_level=access_level, + allowed_emails=emails, + owner_email=owner_email, + ) + + assert result is None + + +# --------------------------------------------------------------------------- +# Property 9: ShareResponse serialization round-trip +# Validates: Requirements 10.5 +# --------------------------------------------------------------------------- + +class TestShareResponseRoundTrip: + """Property 9: ShareResponse serialization round-trip. + + For any valid ShareResponse, serializing to JSON and deserializing + back produces an equivalent object. + """ + + @given(resp=st_share_response()) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_json_round_trip(self, resp): + """Feature: share-conversations, Property 9: ShareResponse serialization round-trip + + Validates: Requirements 10.5 + """ + json_str = resp.model_dump_json(by_alias=True) + restored = ShareResponse.model_validate_json(json_str) + + assert restored.share_id == resp.share_id + assert restored.session_id == resp.session_id + assert restored.owner_id == resp.owner_id + assert restored.access_level == resp.access_level + assert restored.allowed_emails == resp.allowed_emails + assert restored.created_at == resp.created_at + assert restored.share_url == resp.share_url + + +# --------------------------------------------------------------------------- +# Property 4: Non-owner operations return 403 +# Validates: Requirements 1.5, 3.2, 4.6 +# --------------------------------------------------------------------------- + +class TestNonOwnerOperationsReturn403: + """Property 4: Non-owner operations return 403. + + For any share operation attempted by a non-owner, the API returns 403. + """ + + @given( + owner_id=st_user_id, + requester_id=st_user_id, + ) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_non_owner_create_returns_403(self, owner_id, requester_id): + """Feature: share-conversations, Property 4: Non-owner operations return 403 (create) + + Validates: Requirements 1.5, 3.2, 4.6 + """ + assume(owner_id != requester_id) + + app = FastAPI() + app.include_router(conversations_share_router) + + requester = User( + email="requester@example.com", + user_id=requester_id, + name="Requester", + roles=["User"], + ) + mock_auth_user(app, requester) + + mock_svc = AsyncMock() + mock_svc.create_share = AsyncMock(side_effect=NotOwnerError()) + with patch("apis.app_api.shares.routes.get_share_service", return_value=mock_svc): + client = TestClient(app, raise_server_exceptions=False) + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 403 + + @given( + owner_id=st_user_id, + requester_id=st_user_id, + ) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_non_owner_revoke_returns_403(self, owner_id, requester_id): + """Feature: share-conversations, Property 4: Non-owner operations return 403 (revoke) + + Validates: Requirements 1.5, 3.2, 4.6 + """ + assume(owner_id != requester_id) + + app = FastAPI() + app.include_router(shares_router) + + requester = User( + email="requester@example.com", + user_id=requester_id, + name="Requester", + roles=["User"], + ) + mock_auth_user(app, requester) + + mock_svc = AsyncMock() + mock_svc.revoke_share = AsyncMock(side_effect=NotOwnerError()) + with patch("apis.app_api.shares.routes.get_share_service", return_value=mock_svc): + client = TestClient(app, raise_server_exceptions=False) + resp = client.delete("/shares/share-001") + + assert resp.status_code == 403 + + @given( + owner_id=st_user_id, + requester_id=st_user_id, + ) + @settings( + max_examples=100, + suppress_health_check=[HealthCheck.too_slow, HealthCheck.function_scoped_fixture], + ) + def test_non_owner_update_returns_403(self, owner_id, requester_id): + """Feature: share-conversations, Property 4: Non-owner operations return 403 (update) + + Validates: Requirements 1.5, 3.2, 4.6 + """ + assume(owner_id != requester_id) + + app = FastAPI() + app.include_router(shares_router) + + requester = User( + email="requester@example.com", + user_id=requester_id, + name="Requester", + roles=["User"], + ) + mock_auth_user(app, requester) + + mock_svc = AsyncMock() + mock_svc.update_share = AsyncMock(side_effect=NotOwnerError()) + with patch("apis.app_api.shares.routes.get_share_service", return_value=mock_svc): + client = TestClient(app, raise_server_exceptions=False) + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 403 diff --git a/backend/tests/routes/test_shares.py b/backend/tests/routes/test_shares.py new file mode 100644 index 00000000..f06fa257 --- /dev/null +++ b/backend/tests/routes/test_shares.py @@ -0,0 +1,538 @@ +"""Tests for share conversation routes. + +Endpoints under test: +- POST /conversations/{session_id}/share โ†’ 201 create share snapshot +- GET /conversations/{session_id}/shares โ†’ 200 list shares for session +- PATCH /shares/{share_id} โ†’ 200 update share settings +- DELETE /shares/{share_id} โ†’ 204 revoke share +- POST /shares/{share_id}/export โ†’ 201 export to new conversation +- GET /shared/{share_id} โ†’ 200 retrieve shared conversation +""" + +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi import FastAPI + +from apis.app_api.shares.routes import conversations_share_router, shares_router, shared_view_router +from apis.app_api.shares.models import ShareResponse, ShareListResponse, SharedConversationResponse +from apis.app_api.shares.service import ( + AccessDeniedError, + NotOwnerError, + SessionNotFoundError, + ShareNotFoundError, +) +from apis.shared.sessions.models import MessageResponse, MessageContent + +from tests.routes.conftest import mock_auth_user, mock_no_auth + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _make_share_response( + share_id: str = "share-001", + session_id: str = "sess-001", + owner_id: str = "user-001", + access_level: str = "public", + allowed_emails: list | None = None, +) -> ShareResponse: + return ShareResponse( + share_id=share_id, + session_id=session_id, + owner_id=owner_id, + access_level=access_level, + allowed_emails=allowed_emails, + created_at="2025-06-01T00:00:00Z", + share_url=f"/shared/{share_id}", + ) + + +def _make_message_response(msg_id: str = "msg-001") -> MessageResponse: + return MessageResponse( + id=msg_id, + role="assistant", + content=[MessageContent(type="text", text="Hello")], + created_at="2025-06-01T00:00:00Z", + ) + + +def _make_shared_conversation_response( + share_id: str = "share-001", +) -> SharedConversationResponse: + return SharedConversationResponse( + share_id=share_id, + title="Test Conversation", + access_level="public", + created_at="2025-06-01T00:00:00Z", + owner_id="user-001", + messages=[_make_message_response()], + ) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +@pytest.fixture +def app(): + """Minimal FastAPI app mounting share routers.""" + _app = FastAPI() + _app.include_router(conversations_share_router) + _app.include_router(shares_router) + _app.include_router(shared_view_router) + return _app + + +@pytest.fixture +def mock_share_service(): + """Patch get_share_service so routes use our mock.""" + mock = AsyncMock() + with patch("apis.app_api.shares.routes.get_share_service", return_value=mock): + yield mock + + +# --------------------------------------------------------------------------- +# Requirement 1: Share Conversation Snapshot Creation +# --------------------------------------------------------------------------- + +class TestCreateShare: + """POST /conversations/{session_id}/share""" + + def test_create_public_share_returns_201(self, app, make_user, authenticated_client, mock_share_service): + """Req 1.1: Create public share returns 201 with share details.""" + user = make_user() + client = authenticated_client(app, user) + + expected = _make_share_response() + mock_share_service.create_share = AsyncMock(return_value=expected) + + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 201 + body = resp.json() + assert body["shareId"] == "share-001" + assert body["accessLevel"] == "public" + assert body["shareUrl"] == "/shared/share-001" + + def test_create_specific_share_returns_201(self, app, make_user, authenticated_client, mock_share_service): + """Req 1.2: Create specific share with allowed emails returns 201.""" + user = make_user() + client = authenticated_client(app, user) + + expected = _make_share_response( + access_level="specific", + allowed_emails=["test@example.com", "other@example.com"], + ) + mock_share_service.create_share = AsyncMock(return_value=expected) + + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "specific", "allowedEmails": ["other@example.com"]}, + ) + + assert resp.status_code == 201 + body = resp.json() + assert body["accessLevel"] == "specific" + assert "other@example.com" in body["allowedEmails"] + + def test_create_specific_share_without_emails_returns_422(self, app, make_user, authenticated_client): + """Req 1.3: Specific access with empty allowed_emails returns 422.""" + user = make_user() + client = authenticated_client(app, user) + + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "specific", "allowedEmails": []}, + ) + + assert resp.status_code == 422 + + def test_create_private_share_returns_422(self, app, make_user, authenticated_client): + """Private access level is no longer valid and returns 422.""" + user = make_user() + client = authenticated_client(app, user) + + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "private"}, + ) + + assert resp.status_code == 422 + + def test_create_share_non_owner_returns_403(self, app, make_user, authenticated_client, mock_share_service): + """Req 1.5: Non-owner gets 403.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.create_share = AsyncMock(side_effect=NotOwnerError()) + + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 403 + + def test_create_share_session_not_found_returns_404(self, app, make_user, authenticated_client, mock_share_service): + """Req 1.6: Non-existent session returns 404.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.create_share = AsyncMock( + side_effect=SessionNotFoundError("sess-999") + ) + + resp = client.post( + "/conversations/sess-999/share", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 404 + assert "sess-999" in resp.json()["detail"] + + def test_unauthenticated_create_returns_401(self, app, unauthenticated_client): + """Unauthenticated request returns 401.""" + client = unauthenticated_client(app) + + resp = client.post( + "/conversations/sess-001/share", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 401 + + def test_create_multiple_shares_for_same_session(self, app, make_user, authenticated_client, mock_share_service): + """Multiple shares can be created for the same session.""" + user = make_user() + client = authenticated_client(app, user) + + share1 = _make_share_response(share_id="share-001") + share2 = _make_share_response(share_id="share-002") + mock_share_service.create_share = AsyncMock(side_effect=[share1, share2]) + + resp1 = client.post("/conversations/sess-001/share", json={"accessLevel": "public"}) + resp2 = client.post("/conversations/sess-001/share", json={"accessLevel": "public"}) + + assert resp1.status_code == 201 + assert resp2.status_code == 201 + assert resp1.json()["shareId"] != resp2.json()["shareId"] + + +# --------------------------------------------------------------------------- +# Requirement 2: Shared Conversation Retrieval +# --------------------------------------------------------------------------- + +class TestGetSharedConversation: + """GET /shared/{share_id}""" + + def test_get_public_share_returns_200(self, app, make_user, authenticated_client, mock_share_service): + """Req 2.1: Public share returns snapshot data.""" + user = make_user() + client = authenticated_client(app, user) + + expected = _make_shared_conversation_response() + mock_share_service.get_shared_conversation = AsyncMock(return_value=expected) + + resp = client.get("/shared/share-001") + + assert resp.status_code == 200 + body = resp.json() + assert body["shareId"] == "share-001" + assert body["title"] == "Test Conversation" + assert len(body["messages"]) == 1 + + def test_get_share_access_denied_returns_403(self, app, make_user, authenticated_client, mock_share_service): + """Req 2.3/2.4: Access denied returns 403.""" + user = make_user(user_id="other-user") + client = authenticated_client(app, user) + + mock_share_service.get_shared_conversation = AsyncMock( + side_effect=AccessDeniedError() + ) + + resp = client.get("/shared/share-001") + + assert resp.status_code == 403 + assert resp.json()["detail"] == "Access denied" + + def test_get_share_not_found_returns_404(self, app, make_user, authenticated_client, mock_share_service): + """Req 2.7: Non-existent share returns 404.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.get_shared_conversation = AsyncMock( + side_effect=ShareNotFoundError() + ) + + resp = client.get("/shared/share-999") + + assert resp.status_code == 404 + + def test_unauthenticated_get_returns_401(self, app, unauthenticated_client): + """Req 2.6: Unauthenticated request returns 401.""" + client = unauthenticated_client(app) + + resp = client.get("/shared/share-001") + + assert resp.status_code == 401 + + +# --------------------------------------------------------------------------- +# Requirement 3: Share Link Revocation (now by share_id) +# --------------------------------------------------------------------------- + +class TestRevokeShare: + """DELETE /shares/{share_id}""" + + def test_revoke_share_returns_204(self, app, make_user, authenticated_client, mock_share_service): + """Req 3.1: Successful revocation returns 204.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.revoke_share = AsyncMock(return_value=None) + + resp = client.delete("/shares/share-001") + + assert resp.status_code == 204 + + def test_revoke_share_non_owner_returns_403(self, app, make_user, authenticated_client, mock_share_service): + """Req 3.2: Non-owner revocation returns 403.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.revoke_share = AsyncMock(side_effect=NotOwnerError()) + + resp = client.delete("/shares/share-001") + + assert resp.status_code == 403 + + def test_revoke_share_not_found_returns_404(self, app, make_user, authenticated_client, mock_share_service): + """Req 3.3: Revoking non-existent share returns 404.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.revoke_share = AsyncMock(side_effect=ShareNotFoundError()) + + resp = client.delete("/shares/share-999") + + assert resp.status_code == 404 + + +# --------------------------------------------------------------------------- +# Requirement 4: Share Access Level and Allowed Emails Update (now by share_id) +# --------------------------------------------------------------------------- + +class TestUpdateShare: + """PATCH /shares/{share_id}""" + + def test_update_to_public_returns_200(self, app, make_user, authenticated_client, mock_share_service): + """Req 4.1: Update to public clears allowed_emails.""" + user = make_user() + client = authenticated_client(app, user) + + expected = _make_share_response(access_level="public", allowed_emails=None) + mock_share_service.update_share = AsyncMock(return_value=expected) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 200 + body = resp.json() + assert body["accessLevel"] == "public" + assert body.get("allowedEmails") is None + + def test_update_to_specific_returns_200(self, app, make_user, authenticated_client, mock_share_service): + """Req 4.2: Update to specific with emails returns 200.""" + user = make_user() + client = authenticated_client(app, user) + + expected = _make_share_response( + access_level="specific", + allowed_emails=["test@example.com", "new@example.com"], + ) + mock_share_service.update_share = AsyncMock(return_value=expected) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "specific", "allowedEmails": ["new@example.com"]}, + ) + + assert resp.status_code == 200 + assert resp.json()["accessLevel"] == "specific" + + def test_update_to_specific_without_emails_returns_422(self, app, make_user, authenticated_client): + """Req 4.3: Specific without emails returns 422.""" + user = make_user() + client = authenticated_client(app, user) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "specific", "allowedEmails": []}, + ) + + assert resp.status_code == 422 + + def test_update_to_private_returns_422(self, app, make_user, authenticated_client): + """Private access level is no longer valid and returns 422.""" + user = make_user() + client = authenticated_client(app, user) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "private"}, + ) + + assert resp.status_code == 422 + + def test_update_non_owner_returns_403(self, app, make_user, authenticated_client, mock_share_service): + """Req 4.6: Non-owner update returns 403.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.update_share = AsyncMock(side_effect=NotOwnerError()) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 403 + + def test_update_no_active_share_returns_404(self, app, make_user, authenticated_client, mock_share_service): + """Req 4.7: Update with no active share returns 404.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.update_share = AsyncMock(side_effect=ShareNotFoundError()) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "public"}, + ) + + assert resp.status_code == 404 + + def test_update_invalid_access_level_returns_422(self, app, make_user, authenticated_client): + """Req 4.8: Invalid access_level returns 422.""" + user = make_user() + client = authenticated_client(app, user) + + resp = client.patch( + "/shares/share-001", + json={"accessLevel": "invalid_level"}, + ) + + assert resp.status_code == 422 + + +# --------------------------------------------------------------------------- +# List shares for session +# --------------------------------------------------------------------------- + +class TestListSharesForSession: + """GET /conversations/{session_id}/shares""" + + def test_list_shares_returns_200(self, app, make_user, authenticated_client, mock_share_service): + """Returns list of shares for a session.""" + user = make_user() + client = authenticated_client(app, user) + + expected = ShareListResponse(shares=[ + _make_share_response(share_id="share-001"), + _make_share_response(share_id="share-002"), + ]) + mock_share_service.get_shares_for_session = AsyncMock(return_value=expected) + + resp = client.get("/conversations/sess-001/shares") + + assert resp.status_code == 200 + body = resp.json() + assert len(body["shares"]) == 2 + assert body["shares"][0]["shareId"] == "share-001" + assert body["shares"][1]["shareId"] == "share-002" + + def test_list_shares_empty_returns_200(self, app, make_user, authenticated_client, mock_share_service): + """Returns empty list when no shares exist.""" + user = make_user() + client = authenticated_client(app, user) + + expected = ShareListResponse(shares=[]) + mock_share_service.get_shares_for_session = AsyncMock(return_value=expected) + + resp = client.get("/conversations/sess-001/shares") + + assert resp.status_code == 200 + assert resp.json()["shares"] == [] + + def test_list_shares_unauthenticated_returns_401(self, app, unauthenticated_client): + """Unauthenticated request returns 401.""" + client = unauthenticated_client(app) + + resp = client.get("/conversations/sess-001/shares") + + assert resp.status_code == 401 + + +# --------------------------------------------------------------------------- +# Export shared conversation +# --------------------------------------------------------------------------- + +class TestExportSharedConversation: + """POST /shares/{share_id}/export""" + + def test_export_returns_201(self, app, make_user, authenticated_client, mock_share_service): + """Successful export returns 201 with new session details.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.export_shared_conversation = AsyncMock( + return_value={"sessionId": "new-sess-001", "title": "Test Conversation (shared)"} + ) + + resp = client.post("/shares/share-001/export") + + assert resp.status_code == 201 + body = resp.json() + assert body["sessionId"] == "new-sess-001" + assert body["title"] == "Test Conversation (shared)" + + def test_export_access_denied_returns_403(self, app, make_user, authenticated_client, mock_share_service): + """Export with no access returns 403.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.export_shared_conversation = AsyncMock( + side_effect=AccessDeniedError() + ) + + resp = client.post("/shares/share-001/export") + + assert resp.status_code == 403 + + def test_export_not_found_returns_404(self, app, make_user, authenticated_client, mock_share_service): + """Export of non-existent share returns 404.""" + user = make_user() + client = authenticated_client(app, user) + + mock_share_service.export_shared_conversation = AsyncMock( + side_effect=ShareNotFoundError() + ) + + resp = client.post("/shares/share-999/export") + + assert resp.status_code == 404 + + def test_export_unauthenticated_returns_401(self, app, unauthenticated_client): + """Unauthenticated export returns 401.""" + client = unauthenticated_client(app) + + resp = client.post("/shares/share-001/export") + + assert resp.status_code == 401 diff --git a/backend/uv.lock b/backend/uv.lock index df70b547..b9d9566f 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 2 +revision = 3 requires-python = ">=3.10" resolution-markers = [ "python_full_version >= '3.13'", @@ -9,7 +9,7 @@ resolution-markers = [ [[package]] name = "agentcore-stack" -version = "1.0.0b18" +version = "1.0.0b19" source = { editable = "." } dependencies = [ { name = "aiofiles" }, diff --git a/frontend/ai.client/angular.json b/frontend/ai.client/angular.json index f448db11..531f9eea 100644 --- a/frontend/ai.client/angular.json +++ b/frontend/ai.client/angular.json @@ -19,7 +19,6 @@ "options": { "browser": "src/main.ts", "tsConfig": "tsconfig.app.json", - "optimization": false, "extractLicenses": false, "sourceMap": true, "assets": [ @@ -50,6 +49,9 @@ }, "configurations": { "production": { + "optimization": true, + "sourceMap": false, + "extractLicenses": true, "budgets": [ { "type": "initial", @@ -58,8 +60,8 @@ }, { "type": "anyComponentStyle", - "maximumWarning": "4kB", - "maximumError": "5MB" + "maximumWarning": "200kB", + "maximumError": "500kB" } ], "outputHashing": "all" diff --git a/frontend/ai.client/package-lock.json b/frontend/ai.client/package-lock.json index 4f18f106..ba105781 100644 --- a/frontend/ai.client/package-lock.json +++ b/frontend/ai.client/package-lock.json @@ -1,12 +1,12 @@ { "name": "ai.client", - "version": "1.0.0-beta.18", + "version": "1.0.0-beta.19", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ai.client", - "version": "1.0.0-beta.18", + "version": "1.0.0-beta.19", "dependencies": { "@angular/cdk": "21.2.3", "@angular/common": "21.2.5", diff --git a/frontend/ai.client/package.json b/frontend/ai.client/package.json index a94d5c96..544b0604 100644 --- a/frontend/ai.client/package.json +++ b/frontend/ai.client/package.json @@ -1,6 +1,6 @@ { "name": "ai.client", - "version": "1.0.0-beta.18", + "version": "1.0.0-beta.19", "scripts": { "ng": "ng", "start": "ng serve", diff --git a/frontend/ai.client/src/app/app.routes.ts b/frontend/ai.client/src/app/app.routes.ts index 31699634..46aebf60 100644 --- a/frontend/ai.client/src/app/app.routes.ts +++ b/frontend/ai.client/src/app/app.routes.ts @@ -14,6 +14,11 @@ export const routes: Routes = [ loadComponent: () => import('./session/session.page').then(m => m.ConversationPage), canActivate: [authGuard], }, + { + path: 'shared/:shareId', + loadComponent: () => import('./shared/shared-view.page').then(m => m.SharedViewPage), + canActivate: [authGuard], + }, { path: 'auth/login', loadComponent: () => import('./auth/login/login.page').then(m => m.LoginPage), diff --git a/frontend/ai.client/src/app/components/sidenav/components/session-list/session-list.html b/frontend/ai.client/src/app/components/sidenav/components/session-list/session-list.html index 9f0dbf33..9a1a55e9 100644 --- a/frontend/ai.client/src/app/components/sidenav/components/session-list/session-list.html +++ b/frontend/ai.client/src/app/components/sidenav/components/session-list/session-list.html @@ -78,6 +78,16 @@