feat: Drupal AI Agents compatibility (Fiber streaming + native Bedrock chat)

[steveworley]

Summary

Brings the provider into line with what Drupal AI Agents loops (e.g. Drupal Canvas's page builder) actually need: Fiber-aware streaming so long Claude responses don't trip the dashboard's CloudFront origin timeout, plus native Bedrock-flavoured request/response handling (no OpenAI SDK translation in the middle).

• chat() now detects an active Fiber and silently swaps to the streaming endpoint, suspending between SSE chunks and reconstructing a buffered-style ChatOutput so the caller sees one complete response. Mirrors the pattern in ai_provider_amazeeio / OpenAiBasedProviderClientBase::chat().
• maxTokens is clamped to the destination model's maxOutputTokens from ModelsService before sending. Bedrock rejects oversize values with a 400 (Nova Lite caps at 5000 etc.), and the AI Defaults UI happily lets operators configure higher numbers — the clamp prevents opaque upstream failures.
• Tool definitions are now correctly wrapped in Bedrock's {toolSpec: {name, description, inputSchema}} shape (rather than the OpenAI {type, function} shape ToolsInput::renderToolsArray() produces), since the dashboard validator requires toolSpec.
• The SSE iterator is replumbed onto the base class's doIterate() contract and yields proper StreamedChatMessage objects, so reconstructChatOutput() actually sees the messages — the previous anonymous IteratorAggregate bypassed that and produced empty output, breaking the agent loop.
• A tiny StreamedToolCall value object wraps tool-call entries so the base iterator's assembleToolCalls() can call $tool->toArray() (its contract); plain arrays don't satisfy that.
• HTTP error paths in both clients now log the upstream response body (truncated to 2KB) so 4xx/5xx failures are immediately diagnosable from watchdog instead of just status N.

Removes tests/src/Unit/QuantCloudProviderFailureDetectionTest.php — it tested protected helpers on the old chat() (isEmptyResponseContent, isLikelyTokenLimited, getOutputTokenCount, getStopReason) that no longer exist after this refactor.

Net diff: +909, −1,451.

Test plan

• Fresh install: configure provider + OAuth, run buffered chat via drush php:eval with a simple ChatInput — confirm text response
• Streaming chat: set $provider->streamedOutput(TRUE), iterate the returned ChatOutput — confirm SSE chunks flow
• Fiber path: call chat() inside new \Fiber(...), resume until terminated — confirm reconstructed ChatMessage has correct text + tool calls
• Tool-use round-trip: send a ChatInput with tools defined, receive toolUse back, reply with role: tool history, confirm Claude consumes it
• Drupal Canvas page build that exercises the AI Agents loop end-to-end
• Embeddings + image generation still work via QuantCloudClient (unchanged code path)
• Operator-visible: trigger an upstream 4xx and confirm response body appears in watchdog

Notes for reviewers

• QuantCloudChatMessageIterator deliberately skips response.content on the summary frame — it carries the full assembled assistant text already emitted via earlier delta frames; re-emitting would double the message.
• applyMaxTokensClamp() is a no-op when the model's cap is unknown (returns the request value untouched) so newly-added Bedrock models won't be silently capped at zero.
• The dashboard's tool-call response shape puts toolUse as a sibling of content (not nested inside the content array). The parser handles both legitimately, but the sibling layout is what production currently emits.

🤖 Generated with Claude Code

[quantcode-agent]

PR Review: feat/ai-agents-fiber-streaming → fix/release-blockers

PR #11 — "feat: Drupal AI Agents compatibility (Fiber streaming + native Bedrock chat)"
4 commits · 7 files changed · ~909 insertions, ~1,000 deletions

---

Summary

This PR rewrites the chat pipeline to support Drupal AI Agents' Fiber-based streaming model, replaces the old OpenAI-shaped SSE iterator with a native Bedrock/dashboard-frame iterator, adds upstream error body logging, and introduces a getMaxOutputTokens() clamp. The architecture is sound and the intent is clear. There are no exploitable security vulnerabilities. There are several correctness concerns worth fixing before merge, and one deletion that needs justification.

---

🔴 Blockers

1. reconstructChatOutput() is referenced in the class docblock but does not exist

src/QuantCloudChatMessageIterator.php:20 states:

// `reconstructChatOutput()` can recover the final `ChatMessage` (text + tool

The method is referenced in the class-level docblock as the mechanism callers use to get a ChatOutput after consuming the stream. However, reconstructChatOutput() does not appear anywhere in the file. The Fiber path in QuantCloudProvider::chat() calls the iterator and then calls $iterator->reconstructChatOutput() to build the final ChatOutput to return.

If this method is inherited from StreamedChatMessageIterator in the base ai module, this is fine — but the docblock implies it is implemented here. Verify the base class provides this method and that it correctly handles the Bedrock-shaped tool calls emitted by this iterator. If the base class reconstructChatOutput() expects OpenAI-shaped tool objects and StreamedToolCall::toArray() returns a different shape, the Fiber path will silently produce empty or malformed tool results. This is a CWE-241 (Improper Handling of Unexpected Data Type) class of bug.

Remediation: Add an integration test or at minimum confirm (with a code citation) that the base class reconstructChatOutput() consumes StreamedToolCall::toArray() correctly. If it doesn't, override it here.

---

2. Deleted test file removes the only coverage for failure detection — no replacement

tests/src/Unit/QuantCloudProviderFailureDetectionTest.php is deleted entirely (285 lines, covering max_tokens, length, end_turn, unknown, and NULL stop-reason scenarios). No replacement test file is added anywhere in the PR.

The PR simultaneously rewrites parseChatResponse() and the entire streaming path. Deleting the only unit tests for failure/stop-reason detection while rewriting the code under test leaves the module with zero automated coverage for these paths.

Remediation: Either restore the test file (adapted for the new response shape) or add equivalent coverage for parseChatResponse(), doIterate() stop-reason handling, and the applyMaxTokensClamp() clamp logic before merge. The getMaxOutputTokens() helper in ModelsService also has no tests.

---

⚠️ Warnings

3. applyUsage() called twice on the same message in the summary frame

src/QuantCloudChatMessageIterator.php — In the summary frame's tool-use loop, applyUsage() is called once per tool yielded on freshly created messages — that's fine. But in the delta path the pattern createStreamedChatMessage → applyUsage → yield is duplicated in two branches with the same $message variable in scope. If the base class applyUsage() accumulates rather than sets token counts, this will double-count tokens on messages that hit both branches. Verify the base class implementation is idempotent or restructure to call applyUsage() exactly once per message.

4. QuantCloudClient logs the upstream error body unconditionally — potential PII/credential leak in logs

src/Client/QuantCloudClient.php (error handler, new lines):

$body = '';
if ($e instanceof RequestException && $e->getResponse()) {
    $status = $e->getResponse()->getStatusCode();
    $reason = $e->getResponse()->getReasonPhrase();
    $body   = $e->getResponse()->getBody()->getContents();
}
$this->logger->error('...@body', ['@body' => $body]);

The upstream error body is now logged regardless of the advanced.enable_logging config flag. The success path correctly gates logging behind $config->get('advanced.enable_logging'). Error bodies from Bedrock/dashboard can contain request context (prompts, tool inputs) that may include PII or PROTECTED-classification content. Logging them unconditionally to Drupal's watchdog violates APP-11 (security of personal information) and ISM-0988 (log content controls).

Remediation: Either gate the body logging behind the same enable_logging flag, or truncate/redact the body before logging (e.g. first 200 chars, strip any content fields).

5. convertMessage() silently drops images when a tool result is present

src/Plugin/AiProvider/QuantCloudProvider.php ~line 415:

if ($message->getToolsId()) {
    return [
        'role'      => 'user',
        'content'   => $text,
        'toolUseId' => $message->getToolsId(),
    ];
}

The early return means a message that is both a tool result and carries images (valid in multi-modal agentic flows) will silently drop the images. This is unlikely in practice today but will produce confusing silent failures if Drupal AI Agents ever sends vision content alongside a tool result. At minimum, add a // Note: images are intentionally ignored for tool-result messages comment.

6. getMaxOutputTokens() hardcodes a static model→token map with no staleness mechanism

src/Service/ModelsService.php ~line 145 — the map will silently clamp requests for models added after this commit (returning NULL = no clamp, which is safe) and will silently over-clamp if Anthropic raises limits on existing model versions. Add a @todo comment noting the map needs updating when new models are onboarded, and consider whether the dashboard API exposes model caps that could be fetched dynamically.

7. QuantCloudStreamingClient missing declare(strict_types=1)

src/Client/QuantCloudStreamingClient.php:1 — the file header lacks declare(strict_types=1) while all other files touched in this PR have it added. Minor consistency issue.

---

Nits (non-blocking)

• src/QuantCloudChatMessageIterator.php:71 — $maxWarnings = 3 duplicates MAX_SSE_DECODE_WARNINGS (the class constant defined in QuantCloudStreamingClient). Use the constant or move it to this class.
• src/StreamedToolCall.php — the class docblock references StreamedChatMessageIterator::assembleToolCalls() as the consumer; if that method is in the upstream ai module, pin the minimum module version in ai_provider_quant_cloud.info.yml.
• src/Plugin/AiProvider/QuantCloudProvider.php ~line 158 — the comment // We do not advertise ChatFiberSupport could briefly explain why (wire format mismatch).
• src/QuantCloudChatMessageIterator.php — readLine() confirm it handles \r\n line endings (SSE spec allows both \n and \r\n).

---

Verdict

#	Severity	Must fix before prod?
1	Blocker	Yes — verify reconstructChatOutput() contract with StreamedToolCall
2	Blocker	Yes — restore/replace deleted test coverage
3	Warning	Yes — double applyUsage() call risks token count corruption
4	Warning	Yes — unconditional error body logging risks PII in watchdog
5	Warning	No — add comment, low practical risk today
6	Warning	No — add @todo, safe fallback exists
7	Warning	No — consistency fix

Items 1 and 2 are classified Blockers because item 1 has a concrete silent-failure path (tool calls silently malformed if the base class contract isn't met) and item 2 removes the only automated safety net for a rewritten critical path. Both are straightforward to resolve. The rest of the PR — the Fiber-aware streaming design, the SSE frame parser, the applyMaxTokensClamp guard, and the error body logging improvement — are well-structured and a clear improvement over the previous implementation.

---

Automated review by Quant Code