fix: remove Firefox from the supported browser types - #59
Merged
Conversation
Notte runs Chromium-family browsers only. Starting a session with --browser-type firefox is rejected by the API: Value error, Firefox sessions are not supported. Please use 'chromium' or 'chrome' as `browser_type`. The CLI still advertised it in several places. Removed from the README (both the feature list and the --browser-type line) and from the gen-flags field description override, which still read "Can be chromium, chrome or firefox" even though the generated flag help has since been corrected to "Supported values are chromium and chrome". validate.Browser was wrong in both directions: it accepted firefox and webkit, which the API rejects, and rejected chrome, which is valid. That list looks like Playwright's browser set rather than Notte's. It now accepts chromium and chrome plus the chrome-nightly / chrome-turbo legacy aliases, and its error message names the real options. Note validate.Browser is currently dead code - ValidateBrowser is referenced only by its own unit test, never wired to --browser-type, which is why an invalid value reaches the API today. Wiring it up would give a cleaner client-side error but changes behaviour and would stop the integration error-parsing tests exercising the API path, so it is left for a separate decision. internal/api/client.gen.go still carries SessionResponseBrowserTypeFirefox. It is generated from the OpenAPI spec and deliberately untouched; removing it means dropping firefox from the spec's browser_type Literal, which is an API-side change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| internal/validate/validate.go | Corrects the accepted browser set and validation error to match the backend contract. |
| scripts/gen-flags/types.go | Removes Firefox from the fallback browser-type help text used by flag generation. |
| internal/validate/validate_test.go | Covers supported Chromium-family values and rejection of Firefox and WebKit. |
| tests/integration/errors_test.go | Updates integration expectations so Firefox is no longer treated as a valid browser suggestion. |
| README.md | Updates feature and flag documentation to advertise Chromium and Chrome rather than Firefox. |
Reviews (1): Last reviewed commit: "fix: remove Firefox from the supported b..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Notte runs Chromium-family browsers only. Starting a session with
--browser-type firefoxis rejected by the API:The CLI still advertised it in several places.
Changes
README.mdfeature listREADME.mdflag line--browser-type chromium|chrome|firefox--browser-type chromium|chromescripts/gen-flags/types.gointernal/validate/validate.gochromium|firefox|webkitchromium|chrome+ legacy aliasesThe gen-flags override had drifted: the generated flag help already reads "Supported values are chromium and chrome. chrome-nightly and chrome-turbo are legacy aliases for chrome", but the override source it is generated from still mentioned Firefox, so it would come back the next time that override applies.
validate.Browserwas wrong in both directionsThat's Playwright's browser set, not Notte's. It now accepts
chromiumandchromeplus thechrome-nightly/chrome-turbolegacy aliases, and the error message names the real options.Two things deliberately left alone
validate.Browseris dead code.ValidateBrowseris referenced only by its own unit test — never wired to--browser-type, which is why an invalid value reaches the API today rather than failing locally. Wiring it up would give a cleaner client-side error and save a round trip, but it changes behaviour and would stopTestErrorParsing_InvalidBrowserType/TestErrorParsing_ValidationErrorContainsDetailsexercising the API error path, since they use an invalid browser type as their trigger. Happy to do it in a follow-up if you want it — it needs those tests re-pointed at a different invalid input.internal/api/client.gen.gostill hasSessionResponseBrowserTypeFirefox. It is generated from the OpenAPI spec and untouched here. Removing it means droppingfirefoxfrom the spec'sbrowser_typeLiteral — worth doing, since the API currently accepts it through the Literal only to reject it in a second validator, which is what produces the slightly confusing two-layer error above.Testing
go build ./...,go vet ./...,gofmt -lcleango test ./internal/...— all 10 packages passchromeand its aliases are acceptedtests/integration/errors_test.gono longer listsfirefoxamong the browser names it expects to see in a validation errorVerified against prod that
--browser-type firefoxis rejected end to end.🤖 Generated with Claude Code