[Agent Builder] Support posting messages without agent execution - #290223
[Agent Builder] Support posting messages without agent execution#290223brunofarache wants to merge 39 commits into
Conversation
|
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
784c622 to
f5f9b45
Compare
f5f9b45 to
49d325b
Compare
Return the value produced by contextMessagePayloadSchema.validate() instead of casting the raw request body, and move the ContextMessagePayload type next to the other chat wire types in common/http_api/chat.ts.
callbackConversePayloadSchema already rejects trigger_mode at runtime, so omitting it from ChatCallbackRequestBodyPayload only made the type diverge from the other converse payloads. With the omission gone the type extends ChatRequestBodyPayload, which also collapses executeAgent's payload union to a single type.
appendContextMessage minted neither: every caller had to pass a messageId and a createdAt, plus empty-value fallbacks for message and attachments. The route was the only caller and always passed a fresh uuid and the current time, so the replay-identity check the messageId existed for could never match across requests; it is removed along with the parameters.
appendContextMessage hand-rolled a read-modify-write loop because its attachment materialization was async and writeConversation's fields callback is not. Callers elsewhere (sml, attachments routes, runner) already own the attachment state manager and hand the client refs plus snapshot/produced, which reconcileAttachments merges against stored state, so the route now does the same. That makes the field callback synchronous, so the method delegates to writeConversation and drops its duplicate retry loop and the getTypeDefinition parameter. Also drops the unused create and origin inputs, and the read_only guard: read_only is documented as a UI-only presentation flag that carries no authorization meaning, and no other write path enforces it.
The conversation client no longer carries a bespoke appendContextMessage: the operation is now ConversationService.appendContextMessage, which reads the conversation, materializes the caller's attachment inputs and appends a single user message event through the existing appendEvents write path. appendEvents itself is unchanged. The service is the natural home because the actor depends on the requesting user, which is private to it, and because it can own the attachment state manager now that it takes the attachment service as a dependency. The route is left with payload validation, attachment validation and its HTTP error mapping.
prepare_conversation already promoted a user message's attachment inputs into conversation-level attachments, deduplicating by content hash and passing a resolve context so by-reference attachments resolve. Context messages had a simpler loop that did neither. The logic moves to the attachment service, which already builds the resolve context for validate, and both paths now call it. The execution path calls the module directly rather than the service method, since it only holds the public AttachmentsService contract; the dedupe map is no longer threaded through, as seeding it from the state manager is equivalent. Context messages now reuse a stored attachment when the same content is posted again, matching converse.
Narrowing it to Pick<Conversation, 'user'> was not needed: the conversation service holds the conversation, so it can spread it and override the user the way the conversation client did before the move.
The route treats a whitespace-only input as no message when it checks that a request carries input or attachments, so store the trimmed value rather than the raw one.
It only wrapped a comparison the execution paths already made inline, so restore the original conversation.operation === 'CREATE' checks.
chatPayloadSchema now reads as its two modes instead of inlining the extended converse payload in the union.
The entry carried an events array that was always [userMessage], so drop it and let its one consumer derive the events it iterates. Naming the union member also makes TimelineEntry read as its two kinds. The generic stays: the entry is read both on the raw timeline and on the processed one, where the message payload is a ProcessedRoundInput.
Four call sites discriminated the entry union with an inline 'terminated' in entry check. isTimelineRound now holds that knowledge, and isTimelineStandaloneUserMessage reads the other way where a positive condition is clearer. The standalone guard only narrows in positive position: a round satisfies the standalone member structurally, so excluding it leaves never. Sites that need round fields in the negative branch keep isTimelineRound.
The processed timeline loop matched the entry's own message by object identity, which held only because grouping spreads the same references into the round's event list. Comparing ids is equivalent, matches how the rest of the file identifies events, and does not silently drop the message if a future step clones events. formatRoundInput also renders standalone user messages now, so it becomes formatUserInput, and prepareMessages iterates entries rather than rounds.
Hoist every type in context_timeline.ts above the functions, align the missing position fallback between the two comparators, and say why each one sorts the way it does: one repairs a lossy fold, the other a concatenation by entry kind, and neither can order on timestamps alone.
The relevant-skills selector built its recent context from executed rounds only, so messages posted without triggering the agent never informed it. It now receives every timeline entry; an entry with no response renders just its user line, and the character budget counts the message slots that actually exist rather than assuming two per round.
The test posted trigger_mode: never with no conversation_id and expected the conversation to be created, which stopped being true once user messages required a conversation to append to. It now asserts that request is rejected, creates the conversation through the conversations API, and appends to it.
Main reworked attachment validation while this branch was open: validate is now per attachment and returns a result object, carries a validateContext, and skips revalidation when content is unchanged. That contract wins; this branch's plural, throwing variant is gone. The array handling the routes need — validate each input, reject the request on the first invalid one — moves to validate_attachment_inputs, shared by the execution service (which held it privately) and the chat route. Main's validateContext and content-hash check are ported into merge_attachment_inputs, the extracted helper both the execution path and the conversation service use.
brunofarache
added
backport:skip
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The test walks a five-phase flow and was one ~90-line block, so a failure surfaced as a bare assertion with no indication of which phase broke. Each phase is now a labelled step, and the shared request headers are hoisted.
CodeQL alerts dismissed as pre-existingCodeQL flagged six unbounded schemas in All six are pre-existing. Those schemas are identically unbounded on Bounding them changes what the public converse endpoints accept ( Dismissed as "won't fix" with that reasoning recorded on each alert. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the trigger_mode: 'never' (append user message without execution) change. The schema split (chatPayloadSchema oneOf), scoped/atomic append in appendUserMessage, the extracted mergeAttachmentInputs/validateAttachmentInputs, and the timeline-entry refactor (groupTimelineEntries, standaloneUserMessages, buildRecentContext framing math) look faithful and are well covered by unit + scout tests. One concrete finding on error classification is left inline.
Generated by Claude Reviewer for #290223 · claude · opus · 199.7 AIC · ⌖ 46 AIC · ⊞ 7K
A conversation created before the events-native migration cannot take a user message, and the caller cannot fix that by retrying. createInternalError carries no status code, so the route answered 500; it is now a bad request, and the test asserts the class rather than only the message.
There was a problem hiding this comment.
Reviewed the trigger_mode: 'never' append-user-message feature. The schema split (chatPayloadSchema oneOf), route handling, authorization via the scoped conversation client, atomic event/attachment persistence, and the timeline/context refactors are well-structured and thoroughly tested. One concrete concern noted inline about de-duplicated attachment content losing its per-message reference in the new append path.
Generated by Claude Reviewer for #290223 · claude · opus · 228.1 AIC · ⌖ 22.7 AIC · ⊞ 5.5K
| type: TimelineEventType.userMessage, | ||
| created_at: new Date().toISOString(), | ||
| actor: userMessageActor({ ...conversation, user }, { author }), | ||
| data: { message: message.trim(), attachment_refs: stateManager.getAccessedRefs() }, |
There was a problem hiding this comment.
The new user message's attachment_refs are taken solely from stateManager.getAccessedRefs(), but mergeAttachmentInputs (in merge_attachment_inputs.ts) skips an input via continue whenever its content already matches a stored attachment (storedIdByContentKey.has(getContentKey(...))) — that skip path does not add/update/get, so nothing is access-tracked.
Result: if an integration appends a message carrying an attachment whose content is already stored on the conversation (a realistic case for repeated incident/context posts), the attachment is correctly de-duplicated at the conversation level, but the new user_message event is persisted with that attachment omitted from attachment_refs — so the message silently loses its association to the attachment the caller sent.
In the pre-existing round flow this was masked because refs were merged with the input's own attachment_refs (mergeAttachmentRefs(input.attachment_refs, getAccessedRefs())); appendUserMessage has no incoming refs, so getAccessedRefs() is the only source and the dropped ref becomes observable. Consider having the dedup-skip path resolve the existing attachment id into an accessed ref (or otherwise ensure de-duplicated inputs still produce a ref) before building the event. Note the reuses a stored attachment unit test only asserts attachments length, not that the second message references it, so this gap isn't currently covered.
⏳ Build in-progress, with failures
Failed CI Steps
Test Failures
History
|
3 participants
Closes #288967
Adds
trigger_mode: 'never'toPOST /api/chat/converseso integrations can append user messages to an existing conversation without running an agent. For example, an integration can persist incident updates asuser_messageevents, then a later normal request can ask the agent to summarize them.User-message requests require
conversation_idand only supportinputand/orattachmentsin addition totrigger_mode. They do not acceptagent_id,access_control,read_only, model-routing fields, execution fields, prompts, action, browser tools, configuration overrides, or project routing. This keeps the mode append-only for existing event-native conversations and avoids creating conversations throughtrigger_mode: 'never'.User-message writes persist events and attachment versions atomically, enforce scoped conversation access through the conversation client, and return the updated
ConversationWithPermissions.This PR intentionally targets only
POST /api/chat/converse. The streamingPOST /api/chat/converse/asyncendpoint does not accepttrigger_modeyet becausetrigger_mode: 'never'has no agent response to stream. We can addtrigger_modeto the streaming endpoint later whenautois supported. The internal converse callback endpoint also stays execution-only and does not exposetrigger_mode; callers that only want to append history should usePOST /api/chat/converse.Extends event-based context preparation to retain these user messages, attribution, attachment context, and ordering, so a later normal request sees them as ordinary human turns in the prompt. Omitted or explicit
alwaysmode retains existing execution behavior; legacy public converse schemas remain unchanged.Limitations
Compaction still indexes rounds, not timeline entries. A conversation whose history contains standalone user messages therefore excludes them from the compaction token budget, and once a compaction runs they are dropped from the compacted context. Conversations that have not compacted are unaffected. Generalizing compaction to timeline entries — entry-indexed summaries with an event boundary — is deferred to a follow-up.
No UI work is included: the chat UI renders rounds, and nothing in the app reads the
eventstimeline, so appended messages are stored and reach the agent but are not displayed. A conversation that has only appended messages therefore looks empty in the UI until a triggering request creates a round. Showing them is part of the UI work for this feature, tracked separately.These user messages are history/context only:
trigger_mode: 'never'rejectsprompts, does not clear pending prompts, and cannot resume a paused HITL round.Builds on the merged #288986. UI changes, automatic triggering, and legacy-conversation migration are outside this change.
Manual testing
/api/chatis gated by theagentBuilder:experimentalFeaturesadvanced setting, so enable it first. The examples assume a local Kibana athttp://localhost:5601.The calls authenticate with an API key, created from Stack Management → Security → API keys:
Requests authenticated this way are attributed to the key owner's user profile, so the conversation is also visible to that user in the Kibana UI.
Create an empty conversation:
Append two user messages without triggering the agent. They carry a fact the agent could only know from them. Each call returns the updated conversation;
roundsstays empty andeventsgrows by oneuser_message:Opening the conversation in the UI at this point shows nothing — see the limitation above. The responses of the two calls are where the appended messages are visible.
Now ask the agent. This request omits
trigger_mode, so it executes normally and the two messages above are part of the history the model sees. It also passes_execution_mode: local: by default a converse request runs on a Task Manager node, whose reconstructed request does not carry the API key's profile identity, so it cannot read a private conversation and the run fails withConversation … not found.The answer should be
blue-narwhal-7, previouslyorange-swordfish-42. Neither string appears in the triggering message, so answering at all proves the appended messages reached the model, and naming the current one proves they arrived in order.Run locally, the agent answered:
Rejected requests, for the negative cases. A user message needs a conversation to append to:
Neither input nor attachments:
Execution-only options are not accepted in this mode: