fix: treat screenshot failure as non-critical in browser tool

[mason5052]

Description of the Change

Problem

The browser tool's ContentMD(), ContentHTML(), and Links() methods run content fetch and screenshot capture concurrently. When the screenshot request fails for any reason (scraper unavailable, timeout, image too small), the entire operation returns an error -- discarding valid, successfully-fetched page content.

The AI agent then receives an error like:

browser tool 'markdown' handled with error: failed to fetch screenshot by url '...': ...

...and interprets the URL as unreachable, wasting tool calls on retries or fallback approaches, even though the content was sitting in memory.

Additionally, getHTML() was using the wrong minimum content size constant (minMdContentSize = 50 bytes instead of minHtmlContentSize = 300 bytes), accepting HTML responses that are too small to be useful.

The file also mixed Go's stdlib log.Println with logrus structured logging used everywhere else in the codebase.

Solution

Three fixes in backend/pkg/tools/browser.go:

1. Graceful screenshot degradation: In ContentMD(), ContentHTML(), and Links(), screenshot failure is now logged as a warning and the content is returned with an empty screenshot name. This is consistent with wrapCommandResult() which already ignores the PutScreenshot return value (_, _ = b.scp.PutScreenshot(...)).

2. Fix getHTML() size check: Changed from minMdContentSize (50) to minHtmlContentSize (300).

3. Consistent logging: Replaced 3 log.Println calls with logrus.WithField("url", url).Info(...) and removed the unused "log" import.

Closes #149

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• Test update

Areas Affected

• AI Agents (Researcher/Developer/Executor)
• Security Tools Integration

Testing and Verification

Test Configuration

PentAGI Version: v1.2.0 (master @ e97bbe5)
Docker Version: N/A (unit tests only)
Host OS: Windows 11
LLM Provider: N/A

Test Steps

1. Added newTestScraper() helper that creates an httptest.Server simulating the scraper service with configurable screenshot behavior ("ok", "fail", "small")
2. Added 6 new test cases covering all three browser actions:
   • TestContentMD_ScreenshotFailure_ReturnsContent -- scraper returns 500 on /screenshot, content is still returned
   • TestContentHTML_ScreenshotFailure_ReturnsContent -- same for HTML
   • TestLinks_ScreenshotFailure_ReturnsContent -- same for links
   • TestContentMD_ScreenshotSmall_ReturnsContent -- screenshot below minImgContentSize, content still returned
   • TestContentMD_BothSucceed_ReturnsContentAndScreenshot -- happy path, both content and screenshot file verified
   • TestGetHTML_UsesCorrectMinContentSize -- content of 60 bytes (> 50, < 300) is correctly rejected with minHtmlContentSize

Test Results

• Before fix: ContentMD/ContentHTML/Links return error when screenshot fails, discarding valid content
• After fix: Content is returned; screenshot failure logged as warning with empty screenshot name
• Before fix: getHTML() accepts 60-byte HTML (passes minMdContentSize check)
• After fix: getHTML() correctly rejects content below minHtmlContentSize (300 bytes)

Security Considerations

No security impact. This change improves reliability by not discarding valid data on non-critical side-effect failure. No new dependencies or permission changes.

Performance Impact

Negligible. The only change in the success path is the addition of a warning log on screenshot failure, which is a fast operation. Content fetch performance is unchanged.

Documentation Updates

• README.md updates
• API documentation updates
• Configuration documentation updates
• GraphQL schema updates
• Other

No documentation changes required. The fix is internal behavior -- the browser tool API surface remains the same.

Deployment Notes

Backward-compatible. No new environment variables or configuration changes. The only observable difference is that the browser tool now returns content even when screenshots fail, which is the expected behavior described in #149.

Checklist

Code Quality

• My code follows the project's coding standards
• I have added/updated necessary documentation
• I have added tests to cover my changes
• All new and existing tests pass
• I have run go fmt and go vet (for Go code)

Security

• I have considered security implications
• Changes maintain or improve the security model
• Sensitive information has been properly handled

Compatibility

• Changes are backward compatible
• Dependencies are properly updated

Documentation

• Documentation is clear and complete
• Comments are added for non-obvious code

Additional Notes

This PR continues the browser tool hardening work from PR #120 (resource leak prevention). The wrapCommandResult() method already treats screenshot storage as optional (_, _ = b.scp.PutScreenshot(...)), so returning content without a screenshot is consistent with the existing design intent.

[Copilot]

Pull request overview

This PR fixes a critical bug where the browser tool would discard successfully-fetched page content when screenshot capture fails. The fix treats screenshot failures as non-critical warnings rather than fatal errors, ensuring the AI agent receives valid content even when screenshots are unavailable.

Changes:

• Modified ContentMD(), ContentHTML(), and Links() to continue with empty screenshot name when screenshot capture fails, logging a warning instead of returning an error
• Fixed getHTML() to use minHtmlContentSize (300 bytes) instead of minMdContentSize (50 bytes) for size validation
• Replaced stdlib log.Println calls with structured logrus.WithField() logging

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
backend/pkg/tools/browser.go	Implements graceful screenshot failure handling in three methods, fixes HTML size check constant, and standardizes logging to use logrus
backend/pkg/tools/browser_test.go	Adds comprehensive test coverage including helper function newTestScraper and 6 test cases covering screenshot failure scenarios and size validation

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.