feat: browser_file_download
#209
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for a new browser file download tool, alongside accompanying tests and updated type definitions. Key changes include:
- Introducing a TestServer class to facilitate serving files during tests.
- Adding new tests for browser file download functionality, including both HTML and PDF links.
- Extending the tool definitions and context handling to support a download modal state.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fixtures.ts | Introduces a TestServer class for managing a local HTTP server. |
| tests/files.spec.ts | Adds tests verifying browser file download behavior. |
| src/tools/tool.ts | Updates modal state types to include download state. |
| src/tools/files.ts | Implements the downloadFile tool and its schema for file downloads. |
| src/context.ts | Adds modal state tracking and handling for download events. |
Comments suppressed due to low confidence (1)
tests/fixtures.ts:28
- [nitpick] Consider renaming the instance property 'PREFIX' to 'prefix' to indicate that it is not a constant.
PREFIX: string;
| try { | ||
| await this.page.goto(url, { waitUntil: 'domcontentloaded' }); | ||
| } catch (error) { | ||
| if (error instanceof Error && error.message.includes('net::ERR_ABORTED') && this.context.hasModalState('download')) |
There was a problem hiding this comment.
That's be a very Chrome-centric view on the matter.
| @@ -293,6 +297,13 @@ export class Tab { | |||
| fileChooser: chooser, | |||
| }, this); | |||
| }); | |||
| page.on('download', download => { | |||
| this.context.setModalState({ | |||
There was a problem hiding this comment.
I am not sure this is a modal state we'll be requiring to clear. Sounds to me like downloads is more of resources, so maybe just tell user once that download took place and teach it to list the downloads as a tool?
I.e. let's just save it here!
|
Summary from offline discussion:
|
|
Hi! |
|
don't worry José, we're continuing work in #310 :D |
Resolves #154