Skip to content

Add optional chatSessionResource parameter for read operations in ManageTodoListTool#289290

Merged
bhavyaus merged 2 commits intomainfrom
dev/bhavyau/todo-update
Jan 21, 2026
Merged

Add optional chatSessionResource parameter for read operations in ManageTodoListTool#289290
bhavyaus merged 2 commits intomainfrom
dev/bhavyau/todo-update

Conversation

@bhavyaus
Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings January 21, 2026 01:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a chatSessionResource parameter to the IManageTodoListToolInputParams interface to enable read operations when the session resource is not available through the invocation context. The change provides a fallback mechanism for obtaining the chat session resource specifically for read operations.

Changes:

  • Added optional chatSessionResource string parameter to IManageTodoListToolInputParams interface
  • Modified the invoke method to use the parameter as a fallback when invocation.context?.sessionResource is undefined and the operation is 'read'
Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/chat/common/tools/builtinTools/manageTodoListTool.ts:100

  • The new fallback logic for chatSessionResource when operation is 'read' lacks test coverage. Consider adding tests that verify this behavior, particularly testing scenarios where invocation.context.sessionResource is undefined and args.chatSessionResource is provided, as well as cases with invalid URI strings to ensure proper error handling.
		const chatSessionResource = invocation.context?.sessionResource ?? (args.operation === 'read' && args.chatSessionResource ? URI.parse(args.chatSessionResource) : undefined);

Comment thread src/vs/workbench/contrib/chat/common/tools/builtinTools/manageTodoListTool.ts Outdated
…TodoListTool.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
status: 'not-started' | 'in-progress' | 'completed';
}>;
// used for todo read only
chatSessionResource?: string;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't it be a URI?

Copy link
Copy Markdown
Collaborator Author

@bhavyaus bhavyaus Jan 21, 2026

Choose a reason for hiding this comment

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

No. This is coming from an non llm tool call and I can't pass the sessionResource string back from the chat extension as a URI

@bhavyaus bhavyaus merged commit f60f946 into main Jan 21, 2026
22 checks passed
@bhavyaus bhavyaus deleted the dev/bhavyau/todo-update branch January 21, 2026 04:33
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Mar 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants