diff --git a/README.md b/README.md index 61359a2..c5fc3b2 100644 --- a/README.md +++ b/README.md @@ -505,7 +505,7 @@ Triple-C includes optional speech-to-text powered by [Faster Whisper](https://gi | `app/src/components/projects/home/OpenPageDialog.tsx` | Open a URL in the container's browser at a chosen viewport | | `app/src/components/projects/home/ContainerMigrationBanner.tsx` | Base-image staleness banner, migration progress, resume/rollback | | `app/src/components/projects/home/CapabilityTiles.tsx` | Read-only skills/agents/commands/hooks/plugins/MCP counts | -| `app/src/components/projects/ClaudeCodeSettingsEditor.tsx` | Claude Code CLI settings (TUI mode, effort, focus, caching) | +| `app/src/components/projects/ClaudeCodeSettingsEditor.tsx` | Claude Code CLI settings → `tui`, `effortLevel`, `viewMode`, `autoScrollEnabled`, `showThinkingSummaries`, `awaySummaryEnabled`, plus the env-var flags (scrub, 1h caching). Every managed key is re-emitted on each start, `null` meaning "delete". | ### Frontend — settings, terminal and hooks diff --git a/ROADMAP.md b/ROADMAP.md index 3b1a8c2..50e8832 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -26,20 +26,35 @@ scheduler, and the fleet view across many projects. ## Current coverage (v0.3.0) -Triple-C sets exactly five `settings.json` keys, plus a sandbox block: +Triple-C sets exactly six `settings.json` keys, plus a sandbox block: | Key | Surfaced as | |---|---| -| `tui` | TUI Mode select (`fullscreen`) | -| `effort` | Effort Level select (`low`/`medium`/`high`) | -| `autoScrollEnabled` | Auto-Scroll Disabled toggle | -| `focusMode` | Focus Mode toggle | -| `showThinkingSummaries` | Thinking Summaries toggle | +| `tui` | TUI mode select — unset (Claude Code chooses), `default` (classic renderer), `fullscreen` (flicker-free alt-screen). Three distinct states, not two. | +| `effortLevel` | Effort level select (`low`/`medium`/`high`/`xhigh`) | +| `viewMode` | Focus mode toggle, written as `"focus"`. Unset means the user's own `verbose` setting and sticky `/focus` choice still apply. | +| `autoScrollEnabled` | Auto-scroll toggle. Claude Code's default is `true`, so it is the *off* state that writes `false`. | +| `showThinkingSummaries` | Thinking summaries toggle (Claude Code default `false`) | +| `awaySummaryEnabled` | Session recap toggle. Claude Code's recap is **on** by default, so again it is the off state that writes `false`. | | `sandbox.*` | Sandbox toggle (`enabled`, `enableWeakerNestedSandbox`, `allowUnsandboxedCommands`) | +Every one of those keys is emitted on **every** start, with a JSON `null` standing for +"delete this key". `~/.claude/settings.json` sits on the config volume and the entrypoint +merges into it, so a key merely omitted when its control goes off left the previous +on-value in place forever. + Plus four env feature flags — `CLAUDE_CODE_NO_FLICKER`, `CLAUDE_CODE_ENABLE_AWAY_SUMMARY`, `CLAUDE_CODE_SUBPROCESS_ENV_SCRUB`, `ENABLE_PROMPT_CACHING_1H` — and arbitrary user-set -`CLAUDE_CODE_*` vars via the Env Vars modal. +`CLAUDE_CODE_*` vars via the Env Vars modal. The four are written on every container +create *including* their off value, because `docker commit` bakes a container's env into +the snapshot image: a value written once would otherwise ride that snapshot into every +future container. That also makes them Triple-C's to own, so all four are reserved names +— hand-setting one in the Env Vars modal is skipped with a warning, the same as any other +`triple-c.*`-managed variable. `CLAUDE_CODE_ENABLE_AWAY_SUMMARY` is what actually enforces +the recap choice — it takes precedence over `awaySummaryEnabled` *and* over the +in-container `/config` toggle, so turning the control off sends `0` while leaving it on +sends an empty value rather than `1`: Triple-C's default must not overrule a `/config` +choice it never asked about. Also covered: per-project auth backends (Anthropic OAuth, Bedrock incl. SSO refresh, Ollama, OpenAI-compatible), user-level `CLAUDE.md` composition, `claude update` on every diff --git a/app/src-tauri/src/commands/inspect_commands.rs b/app/src-tauri/src/commands/inspect_commands.rs index 68749f3..4bd4cf9 100644 --- a/app/src-tauri/src/commands/inspect_commands.rs +++ b/app/src-tauri/src/commands/inspect_commands.rs @@ -1200,9 +1200,14 @@ pub async fn add_scheduled_task( /// * **In that order**, so a rejected `add` leaves the original untouched /// rather than deleting a prompt the user cannot get back. The cost is a /// sub-second window in which both tasks are in the crontab. -/// * The task therefore gets a **new id**. Its old log directory -/// (`~/.claude/scheduler/logs//`) stays behind under the old id; the -/// UI warns about this before saving. +/// * The task therefore gets a **new id**, and its old log directory +/// (`~/.claude/scheduler/logs//`) goes with the removal — the +/// scheduler reaps a task's logs when the task stops existing, because +/// nothing can name that id again afterwards. The UI warns before saving. +/// (A project still running an older base image carries the older +/// `/usr/local/bin/triple-c-scheduler`, which left the directory behind; +/// `/usr/local/bin` only changes on a base-image migration or a Reset. The +/// copy is deliberately written for the case that loses data.) /// * `enabled` is carried over explicitly, because `add` always creates an /// enabled task and silently re-enabling a task the user had switched off /// would schedule a run they did not ask for. diff --git a/app/src-tauri/src/commands/migration_commands.rs b/app/src-tauri/src/commands/migration_commands.rs index f43e348..907b0ed 100644 --- a/app/src-tauri/src/commands/migration_commands.rs +++ b/app/src-tauri/src/commands/migration_commands.rs @@ -863,7 +863,7 @@ pub async fn confirm_migration( // waiting for the project's next recreation would leave it lying around // indefinitely. tauri::async_runtime::spawn(async { - crate::docker::sweep_orphaned_snapshots().await; + crate::docker::sweep_orphaned_snapshots_logged("after migration confirmed").await; }); Ok(()) @@ -957,6 +957,16 @@ pub async fn rollback_migration( let _ = mig::untag_image(&rollback_ref).await; migration_store::clear_staging(&project_id)?; migration_store::clear(&project_id)?; + + // Retagging above moved `:latest` off the *migrated* snapshot, and the + // container that was built from it was removed a few lines up — so a + // multi-gigabyte image is sitting there untagged and unreferenced with + // nothing else in the app that would ever look at it again. The confirm + // path sweeps for exactly this reason; rolling back orphans just as much + // and did not. + tauri::async_runtime::spawn(async { + crate::docker::sweep_orphaned_snapshots_logged("after migration rollback").await; + }); emit_progress( &app_handle, &project_id, diff --git a/app/src-tauri/src/commands/project_commands.rs b/app/src-tauri/src/commands/project_commands.rs index a158925..09030ef 100644 --- a/app/src-tauri/src/commands/project_commands.rs +++ b/app/src-tauri/src/commands/project_commands.rs @@ -459,7 +459,7 @@ pub async fn start_project_container( // just this one, so recreations that happened before the sweep // existed are cleaned up too. tauri::async_runtime::spawn(async { - docker::sweep_orphaned_snapshots().await; + docker::sweep_orphaned_snapshots_logged("after recreation").await; }); new_id diff --git a/app/src-tauri/src/docker/container.rs b/app/src-tauri/src/docker/container.rs index f4ca0c0..0799b44 100644 --- a/app/src-tauri/src/docker/container.rs +++ b/app/src-tauri/src/docker/container.rs @@ -218,6 +218,12 @@ pub const SECRET_ENV_KEYS: &[&str] = &[ /// which is what keeps the sweep away from the user's own images. const LABEL_MANAGED: &str = "triple-c.managed"; +/// Marks the image built from `container/Dockerfile` itself, as opposed to a +/// project snapshot committed from a container. Only ever `"true"` on a base +/// image; `create_container` writes it explicitly empty so an inherited value +/// cannot travel onto a snapshot. See the `LABEL` block in the Dockerfile. +const LABEL_BASE: &str = "triple-c.base"; + const RESERVED_ENV_PREFIXES: &[&str] = &["ANTHROPIC_", "AWS_", "GIT_", "HOST_", "TRIPLE_C_"]; /// Exact env var names Triple-C manages itself. Not covered by @@ -235,6 +241,15 @@ const RESERVED_ENV_EXACT: &[&str] = &[ "MISSION_CONTROL_ENABLED", "VPN_SUPPORT_ENABLED", "TRIPLE_C_PERMISSION_MODE", + // The four env vars the Claude Code settings editor drives. Reserved for + // the `VPN_SUPPORT_ENABLED` reason: each is now written on every create, + // including its off value, so a hand-set custom var of the same name would + // either be overridden without explanation or override the setting behind + // the UI's back, depending on which one Docker kept. + "CLAUDE_CODE_NO_FLICKER", + "CLAUDE_CODE_ENABLE_AWAY_SUMMARY", + "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB", + "ENABLE_PROMPT_CACHING_1H", CLAUDE_OAUTH_TOKEN_ENV, // The model-alias vars are already covered by the `ANTHROPIC_` prefix // above; they are listed explicitly so that a future narrowing of the @@ -633,7 +648,7 @@ fn merge_claude_code_settings( auto_scroll_disabled: if p.auto_scroll_disabled { true } else { g.auto_scroll_disabled }, focus_mode: if p.focus_mode { true } else { g.focus_mode }, show_thinking_summaries: if p.show_thinking_summaries { true } else { g.show_thinking_summaries }, - enable_session_recap: if p.enable_session_recap { true } else { g.enable_session_recap }, + session_recap_disabled: if p.session_recap_disabled { true } else { g.session_recap_disabled }, env_scrub: if p.env_scrub { true } else { g.env_scrub }, prompt_caching_1h: if p.prompt_caching_1h { true } else { g.prompt_caching_1h }, }) @@ -660,7 +675,7 @@ fn compute_claude_code_settings_fingerprint( format!("{}", s.auto_scroll_disabled), format!("{}", s.focus_mode), format!("{}", s.show_thinking_summaries), - format!("{}", s.enable_session_recap), + format!("{}", s.session_recap_disabled), format!("{}", s.env_scrub), format!("{}", s.prompt_caching_1h), ]; @@ -674,34 +689,164 @@ fn compute_claude_code_settings_fingerprint( } } -/// Build the settings.json content for Claude Code. -/// Returns a JSON string of the settings to be written to ~/.claude/settings.json. -/// Always emits a `sandbox.enabled` key reflecting the current per-project -/// toggle so that flipping it off in triple-c overrides any prior on-state -/// stored in the persisted settings.json (which lives in a named volume). +/// The four Claude Code env vars the settings editor drives, as `KEY=VALUE`. +/// +/// **All four are emitted on every create, including their off value.** This is +/// the `MANAGED_AUTH_KEYS` rule: `docker commit` bakes a container's env into +/// the snapshot image, and the next container inherits anything the create does +/// not override. A `=1` written once would ride that snapshot into every future +/// container and make the switch impossible to turn back off — the same +/// stickiness [`build_claude_code_settings_json`] fixes on the settings.json +/// side, in a place where it is even less visible. +/// +/// Two of them use an **empty** value for "off", and the distinction matters: +/// +/// * `CLAUDE_CODE_NO_FLICKER` documents `1` as fullscreen-on and `0` as +/// fullscreen-*off*, and it overrides the `tui` setting. `0` is therefore not +/// neutral — it would silently pin every project that has expressed no +/// preference to the classic renderer, when an unset `tui` is supposed to let +/// Claude Code choose. Empty is neither value, so it reads as unset while +/// still overriding a baked `1`. +/// * `CLAUDE_CODE_ENABLE_AWAY_SUMMARY` outranks both `awaySummaryEnabled` and +/// the in-container `/config` toggle. `0` is exactly right for "the user +/// turned the recap off in Triple-C", but a blanket `1` for the default state +/// would force the recap back on for someone who had turned it off with +/// `/config` inside their own container. Triple-C's default must not overrule +/// a choice it never asked about. +/// +/// The other two are documented as "set to `1` to …" with no meaning attached +/// to `0`, so `0` is unambiguously neutral and is stated outright. +fn claude_code_env_vars(settings: Option<&ClaudeCodeSettings>) -> Vec { + let owned; + let s = match settings { + Some(s) => s, + None => { + owned = ClaudeCodeSettings::default(); + &owned + } + }; + + vec![ + format!( + "CLAUDE_CODE_NO_FLICKER={}", + match s.tui_mode.as_deref() { + Some("fullscreen") => "1", + Some("default") => "0", + _ => "", + } + ), + format!( + "CLAUDE_CODE_ENABLE_AWAY_SUMMARY={}", + if s.session_recap_disabled { "0" } else { "" } + ), + format!( + "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB={}", + if s.env_scrub { "1" } else { "0" } + ), + format!( + "ENABLE_PROMPT_CACHING_1H={}", + if s.prompt_caching_1h { "1" } else { "0" } + ), + ] +} + +/// Build the settings.json payload for Claude Code, handed to the container as +/// `CLAUDE_CODE_SETTINGS_JSON` and applied by `entrypoint.sh`. +/// +/// ## Every managed key is always present, and `null` means "delete" +/// +/// The settings file lives on `triple-c-claude-config-{projectId}`, a named +/// volume that outlives the container, and the entrypoint *merges* into it. So +/// a key emitted only when it is non-default can be written once and never +/// taken back: turning the setting off simply omits the key, the merge +/// preserves whatever was there, and the setting stays on forever. Only a +/// destructive Reset — which also deletes the OAuth login, skills and +/// transcripts — ever cleared it. Four of the five keys here were sticky that +/// way; the `sandbox` block already carried the workaround and the comment +/// explaining it, and this is the same treatment applied to the rest. +/// +/// Two shapes of "off" are needed, because Claude Code's own defaults differ: +/// +/// * **A boolean with a documented default** (`autoScrollEnabled` is `true`, +/// `showThinkingSummaries` is `false`) is emitted with that neutral value. +/// * **A key whose neutral state is *unset*** (`tui`, `effortLevel`, +/// `viewMode`, `awaySummaryEnabled`) is emitted as JSON `null`, and the +/// entrypoint deletes rather than merges those. Writing a stand-in value +/// would not be neutral: an unset `tui` lets Claude Code choose the renderer +/// (`"default"` pins the classic one), and an unset `viewMode` lets the +/// user's own sticky `/focus` choice and `verbose` setting apply +/// (`"default"` overrides both). +/// +/// Returns a `String` rather than an `Option`: there is no longer any +/// input for which this produces nothing to say. fn build_claude_code_settings_json( settings: Option<&ClaudeCodeSettings>, sandbox_enabled: bool, -) -> Option { +) -> String { + let owned; + let s = match settings { + Some(s) => s, + // No struct at all is not "say nothing" — it is "every setting is at + // its default", which still has to be asserted over a stale file. + None => { + owned = ClaudeCodeSettings::default(); + &owned + } + }; + let mut map = serde_json::Map::new(); - if let Some(s) = settings { - if let Some(ref tui) = s.tui_mode { - map.insert("tui".to_string(), serde_json::json!(tui)); - } - if let Some(ref effort) = s.effort { - map.insert("effort".to_string(), serde_json::json!(effort)); - } - if s.auto_scroll_disabled { - map.insert("autoScrollEnabled".to_string(), serde_json::json!(false)); - } + // `null` clears; see the module doc above. + map.insert( + "tui".to_string(), + match s.tui_mode { + Some(ref tui) => serde_json::json!(tui), + None => serde_json::Value::Null, + }, + ); + // `effortLevel`, not `effort`. Claude Code has never read a key called + // `effort`, so the previous value was written and silently ignored. + map.insert( + "effortLevel".to_string(), + match s.effort { + Some(ref effort) => serde_json::json!(effort), + None => serde_json::Value::Null, + }, + ); + // Documented default `true`, so the neutral value is a value. + map.insert( + "autoScrollEnabled".to_string(), + serde_json::json!(!s.auto_scroll_disabled), + ); + // Documented default `false`. + map.insert( + "showThinkingSummaries".to_string(), + serde_json::json!(s.show_thinking_summaries), + ); + // `viewMode: "focus"` is the real setting behind what the UI calls focus + // mode — "collapses tool output to one-line summaries" is that key's + // documented behaviour. The `focusMode` key it replaces was invented and + // did nothing. + map.insert( + "viewMode".to_string(), if s.focus_mode { - map.insert("focusMode".to_string(), serde_json::json!(true)); - } - if s.show_thinking_summaries { - map.insert("showThinkingSummaries".to_string(), serde_json::json!(true)); - } - } + serde_json::json!("focus") + } else { + serde_json::Value::Null + }, + ); + // The recap is on by default, so only the *off* case has anything to write. + // `CLAUDE_CODE_ENABLE_AWAY_SUMMARY` (set unconditionally at creation) takes + // precedence over this key and is what actually enforces the choice; this + // is here so the container's settings.json does not contradict it. + map.insert( + "awaySummaryEnabled".to_string(), + if s.session_recap_disabled { + serde_json::json!(false) + } else { + serde_json::Value::Null + }, + ); // Always emit `sandbox.enabled` so that toggling the per-project sandbox // off in triple-c clears any prior on-state in the persisted @@ -719,11 +864,7 @@ fn build_claude_code_settings_json( }; map.insert("sandbox".to_string(), sandbox_obj); - if map.is_empty() { - None - } else { - Some(serde_json::Value::Object(map).to_string()) - } + serde_json::Value::Object(map).to_string() } pub async fn find_existing_container(project: &Project) -> Result, String> { @@ -1320,31 +1461,20 @@ pub async fn create_container( global_claude_code_settings, project.claude_code_settings.as_ref(), ); - if let Some(ref cc) = merged_cc_settings { - // Env-var-based settings (these are read directly by Claude Code) - if cc.tui_mode.as_deref() == Some("fullscreen") { - env_vars.push("CLAUDE_CODE_NO_FLICKER=1".to_string()); - } - if cc.enable_session_recap { - env_vars.push("CLAUDE_CODE_ENABLE_AWAY_SUMMARY=1".to_string()); - } - if cc.env_scrub { - env_vars.push("CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1".to_string()); - } - if cc.prompt_caching_1h { - env_vars.push("ENABLE_PROMPT_CACHING_1H=1".to_string()); - } - } + // Env-var-based settings, read directly by Claude Code. Extracted and unit + // tested for the `vpn_host_config` reason: a container is created once, by + // a very long function, and a variable emitted with the wrong value here is + // invisible until someone wonders why a switch does nothing. + env_vars.extend(claude_code_env_vars(merged_cc_settings.as_ref())); - // settings.json-based settings (written by the entrypoint). - // Always invoked so per-project sandbox state is injected even when no - // ClaudeCodeSettings struct is present. - if let Some(settings_json) = build_claude_code_settings_json( - merged_cc_settings.as_ref(), - project.sandbox_mode_enabled, - ) { - env_vars.push(format!("CLAUDE_CODE_SETTINGS_JSON={}", settings_json)); - } + // settings.json-based settings (applied by the entrypoint). Always emitted, + // even with no `ClaudeCodeSettings` struct present: the payload asserts the + // *whole* managed key set, so "no settings" still has to be stated over a + // settings.json left behind on the config volume by a previous config. + env_vars.push(format!( + "CLAUDE_CODE_SETTINGS_JSON={}", + build_claude_code_settings_json(merged_cc_settings.as_ref(), project.sandbox_mode_enabled) + )); let mut mounts: Vec = Vec::new(); @@ -1571,6 +1701,14 @@ pub async fn create_container( // always meant to do. labels.insert("triple-c.mcp-fingerprint".to_string(), String::new()); + // Same defence, for the label `container/Dockerfile` now stamps on the base + // image. Docker merges an image's labels into the container it creates, and + // `docker commit` copies the container's labels onto the snapshot — so + // without this line every project snapshot would inherit + // `triple-c.base=true` from the base it descends from and claim to *be* a + // base image. Writing it explicitly empty overrides the inherited value. + labels.insert(LABEL_BASE.to_string(), String::new()); + for (key, value) in extras.extra_labels { labels.insert((*key).to_string(), (*value).to_string()); } @@ -1581,6 +1719,7 @@ pub async fn create_container( mounts: Some(mounts), port_bindings: if port_bindings.is_empty() { None } else { Some(port_bindings) }, init: Some(true), + log_config: Some(capped_log_config()), cap_add, devices, sysctls, @@ -1618,6 +1757,36 @@ pub async fn create_container( Ok(response.id) } +/// The rotation policy every Triple-C container is created with. +/// +/// Without this a container inherits the daemon's `json-file` default, which +/// has **no size limit at all** — `docker logs` for a container that has been +/// up for weeks is a single file that grows until the disk does not have room +/// for it. Triple-C containers are long-lived by design (stop/start, not +/// create/destroy), and the entrypoint plus anything a session leaves running +/// on stdout all land in that one file. +/// +/// 10 MiB × 3 keeps roughly the last 30 MiB, which is far more scrollback than +/// anything reads, and bounds the worst case at 30 MiB per project instead of +/// unbounded. +/// +/// **Deliberately not part of `container_needs_recreation`.** That check is +/// label-based, so participating would mean a new `triple-c.*` label whose only +/// effect is to recreate every existing project once — and a recreation costs a +/// `docker commit`, i.e. a permanent multi-gigabyte layer, which is the very +/// thing this whole change set exists to avoid. Containers pick the policy up +/// on their next natural recreation instead; an existing container keeps its +/// unbounded log until then, which is exactly the status quo. +fn capped_log_config() -> bollard::models::HostConfigLogConfig { + bollard::models::HostConfigLogConfig { + typ: Some("json-file".to_string()), + config: Some(HashMap::from([ + ("max-size".to_string(), "10m".to_string()), + ("max-file".to_string(), "3".to_string()), + ])), + } +} + pub async fn start_container(container_id: &str) -> Result<(), String> { let docker = get_docker()?; docker @@ -1768,6 +1937,155 @@ chmod 600 "$HOME/.aws/credentials""#; Ok(()) } +/// Paths deleted from a container's writable layer immediately before +/// [`commit_container_snapshot`] runs. +/// +/// ## Why this exists +/// +/// Every recreation commits the container, and a commit **stacks a new layer** +/// on top of the previous snapshot — it never rewrites one. Deleting a file +/// after it has been committed does not give the bytes back; it writes a +/// whiteout entry and the original bytes stay in the layer below, forever. One +/// project was measured carrying 14 stacked commit layers and ~5.1 GB above its +/// base image, and 24 different conditions trigger a recreation, so changing a +/// single settings field costs a multi-gigabyte layer that nothing can reclaim. +/// +/// The only moment the bytes are still free to drop is *before* the commit that +/// captures them. Measured on one container's 4.48 GB pending writable layer: +/// 3.0 GB of agent scratchpad under `/tmp/claude-*`, the drag-and-drop staging +/// area (up to 256 MiB per dropped file, and nothing in the app ever removes +/// one), one PNG per pasted image, and the apt lists/cache/logs left by every +/// runtime `apt-get install` the browser-view installer and the Playwright +/// healer run — none of which has an `apt-get clean` behind it. +/// +/// ## Why a hardcoded list and not a heuristic +/// +/// A snapshot is the user's system layer: their packages, their `/opt`, their +/// `/var/lib/postgresql`. Nothing here may guess. Every entry is an absolute +/// path anchored to a directory Triple-C or a package manager owns, and the +/// three globs are anchored to `/tmp` specifically: +/// +/// * `/workspace/{mount_name}` subtrees are **host bind mounts** — the user's +/// real project directories. No entry may ever reach one, which is why no +/// pattern here starts with `/workspace`. +/// * The only bind mounts under `/tmp` are `/tmp/.host-ca` and `/tmp/.host-aws` +/// (both read-only). A leading-dot name is not matched by a shell glob, and +/// none of the three patterns share their prefix, so neither can be selected +/// even by accident. +/// * The apt entries keep their parent directory and remove only its contents +/// (`lists/*`, `archives/*.deb`, `apt/*`); `apt-get` is unhappy when the +/// directories themselves are missing. +/// +/// A unit test pins the list, because the blast radius of a wrong entry here is +/// a user's data and the code that consumes it is a shell string. +pub(crate) const SNAPSHOT_SCRUB_PATHS: &[&str] = &[ + // Agent scratchpads. The user's global CLAUDE.md instructs every agent to + // put temporary files under a scratchpad directory in /tmp, so this is + // where a long-running project's writable layer actually goes. + "/tmp/claude-*", + // Files drag-dropped into a terminal, staged by + // `commands/terminal_commands.rs` at up to 256 MiB each. Nothing in the + // repo deletes them. + "/tmp/triple-c-drops/*", + // One PNG per pasted image, from the same module. Also never deleted. + "/tmp/clipboard_*.png", + // Runtime apt debris. `browser_view/install.rs` and + // `container/triple-c-playwright-heal` both run `apt-get install` inside a + // live container without an `apt-get clean` after it. + "/var/lib/apt/lists/*", + "/var/cache/apt/archives/*.deb", + "/var/log/apt/*", + "/var/log/dpkg.log", +]; + +/// Marker the scrub script prints so the byte total can be read back out of the +/// exec's interleaved stdout/stderr. +const SCRUB_MARKER: &str = "###TRIPLE-C-SCRUBBED "; + +/// The `/bin/sh` program run inside the container to perform the scrub. +/// +/// Built here rather than inline so a test can read it. The path list is +/// interpolated **unquoted** on the `for` line, which is the whole point: the +/// shell expands the three globs there. An unmatched glob expands to itself, +/// the `[ -e ]` guard then fails, and the entry is skipped — so a pattern that +/// matches nothing is a no-op rather than an `rm` of a literal path. +/// Inside the loop `$p` is quoted, so a filename containing whitespace is one +/// argument. +fn snapshot_scrub_script() -> String { + format!( + r#"total=0 +for p in {paths}; do + [ -e "$p" ] || continue + sz=$(du -sb "$p" 2>/dev/null | cut -f1) + case "$sz" in ''|*[!0-9]*) sz=0 ;; esac + rm -rf -- "$p" 2>/dev/null && total=$((total + sz)) +done +echo "{marker}$total" +exit 0 +"#, + paths = SNAPSHOT_SCRUB_PATHS.join(" "), + marker = SCRUB_MARKER, + ) +} + +/// Parse the byte total the scrub script reports. Returns `None` when the +/// marker is absent, which is how a container that never ran the script (or a +/// `sh` that died early) is told apart from one that reclaimed nothing. +fn parse_scrub_total(output: &str) -> Option { + output + .lines() + .rev() + .find_map(|line| line.trim().strip_prefix(SCRUB_MARKER)?.trim().parse().ok()) +} + +/// Delete the throwaway files listed in [`SNAPSHOT_SCRUB_PATHS`] from a +/// container's writable layer so the commit that follows does not bake them in. +/// +/// Runs as **root**: the apt debris is root-owned while the scratchpads belong +/// to `claude`, and root can remove both. +/// +/// **Never fails the caller, by design.** A scrub is an optimisation; a commit +/// is the only copy of the user's system layer. Losing some disk is a strictly +/// better outcome than refusing to snapshot, so every failure here is a log +/// line and nothing more. Note that one caller (the pre-swap commit in +/// `migrate_project_to_base`) has already *stopped* the container, so `docker +/// exec` legitimately fails there — that path simply commits unscrubbed. +pub async fn scrub_writable_layer(container_id: &str) -> u64 { + let script = snapshot_scrub_script(); + let cmd = vec!["/bin/sh".to_string(), "-c".to_string(), script]; + + match crate::docker::exec::exec_oneshot_as(container_id, "root", cmd, Vec::new()).await { + Ok((output, _exit_code)) => match parse_scrub_total(&output) { + Some(bytes) => { + if bytes > 0 { + log::info!( + "Pre-commit scrub of container {} reclaimed {:.2} MB before it could be committed", + container_id, + bytes as f64 / 1_048_576.0 + ); + } + bytes + } + None => { + log::warn!( + "Pre-commit scrub of container {} did not report a total; committing anyway. Output: {}", + container_id, + output.trim() + ); + 0 + } + }, + Err(e) => { + log::warn!( + "Pre-commit scrub of container {} could not run ({}); committing anyway", + container_id, + e + ); + 0 + } + } +} + /// Commit the container's filesystem to a snapshot image so that system-level /// changes (apt/pip/npm installs, ~/.claude.json, etc.) survive container /// removal. @@ -1795,10 +2113,21 @@ chmod 600 "$HOME/.aws/credentials""#; /// /// Non-secret env (PATH, TZ, model aliases, instructions) is inherited as /// before, so nothing about the snapshot's behaviour changes. +/// +/// ## Why it scrubs first +/// +/// See [`SNAPSHOT_SCRUB_PATHS`]. Every commit stacks a layer, so a file present +/// here is a file the project's image carries for the rest of its life. pub async fn commit_container_snapshot(container_id: &str, project: &Project) -> Result<(), String> { let docker = get_docker()?; let image_name = get_snapshot_image_name(project); + // Drop the throwaway files *before* the commit captures them. A commit + // stacks a layer and never rewrites one, so anything present at this + // instant is paid for permanently — see [`SNAPSHOT_SCRUB_PATHS`]. Failure + // is swallowed inside; a scrub must never be able to block a snapshot. + scrub_writable_layer(container_id).await; + // Parse repo:tag let (repo, tag) = match image_name.rsplit_once(':') { Some((r, t)) => (r.to_string(), t.to_string()), @@ -1890,15 +2219,30 @@ fn orphan_sweep_filters() -> HashMap> { /// `triple-c-snapshot-{id}:latest` is what a project is rebuilt from, and a /// migration's `pre-migration-*` pin is the only copy of a rollback target. /// Neither can ever match this filter, so neither can be swept. -/// * **`triple-c.managed=true`** — only images Triple-C itself committed. -/// `docker commit` copies the container's labels onto the image, which is what -/// makes the label a reliable mark of provenance. The user's own dangling -/// images are none of our business. +/// * **`triple-c.managed=true`** — only images Triple-C itself built or +/// committed. `docker commit` copies the container's labels onto the image, +/// and `container/Dockerfile` stamps the same label on the base, which is +/// what makes it a reliable mark of provenance. The user's own dangling +/// images are none of our business — the daemon this runs against is shared +/// with their unrelated work, so an unfiltered prune is never an option. +/// +/// **Superseded base images are collected by exactly the same two conditions.** +/// They used to be unreachable: `container/Dockerfile` carried no `LABEL` at +/// all, so a base image left untagged when a newer build claimed +/// `triple-c-sandbox:latest` was dangling but not labelled and could never +/// match. ~11.9 GB was measured stranded that way. The Dockerfile now stamps +/// `triple-c.managed=true`, so no change is needed here beyond knowing that +/// this function is what reclaims them. /// /// Removal is not forced, so Docker refuses (409) while any container is still /// built from the image — including the stopped containers of projects that are /// not running. That refusal is the third safety net and it is the daemon's, /// not ours; those orphans are simply counted and left for a later sweep. +/// **This is why `force: false` stays.** Forcing would untag and delete an +/// image out from under a stopped project, and Docker would leave that +/// container unable to start. A superseded base image pinned by one stopped +/// container is therefore not reclaimed until that project is next recreated or +/// removed, which the startup sweep will notice on some later run. /// /// Never fails the caller: this is housekeeping, and a full disk is a better /// outcome than a project that will not start. @@ -1969,6 +2313,38 @@ pub async fn sweep_orphaned_snapshots() -> SnapshotSweepReport { report } +/// Run [`sweep_orphaned_snapshots`] and write the whole outcome to the log, +/// tagged with *why* the sweep ran. +/// +/// Every caller of the sweep is housekeeping fired off in a detached task, and +/// every one of them dropped the `SnapshotSweepReport` on the floor — including +/// `reclaimed_bytes`, `failed` and `unavailable`, which are the only evidence +/// that a sweep ever happened or that it could not. When a user asks where +/// 116 GB went, "nothing was logged" is not an answer. There is no UI for this +/// yet by deliberate choice (prevention first), so the log is the whole +/// interface. +pub async fn sweep_orphaned_snapshots_logged(context: &str) { + let report = sweep_orphaned_snapshots().await; + + if let Some(ref why) = report.unavailable { + log::warn!("Snapshot sweep ({}) could not run: {}", context, why); + return; + } + + log::info!( + "Snapshot sweep ({}): {} removed, {:.2} GB reclaimed, {} still pinned by a container, {} failed", + context, + report.removed.len(), + report.reclaimed_bytes as f64 / 1_073_741_824.0, + report.in_use, + report.failed.len(), + ); + + for (id, error) in &report.failed { + log::warn!("Snapshot sweep ({}) could not remove {}: {}", context, id, error); + } +} + /// Outcome of [`scrub_secrets_from_snapshots`], so callers can tell the user /// what actually happened rather than guessing. #[derive(Debug, Default, Clone, serde::Serialize)] @@ -3119,4 +3495,334 @@ mod tests { }; assert!(blind.left_something_behind()); } + + // ── Pre-commit scrub (A1) ──────────────────────────────────────────────── + + #[test] + fn no_scrub_path_can_reach_a_host_bind_mount() { + // `/workspace/{mount_name}` is the user's own project directory, bound + // in from the host. Nothing in this list may ever name one — and the + // two read-only host mounts under /tmp must be equally unreachable. + for path in SNAPSHOT_SCRUB_PATHS { + assert!( + path.starts_with('/'), + "{} is not absolute, so the shell would resolve it against an unknown cwd", + path + ); + assert!( + !path.starts_with("/workspace"), + "{} reaches into a host bind mount", + path + ); + assert!( + !path.starts_with("/tmp/."), + "{} could select /tmp/.host-ca or /tmp/.host-aws", + path + ); + assert!( + !path.starts_with("/home"), + "{} reaches into the persisted home volume", + path + ); + } + } + + #[test] + fn no_scrub_path_is_a_whole_system_directory() { + // A trailing `/*` on a directory the system needs is fine; the + // directory *itself* is not. Guards against an edit that shortens an + // entry by one path component. + const FORBIDDEN: &[&str] = &[ + "/", "/tmp", "/var", "/var/lib", "/var/log", "/var/cache", "/etc", "/usr", "/opt", + "/workspace", "/home", "/home/claude", "/var/lib/apt", "/var/cache/apt", + ]; + for path in SNAPSHOT_SCRUB_PATHS { + let trimmed = path.trim_end_matches('/'); + assert!( + !FORBIDDEN.contains(&trimmed), + "{} would delete a directory the container needs", + path + ); + } + } + + #[test] + fn the_scrub_list_covers_every_measured_source_of_writable_layer_growth() { + // Each of these was measured in a real container's pending commit. + // Dropping one silently gives back multiple gigabytes per project. + for expected in [ + "/tmp/claude-*", // agent scratchpads, 3.0 GB measured + "/tmp/triple-c-drops/*", // terminal drag-and-drop staging + "/tmp/clipboard_*.png", // pasted images + "/var/lib/apt/lists/*", // runtime apt, no `apt-get clean` behind it + "/var/cache/apt/archives/*.deb", + "/var/log/apt/*", + "/var/log/dpkg.log", + ] { + assert!( + SNAPSHOT_SCRUB_PATHS.contains(&expected), + "{} is no longer scrubbed before commit", + expected + ); + } + } + + #[test] + fn the_scrub_script_expands_globs_but_quotes_the_match() { + let script = snapshot_scrub_script(); + // Unquoted on the `for` line — that is what makes the shell expand the + // globs at all. + assert!(script.contains("for p in /tmp/claude-* /tmp/triple-c-drops/*")); + // Quoted everywhere it is *used*, so a filename with a space is one + // argument and not two paths. + assert!(script.contains(r#"[ -e "$p" ] || continue"#)); + assert!(script.contains(r#"rm -rf -- "$p""#)); + // `rm -rf /` would be catastrophic and is exactly what a botched + // interpolation produces. + assert!(!script.contains("rm -rf -- /\n")); + assert!(!script.contains(" / ")); + } + + #[test] + fn the_scrub_total_is_read_back_from_the_marker_line() { + assert_eq!( + parse_scrub_total("some noise\n###TRIPLE-C-SCRUBBED 4812345\n"), + Some(4812345) + ); + // Nothing reclaimed is a real answer and must not read as a failure. + assert_eq!(parse_scrub_total("###TRIPLE-C-SCRUBBED 0"), Some(0)); + // No marker means the script never got to the end — a different thing + // from reclaiming nothing, and the caller logs it differently. + assert_eq!(parse_scrub_total("sh: du: not found"), None); + assert_eq!(parse_scrub_total(""), None); + } + + // ── Container log rotation (A2) ────────────────────────────────────────── + + #[test] + fn every_container_is_created_with_a_bounded_log() { + let cfg = capped_log_config(); + assert_eq!(cfg.typ.as_deref(), Some("json-file")); + let config = cfg.config.expect("a json-file driver with no config is unbounded"); + assert_eq!(config.get("max-size").map(String::as_str), Some("10m")); + assert_eq!(config.get("max-file").map(String::as_str), Some("3")); + } + + // ── Claude Code settings.json (Part B) ─────────────────────────────────── + + fn settings_json(s: Option<&ClaudeCodeSettings>, sandbox: bool) -> serde_json::Value { + serde_json::from_str(&build_claude_code_settings_json(s, sandbox)) + .expect("the payload must be valid JSON — the entrypoint pipes it into jq") + } + + /// The five keys that used to be emitted only when non-default, plus the + /// sandbox block that already knew better. + const MANAGED_SETTINGS_KEYS: &[&str] = &[ + "tui", + "effortLevel", + "autoScrollEnabled", + "showThinkingSummaries", + "viewMode", + "awaySummaryEnabled", + "sandbox", + ]; + + #[test] + fn turning_a_setting_off_clears_it_rather_than_omitting_it() { + // This is the whole bug. `~/.claude/settings.json` lives on a persisted + // volume and the entrypoint merges into it, so a key that is merely + // *absent* when the setting is off leaves the previous on-value in + // place — the setting could never be turned back off short of a + // destructive Reset. + let on = ClaudeCodeSettings { + tui_mode: Some("fullscreen".to_string()), + effort: Some("xhigh".to_string()), + auto_scroll_disabled: true, + focus_mode: true, + show_thinking_summaries: true, + session_recap_disabled: true, + ..Default::default() + }; + let hot = settings_json(Some(&on), true); + assert_eq!(hot["tui"], serde_json::json!("fullscreen")); + assert_eq!(hot["effortLevel"], serde_json::json!("xhigh")); + assert_eq!(hot["autoScrollEnabled"], serde_json::json!(false)); + assert_eq!(hot["showThinkingSummaries"], serde_json::json!(true)); + assert_eq!(hot["viewMode"], serde_json::json!("focus")); + assert_eq!(hot["awaySummaryEnabled"], serde_json::json!(false)); + assert_eq!(hot["sandbox"]["enabled"], serde_json::json!(true)); + + // Now everything back to default. Every key must still be *present*, + // carrying either its neutral value or a null the entrypoint deletes. + let cold = settings_json(Some(&ClaudeCodeSettings::default()), false); + for key in MANAGED_SETTINGS_KEYS { + assert!( + cold.get(*key).is_some(), + "{} is missing when the setting is off, so a stale on-value survives the merge", + key + ); + } + assert_eq!(cold["tui"], serde_json::Value::Null); + assert_eq!(cold["effortLevel"], serde_json::Value::Null); + assert_eq!(cold["autoScrollEnabled"], serde_json::json!(true)); + assert_eq!(cold["showThinkingSummaries"], serde_json::json!(false)); + assert_eq!(cold["viewMode"], serde_json::Value::Null); + assert_eq!(cold["awaySummaryEnabled"], serde_json::Value::Null); + assert_eq!(cold["sandbox"]["enabled"], serde_json::json!(false)); + } + + #[test] + fn no_settings_struct_at_all_still_asserts_every_key() { + // "This project has no Claude Code settings" is not "say nothing" — + // the file on the config volume may still hold a previous project + // configuration's values. + let cold = settings_json(None, false); + for key in MANAGED_SETTINGS_KEYS { + assert!(cold.get(*key).is_some(), "{} is missing with no settings struct", key); + } + } + + #[test] + fn the_settings_payload_uses_the_key_names_claude_code_actually_reads() { + let s = ClaudeCodeSettings { + effort: Some("high".to_string()), + focus_mode: true, + ..Default::default() + }; + let json = settings_json(Some(&s), false); + // `effort` and `focusMode` were both invented; Claude Code reads + // `effortLevel` and `viewMode`. + assert!(json.get("effort").is_none(), "`effort` is not a Claude Code setting"); + assert!(json.get("focusMode").is_none(), "`focusMode` is not a Claude Code setting"); + assert_eq!(json["effortLevel"], serde_json::json!("high")); + assert_eq!(json["viewMode"], serde_json::json!("focus")); + } + + #[test] + fn tui_can_be_pinned_to_the_classic_renderer_as_well_as_left_automatic() { + // Three distinct states; "automatic" is not "classic". + let auto = settings_json(Some(&ClaudeCodeSettings::default()), false); + assert_eq!(auto["tui"], serde_json::Value::Null); + + let classic = settings_json( + Some(&ClaudeCodeSettings { + tui_mode: Some("default".to_string()), + ..Default::default() + }), + false, + ); + assert_eq!(classic["tui"], serde_json::json!("default")); + } + + #[test] + fn a_project_that_never_touched_session_recap_leaves_it_alone() { + // Claude Code's recap is on by default, so the zero value of the field + // has to be "don't interfere". Getting this backwards would have + // silently disabled recaps for every existing project. + let untouched = ClaudeCodeSettings::default(); + assert!(!untouched.session_recap_disabled); + assert_eq!( + settings_json(Some(&untouched), false)["awaySummaryEnabled"], + serde_json::Value::Null + ); + } + + #[test] + fn a_disabled_setting_changes_the_recreation_fingerprint() { + // `container_needs_recreation` is label-based and never diffs env, so + // the settings only reach a container if the fingerprint moves. + let on = ClaudeCodeSettings { + focus_mode: true, + ..Default::default() + }; + let off = ClaudeCodeSettings::default(); + assert_ne!( + compute_claude_code_settings_fingerprint(Some(&on), false), + compute_claude_code_settings_fingerprint(Some(&off), false), + ); + let recap_off = ClaudeCodeSettings { + session_recap_disabled: true, + ..Default::default() + }; + assert_ne!( + compute_claude_code_settings_fingerprint(Some(&recap_off), false), + compute_claude_code_settings_fingerprint(Some(&off), false), + ); + } + + #[test] + fn the_claude_code_env_vars_are_reserved_from_custom_env() { + for key in [ + "CLAUDE_CODE_NO_FLICKER", + "CLAUDE_CODE_ENABLE_AWAY_SUMMARY", + "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB", + "ENABLE_PROMPT_CACHING_1H", + ] { + assert!(is_reserved_env_key(key), "{} must not be hand-settable", key); + assert!(is_reserved_env_key(&key.to_lowercase())); + } + } + + #[test] + fn every_claude_code_env_var_is_emitted_on_every_create() { + // `docker commit` bakes env into the snapshot image, so a name that + // goes missing when its setting is off keeps whatever the image + // carries. All four must appear whatever the settings say. + for settings in [None, Some(&ClaudeCodeSettings::default())] { + let emitted = claude_code_env_vars(settings); + let names: Vec<&str> = emitted + .iter() + .map(|entry| entry.split_once('=').expect("every entry is KEY=VALUE").0) + .collect(); + for expected in [ + "CLAUDE_CODE_NO_FLICKER", + "CLAUDE_CODE_ENABLE_AWAY_SUMMARY", + "CLAUDE_CODE_SUBPROCESS_ENV_SCRUB", + "ENABLE_PROMPT_CACHING_1H", + ] { + assert!(names.contains(&expected), "{} was not emitted", expected); + } + } + } + + #[test] + fn the_neutral_state_of_the_overriding_env_vars_is_empty_not_zero() { + // Both of these outrank a setting the user can change from inside the + // container, so their "off" has to be silence, not an instruction. + let vars = claude_code_env_vars(Some(&ClaudeCodeSettings::default())); + assert!(vars.contains(&"CLAUDE_CODE_NO_FLICKER=".to_string())); + assert!(vars.contains(&"CLAUDE_CODE_ENABLE_AWAY_SUMMARY=".to_string())); + // These two have no documented meaning for `0`, so it is safe to say. + assert!(vars.contains(&"CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=0".to_string())); + assert!(vars.contains(&"ENABLE_PROMPT_CACHING_1H=0".to_string())); + } + + #[test] + fn turning_the_session_recap_off_actually_forces_it_off() { + // The whole point of B3: `=1` when enabled was a no-op against a + // feature that was already on, and there was no off path at all. + let off = ClaudeCodeSettings { + session_recap_disabled: true, + ..Default::default() + }; + assert!(claude_code_env_vars(Some(&off)) + .contains(&"CLAUDE_CODE_ENABLE_AWAY_SUMMARY=0".to_string())); + } + + #[test] + fn the_tui_choice_reaches_the_env_var_that_outranks_the_setting() { + let fullscreen = ClaudeCodeSettings { + tui_mode: Some("fullscreen".to_string()), + ..Default::default() + }; + assert!(claude_code_env_vars(Some(&fullscreen)) + .contains(&"CLAUDE_CODE_NO_FLICKER=1".to_string())); + + let classic = ClaudeCodeSettings { + tui_mode: Some("default".to_string()), + ..Default::default() + }; + assert!(claude_code_env_vars(Some(&classic)) + .contains(&"CLAUDE_CODE_NO_FLICKER=0".to_string())); + } } diff --git a/app/src-tauri/src/docker/migration.rs b/app/src-tauri/src/docker/migration.rs index 8c39346..323d346 100644 --- a/app/src-tauri/src/docker/migration.rs +++ b/app/src-tauri/src/docker/migration.rs @@ -712,13 +712,76 @@ pub async fn run_throwaway(image: &str, script: &str) -> Result Self { + Self { id, armed: true } + } + + fn id(&self) -> &str { + &self.id + } + + async fn remove_now(mut self) { + self.armed = false; + remove_probe_container(&self.id).await; + } +} + +impl Drop for ProbeContainerGuard { + fn drop(&mut self) { + if !self.armed { + return; + } + let id = std::mem::take(&mut self.id); + log::warn!("Probe container {} was abandoned; removing it in the background", id); + tauri::async_runtime::spawn(async move { + remove_probe_container(&id).await; + }); + } +} + +/// Force-remove one probe container. Missing is success — the point is that the +/// container is gone. +async fn remove_probe_container(id: &str) { + let Ok(docker) = get_docker() else { + return; + }; + match docker .remove_container( - &id, + id, Some(RemoveContainerOptions { force: true, v: true, @@ -727,10 +790,57 @@ pub async fn run_throwaway(image: &str, script: &str) -> Result {} + Err(e) => log::warn!("Failed to remove probe container {}: {}", id, e), } +} - result +/// Remove probe containers left behind by a previous run of the app. +/// +/// A probe is labelled [`LABEL_PROBE`] precisely so it stays findable after a +/// crash, but until now nothing ever went looking. One leftover probe pins the +/// base image it was created from — several gigabytes that +/// `sweep_orphaned_snapshots` then reports as "in use" and correctly refuses to +/// touch, with no way for the user to find out why. +/// +/// Safe to run at startup: a probe is a short-lived `/bin/sh` with no mounts +/// and no volumes, owned entirely by a `run_throwaway` call. If one is running +/// right now it belongs to this process — and this runs before any migration +/// can be started, so there is none to interrupt. +pub async fn reap_probe_containers() { + let Ok(docker) = get_docker() else { + return; + }; + + let filters: HashMap> = HashMap::from([( + "label".to_string(), + vec![format!("{}={}", LABEL_PROBE, PROBE_LABEL_MIGRATION)], + )]); + + let containers = match docker + .list_containers(Some(bollard::container::ListContainersOptions { + all: true, + filters, + ..Default::default() + })) + .await + { + Ok(list) => list, + Err(e) => { + log::warn!("Could not list leftover probe containers: {}", e); + return; + } + }; + + for c in containers { + if let Some(id) = c.id { + log::info!("Removing leftover migration probe container {}", id); + remove_probe_container(&id).await; + } + } } async fn run_throwaway_inner(id: &str) -> Result { @@ -910,6 +1020,145 @@ pub fn rollback_tag(now: &chrono::DateTime) -> String { format!("pre-migration-{}", now.format("%Y%m%d-%H%M%S")) } +/// How long a rollback pin may sit with no migration record behind it before +/// [`reap_stale_migration_pins`] drops the tag. +/// +/// Two weeks, chosen to be far longer than anyone deliberates over a base +/// update and far shorter than "forever", which is what it was. +pub const STALE_PIN_MAX_AGE_DAYS: i64 = 14; + +/// Recover the timestamp encoded in a tag produced by [`rollback_tag`]. +/// +/// `None` for anything that is not one of ours — a tag that merely *starts* +/// with `pre-migration-` but does not carry a parseable timestamp is left alone +/// rather than guessed at, because the consequence of guessing wrong is +/// deleting the only copy of somebody's system layer. +pub fn parse_rollback_tag(tag: &str) -> Option> { + let stamp = tag.strip_prefix("pre-migration-")?; + let naive = chrono::NaiveDateTime::parse_from_str(stamp, "%Y%m%d-%H%M%S").ok()?; + Some(naive.and_utc()) +} + +/// Split `triple-c-snapshot-:` into the project id and the tag. +/// +/// `None` when the reference is not a snapshot repo at all. +pub fn parse_snapshot_reference(reference: &str) -> Option<(String, String)> { + let (repo, tag) = split_image_ref(reference); + let project_id = repo.strip_prefix("triple-c-snapshot-")?.to_string(); + if project_id.is_empty() { + return None; + } + Some((project_id, tag)) +} + +/// Whether a rollback pin is safe to drop, given how old it is and whether the +/// project it belongs to still has a migration record. +/// +/// Pure so the decision can be tested without a daemon. The order of the two +/// conditions is the point: **a pin whose migration is still awaiting +/// confirmation is never reaped at any age**, because it is the only copy of +/// the rollback target and the user has not yet said they are happy with the +/// new base. +pub fn pin_is_reapable( + tag: &str, + has_migration_record: bool, + now: &chrono::DateTime, +) -> bool { + if has_migration_record { + return false; + } + let Some(created) = parse_rollback_tag(tag) else { + return false; + }; + (*now - created).num_days() >= STALE_PIN_MAX_AGE_DAYS +} + +/// Drop `triple-c-snapshot-*:pre-migration-*` tags that no migration record +/// claims any more, so the images behind them become sweepable. +/// +/// ## Why this scans tags instead of reading the records +/// +/// Every other path to a rollback pin starts from +/// `migration_store::load`, and `load` reports an unparseable state file as +/// *absent* — so a single corrupt record used to strand a 4–12 GB image that no +/// code could ever name again. Confirming or rolling back both remove the +/// record and drop the tag together, so a `pre-migration-*` tag with no record +/// beside it is by definition one that lost its owner: a crash between the two, +/// a record that was deleted by hand, or the corrupt-file case. +/// +/// Scanning the *tag pattern* is the only way to find those. `load` moving a +/// corrupt record aside (see `migration_store::load`) is what stops that case +/// from being permanently invisible here too. +/// +/// ## Why it only untags +/// +/// Dropping the tag turns the image dangling, and it is already labelled +/// `triple-c.managed=true` because `docker commit` created it — so +/// `sweep_orphaned_snapshots` collects it on the same pass, under the same two +/// safety conditions, with the daemon's "still in use by a container" refusal +/// still in front of it. Nothing here calls `docker rmi` on a reachable image. +pub async fn reap_stale_migration_pins() -> usize { + use bollard::image::ListImagesOptions; + + let Ok(docker) = get_docker() else { + return 0; + }; + + // `reference` matches against `repo:tag`, so this asks the daemon for + // exactly the shape [`rollback_tag`] produces and nothing else. + let filters: HashMap> = HashMap::from([( + "reference".to_string(), + vec!["triple-c-snapshot-*:pre-migration-*".to_string()], + )]); + + let images = match docker + .list_images(Some(ListImagesOptions { + all: false, + filters, + ..Default::default() + })) + .await + { + Ok(images) => images, + Err(e) => { + log::warn!("Could not list rollback pins: {}", e); + return 0; + } + }; + + let now = chrono::Utc::now(); + let mut reaped = 0usize; + + for summary in images { + for reference in &summary.repo_tags { + let Some((project_id, tag)) = parse_snapshot_reference(reference) else { + continue; + }; + // Filesystem presence, not `load`: a record we cannot parse must + // still count as "somebody may want this back". + let has_record = + crate::storage::migration_store::has_record(&project_id).unwrap_or(true); + if !pin_is_reapable(&tag, has_record, &now) { + continue; + } + match untag_image(reference).await { + Ok(()) => { + log::info!( + "Dropped stale rollback pin {} ({:.2} GB) — no migration record has claimed it for {} days", + reference, + summary.size as f64 / 1_073_741_824.0, + STALE_PIN_MAX_AGE_DAYS, + ); + reaped += 1; + } + Err(e) => log::warn!("Could not drop stale rollback pin {}: {}", reference, e), + } + } + } + + reaped +} + /// Split `repo:tag` into its parts, defaulting the tag to `latest`. pub fn split_image_ref(image: &str) -> (String, String) { match image.rsplit_once(':') { @@ -1622,4 +1871,73 @@ mod tests { assert_eq!(shell_single_quote("/opt/a'b"), r#"'/opt/a'\''b'"#); } + + // ── Stale rollback pins (A5) ───────────────────────────────────────────── + + fn at(y: i32, m: u32, d: u32) -> chrono::DateTime { + chrono::NaiveDate::from_ymd_opt(y, m, d) + .unwrap() + .and_hms_opt(12, 0, 0) + .unwrap() + .and_utc() + } + + #[test] + fn a_rollback_tag_round_trips_through_its_parser() { + let made = at(2026, 3, 14); + assert_eq!(parse_rollback_tag(&rollback_tag(&made)), Some(made)); + } + + #[test] + fn only_a_real_rollback_tag_parses() { + assert_eq!(parse_rollback_tag("latest"), None); + assert_eq!(parse_rollback_tag("pre-migration-"), None); + // Looks like ours but carries no timestamp we produced. Guessing here + // would mean deleting the only copy of somebody's system layer. + assert_eq!(parse_rollback_tag("pre-migration-keepme"), None); + assert_eq!(parse_rollback_tag("pre-migration-20260231-000000"), None); + } + + #[test] + fn a_snapshot_reference_yields_its_project_id() { + assert_eq!( + parse_snapshot_reference("triple-c-snapshot-abc-123:pre-migration-20260101-101500"), + Some(("abc-123".to_string(), "pre-migration-20260101-101500".to_string())) + ); + // Not ours: a base image, and a repo that merely shares a prefix. + assert_eq!(parse_snapshot_reference("triple-c-sandbox:latest"), None); + assert_eq!(parse_snapshot_reference("triple-c-snapshot-:latest"), None); + } + + #[test] + fn a_pin_awaiting_confirmation_is_never_reaped_at_any_age() { + // The one rule that cannot bend: while a migration record exists, this + // image is the only copy of the rollback target and the user has not + // yet said they are happy on the new base. + let ancient = rollback_tag(&at(2020, 1, 1)); + assert!(!pin_is_reapable(&ancient, true, &at(2026, 8, 23))); + } + + #[test] + fn an_unclaimed_pin_is_reaped_only_once_it_is_old() { + let made = at(2026, 8, 1); + let tag = rollback_tag(&made); + assert!(!pin_is_reapable(&tag, false, &at(2026, 8, 2))); + assert!(!pin_is_reapable( + &tag, + false, + &(made + chrono::Duration::days(STALE_PIN_MAX_AGE_DAYS - 1)) + )); + assert!(pin_is_reapable( + &tag, + false, + &(made + chrono::Duration::days(STALE_PIN_MAX_AGE_DAYS)) + )); + } + + #[test] + fn a_tag_we_cannot_date_is_left_alone() { + assert!(!pin_is_reapable("pre-migration-handmade", false, &at(2026, 8, 23))); + assert!(!pin_is_reapable("latest", false, &at(2026, 8, 23))); + } } diff --git a/app/src-tauri/src/lib.rs b/app/src-tauri/src/lib.rs index a918c19..e554dc3 100644 --- a/app/src-tauri/src/lib.rs +++ b/app/src-tauri/src/lib.rs @@ -235,6 +235,30 @@ pub fn run() { } } + // ── Startup disk housekeeping ──────────────────────────────── + // Until now the only sweep ran *after* a recreation, so a user who + // simply stopped launching a project kept its orphaned snapshot + // layers forever, and anything a crash left behind (a probe + // container pinning a base image, a rollback pin whose migration + // record is gone) had no path back at all. All three are + // read-mostly and finish in well under a second on an idle daemon, + // but they are detached anyway: housekeeping must never delay the + // window appearing, and a daemon that is not running yet is a + // logged warning rather than a failed start. + // + // Ordering matters. Probes are removed first because a probe holds + // an image open and the sweep will not force; pins are untagged + // second so the images they were holding are dangling by the time + // the sweep lists them; the sweep runs last and collects both. + tauri::async_runtime::spawn(async { + crate::docker::reap_probe_containers().await; + let reaped = crate::docker::reap_stale_migration_pins().await; + if reaped > 0 { + log::info!("Startup housekeeping dropped {} stale rollback pin(s)", reaped); + } + crate::docker::sweep_orphaned_snapshots_logged("startup").await; + }); + // Auto-start web terminal server if enabled in settings let settings = settings_store_setup.get(); if settings.web_terminal.enabled { diff --git a/app/src-tauri/src/models/project.rs b/app/src-tauri/src/models/project.rs index 385ee8a..c0a8f30 100644 --- a/app/src-tauri/src/models/project.rs +++ b/app/src-tauri/src/models/project.rs @@ -86,24 +86,42 @@ impl PermissionMode { /// These map to Claude Code env vars and ~/.claude/settings.json entries. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default)] pub struct ClaudeCodeSettings { - /// TUI rendering mode: None = default, Some("fullscreen") = flicker-free alt-screen + /// TUI renderer. `None` leaves settings.json's `tui` key unset, which is + /// what lets Claude Code pick the renderer itself; `Some("default")` pins + /// the classic main-screen renderer and `Some("fullscreen")` the alt-screen + /// one. All three are distinct — "let it choose" is not "classic". #[serde(default)] pub tui_mode: Option, - /// Effort level: None = default, Some("low"|"medium"|"high") + /// Saved `/effort` level: `None` = unset, otherwise one of + /// `"low" | "medium" | "high" | "xhigh"`. Written to settings.json as + /// `effortLevel` (**not** `effort`, which Claude Code has never read). #[serde(default)] pub effort: Option, - /// Disable auto-scroll in fullscreen TUI mode + /// Disable auto-scroll in fullscreen TUI mode. Held in the *disabled* sense + /// because Claude Code's `autoScrollEnabled` defaults to `true`, so the + /// zero value of this field has to mean "leave it on". #[serde(default)] pub auto_scroll_disabled: bool, - /// Enable focus mode (collapsed tool output) + /// Collapse tool output to one-line summaries. Written to settings.json as + /// `viewMode: "focus"`; there is no `focusMode` key in Claude Code. #[serde(default)] pub focus_mode: bool, /// Show thinking summaries in responses #[serde(default)] pub show_thinking_summaries: bool, - /// Enable session recap when returning to a session + /// Turn the session recap **off**. + /// + /// Held in the disabled sense for the same reason as `auto_scroll_disabled`, + /// and the rename from the old `enable_session_recap` is load-bearing rather + /// than cosmetic. Claude Code's recap is on by default, so the old field was + /// inverted: switching it on was a no-op and switching it off did nothing at + /// all. Reusing the name with the opposite meaning would have read every + /// stored `enable_session_recap: false` — which is what every project that + /// never touched the control holds — as "the user turned the recap off" and + /// silently disabled it for all of them. A new name lets the old key be + /// ignored, which lands every existing project on the correct default. #[serde(default)] - pub enable_session_recap: bool, + pub session_recap_disabled: bool, /// Strip credentials from subprocess environments #[serde(default)] pub env_scrub: bool, diff --git a/app/src-tauri/src/storage/migration_store.rs b/app/src-tauri/src/storage/migration_store.rs index 88a4b80..859d916 100644 --- a/app/src-tauri/src/storage/migration_store.rs +++ b/app/src-tauri/src/storage/migration_store.rs @@ -49,6 +49,16 @@ fn sanitize(project_id: &str) -> String { /// Read a project's migration state. `Ok(None)` means no migration is in /// flight; an unparseable file is treated the same way (and logged) rather than /// blocking every future migration on a corrupt record. +/// +/// **A corrupt record is moved aside, not merely ignored.** Reporting "absent" +/// while leaving the file in place strands the rollback pin it describes: the +/// `:pre-migration-*` tag holding a 4–12 GB image stays on disk, no code path +/// can find it again (every one of them starts here and is told there is no +/// migration), and the leftover file goes on making the project look like it +/// has a migration in flight to anything that checks for the file rather than +/// parsing it — including [`has_record`], which the pin reaper relies on. +/// Renaming to `.bak` follows the same convention as `projects.json`: the +/// user's bytes are kept, but they stop pinning gigabytes. pub fn load(project_id: &str) -> Result, String> { let path = state_path(project_id)?; if !path.exists() { @@ -59,16 +69,34 @@ pub fn load(project_id: &str) -> Result, String> { match serde_json::from_str::(&data) { Ok(state) => Ok(Some(state)), Err(e) => { + let backup = path.with_extension("json.bak"); + let moved = fs::rename(&path, &backup); log::error!( - "Failed to parse migration state for project {}: {} — treating as absent", + "Failed to parse migration state for project {}: {} — treating as absent{}", project_id, - e + e, + match moved { + Ok(()) => format!(" and moved the record to {}", backup.display()), + Err(ref e) => format!(" (could not move the record aside: {})", e), + } ); Ok(None) } } } +/// Whether a project has a migration record on disk *at all*, without parsing +/// it. +/// +/// The pin reaper needs "is this project's rollback image still somebody's only +/// copy?" and must answer it conservatively. [`load`] cannot be used for that +/// question on its own — it deliberately reports a corrupt record as absent — +/// so this asks the filesystem instead. `load` moving a corrupt record aside is +/// what keeps the two answers from disagreeing forever. +pub fn has_record(project_id: &str) -> Result { + Ok(state_path(project_id)?.exists()) +} + /// Atomically write a project's migration state. pub fn save(project_id: &str, state: &MigrationState) -> Result<(), String> { let path = state_path(project_id)?; diff --git a/app/src/components/projects/ClaudeCodeSettingsEditor.test.tsx b/app/src/components/projects/ClaudeCodeSettingsEditor.test.tsx new file mode 100644 index 0000000..63f1803 --- /dev/null +++ b/app/src/components/projects/ClaudeCodeSettingsEditor.test.tsx @@ -0,0 +1,62 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import ClaudeCodeSettingsEditor, { CLAUDE_CODE_DEFAULTS } from "./ClaudeCodeSettingsEditor"; +import type { ClaudeCodeSettings } from "../../lib/types"; + +function renderEditor(settings: ClaudeCodeSettings | null) { + const onSave = vi.fn().mockResolvedValue(undefined); + render( + , + ); + return onSave; +} + +describe("ClaudeCodeSettingsEditor", () => { + it("shows the two default-on settings as on for a project that never touched them", () => { + // Claude Code's session recap and fullscreen auto-scroll are both on by + // default, and the fields behind them store the *disabled* sense. A toggle + // rendered straight from the field would tell every existing user their + // recap is off. + renderEditor(null); + expect(screen.getByRole("switch", { name: "Session recap" })).toBeChecked(); + expect(screen.getByRole("switch", { name: "Auto-scroll" })).toBeChecked(); + expect(screen.getByRole("switch", { name: "Focus mode" })).not.toBeChecked(); + }); + + it("stores the disabled sense when an inverted toggle is switched off", () => { + const onSave = renderEditor(null); + fireEvent.click(screen.getByRole("switch", { name: "Session recap" })); + expect(onSave).toHaveBeenCalledWith( + expect.objectContaining({ session_recap_disabled: true }), + ); + }); + + it("collapses back to null once every setting is at its default again", () => { + // `null` is what tells the backend this project adds nothing over the + // global settings, so the round trip has to land exactly back on it. + const onSave = renderEditor({ ...CLAUDE_CODE_DEFAULTS, session_recap_disabled: true }); + fireEvent.click(screen.getByRole("switch", { name: "Session recap" })); + expect(onSave).toHaveBeenCalledWith(null); + }); + + it("offers the classic renderer as a choice distinct from automatic", () => { + // Leaving `tui` unset lets Claude Code pick; pinning "default" is a + // different, and previously unreachable, instruction. + const onSave = renderEditor(null); + const tui = screen.getByLabelText("TUI mode"); + expect( + Array.from(tui.querySelectorAll("option")).map((o) => o.getAttribute("value")), + ).toEqual(["", "default", "fullscreen"]); + fireEvent.change(tui, { target: { value: "default" } }); + expect(onSave).toHaveBeenCalledWith(expect.objectContaining({ tui_mode: "default" })); + }); + + it("offers every effort level Claude Code accepts", () => { + renderEditor(null); + expect( + Array.from( + screen.getByLabelText("Effort level").querySelectorAll("option"), + ).map((o) => o.getAttribute("value")), + ).toEqual(["", "low", "medium", "high", "xhigh"]); + }); +}); diff --git a/app/src/components/projects/ClaudeCodeSettingsEditor.tsx b/app/src/components/projects/ClaudeCodeSettingsEditor.tsx index d61c3e6..caa9a71 100644 --- a/app/src/components/projects/ClaudeCodeSettingsEditor.tsx +++ b/app/src/components/projects/ClaudeCodeSettingsEditor.tsx @@ -16,7 +16,7 @@ export const CLAUDE_CODE_DEFAULTS: ClaudeCodeSettings = { auto_scroll_disabled: false, focus_mode: false, show_thinking_summaries: false, - enable_session_recap: false, + session_recap_disabled: false, env_scrub: false, prompt_caching_1h: false, }; @@ -28,16 +28,25 @@ function isAllDefaults(s: ClaudeCodeSettings): boolean { s.auto_scroll_disabled === false && s.focus_mode === false && s.show_thinking_summaries === false && - s.enable_session_recap === false && + s.session_recap_disabled === false && s.env_scrub === false && s.prompt_caching_1h === false ); } +/** + * Two of Claude Code's settings are **on by default**, so the field behind them + * stores the *disabled* sense (`auto_scroll_disabled`, `session_recap_disabled`) + * — that is what makes an untouched project mean "leave Claude Code alone" + * rather than "the user turned this off". `invert` is what lets those still + * read as an ordinary on/off switch here: the toggle shows the feature's state, + * the field stores the deviation from the default. + */ const BOOLEAN_FIELDS: { key: keyof Omit; label: string; hint: string; + invert?: boolean; }[] = [ { key: "focus_mode", label: "Focus mode", hint: "Collapses tool output to one-line summaries." }, { @@ -46,14 +55,16 @@ const BOOLEAN_FIELDS: { hint: "Shows Claude's thinking process as summaries.", }, { - key: "enable_session_recap", + key: "session_recap_disabled", label: "Session recap", - hint: "Provides context when returning to a session.", + hint: "Shows a one-line recap when you return to the terminal after a few minutes away.", + invert: true, }, { key: "auto_scroll_disabled", - label: "Auto-scroll disabled", - hint: "Disables auto-scroll when in fullscreen TUI mode.", + label: "Auto-scroll", + hint: "Follows new output to the bottom in fullscreen rendering.", + invert: true, }, { key: "env_scrub", @@ -95,9 +106,16 @@ export default function ClaudeCodeSettingsEditor({

)} + {/* + Three states, not two. Leaving `tui` unset is what lets Claude Code pick + the renderer for itself, which is not the same as pinning the classic + one — and the key is now always written (or explicitly deleted), so + "Automatic" has to be selectable rather than merely being what you get + when nothing is emitted. + */} - + + } @@ -127,11 +146,12 @@ export default function ClaudeCodeSettingsEditor({ + } /> - {BOOLEAN_FIELDS.map(({ key, label, hint }) => ( + {BOOLEAN_FIELDS.map(({ key, label, hint, invert }) => ( apply({ [key]: v } as Partial)} + onChange={(v) => + apply({ [key]: invert ? !v : v } as Partial) + } /> } /> diff --git a/app/src/components/projects/ConfirmRemoveModal.tsx b/app/src/components/projects/ConfirmRemoveModal.tsx index 215c3fa..1fc53b4 100644 --- a/app/src/components/projects/ConfirmRemoveModal.tsx +++ b/app/src/components/projects/ConfirmRemoveModal.tsx @@ -28,10 +28,24 @@ export default function ConfirmRemoveModal({ projectName, onConfirm, onCancel }: } > + {/* + Everything remove_project() destroys, named. It removes the container, + *both* named volumes (triple-c-home-{id} and triple-c-claude-config-{id}), + the triple-c-snapshot-{id} image and the project's keychain secrets — so + an accurate warning has to reach past "the config volume". The last + sentence is the reassuring half and matters just as much: project folders + are bind mounts from the host and nothing here touches them. + */}

Are you sure you want to remove{" "} - {projectName}? This will - delete the container, config volume, and stored credentials. + {projectName}? This deletes + its container, both of its volumes and its saved container image — so the home + directory, the Claude login and config, installed skills, session transcripts, + scheduled tasks and any stored credentials all go with it. +

+

+ Your project folders on this machine are mounted in, not copied, and are left + untouched.

); diff --git a/app/src/components/projects/home/TaskEditorModal.tsx b/app/src/components/projects/home/TaskEditorModal.tsx index 7e4d34f..09a57ca 100644 --- a/app/src/components/projects/home/TaskEditorModal.tsx +++ b/app/src/components/projects/home/TaskEditorModal.tsx @@ -311,10 +311,22 @@ export default function TaskEditorModal({ project, task, onClose, onSaved }: Pro {task && ( + /* + An edit is `add` then `remove` (see `update_scheduled_task`), and + `triple-c-scheduler`'s remove now reaps the task's log directory — + so on a current container the old logs are gone, not merely filed + under the old id, which is what this used to promise. + + It is deliberately not stated as a certainty. `/usr/local/bin` only + changes on base-image migration or Reset, so a project still running + an older base image carries the older scheduler, whose remove leaves + the log directory behind. "Assume they go with it" is true in both + worlds and spares the user a paragraph about which one they are in. + */

The scheduler has no edit command, so saving re-creates this task under a new id and - removes {task.id}. Its previous run logs stay under - the old id. + removes {task.id}. Assume its earlier run logs go + with it.

)} diff --git a/app/src/lib/types.ts b/app/src/lib/types.ts index b4eec82..3510ecd 100644 --- a/app/src/lib/types.ts +++ b/app/src/lib/types.ts @@ -134,12 +134,19 @@ export interface OpenAiCompatibleConfig { } export interface ClaudeCodeSettings { + /** `null` = let Claude Code choose the renderer; `"default"` = classic, `"fullscreen"` = alt-screen. */ tui_mode: string | null; + /** `null` = unset, else `"low" | "medium" | "high" | "xhigh"`. Written as `effortLevel`. */ effort: string | null; auto_scroll_disabled: boolean; + /** Written as `viewMode: "focus"`. */ focus_mode: boolean; show_thinking_summaries: boolean; - enable_session_recap: boolean; + /** + * Turns the session recap **off**. Held in the disabled sense because Claude + * Code's recap is on by default — see the Rust doc on `ClaudeCodeSettings`. + */ + session_recap_disabled: boolean; env_scrub: boolean; prompt_caching_1h: boolean; } diff --git a/container/Dockerfile b/container/Dockerfile index 8b4cc41..fc81ee9 100644 --- a/container/Dockerfile +++ b/container/Dockerfile @@ -1,5 +1,24 @@ FROM ubuntu:24.04 +# ── Provenance labels ──────────────────────────────────────────────────────── +# Without these the base image carries no labels at all, and +# `sweep_orphaned_snapshots` (app/src-tauri/src/docker/container.rs) filters on +# `dangling=true` **and** `triple-c.managed=true` — so a superseded base image, +# left untagged when a newer build claims `triple-c-sandbox:latest`, could never +# match and was never collected. ~11.9 GB of stranded base images was measured +# on one developer's daemon this way. +# +# `triple-c.managed=true` is what makes them sweepable. Note that Docker merges +# an image's labels into the containers created from it and `docker commit` +# copies a container's labels onto the image, so this value also arrives on +# every container and every snapshot — which is harmless, because +# `create_container` writes the same key explicitly anyway. +# +# `triple-c.base=true` marks *this* image specifically, so a base image can be +# told apart from a project snapshot without parsing repository names. +LABEL triple-c.managed=true +LABEL triple-c.base=true + # Multi-arch: builds for linux/amd64 and linux/arm64 (Apple Silicon) # Avoid interactive prompts during package install ENV DEBIAN_FRONTEND=noninteractive diff --git a/container/entrypoint.sh b/container/entrypoint.sh index 2e859b2..9bc0c8e 100644 --- a/container/entrypoint.sh +++ b/container/entrypoint.sh @@ -405,22 +405,37 @@ install_feature_skill pia-vpn "${VPN_SUPPORT_ENABLED:-0}" unset VPN_SUPPORT_ENABLED # ── Claude Code settings ──────────────────────────────────────────────────── -# Merge Claude Code settings into ~/.claude/settings.json (preserves existing -# keys). Creates the file if it doesn't exist. These control TUI mode, effort -# level, focus mode, thinking summaries, and other CLI behavior. +# Apply the managed Claude Code settings to ~/.claude/settings.json, keeping +# every key the user set inside the container. +# +# `settings.json` lives on the persisted triple-c-claude-config-{id} volume, so +# it outlives the container and a plain `.[0] * .[1]` merge could only ever +# *add*. That is what made every one of these settings one-way: switching one +# off in Triple-C omitted its key, the merge preserved the old on-value, and the +# setting stayed on until a destructive Reset. So the payload from Rust states +# the whole managed key set on every start, and a JSON **null** in it means +# "delete this key" rather than "merge a null" — which is how a setting whose +# neutral state is *unset* (`tui`, `effortLevel`, `viewMode`, +# `awaySummaryEnabled`) is turned back off without pinning a stand-in value. +# See `build_claude_code_settings_json` in app/src-tauri/src/docker/container.rs. if [ -n "$CLAUDE_CODE_SETTINGS_JSON" ]; then SETTINGS_FILE="/home/claude/.claude/settings.json" mkdir -p /home/claude/.claude - if [ -f "$SETTINGS_FILE" ]; then - # Merge: existing settings + new settings (new keys override on conflict) - MERGED=$(jq -s '.[0] * .[1]' "$SETTINGS_FILE" <(printf '%s' "$CLAUDE_CODE_SETTINGS_JSON") 2>/dev/null) - if [ -n "$MERGED" ]; then - printf '%s\n' "$MERGED" > "$SETTINGS_FILE" - else - echo "entrypoint: warning — failed to merge Claude Code settings into $SETTINGS_FILE" - fi + # One code path for "file exists" and "file doesn't": seeding an empty + # object means the null-deleting merge below runs in both cases, so a fresh + # container never gets a settings.json with literal nulls written into it. + [ -f "$SETTINGS_FILE" ] || printf '{}\n' > "$SETTINGS_FILE" + MERGED=$(jq -s ' + .[0] as $current + | .[1] as $managed + | ($managed | with_entries(select(.value != null))) as $set + | ($managed | to_entries | map(select(.value == null) | [.key])) as $clear + | ($current * $set) | delpaths($clear) + ' "$SETTINGS_FILE" <(printf '%s' "$CLAUDE_CODE_SETTINGS_JSON") 2>/dev/null) + if [ -n "$MERGED" ]; then + printf '%s\n' "$MERGED" > "$SETTINGS_FILE" else - printf '%s\n' "$CLAUDE_CODE_SETTINGS_JSON" > "$SETTINGS_FILE" + echo "entrypoint: warning — failed to merge Claude Code settings into $SETTINGS_FILE" fi chown claude:claude "$SETTINGS_FILE" chmod 600 "$SETTINGS_FILE" diff --git a/container/triple-c-scheduler b/container/triple-c-scheduler index e75fcae..46d1e38 100644 --- a/container/triple-c-scheduler +++ b/container/triple-c-scheduler @@ -20,6 +20,27 @@ generate_id() { head -c 4 /dev/urandom | od -An -tx1 | tr -d ' \n' } +# Delete a task's log directory, called wherever a task stops existing. +# +# The task file is the only index of a task, so a log directory that outlives +# it is unreachable — `logs --id` needs an id nothing can hand you any more — +# and it sits on the home volume for the life of the project. The moment of +# removal is the last point at which we still know what to delete. +# +# The `rm -rf` deserves paranoia, so the id is re-validated here rather than +# trusted from the caller: the pattern rejects an empty id (which would expand +# to $LOGS_DIR itself), anything containing `/` or `.` (which could climb out +# of $LOGS_DIR), and a leading `-`. It matches validate_task_id() in +# app/src-tauri/src/commands/inspect_commands.rs. Always one literal path, +# never a glob. +reap_task_logs() { + local id="${1:-}" + [[ "$id" =~ ^[A-Za-z0-9][A-Za-z0-9_-]*$ ]] || return 0 + local dir="${LOGS_DIR:?}/${id}" + [ -d "$dir" ] || return 0 + rm -rf -- "$dir" +} + # Live run state for a task: prints "pidstarted_epochlog" and returns # 0 when the task is genuinely running, returns 1 otherwise. # @@ -292,6 +313,7 @@ cmd_remove() { local name name=$(jq -r '.name' "$task_file") rm -f "$task_file" + reap_task_logs "$id" rebuild_crontab echo "Removed task '$name' ($id)" } diff --git a/container/triple-c-task-runner b/container/triple-c-task-runner index 79ffa60..7ce583b 100644 --- a/container/triple-c-task-runner +++ b/container/triple-c-task-runner @@ -125,6 +125,37 @@ fi echo "=== Exit code: $EXIT_CODE ===" } >> "$LOG_FILE" +# ── Cap the size of this run's log ────────────────────────────────────────── +# `claude -p` output is unbounded — a task told to walk a large tree can emit +# hundreds of megabytes in one run — and the pruning below counts *files*, not +# bytes, so twenty logs of any size are twenty logs. One chatty task can +# therefore fill the home volume, which is also where ~/.claude and the OAuth +# credential live. +# +# The tail is the half worth keeping: `claude -p` writes its answer at the end, +# and the footer just appended carries the exit code that `status` and the app +# both grep for. So an oversize log is rewritten as a marker line plus its last +# MAX_LOG_BYTES rather than being deleted or capped from the front. This runs +# before the notification below so the summary is taken from the capped file. +# +# Best effort throughout: the run's real result is already recorded, so a +# failure here must not change the exit status. Note that `run` may be tailing +# this file — it has already streamed everything up to here, and nothing is +# appended after this point, so replacing the inode is invisible to it. +MAX_LOG_BYTES=$(( 5 * 1024 * 1024 )) +LOG_BYTES=$(wc -c < "$LOG_FILE" 2>/dev/null || echo 0) +if [ "${LOG_BYTES:-0}" -gt "$MAX_LOG_BYTES" ]; then + TRUNC_FILE="${LOG_FILE}.trunc" + if { + echo "=== Log truncated: $(( LOG_BYTES - MAX_LOG_BYTES )) bytes dropped from the start (cap ${MAX_LOG_BYTES} bytes) ===" + tail -c "$MAX_LOG_BYTES" "$LOG_FILE" + } > "$TRUNC_FILE" 2>/dev/null; then + mv -f "$TRUNC_FILE" "$LOG_FILE" 2>/dev/null || rm -f "$TRUNC_FILE" + else + rm -f "$TRUNC_FILE" + fi +fi + # ── Write notification ────────────────────────────────────────────────────── mkdir -p "$NOTIFICATIONS_DIR" NOTIFY_FILE="${NOTIFICATIONS_DIR}/${TASK_ID}_${TIMESTAMP}.notify" @@ -176,6 +207,35 @@ if [ "$LOG_COUNT" -gt 20 ]; then find "$TASK_LOG_DIR" -name "*.log" -type f | sort | head -n $((LOG_COUNT - 20)) | xargs rm -f fi +# ── Reap log dirs of tasks that no longer exist ───────────────────────────── +# `triple-c-scheduler remove` deletes a task's log dir with the task, but a +# one-time task deletes its own task file above, so `remove` can never be run +# for it — nothing knows the id any more — and its directory would sit on the +# home volume forever. This is the sweep for that case. +# +# Deliberately delayed rather than done in the cleanup above: the run that just +# finished has only just written the sole record of itself, `run` and the app's +# Automation tab may still be tailing it, and `logs --id` keeps working for a +# task whose file is gone. So a dir is reaped only once nothing in it has been +# touched for LOG_RETENTION_DAYS, and never while a run is publishing state for +# that id. The sweep rides on task runs, so a container whose only task was +# one-time keeps that one directory until something else runs. +# +# Same paranoia as reap_task_logs() in triple-c-scheduler: the id comes from a +# directory name and is re-validated before it is used to build an `rm -rf` +# path, so no empty or path-bearing name can reach beyond $LOGS_DIR. +LOG_RETENTION_DAYS=7 +for ORPHAN_DIR in "$LOGS_DIR"/*/; do + [ -d "$ORPHAN_DIR" ] || continue + ORPHAN_ID=$(basename "$ORPHAN_DIR") + [[ "$ORPHAN_ID" =~ ^[A-Za-z0-9][A-Za-z0-9_-]*$ ]] || continue + [ -f "${TASKS_DIR}/${ORPHAN_ID}.json" ] && continue + [ -f "${RUNNING_DIR}/${ORPHAN_ID}.json" ] && continue + # Anything modified inside the window keeps the whole directory. + [ -n "$(find "$ORPHAN_DIR" -mmin "-$(( LOG_RETENTION_DAYS * 1440 ))" -print -quit 2>/dev/null)" ] && continue + rm -rf -- "${LOGS_DIR:?}/${ORPHAN_ID}" +done + # ── Prune old notifications (keep 50 total) ───────────────────────────────── NOTIFY_COUNT=$(find "$NOTIFICATIONS_DIR" -name "*.notify" -type f 2>/dev/null | wc -l) if [ "$NOTIFY_COUNT" -gt 50 ]; then