Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## Git

- DO NOT include "Co-Authored-By" in commit messages.
- DO NOT include "Generated with Claude Code" or any similar attribution in PR descriptions.

## Design Comments

Expand Down
41 changes: 36 additions & 5 deletions examples/integrations/outlook/connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,28 @@ function listMessages(
headers: Record<string, string>,
params: Record<string, unknown>,
) {
const top = Math.min((params.top as number) || 25, 100);
const requestedTop = Math.min((params.top as number) || 25, 100);
const orderby = (params.orderby as string) || 'receivedDateTime desc';

// Build filter: prefer explicit cursor param, fall back to raw filter string
const cursor = params.cursor as { receivedDateTime?: string } | undefined;
// Build filter: prefer explicit cursor param, fall back to raw filter string.
// Use `ge` (>=) so that messages at the cursor's exact second are included;
// the cursor message itself is excluded by position-based skip below.
const cursor = params.cursor as
| { receivedDateTime?: string; messageId?: string }
| undefined;
let filter = (params.filter as string) || '';
if (cursor?.receivedDateTime && !filter) {
filter = 'receivedDateTime gt ' + cursor.receivedDateTime;
filter = 'receivedDateTime ge ' + cursor.receivedDateTime;
}

// Over-fetch when cursor is present: Graph API may truncate receivedDateTime
// to seconds while filtering with sub-second precision internally, causing
// messages at the cursor's truncated second to re-appear. Extra buffer lets
// us filter them out and still return genuinely new messages.
const top = cursor?.receivedDateTime
? Math.min(requestedTop + 10, 100)
: requestedTop;

// Graph API does not support $orderby combined with $filter on conversationId.
// When this combination is detected, drop $orderby from the request and sort
// the results in memory afterwards.
Expand Down Expand Up @@ -369,7 +381,26 @@ function listMessages(

const data = response.json() as Record<string, unknown>;
const accountEmail = getAccountEmail(http, headers);
const rawValues = (data.value || []) as GraphMessage[];
let rawValues = (data.value || []) as GraphMessage[];

// Position-based skip: find the cursor message by ID in the asc-ordered
// results and take only messages after it. This avoids timestamp precision
// issues and correctly handles multiple messages within the same second.
// Then trim to the originally requested $top so the caller gets exactly
// the number of messages it asked for.
if (cursor?.messageId) {
const cursorIndex = rawValues.findIndex(function (msg) {
return msg.id === cursor.messageId;
});
if (cursorIndex !== -1) {
rawValues = rawValues.slice(
cursorIndex + 1,
cursorIndex + 1 + requestedTop,
);
}
// If cursor message not found (deleted or outside window), keep all results
}
Comment on lines +386 to +402
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Comment inaccuracy and missing trim when cursor not found.

Two issues:

  1. The comment on line 386 says "asc-ordered results" but the default orderby is receivedDateTime desc (line 314). The logic is correct, but the comment is misleading.

  2. When the cursor message is not found (line 401 fallback), rawValues retains up to requestedTop + 10 messages from the over-fetch. Callers may receive more messages than requested, which could cause unexpected behavior.

Proposed fix
   // Position-based skip: find the cursor message by ID in the
-  // asc-ordered results and take only messages after it. This avoids
+  // results and take only messages after it. This avoids
   // timestamp precision issues and correctly handles multiple messages
   // within the same second. Then trim to the originally requested $top
   // so the caller gets exactly the number of messages it asked for.
   if (cursor?.messageId) {
     const cursorIndex = rawValues.findIndex(function (msg) {
       return msg.id === cursor.messageId;
     });
     if (cursorIndex !== -1) {
       rawValues = rawValues.slice(
         cursorIndex + 1,
         cursorIndex + 1 + requestedTop,
       );
+    } else {
+      // Cursor message not found (deleted or outside window) — trim to
+      // requested count to avoid returning more messages than expected.
+      rawValues = rawValues.slice(0, requestedTop);
     }
-    // If cursor message not found (deleted or outside window), keep all results
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Position-based skip: find the cursor message by ID in the asc-ordered
// results and take only messages after it. This avoids timestamp precision
// issues and correctly handles multiple messages within the same second.
// Then trim to the originally requested $top so the caller gets exactly
// the number of messages it asked for.
if (cursor?.messageId) {
const cursorIndex = rawValues.findIndex(function (msg) {
return msg.id === cursor.messageId;
});
if (cursorIndex !== -1) {
rawValues = rawValues.slice(
cursorIndex + 1,
cursorIndex + 1 + requestedTop,
);
}
// If cursor message not found (deleted or outside window), keep all results
}
// Position-based skip: find the cursor message by ID in the
// results and take only messages after it. This avoids
// timestamp precision issues and correctly handles multiple messages
// within the same second. Then trim to the originally requested $top
// so the caller gets exactly the number of messages it asked for.
if (cursor?.messageId) {
const cursorIndex = rawValues.findIndex(function (msg) {
return msg.id === cursor.messageId;
});
if (cursorIndex !== -1) {
rawValues = rawValues.slice(
cursorIndex + 1,
cursorIndex + 1 + requestedTop,
);
} else {
// Cursor message not found (deleted or outside window) — trim to
// requested count to avoid returning more messages than expected.
rawValues = rawValues.slice(0, requestedTop);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/integrations/outlook/connector.ts` around lines 386 - 402, Update
the misleading comment to reflect that results are fetched in descending order
by receivedDateTime (orderby: receivedDateTime desc) and ensure we always trim
any over-fetched results down to requestedTop; specifically, after the cursor
handling in the block that references cursor?.messageId, keep the existing
findIndex/slice behavior when the cursor is found, and add a fallback that sets
rawValues = rawValues.slice(0, requestedTop) when the cursor message is not
found (so rawValues never contains more than requestedTop); refer to the
variables cursor, cursor.messageId, rawValues, and requestedTop when making
these changes.


const messages =
params.format === 'email'
? rawValues.map(function (msg) {
Expand Down
10 changes: 5 additions & 5 deletions examples/workflows/contract-generation/config.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,12 @@ describe('MessageList', () => {
afterEach(cleanup);

describe('approval rendering', () => {
it('renders approval inline after its linked message', () => {
it('renders a pending approval at the bottom', () => {
const message = createMessage({ id: 'msg-1', content: 'AI response' });
const approval = createUpdateApproval({
_id: 'approval-1' as Id<'approvals'>,
messageId: 'msg-1',
status: 'pending',
});

render(
Expand All @@ -119,102 +120,109 @@ describe('MessageList', () => {
).toBeInTheDocument();
});

it('does not render approval whose messageId does not match any loaded message', () => {
it('does not render completed approvals', () => {
const message = createMessage({ id: 'msg-2', content: 'Recent message' });
const orphanedApproval = createUpdateApproval({
_id: 'orphaned-approval' as Id<'approvals'>,
messageId: 'msg-evicted',
const completedApproval = createUpdateApproval({
_id: 'completed-ap' as Id<'approvals'>,
messageId: 'msg-2',
status: 'completed',
});

render(
<MessageList
{...defaultProps}
displayMessages={[message]}
workflowUpdateApprovals={[orphanedApproval]}
workflowUpdateApprovals={[completedApproval]}
workflowCreationApprovals={[]}
/>,
);

expect(
screen.queryByTestId('update-approval-orphaned-approval'),
screen.queryByTestId('update-approval-completed-ap'),
).not.toBeInTheDocument();
});

it('does not render approval without a messageId', () => {
it('does not render rejected approvals', () => {
const message = createMessage({ id: 'msg-3' });
const noMessageIdApproval = createUpdateApproval({
_id: 'no-mid-approval' as Id<'approvals'>,
messageId: undefined,
const rejectedApproval = createUpdateApproval({
_id: 'rejected-ap' as Id<'approvals'>,
messageId: 'msg-3',
status: 'rejected',
});

render(
<MessageList
{...defaultProps}
displayMessages={[message]}
workflowUpdateApprovals={[noMessageIdApproval]}
workflowUpdateApprovals={[rejectedApproval]}
workflowCreationApprovals={[]}
/>,
);

expect(
screen.queryByTestId('update-approval-no-mid-approval'),
screen.queryByTestId('update-approval-rejected-ap'),
).not.toBeInTheDocument();
});

it('renders multiple approvals on the same message', () => {
it('shows only the latest pending approval when multiple exist', () => {
const message = createMessage({ id: 'msg-4' });
const approval1 = createUpdateApproval({
const olderApproval = createUpdateApproval({
_id: 'ap-1' as Id<'approvals'>,
messageId: 'msg-4',
status: 'pending',
_creationTime: 1000,
});
const approval2 = createCreationApproval({
const newerApproval = createCreationApproval({
_id: 'ap-2' as Id<'approvals'>,
messageId: 'msg-4',
status: 'pending',
_creationTime: 2000,
});

render(
<MessageList
{...defaultProps}
displayMessages={[message]}
workflowUpdateApprovals={[approval1]}
workflowCreationApprovals={[approval2]}
workflowUpdateApprovals={[olderApproval]}
workflowCreationApprovals={[newerApproval]}
/>,
);

expect(screen.getByTestId('update-approval-ap-1')).toBeInTheDocument();
// Only the latest (ap-2) should be shown
expect(screen.getByTestId('creation-approval-ap-2')).toBeInTheDocument();
expect(
screen.queryByTestId('update-approval-ap-1'),
).not.toBeInTheDocument();
});

it('does not render orphaned approvals at the bottom when messages are evicted from pagination', () => {
const loadedMessage = createMessage({
id: 'msg-recent',
content: 'Recent',
});
const evictedApproval = createUpdateApproval({
_id: 'evicted-ap' as Id<'approvals'>,
messageId: 'msg-old-evicted',
status: 'rejected',
it('shows only the latest active approval when multiple pending exist across types', () => {
const message = createMessage({ id: 'msg-5' });
const olderPending = createUpdateApproval({
_id: 'older-ap' as Id<'approvals'>,
status: 'pending',
_creationTime: 1000,
});
const linkedApproval = createUpdateApproval({
_id: 'linked-ap' as Id<'approvals'>,
messageId: 'msg-recent',
const newerPending = createUpdateApproval({
_id: 'newer-ap' as Id<'approvals'>,
status: 'pending',
_creationTime: 2000,
});

render(
<MessageList
{...defaultProps}
displayMessages={[loadedMessage]}
workflowUpdateApprovals={[evictedApproval, linkedApproval]}
displayMessages={[message]}
workflowUpdateApprovals={[olderPending, newerPending]}
workflowCreationApprovals={[]}
/>,
);

// Only the latest (newer-ap) should render
expect(
screen.getByTestId('update-approval-linked-ap'),
screen.getByTestId('update-approval-newer-ap'),
).toBeInTheDocument();
expect(
screen.queryByTestId('update-approval-evicted-ap'),
screen.queryByTestId('update-approval-older-ap'),
).not.toBeInTheDocument();
});
});
Expand Down
Loading
Loading