From 03b5e453e3b9252f494d46b0dc5a5bb067d42923 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 01:05:40 -0700 Subject: [PATCH 1/2] chore(selfhost): deduplicate the Docker prune scripts scripts/docker-prune.sh and scripts/selfhost-docker-prune.sh accidentally shipped from two separate PRs and did overlapping jobs. Keeps the systemd-timer-integrated, docs-referenced, tested one (selfhost-docker-prune.sh) as the single canonical script and folds in the genuinely useful behavior the other had: - Stopped-container pruning (docker container prune), age-filtered the same way images/build cache already were. - An opt-in --dry-run preview mode that reports disk usage without deleting anything; the systemd timer's zero-argument default call still prunes for real, unchanged. - A root filesystem usage report (df -h /) alongside docker system df. - A docker-on-PATH presence check. Removes scripts/docker-prune.sh and repoints every reference to it (docker-compose.yml, docker-compose.override.yml.example, .env.example) at the canonical script. Also documents the existing GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS env var in .env.example, which was missing before. --- .env.example | 8 +- .../src/lib/selfhost-env-reference.ts | 44 +++++----- docker-compose.override.yml.example | 5 +- docker-compose.yml | 2 +- scripts/docker-prune.sh | 82 ------------------- scripts/selfhost-docker-prune.sh | 49 +++++++++-- .../unit/selfhost-docker-prune-script.test.ts | 54 ++++++++++-- 7 files changed, 122 insertions(+), 122 deletions(-) delete mode 100755 scripts/docker-prune.sh mode change 100644 => 100755 scripts/selfhost-docker-prune.sh diff --git a/.env.example b/.env.example index 4be253f4c1..21f9d21c53 100644 --- a/.env.example +++ b/.env.example @@ -305,11 +305,13 @@ REDIS_URL=redis://redis:6379 # REQUIRED for the self-host review # RUNNER_NAME=gittensory-runner # RUNNER_LABELS=self-hosted,linux -# --- Docker disk hygiene (#selfhost-runtime-pressure) --- +# --- Docker disk hygiene (#audit-rate-headroom / #selfhost-runtime-pressure) --- # Build cache and unused images accumulate fast on a box that builds from source or runs CI runners; a root # disk over ~80-85% full slows down the WHOLE host (fsync latency, container scheduling), not just Docker. -# ./scripts/docker-prune.sh reports usage read-only by default; pass --yes to actually reclaim space (never -# touches volumes/application data). See the script's header for a cron example. +# Install the systemd timer (systemd/gittensory-docker-prune.{service,timer}.example) to reclaim stopped +# containers, unused images, and build cache on a schedule -- never volumes/application data. Run it +# manually at any time with `sh scripts/selfhost-docker-prune.sh`, or preview first with `--dry-run`. +# GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS=168 # age floor before something unused is eligible for pruning (7 days) # --- Observability: metrics + alerts + logs (#1206; requires --profile observability) --- # The observability profile starts Prometheus (scrapes /metrics) + Alertmanager (alert rules in diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index d0b823661d..34c0a4c2b7 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -7,7 +7,7 @@ export type SelfHostEnvReferenceRow = { export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ { name: "AI_COMBINE", - firstReference: "src/selfhost/ai.ts:848", + firstReference: "src/selfhost/ai.ts:888", }, { name: "AI_EMBED_API_KEY", @@ -19,11 +19,11 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "AI_EMBED_MODEL", - firstReference: "src/selfhost/ai.ts:744", + firstReference: "src/selfhost/ai.ts:784", }, { name: "AI_ON_MERGE", - firstReference: "src/selfhost/ai.ts:850", + firstReference: "src/selfhost/ai.ts:890", }, { name: "AI_PROVIDER", @@ -31,7 +31,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "ANTHROPIC_AI_BASE_URL", - firstReference: "src/selfhost/ai.ts:748", + firstReference: "src/selfhost/ai.ts:788", }, { name: "ANTHROPIC_AI_MODEL", @@ -39,7 +39,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "ANTHROPIC_API_KEY", - firstReference: "src/selfhost/ai.ts:747", + firstReference: "src/selfhost/ai.ts:787", }, { name: "BACKUP_ACKNOWLEDGED", @@ -131,11 +131,11 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "OLLAMA_AI_API_KEY", - firstReference: "src/selfhost/ai.ts:741", + firstReference: "src/selfhost/ai.ts:781", }, { name: "OLLAMA_AI_BASE_URL", - firstReference: "src/selfhost/ai.ts:737", + firstReference: "src/selfhost/ai.ts:777", }, { name: "OLLAMA_AI_MODEL", @@ -143,7 +143,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "OPENAI_AI_BASE_URL", - firstReference: "src/selfhost/ai.ts:739", + firstReference: "src/selfhost/ai.ts:779", }, { name: "OPENAI_AI_MODEL", @@ -151,15 +151,15 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "OPENAI_API_KEY", - firstReference: "src/selfhost/ai.ts:741", + firstReference: "src/selfhost/ai.ts:781", }, { name: "OPENAI_COMPATIBLE_AI_API_KEY", - firstReference: "src/selfhost/ai.ts:741", + firstReference: "src/selfhost/ai.ts:781", }, { name: "OPENAI_COMPATIBLE_AI_BASE_URL", - firstReference: "src/selfhost/ai.ts:740", + firstReference: "src/selfhost/ai.ts:780", }, { name: "OPENAI_COMPATIBLE_AI_MODEL", @@ -302,15 +302,15 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| Name | First reference |", "| --- | --- |", - "| `AI_COMBINE` | `src/selfhost/ai.ts:848` |", + "| `AI_COMBINE` | `src/selfhost/ai.ts:888` |", "| `AI_EMBED_API_KEY` | `src/server.ts:419` |", "| `AI_EMBED_BASE_URL` | `src/server.ts:416` |", - "| `AI_EMBED_MODEL` | `src/selfhost/ai.ts:744` |", - "| `AI_ON_MERGE` | `src/selfhost/ai.ts:850` |", + "| `AI_EMBED_MODEL` | `src/selfhost/ai.ts:784` |", + "| `AI_ON_MERGE` | `src/selfhost/ai.ts:890` |", "| `AI_PROVIDER` | `src/selfhost/ai-config.ts:43` |", - "| `ANTHROPIC_AI_BASE_URL` | `src/selfhost/ai.ts:748` |", + "| `ANTHROPIC_AI_BASE_URL` | `src/selfhost/ai.ts:788` |", "| `ANTHROPIC_AI_MODEL` | `src/selfhost/ai.ts:57` |", - "| `ANTHROPIC_API_KEY` | `src/selfhost/ai.ts:747` |", + "| `ANTHROPIC_API_KEY` | `src/selfhost/ai.ts:787` |", "| `BACKUP_ACKNOWLEDGED` | `src/server.ts:358` |", "| `BROWSER_WS_ENDPOINT` | `src/selfhost/stubs/puppeteer.ts:11` |", "| `CLAUDE_AI_EFFORT` | `src/selfhost/ai.ts:108` |", @@ -333,14 +333,14 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `MIGRATIONS_DIR` | `src/server.ts:371` |", "| `OBSERVABILITY_SMOKE_POLL_MS` | `scripts/smoke-observability-traces.mjs:8` |", "| `OBSERVABILITY_SMOKE_TIMEOUT_MS` | `scripts/smoke-observability-traces.mjs:6` |", - "| `OLLAMA_AI_API_KEY` | `src/selfhost/ai.ts:741` |", - "| `OLLAMA_AI_BASE_URL` | `src/selfhost/ai.ts:737` |", + "| `OLLAMA_AI_API_KEY` | `src/selfhost/ai.ts:781` |", + "| `OLLAMA_AI_BASE_URL` | `src/selfhost/ai.ts:777` |", "| `OLLAMA_AI_MODEL` | `src/selfhost/ai.ts:61` |", - "| `OPENAI_AI_BASE_URL` | `src/selfhost/ai.ts:739` |", + "| `OPENAI_AI_BASE_URL` | `src/selfhost/ai.ts:779` |", "| `OPENAI_AI_MODEL` | `src/selfhost/ai.ts:62` |", - "| `OPENAI_API_KEY` | `src/selfhost/ai.ts:741` |", - "| `OPENAI_COMPATIBLE_AI_API_KEY` | `src/selfhost/ai.ts:741` |", - "| `OPENAI_COMPATIBLE_AI_BASE_URL` | `src/selfhost/ai.ts:740` |", + "| `OPENAI_API_KEY` | `src/selfhost/ai.ts:781` |", + "| `OPENAI_COMPATIBLE_AI_API_KEY` | `src/selfhost/ai.ts:781` |", + "| `OPENAI_COMPATIBLE_AI_BASE_URL` | `src/selfhost/ai.ts:780` |", "| `OPENAI_COMPATIBLE_AI_MODEL` | `src/selfhost/ai.ts:63` |", "| `ORB_AIR_GAP` | `src/selfhost/orb-collector.ts:161` |", "| `ORB_ANONYMIZE` | `src/selfhost/orb-collector.ts:174` |", diff --git a/docker-compose.override.yml.example b/docker-compose.override.yml.example index 6976d6b23e..96b5605f1d 100644 --- a/docker-compose.override.yml.example +++ b/docker-compose.override.yml.example @@ -29,8 +29,9 @@ # This is the HOST-level (Docker CPU scheduling) half of the fix. The APPLICATION-level half is the # MAINTENANCE_ADMISSION_* knobs in .env.example: gittensory itself defers its own maintenance sweeps # (contributor evidence, RAG indexing, drift scans...) under queue/host pressure so live webhook/review -# work always wins there too. Also see ./scripts/docker-prune.sh for the disk-usage side of running CI -# builds on the same box (build cache/image growth, not just CPU). +# work always wins there too. Also see ./scripts/selfhost-docker-prune.sh (and its systemd timer under +# systemd/gittensory-docker-prune.*.example) for the disk-usage side of running CI builds on the same box +# (build cache/image growth, not just CPU). # The RECOMMENDED alternative to all of this: don't co-locate runners with the review stack at all -- use # GitHub-hosted CI, or a separate dedicated runner host (see the runner service's comment in # docker-compose.yml). diff --git a/docker-compose.yml b/docker-compose.yml index 19c4a768a8..273963cef0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -636,7 +636,7 @@ services: # app of CPU with no limits set (the default below) -- confirmed in production: 3 uncapped runner # containers on an 8-vCPU box left the app starved under load. See docker-compose.override.yml.example # for a proven CPU-priority pattern (relative cpu_shares + a per-container cpus ceiling) before scaling - # this up, and ./scripts/docker-prune.sh for reclaiming the disk space CI builds accumulate. + # this up, and ./scripts/selfhost-docker-prune.sh for reclaiming the disk space CI builds accumulate. # One-shot bootstrap for the runner's TMPDIR (below): guarantees /tmp/runner/tmp exists (and is # world-writable, matching real /tmp's mode -- the runner image's runtime user isn't guaranteed to be # root) on the runner-work volume BEFORE the runner container starts. A fresh named volume has nothing diff --git a/scripts/docker-prune.sh b/scripts/docker-prune.sh deleted file mode 100755 index f7118f87f6..0000000000 --- a/scripts/docker-prune.sh +++ /dev/null @@ -1,82 +0,0 @@ -#!/bin/sh -# Safe Docker disk hygiene for a self-host VPS (#selfhost-runtime-pressure). Build cache and unused images -# accumulate quickly on a box that rebuilds `gittensory` from source or runs GitHub Actions runners -- -# multi-GB `docker builder prune` growth per week is normal, and a root disk over ~80-85% full slows down -# EVERYTHING on the box (Postgres/SQLite fsync latency, container scheduling, log writes), not just Docker -# itself, well before it fills up completely. -# -# SAFE BY DESIGN: only prunes build cache, dangling/unused IMAGES, and stopped containers -- NEVER volumes -# (gittensory-data, gittensory-backups, postgres-data, qdrant-storage, etc.), so it cannot delete application -# data, backups, or vector-store state. Read-only by default (`--dry-run` -- or run without `--yes`, see -# below) so you can review what would be reclaimed before anything is deleted. -# -# Usage: -# ./scripts/docker-prune.sh # report current disk usage only, delete nothing -# ./scripts/docker-prune.sh --dry-run # same as above (explicit) -# ./scripts/docker-prune.sh --yes # actually prune (build cache + dangling images + stopped containers) -# ./scripts/docker-prune.sh --yes --images # also prune UNUSED (not just dangling) images -- more aggressive, -# # will re-pull/rebuild on next deploy if an image isn't running -# -# Cron example (weekly, Sunday 04:00, low-traffic window): -# 0 4 * * 0 cd /path/to/gittensory && ./scripts/docker-prune.sh --yes >> /var/log/gittensory-docker-prune.log 2>&1 -set -eu - -DRY_RUN=1 -PRUNE_UNUSED_IMAGES=0 -for arg in "$@"; do - case "$arg" in - --yes) DRY_RUN=0 ;; - --dry-run) DRY_RUN=1 ;; - --images) PRUNE_UNUSED_IMAGES=1 ;; - *) - echo "[docker-prune] unknown argument: $arg (expected --yes, --dry-run, and/or --images)" >&2 - exit 1 - ;; - esac -done - -if ! command -v docker >/dev/null 2>&1; then - echo "[docker-prune] docker not found on PATH" >&2 - exit 1 -fi - -echo "[docker-prune] disk usage before:" -docker system df - -echo "[docker-prune] root filesystem usage:" -df -h / 2>/dev/null || true - -if [ "$DRY_RUN" = 1 ]; then - echo "[docker-prune] DRY RUN (default) -- nothing will be deleted. Re-run with --yes to actually prune." - echo "[docker-prune] would run: docker builder prune -f" - echo "[docker-prune] would run: docker container prune -f" - if [ "$PRUNE_UNUSED_IMAGES" = 1 ]; then - echo "[docker-prune] would run: docker image prune -a -f" - else - echo "[docker-prune] would run: docker image prune -f (dangling only; pass --images for unused-but-tagged images too)" - fi - echo "[docker-prune] volumes are NEVER pruned by this script -- application data, backups, and vector-store state are always safe." - exit 0 -fi - -echo "[docker-prune] pruning build cache..." -docker builder prune -f - -echo "[docker-prune] pruning stopped containers..." -docker container prune -f - -if [ "$PRUNE_UNUSED_IMAGES" = 1 ]; then - echo "[docker-prune] pruning ALL unused images (not just dangling)..." - docker image prune -a -f -else - echo "[docker-prune] pruning dangling images..." - docker image prune -f -fi - -echo "[docker-prune] disk usage after:" -docker system df - -echo "[docker-prune] root filesystem usage:" -df -h / 2>/dev/null || true - -echo "[docker-prune] complete -- volumes were never touched." diff --git a/scripts/selfhost-docker-prune.sh b/scripts/selfhost-docker-prune.sh old mode 100644 new mode 100755 index cb3fe083fd..8e94e65d20 --- a/scripts/selfhost-docker-prune.sh +++ b/scripts/selfhost-docker-prune.sh @@ -6,17 +6,54 @@ # service comments on why raw docker.sock exposure into a container is avoided). # # Age-filtered so nothing built/pulled recently is touched -- a rollback within the retention window still -# has its image available. `docker image prune -a` and `docker builder prune` only ever remove resources -# Docker itself already reports as unused (a running container's own image, or an active build-cache entry -# a build is currently using, are never candidates) -- this script does not change that safety property, it -# only adds the age floor on top of it. +# has its image available. `docker container prune`, `docker image prune -a`, and `docker builder prune` +# only ever remove resources Docker itself already reports as unused (a running container, its own image, +# or an active build-cache entry a build is currently using, are never candidates) -- this script does not +# change that safety property, it only adds the age floor on top of it. +# +# SAFE BY DESIGN: only prunes stopped containers, unused images, and build cache -- NEVER volumes +# (gittensory-data, gittensory-backups, postgres-data, qdrant-storage, runner-work, etc.), so it cannot +# delete application data, backups, vector-store state, or a runner's registration/job data. +# +# Usage: +# sh scripts/selfhost-docker-prune.sh # prune for real -- the systemd timer's default call +# sh scripts/selfhost-docker-prune.sh --dry-run # preview only: report disk usage, delete nothing set -eu RETAIN_HOURS=${GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS:-168} # 7 days -echo "[docker-prune] $(date -u +%FT%TZ) starting (retain: ${RETAIN_HOURS}h)" +DRY_RUN=0 +for arg in "$@"; do + case "$arg" in + --dry-run) DRY_RUN=1 ;; + *) + echo "[docker-prune] unknown argument: $arg (expected --dry-run)" >&2 + exit 1 + ;; + esac +done + +if ! command -v docker >/dev/null 2>&1; then + echo "[docker-prune] docker not found on PATH" >&2 + exit 1 +fi + +echo "[docker-prune] $(date -u +%FT%TZ) starting (retain: ${RETAIN_HOURS}h, dry-run: ${DRY_RUN})" echo "[docker-prune] before:" docker system df +echo "[docker-prune] root filesystem usage:" +df -h / 2>/dev/null || true + +if [ "$DRY_RUN" = 1 ]; then + echo "[docker-prune] DRY RUN -- would run: docker container prune -f --filter until=${RETAIN_HOURS}h" + echo "[docker-prune] DRY RUN -- would run: docker image prune -af --filter until=${RETAIN_HOURS}h" + echo "[docker-prune] DRY RUN -- would run: docker builder prune -af --filter until=${RETAIN_HOURS}h" + echo "[docker-prune] volumes are NEVER pruned by this script -- application data, backups, and runner state are always safe." + exit 0 +fi + +echo "[docker-prune] pruning stopped containers older than ${RETAIN_HOURS}h..." +docker container prune -f --filter "until=${RETAIN_HOURS}h" echo "[docker-prune] pruning unused images older than ${RETAIN_HOURS}h..." docker image prune -af --filter "until=${RETAIN_HOURS}h" @@ -26,5 +63,7 @@ docker builder prune -af --filter "until=${RETAIN_HOURS}h" echo "[docker-prune] after:" docker system df +echo "[docker-prune] root filesystem usage:" +df -h / 2>/dev/null || true echo "[docker-prune] $(date -u +%FT%TZ) done" diff --git a/test/unit/selfhost-docker-prune-script.test.ts b/test/unit/selfhost-docker-prune-script.test.ts index 474b8b642a..8bcebc8614 100644 --- a/test/unit/selfhost-docker-prune-script.test.ts +++ b/test/unit/selfhost-docker-prune-script.test.ts @@ -29,9 +29,9 @@ function stubDocker(root: string): { logFile: string; binDir: string } { return { logFile, binDir }; } -function runPruneScript(root: string, env: Record = {}): string { +function runPruneScript(root: string, env: Record = {}, args: string[] = []): string { const { logFile, binDir } = stubDocker(root); - execFileSync("sh", ["scripts/selfhost-docker-prune.sh"], { + execFileSync("sh", ["scripts/selfhost-docker-prune.sh", ...args], { cwd: process.cwd(), env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}`, ...env }, }); @@ -43,13 +43,14 @@ afterEach(() => { }); describe("selfhost-docker-prune.sh", () => { - it("prunes images and build cache with the default 7-day (168h) age floor, never a blind full wipe", () => { + it("prunes stopped containers, images, and build cache with the default 7-day (168h) age floor, never a blind full wipe", () => { const calls = runPruneScript(tmpRoot()); + expect(calls).toContain("container prune -f --filter until=168h"); expect(calls).toContain("image prune -af --filter until=168h"); expect(calls).toContain("builder prune -af --filter until=168h"); - // Both prune calls must always carry an `until=` filter -- a bare `docker image prune -af` (no filter) - // would also remove something built moments ago, defeating the rollback-safety window. + // Every prune call must always carry an `until=` filter -- a bare `docker image prune -af` (no filter) + // would also remove something built/started moments ago, defeating the rollback-safety window. for (const line of calls.trim().split("\n")) { if (line.includes("prune")) expect(line).toMatch(/--filter until=\d+h/); } @@ -58,6 +59,7 @@ describe("selfhost-docker-prune.sh", () => { it("honors GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS to widen or narrow the safety window", () => { const calls = runPruneScript(tmpRoot(), { GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS: "24" }); + expect(calls).toContain("container prune -f --filter until=24h"); expect(calls).toContain("image prune -af --filter until=24h"); expect(calls).toContain("builder prune -af --filter until=24h"); expect(calls).not.toContain("168h"); @@ -67,9 +69,47 @@ describe("selfhost-docker-prune.sh", () => { const calls = runPruneScript(tmpRoot()); const invocations = calls.trim().split("\n"); - // "system df" (no prune flags) must appear before AND after the two prune calls, so an operator watching - // logs can see what was actually reclaimed. + // "system df" (no prune flags) must appear before AND after the three prune calls, so an operator + // watching logs can see what was actually reclaimed. const dfCalls = invocations.filter((line) => line === "system df"); expect(dfCalls).toHaveLength(2); }); + + it("--dry-run reports usage but issues no destructive prune call, and volumes are never touched by either mode", () => { + const calls = runPruneScript(tmpRoot(), {}, ["--dry-run"]); + const invocations = calls.trim().split("\n"); + + // Only the read-only "before" system df call -- no "after" (nothing was pruned to report on), and no + // container/image/builder prune invocation reached the real `docker` binary at all. + expect(invocations.filter((line) => line === "system df")).toHaveLength(1); + expect(calls).not.toMatch(/prune -f\b/); + expect(calls).not.toMatch(/prune -af\b/); + // Neither mode ever names a volume subcommand -- this script cannot delete application/backup/runner state. + expect(calls).not.toMatch(/\bvolume\b/); + }); + + it("--dry-run still honors GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS in its preview output", () => { + const root = tmpRoot(); + const { binDir } = stubDocker(root); + const stdout = execFileSync("sh", ["scripts/selfhost-docker-prune.sh", "--dry-run"], { + cwd: process.cwd(), + env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}`, GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS: "48" }, + }).toString(); + + expect(stdout).toContain("until=48h"); + expect(stdout).not.toContain("until=168h"); + }); + + it("rejects an unrecognized argument instead of silently ignoring it", () => { + const root = tmpRoot(); + const { binDir } = stubDocker(root); + + expect(() => + execFileSync("sh", ["scripts/selfhost-docker-prune.sh", "--nonsense"], { + cwd: process.cwd(), + env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}` }, + stdio: ["ignore", "ignore", "pipe"], + }), + ).toThrow(); + }); }); From 69cb9fcab6dfd047f386b1419c9e008f063ba852 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 3 Jul 2026 02:07:04 -0700 Subject: [PATCH 2/2] fix(selfhost): prune stopped containers by actual stop time, not creation time Docker's `container prune --filter until=h` filters by creation time, not stop time, so a long-lived container stopped moments ago was already eligible for deletion on the very next scheduled run. prune_stopped_containers() now inspects each exited container's State.FinishedAt directly, comparing it against a cutoff computed from epoch arithmetic and formatted portably for both GNU and BusyBox date, so only containers actually stopped for at least RETAIN_HOURS are ever removed. --- .../src/lib/selfhost-env-reference.ts | 60 ++--- scripts/selfhost-docker-prune.sh | 73 +++++- .../unit/selfhost-docker-prune-script.test.ts | 230 ++++++++++++++++-- 3 files changed, 307 insertions(+), 56 deletions(-) diff --git a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts index 34c0a4c2b7..37efbf514f 100644 --- a/apps/gittensory-ui/src/lib/selfhost-env-reference.ts +++ b/apps/gittensory-ui/src/lib/selfhost-env-reference.ts @@ -11,11 +11,11 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "AI_EMBED_API_KEY", - firstReference: "src/server.ts:419", + firstReference: "src/server.ts:420", }, { name: "AI_EMBED_BASE_URL", - firstReference: "src/server.ts:416", + firstReference: "src/server.ts:417", }, { name: "AI_EMBED_MODEL", @@ -43,7 +43,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "BACKUP_ACKNOWLEDGED", - firstReference: "src/server.ts:358", + firstReference: "src/server.ts:359", }, { name: "BROWSER_WS_ENDPOINT", @@ -75,11 +75,11 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "CRON_INTERVAL_MS", - firstReference: "src/server.ts:839", + firstReference: "src/server.ts:845", }, { name: "DATABASE_PATH", - firstReference: "src/server.ts:241", + firstReference: "src/server.ts:242", }, { name: "DATABASE_URL", @@ -103,11 +103,11 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "GITHUB_CACHE_TTL_SECONDS", - firstReference: "src/server.ts:487", + firstReference: "src/server.ts:488", }, { name: "GITTENSORY_REPO_CONFIG_DIR", - firstReference: "src/server.ts:275", + firstReference: "src/server.ts:276", }, { name: "GITTENSORY_VERSION", @@ -119,7 +119,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "MIGRATIONS_DIR", - firstReference: "src/server.ts:371", + firstReference: "src/server.ts:372", }, { name: "OBSERVABILITY_SMOKE_POLL_MS", @@ -179,7 +179,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "ORB_BROKER_URL", - firstReference: "src/server.ts:883", + firstReference: "src/server.ts:889", }, { name: "ORB_COLLECTOR_TOKEN", @@ -195,7 +195,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "ORB_RELAY_MODE", - firstReference: "src/server.ts:885", + firstReference: "src/server.ts:891", }, { name: "OTEL_EXPORTER_OTLP_ENDPOINT", @@ -227,11 +227,11 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "PGVECTOR_ENABLED", - firstReference: "src/server.ts:221", + firstReference: "src/server.ts:222", }, { name: "PORT", - firstReference: "src/server.ts:643", + firstReference: "src/server.ts:644", }, { name: "PUBLIC_API_ORIGIN", @@ -247,7 +247,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "QDRANT_URL", - firstReference: "src/server.ts:506", + firstReference: "src/server.ts:507", }, { name: "QUEUE_BACKGROUND_CONCURRENCY", @@ -259,7 +259,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "REVIEW_AUDIT_DIR", - firstReference: "src/server.ts:551", + firstReference: "src/server.ts:552", }, { name: "SELFHOST_BUNDLE_ALL", @@ -295,7 +295,7 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [ }, { name: "SETUP_OUTPUT_PATH", - firstReference: "src/server.ts:760", + firstReference: "src/server.ts:761", }, ]; @@ -303,15 +303,15 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| Name | First reference |", "| --- | --- |", "| `AI_COMBINE` | `src/selfhost/ai.ts:888` |", - "| `AI_EMBED_API_KEY` | `src/server.ts:419` |", - "| `AI_EMBED_BASE_URL` | `src/server.ts:416` |", + "| `AI_EMBED_API_KEY` | `src/server.ts:420` |", + "| `AI_EMBED_BASE_URL` | `src/server.ts:417` |", "| `AI_EMBED_MODEL` | `src/selfhost/ai.ts:784` |", "| `AI_ON_MERGE` | `src/selfhost/ai.ts:890` |", "| `AI_PROVIDER` | `src/selfhost/ai-config.ts:43` |", "| `ANTHROPIC_AI_BASE_URL` | `src/selfhost/ai.ts:788` |", "| `ANTHROPIC_AI_MODEL` | `src/selfhost/ai.ts:57` |", "| `ANTHROPIC_API_KEY` | `src/selfhost/ai.ts:787` |", - "| `BACKUP_ACKNOWLEDGED` | `src/server.ts:358` |", + "| `BACKUP_ACKNOWLEDGED` | `src/server.ts:359` |", "| `BROWSER_WS_ENDPOINT` | `src/selfhost/stubs/puppeteer.ts:11` |", "| `CLAUDE_AI_EFFORT` | `src/selfhost/ai.ts:108` |", "| `CLAUDE_AI_MODEL` | `src/selfhost/ai.ts:49` |", @@ -319,18 +319,18 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `CODEX_AI_EFFORT` | `src/selfhost/ai.ts:112` |", "| `CODEX_AI_MODEL` | `src/selfhost/ai.ts:53` |", "| `CODEX_AI_TIMEOUT_MS` | `src/selfhost/ai.ts:112` |", - "| `CRON_INTERVAL_MS` | `src/server.ts:839` |", - "| `DATABASE_PATH` | `src/server.ts:241` |", + "| `CRON_INTERVAL_MS` | `src/server.ts:845` |", + "| `DATABASE_PATH` | `src/server.ts:242` |", "| `DATABASE_URL` | `src/selfhost/preflight.ts:201` |", "| `DISCORD_REPO_WEBHOOKS` | `src/selfhost/discord-notify.ts:31` |", "| `DISCORD_WEBHOOK_URL` | `src/selfhost/discord-notify.ts:40` |", "| `GITHUB_APP_ID` | `src/selfhost/orb-collector.ts:59` |", "| `GITHUB_APP_PRIVATE_KEY` | `src/selfhost/orb-collector.ts:166` |", - "| `GITHUB_CACHE_TTL_SECONDS` | `src/server.ts:487` |", - "| `GITTENSORY_REPO_CONFIG_DIR` | `src/server.ts:275` |", + "| `GITHUB_CACHE_TTL_SECONDS` | `src/server.ts:488` |", + "| `GITTENSORY_REPO_CONFIG_DIR` | `src/server.ts:276` |", "| `GITTENSORY_VERSION` | `src/selfhost/health.ts:29` |", "| `MAINTENANCE_ADMISSION_ENABLED` | `src/selfhost/maintenance-admission.ts:83` |", - "| `MIGRATIONS_DIR` | `src/server.ts:371` |", + "| `MIGRATIONS_DIR` | `src/server.ts:372` |", "| `OBSERVABILITY_SMOKE_POLL_MS` | `scripts/smoke-observability-traces.mjs:8` |", "| `OBSERVABILITY_SMOKE_TIMEOUT_MS` | `scripts/smoke-observability-traces.mjs:6` |", "| `OLLAMA_AI_API_KEY` | `src/selfhost/ai.ts:781` |", @@ -345,11 +345,11 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `ORB_AIR_GAP` | `src/selfhost/orb-collector.ts:161` |", "| `ORB_ANONYMIZE` | `src/selfhost/orb-collector.ts:174` |", "| `ORB_APP_ID` | `src/selfhost/orb-collector.ts:59` |", - "| `ORB_BROKER_URL` | `src/server.ts:883` |", + "| `ORB_BROKER_URL` | `src/server.ts:889` |", "| `ORB_COLLECTOR_TOKEN` | `src/selfhost/orb-collector.ts:205` |", "| `ORB_COLLECTOR_URL` | `src/selfhost/orb-collector.ts:172` |", "| `ORB_ENROLLMENT_SECRET` | `src/selfhost/orb-collector.ts:165` |", - "| `ORB_RELAY_MODE` | `src/server.ts:885` |", + "| `ORB_RELAY_MODE` | `src/server.ts:891` |", "| `OTEL_EXPORTER_OTLP_ENDPOINT` | `src/selfhost/otel.ts:47` |", "| `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` | `src/selfhost/otel.ts:45` |", "| `OTEL_SERVICE_ENVIRONMENT` | `src/selfhost/otel.ts:60` |", @@ -357,15 +357,15 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `OTEL_TRACES_EXPORTER` | `src/selfhost/otel.ts:40` |", "| `OTEL_TRACES_SAMPLER` | `src/selfhost/otel.ts:74` |", "| `OTEL_TRACES_SAMPLER_ARG` | `src/selfhost/otel.ts:76` |", - "| `PGVECTOR_ENABLED` | `src/server.ts:221` |", - "| `PORT` | `src/server.ts:643` |", + "| `PGVECTOR_ENABLED` | `src/server.ts:222` |", + "| `PORT` | `src/server.ts:644` |", "| `PUBLIC_API_ORIGIN` | `src/selfhost/preflight.ts:192` |", "| `QDRANT_API_KEY` | `src/selfhost/qdrant-vectorize.ts:50` |", "| `QDRANT_DIM` | `src/selfhost/qdrant-vectorize.ts:71` |", - "| `QDRANT_URL` | `src/server.ts:506` |", + "| `QDRANT_URL` | `src/server.ts:507` |", "| `QUEUE_BACKGROUND_CONCURRENCY` | `src/selfhost/queue-common.ts:102` |", "| `REDIS_URL` | `src/selfhost/preflight.ts:144` |", - "| `REVIEW_AUDIT_DIR` | `src/server.ts:551` |", + "| `REVIEW_AUDIT_DIR` | `src/server.ts:552` |", "| `SELFHOST_BUNDLE_ALL` | `scripts/build-selfhost.mjs:13` |", "| `SELFHOST_SERVICE` | `scripts/smoke-observability-traces.mjs:5` |", "| `SELFHOST_SETUP_TOKEN` | `src/selfhost/preflight.ts:186` |", @@ -374,5 +374,5 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [ "| `SENTRY_RELEASE` | `src/selfhost/otel.ts:62` |", "| `SENTRY_SERVER_NAME` | `src/selfhost/sentry.ts:373` |", "| `SENTRY_TRACES_SAMPLE_RATE` | `src/selfhost/sentry.ts:161` |", - "| `SETUP_OUTPUT_PATH` | `src/server.ts:760` |", + "| `SETUP_OUTPUT_PATH` | `src/server.ts:761` |", ].join("\n"); diff --git a/scripts/selfhost-docker-prune.sh b/scripts/selfhost-docker-prune.sh index 8e94e65d20..7bf53dda08 100755 --- a/scripts/selfhost-docker-prune.sh +++ b/scripts/selfhost-docker-prune.sh @@ -6,10 +6,20 @@ # service comments on why raw docker.sock exposure into a container is avoided). # # Age-filtered so nothing built/pulled recently is touched -- a rollback within the retention window still -# has its image available. `docker container prune`, `docker image prune -a`, and `docker builder prune` -# only ever remove resources Docker itself already reports as unused (a running container, its own image, -# or an active build-cache entry a build is currently using, are never candidates) -- this script does not -# change that safety property, it only adds the age floor on top of it. +# has its image available. `docker image prune -a` and `docker builder prune` only ever remove resources +# Docker itself already reports as unused (a running container's own image, or an active build-cache entry +# a build is currently using, are never candidates) -- this script does not change that safety property, it +# only adds the age floor on top of it. Docker's own `until=` filter for both is CREATION time, which is +# fine there: an in-use image/cache entry can never be a prune candidate in the first place regardless of +# how old it is, so the age floor only ever protects a recently built one that isn't in use yet. +# +# Containers are handled differently and deliberately do NOT use `docker container prune --filter until=`: +# that filter is ALSO creation time, not stop time (verified against Docker's own docs) -- a long-running +# container stopped moments ago (e.g. an operator's `docker compose stop ` to inspect a live issue) +# would already be older than the retention window by creation date, so a creation-time filter deletes it +# on the very next scheduled run instead of giving the intended grace period. prune_stopped_containers() +# below inspects each exited container's ACTUAL State.FinishedAt instead, so only a container that has +# itself been stopped for at least RETAIN_HOURS is ever removed. # # SAFE BY DESIGN: only prunes stopped containers, unused images, and build cache -- NEVER volumes # (gittensory-data, gittensory-backups, postgres-data, qdrant-storage, runner-work, etc.), so it cannot @@ -38,23 +48,70 @@ if ! command -v docker >/dev/null 2>&1; then exit 1 fi +# Prunes (real mode) or reports (dry-run) stopped containers whose ACTUAL stop time is at least +# RETAIN_HOURS in the past -- see the header comment for why `docker container prune --filter until=` +# itself is not used. Walks each exited container's `docker inspect` State.FinishedAt individually. +# +# Deliberately never parses FinishedAt's ISO-8601 string with `date -d` -- GNU coreutils' `date -d` accepts +# it, but BusyBox `date` (Alpine and other minimal Linux hosts, a common lightweight Docker host choice) +# only understands a small fixed set of `-d` inputs (`@`, `hh:mm[:ss]`, `YYYY-MM-DD hh:mm[:ss]`, ...) +# and rejects FinishedAt's fractional-second form outright. Because that failure was wrapped in +# `2>/dev/null || continue`, it used to fail SILENTLY -- every container would be skipped forever, with no +# error, defeating this feature's whole purpose on a BusyBox host without so much as a warning. +# +# Instead: format the cutoff (an EPOCH INTEGER computed by plain arithmetic, never parsed from a string) as +# an ISO-8601-prefix string via `date -d @` -- the `@` form IS in BusyBox's small supported +# set, unlike arbitrary ISO-8601 parsing -- then compare that against FinishedAt's own first-19-characters +# prefix LEXICOGRAPHICALLY. That comparison is chronologically correct because both sides are the same +# fixed-width, zero-padded, UTC "YYYY-MM-DDTHH:MM:SS" shape, and needs no date-string PARSING at all. +prune_stopped_containers() { + now_epoch=$(date -u +%s) + cutoff_epoch=$((now_epoch - RETAIN_HOURS * 3600)) + cutoff_iso=$(date -u -d "@${cutoff_epoch}" +%Y-%m-%dT%H:%M:%S) + container_list=$(mktemp) + docker ps -a --filter status=exited --format '{{.ID}}' > "$container_list" + # Reads from a FILE (not a pipe) deliberately: `cmd | while read; do ...; done` runs the loop body in a + # subshell under POSIX sh, silently discarding any variable set inside it once the loop ends -- harmless + # today (nothing here is read after the loop), but a real trap for a future edit that adds e.g. a + # removed-count summary. `done < file` has no such subshell. + while IFS= read -r cid; do + [ -n "$cid" ] || continue + finished_at=$(docker inspect -f '{{.State.FinishedAt}}' "$cid" 2>/dev/null) || continue + finished_prefix=$(printf '%s' "$finished_at" | cut -c1-19) + case "$finished_prefix" in + [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]T[0-9][0-9]:[0-9][0-9]:[0-9][0-9]) ;; + *) continue ;; # not a recognizable timestamp shape -- skip rather than guess + esac + # `[ a \< b ]` is not defined by POSIX test for strings; `expr` IS specified to compare non-numeric + # operands lexicographically, so it's used here for a portable string-ordering check. + expr "$finished_prefix" '<' "$cutoff_iso" >/dev/null || continue + if [ "$DRY_RUN" = 1 ]; then + echo "[docker-prune] DRY RUN -- would remove stopped container $cid (stopped before ${cutoff_iso}Z)" + elif docker rm "$cid" >/dev/null 2>&1; then + echo "[docker-prune] removed stopped container $cid (stopped before ${cutoff_iso}Z)" + else + echo "[docker-prune] WARNING: failed to remove stopped container $cid" >&2 + fi + done < "$container_list" + rm -f "$container_list" +} + echo "[docker-prune] $(date -u +%FT%TZ) starting (retain: ${RETAIN_HOURS}h, dry-run: ${DRY_RUN})" echo "[docker-prune] before:" docker system df echo "[docker-prune] root filesystem usage:" df -h / 2>/dev/null || true +echo "[docker-prune] pruning stopped containers older than ${RETAIN_HOURS}h (by actual stop time)..." +prune_stopped_containers + if [ "$DRY_RUN" = 1 ]; then - echo "[docker-prune] DRY RUN -- would run: docker container prune -f --filter until=${RETAIN_HOURS}h" echo "[docker-prune] DRY RUN -- would run: docker image prune -af --filter until=${RETAIN_HOURS}h" echo "[docker-prune] DRY RUN -- would run: docker builder prune -af --filter until=${RETAIN_HOURS}h" echo "[docker-prune] volumes are NEVER pruned by this script -- application data, backups, and runner state are always safe." exit 0 fi -echo "[docker-prune] pruning stopped containers older than ${RETAIN_HOURS}h..." -docker container prune -f --filter "until=${RETAIN_HOURS}h" - echo "[docker-prune] pruning unused images older than ${RETAIN_HOURS}h..." docker image prune -af --filter "until=${RETAIN_HOURS}h" diff --git a/test/unit/selfhost-docker-prune-script.test.ts b/test/unit/selfhost-docker-prune-script.test.ts index 8bcebc8614..76153ad044 100644 --- a/test/unit/selfhost-docker-prune-script.test.ts +++ b/test/unit/selfhost-docker-prune-script.test.ts @@ -12,25 +12,109 @@ function tmpRoot(): string { return dir; } -// Stubs `docker` on PATH with a fake binary that just records every invocation's arguments (one line per -// call) instead of touching a real Docker daemon -- the self-hosted runner this suite actually runs on has -// no Docker-in-Docker access, so a test that shells out to a real `docker image prune` would be -// unreliable/environment-dependent (same constraint as the compose-file structural tests). -function stubDocker(root: string): { logFile: string; binDir: string } { +type ContainerFixture = { + id: string; + /** ISO timestamp for docker inspect's State.FinishedAt. */ + finishedAt: string; + /** When true, `docker rm ` exits non-zero (simulates a removal failure/race). */ + failRemove?: boolean; +}; + +// Stubs `docker` on PATH with a fake binary that records every invocation's arguments (one line per call) +// instead of touching a real Docker daemon -- the self-hosted runner this suite actually runs on has no +// Docker-in-Docker access, so a test that shells out to a real `docker image prune`/`docker ps` would be +// unreliable/environment-dependent (same constraint as the compose-file structural tests). `containers` +// fixtures let a test simulate specific exited containers with specific real stop times (docker inspect's +// State.FinishedAt) -- this is what actually exercises prune_stopped_containers()'s stop-time logic, +// distinct from the generic "TYPE TOTAL SIZE RECLAIMABLE" fallback used for every other docker subcommand. +function stubDocker(root: string, containers: ContainerFixture[] = []): { logFile: string; binDir: string } { const binDir = join(root, "bin"); mkdirSync(binDir, { recursive: true }); const logFile = join(root, "docker-calls.log"); + const fixturesFile = join(root, "containers.tsv"); + writeFileSync( + fixturesFile, + containers.map((c) => `${c.id}\t${c.finishedAt}\t${c.failRemove ? "1" : "0"}`).join("\n"), + ); writeFileSync( join(binDir, "docker"), - ["#!/bin/sh", `echo "$@" >> "${logFile}"`, "echo 'TYPE TOTAL SIZE RECLAIMABLE'", "exit 0", ""].join("\n"), + [ + "#!/bin/sh", + `echo "$@" >> "${logFile}"`, + `FIXTURES="${fixturesFile}"`, + // `docker ps -a --filter status=exited --format '{{.ID}}'` -> one container id per line. + 'if [ "$1 $2" = "ps -a" ]; then', + ' awk -F"\\t" \'{print $1}\' "$FIXTURES"', + " exit 0", + "fi", + // `docker inspect -f '{{.State.FinishedAt}}' ` -> that container's fixture FinishedAt. + 'if [ "$1" = "inspect" ]; then', + ' id="$4"', + ' awk -F"\\t" -v id="$id" \'$1==id{print $2}\' "$FIXTURES"', + " exit 0", + "fi", + // `docker rm ` -> succeeds unless the fixture marks it as a forced failure. + 'if [ "$1" = "rm" ]; then', + ' id="$2"', + ' fail=$(awk -F"\\t" -v id="$id" \'$1==id{print $3}\' "$FIXTURES")', + ' [ "$fail" = "1" ] && exit 1', + " exit 0", + "fi", + "echo 'TYPE TOTAL SIZE RECLAIMABLE'", + "exit 0", + "", + ].join("\n"), { mode: 0o755 }, ); chmodSync(join(binDir, "docker"), 0o755); + stubDate(binDir); return { logFile, binDir }; } -function runPruneScript(root: string, env: Record = {}, args: string[] = []): string { - const { logFile, binDir } = stubDocker(root); +// The script under test deliberately avoids GNU-coreutils-only `date -d ` (that's exactly +// the portability bug this fix corrects -- BusyBox `date` can't parse it), using only two forms instead: +// `date -u +%s` (current epoch; already identical across GNU/BSD/BusyBox) and `date -u -d "@" +FORMAT` +// (format a KNOWN-GOOD epoch; BusyBox supports the `@` input, but this repo's CI/dev hosts are a mix +// of GNU coreutils and macOS/BSD date, which does NOT support `-d` at all). Stubbing `date` with a +// deterministic python3-backed fake -- instead of relying on whichever `date` the test happens to run on -- +// lets this suite exercise the real GNU/BusyBox-compatible code path even on a macOS dev machine, the same +// way `docker` itself is stubbed rather than requiring a real daemon. +function stubDate(binDir: string): void { + writeFileSync( + join(binDir, "date"), + [ + "#!/bin/sh", + 'if [ "$1 $2" = "-u +%s" ]; then', + ' python3 -c "import time; print(int(time.time()))"', + " exit 0", + "fi", + 'if [ "$1 $2" = "-u -d" ]; then', + ' epoch="${3#@}"', + ' fmt="${4#+}"', + ' python3 -W ignore::DeprecationWarning -c "import sys, datetime; print(datetime.datetime.utcfromtimestamp(int(sys.argv[1])).strftime(sys.argv[2]))" "$epoch" "$fmt"', + " exit 0", + "fi", + // Only used for the human-readable "starting"/"done" log lines -- no test asserts on the exact value. + "python3 -W ignore::DeprecationWarning -c \"import datetime; print(datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ'))\"", + "exit 0", + "", + ].join("\n"), + { mode: 0o755 }, + ); + chmodSync(join(binDir, "date"), 0o755); +} + +function isoHoursAgo(hours: number): string { + return new Date(Date.now() - hours * 3600_000).toISOString(); +} + +function runPruneScript( + root: string, + env: Record = {}, + args: string[] = [], + containers: ContainerFixture[] = [], +): string { + const { logFile, binDir } = stubDocker(root, containers); execFileSync("sh", ["scripts/selfhost-docker-prune.sh", ...args], { cwd: process.cwd(), env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}`, ...env }, @@ -38,28 +122,47 @@ function runPruneScript(root: string, env: Record = {}, args: st return readFileSync(logFile, "utf8"); } +/** Like runPruneScript, but also returns the script's own combined stdout+stderr (for assertions on the + * human-readable log lines, e.g. "removed stopped container ..." on stdout / "WARNING: ..." on stderr), + * not just the raw docker-call log. Merges stderr into stdout via shell redirection since + * execFileSync's return value only ever captures one stream. */ +function runPruneScriptCapturingOutput( + root: string, + env: Record = {}, + args: string[] = [], + containers: ContainerFixture[] = [], +): { calls: string; output: string } { + const { logFile, binDir } = stubDocker(root, containers); + const quotedArgs = args.map((a) => `'${a.replace(/'/g, "'\\''")}'`).join(" "); + const output = execFileSync("sh", ["-c", `sh scripts/selfhost-docker-prune.sh ${quotedArgs} 2>&1`], { + cwd: process.cwd(), + env: { ...process.env, PATH: `${binDir}:${process.env.PATH ?? ""}`, ...env }, + }).toString(); + return { calls: readFileSync(logFile, "utf8"), output }; +} + afterEach(() => { for (const dir of tmpRoots.splice(0)) rmSync(dir, { force: true, recursive: true }); }); describe("selfhost-docker-prune.sh", () => { - it("prunes stopped containers, images, and build cache with the default 7-day (168h) age floor, never a blind full wipe", () => { + it("prunes images and build cache with the default 7-day (168h) age floor, never a blind full wipe", () => { const calls = runPruneScript(tmpRoot()); - expect(calls).toContain("container prune -f --filter until=168h"); expect(calls).toContain("image prune -af --filter until=168h"); expect(calls).toContain("builder prune -af --filter until=168h"); - // Every prune call must always carry an `until=` filter -- a bare `docker image prune -af` (no filter) - // would also remove something built/started moments ago, defeating the rollback-safety window. + // Every image/builder prune call must always carry an `until=` filter -- a bare `docker image prune -af` + // (no filter) would also remove something built moments ago, defeating the rollback-safety window. for (const line of calls.trim().split("\n")) { - if (line.includes("prune")) expect(line).toMatch(/--filter until=\d+h/); + if (line.includes("image prune") || line.includes("builder prune")) expect(line).toMatch(/--filter until=\d+h/); } + // Never the old creation-time container filter -- that is exactly the defect this script now avoids. + expect(calls).not.toContain("container prune"); }); it("honors GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS to widen or narrow the safety window", () => { const calls = runPruneScript(tmpRoot(), { GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS: "24" }); - expect(calls).toContain("container prune -f --filter until=24h"); expect(calls).toContain("image prune -af --filter until=24h"); expect(calls).toContain("builder prune -af --filter until=24h"); expect(calls).not.toContain("168h"); @@ -69,8 +172,8 @@ describe("selfhost-docker-prune.sh", () => { const calls = runPruneScript(tmpRoot()); const invocations = calls.trim().split("\n"); - // "system df" (no prune flags) must appear before AND after the three prune calls, so an operator - // watching logs can see what was actually reclaimed. + // "system df" (no prune flags) must appear before AND after the prune steps, so an operator watching + // logs can see what was actually reclaimed. const dfCalls = invocations.filter((line) => line === "system df"); expect(dfCalls).toHaveLength(2); }); @@ -80,9 +183,8 @@ describe("selfhost-docker-prune.sh", () => { const invocations = calls.trim().split("\n"); // Only the read-only "before" system df call -- no "after" (nothing was pruned to report on), and no - // container/image/builder prune invocation reached the real `docker` binary at all. + // image/builder prune invocation reached the real `docker` binary at all. expect(invocations.filter((line) => line === "system df")).toHaveLength(1); - expect(calls).not.toMatch(/prune -f\b/); expect(calls).not.toMatch(/prune -af\b/); // Neither mode ever names a volume subcommand -- this script cannot delete application/backup/runner state. expect(calls).not.toMatch(/\bvolume\b/); @@ -112,4 +214,96 @@ describe("selfhost-docker-prune.sh", () => { }), ).toThrow(); }); + + describe("stopped-container pruning uses ACTUAL stop time, not creation time (#gate-review docker-prune-2721)", () => { + it("removes a container that has genuinely been stopped longer than the retention window", () => { + const { calls, output } = runPruneScriptCapturingOutput( + tmpRoot(), + {}, + [], + [{ id: "old-stopped", finishedAt: isoHoursAgo(200) }], + ); + + expect(calls).toContain("rm old-stopped"); + expect(output).toContain("removed stopped container old-stopped"); + }); + + it("REGRESSION: leaves a container alone that was created long ago but stopped moments ago -- the exact defect the old creation-time filter had", () => { + // A long-lived container (irrelevant when it was CREATED) that exited 5 minutes ago must survive a + // 168h-retention run: the old `docker container prune --filter until=168h` would have deleted this + // immediately because that filter keys off creation time, not stop time. + const root = tmpRoot(); + const calls = runPruneScript(root, {}, [], [{ id: "just-stopped", finishedAt: isoHoursAgo(0.08) }]); + + expect(calls).not.toContain("rm just-stopped"); + }); + + it("does not remove a container stopped just under the retention window, and does remove one stopped just over it", () => { + const root = tmpRoot(); + const calls = runPruneScript( + root, + { GITTENSORY_DOCKER_PRUNE_RETAIN_HOURS: "24" }, + [], + [ + { id: "just-under", finishedAt: isoHoursAgo(23) }, + { id: "just-over", finishedAt: isoHoursAgo(25) }, + ], + ); + + expect(calls).not.toContain("rm just-under"); + expect(calls).toContain("rm just-over"); + }); + + it("--dry-run reports which stopped containers WOULD be removed, without ever calling docker rm", () => { + const { calls, output } = runPruneScriptCapturingOutput( + tmpRoot(), + {}, + ["--dry-run"], + [{ id: "would-remove", finishedAt: isoHoursAgo(200) }], + ); + + expect(output).toContain("DRY RUN -- would remove stopped container would-remove"); + expect(calls).not.toContain("rm would-remove"); + }); + + it("does nothing (no error, no removal) when there are no exited containers at all", () => { + const calls = runPruneScript(tmpRoot(), {}, [], []); + expect(calls).not.toMatch(/\brm\b/); + }); + + it("warns but does not abort the rest of the run when a container's removal itself fails", () => { + const { output } = runPruneScriptCapturingOutput( + tmpRoot(), + {}, + [], + [{ id: "fails-to-remove", finishedAt: isoHoursAgo(200), failRemove: true }], + ); + + expect(output).toContain("WARNING: failed to remove stopped container fails-to-remove"); + // The run must still reach and report the image/build-cache steps afterward -- one failed removal + // must never abort the rest of the hygiene pass. + expect(output).toContain("pruning unused images"); + expect(output).toContain("pruning build cache"); + }); + + it("handles multiple exited containers independently, removing only the ones past the retention window", () => { + const root = tmpRoot(); + const calls = runPruneScript( + root, + {}, + [], + [ + { id: "keep-1", finishedAt: isoHoursAgo(1) }, + { id: "remove-1", finishedAt: isoHoursAgo(500) }, + { id: "keep-2", finishedAt: isoHoursAgo(10) }, + { id: "remove-2", finishedAt: isoHoursAgo(1000) }, + ], + ); + + expect(calls).toContain("rm remove-1"); + expect(calls).toContain("rm remove-2"); + expect(calls).not.toContain("rm keep-1"); + expect(calls).not.toContain("rm keep-2"); + }); + }); });