Skip to content

Add screenshot persistence to panel_take_screenshot tool#145

Merged
5 commits merged into
mainfrom
copilot/save-screenshot-implementation
Feb 19, 2026
Merged

Add screenshot persistence to panel_take_screenshot tool#145
5 commits merged into
mainfrom
copilot/save-screenshot-implementation

Conversation

Copilot AI commented Feb 9, 2026

Copy link
Copy Markdown
Contributor

Screenshot Saving Feature Implementation

  • Review the current take_screenshot implementation
  • Add save_screenshot parameter to take_screenshot tool (default: True)
    • True: Save to .screenshots subfolder in user data directory
    • False: Don't save (returns image only to AI)
    • str (absolute path): Save to specified path
  • Add screenshot directory configuration to ServerConfig in config/models.py
  • Implement path validation (ensure absolute path or raise exception)
  • Implement file saving logic with timestamp-based naming
  • Keep return value as ImageContent only (per user request)
  • Add comprehensive tests for:
    • Default save behavior (save_screenshot=True)
    • Disabled save behavior (save_screenshot=False)
    • Custom path save behavior (save_screenshot="/absolute/path/to/file.png")
    • Path validation (raise exception for relative paths)
  • Update documentation
  • Move imports to top of file (code review feedback)
  • Run linters and tests (will run in CI)
  • Request code review
  • Run security scan (CodeQL)
Original prompt

This section details on the original issue you should resolve

<issue_title>Save screenshot</issue_title>
<issue_description>When using the panel_takescreenshot tool it would be really nice if the screenshot was saved such that it can be accessed by a human.

I would need to analyze if there are more things I would like to save.

  • I would suggest that screenshots are saved by default (True) to .screenshots subfolder of the data folder
  • I would suggest that its possible to disable screenshot save (False)
  • I would suggest its possible to provide a path as argument. The path should be absolute. If not an exception should be raised.</issue_description>

<agent_instructions>Please first review this suggestion. Please list improvements identified if any. Then make a plan for implementing this. Come back and ask me to review the plan.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Add functionality to save screenshots from panel_takescreenshot tool Add screenshot persistence to panel_take_screenshot tool Feb 9, 2026
Copilot AI requested a review from MarcSkovMadsen February 9, 2026 12:03

@MarcSkovMadsen MarcSkovMadsen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks like a good plan. But don't change the response. Continue to return the image only as response.

Please implement

Copilot AI and others added 2 commits February 9, 2026 12:49
Co-authored-by: MarcSkovMadsen <42288570+MarcSkovMadsen@users.noreply.github.com>
…eenshot parameter

Co-authored-by: MarcSkovMadsen <42288570+MarcSkovMadsen@users.noreply.github.com>

Copilot AI commented Feb 9, 2026

Copy link
Copy Markdown
Contributor Author

Looks like a good plan. But don't change the response. Continue to return the image only as response.

Please implement

Implemented in commits bcfd181 and 5051564. The take_screenshot tool now:

  • Accepts save_screenshot parameter (default: True)
    • True: Saves to ~/.holoviz-mcp/screenshots/ with timestamp-based filename
    • False: Skips saving, returns image only
    • str: Saves to specified absolute path (validates path is absolute)
  • Returns only ImageContent (unchanged from original behavior)
  • Logs save location when screenshots are saved
  • Added comprehensive tests for all save modes and path validation

Configuration can be customized via server.screenshots_dir in config.

Comment thread src/holoviz_mcp/panel_mcp/server.py Outdated

@MarcSkovMadsen MarcSkovMadsen left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please fix any comments

Co-authored-by: MarcSkovMadsen <42288570+MarcSkovMadsen@users.noreply.github.com>

Copilot AI commented Feb 9, 2026

Copy link
Copy Markdown
Contributor Author

Please fix any comments

All comments addressed in commit 8a8251d.

@MarcSkovMadsen MarcSkovMadsen closed this pull request by merging all changes into main in b098ce4 Feb 19, 2026
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.

Save screenshot

2 participants