fix: #3586589 Do not assume action definitions have a type when deriving action plugin functions

Ahmad Khalilrequested to merge
issue/ai-3586539:3586539-add-a-chatmemoryinterface-for-storing-chatbot-chat-history into 1.x

What this MR does

Implements #3586539: adds a ChatMemory plugin type to the core AI module so chatbots can optionally persist conversation history server-side, and wires it into the DeepChat block as the reference integration.

Why

Chatbots currently have no standardized way to keep conversation history across requests. The ChatProcessor-based DeepChat block (#3585077 (closed)) relies entirely on the client resending the full history on every request: a page reload loses the conversation, the thread_id sent by the JS is ignored server-side, and DeepChatFormBlock::historicalMessages() was a stub returning []. The old assistant-based path has its own ad-hoc tempstore history that nothing else can reuse. This MR standardizes that capability while keeping the issue's core principle: each chatbot remains responsible for its own memory handling — client-side-only history stays fully supported and remains the default.

What was added

1. ChatMemory plugin type (core ai module)

Mirrors the ChatProcessor conventions exactly (attribute-based discovery, base class, plugin manager):

  • src/Attribute/ChatMemory.php, src/Plugin/ChatMemory/ChatMemoryInterface.php, src/Base/ChatMemoryPluginBase.php, src/PluginManager/ChatMemoryPluginManager.php, registered as plugin.manager.ai.chat_memory.
  • Interface: createThreadId(), hasThread(), loadMessages(), saveMessages(), appendMessages(), clearMessages(), deleteThread() — the load/save/append/clear + thread handling asked for in the issue. The base class derives append/clear from load/save, so a minimal plugin implements five methods.
  • Distinct from AiShortTermMemory by design: that type transforms an in-flight history before an AI call (compaction/summarization) and owns no storage; ChatMemory is the persistence layer. Documented.

2. tempstore default implementation

A shipped, working backend so the feature is usable out of the box:

  • Stores threads in expirable key-value storage, scoped per user (uid) or per anonymous session (random owner key in the session — the same key the private temp store uses), with configurable expiry (default 24h) and optional max-messages pruning.
  • Uses ChatMessage::toArray()/fromArray() for serialization — no new serialization format.
  • Why not PrivateTempStore directly: every PrivateTempStore operation resolves its owner through the session. Streamed responses append the assistant message after the response has started, when the session middleware has already closed the session — a write at that point tries to restart it and dies with "headers already sent", silently losing the thread for anonymous users. The plugin therefore resolves the owner once, early in the request, and caches it, so the write path never touches the session. This constraint is documented as a best practice for contrib plugin authors.

3. DeepChat block/API integration (ai_chatbot)

  • Block gets a Chat Memory select (default: None — client-side history, preserving existing behavior exactly) with an AJAX per-plugin configuration subform; config schema added.
  • DeepChatApi keeps the current thread id in the server-side session — the client never chooses which thread is written to. With memory enabled, stored history is authoritative and only the latest user message is taken from the client (a client cannot inject a fabricated history); the exchange is appended after execution — for streamed responses, after the stream completes.
  • historicalMessages() now preloads the thread on page render, so conversations survive reloads. Note the lazy-session subtlety handled there: anonymous users with a valid session cookie don't have a started session at render time, so the block checks for the cookie and lets the read start the session lazily (without ever creating sessions for fresh visitors).
  • New flood-protected POST /api/deepchat/reset-thread route backs the Clear History button (deletes the thread, rotates the id); deepchat-init.js prefers it when memory is enabled and falls back to the old assistant reset otherwise. Flood registers failed attempts only (mirrors ResetSession), so legitimate users can't lock themselves out.
  • The markdown→safe-HTML logic was extracted from the controller into a shared ai_chatbot.message_renderer service so preloaded history renders identically to live responses (controller delegates to it; no behavior change).

4. Drive-by fix

The block stores show_tool_usage but the schema only defined show_tools_usage (plural) — the block couldn't be placed in any strict-schema test. Added the missing entry as a separate commit; the legacy plural key is kept.

5. Tests (22 tests / 214 assertions)

  • Unit ChatMemoryPluginBaseTest: base-class contract every contrib plugin inherits (append ordering, type validation, clear-keeps-thread, config boilerplate).
  • Kernel ChatMemoryPluginManagerTest + TempStoreChatMemoryTest: discovery, full lifecycle, expiry, pruning, per-user isolation, and a regression test for the streamed-append/session constraint (append with the session gone must succeed under the cached owner).
  • Kernel DeepChatMessageRendererTest: XSS sanitization, markdown conversion, relative-link rewriting.
  • Functional DeepChatApiChatMemoryTest: storage via the HTTP route, fabricated-history injection is ignored, invalid plugin → 400, opt-out stores nothing, reset flow, flood (3 invalid → 429; valid never throttled).
  • Functional DeepChatBlockMemoryPreloadTest: full anonymous round trip — chat via API, next page view preloads history into drupalSettings; memory off → nothing preloads, reset URL gone.
  • A state_memory test plugin in ai_test.

6. Documentation

docs/developers/writing_a_chat_memory_plugin.md (+ mkdocs nav): plugin anatomy, server-side vs client-side choice, thread-ownership security requirements, the AiShortTermMemory distinction, and the no-session-in-write-paths best practice.

How to test

Automated

vendor/bin/phpunit -c phpunit.ai.xml \ web/modules/contrib/ai/tests/src/Unit/Base/ChatMemoryPluginBaseTest.php \ web/modules/contrib/ai/tests/src/Kernel/PluginManager/ChatMemoryPluginManagerTest.php \ web/modules/contrib/ai/tests/src/Kernel/Plugin/ChatMemory/TempStoreChatMemoryTest.php \ web/modules/contrib/ai/modules/ai_chatbot/tests/src/Kernel/DeepChatMessageRendererTest.php \ web/modules/contrib/ai/modules/ai_chatbot/tests/src/Functional/Controller/DeepChatApiChatMemoryTest.php \ web/modules/contrib/ai/modules/ai_chatbot/tests/src/Functional/DeepChatBlockMemoryPreloadTest.php

Manual

Setup: enable ai_chatbot and a ChatProcessor source (ai_test provides test_processor, no API key needed; requires $settings['extension_discovery_scan_tests'] = TRUE;). Grant Access DeepChat API to the roles you'll test (include Anonymous). Place the AI Chatbot (DeepChat) block; under Styling set Placement to Bottom right when placing in a page region.

  1. Config UI: in the block form, verify the Chat Memory select defaults to None (client-side history). Selecting Temporary storage reveals its config fieldset (expiry, max messages) via AJAX. Save, re-edit, values persist.
  2. History survives reload: with memory on, chat 2–3 messages, reload → conversation reappears. Test logged in and anonymous (private window). If streaming is available, test with streaming on as well — the assistant reply must be stored once the stream finishes.
  3. Clear History: open the hamburger menu in the chatbot header → Clear History. The network tab shows POST /api/deepchat/reset-thread returning a new thread_id; reload → still empty. Repeated valid resets always succeed; invalid payloads get 3 tries then HTTP 429.
  4. Isolation: open the site in a second browser/profile → empty chat, never another session's messages. Same after deleting the SSESS… cookie. (Note: closing an incognito window only discards cookies once all incognito windows are closed; while logged in, history surviving a browser restart is expected — Drupal's session cookie is persistent.)
  5. Config options: set Max messages = 2 → after 3 exchanges only the last exchange preloads. Set Expiry = 60 → history gone after a minute. Set memory back to None → behavior is identical to before this MR (history lost on reload, old reset path used, no errors).
  6. Watch watchdog throughout — no errors should appear (in particular no "Failed to start the session because headers have already been sent").

Merge request reports