vxcontrol/pentagi
1
0
Fork 0
mirror of https://github.com/vxcontrol/pentagi.git synced 2026-07-21 08:15:23 -04:00
Code Issues 45 Packages Projects Releases 9 Wiki Activity

[PR #165] [CLOSED] fix: apply constructed opts to Google search service creation #206

New Issue
Closed
opened 2026-06-06 22:09:41 -04:00 by yindo · 0 comments
yindo commented 2026-06-06 22:09:41 -04:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/165
Author: @mason5052
Created: 3/2/2026
Status: ❌ Closed

Base: master ← Head: fix/google-proxy-opts-unused

📝 Commits (10+)

  • c68f933 fix: treat screenshot failure as non-critical in browser tool
  • 5720402 fix: propagate container lookup error in GetTool for terminal/file tools
  • e36f47f fix: prevent http.DefaultClient mutation in search tools
  • c716ec6 test: remove unused test server and dead imports in search tool tests
  • ddd0b0e test: add unit tests for perplexity and google search tools
  • c538b80 test: address review feedback on search tool tests
  • 8354f7e chore(deps): bump rollup
  • f4da45e chore(deps): bump go.opentelemetry.io/otel/sdk
  • 1c3f149 feat: add Novita AI as optional LLM provider
  • 47e1469 chore: expand .gitignore with IDE and OS patterns

📊 Changes

20 files changed (+1904 additions, -165 deletions)

View changed files

📝 .gitignore (+9 -0)
📝 .vscode/launch.json (+2 -0)
📝 Dockerfile (+1 -0)
📝 README.md (+10 -4)
📝 backend/cmd/ftester/worker/executor.go (+8 -2)
📝 backend/go.mod (+9 -8)
📝 backend/go.sum (+18 -16)
📝 backend/pkg/tools/browser.go (+14 -10)
📝 backend/pkg/tools/browser_test.go (+191 -0)
📝 backend/pkg/tools/google.go (+1 -1)
➕ backend/pkg/tools/google_test.go (+172 -0)
➕ backend/pkg/tools/perplexity_test.go (+349 -0)
📝 backend/pkg/tools/tavily.go (+5 -3)
➕ backend/pkg/tools/tavily_test.go (+148 -0)
📝 backend/pkg/tools/terminal.go (+1 -1)
📝 backend/pkg/tools/traversaal.go (+5 -3)
➕ backend/pkg/tools/traversaal_test.go (+140 -0)
➕ examples/configs/novita.provider.yml (+146 -0)
➕ examples/tests/novita-report.md (+537 -0)
📝 frontend/package-lock.json (+138 -117)

📄 Description

Description of the Change

Problem

In newSearchService() (backend/pkg/tools/google.go), the opts slice is correctly constructed with both the API key and (when configured) a proxy HTTP client:

opts := []option.ClientOption{
    option.WithAPIKey(g.apiKey),
}

if g.proxyURL != "" {
    opts = append(opts, option.WithHTTPClient(&http.Client{
        Transport: &http.Transport{
            Proxy: func(req *http.Request) (*url.URL, error) {
                return url.Parse(g.proxyURL)
            },
        },
    }))
}

However, the actual service creation call ignores opts entirely and passes a hardcoded option.WithAPIKey(g.apiKey):

svc, err := customsearch.NewService(ctx, option.WithAPIKey(g.apiKey))  // opts is unused!

This means proxy configuration has no effect on Google search requests, even when proxyURL is set.

Solution

Replace the hardcoded argument with opts...:

svc, err := customsearch.NewService(ctx, opts...)

This ensures the proxy HTTP client (when configured) is actually applied to the Google Custom Search API client.

Type of Change

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

Areas Affected

  • External Integrations (Google Custom Search API)

Testing and Verification

Test Configuration

PentAGI Version: feature/next_release @ 08359b6
Docker Version: N/A (code review + existing tests)
Host OS: Windows 11
LLM Provider: N/A

Test Steps

  1. Read google.go and identified opts variable never passed to NewService
  2. Verified the same proxy pattern works correctly in other tools (tavily.go, traversaal.go, browser.go)
  3. Confirmed existing google_test.go tests pass (TestGoogleNewSearchServiceWithoutProxy, TestGoogleNewSearchServiceWithProxy)

Test Results

  • The fix is a single-token change: option.WithAPIKey(g.apiKey) -> opts...
  • Existing test TestGoogleNewSearchServiceWithProxy now actually verifies that the proxy client option is applied (previously the test passed vacuously because the proxy opts were built but silently dropped)

Security Considerations

This fix improves security by ensuring proxy configuration is honored. When a proxy is configured (e.g., for traffic inspection or network isolation), the current code silently bypasses it. After this fix, Google search traffic correctly routes through the configured proxy.

Performance Impact

None. The only change is which ClientOption values are passed to NewService.

Documentation Updates

No documentation changes required.

Deployment Notes

Backward-compatible. When proxyURL is empty, opts contains only option.WithAPIKey(g.apiKey), producing identical behavior to the current code.

Checklist

Code Quality

  • My code follows the project's coding standards
  • I have added/updated necessary documentation
  • 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 bug was spotted during Copilot code review on PR #153 (google_test.go), which noted: "The production code at google.go:141 calls NewService with only option.WithAPIKey, ignoring the opts slice that was built to include the proxy client."

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/165 **Author:** [@mason5052](https://github.com/mason5052) **Created:** 3/2/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `fix/google-proxy-opts-unused` --- ### 📝 Commits (10+) - [`c68f933`](https://github.com/vxcontrol/pentagi/commit/c68f9335e06ae39606c4376a777a19cf624882ca) fix: treat screenshot failure as non-critical in browser tool - [`5720402`](https://github.com/vxcontrol/pentagi/commit/5720402301ac6fedadb2005f592e877436f81fda) fix: propagate container lookup error in GetTool for terminal/file tools - [`e36f47f`](https://github.com/vxcontrol/pentagi/commit/e36f47ffd379ea93c944c7a5303e990884834cc0) fix: prevent http.DefaultClient mutation in search tools - [`c716ec6`](https://github.com/vxcontrol/pentagi/commit/c716ec6a7430fb47df5de3b2bc114afb9676c418) test: remove unused test server and dead imports in search tool tests - [`ddd0b0e`](https://github.com/vxcontrol/pentagi/commit/ddd0b0e16e168abafa48bac04909201ce95f30f8) test: add unit tests for perplexity and google search tools - [`c538b80`](https://github.com/vxcontrol/pentagi/commit/c538b803dcd39c34e37f1b17d783d2767e3f044e) test: address review feedback on search tool tests - [`8354f7e`](https://github.com/vxcontrol/pentagi/commit/8354f7e3a992a6bde892d791daaf40beb6a5e331) chore(deps): bump rollup - [`f4da45e`](https://github.com/vxcontrol/pentagi/commit/f4da45e63b6f9e7b47994fd90eca58a95201d48c) chore(deps): bump go.opentelemetry.io/otel/sdk - [`1c3f149`](https://github.com/vxcontrol/pentagi/commit/1c3f14971feeda7b65852dbb81fac6a4d1ab9b31) feat: add Novita AI as optional LLM provider - [`47e1469`](https://github.com/vxcontrol/pentagi/commit/47e14696779d339b801f98ad153be531f3d63a31) chore: expand .gitignore with IDE and OS patterns ### 📊 Changes **20 files changed** (+1904 additions, -165 deletions) <details> <summary>View changed files</summary> 📝 `.gitignore` (+9 -0) 📝 `.vscode/launch.json` (+2 -0) 📝 `Dockerfile` (+1 -0) 📝 `README.md` (+10 -4) 📝 `backend/cmd/ftester/worker/executor.go` (+8 -2) 📝 `backend/go.mod` (+9 -8) 📝 `backend/go.sum` (+18 -16) 📝 `backend/pkg/tools/browser.go` (+14 -10) 📝 `backend/pkg/tools/browser_test.go` (+191 -0) 📝 `backend/pkg/tools/google.go` (+1 -1) ➕ `backend/pkg/tools/google_test.go` (+172 -0) ➕ `backend/pkg/tools/perplexity_test.go` (+349 -0) 📝 `backend/pkg/tools/tavily.go` (+5 -3) ➕ `backend/pkg/tools/tavily_test.go` (+148 -0) 📝 `backend/pkg/tools/terminal.go` (+1 -1) 📝 `backend/pkg/tools/traversaal.go` (+5 -3) ➕ `backend/pkg/tools/traversaal_test.go` (+140 -0) ➕ `examples/configs/novita.provider.yml` (+146 -0) ➕ `examples/tests/novita-report.md` (+537 -0) 📝 `frontend/package-lock.json` (+138 -117) </details> ### 📄 Description ### Description of the Change #### Problem In `newSearchService()` (`backend/pkg/tools/google.go`), the `opts` slice is correctly constructed with both the API key and (when configured) a proxy HTTP client: ```go opts := []option.ClientOption{ option.WithAPIKey(g.apiKey), } if g.proxyURL != "" { opts = append(opts, option.WithHTTPClient(&http.Client{ Transport: &http.Transport{ Proxy: func(req *http.Request) (*url.URL, error) { return url.Parse(g.proxyURL) }, }, })) } ``` However, the actual service creation call ignores `opts` entirely and passes a hardcoded `option.WithAPIKey(g.apiKey)`: ```go svc, err := customsearch.NewService(ctx, option.WithAPIKey(g.apiKey)) // opts is unused! ``` This means **proxy configuration has no effect** on Google search requests, even when `proxyURL` is set. #### Solution Replace the hardcoded argument with `opts...`: ```go svc, err := customsearch.NewService(ctx, opts...) ``` This ensures the proxy HTTP client (when configured) is actually applied to the Google Custom Search API client. ### Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ### Areas Affected - [x] External Integrations (Google Custom Search API) ### Testing and Verification #### Test Configuration ```yaml PentAGI Version: feature/next_release @ 08359b6 Docker Version: N/A (code review + existing tests) Host OS: Windows 11 LLM Provider: N/A ``` #### Test Steps 1. Read `google.go` and identified `opts` variable never passed to `NewService` 2. Verified the same proxy pattern works correctly in other tools (tavily.go, traversaal.go, browser.go) 3. Confirmed existing `google_test.go` tests pass (TestGoogleNewSearchServiceWithoutProxy, TestGoogleNewSearchServiceWithProxy) #### Test Results - The fix is a single-token change: `option.WithAPIKey(g.apiKey)` -> `opts...` - Existing test `TestGoogleNewSearchServiceWithProxy` now actually verifies that the proxy client option is applied (previously the test passed vacuously because the proxy opts were built but silently dropped) ### Security Considerations This fix **improves security** by ensuring proxy configuration is honored. When a proxy is configured (e.g., for traffic inspection or network isolation), the current code silently bypasses it. After this fix, Google search traffic correctly routes through the configured proxy. ### Performance Impact None. The only change is which `ClientOption` values are passed to `NewService`. ### Documentation Updates No documentation changes required. ### Deployment Notes Backward-compatible. When `proxyURL` is empty, `opts` contains only `option.WithAPIKey(g.apiKey)`, producing identical behavior to the current code. ### Checklist #### Code Quality - [x] My code follows the project's coding standards - [x] I have added/updated necessary documentation - [x] All new and existing tests pass - [x] I have run `go fmt` and `go vet` (for Go code) #### Security - [x] I have considered security implications - [x] Changes maintain or improve the security model - [x] Sensitive information has been properly handled #### Compatibility - [x] Changes are backward compatible - [x] Dependencies are properly updated #### Documentation - [x] Documentation is clear and complete - [x] Comments are added for non-obvious code ### Additional Notes This bug was spotted during Copilot code review on PR #153 (google_test.go), which noted: "The production code at google.go:141 calls NewService with only option.WithAPIKey, ignoring the opts slice that was built to include the proxy client." --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-06-06 22:09:41 -04:00
yindo closed this issue 2026-06-06 22:09:41 -04:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
enhancement
help wanted
pull-request
Mirrored from GitHub Pull Request
 question
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#206
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?