Conversation
- adjusts flag logic to merge flags and treat /chromium/flags as a runtime overlay
There was a problem hiding this comment.
Performed full review of e0ca1d9...7993d69
Analysis
-
Shell Script Flag Parsing Vulnerabilities - The flag merging logic may have edge cases with complex flag formats that could lead to unexpected behavior or security issues.
-
Hardcoded Ownership Assumptions - The implementation contains hardcoded ownership changes that make specific deployment assumptions, reducing flexibility for different environments.
-
External Dependencies in Tests - End-to-end tests rely on external services which could impact reliability and cause intermittent test failures.
-
Restart Coordination Race Conditions - Despite attention to synchronization, potential race conditions may exist in the DevTools proxy pub-sub mechanism during browser restarts.
Tip
⚡ Quick Actions
This review was generated by Mesa.
Actions:
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
10 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
Sayan-
left a comment
There was a problem hiding this comment.
generally looks good! a few small q's about the API
server/cmd/api/api/chromium.go
Outdated
| if fieldName != "extensions.zip_file" && fieldName != "extensions.name" { | ||
| return oapi.UploadExtensionsAndRestart400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: "invalid form field: " + part.FormName()}}, nil | ||
| } |
There was a problem hiding this comment.
nit - redundant with the following switch
Sayan-
left a comment
There was a problem hiding this comment.
flag changes are slick! pending bot comments and otherwise lgtm
rgarcia
left a comment
There was a problem hiding this comment.
not finished yet but submitting comments so far
Checklist
Note
Adds POST /chromium/upload-extensions-and-restart and a new Go-based chromium-launcher, updates flag handling, DevTools proxy, OpenAPI/client, Docker images, and tests/CI.
POST /chromium/upload-extensions-and-restart: accepts zipped extensions, extracts to/home/kernel/extensions/<name>, updates runtime flags (/chromium/flags), restarts Chromium viasupervisorctl, and waits for DevTools readiness.UpstreamManagerand scale-to-zero controller intoApiServiceand server startup.chromiumflagslib to parse/merge base/runtime flags and write/read structured flag files; comprehensive unit tests.start-chromium.shwith Gochromium-launcher(supports headless/headful, flag merge, run-as-user); add unit tests and CI workflow.chromium-launcherand supervisor configs to invoke it.Subscribe) toUpstreamManagerand broadcast updates; tests updated/added.oapiclient/server types and handlers.TestExtensionUploadAndActivationand Playwright command to verify title changes via content script.Written by Cursor Bugbot for commit 322545f. This will update automatically on new commits. Configure here.
TL;DR
Adds an API endpoint to upload and load custom browser extensions, triggering a browser restart.
Why we made these changes
This addresses a feature request to allow users to install their own browser extensions (e.g., ad-blockers, testing tools), enabling greater customization of the browser environment.
What changed?
POST /chromium/upload-extensions-and-restartendpoint. It handles uploading zipped extensions, extracting them, updating runtime flags, and restarting the browser viasupervisorctl.start-chromium.shscripts with a new, unit-tested Go binary (chromium-launcher). This launcher is responsible for merging base and runtime flags, allowing extensions to be loaded dynamically.UpstreamManagerto notify the API service when the browser is ready after a restart, ensuring the request waits for completion.openapi.yamland regenerated the Go client to include the new extension upload endpoint.Validation
TestExtensionUploadAndActivation) that uploads a sample extension and uses Playwright to verify it is loaded and active in the browser.chromium-launcherandchromiumflagslibrary.Description generated by Mesa. Update settings