[SecuritySolution] Make last conversation local storage keys space aware by angorayc · Pull Request #214794 · elastic/kibana

angorayc · 2025-03-17T14:55:05Z

Summary

Issue and steps to reproduce:
#214114

Screen.Recording.2025-03-18.at.14.52.12.mov

The best approach to fix this is to make local storage keys space aware. In this use case, the current key is elasticAssistantDefault.lastSelectedConversation.

Ideally it should be e.g.: elasticAssistantDefault.lastSelectedConversation.{spaceId}

To retrieve spaceId properly, we have to make sure spaceId has been available when reading the local storage. In other words, the spaceId cannot be null, undefined, or a fallback value when accessing it.

To achieve that, we have to only render the assistant overlay after spaceId is available and remove existing spaceId from the AssistantContext. The reason I removed spaceId from AssistantContext is because it is at the top of the rendering tree and can block the entire app if waiting for the spaceId becomes available there.

useAssistantSpaceId comes from SpaceIdContext which should be render only when spaceId is available, should be safe to access the exact spaceId inside Assistant overly.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Unit or functional tests were updated or added to match the most common scenarios

angorayc · 2025-03-17T14:59:30Z

x-pack/solutions/security/plugins/security_solution/public/assistant/provider.tsx

      inferenceEnabled={inferenceEnabled}
      navigateToApp={navigateToApp}
      productDocBase={productDocBase}
-      spaceId={spaceId ?? 'default'}


This might be going to use for reading the local storage keys. In other words, the spaceId cannot be null, undefined, or a fallback value when accessing it. So I moved it here x-pack/solutions/security/plugins/security_solution/public/assistant/overlay.tsx
to make sure we get space id in ElasticAssistantOverlay correctly.

stephmilovic · 2025-03-17T17:47:36Z

...form/packages/shared/kbn-elastic-assistant/impl/assistant/use_current_conversation/index.tsx

+    async ({
+      cId,
+      isStreamRefetch = false,
+      silent,


I think we should only silence the error when the id comes from the lastConversation, so down in the useEffect on L256

Also, if you're doing the space aware local storage keys, maybe we don't need to silence the error at all?

Yeah, you are right.
I wasn't sure if I wanted to make local storage keys space aware in this PR, as wasn't sure how big the scope were 😅
After having another look, I move the last conversation relevant keys to a separate hook, looks alright so far.
To make the scope smaller, I am only making the last conversation local storage keys space aware.

@stephmilovic I still snooze the error when conversation not found after make the local storage space aware. As I found if I created some conversations, and restart my ES. The not found error would show up.

…-fix'

…s-214114

elasticmachine · 2025-03-26T12:03:50Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 64c7e0c

Failed CI Steps

FTR Configs #75

Test Failures

[job] [logs] FTR Configs #75 / EQL execution logic API @ess @serverless @serverlessQA EQL type rules parses shard failures for EQL event query

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`automaticImport`	723	726	+3
`securitySolution`	7088	7091	+3
total			+6

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/elastic-assistant`	135	141	+6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`securitySolution`	8.9MB	8.9MB	+1.6KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`@kbn/elastic-assistant`	9	11	+2

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`securitySolution`	88.3KB	88.3KB	-1.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/elastic-assistant`	164	170	+6

History

💔 Build #287101 failed 996e07d
💔 Build #286845 failed 10a9349
💔 Build #286744 failed 98af4bc
💔 Build #286709 failed cd74c0b
💛 Build #285721 was flaky cf4d361

stephmilovic

LGTM, thanks for the improvement! It might be too late to backport to 8.18. Make sure you have the 8.19 label on here too!

kibanamachine · 2025-03-26T16:35:07Z

Starting backport for target branches: 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/14088634830

kibanamachine · 2025-03-26T16:35:30Z

Starting backport for target branches: 8.18, 8.x, 9.0

https://github.com/elastic/kibana/actions/runs/14088643235

…are (elastic#214794) ## Summary Issue and steps to reproduce: elastic#214114 https://github.com/user-attachments/assets/881fb082-a879-4816-b251-da3f2af77661 The best approach to fix this is to make local storage keys space aware. In this use case, the current key is `elasticAssistantDefault.lastSelectedConversation`. Ideally it should be e.g.: `elasticAssistantDefault.lastSelectedConversation.{spaceId}` To retrieve spaceId properly, we have to make sure spaceId has been available when reading the local storage. In other words, the spaceId cannot be null, undefined, or a fallback value when accessing it. To achieve that, we have to only render the assistant overlay after spaceId is available and remove existing spaceId from the AssistantContext. The reason I removed spaceId from AssistantContext is because it is at the top of the rendering tree and can block the entire app if waiting for the spaceId becomes available there. `useAssistantSpaceId` comes from `SpaceIdContext` which should be render only when spaceId is available, should be safe to access the exact spaceId inside Assistant overly. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 169abec)

kibanamachine · 2025-03-26T16:41:31Z

💔 Some backports could not be created

Status	Branch	Result
❌	8.18	Backport failed because of merge conflicts
✅	8.x
❌	9.0	Backport failed because of merge conflicts

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 214794

Questions ?

Please refer to the Backport tool documentation

…are (elastic#214794) ## Summary Issue and steps to reproduce: elastic#214114 https://github.com/user-attachments/assets/881fb082-a879-4816-b251-da3f2af77661 The best approach to fix this is to make local storage keys space aware. In this use case, the current key is `elasticAssistantDefault.lastSelectedConversation`. Ideally it should be e.g.: `elasticAssistantDefault.lastSelectedConversation.{spaceId}` To retrieve spaceId properly, we have to make sure spaceId has been available when reading the local storage. In other words, the spaceId cannot be null, undefined, or a fallback value when accessing it. To achieve that, we have to only render the assistant overlay after spaceId is available and remove existing spaceId from the AssistantContext. The reason I removed spaceId from AssistantContext is because it is at the top of the rendering tree and can block the entire app if waiting for the spaceId becomes available there. `useAssistantSpaceId` comes from `SpaceIdContext` which should be render only when spaceId is available, should be safe to access the exact spaceId inside Assistant overly. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 169abec)

kibanamachine · 2025-03-26T16:43:00Z

💔 Some backports could not be created

Status	Branch	Result
❌	8.18	Backport failed because of merge conflicts
✅	8.x
❌	9.0	Backport failed because of merge conflicts

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 214794

Questions ?

Please refer to the Backport tool documentation

…ace aware (#214794) (#216085) # Backport This will backport the following commits from `main` to `8.x`: - [[SecuritySolution] Make last conversation local storage keys space aware (#214794)](#214794)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)  Co-authored-by: Angela Chuang <6295984+angorayc@users.noreply.github.com>

…are (elastic#214794) Issue and steps to reproduce: elastic#214114 https://github.com/user-attachments/assets/881fb082-a879-4816-b251-da3f2af77661 The best approach to fix this is to make local storage keys space aware. In this use case, the current key is `elasticAssistantDefault.lastSelectedConversation`. Ideally it should be e.g.: `elasticAssistantDefault.lastSelectedConversation.{spaceId}` To retrieve spaceId properly, we have to make sure spaceId has been available when reading the local storage. In other words, the spaceId cannot be null, undefined, or a fallback value when accessing it. To achieve that, we have to only render the assistant overlay after spaceId is available and remove existing spaceId from the AssistantContext. The reason I removed spaceId from AssistantContext is because it is at the top of the rendering tree and can block the entire app if waiting for the spaceId becomes available there. `useAssistantSpaceId` comes from `SpaceIdContext` which should be render only when spaceId is available, should be safe to access the exact spaceId inside Assistant overly. Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 169abec)

…ace aware (#214794) (#216214) # Backport This will backport the following commits from `main` to `9.0`: - [[SecuritySolution] Make last conversation local storage keys space aware (#214794)](#214794) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…are (elastic#214794) ## Summary Issue and steps to reproduce: elastic#214114 https://github.com/user-attachments/assets/881fb082-a879-4816-b251-da3f2af77661 The best approach to fix this is to make local storage keys space aware. In this use case, the current key is `elasticAssistantDefault.lastSelectedConversation`. Ideally it should be e.g.: `elasticAssistantDefault.lastSelectedConversation.{spaceId}` To retrieve spaceId properly, we have to make sure spaceId has been available when reading the local storage. In other words, the spaceId cannot be null, undefined, or a fallback value when accessing it. To achieve that, we have to only render the assistant overlay after spaceId is available and remove existing spaceId from the AssistantContext. The reason I removed spaceId from AssistantContext is because it is at the top of the rendering tree and can block the entire app if waiting for the spaceId becomes available there. `useAssistantSpaceId` comes from `SpaceIdContext` which should be render only when spaceId is available, should be safe to access the exact spaceId inside Assistant overly. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

angorayc added 2 commits March 17, 2025 14:41

reimplement spaceId in AI Assistant overlay

5cc98d1

reimplement spaceId in AI Assistant overlay

dc1a5dd

angorayc commented Mar 17, 2025

View reviewed changes

angorayc requested review from andrew-goldstein and stephmilovic March 17, 2025 14:59

stephmilovic reviewed Mar 17, 2025

View reviewed changes

angorayc added 4 commits March 18, 2025 15:12

isolate assistant last conversation keys

6ba9d6d

organise files

74b73b8

fix types

f9de988

unit tests

fd67322

angorayc added v9.1.0 v8.19.0 labels Mar 19, 2025

angorayc changed the title ~~[SecuritySolution] Do not show error if the last conversation was not found~~ [SecuritySolution] Make last conversation local storage keys space aware Mar 19, 2025

angorayc added v8.18.0 Team:Security Generative AI Security Generative AI bug Fixes for quality problems that affect the customer experience and removed v8.19.0 labels Mar 19, 2025

angorayc and others added 8 commits March 19, 2025 12:58

use last conversation hook

12427e6

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

6c60a59

…-fix'

fix types

374d624

Merge branch 'issues-214114' of github.com:angorayc/kibana into issue…

6cba8b1

…s-214114

fix import

38cc690

fix types

8a84540

cypress

e6287ce

cypress

cf4d361

angorayc marked this pull request as ready for review March 20, 2025 15:08

angorayc requested review from a team as code owners March 20, 2025 15:08

angorayc added the release_note:skip Skip the PR/issue when compiling release notes label Mar 20, 2025

fix unit tests

64c7e0c

revert unnecessary changes

0a80791

stephmilovic approved these changes Mar 26, 2025

View reviewed changes

angorayc added v8.19.0 v9.0.0 and removed backport:8.18 labels Mar 26, 2025

stephmilovic added the backport:version Backport to applied version labels label Mar 26, 2025

angorayc added v8.18.1 backport:prev-minor and removed backport:version Backport to applied version labels v8.18.0 labels Mar 26, 2025

angorayc merged commit 169abec into elastic:main Mar 26, 2025
22 checks passed

angorayc added backport:version Backport to applied version labels and removed backport:prev-minor labels Mar 26, 2025

kibanamachine mentioned this pull request Mar 26, 2025

[8.x] [SecuritySolution] Make last conversation local storage keys space aware (#214794) #216085

Merged

kibanamachine mentioned this pull request Mar 27, 2025

Audit info and error logs in observability plugin #216111

Merged

angorayc mentioned this pull request Mar 27, 2025

[9.0] [SecuritySolution] Make last conversation local storage keys space aware (#214794) #216214

Merged

angorayc mentioned this pull request Apr 15, 2025

[AI4DSOC] Alert Flyout #218018

Merged

kibanamachine mentioned this pull request Sep 25, 2025

[Security Solution] [GenAi] Use default LLM setting for security GenAi features #234480

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SecuritySolution] Make last conversation local storage keys space aware#214794

[SecuritySolution] Make last conversation local storage keys space aware#214794
angorayc merged 23 commits intoelastic:mainfrom
angorayc:issues-214114

angorayc commented Mar 17, 2025 •

edited by kibanamachine

Loading

Uh oh!

angorayc Mar 17, 2025

Uh oh!

stephmilovic Mar 17, 2025

Uh oh!

stephmilovic Mar 17, 2025

Uh oh!

angorayc Mar 18, 2025 •

edited

Loading

Uh oh!

angorayc Mar 24, 2025

Uh oh!

elasticmachine commented Mar 26, 2025

API count

Uh oh!

stephmilovic left a comment

Uh oh!

Uh oh!

kibanamachine commented Mar 26, 2025

Uh oh!

kibanamachine commented Mar 26, 2025

Uh oh!

kibanamachine commented Mar 26, 2025

Uh oh!

kibanamachine commented Mar 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

angorayc commented Mar 17, 2025 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

angorayc Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

stephmilovic Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

stephmilovic Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

angorayc Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

angorayc Mar 24, 2025

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Mar 26, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

Uh oh!

stephmilovic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Mar 26, 2025

Uh oh!

kibanamachine commented Mar 26, 2025

Uh oh!

kibanamachine commented Mar 26, 2025

💔 Some backports could not be created

Manual backport

Questions ?

Uh oh!

kibanamachine commented Mar 26, 2025

💔 Some backports could not be created

Manual backport

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

angorayc commented Mar 17, 2025 •

edited by kibanamachine

Loading

angorayc Mar 18, 2025 •

edited

Loading