Skip to content
109 changes: 109 additions & 0 deletions packages/ai-bot/tests/prompt-construction-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2763,6 +2763,115 @@ Attached Files (files with newer versions don't show their content):
assert.equal(messageText(result[5]).trim(), expected.trim());
});

test('a later applied result supersedes an earlier failed result for the same request', async () => {
// A tool can fail and then succeed on a user Retry; both result events
// stay in the room forever. The prompt must reflect the latest one, or
// the model is permanently told the call failed and may re-issue it.
const history: DiscreteMatrixEvent[] = [
{
type: 'm.room.message',
room_id: 'room-id-1',
sender: '@user:localhost',
content: {
body: 'set the title',
msgtype: APP_BOXEL_MESSAGE_MSGTYPE,
format: 'org.matrix.custom.html',
data: { context: { tools: [], functions: [] } },
},
origin_server_ts: 1722242847000,
unsigned: { age: 1000, transaction_id: 't0' },
event_id: 'user-event-id-1',
status: EventStatus.SENT,
},
{
type: 'm.room.message',
room_id: 'room-id-1',
sender: '@aibot:localhost',
content: {
body: 'Setting the title',
msgtype: APP_BOXEL_MESSAGE_MSGTYPE,
format: 'org.matrix.custom.html',
data: { context: { functions: [] } },
[APP_BOXEL_TOOL_REQUESTS_KEY]: [
{
id: 'retried-tool-call-id-1',
name: 'patchCardInstance',
arguments: JSON.stringify({
attributes: { description: 'Set the title' },
}),
},
],
},
origin_server_ts: 1722242849000,
unsigned: { age: 900, transaction_id: 't1' },
event_id: 'retried-command-event-id-1',
status: EventStatus.SENT,
},
{
type: APP_BOXEL_TOOL_RESULT_EVENT_TYPE,
room_id: 'room-id-1',
sender: '@user:localhost',
content: {
'm.relates_to': {
event_id: 'retried-command-event-id-1',
rel_type: APP_BOXEL_TOOL_RESULT_REL_TYPE,
key: 'failed',
},
msgtype: APP_BOXEL_TOOL_RESULT_WITH_NO_OUTPUT_MSGTYPE,
commandRequestId: 'retried-tool-call-id-1',
failureReason: 'store exploded',
data: { context: { tools: [], functions: [] } },
},
origin_server_ts: 1722242853000,
unsigned: { age: 800, transaction_id: 't2' },
event_id: 'failed-result-id-1',
status: EventStatus.SENT,
},
{
type: APP_BOXEL_TOOL_RESULT_EVENT_TYPE,
room_id: 'room-id-1',
sender: '@user:localhost',
content: {
'm.relates_to': {
event_id: 'retried-command-event-id-1',
rel_type: APP_BOXEL_TOOL_RESULT_REL_TYPE,
key: 'applied',
},
msgtype: APP_BOXEL_TOOL_RESULT_WITH_NO_OUTPUT_MSGTYPE,
commandRequestId: 'retried-tool-call-id-1',
data: { context: { tools: [], functions: [] } },
},
origin_server_ts: 1722242857000,
unsigned: { age: 700, transaction_id: 't3' },
event_id: 'applied-result-id-1',
status: EventStatus.SENT,
},
];
const result = await buildPromptForModel(
history,
'@aibot:localhost',
[],
[],
[],
fakeMatrixClient,
);
let toolMessages = result.filter((m) => m.role === 'tool');
assert.equal(toolMessages.length, 1, 'one tool message per request');
assert.equal(
(toolMessages[0] as { tool_call_id?: string }).tool_call_id,
'retried-tool-call-id-1',
);
let content = messageText(toolMessages[0]);
assert.true(
content.includes('executed'),
`the retried call reads as executed, got: ${content}`,
);
assert.false(
content.includes('failed'),
`no stale failure text survives the retry, got: ${content}`,
);
});

test('pairs a pre-rename request/result (legacy wire keys) with the same tool_call_id', async () => {
// A room whose history predates the command → tool rename replays events
// with the legacy spellings forever; prompt assembly must pair them
Expand Down
26 changes: 25 additions & 1 deletion packages/host/app/components/matrix/room-message-tool.gts
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,13 @@ export default class RoomMessageTool extends Component<Signature> {
return this.matrixService.failedToolState.get(toolRequest.id);
}

// Execution failure reported through a room event (as opposed to
// failedToolState, which is this tab's in-memory state for a failure it
// produced itself).
private get failedToolCallState() {
return this.args.messageTool.status === 'failed' && !this.failedToolState;
}

private get invalidToolCallState() {
return (
this.args.messageTool.status === 'invalid' &&
Expand All @@ -237,7 +244,11 @@ export default class RoomMessageTool extends Component<Signature> {
}

private get hasFailedState() {
return !!(this.failedToolState || this.didFailCorrectnessCheck);
return !!(
this.failedToolState ||
this.failedToolCallState ||
this.didFailCorrectnessCheck
);
}

<template>
Expand Down Expand Up @@ -301,6 +312,19 @@ export default class RoomMessageTool extends Component<Signature> {
<Alert.Messages @messages={{array this.failedToolState.message}} />
<Alert.Action @action={{@runCommand}} @actionName='Retry' />
</Alert>
{{else if this.failedToolCallState}}
<Alert @type='error' as |Alert|>
<Alert.Messages
@messages={{array
(if
@messageTool.failureReason
@messageTool.failureReason
'Tool call failed.'
)
}}
/>
<Alert.Action @action={{@runCommand}} @actionName='Retry' />
</Alert>
{{else if this.invalidToolCallState}}
<Alert @type='warning' as |Alert|>
<Alert.Messages @messages={{array @messageTool.failureReason}} />
Expand Down
38 changes: 24 additions & 14 deletions packages/host/app/lib/matrix-classes/message-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,23 +371,33 @@ export default class MessageBuilder {
message: Message,
toolRequest: Partial<ToolRequest>,
) {
let toolResultEvent =
this.builderContext.toolResultEvent ??
(this.builderContext.events.find((e: any) => {
let r = e.content['m.relates_to'];
// Correlate the result to its command by commandRequestId (the
// globally unique LLM tool-call id), not by the result's
// m.relates_to.event_id. A reload strips the m.replace edits and loads
// only the original event, so the result's link id — pointing at the
// final edit — matches no loaded event. commandRequestId is stable
// across edits and present on every one, so it resolves the command on
// both the live and reload paths.
return (
let toolResultEvent = this.builderContext.toolResultEvent;
if (!toolResultEvent) {
// Correlate the result to its command by commandRequestId (the
// globally unique LLM tool-call id), not by the result's
// m.relates_to.event_id. A reload strips the m.replace edits and loads
// only the original event, so the result's link id — pointing at the
// final edit — matches no loaded event. commandRequestId is stable
// across edits and present on every one, so it resolves the command on
// both the live and reload paths.
//
// Scan newest-first: a request can have several result events (a
// 'failed' first attempt, an 'applied' retry) and the latest one is
// the call's actual outcome.
let events = this.builderContext.events;
for (let i = events.length - 1; i >= 0; i--) {
let e = events[i] as any;
let r = e.content?.['m.relates_to'];
if (
isToolResultEventType(e.type) &&
isToolResultRelType(r?.rel_type) &&
e.content.commandRequestId === toolRequest.id
);
}) as ToolResultEvent | undefined);
) {
toolResultEvent = e as ToolResultEvent;
break;
}
}
Comment on lines +384 to +399

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Claude Code 🤖] Confirmation — the ordering this scan depends on holds under both states the array can be in, which is not obvious from here.

Room.addEvent (packages/host/app/lib/matrix-classes/room.ts) appends to _events in arrival order and replaces edits in place, so within a room a failed result precedes its applied retry positionally. Separately, RoomResource.sortedEvents calls .sort((a, b) => a.origin_server_ts - b.origin_server_ts) on that same array — Array.prototype.sort mutates in place, so once anything has read sortedEvents the array is also timestamp-ascending. Both orders put the newest matching result last, so walking backwards picks the right one either way. Also good that the loop now uses e.content?.[...] — the old .find dereferenced e.content unguarded on every event in the room.

Two notes for whoever edits this next:

This is the rebuild path, not the live one. When a result event arrives live, updateMessageCommandResult takes the builderContext.toolResultEvent fast path above and assigns toolCallStatus unconditionally, so the last event to arrive wins there. That agrees with this scan only because both now mean "latest wins" — if either side ever flips back to first-wins, the live view and the post-reload view of a retried tool diverge with nothing failing.

Which makes the pair worth pinning. The new packages/ai-bot/tests/prompt-construction-test.ts case covers the ai-bot half of latest-wins; the host half has no equivalent. A rendering or unit test that builds a message from a history containing failed then applied for one request and asserts messageTool.status === 'applied' (and no stale failureReason) would cover the reload path this scan owns.

Scope. Confirmation + suggested test. Non-blocking.


Generated by Claude Code

}

// ai-bot ran this one itself (e.g. readRealmFile), so the host never
// resolves a command class or runs it. Skip the skill lookup below — it's
Expand Down
2 changes: 1 addition & 1 deletion packages/host/app/lib/matrix-classes/message-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { Message } from './message';
import type { CardDef } from '@cardstack/base/card-api';
import type { SerializedFile } from '@cardstack/base/file-api';

type ToolCallStatus = 'applied' | 'ready' | 'applying' | 'invalid';
type ToolCallStatus = 'applied' | 'ready' | 'applying' | 'invalid' | 'failed';

export default class MessageTool {
@tracked toolRequest: Partial<ToolRequest>;
Expand Down
Loading
Loading