Skip to content

fix: harden tool approval resume flow - #1180

Draft
LukasParke wants to merge 1 commit into
mainfrom
devin/1791237137-approval-flow-fixes
Draft

LukasParke wants to merge 1 commit into
mainfrom
devin/1791237137-approval-flow-fixes

Conversation

@LukasParke

Copy link
Copy Markdown
Contributor

Summary

This PR fixes three defects in the hand-written callModel approval flow in src/lib. Persistent edits are enabled, so these changes survive regeneration.

  • Approval predicate runs against raw model arguments before the input schema is applied #520: toolRequiresApproval passed the raw model arguments to a tool's requireApproval(args) predicate. A model could leave out a field whose schema default is sensitive, the predicate would see undefined and skip approval, and the tool would then execute with the default. The predicate now receives z4.safeParse(inputSchema, args).data and falls back to the raw args only when parsing fails. In that case execution fails validation anyway.
    inputSchema: z.object({ recursive: z.boolean().default(true) }),
    requireApproval: (args) => args.recursive, // was: undefined -> no approval
  • Approval decisions are not de-duplicated, so an approval-gated tool can execute twice #518: approveToolCalls and rejectToolCalls are now de-duplicated, so an ID listed twice no longer executes a side-effecting tool twice. The ModelResult constructor now throws when an ID appears in both lists, instead of executing the tool and also recording a rejection.
  • Resuming an awaiting_approval conversation with no decision advances it without recording an approval #517: Resuming a conversation in the awaiting_approval state with no decisions used to fall through to normal initialization and move the conversation to in_progress. It now returns early with isResumingFromApproval = true. This is the same path a partial-decision resume already takes, so getToolExecution exits while the conversation stays paused.

Added a unit test for #520. The test fails without the change. Full unit suite: 211 tests pass.

Searched existing PRs; none matched.

Link to Devin session: https://openrouter.devinenterprise.com/sessions/78072f08dc274fda9d8309045b1ab806
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/78072f08dc274fda9d8309045b1ab806?variant=devin
Requested by: @LukasParke

@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

Original prompt from Luke

SYSTEM:
<latest_message>
Luke Parke (U0BF2BKT52N) [ts=1791235775.813579]: @Devin audit all open issues on our public Typescript, Python, and Go SDKs, and the terraform Provider repo, and list out the unique problems, and vet them to determine validity and severity
</latest_message>

=== BEGIN THREAD HISTORY (in #agents) ===
Luke Parke (U0BF2BKT52N) [ts=1791235775.813579]: @Devin audit all open issues on our public Typescript, Python, and Go SDKs, and the terraform Provider repo, and list out the unique problems, and vet them to determine validity and severity
=== END THREAD HISTORY ===
Channel ID: C07UF9XLTFF
Thread URL: https://openrouter.slack.com/archives/C07UF9XLTFF/p1791235775813579?thread_ts=1791235775.813579&amp;cid=C07UF9XLTFF

The <latest_message> is the message that you should use to guide your goals + task for this session, and you should use the rest of the slack thread as context.
A [ts=...] marker on a Slack message is that message's timestamp. To act on a specific message with the slack tool (e.g. adding an emoji reaction via the reaction command), pass that value as timestamp along with the Channel ID — no extra lookup call is needed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant