Skip to content

Commit a8d168d

Browse files
authored
Merge pull request #5977 from JSONbored/fix/selfhost-config-dir-empty-guardrail
feat(selfhost): warn loudly when the private config mount is empty
2 parents 68fac24 + 6b97ffe commit a8d168d

8 files changed

Lines changed: 135 additions & 6 deletions

File tree

apps/loopover-ui/src/lib/selfhost-env-reference.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
129129
name: "CODEX_HOME",
130130
firstReference: "src/selfhost/ai.ts",
131131
},
132+
{
133+
name: "CONFIG_DIR_EMPTY_ACKNOWLEDGED",
134+
firstReference: "src/server.ts",
135+
},
132136
{
133137
name: "CRON_INTERVAL_MS",
134138
firstReference: "src/server.ts",
@@ -513,6 +517,7 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [
513517
"| `CODEX_AI_MODEL` | `src/selfhost/ai.ts` |",
514518
"| `CODEX_AI_TIMEOUT_MS` | `src/selfhost/ai.ts` |",
515519
"| `CODEX_HOME` | `src/selfhost/ai.ts` |",
520+
"| `CONFIG_DIR_EMPTY_ACKNOWLEDGED` | `src/server.ts` |",
516521
"| `CRON_INTERVAL_MS` | `src/server.ts` |",
517522
"| `DATABASE_PATH` | `src/server.ts` |",
518523
"| `DATABASE_URL` | `src/selfhost/preflight.ts` |",

apps/loopover-ui/src/routes/docs.self-hosting-operations.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1142,7 +1142,8 @@ git merge --ff-only origin/main
11421142
curl -sf http://localhost:8787/ready
11431143
docker compose ps loopover
11441144
grep -E '^(LOOPOVER_IMAGE|LOOPOVER_VERSION|SENTRY_RELEASE)=' .env
1145-
docker inspect --format '{{.Config.Image}}' "$(docker compose ps -q loopover)"`}
1145+
docker inspect --format '{{.Config.Image}}' "$(docker compose ps -q loopover)"
1146+
docker exec "$(docker compose ps -q loopover)" sh -c 'ls -A "\${LOOPOVER_REPO_CONFIG_DIR:-/config}" | wc -l'`}
11461147
/>
11471148
<p>
11481149
If any check fails, see <Link to="/docs/self-hosting-troubleshooting">Troubleshooting</Link>

scripts/selfhost-post-update-check.sh

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
# Run after deploy-selfhost-image.sh, deploy-selfhost-prebuilt.sh, or any manual
55
# `docker compose up -d --no-deps loopover` that ships a new app image.
66
#
7-
# Checks /ready, compose health, .env release metadata, and the running container image.
7+
# Checks /ready, compose health, .env release metadata, the running container image, and whether the
8+
# container-private config mount (LOOPOVER_REPO_CONFIG_DIR) is unexpectedly empty.
89
# Does not modify .env, volumes, loopover-config/, or any profile service.
910
set -euo pipefail
1011

@@ -93,4 +94,18 @@ fi
9394

9495
running_image="$(docker inspect --format '{{.Config.Image}}' "$container_id")"
9596
echo "selfhost post-update check: running image=$running_image"
97+
98+
# Config-drift guardrail (a live incident during the gittensory->loopover rename): docker-compose.yml's
99+
# LOOPOVER_REPO_CONFIG_DIR bind mount silently degrades to an empty directory -- not an error -- when its host
100+
# source directory doesn't exist (e.g. renamed/moved without updating the mount, or simply never created). Every
101+
# per-repo and global setting then falls back to built-in defaults with zero visible symptoms until someone
102+
# notices the behavior change. This is a READ-ONLY check (matches the file header above: never modifies
103+
# loopover-config/) run every time this script runs, i.e. after every deploy -- exactly when a mount-path change
104+
# would land. Non-fatal: an empty mount is also the correct, expected state for a fresh install with no private
105+
# config written yet, so this warns rather than exits non-zero.
106+
config_dir_entries="$(docker exec "$container_id" sh -c 'dir="${LOOPOVER_REPO_CONFIG_DIR:-/config}"; [ -d "$dir" ] && ls -A "$dir" | wc -l || echo 0' 2>/dev/null || true)"
107+
if [[ "$config_dir_entries" =~ ^[0-9]+$ ]] && [ "$config_dir_entries" -eq 0 ]; then
108+
echo "selfhost post-update check: warning — the container's private config directory (LOOPOVER_REPO_CONFIG_DIR, default /config) is empty; every per-repo and global setting is silently using built-in defaults. If you expected private config to apply, verify the host directory wasn't renamed or moved without updating docker-compose.yml's bind mount." >&2
109+
fi
110+
96111
echo "selfhost post-update check: ok"

src/selfhost/health.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,3 +231,26 @@ export function publicOriginAcknowledgedGaugeValue(opts: {
231231
}): 0 | 1 {
232232
return publicOriginReachabilityAdvisory(opts) === null ? 1 : 0;
233233
}
234+
235+
/** Boot-time advisory (a live incident during the gittensory->loopover rename): `LOOPOVER_REPO_CONFIG_DIR`
236+
* points the focus-manifest loader at a container-private per-repo config mount (`private-config.ts`) that
237+
* silently and validly degrades to "no local config" when the mounted directory is empty — every setting
238+
* (labels, gate, autonomy, ...) then falls back to built-in defaults with NO error, because an empty mount is
239+
* also the correct, expected state for a brand-new install that hasn't written any `.loopover.yml` yet. The
240+
* incident: a docker-compose.yml change renamed the bind-mount source directory convention
241+
* (`./gittensory-config` -> `./loopover-config`) as a documented breaking change requiring operators to `mv`
242+
* their existing directory to match — that manual step was missed on deploy, Docker silently created an empty
243+
* directory at the new path, and every repo's config-driven settings (including `autoLabelEnabled`) reverted
244+
* to defaults for about a day before anyone noticed. Mirrors {@link sqliteBackupAdvisory}'s shape: warns
245+
* rather than blocks (an empty dir is legitimate for a fresh install), and the operator can silence it with
246+
* `CONFIG_DIR_EMPTY_ACKNOWLEDGED=true` once they've confirmed it's intentional. */
247+
export function emptyConfigDirAdvisory(opts: { configured: boolean; entryCount: number; acknowledged: boolean }): string | null {
248+
if (!opts.configured || opts.acknowledged || opts.entryCount > 0) return null;
249+
return `LOOPOVER_REPO_CONFIG_DIR is set but the mounted directory is empty — every per-repo and global setting (labels, gate, autonomy, ...) is silently using built-in defaults instead of your .loopover.yml config. This usually means the host directory was renamed or moved without updating the bind mount (see docker-compose.yml's "volumes:" comment), or the volume didn't mount as expected. If this is intentional — a fresh install with no config written yet — set CONFIG_DIR_EMPTY_ACKNOWLEDGED=true to silence this warning.`;
250+
}
251+
252+
/** Prometheus gauge value mirroring {@link emptyConfigDirAdvisory}: 1 when the mount isn't configured, has
253+
* entries, or the operator acknowledged it, 0 when the advisory would fire. */
254+
export function emptyConfigDirAcknowledgedGaugeValue(opts: { configured: boolean; entryCount: number; acknowledged: boolean }): 0 | 1 {
255+
return emptyConfigDirAdvisory(opts) === null ? 1 : 0;
256+
}

src/selfhost/metrics.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [
5454
["loopover_clock_skew_seconds", { help: "Clock skew in seconds between this process and GitHub's server time (positive = ahead), sampled from GitHub App JWT-mint response Date headers.", type: "gauge" }],
5555
["loopover_uptime_seconds", { help: "Self-host process uptime in seconds.", type: "gauge" }],
5656
["loopover_backup_acknowledged", { help: "1 when SQLite backup is acknowledged or Postgres is in use; 0 when the boot backup advisory would fire.", type: "gauge" }],
57+
["loopover_config_dir_empty_acknowledged", { help: "1 when LOOPOVER_REPO_CONFIG_DIR is unset, has entries, or is acknowledged; 0 when it's configured but the mounted directory is empty.", type: "gauge" }],
5758
["loopover_http_requests_total", { help: "HTTP app requests by response status class.", type: "counter" }],
5859
["loopover_http_request_duration_seconds", { help: "HTTP app request duration in seconds.", type: "histogram" }],
5960
["loopover_webhook_dedup_total", { help: "Webhook deliveries deduplicated before enqueue.", type: "counter" }],

src/server.ts

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// Serves the Hono app via @hono/node-server, drives the queue with the same processJob, ticks the same
77
// scheduled handler on a timer, exposes /health /ready /metrics, and shuts down gracefully. The Cloudflare
88
// Worker (src/index.ts) is untouched — this is a parallel entry the self-host esbuild build bundles.
9-
import { existsSync, writeFileSync } from "node:fs";
9+
import { existsSync, readdirSync, writeFileSync } from "node:fs";
1010
import { delimiter, join } from "node:path";
1111
import { randomUUID } from "node:crypto";
1212
import { DatabaseSync } from "node:sqlite";
@@ -48,6 +48,8 @@ import {
4848
backupAcknowledgedGaugeValue,
4949
buildHealthBody,
5050
codexAuthReadinessProbe,
51+
emptyConfigDirAcknowledgedGaugeValue,
52+
emptyConfigDirAdvisory,
5153
githubAppReadinessProbe,
5254
publicOriginAcknowledgedGaugeValue,
5355
publicOriginReachabilityAdvisory,
@@ -113,6 +115,16 @@ function nonBlank(value: string | undefined): string | undefined {
113115
return trimmed ? trimmed : undefined;
114116
}
115117

118+
/** Top-level entry count of `dir` (files + subdirectories, dotfiles included), or 0 on any read error --
119+
* a missing/unreadable directory is reported the same as an empty one rather than crashing boot. */
120+
function safeReaddirCount(dir: string): number {
121+
try {
122+
return readdirSync(dir).length;
123+
} catch {
124+
return 0;
125+
}
126+
}
127+
116128

117129
interface Backend {
118130
db: D1Database;
@@ -298,15 +310,36 @@ async function main(): Promise<void> {
298310
// Boot-time visibility (config-drift guardrail): state which config dir is actually in effect, unconditionally
299311
// -- neither reader above logs anything, so an operator previously had no way to confirm from the logs alone
300312
// which directory (if any) was live, which is exactly the ambiguity that let a stale, no-longer-mounted config
301-
// path get mistaken for the real one during a past incident. Never touches or validates any file; this is
302-
// purely a log line, same "state what's in effect" shape as the sentry/otel boot logs below.
313+
// path get mistaken for the real one during a past incident. `entryCount` is a cheap, one-time top-level
314+
// listing (never recursive, never touches file contents) so a SECOND incident of the same shape -- the mount
315+
// resolving but landing on an empty directory -- is visible in the log line itself, not just "some path is
316+
// configured" (see emptyConfigDirAdvisory below for the loud version of this same signal).
317+
const configDirOpts = {
318+
configured: Boolean(repoConfigDir),
319+
// A missing (as opposed to merely empty) directory is treated the same as zero entries -- both mean "no
320+
// local config was actually read" -- rather than letting a bad path crash the whole server at boot.
321+
entryCount: repoConfigDir ? safeReaddirCount(repoConfigDir) : 0,
322+
acknowledged: process.env.CONFIG_DIR_EMPTY_ACKNOWLEDGED === "true",
323+
};
303324
console.log(
304325
JSON.stringify({
305326
event: "selfhost_config_dir",
306-
configured: Boolean(repoConfigDir),
327+
configured: configDirOpts.configured,
307328
dir: repoConfigDir ?? null,
329+
entryCount: repoConfigDir ? configDirOpts.entryCount : null,
308330
}),
309331
);
332+
// Config-drift advisory: warn LOUDLY (not just the log line above) when the mount resolves but is empty --
333+
// see emptyConfigDirAdvisory's own doc comment for the incident this guards against.
334+
const configDirAdvisory = emptyConfigDirAdvisory(configDirOpts);
335+
if (configDirAdvisory)
336+
console.warn(
337+
JSON.stringify({
338+
level: "warn",
339+
event: "selfhost_config_dir_empty_advisory",
340+
message: configDirAdvisory,
341+
}),
342+
);
310343
// Error tracking (#1468): opt-in via SENTRY_DSN — a complete no-op when unset. When on, capture uncaught crashes
311344
// + unhandled rejections (flush before exit for the fatal case); per-subsystem captures (queue dead-letter,
312345
// review failures) are wired at their sites.
@@ -781,6 +814,7 @@ async function main(): Promise<void> {
781814
);
782815
gauge("loopover_backup_acknowledged", () => backupAcknowledgedGaugeValue(sqliteBackupOpts));
783816
gauge("loopover_public_origin_acknowledged", () => publicOriginAcknowledgedGaugeValue(publicOriginOpts));
817+
gauge("loopover_config_dir_empty_acknowledged", () => emptyConfigDirAcknowledgedGaugeValue(configDirOpts));
784818
// Pre-initialize job counters to 0 so they appear in the first Prometheus scrape (lazy counters
785819
// created on first use would otherwise cause "No data" in Grafana until the first job event).
786820
for (const c of [

test/unit/selfhost-health.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import {
88
backupAcknowledgedGaugeValue,
99
buildHealthBody,
1010
codexAuthReadinessProbe,
11+
emptyConfigDirAcknowledgedGaugeValue,
12+
emptyConfigDirAdvisory,
1113
githubAppReadinessProbe,
1214
publicOriginAcknowledgedGaugeValue,
1315
publicOriginReachabilityAdvisory,
@@ -311,6 +313,35 @@ describe("publicOriginAcknowledgedGaugeValue (#4180)", () => {
311313
});
312314
});
313315

316+
describe("emptyConfigDirAdvisory (gittensory->loopover rename incident)", () => {
317+
it("is silent when LOOPOVER_REPO_CONFIG_DIR is unset entirely — a normal, unconfigured install", () => {
318+
expect(emptyConfigDirAdvisory({ configured: false, entryCount: 0, acknowledged: false })).toBeNull();
319+
});
320+
321+
it("is silent when the mounted directory has at least one entry", () => {
322+
expect(emptyConfigDirAdvisory({ configured: true, entryCount: 1, acknowledged: false })).toBeNull();
323+
});
324+
325+
it("is silent when acknowledged, even if configured and empty", () => {
326+
expect(emptyConfigDirAdvisory({ configured: true, entryCount: 0, acknowledged: true })).toBeNull();
327+
});
328+
329+
it("regression: warns when configured but the mounted directory is empty — the exact incident shape", () => {
330+
const message = emptyConfigDirAdvisory({ configured: true, entryCount: 0, acknowledged: false });
331+
expect(message).toMatch(/mounted directory is empty/);
332+
expect(message).toMatch(/CONFIG_DIR_EMPTY_ACKNOWLEDGED/);
333+
});
334+
});
335+
336+
describe("emptyConfigDirAcknowledgedGaugeValue (gittensory->loopover rename incident)", () => {
337+
it("mirrors the advisory: 0 only when configured and empty and unacknowledged", () => {
338+
expect(emptyConfigDirAcknowledgedGaugeValue({ configured: true, entryCount: 0, acknowledged: false })).toBe(0);
339+
expect(emptyConfigDirAcknowledgedGaugeValue({ configured: true, entryCount: 0, acknowledged: true })).toBe(1);
340+
expect(emptyConfigDirAcknowledgedGaugeValue({ configured: true, entryCount: 1, acknowledged: false })).toBe(1);
341+
expect(emptyConfigDirAcknowledgedGaugeValue({ configured: false, entryCount: 0, acknowledged: false })).toBe(1);
342+
});
343+
});
344+
314345
describe("readiness (#982)", () => {
315346
afterEach(() => {
316347
vi.restoreAllMocks();

test/unit/selfhost-post-update-check-script.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@ if [ "$1" = "inspect" ] && [ "$2" = "--format" ]; then
4444
fi
4545
exit 0
4646
fi
47+
if [ "$1" = "exec" ]; then
48+
printf '%s\\n' "\${CONFIG_DIR_ENTRIES:-3}"
49+
exit 0
50+
fi
4751
exit 0
4852
`,
4953
);
@@ -100,4 +104,19 @@ describe("selfhost-post-update-check.sh", () => {
100104
expect(result.status).toBe(1);
101105
expect(result.stderr).toContain("after 2 attempts (6s)");
102106
});
107+
108+
it("regression: warns (without failing) when the container's private config mount is empty", () => {
109+
const result = run({ CONFIG_DIR_ENTRIES: "0" });
110+
111+
expect(result.status, result.stderr).toBe(0);
112+
expect(result.stderr).toContain("private config directory (LOOPOVER_REPO_CONFIG_DIR, default /config) is empty");
113+
expect(result.stdout).toContain("selfhost post-update check: ok");
114+
});
115+
116+
it("is silent when the container's private config mount has entries", () => {
117+
const result = run({ CONFIG_DIR_ENTRIES: "4" });
118+
119+
expect(result.status, result.stderr).toBe(0);
120+
expect(result.stderr).not.toContain("private config directory");
121+
});
103122
});

0 commit comments

Comments
 (0)