[Chat] Add agentic memory provider#11380
Conversation
Signed-off-by: Lin Wang <wonglam@amazon.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant ChatPlugin
participant AgenticMemoryProvider
participant ServerRoutes
participant MLRouter
participant MLCommons
ChatPlugin->>ChatPlugin: Read mlCommonsAgentId from config
ChatPlugin->>AgenticMemoryProvider: new AgenticMemoryProvider(http)
ChatPlugin->>ChatPlugin: Register via setMemoryProvider
Note over ChatPlugin,MLCommons: Conversation Retrieval Flow
ChatPlugin->>AgenticMemoryProvider: getConversation(threadId)
AgenticMemoryProvider->>ServerRoutes: POST /api/chat/proxy (stream)
ServerRoutes->>MLRouter: proxyRequest({path, body, context})
MLRouter->>MLCommons: Agent API call (streaming SSE)
MLCommons-->>MLRouter: Streamed events (RUN_STARTED, MESSAGES_SNAPSHOT, RUN_FINISHED)
MLRouter-->>ServerRoutes: Event stream
ServerRoutes-->>AgenticMemoryProvider: Parsed response body
AgenticMemoryProvider->>AgenticMemoryProvider: parseStreamingResponse(text)
AgenticMemoryProvider-->>ChatPlugin: Event sequence
sequenceDiagram
participant Client
participant ConversationHistoryService
participant AgenticMemoryProvider
participant ServerRoutes
participant MemoryAPI
Client->>ConversationHistoryService: getConversations(params)
ConversationHistoryService->>AgenticMemoryProvider: getConversations(params)
AgenticMemoryProvider->>ServerRoutes: POST /api/chat/memory/sessions/search
ServerRoutes->>MemoryAPI: Search sessions with pagination
MemoryAPI-->>ServerRoutes: {hits, total}
ServerRoutes-->>AgenticMemoryProvider: Sessions data
AgenticMemoryProvider->>AgenticMemoryProvider: Map to SavedConversation objects
AgenticMemoryProvider-->>ConversationHistoryService: {conversations, total, hasMore}
ConversationHistoryService-->>Client: Paginated conversations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11380 +/- ##
==========================================
+ Coverage 60.29% 60.31% +0.02%
==========================================
Files 4664 4666 +2
Lines 130372 130449 +77
Branches 22233 22252 +19
==========================================
+ Hits 78603 78676 +73
+ Misses 46161 46159 -2
- Partials 5608 5614 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wanglam
requested review from
AMoo-Miki,
BionIT,
Flyingliuhub,
LDrago27,
SuZhou-Joe,
ZilongX,
abbyhu2000,
ananzh,
ashwin-pc,
bandinib-amzn,
curq,
huyaboo,
joshuali925,
joshuarrrr,
kavilla,
manasvinibs,
mengweieric,
ruanyl,
sejli,
virajsanghvi,
xinruiba,
zengyan-amazon,
zhongnansu and
zhyuanqi
as code owners
wanglam
requested review from
TackAdam,
d-buckner,
paulstn,
ps48,
raintygao,
ruchidh and
sumukhswamy
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts (1)
47-53: Preferunknownoveranyforbodyand return type.Per coding guidelines,
anyshould be avoided. Usingunknownforbodyand the return type would enforce type narrowing at call sites.Proposed fix
proxyRequest(options: { context: RequestHandlerContext; method: string; path: string; - body?: any; + body?: unknown; dataSourceId?: string; - }): Promise<any>; + }): Promise<unknown>;As per coding guidelines, "Use TypeScript for all new code; avoid
anytype (useunknownor generics instead)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts` around lines 47 - 53, Update the proxyRequest signature in ml_agent_router.ts to replace any usages of "any" with "unknown": change the parameter "body?: any" to "body?: unknown" and change the return type from Promise<any> to Promise<unknown> (or a specific generic type if callers can provide one); update the implementation of proxyRequest (and its callers) to properly narrow/validate the unknown body and cast or map the unknown return before use so type-safety is preserved in functions referencing proxyRequest.src/plugins/chat/public/services/conversation_history_service.ts (1)
74-79: RedundantgetMemoryProvider()call and minor grammar nit.
getMemoryProvider()is called on line 74 for the guard check and again on line 79. Store the result in a local variable to avoid the redundant call. Also, line 75 comment: "Don't need to manual save" → "Don't need to manually save".Proposed fix
+ const provider = this.getMemoryProvider(); + - if (!this.getMemoryProvider().includeFullHistory) { - // Don't need to manual save conversations + if (!provider.includeFullHistory) { + // Don't need to manually save conversations return; } - const provider = this.getMemoryProvider(); // Get existing conversation events to preserve createdAt timestamp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/chat/public/services/conversation_history_service.ts` around lines 74 - 79, Replace the redundant getMemoryProvider() call by caching its result in a local variable and update the comment grammar: call getMemoryProvider() once (e.g., const provider = this.getMemoryProvider()) before the guard, use provider.includeFullHistory for the check, and keep using provider afterwards (replace uses of this.getMemoryProvider()); also change the comment text from "Don't need to manual save" to "Don't need to manually save". Ensure these changes are made in the method containing getMemoryProvider() and where provider is used to control conversation saving.src/plugins/chat/public/services/agentic_memory_provider.test.ts (2)
7-8: Combine imports from the same module.Lines 7 and 8 both import from
'../../../../core/public'. Merge into a single import statement.Proposed fix
-import { HttpSetup } from '../../../../core/public'; -import { EventType } from '../../../../core/public'; +import { HttpSetup, EventType } from '../../../../core/public';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/chat/public/services/agentic_memory_provider.test.ts` around lines 7 - 8, Merge the two separate imports from the same module into one import statement: replace the two lines that individually import HttpSetup and EventType with a single combined import that imports both symbols (HttpSetup, EventType) from '../../../../core/public' so you only have one import declaration for that module.
136-158: DuplicatedcreateMockStreamingResponsehelper.The
createMockStreamingResponsefunction is identically defined in both thegetConversationdescribe block (lines 136–158) and theintegration scenariosdescribe block (lines 333–355). Extract it to the top-leveldescribe('AgenticMemoryProvider')scope to avoid duplication.Proposed fix — hoist the helper
Move the helper before the nested describe blocks (e.g., after
beforeEach/afterEachat lines 29–31), and handlefetchsave/restore at that level as well:afterEach(() => { jest.clearAllMocks(); }); + + const createMockStreamingResponse = (content: string) => { + const encoder = new TextEncoder(); + const bytes = encoder.encode(content); + let readCalled = false; + return { + ok: true, + status: 200, + body: { + getReader: () => ({ + read: async () => { + if (readCalled) { + return { done: true, value: undefined }; + } + readCalled = true; + return { done: false, value: bytes }; + }, + releaseLock: () => {}, + }), + }, + } as Response; + };Then remove the duplicate definitions in both inner
describeblocks.Also applies to: 333-355
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/chat/public/services/agentic_memory_provider.test.ts` around lines 136 - 158, The helper createMockStreamingResponse is duplicated inside two nested describe blocks; move a single implementation to the top-level describe('AgenticMemoryProvider') scope (e.g., immediately after the existing beforeEach/afterEach) and remove the two inner copies. Also hoist any fetch save/restore logic currently handled inside those inner describes to the top-level scope so the global test setup/teardown applies once; update references to createMockStreamingResponse in tests to use the hoisted helper.src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts (1)
88-100: Multipleanytypes inproxyRequestsignature.
body?: anyandPromise<any>violate the coding guidelines. Consider usingunknownto enforce type safety at call sites.As per coding guidelines, "Use TypeScript for all new code; avoid
anytype (useunknownor generics instead)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts` around lines 88 - 100, The proxyRequest signature currently uses unsafe any for the request body and return type; update the method to avoid any by either making it generic (e.g., proxyRequest<TBody = unknown, TResult = unknown>(... body?: TBody): Promise<TResult>) or at minimum replace body?: any with body?: unknown and Promise<any> with Promise<unknown> in the proxyRequest declaration and all related call sites/returns; keep the function name proxyRequest and its parameter structure unchanged, adjust internal typing/casts where necessary to assert or parse the unknown body/result before use.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@changelogs/fragments/11380.yml`:
- Around line 1-2: Add a YAML document start and fix the list indentation and
quoting: add a leading '---' line, ensure the 'feat:' key has the list item
indented under it (the '-' must be on the next indented line under 'feat'), wrap
the special string starting with [Chat] in quotes to avoid being parsed as a
flow sequence, and ensure the file ends with a trailing newline; locate this
change around the 'feat' key and the existing list item text to apply the fixes.
In `@src/plugins/chat/public/services/agentic_memory_provider.ts`:
- Around line 98-100: The threadId is incorrectly set to hit._id; update the
mapping in the list construction (the object that currently sets id: hit._id,
threadId: hit._id, name: ...) to use the memory container identifier from the
search result (use hit._source.memory_container_id for threadId), and keep a
fallback to hit._id if memory_container_id is undefined so existing results
remain selectable; locate this change in the agentic_memory_provider
list/transform code where id and threadId are assigned.
- Around line 158-177: The SSE read loop drops a trailing incomplete line
because sseBuffer is overwritten but never flushed when the stream ends; after
the while(true) loop that reads from reader (and uses sseBuffer, responseText,
TextDecoder, reader.read()), check if sseBuffer is non-empty and append it to
responseText (add a trailing newline or otherwise treat it as a complete line)
so the final buffered "data:" line is not lost before any downstream parsing
logic.
- Around line 131-147: The fetch call hardcodes '/api/chat/proxy' which ignores
Kibana/Dashboards basePath; update the URL in the fetch inside
agentic_memory_provider.ts to use this.http.basePath.prepend('/api/chat/proxy')
(e.g., const url = this.http.basePath.prepend('/api/chat/proxy'); const response
= await fetch(url, { ... })) so the request respects the configured base path;
ensure headers/body remain unchanged and only the request URL is replaced.
In `@src/plugins/chat/server/routes/index.ts`:
- Around line 161-165: The handler currently substitutes a hardcoded
'fake-memory-container-id' when memoryContainerId is missing (variable
memoryContainerId in the route), which leads to broken downstream searches;
instead, remove the fallback and fail fast: when memoryContainerId is falsy, log
an error with logger.error including context (agent id or request info) and
return an error response to the caller (e.g., throw or send a 400/500 with a
clear message) so the missing configuration is surfaced rather than performing a
search with a fake id.
- Around line 184-192: The catch block uses error (unknown) directly for
error.statusCode and error.message; add type narrowing before accessing those
properties: inside the catch in the same function wrap access with a guard such
as checking typeof error === 'object' && error !== null and 'statusCode' in
error (and that (error as any).statusCode is a number) to extract statusCode,
and use error instanceof Error to get the message (falling back to (error as
any).message or a default string). Update the logger.error and
response.customError calls to use the narrowed variables (e.g., statusCode and
message) instead of accessing error.statusCode/error.message directly so
logger.error, response.customError, and the catch block handle unknown errors
safely.
In `@src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts`:
- Around line 115-118: The proxyRequest implementation currently passes an empty
object cast as OpenSearchDashboardsRequest to mlClient.request(), which can drop
headers/auth used by external ML clients; update proxyRequest (and its call
sites) to accept an optional request parameter (matching the forward method
signature) and forward that actual request object into mlClient.request(request,
context) instead of {} so external ML client plugins receive real request
properties; adjust proxyRequest's signature and any callers to pass through the
original OpenSearchDashboardsRequest when available.
---
Nitpick comments:
In `@src/plugins/chat/public/services/agentic_memory_provider.test.ts`:
- Around line 7-8: Merge the two separate imports from the same module into one
import statement: replace the two lines that individually import HttpSetup and
EventType with a single combined import that imports both symbols (HttpSetup,
EventType) from '../../../../core/public' so you only have one import
declaration for that module.
- Around line 136-158: The helper createMockStreamingResponse is duplicated
inside two nested describe blocks; move a single implementation to the top-level
describe('AgenticMemoryProvider') scope (e.g., immediately after the existing
beforeEach/afterEach) and remove the two inner copies. Also hoist any fetch
save/restore logic currently handled inside those inner describes to the
top-level scope so the global test setup/teardown applies once; update
references to createMockStreamingResponse in tests to use the hoisted helper.
In `@src/plugins/chat/public/services/conversation_history_service.ts`:
- Around line 74-79: Replace the redundant getMemoryProvider() call by caching
its result in a local variable and update the comment grammar: call
getMemoryProvider() once (e.g., const provider = this.getMemoryProvider())
before the guard, use provider.includeFullHistory for the check, and keep using
provider afterwards (replace uses of this.getMemoryProvider()); also change the
comment text from "Don't need to manual save" to "Don't need to manually save".
Ensure these changes are made in the method containing getMemoryProvider() and
where provider is used to control conversation saving.
In `@src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts`:
- Around line 88-100: The proxyRequest signature currently uses unsafe any for
the request body and return type; update the method to avoid any by either
making it generic (e.g., proxyRequest<TBody = unknown, TResult = unknown>(...
body?: TBody): Promise<TResult>) or at minimum replace body?: any with body?:
unknown and Promise<any> with Promise<unknown> in the proxyRequest declaration
and all related call sites/returns; keep the function name proxyRequest and its
parameter structure unchanged, adjust internal typing/casts where necessary to
assert or parse the unknown body/result before use.
In `@src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts`:
- Around line 47-53: Update the proxyRequest signature in ml_agent_router.ts to
replace any usages of "any" with "unknown": change the parameter "body?: any" to
"body?: unknown" and change the return type from Promise<any> to
Promise<unknown> (or a specific generic type if callers can provide one); update
the implementation of proxyRequest (and its callers) to properly narrow/validate
the unknown body and cast or map the unknown return before use so type-safety is
preserved in functions referencing proxyRequest.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
changelogs/fragments/11380.ymlsrc/plugins/chat/public/plugin.tssrc/plugins/chat/public/services/agentic_memory_provider.test.tssrc/plugins/chat/public/services/agentic_memory_provider.tssrc/plugins/chat/public/services/conversation_history_service.tssrc/plugins/chat/public/services/index.tssrc/plugins/chat/server/routes/index.tssrc/plugins/chat/server/routes/ml_routes/generic_ml_router.tssrc/plugins/chat/server/routes/ml_routes/ml_agent_router.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json,scss,css,html,yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
Use snake_case for all filenames
Files:
changelogs/fragments/11380.ymlsrc/plugins/chat/server/routes/index.tssrc/plugins/chat/public/services/conversation_history_service.tssrc/plugins/chat/public/services/index.tssrc/plugins/chat/public/services/agentic_memory_provider.test.tssrc/plugins/chat/server/routes/ml_routes/generic_ml_router.tssrc/plugins/chat/public/services/agentic_memory_provider.tssrc/plugins/chat/server/routes/ml_routes/ml_agent_router.tssrc/plugins/chat/public/plugin.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript for all new code; avoidanytype (useunknownor generics instead)
Avoid non-null assertions (!.) and@ts-ignorecomments
HTML attributes:idanddata-test-subjvalues should be camelCase
Import.scssfiles at the top of the component file and use 3-letter prefix on class names for scoping (e.g.,plgComponent)
Prefer functional components in React and name action props ason<Subject><Change>
Files:
src/plugins/chat/server/routes/index.tssrc/plugins/chat/public/services/conversation_history_service.tssrc/plugins/chat/public/services/index.tssrc/plugins/chat/public/services/agentic_memory_provider.test.tssrc/plugins/chat/server/routes/ml_routes/generic_ml_router.tssrc/plugins/chat/public/services/agentic_memory_provider.tssrc/plugins/chat/server/routes/ml_routes/ml_agent_router.tssrc/plugins/chat/public/plugin.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Use named exports, not default exports
Use ES2015 import/export syntax; only import top-level module APIs, never reach into internal paths
Use single quotes for strings with 100-character print width and ES5 trailing commas (enforced by Prettier)
Files:
src/plugins/chat/server/routes/index.tssrc/plugins/chat/public/services/conversation_history_service.tssrc/plugins/chat/public/services/index.tssrc/plugins/chat/public/services/agentic_memory_provider.test.tssrc/plugins/chat/server/routes/ml_routes/generic_ml_router.tssrc/plugins/chat/public/services/agentic_memory_provider.tssrc/plugins/chat/server/routes/ml_routes/ml_agent_router.tssrc/plugins/chat/public/plugin.ts
**/server/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
API routes must start with
/api/and usesnake_casefor paths, parameters, and body fields
Files:
src/plugins/chat/server/routes/index.tssrc/plugins/chat/server/routes/ml_routes/generic_ml_router.tssrc/plugins/chat/server/routes/ml_routes/ml_agent_router.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Unit tests must achieve 80%+ code coverage (enforced by Codecov); use react-testing-library for components, not enzyme snapshots
Files:
src/plugins/chat/public/services/agentic_memory_provider.test.ts
🧬 Code graph analysis (5)
src/plugins/chat/server/routes/index.ts (2)
src/plugins/chat/server/routes/ml_routes/router_registry.ts (1)
MLAgentRouterRegistry(16-53)src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts (1)
MLAgentRouterFactory(65-90)
src/plugins/chat/public/services/agentic_memory_provider.test.ts (1)
src/plugins/chat/public/services/agentic_memory_provider.ts (1)
AgenticMemoryProvider(30-289)
src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts (1)
src/core/server/index.ts (3)
RequestHandlerContext(436-460)OpenSearchClient(146-146)OpenSearchDashboardsRequest(187-187)
src/plugins/chat/server/routes/ml_routes/ml_agent_router.ts (1)
src/core/server/index.ts (1)
RequestHandlerContext(436-460)
src/plugins/chat/public/plugin.ts (1)
src/plugins/chat/public/services/agentic_memory_provider.ts (1)
AgenticMemoryProvider(30-289)
🪛 YAMLlint (1.38.0)
changelogs/fragments/11380.yml
[error] 2-2: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (3)
src/plugins/chat/public/plugin.ts (1)
142-152: LGTM!Good defensive pattern — the try/catch ensures the plugin doesn't crash if the agentic memory provider fails to initialize, falling back gracefully to the default provider. The conditional check on
setMemoryProviderensures backward compatibility.src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts (1)
165-166: LGTM — extractingrequestBodyas a local variable.Clean refactor to avoid repeated
request.bodycasts.src/plugins/chat/public/services/index.ts (1)
8-8: No action required—ChatWindowStateremoval poses no risk.No code imports
ChatWindowStatefrom the services module. All imports source it directly fromcore/public, making this change safe.
| feat: | ||
| - [Chat] Add agentic memory provider ([#11380](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380)) No newline at end of file |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
# First, check if yamllint is available and test the file
if command -v yamllint &> /dev/null; then
echo "=== Testing original file with yamllint ==="
yamllint changelogs/fragments/11380.yml
else
echo "yamllint not available, trying python yaml parser"
python3 << 'PYEOF'
import yaml
try:
with open('changelogs/fragments/11380.yml', 'r') as f:
content = f.read()
print("File content:")
print(content)
print("\n=== Parsing with PyYAML ===")
result = yaml.safe_load(content)
print("✓ Valid YAML")
print("Parsed result:", result)
except yaml.YAMLError as e:
print("✗ YAML Error:", e)
PYEOF
fiRepository: opensearch-project/OpenSearch-Dashboards
Repository: opensearch-project/OpenSearch-Dashboards
Exit code: 1
stdout:
=== Testing original file with yamllint ===
changelogs/fragments/11380.yml
1:1 warning missing document start "---" (document-start)
2:1 error wrong indentation: expected at least 1 (indentation)
2:10 error syntax error: expected <block end>, but found '<scalar>' (syntax)
2:120 error no new line character at the end of file (new-line-at-end-of-file)
Script executed:
#!/bin/bash
# Check other changelog fragments to see markdown link patterns
echo "=== Checking other changelog fragments ==="
find changelogs/fragments -name "*.yml" -type f ! -name "11380.yml" | head -10 | while read file; do
echo "--- File: $file ---"
cat "$file"
echo ""
doneRepository: opensearch-project/OpenSearch-Dashboards
Repository: opensearch-project/OpenSearch-Dashboards
Exit code: 0
stdout:
=== Checking other changelog fragments ===
--- File: changelogs/fragments/11020.yml ---
fix:
- Add support for create api when there is no specifying id ([`#11020`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11020))
--- File: changelogs/fragments/11132.yml ---
fix:
- Update the graph ([`#11132`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11132))
--- File: changelogs/fragments/11187.yml ---
infra:
- [Core] Upgrade React from 16 to 18 ([`#11187`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11187))
--- File: changelogs/fragments/11253.yml ---
feat:
- Update discover layout ([`#11253`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11253))
--- File: changelogs/fragments/11261.yml ---
test:
- Minor Updates to the test utils ([`#11261`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11261))
--- File: changelogs/fragments/11280.yml ---
fix:
- Remove meta field for prometheus data-connection ([`#11280`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11280))
--- File: changelogs/fragments/11285.yml ---
fix:
- Remove no need global variables ch in mocha ([`#11285`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11285))
--- File: changelogs/fragments/11287.yml ---
feat:
- Add include screenshot in chatbot for dashboards page ([`#11287`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11287))
--- File: changelogs/fragments/11290.yml ---
fix:
- Replace shallow rendering with full rendering as part of react 18 recommendation ([`#11290`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11290))
--- File: changelogs/fragments/11291.yml ---
chore:
- Add starter CLAUDE.md ([`#11291`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11291))
Script executed:
#!/bin/bash
# Test the proposed fix
echo "=== Testing proposed fix ==="
python3 << 'PYEOF'
import yaml
test_content = '''feat:
- "[Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))"
'''
try:
result = yaml.safe_load(test_content)
print("✓ Quoted version is valid YAML")
print("Parsed result:", result)
except yaml.YAMLError as e:
print("✗ YAML Error with quoted version:", e)
# Also test without quotes
test_content_unquoted = '''feat:
- [Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))
'''
try:
result = yaml.safe_load(test_content_unquoted)
print("\n✓ Unquoted version is valid YAML")
print("Parsed result:", result)
except yaml.YAMLError as e:
print("\n✗ YAML Error with unquoted version:", e)
PYEOFRepository: opensearch-project/OpenSearch-Dashboards
Repository: opensearch-project/OpenSearch-Dashboards
Exit code: 0
stdout:
=== Testing proposed fix ===
✓ Quoted version is valid YAML
Parsed result: {'feat': ['[Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))']}
✗ YAML Error with unquoted version: while parsing a block mapping
in "<unicode string>", line 1, column 1:
feat:
^
expected <block end>, but found '<scalar>'
in "<unicode string>", line 2, column 10:
- [Chat] Add agentic memory provider ([`#1` ...
^
Script executed:
#!/bin/bash
# Test the quoted fix with yamllint
python3 << 'PYEOF'
# Create test file content with the proposed fix and proper formatting
test_content_fixed = '''---
feat:
- "[Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))"
'''
import yaml
try:
result = yaml.safe_load(test_content_fixed)
print("✓ Fixed version (with document start and newline) is valid YAML")
print("Parsed result:", result)
except yaml.YAMLError as e:
print("✗ YAML Error:", e)
PYEOFRepository: opensearch-project/OpenSearch-Dashboards
Repository: opensearch-project/OpenSearch-Dashboards
Exit code: 0
stdout:
✓ Fixed version (with document start and newline) is valid YAML
Parsed result: {'feat': ['[Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))']}
Script executed:
#!/bin/bash
# Test with yamllint specifically
if command -v yamllint &> /dev/null; then
# Create a temporary file with the proposed fix
cat > /tmp/test_fix.yml << 'EOF'
---
feat:
- "[Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))"
EOF
echo "=== Testing fixed version with yamllint ==="
yamllint /tmp/test_fix.yml
fiRepository: opensearch-project/OpenSearch-Dashboards
Repository: opensearch-project/OpenSearch-Dashboards
Exit code: 1
stdout:
=== Testing fixed version with yamllint ===
/tmp/test_fix.yml
3:1 error wrong indentation: expected at least 1 (indentation)
3:81 error line too long (121 > 80 characters) (line-length)
Fix multiple YAML formatting issues: add proper indentation, quote special characters, and ensure correct document structure.
The file has several YAML validation errors:
- Line 2 indentation: The list item (
-) must be indented underfeat:to be valid YAML - Line 2 syntax: The unquoted string starting with
[Chat]is parsed as a YAML flow sequence, causing the syntax error - Missing document start: Add
---at the beginning - Missing newline: Add newline at end of file
The proposed quoting alone doesn't fix the indentation error. The list item must be properly indented:
Proposed fix
+---
feat:
- - [Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))
+ - "[Chat] Add agentic memory provider ([`#11380`](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/11380))"🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 2-2: syntax error: expected , but found ''
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@changelogs/fragments/11380.yml` around lines 1 - 2, Add a YAML document start
and fix the list indentation and quoting: add a leading '---' line, ensure the
'feat:' key has the list item indented under it (the '-' must be on the next
indented line under 'feat'), wrap the special string starting with [Chat] in
quotes to avoid being parsed as a flow sequence, and ensure the file ends with a
trailing newline; locate this change around the 'feat' key and the existing list
item text to apply the fixes.
| id: hit._id, | ||
| threadId: hit._id, | ||
| name: hit._source.summary || 'Untitled Conversation', |
There was a problem hiding this comment.
Map threadId from memory_container_id, not _id.
This list API returns memory_container_id, but threadId is currently set to _id. If those diverge, loading a selected conversation can target the wrong thread.
Proposed fix
}) => ({
id: hit._id,
- threadId: hit._id,
+ threadId: hit._source.memory_container_id || hit._id,
name: hit._source.summary || 'Untitled Conversation',
messages: [], // Messages will be loaded separately when needed
createdAt: hit._source.created_time,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id: hit._id, | |
| threadId: hit._id, | |
| name: hit._source.summary || 'Untitled Conversation', | |
| id: hit._id, | |
| threadId: hit._source.memory_container_id || hit._id, | |
| name: hit._source.summary || 'Untitled Conversation', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/chat/public/services/agentic_memory_provider.ts` around lines 98
- 100, The threadId is incorrectly set to hit._id; update the mapping in the
list construction (the object that currently sets id: hit._id, threadId:
hit._id, name: ...) to use the memory container identifier from the search
result (use hit._source.memory_container_id for threadId), and keep a fallback
to hit._id if memory_container_id is undefined so existing results remain
selectable; locate this change in the agentic_memory_provider list/transform
code where id and threadId are assigned.
There was a problem hiding this comment.
The memory_container_id can't been used as a threadId. Let's keep the current implementation.
| const response = await fetch('/api/chat/proxy', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'text/event-stream', | ||
| 'osd-xsrf': 'true', | ||
| }, | ||
| body: JSON.stringify({ | ||
| threadId, | ||
| runId: `restore-${Date.now()}`, | ||
| messages: [], | ||
| tools: [], | ||
| context: [], | ||
| state: {}, | ||
| forwardedProps: {}, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if file exists =="
if [ -f "src/plugins/chat/public/services/agentic_memory_provider.ts" ]; then
echo "File found."
echo "== Examine lines 131-147 =="
sed -n '131,147p' src/plugins/chat/public/services/agentic_memory_provider.ts
else
echo "File not found."
fiRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for chat proxy calls and basePath usage =="
rg -n --type=ts --type=tsx -C2 "chat/proxy|basePath\.prepend\('/api/chat/proxy'\)|basePath\.prepend\(\"/api/chat/proxy\"\)"
echo
echo "== Inspect HttpSetup/basePath references =="
rg -n --type=ts -C2 "interface\s+HttpSetup|type\s+HttpSetup|basePath\s*:" | head -50Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Examine class structure and constructor for http property =="
sed -n '1,150p' src/plugins/chat/public/services/agentic_memory_provider.ts | head -100Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 2996
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find HttpSetup interface definition =="
rg -n "interface HttpSetup|type HttpSetup" --type=ts | head -20
echo
echo "== Search for basePath.prepend usage patterns =="
rg -n "basePath\.prepend\(" --type=ts | head -30
echo
echo "== Search for /api/ calls with fetch or http.post/http.get =="
rg -n "fetch\('\/api\/|http\.(post|get)\('\/api\/" --type=ts | head -30Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 4942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for other /api/ calls in the agentic_memory_provider file =="
cat src/plugins/chat/public/services/agentic_memory_provider.ts | grep -n "fetch\|http\." | head -20Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 341
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Look at full agentic_memory_provider file to see all API patterns =="
wc -l src/plugins/chat/public/services/agentic_memory_provider.ts
echo
echo "== Check for other fetch calls in the file =="
rg -n "fetch\(" src/plugins/chat/public/services/agentic_memory_provider.ts -A2 -B2Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 557
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check HttpSetup interface definition =="
sed -n '35,80p' src/core/public/http/types.ts
echo
echo "== Search for fetch() calls with basePath.prepend =="
rg -n "basePath\.prepend" --type=ts -B2 -A2 | grep -E "fetch|basePath" | head -30
echo
echo "== Check the full http.post call in agentic_memory_provider =="
sed -n '60,85p' src/plugins/chat/public/services/agentic_memory_provider.ts
echo
echo "== Search for other /api/ fetch calls in chat plugin =="
rg -n "fetch\(" src/plugins/chat/public --type=ts | head -20Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 6091
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Look at how other services handle streaming/fetch with basePath =="
rg -n "fetch.*basePath|basePath.*fetch" --type=ts -B2 -A2 | head -40
echo
echo "== Check if there are any basePath examples with API calls =="
rg -n "basePath\.prepend\('\/api" --type=ts | head -20Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 1172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check IBasePath interface to understand basePath.prepend =="
rg -n "interface IBasePath|type IBasePath" --type=ts -A20 | head -50
echo
echo "== Look for native fetch usage with API paths in the codebase =="
rg -n "fetch\(" src/plugins/chat/public --type=ts -B2 -A5
echo
echo "== Check if http.post also needs basePath for the memory API call =="
sed -n '55,90p' src/plugins/chat/public/services/agentic_memory_provider.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 4157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search for how http.post is called with API paths =="
rg -n "this\.http\.post\(" src/plugins/chat/public --type=ts -B1 -A1
echo
echo "== Check if http.post automatically handles basePath =="
rg -n "class.*Fetch|class.*HttpClient" src/core/public/http --type=ts | head -5
echo
echo "== Look at HttpHandler implementation =="
rg -n "type HttpHandler|interface HttpHandler" src/core/public/http/types.ts -A10Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check HttpHandler to see if it automatically handles basePath =="
rg -n "type HttpHandler" src/core/public/http/types.ts -A15
echo
echo "== Check Fetch class to understand how it handles basePath =="
sed -n '1,100p' src/core/public/http/fetch.ts | head -80
echo
echo "== Verify ag_ui_agent.ts proxyUrl handling =="
sed -n '40,65p' src/plugins/chat/public/services/ag_ui_agent.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check how Fetch service processes URLs =="
rg -n "basePath" src/core/public/http/fetch.ts -B2 -A2 | head -40
echo
echo "== Confirm if http.post/http.get automatically prepend basePath =="
rg -n "prepend.*url|basePath\.prepend" src/core/public/http/fetch.ts | head -10Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 1148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full context of ag_ui_agent initialization and proxyUrl =="
sed -n '1,80p' src/plugins/chat/public/services/ag_ui_agent.ts
echo
echo "== Check if there's basePath handling in AgUiAgent class =="
rg -n "class AgUiAgent|constructor|basePath" src/plugins/chat/public/services/ag_ui_agent.ts -B1 -A1 | head -40
echo
echo "== Check how the agent is instantiated and passed proxyUrl =="
rg -n "new AgUiAgent|AgUiAgent\(" src/plugins/chat/public --type=ts -B2 -A2Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 4769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if there are examples of basePath.prepend used with fetch =="
rg -n "fetch\(" src/plugins --type=ts -B3 | grep -E "basePath|prepend|fetch" | head -40
echo
echo "== Check ChatService to see full initialization context =="
sed -n '60,85p' src/plugins/chat/public/services/chat_service.ts
echo
echo "== Verify: is shouldPrependBasePath true for API paths =="
rg -n "shouldPrependBasePath" src/core/public/http/fetch.ts -B3 -A3Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 5906
🏁 Script executed:
#!/bin/bash
set -euoripefail
echo "== Check isRelativeUrl logic to understand when basePath is prepended =="
rg -n "isRelativeUrl|shouldPrependBasePath" src/core/public/http/fetch.ts -B2 -A2 | head -60
echo
echo "== Confirm: '/api/chat/proxy' would be considered relative (not absolute URL) =="
rg -n "function isRelativeUrl|const isRelativeUrl" src/core/public/http --type=ts -A5Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 758
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check isRelativeUrl logic to understand when basePath is prepended =="
rg -n "isRelativeUrl|shouldPrependBasePath" src/core/public/http/fetch.ts -B2 -A2 | head -60
echo
echo "== Find isRelativeUrl function definition =="
rg -n "function isRelativeUrl|const isRelativeUrl" src/core/public/http --type=ts -A5Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full context around shouldPrependBasePath logic =="
sed -n '140,175p' src/core/public/http/fetch.ts
echo
echo "== Check if paths starting with / are relative or absolute =="
rg -n "shouldPrependBasePath.*=" src/core/public/http/fetch.ts -B5 -A2Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 1702
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if there's a way proxyUrl could be passed with basePath already included =="
rg -n "proxyUrl|proxy.*url" src/plugins/chat/public/services/agentic_memory_provider.ts
echo
echo "== Verify: is isRelativeUrl from `@osd/std` library =="
rg -n "isRelativeUrl" src --type=ts | grep "export" | head -5
echo
echo "== Check what isRelativeUrl considers relative =="
find src -name "*.ts" -type f -exec grep -l "@osd/std" {} \; | head -1 | xargs grep -l "isRelativeUrl" | head -1Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full agentic_memory_provider.ts to see complete context =="
wc -l src/plugins/chat/public/services/agentic_memory_provider.ts
echo
echo "== Check class members and constructor to confirm http is available throughout =="
sed -n '27,55p' src/plugins/chat/public/services/agentic_memory_provider.ts
echo
echo "== Verify the exact location and context of the fetch call =="
sed -n '125,150p' src/plugins/chat/public/services/agentic_memory_provider.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 2251
Use basePath-aware URL for proxy fetch.
Hardcoding '/api/chat/proxy' breaks when Dashboards is hosted under a non-root base path. Unlike this.http.post() which automatically prepends basePath, native fetch() does not. Build the URL via this.http.basePath.prepend('/api/chat/proxy').
Proposed fix
- const response = await fetch('/api/chat/proxy', {
+ const proxyUrl = this.http.basePath.prepend('/api/chat/proxy');
+ const response = await fetch(proxyUrl, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Accept: 'text/event-stream',
'osd-xsrf': 'true',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const response = await fetch('/api/chat/proxy', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Accept: 'text/event-stream', | |
| 'osd-xsrf': 'true', | |
| }, | |
| body: JSON.stringify({ | |
| threadId, | |
| runId: `restore-${Date.now()}`, | |
| messages: [], | |
| tools: [], | |
| context: [], | |
| state: {}, | |
| forwardedProps: {}, | |
| }), | |
| }); | |
| const proxyUrl = this.http.basePath.prepend('/api/chat/proxy'); | |
| const response = await fetch(proxyUrl, { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Accept: 'text/event-stream', | |
| 'osd-xsrf': 'true', | |
| }, | |
| body: JSON.stringify({ | |
| threadId, | |
| runId: `restore-${Date.now()}`, | |
| messages: [], | |
| tools: [], | |
| context: [], | |
| state: {}, | |
| forwardedProps: {}, | |
| }), | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/chat/public/services/agentic_memory_provider.ts` around lines 131
- 147, The fetch call hardcodes '/api/chat/proxy' which ignores
Kibana/Dashboards basePath; update the URL in the fetch inside
agentic_memory_provider.ts to use this.http.basePath.prepend('/api/chat/proxy')
(e.g., const url = this.http.basePath.prepend('/api/chat/proxy'); const response
= await fetch(url, { ... })) so the request respects the configured base path;
ensure headers/body remain unchanged and only the request URL is replaced.
| let sseBuffer = ''; | ||
| let responseText = ''; | ||
|
|
||
| try { | ||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) break; | ||
|
|
||
| // Parse Server-Sent Events with proper buffering (same as AgUiAgent) | ||
| const chunk = new TextDecoder().decode(value); | ||
| const allData = sseBuffer + chunk; | ||
| const lines = allData.split('\n'); | ||
|
|
||
| // Keep the last incomplete line in buffer | ||
| sseBuffer = lines[lines.length - 1]; | ||
| const completeLines = lines.slice(0, -1); | ||
|
|
||
| // Accumulate all complete lines for parsing | ||
| responseText += completeLines.join('\n') + '\n'; | ||
| } |
There was a problem hiding this comment.
Flush final buffered SSE line after stream end.
The parser only appends newline-terminated lines. If the final data: line has no trailing newline, it is dropped and events/messages can be lost.
Proposed fix
try {
while (true) {
const { done, value } = await reader.read();
if (done) break;
// Parse Server-Sent Events with proper buffering (same as AgUiAgent)
const chunk = new TextDecoder().decode(value);
const allData = sseBuffer + chunk;
const lines = allData.split('\n');
// Keep the last incomplete line in buffer
sseBuffer = lines[lines.length - 1];
const completeLines = lines.slice(0, -1);
// Accumulate all complete lines for parsing
responseText += completeLines.join('\n') + '\n';
}
+
+ if (sseBuffer.trim().length > 0) {
+ responseText += sseBuffer + '\n';
+ }
} finally {
reader.releaseLock();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let sseBuffer = ''; | |
| let responseText = ''; | |
| try { | |
| while (true) { | |
| const { done, value } = await reader.read(); | |
| if (done) break; | |
| // Parse Server-Sent Events with proper buffering (same as AgUiAgent) | |
| const chunk = new TextDecoder().decode(value); | |
| const allData = sseBuffer + chunk; | |
| const lines = allData.split('\n'); | |
| // Keep the last incomplete line in buffer | |
| sseBuffer = lines[lines.length - 1]; | |
| const completeLines = lines.slice(0, -1); | |
| // Accumulate all complete lines for parsing | |
| responseText += completeLines.join('\n') + '\n'; | |
| } | |
| let sseBuffer = ''; | |
| let responseText = ''; | |
| try { | |
| while (true) { | |
| const { done, value } = await reader.read(); | |
| if (done) break; | |
| // Parse Server-Sent Events with proper buffering (same as AgUiAgent) | |
| const chunk = new TextDecoder().decode(value); | |
| const allData = sseBuffer + chunk; | |
| const lines = allData.split('\n'); | |
| // Keep the last incomplete line in buffer | |
| sseBuffer = lines[lines.length - 1]; | |
| const completeLines = lines.slice(0, -1); | |
| // Accumulate all complete lines for parsing | |
| responseText += completeLines.join('\n') + '\n'; | |
| } | |
| if (sseBuffer.trim().length > 0) { | |
| responseText += sseBuffer + '\n'; | |
| } | |
| } finally { | |
| reader.releaseLock(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/chat/public/services/agentic_memory_provider.ts` around lines 158
- 177, The SSE read loop drops a trailing incomplete line because sseBuffer is
overwritten but never flushed when the stream ends; after the while(true) loop
that reads from reader (and uses sseBuffer, responseText, TextDecoder,
reader.read()), check if sseBuffer is non-empty and append it to responseText
(add a trailing newline or otherwise treat it as a complete line) so the final
buffered "data:" line is not lost before any downstream parsing logic.
| // Use fallback fake memory container ID if not found | ||
| if (!memoryContainerId) { | ||
| memoryContainerId = 'fake-memory-container-id'; | ||
| logger.info('Using fallback fake memory container ID'); | ||
| } |
There was a problem hiding this comment.
Fallback to 'fake-memory-container-id' will silently produce broken search requests.
If the agent detail API fails or the agent doesn't have a memory_container_id, the route proceeds with a hardcoded fake ID. The downstream search against /_plugins/_ml/memory_containers/fake-memory-container-id/memories/sessions/_search will almost certainly fail or return empty results, masking the real configuration problem.
Consider returning an error to the caller instead so the issue is surfaced, or at minimum, document why this is an acceptable fallback.
Proposed fix — fail fast instead of using a fake ID
- // Use fallback fake memory container ID if not found
- if (!memoryContainerId) {
- memoryContainerId = 'fake-memory-container-id';
- logger.info('Using fallback fake memory container ID');
- }
+ if (!memoryContainerId) {
+ logger.error('Agent does not have a memory_container_id configured');
+ return response.customError({
+ statusCode: 503,
+ body: {
+ message: 'Agent memory container not configured. Ensure the ML agent has a memory_container_id.',
+ },
+ });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/chat/server/routes/index.ts` around lines 161 - 165, The handler
currently substitutes a hardcoded 'fake-memory-container-id' when
memoryContainerId is missing (variable memoryContainerId in the route), which
leads to broken downstream searches; instead, remove the fallback and fail fast:
when memoryContainerId is falsy, log an error with logger.error including
context (agent id or request info) and return an error response to the caller
(e.g., throw or send a 400/500 with a clear message) so the missing
configuration is surfaced rather than performing a search with a fake id.
| } catch (error) { | ||
| logger.error(`Failed to search memory sessions: ${error}`); | ||
| return response.customError({ | ||
| statusCode: error.statusCode || 500, | ||
| body: { | ||
| message: error instanceof Error ? error.message : 'Failed to search memory sessions', | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Accessing error.statusCode without type narrowing.
In the catch block, error is of type unknown. Accessing error.statusCode (line 187) without narrowing will cause a TypeScript compilation error. Narrow the type or provide a default.
Proposed fix
} catch (error) {
logger.error(`Failed to search memory sessions: ${error}`);
+ const statusCode = error instanceof Error && 'statusCode' in error ? (error as any).statusCode : 500;
return response.customError({
- statusCode: error.statusCode || 500,
+ statusCode: statusCode || 500,
body: {
message: error instanceof Error ? error.message : 'Failed to search memory sessions',
},
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/chat/server/routes/index.ts` around lines 184 - 192, The catch
block uses error (unknown) directly for error.statusCode and error.message; add
type narrowing before accessing those properties: inside the catch in the same
function wrap access with a guard such as checking typeof error === 'object' &&
error !== null and 'statusCode' in error (and that (error as any).statusCode is
a number) to extract statusCode, and use error instanceof Error to get the
message (falling back to (error as any).message or a default string). Update the
logger.error and response.customError calls to use the narrowed variables (e.g.,
statusCode and message) instead of accessing error.statusCode/error.message
directly so logger.error, response.customError, and the catch block handle
unknown errors safely.
| }, | ||
| {} as OpenSearchDashboardsRequest, | ||
| context | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the full context of the proxyRequest method
sed -n '80,140p' src/plugins/chat/server/routes/ml_routes/generic_ml_router.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 1774
🏁 Script executed:
#!/bin/bash
# Check the ML client type/interface definition
rg -n "mlClient\.request" --type=ts -C3 src/plugins/chat/server/routes/ml_routes/Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 1363
🏁 Script executed:
#!/bin/bash
# Search for OpenSearchDashboardsRequest usage patterns in the codebase
rg "OpenSearchDashboardsRequest" --type=ts src/plugins/chat/server/ -A2 -B2 | head -50Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 3479
🏁 Script executed:
#!/bin/bash
# Look for the forward method to see how it handles the request parameter
rg -n "forward\s*\(" --type=ts -A20 src/plugins/chat/server/routes/ml_routes/generic_ml_router.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 857
🏁 Script executed:
#!/bin/bash
# Check the forward method and how it calls mlClient.request (around line 223 mentioned in review)
sed -n '142,250p' src/plugins/chat/server/routes/ml_routes/generic_ml_router.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 3482
🏁 Script executed:
#!/bin/bash
# Search for the ML client type definition/interface
rg "interface.*MLClient|type.*MLClient" --type=ts -A10 -B2 src/plugins/chat/Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 1223
🏁 Script executed:
#!/bin/bash
# Search for where mlClient.request is called to see all usages
rg "mlClient\.request\(" --type=ts -B5 -A10 src/plugins/chat/Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 2955
🏁 Script executed:
#!/bin/bash
# Look for MLClient interface/type definition or any imports from ml plugin
rg "MLClient|from.*ml" --type=ts src/plugins/chat/server/routes/ml_routes/ | head -30Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 1822
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about mlClient.request
rg -B5 -A5 "mlClient\.request" --type=ts src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts | grep -E "//|/\*|\*" | head -40Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 232
🏁 Script executed:
#!/bin/bash
# Check findMLClient function to understand where ML client comes from
rg -n "findMLClient" --type=ts -B3 -A15 src/plugins/chat/Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 7707
🏁 Script executed:
#!/bin/bash
# Check the test file to see how mlClient.request is expected to work
sed -n '1,150p' src/plugins/chat/server/routes/ml_routes/generic_ml_router.test.tsRepository: opensearch-project/OpenSearch-Dashboards
Length of output: 4299
🏁 Script executed:
#!/bin/bash
# Look for any external ML client mocks or how they're tested
rg "mlClient" --type=ts src/plugins/chat/server/routes/ml_routes/generic_ml_router.test.ts -B2 -A5Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 66
🏁 Script executed:
#!/bin/bash
# Check if there are any type definitions or interfaces for the mlClient
rg "request.*\(" --type=ts src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts | grep -E "async|=>" | head -20Repository: opensearch-project/OpenSearch-Dashboards
Length of output: 125
Inconsistent request object handling in proxyRequest method—empty object may break external ML client integration.
The proxyRequest method passes an empty object cast as OpenSearchDashboardsRequest to mlClient.request() (line 116), while the forward method passes the actual request object (line 214). The fallback ML client implementation doesn't use this parameter, but external ML client plugins may rely on request properties (headers, auth tokens, etc.). Align proxyRequest to accept an optional request parameter and pass it to the ML client, matching the pattern used in forward.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts` around lines
115 - 118, The proxyRequest implementation currently passes an empty object cast
as OpenSearchDashboardsRequest to mlClient.request(), which can drop
headers/auth used by external ML clients; update proxyRequest (and its call
sites) to accept an optional request parameter (matching the forward method
signature) and forward that actual request object into mlClient.request(request,
context) instead of {} so external ML client plugins receive real request
properties; adjust proxyRequest's signature and any callers to pass through the
original OpenSearchDashboardsRequest when available.
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
| const messages: Message[] = []; | ||
|
|
||
| // Process collected events | ||
| for (const data of collectedEvents) { |
There was a problem hiding this comment.
Seems we only patch when type === MESSAGES_SNAPSHOT, can we put other branches into an else?
SuZhou-Joe
left a comment
SuZhou-Joe
left a comment
There was a problem hiding this comment.
Thanks for the PR, LGTM but the part we patch the event and payload. Can we ask ML side to make sure agent returns expected result / event?
|
Hi @jiapingzeng, could you help take a look at the comments @SuZhou-Joe left above? We have generated a fake message ID in the frontend to avoid frontend functionality breaking. Can we return the real message ID from the backend side? |
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
Signed-off-by: Lin Wang <wonglam@amazon.com>
PR Reviewer Guide 🔍(Review updated until commit 0ddb055)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 0ddb055
Previous suggestionsSuggestions up to commit c0b7a45
|
|
Checked failed ci group 6 is not related to the code changes in current PR. Should be safe to merge. |
PR Code Analyzer ❗AI-powered 'Code-Diff-Analyzer' found issues on commit 0ddb055.
The table above displays the top 10 most important findings. Pull Requests Author(s): Please update your Pull Request according to the report above. Repository Maintainer(s): You can Thanks. |
|
Persistent review updated to latest commit 0ddb055 |
* Add agentic memory provider Signed-off-by: Lin Wang <wonglam@amazon.com> * Changeset file for PR opensearch-project#11380 created/updated * Refactor to AgUiAgent Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix opensearch request not provided Signed-off-by: Lin Wang <wonglam@amazon.com> * Add unit tests for proxyRequest Signed-off-by: Lin Wang <wonglam@amazon.com> * Add unit tests for chat memory search routes Signed-off-by: Lin Wang <wonglam@amazon.com> * Remove event transfer logic Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix failed unit tests Signed-off-by: Lin Wang <wonglam@amazon.com> * Fix resize bar UI and tool call plural Signed-off-by: Lin Wang <wonglam@amazon.com> --------- Signed-off-by: Lin Wang <wonglam@amazon.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: SuZhou-Joe <suzhou@amazon.com> Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>
4 participants
Description
Introduces
AgenticMemoryProvider, a newConversationMemoryProviderimplementation that stores and retrieves conversation history using ML Commons Agent Memory APIs. Unlike the existing provider, configuration (agent ID and memory container ID) is managed server-side, and conversation state is automatically persisted by the agent — makingsaveConversationa no-op kept for interface compatibility.The provider supports paginated conversation listing via a search endpoint, and restores conversation history by replaying AG-UI events (
RUN_STARTED,MESSAGES_SNAPSHOT,RUN_FINISHED) collected fromAgUiAgent. Conversation deletion is not supported, as the agent manages its own memory lifecycle.New server-side routes and utilities for memory container ID resolution are also included.
Changelog
Check List
yarn test:jestyarn test:jest_integration