fix(browser): enforce limits and add cleanup

changelog.d/20260525_000000_browser_limits_cleanup.md

@@ -0,0 +1,6 @@
+---
+bump: patch
+---
+
+### Fixed
+- Enforce docker-git's browser CPU/RAM limits in the Rust browser container startup and add a Rust stop command for shutdown cleanup.

src/browser.rs

@@ -21,14 +21,19 @@ use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 use std::time::{Duration, SystemTime, UNIX_EPOCH};
 
-use crate::BrowserSpec;
+use crate::{BrowserResourceLimits, BrowserSpec};
 
 pub(crate) struct BrowserRuntime {
     pub container_name: String,
     pub novnc_url: String,
     pub cdp_url: String,
 }
 
+pub(crate) struct BrowserStopRuntime {
+    pub container_name: String,
+    pub removed: bool,
+}
+
 const BROWSER_DOCKERFILE: &str = r#"FROM kechangdev/browser-vnc:latest
 
 # bash/procps keep upstream startup scripts compatible; socat exposes a stable CDP port.
@@ -89,7 +94,11 @@ impl DockerBrowserShell {
         Self
     }
 
-    pub fn ensure_browser_container(&self, spec: &BrowserSpec) -> Result<BrowserRuntime> {
+    pub fn ensure_browser_container(
+        &self,
+        spec: &BrowserSpec,
+        limits: &BrowserResourceLimits,
+    ) -> Result<BrowserRuntime> {
         ensure_docker_available()?;
         ensure_browser_image(spec)?;
 
@@ -119,14 +128,37 @@ impl DockerBrowserShell {
         );
 
         ensure_volume(&runtime_spec.volume_name)?;
-        run_browser_container(&runtime_spec)?;
+        run_browser_container(&runtime_spec, limits)?;
         let (cdp_url, novnc_url) = runtime_urls(&runtime_spec)?;
         Ok(BrowserRuntime {
             container_name: runtime_spec.container_name,
             novnc_url,
             cdp_url,
         })
     }
+
+    pub fn stop_browser_container(&self, spec: &BrowserSpec) -> Result<BrowserStopRuntime> {
+        ensure_docker_available()?;
+        let removed = match inspect_container_state(&spec.container_name)? {
+            Some(_) => {
+                match docker(["rm", "-f", &spec.container_name], "docker rm browser") {
+                    Ok(_) => {}
+                    Err(error) => {
+                        if inspect_container_state(&spec.container_name)?.is_some() {
+                            return Err(error);
+                        }
+                    }
+                }
+                true
+            }
+            None => false,
+        };
+
+        Ok(BrowserStopRuntime {
+            container_name: spec.container_name.clone(),
+            removed,
+        })
+    }
 }
 
 fn docker<const N: usize>(args: [&str; N], label: &str) -> Result<String> {
@@ -346,7 +378,21 @@ fn ensure_volume(volume_name: &str) -> Result<()> {
     docker(["volume", "create", volume_name], "docker volume create").map(|_| ())
 }
 
-fn run_browser_container(spec: &BrowserSpec) -> Result<()> {
+fn browser_resource_limit_args(limits: &BrowserResourceLimits) -> Vec<String> {
+    let mut args = Vec::new();
+
+    if let Some(cpu_limit) = &limits.cpu_limit {
+        args.extend(["--cpus".to_string(), cpu_limit.clone()]);
+    }
+
+    if let Some(ram_limit) = &limits.ram_limit {
+        args.extend(["--memory".to_string(), ram_limit.clone()]);
+    }
+
+    args
+}
+
+fn run_browser_container(spec: &BrowserSpec, limits: &BrowserResourceLimits) -> Result<()> {
     let mut args = vec![
         "run".to_string(),
         "-d".to_string(),
@@ -362,6 +408,8 @@ fn run_browser_container(spec: &BrowserSpec) -> Result<()> {
         "2g".to_string(),
     ];
 
+    args.extend(browser_resource_limit_args(limits));
+
     if should_publish_ports(&spec.network_mode) {
         args.extend([
             "-p".to_string(),
@@ -639,6 +687,27 @@ mod tests {
         assert_eq!(effective_network_mode("bridge", None), "bridge");
     }
 
+    #[test]
+    fn browser_resource_limit_args_are_omitted_when_limits_are_empty() {
+        assert!(browser_resource_limit_args(&BrowserResourceLimits::none()).is_empty());
+    }
+
+    #[test]
+    fn browser_resource_limit_args_render_docker_run_limits() {
+        assert_eq!(
+            browser_resource_limit_args(&BrowserResourceLimits::from_values(
+                Some("0.5"),
+                Some("1g")
+            )),
+            vec![
+                "--cpus".to_string(),
+                "0.5".to_string(),
+                "--memory".to_string(),
+                "1g".to_string()
+            ]
+        );
+    }
+
     #[test]
     fn docker_host_autodetect_keeps_explicit_env_and_project_env_precedence() {
         assert_eq!(

src/lib.rs

@@ -70,6 +70,32 @@ pub struct BrowserSpec {
     pub ports: BrowserPorts,
 }
 
+#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize)]
+pub struct BrowserResourceLimits {
+    pub cpu_limit: Option<String>,
+    pub ram_limit: Option<String>,
+}
+
+impl BrowserResourceLimits {
+    pub fn none() -> Self {
+        Self::default()
+    }
+
+    pub fn from_values(cpu_limit: Option<&str>, ram_limit: Option<&str>) -> Self {
+        Self {
+            cpu_limit: normalize_optional_limit(cpu_limit),
+            ram_limit: normalize_optional_limit(ram_limit),
+        }
+    }
+
+    pub fn with_cli_overrides(&self, cpu_limit: Option<&str>, ram_limit: Option<&str>) -> Self {
+        Self {
+            cpu_limit: normalize_optional_limit(cpu_limit).or_else(|| self.cpu_limit.clone()),
+            ram_limit: normalize_optional_limit(ram_limit).or_else(|| self.ram_limit.clone()),
+        }
+    }
+}
+
 #[derive(Debug, Clone, Serialize)]
 pub struct BrowserInfo {
     pub project_id: String,
@@ -78,6 +104,13 @@ pub struct BrowserInfo {
     pub cdp_url: String,
 }
 
+#[derive(Debug, Clone, Serialize)]
+pub struct BrowserStopInfo {
+    pub project_id: String,
+    pub container_name: String,
+    pub removed: bool,
+}
+
 fn resolved_or<F>(resolve: &F, name: &str, fallback: String) -> String
 where
     F: Fn(&str) -> Option<String>,
@@ -88,6 +121,13 @@ where
         .unwrap_or(fallback)
 }
 
+fn normalize_optional_limit(value: Option<&str>) -> Option<String> {
+    value
+        .map(str::trim)
+        .filter(|value| !value.is_empty())
+        .map(str::to_string)
+}
+
 fn env_value(name: &str) -> Option<String> {
     env::var(name).ok()
 }
@@ -190,6 +230,29 @@ pub fn browser_spec_from_env(project_id: &str, network: Option<&str>) -> Browser
     browser_spec_from_resolver(project_id, network, env_value)
 }
 
+// CHANGE: read browser container resource ceilings from docker-git's generated environment.
+// WHY: docker-git emits DOCKER_GIT_BROWSER_*_LIMIT for the Rust-owned browser container.
+// QUOTE(ТЗ): "When users set --playwright-cpu/--playwright-ram or rely on the defaults"
+// REF: issue-347-review
+// SOURCE: n/a
+// FORMAT THEOREM: env.limit != empty -> docker_run_args contains the corresponding Docker limit flag.
+// PURITY: CORE (except environment boundary read isolated in browser_resource_limits_from_env)
+// INVARIANT: empty env values do not create malformed Docker flags.
+// COMPLEXITY: O(n)/O(n), n = total limit string length.
+fn browser_resource_limits_from_resolver<F>(resolve: F) -> BrowserResourceLimits
+where
+    F: Fn(&str) -> Option<String>,
+{
+    BrowserResourceLimits::from_values(
+        resolve("DOCKER_GIT_BROWSER_CPU_LIMIT").as_deref(),
+        resolve("DOCKER_GIT_BROWSER_RAM_LIMIT").as_deref(),
+    )
+}
+
+pub fn browser_resource_limits_from_env() -> BrowserResourceLimits {
+    browser_resource_limits_from_resolver(env_value)
+}
+
 pub fn render_novnc_url() -> String {
     format!(
         "http://127.0.0.1:{}/vnc.html?autoconnect=true&resize=remote&path=websockify",
@@ -228,8 +291,18 @@ impl BrowserConnection {
     }
 
     pub fn start_browser(&self, project_id: &str, network: Option<&str>) -> Result<BrowserInfo> {
+        let limits = browser_resource_limits_from_env();
+        self.start_browser_with_limits(project_id, network, &limits)
+    }
+
+    pub fn start_browser_with_limits(
+        &self,
+        project_id: &str,
+        network: Option<&str>,
+        limits: &BrowserResourceLimits,
+    ) -> Result<BrowserInfo> {
         let spec = browser_spec_from_env(project_id, network);
-        let runtime = self.shell.ensure_browser_container(&spec)?;
+        let runtime = self.shell.ensure_browser_container(&spec, limits)?;
 
         Ok(BrowserInfo {
             project_id: spec.project_id,
@@ -239,6 +312,17 @@ impl BrowserConnection {
         })
     }
 
+    pub fn stop_browser(&self, project_id: &str) -> Result<BrowserStopInfo> {
+        let spec = browser_spec_from_env(project_id, None);
+        let runtime = self.shell.stop_browser_container(&spec)?;
+
+        Ok(BrowserStopInfo {
+            project_id: spec.project_id,
+            container_name: runtime.container_name,
+            removed: runtime.removed,
+        })
+    }
+
     pub fn get_novnc_url(&self, project_id: &str) -> String {
         render_novnc_url_for_ports(compute_browser_ports(project_id))
     }
@@ -282,4 +366,25 @@ mod tests {
         assert_eq!(spec.network_mode, "container:dg-docker-git-issue-347");
         assert_eq!(spec.ports, compute_browser_ports("dg-docker-git-issue-347"));
     }
+
+    #[test]
+    fn browser_resource_limits_from_resolver_trims_empty_values() {
+        let limits = browser_resource_limits_from_resolver(|name| match name {
+            "DOCKER_GIT_BROWSER_CPU_LIMIT" => Some(" 0.5 ".to_string()),
+            "DOCKER_GIT_BROWSER_RAM_LIMIT" => Some("   ".to_string()),
+            _ => None,
+        });
+
+        assert_eq!(limits.cpu_limit, Some("0.5".to_string()));
+        assert_eq!(limits.ram_limit, None);
+    }
+
+    #[test]
+    fn browser_resource_limits_cli_overrides_env_values() {
+        let env_limits = BrowserResourceLimits::from_values(Some("1.5"), Some("2g"));
+        let limits = env_limits.with_cli_overrides(Some("0.5"), None);
+
+        assert_eq!(limits.cpu_limit, Some("0.5".to_string()));
+        assert_eq!(limits.ram_limit, Some("2g".to_string()));
+    }
 }

src/main.rs

@@ -7,7 +7,9 @@
 //! to guarantee a single unified browser (noVNC + CDP).
 
 use clap::{Parser, Subcommand};
-use docker_git_browser_connection::{BrowserConnection, BrowserInfo};
+use docker_git_browser_connection::{
+    browser_resource_limits_from_env, BrowserConnection, BrowserInfo,
+};
 
 #[derive(Parser)]
 #[command(
@@ -29,6 +31,17 @@ enum Commands {
         /// Docker network mode for the browser container. Defaults to container:<project container>.
         #[arg(long)]
         network: Option<String>,
+        /// Docker --cpus limit for the browser container. Defaults to DOCKER_GIT_BROWSER_CPU_LIMIT.
+        #[arg(long)]
+        cpu_limit: Option<String>,
+        /// Docker --memory limit for the browser container. Defaults to DOCKER_GIT_BROWSER_RAM_LIMIT.
+        #[arg(long)]
+        ram_limit: Option<String>,
+    },
+    /// Stop and remove the single browser container for a project
+    Stop {
+        #[arg(long)]
+        project: String,
     },
     /// Show status / URLs for the project's browser
     Status {
@@ -44,15 +57,31 @@ fn main() -> anyhow::Result<()> {
     let conn = BrowserConnection::new()?;
 
     match cli.command {
-        Commands::Start { project, network } => {
+        Commands::Start {
+            project,
+            network,
+            cpu_limit,
+            ram_limit,
+        } => {
             let net_ref = network.as_deref();
-            let info: BrowserInfo = conn.start_browser(&project, net_ref)?;
+            let limits = browser_resource_limits_from_env()
+                .with_cli_overrides(cpu_limit.as_deref(), ram_limit.as_deref());
+            let info: BrowserInfo = conn.start_browser_with_limits(&project, net_ref, &limits)?;
             println!("Browser started for project: {}", info.project_id);
             println!("Container: {}", info.container_name);
             println!("noVNC: {}", info.novnc_url);
             println!("CDP (for MCP Playwright / Hermes): {}", info.cdp_url);
             println!("Use the CDP URL in your MCP Playwright config to get automatic noVNC.");
         }
+        Commands::Stop { project } => {
+            let info = conn.stop_browser(&project)?;
+            if info.removed {
+                println!("Browser stopped for project: {}", info.project_id);
+            } else {
+                println!("Browser already absent for project: {}", info.project_id);
+            }
+            println!("Container: {}", info.container_name);
+        }
         Commands::Status { project } => {
             let novnc = conn.get_novnc_url(&project);
             let cdp = conn.get_cdp_url(&project);

tests/cli_status.rs

@@ -18,3 +18,28 @@ fn status_command_prints_single_browser_urls_without_docker() {
     assert!(stdout.contains(&format!("CDP: {}", render_cdp_url_for_ports(ports))));
     assert!(stdout.contains("Invariant check: true"));
 }
+
+#[test]
+fn start_help_exposes_resource_limit_flags_without_docker() {
+    let output = Command::new(env!("CARGO_BIN_EXE_docker-git-browser-connection"))
+        .args(["start", "--help"])
+        .output()
+        .expect("Failed to execute binary");
+
+    assert!(output.status.success());
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    assert!(stdout.contains("--cpu-limit"));
+    assert!(stdout.contains("--ram-limit"));
+}
+
+#[test]
+fn root_help_exposes_stop_command_without_docker() {
+    let output = Command::new(env!("CARGO_BIN_EXE_docker-git-browser-connection"))
+        .args(["--help"])
+        .output()
+        .expect("Failed to execute binary");
+
+    assert!(output.status.success());
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    assert!(stdout.contains("stop"));
+}