Repository navigation
Conversation
commit: |
e1719f8 to
538a49a
Compare
There was a problem hiding this comment.
5 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/vercel/client/toolApprovals.test.ts">
<violation number="1" location="src/vercel/client/toolApprovals.test.ts:280">
P2: `wholeThread` silently drops messages older than the first 100, so these regression tests stop exercising full-history context handling once a thread grows beyond that page. Iterate with `continueCursor` until `isDone` before converting the documents.</violation>
</file>
<file name="src/vercel/client/search.ts">
<violation number="1" location="src/vercel/client/search.ts:696">
P2: Sort the planning input chronologically. Search results are placed before recent messages regardless of order, but `planApprovals` depends on chronological order to pair requests, results, and decisions. Sorting only this input leaves the model-visible order unchanged.</violation>
</file>
<file name="docs/tool-approval.mdx">
<violation number="1" location="docs/tool-approval.mdx:171">
P2: A context handler does not automatically see every message in the thread; it receives the assembled recent/search context, which can omit older approval parts. Describe this as the assembled context and tell users to fetch full history explicitly when needed.</violation>
</file>
<file name="src/vercel/client/prepareApprovalContext.ts">
<violation number="1" location="src/vercel/client/prepareApprovalContext.ts:65">
P2: Conflicting stored decisions for the same approval are silently ignored, so a continuation can execute using the first response instead of detecting inconsistent approval state. Compare duplicate responses with the canonical decision and throw when they differ.</violation>
<violation number="2" location="src/vercel/client/prepareApprovalContext.ts:175">
P2: A handler that moves or omits a denied call's request causes the call to be dropped instead of receiving its stored denial. Associate denials with the stored call ID and use that mapping when answering retained calls.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const wholeThread: ContextHandler = async (ctx, { threadId }) => { | ||
| const { page } = await listMessages(ctx, components.agent, { | ||
| threadId: threadId!, | ||
| paginationOpts: { cursor: null, numItems: 100 }, |
There was a problem hiding this comment.
P2: wholeThread silently drops messages older than the first 100, so these regression tests stop exercising full-history context handling once a thread grows beyond that page. Iterate with continueCursor until isDone before converting the documents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/vercel/client/toolApprovals.test.ts, line 280:
<comment>`wholeThread` silently drops messages older than the first 100, so these regression tests stop exercising full-history context handling once a thread grows beyond that page. Iterate with `continueCursor` until `isDone` before converting the documents.</comment>
<file context>
@@ -282,17 +270,18 @@ const handlers = {
+const wholeThread: ContextHandler = async (ctx, { threadId }) => {
+ const { page } = await listMessages(ctx, components.agent, {
+ threadId: threadId!,
+ paginationOpts: { cursor: null, numItems: 100 },
+ });
+ return docsToModelMessages(page.reverse());
</file context>
|
|
||
| const continuation = | ||
| const approvals = planApprovals( | ||
| [...searchMessages, ...recentMessages], |
There was a problem hiding this comment.
P2: Sort the planning input chronologically. Search results are placed before recent messages regardless of order, but planApprovals depends on chronological order to pair requests, results, and decisions. Sorting only this input leaves the model-visible order unchanged.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/vercel/client/search.ts, line 696:
<comment>Sort the planning input chronologically. Search results are placed before recent messages regardless of order, but `planApprovals` depends on chronological order to pair requests, results, and decisions. Sorting only this input leaves the model-visible order unchanged.</comment>
<file context>
@@ -693,10 +692,12 @@ export async function fetchContextWithPrompt(
- const continuation =
+ const approvals = planApprovals(
+ [...searchMessages, ...recentMessages],
args.prompt === undefined
- ? approvalContinuation(promptMessage, searchMessages, recentMessages)
</file context>
| [...searchMessages, ...recentMessages], | |
| sorted([...searchMessages, ...recentMessages]), |
| context. They stay pending in storage, so you can still decide and resume them | ||
| later. A denial that was never resumed is shown to the model with its reason. | ||
|
|
||
| A [context handler](./context.mdx) sees every message, approval requests and |
There was a problem hiding this comment.
P2: A context handler does not automatically see every message in the thread; it receives the assembled recent/search context, which can omit older approval parts. Describe this as the assembled context and tell users to fetch full history explicitly when needed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/tool-approval.mdx, line 171:
<comment>A context handler does not automatically see every message in the thread; it receives the assembled recent/search context, which can omit older approval parts. Describe this as the assembled context and tell users to fetch full history explicitly when needed.</comment>
<file context>
@@ -163,16 +163,18 @@ single-decision helpers so the request history is read only once.
+context. They stay pending in storage, so you can still decide and resume them
+later. A denial that was never resumed is shown to the model with its reason.
+
+A [context handler](./context.mdx) sees every message, approval requests and
+decisions included, and can return any history. The approval requests and
+decisions it returns are not sent. When generation resumes from a decision, the
</file context>
| } else if ( | ||
| part.type === "tool-approval-response" && | ||
| isProviderCall(stored, storedRequests.get(part.approvalId)!.toolCallId) | ||
| !decisions.has(part.approvalId) |
There was a problem hiding this comment.
P2: Conflicting stored decisions for the same approval are silently ignored, so a continuation can execute using the first response instead of detecting inconsistent approval state. Compare duplicate responses with the canonical decision and throw when they differ.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/vercel/client/prepareApprovalContext.ts, line 65:
<comment>Conflicting stored decisions for the same approval are silently ignored, so a continuation can execute using the first response instead of detecting inconsistent approval state. Compare duplicate responses with the canonical decision and throw when they differ.</comment>
<file context>
@@ -1,340 +1,246 @@
+ } else if (
part.type === "tool-approval-response" &&
- isProviderCall(stored, storedRequests.get(part.approvalId)!.toolCallId)
+ !decisions.has(part.approvalId)
) {
- waitingProviderApprovalIds.add(part.approvalId);
</file context>
| if (part.providerExecuted) return undefined; | ||
| const result = localResults.get(`${i}:${part.toolCallId}`); | ||
| if (result) return result; | ||
| const request = partsOf(messages[i]).find( |
There was a problem hiding this comment.
P2: A handler that moves or omits a denied call's request causes the call to be dropped instead of receiving its stored denial. Associate denials with the stored call ID and use that mapping when answering retained calls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/vercel/client/prepareApprovalContext.ts, line 175:
<comment>A handler that moves or omits a denied call's request causes the call to be dropped instead of receiving its stored denial. Associate denials with the stored call ID and use that mapping when answering retained calls.</comment>
<file context>
@@ -1,340 +1,246 @@
+ if (part.providerExecuted) return undefined;
+ const result = localResults.get(`${i}:${part.toolCallId}`);
+ if (result) return result;
+ const request = partsOf(messages[i]).find(
+ (other) =>
+ other.type === "tool-approval-request" &&
</file context>
Closes #411. Supersedes #416.
When a context handler returned older history than the agent had fetched, the next approval continuation failed. The handler's approval parts were checked against only the fetched messages.
Context handlers now see the whole thread and can return any history. No approval request or decision that a handler returns is sent to the model. A continuation appends its call, request and decision from storage, and takes the call from the request's own message. Any other approval call is sent only with its own result, placed right after the call, or with its stored denial. Unresolved approvals, including approved ones that were never resumed, are left out of other generations instead of being auto-denied, and they stay pending.
An overlapping or retried continuation can still run a decision twice; at-most-once comes with durable runs.