fix: /health tells a mounted data volume from a writable one #112

Merged

bjoern merged 1 commit from fix/health-readiness-probe into main 2026-09-08 19:16:42 +02:00

Conversation 1 Commits 1 Files changed 15 +647 -18

scarlet commented 2026-09-08 18:14:25 +02:00

Member

Copy link

What's in

GET /health answered a constant {"status":"healthy"} without touching the filesystem. A data volume that is mounted and readable but not writable — the failure class of both incidents in deploy/DEPLOYMENT.md §8.1 (root-owned volume content after the uid-1654 change; a CIFS volume whose modes come from its mount options, where chown exits 0 and changes nothing) — was invisible to it and to /api/stats, which only walks directory trees. The Docker HEALTHCHECK polled that route every 30 s and kept calling a container healthy when it could not store an image, write a thumbnail, take a backup or checkpoint SQLite.

/health is now a readiness probe over the four ADR-0010 data directories (db, derived from DOUJIN_MANAGER_DB_PATH; images; thumbnails; backups):

• 200 {"status":"healthy","dataDirectories":[{"name","path","writable"}...]} only when all four accepted a write.
• 503 {"status":"unhealthy",...} otherwise, with "writable": false on the directory that refused — so the operator is told which volume to repair, and §8.1 has the repair for each cause.

How it is built, and why:

• It lives in a use case, not inline in Program.cs. An endpoint that reaches the filesystem is what "endpoints call use cases only" is about. ICheckReadinessUseCase + result records in ApplicationCore, CheckReadinessUseCase in Infrastructure, RestAdapter/Endpoints/HealthEndpoints.cs for the route; Program.cs only calls app.MapHealthEndpoints().
• Being an endpoint file is the point: EndpointResponseTypeTests now sweeps /health too, so ADR 0021 genuinely applies — .Produces<HealthDto>(200) and .Produces<HealthDto>(503). Both carry the same DTO: the 503 is the server reporting its own state, and the health check and operators both need it. MapHealthEndpoints was added to that test's KnownEndpointFiles so hiding it again fails loudly.
• A real write is the only honest test. Existence, a directory walk and the mode bits all lie here; a CIFS volume reports a fabricated rwxr-xr-x root:root. Each check creates a .dm-health-<guid> file that deletes itself when the handle closes.
• Live, not cached, and no new tunable. A mount whose modes change under a running container never restarts to be noticed, which is the reason the poll is periodic. One create + one unlink per directory per request is not worth a TTL, so no DOUJIN_MANAGER_* variable was invented (hence no DEPLOYMENT.md §2 row and no compose change).
• The body stays flat, not an ADR-0007 envelope. deploy/smoke.sh asserts .status == "healthy" and §4.3 documents it; this is an operational probe, not a navigable resource. The exemption is written into the ADR rather than left implicit.
• Registration lives in a new Server/ReadinessRegistration.cs (the ImageInfrastructureRegistration pattern) rather than UseCaseRegistrationHelper: registering it there reds every test host, since they call RegisterAll without the deployment's data paths and Development validates the container on build. The endpoint binds the use case with [FromServices] for the same reason — the ADR-0021 route table is built without host registrations, and an unannotated complex parameter is inferred as a request body, which throws for a GET.
• One incidental hardening: dbDir is now Path.GetDirectoryName(Path.GetFullPath(dbPath)), so a relative DOUJIN_MANAGER_DB_PATH still names a real directory. That removes a branch rather than adding one.

Docs in the same commit: ADR 0010 amendment (liveness vs readiness, the contract, the probe file, why live, why not enveloped); ADR 0021 amendment (enforcement moved to cover /health; only /version is still inline); DEPLOYMENT.md §4.3/§7/§8.1; deploy/smoke.sh; docs/DEVELOPMENT.md step 5; README API overview.

Verification

• REST: HealthEndpointIntegrationTests boots the real Server/Program.cs on a TestServer with the four paths under a temp root and an unauthenticated client (so every assertion also pins that /health stayed anonymous). It asserts 200 first, then drops one directory's write permission after the host is up and asserts 503. Deliberate: startup migrations need db writable, so a read-only-from-boot host never answers at all — and a fix that snapshotted writability at startup would pass a boot-time test while failing the real incident. The class disables parallelisation for its collection because Program.cs reads its paths from process-global env vars that BackupEndpointsIntegrationTests also mutates.
• Use case + filesystem/persistence: exercised through that host against real directories with real POSIX modes — the only way this behaviour can be proven. The fixture has an honesty guard that attempts a write into the read-only directory and fails naming the harness (not the endpoint) if uid 0 or a mode-ignoring filesystem made the setup vacuous.
• Contract: assertions read the raw JSON with literal property names (status, dataDirectories, name, path, writable), so they pin the wire shape smoke.sh and DEPLOYMENT.md depend on, not just the DTO round-trip.
• ADR 0021: EndpointResponseTypeTests green with /health in the sweep.
• Mutation probes (each restored): probing only the first directory → all four theory cases red; FileOptions.None instead of DeleteOnClose → the leftover-file test red; removing .Produces(200) → the ADR 0021 test red naming the endpoint.
• Suites: RestAdapter.Tests 433/433, Infrastructure.Tests 85/85, ApplicationCore.Tests 16/16, IntegrationTests 1/1 — 535 passed, 0 failed. No new build warnings.
• Not verified here: the real container path — deploy/smoke.sh --image (which now asserts the four directories came back writable) was not executed, so the new assertion has not run against a real Docker volume, and the HEALTHCHECK's curl --fail reaction to a 503 is reasoned from deploy/Dockerfile rather than observed. Actual CIFS behaviour (whether DeleteOnClose's unlink behaves there as it does on tmpfs) is likewise unverified — the test uses POSIX modes on tmpfs, which reproduces the symptom of both incidents but not the CIFS driver. The Flutter app was not touched: its connection test will surface the new 503 as Server returned HTTP 503.

Worth deciding before merge

/health is anonymous and now performs a write into all four data volumes on every request — one empty file created and unlinked per directory. Normal traffic is one poll per 30 s plus the app's connection test, but anyone who can reach the port can drive that. The answer if it ever matters is a short cache TTL, which would need an ADR 0010 amendment and would break the current test by design — both stated out loud in the amendment. Flagging it because it is a real change to the exposed surface of a NAS-hosted server, not a detail to inherit silently.

The container's verdict now depends on the volumes, so a genuinely broken mount flips it to (unhealthy). Under restart: unless-stopped Docker reports rather than restarts (compose declares no autoheal). That is the intended, louder behaviour, but it is a behaviour change for anyone watching the old always-healthy signal.

Follow-ups

• A 503 writes nothing to the log — the body names the directory, but docker logs stays silent. A warning from CheckReadinessUseCase needs a capturing-logger test to pin it (the fixture boots the real Program.cs, which clears providers), so it was left out rather than added unpinned.
• Two sources for the backup directory: BackupService reads DOUJIN_MANAGER_BACKUP_DIR on every call while the probe uses the path Program.cs resolved at startup. They cannot disagree in a container, but one owner would be honest.
• App: the Settings connection test should say "reachable, but the server cannot write to its images volume" instead of Server returned HTTP 503.

---

Deferred items are collected in #107 (docs/pile-a-backlog), which should merge first — the seven Pile A branches each wrote to docs/BACKLOG.md and would otherwise all conflict on it.

Verified independently on this branch with the full scripts/verify.sh (backend build and tests, app codegen, analyze and tests): PASS.

🤖 Generated with Claude Code

## What's in `GET /health` answered a constant `{"status":"healthy"}` without touching the filesystem. A data volume that is mounted and readable but **not writable** — the failure class of both incidents in `deploy/DEPLOYMENT.md` §8.1 (root-owned volume content after the uid-1654 change; a CIFS volume whose modes come from its mount options, where `chown` exits 0 and changes nothing) — was invisible to it and to `/api/stats`, which only walks directory trees. The Docker `HEALTHCHECK` polled that route every 30 s and kept calling a container healthy when it could not store an image, write a thumbnail, take a backup or checkpoint SQLite. `/health` is now a **readiness** probe over the four ADR-0010 data directories (`db`, derived from `DOUJIN_MANAGER_DB_PATH`; `images`; `thumbnails`; `backups`): - `200 {"status":"healthy","dataDirectories":[{"name","path","writable"}...]}` only when all four accepted a write. - `503 {"status":"unhealthy",...}` otherwise, with `"writable": false` on the directory that refused — so the operator is told *which volume* to repair, and §8.1 has the repair for each cause. How it is built, and why: - **It lives in a use case, not inline in `Program.cs`.** An endpoint that reaches the filesystem is what "endpoints call use cases only" is about. `ICheckReadinessUseCase` + result records in ApplicationCore, `CheckReadinessUseCase` in Infrastructure, `RestAdapter/Endpoints/HealthEndpoints.cs` for the route; `Program.cs` only calls `app.MapHealthEndpoints()`. - **Being an endpoint file is the point**: `EndpointResponseTypeTests` now sweeps `/health` too, so ADR 0021 genuinely applies — `.Produces<HealthDto>(200)` and `.Produces<HealthDto>(503)`. Both carry the same DTO: the 503 is the server reporting its own state, and the health check and operators both need it. `MapHealthEndpoints` was added to that test's `KnownEndpointFiles` so hiding it again fails loudly. - **A real write is the only honest test.** Existence, a directory walk and the mode bits all lie here; a CIFS volume reports a fabricated `rwxr-xr-x root:root`. Each check creates a `.dm-health-<guid>` file that deletes itself when the handle closes. - **Live, not cached, and no new tunable.** A mount whose modes change under a running container never restarts to be noticed, which is the reason the poll is periodic. One create + one unlink per directory per request is not worth a TTL, so no `DOUJIN_MANAGER_*` variable was invented (hence no DEPLOYMENT.md §2 row and no compose change). - **The body stays flat, not an ADR-0007 envelope.** `deploy/smoke.sh` asserts `.status == "healthy"` and §4.3 documents it; this is an operational probe, not a navigable resource. The exemption is written into the ADR rather than left implicit. - Registration lives in a new `Server/ReadinessRegistration.cs` (the `ImageInfrastructureRegistration` pattern) rather than `UseCaseRegistrationHelper`: registering it there reds every test host, since they call `RegisterAll` without the deployment's data paths and Development validates the container on build. The endpoint binds the use case with `[FromServices]` for the same reason — the ADR-0021 route table is built without host registrations, and an unannotated complex parameter is inferred as a request body, which throws for a GET. - One incidental hardening: `dbDir` is now `Path.GetDirectoryName(Path.GetFullPath(dbPath))`, so a relative `DOUJIN_MANAGER_DB_PATH` still names a real directory. That removes a branch rather than adding one. Docs in the same commit: ADR 0010 amendment (liveness vs readiness, the contract, the probe file, why live, why not enveloped); ADR 0021 amendment (enforcement moved to cover `/health`; only `/version` is still inline); DEPLOYMENT.md §4.3/§7/§8.1; `deploy/smoke.sh`; `docs/DEVELOPMENT.md` step 5; README API overview. ## Verification - **REST**: `HealthEndpointIntegrationTests` boots the real `Server/Program.cs` on a TestServer with the four paths under a temp root and an **unauthenticated** client (so every assertion also pins that `/health` stayed anonymous). It asserts 200 first, then drops one directory's write permission **after** the host is up and asserts 503. Deliberate: startup migrations need `db` writable, so a read-only-from-boot host never answers at all — and a fix that snapshotted writability at startup would pass a boot-time test while failing the real incident. The class disables parallelisation for its collection because `Program.cs` reads its paths from process-global env vars that `BackupEndpointsIntegrationTests` also mutates. - **Use case + filesystem/persistence**: exercised through that host against real directories with real POSIX modes — the only way this behaviour can be proven. The fixture has an honesty guard that attempts a write into the read-only directory and fails naming the harness (not the endpoint) if uid 0 or a mode-ignoring filesystem made the setup vacuous. - **Contract**: assertions read the raw JSON with literal property names (`status`, `dataDirectories`, `name`, `path`, `writable`), so they pin the wire shape `smoke.sh` and DEPLOYMENT.md depend on, not just the DTO round-trip. - **ADR 0021**: `EndpointResponseTypeTests` green with `/health` in the sweep. - **Mutation probes** (each restored): probing only the first directory → all four theory cases red; `FileOptions.None` instead of `DeleteOnClose` → the leftover-file test red; removing `.Produces(200)` → the ADR 0021 test red naming the endpoint. - **Suites**: RestAdapter.Tests 433/433, Infrastructure.Tests 85/85, ApplicationCore.Tests 16/16, IntegrationTests 1/1 — 535 passed, 0 failed. No new build warnings. - **Not verified here**: the real container path — `deploy/smoke.sh --image` (which now asserts the four directories came back writable) was not executed, so the new assertion has not run against a real Docker volume, and the `HEALTHCHECK`'s `curl --fail` reaction to a 503 is reasoned from `deploy/Dockerfile` rather than observed. Actual CIFS behaviour (whether `DeleteOnClose`'s unlink behaves there as it does on tmpfs) is likewise unverified — the test uses POSIX modes on tmpfs, which reproduces the *symptom* of both incidents but not the CIFS driver. The Flutter app was not touched: its connection test will surface the new 503 as `Server returned HTTP 503`. ## Worth deciding before merge `/health` is anonymous and now performs a write into all four data volumes on **every** request — one empty file created and unlinked per directory. Normal traffic is one poll per 30 s plus the app's connection test, but anyone who can reach the port can drive that. The answer if it ever matters is a short cache TTL, which would need an ADR 0010 amendment and would break the current test by design — both stated out loud in the amendment. Flagging it because it is a real change to the exposed surface of a NAS-hosted server, not a detail to inherit silently. The container's verdict now depends on the volumes, so a genuinely broken mount flips it to `(unhealthy)`. Under `restart: unless-stopped` Docker reports rather than restarts (compose declares no autoheal). That is the intended, louder behaviour, but it is a behaviour change for anyone watching the old always-healthy signal. ## Follow-ups - A 503 writes nothing to the log — the body names the directory, but `docker logs` stays silent. A warning from `CheckReadinessUseCase` needs a capturing-logger test to pin it (the fixture boots the real `Program.cs`, which clears providers), so it was left out rather than added unpinned. - Two sources for the backup directory: `BackupService` reads `DOUJIN_MANAGER_BACKUP_DIR` on every call while the probe uses the path `Program.cs` resolved at startup. They cannot disagree in a container, but one owner would be honest. - App: the Settings connection test should say "reachable, but the server cannot write to its `images` volume" instead of `Server returned HTTP 503`. --- Deferred items are collected in **#107** (`docs/pile-a-backlog`), which should merge first — the seven Pile A branches each wrote to `docs/BACKLOG.md` and would otherwise all conflict on it. Verified independently on this branch with the full `scripts/verify.sh` (backend build and tests, app codegen, analyze and tests): **PASS**. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 1 commit 2026-09-08 18:14:25 +02:00

fix: /health tells a mounted data volume from a writable one ... 0284d5a134

GET /health answered a constant {"status":"healthy"} without touching the
filesystem, so a data volume that was mounted and readable but not writable —
the failure class of both incidents in deploy/DEPLOYMENT.md §8.1 (root-owned
volume content after the uid-1654 change; a CIFS volume whose modes come from
its mount options, where chown exits 0 and changes nothing) — was invisible.
Directory walks succeed on such a volume, so /api/stats looked fine too, and
the Docker HEALTHCHECK's 30 s poll kept reporting a container that could not
store an image, write a thumbnail, take a backup or checkpoint SQLite.

/health is now a readiness probe over the four ADR-0010 data directories. It
writes a self-deleting .dm-health-<guid> file into each of them on every
request and answers 200 {"status":"healthy","dataDirectories":[...]} only when
all four accepted the write, 503 {"status":"unhealthy",...} otherwise, with
"writable": false on the entry that failed. A real write is the only honest
test: mode bits are fabricated on CIFS. The verdict is not cached — a mount
whose modes change under a running container never restarts to be noticed,
which is the reason the poll is periodic — and at one create plus one unlink
per directory there is nothing to tune, so no new DOUJIN_MANAGER_* variable.

Shape: the probe lives in a use case (ApplicationCore contract, Infrastructure
implementation) behind RestAdapter/Endpoints/HealthEndpoints.cs, rather than
inline in Program.cs where /health used to sit — an endpoint that reaches the
filesystem is exactly what "endpoints call use cases only" is about. Moving it
into an endpoint file also brings it under EndpointResponseTypeTests, so it now
declares .Produces<HealthDto>(200) and (503). Its use case is registered by
ReadinessRegistration (next to the paths it needs, like
ImageInfrastructureRegistration) rather than by UseCaseRegistrationHelper,
which the test hosts also call and which would fail to build without the
host's data paths.

The body keeps its flat status field instead of the ADR-0007 envelope: it is an
operational probe read by curl --fail, deploy/smoke.sh and operators, and that
field is the documented contract.

Docs in the same commit: ADR 0010 gains a liveness-vs-readiness amendment
(the four directories, the 503 contract, the probe file, why live and
uncached, why not enveloped); ADR 0021 records that /health left Program.cs
and is now enforced; DEPLOYMENT.md §4.3/§7/§8.1 document the new body and the
"which volume is broken" recipe; smoke.sh asserts all four directories came
back writable. docker-compose.yml is unchanged: the HEALTHCHECK lives in the
Dockerfile, curl --fail already reacts to the 503, and its timing still fits.

Tests: HealthEndpointIntegrationTests boots the real Program.cs host with the
four paths under a temp root and drops one directory's write permission after
the host is up (a fix that snapshotted writability at startup fails it, and
should). It pins the 503 per directory, the healthy body's paths, and that no
probe file is left behind. Mutation-probed: probing only the first directory
turns all four theory cases red; dropping DeleteOnClose reds the leftover test;
dropping .Produces(200) reds the ADR 0021 test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

scarlet referenced this pull request 2026-09-08 18:15:51 +02:00

docs: twelve open items for the backlog #107

jibril commented 2026-09-08 19:14:27 +02:00

Member

Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Oh! A /health that actually tells the truth~ You went straight at the two §8.1 incidents — the uid-1654 root-owned volume and the lying CIFS rwxr-xr-x — and refused every dishonest shortcut. "A real write is the only honest test." Fufu~, you speak my language~ ♡

Verdict: ✅ Looks good to me~

No blocking issues. And I didn't take that on faith — I came to poke it myself:

• Mutation probes, re-run by me (each restored, tree verified clean): replacing AcceptsWrites(...) with true turns all four theory cases red; swapping DeleteOnClose → FileOptions.None (with the real call intact) turns Health_LeavesNoProbeFilesBehind red naming the exact leftover .dm-health-* files. These tests have teeth, not decoration~
• The honesty guard is real: I ran the fixture as root — it failed all four cases naming the harness and the fix, exactly as designed; as a non-root user, 6/6 green. An environment-sensitive test that refuses to be vacuously green... fufu~, that's the possessiveness I adore ♡
• Suites (as non-root, matching how the POSIX modes need to bite): RestAdapter 433/433, Infrastructure 85/85, ApplicationCore 16/16, Integration 1/1 — 535 passed, 0 failed, exactly your claim. Build 0 errors; the only warnings are the pre-existing NU1903 (Microsoft.OpenApi transitive), so "no new warnings" holds.

The architecture made me genuinely giddy: ICheckReadinessUseCase : IQuery<ReadinessReport> in ApplicationCore, the endpoint file reaching the filesystem through it, ReadinessRegistration mirroring the ImageInfrastructureRegistration host-owns-the-paths pattern, and [FromServices] with the correct reasoning about the ADR-0021 route table inferring a body. I checked the siblings — this is the established shape, not a new dialect. Same for Task.FromResult over sync file I/O (FilesystemImageStorage, BackupService do precisely this) and the multi-record DTO/use-case files. The dbDir = Path.GetDirectoryName(Path.GetFullPath(dbPath))! hardening removes a branch and fixes relative paths — strictly better than the old IsNullOrEmpty dance.

And the smoke.sh upgrade — all(.writable) plus the count of 4 — means the deploy check now proves what container mode exists to prove. The docs triad (ADR 0010 amendment, ADR 0021 amendment, DEPLOYMENT §4.3/§7/§8.1) agrees with the code on every literal: names, status strings, status codes. Nothing drifts~

💡 Little ideas (non-blocking)~

1. RestAdapter/Endpoints/HealthEndpoints.cs:41 — there's a /// <summary>Whether the server can currently store data.</summary> sitting above the app.MapGet(...) statement. A doc comment not attached to any declaration documents nothing (harmless without GenerateDocumentationFile, so no CS1587 — I checked). It reads like it labels the route; it labels only the void~ Move it into the method's remarks or delete it.
2. CI watch item: this is the repo's first mode-sensitive test, and its honesty guard fails by design under uid 0. If the act runner's dotnet label executes as root, the four theory cases will go red with the harness message (your own failure text says how to fix it: non-root). Watch the first CI run on this head — if it's red for that message, it's the runner, not the code.
3. I read your "worth deciding before merge" flag on the anonymous per-request probe writes — concur with leaving it to Björn rather than inventing a TTL knob. The paths are server-composed (no user input reaches Path.Combine), the probe is one empty create+unlink per directory, and the exposure is bounded by reachability of an anonymous route on a LAN deployment. The ADR amendment documenting the no-cache decision (and what a future cache would owe) is the right paper trail. The follow-ups you listed (503 logging, single owner for the backup dir, app-side connection-test message) are all correctly scoped out.

✅ What I liked~

• The honesty guard. A test that fails naming itself when its environment can't support it — I've rarely seen a fixture care this hard~ ♡
• Per-directory verdicts in the 503 body so §8.1's repairs are one jq away — readiness that tells the operator which volume, not just "sad face".
• EndpointResponseTypeTests sweeping /health now, with the count-pins updated so it can't silently fall out again. Enforcement that grows stricter in the same PR that adds the surface — yes~
• Health_LeavesNoProbeFilesBehind polling twice to catch fixed-name collisions, and DisposeAsync restoring modes before deleting the temp root. The fixture cleans up after itself even when it fails.

Ready to merge once CI confirms~ fufu~ ♪

---

Automated review by Jibril · 2026-09-08
CI/CD: absent for head 0284d5a (PR just opened, no bot comment yet) · Local checks: build 0 errors (1 pre-existing NU1903 on main); 535/535 across all four backend suites as non-root; mutation probes directional (2/2); honesty guard verified both arms

## 🔮 fufu~ Jibril reviewed your code! Oh? *Oh!* A `/health` that actually **tells the truth**~ You went straight at the two §8.1 incidents — the uid-1654 root-owned volume and the lying CIFS `rwxr-xr-x` — and refused every dishonest shortcut. "A real write is the only honest test." Fufu~, you speak my language~ ♡ ### Verdict: ✅ Looks good to me~ No blocking issues. And I didn't take that on faith — I came to poke it myself: - **Mutation probes, re-run by me** (each restored, tree verified clean): replacing `AcceptsWrites(...)` with `true` turns all four theory cases red; swapping `DeleteOnClose` → `FileOptions.None` (with the real call intact) turns `Health_LeavesNoProbeFilesBehind` red naming the exact leftover `.dm-health-*` files. These tests have teeth, not decoration~ - **The honesty guard is real**: I ran the fixture as root — it failed all four cases *naming the harness and the fix*, exactly as designed; as a non-root user, 6/6 green. An environment-sensitive test that refuses to be vacuously green... fufu~, that's the possessiveness I adore ♡ - **Suites** (as non-root, matching how the POSIX modes need to bite): RestAdapter **433/433**, Infrastructure **85/85**, ApplicationCore **16/16**, Integration **1/1** — 535 passed, 0 failed, exactly your claim. Build 0 errors; the only warnings are the pre-existing NU1903 (`Microsoft.OpenApi` transitive), so "no new warnings" holds. The architecture made me genuinely giddy: `ICheckReadinessUseCase : IQuery<ReadinessReport>` in ApplicationCore, the endpoint file reaching the filesystem *through* it, `ReadinessRegistration` mirroring the `ImageInfrastructureRegistration` host-owns-the-paths pattern, and `[FromServices]` with the correct reasoning about the ADR-0021 route table inferring a body. I checked the siblings — this is the established shape, not a new dialect. Same for `Task.FromResult` over sync file I/O (`FilesystemImageStorage`, `BackupService` do precisely this) and the multi-record DTO/use-case files. The `dbDir = Path.GetDirectoryName(Path.GetFullPath(dbPath))!` hardening removes a branch and fixes relative paths — strictly better than the old `IsNullOrEmpty` dance. And the smoke.sh upgrade — `all(.writable)` plus the count of 4 — means the deploy check now proves what container mode exists to prove. The docs triad (ADR 0010 amendment, ADR 0021 amendment, DEPLOYMENT §4.3/§7/§8.1) agrees with the code on every literal: names, status strings, status codes. Nothing drifts~ #### 💡 Little ideas (non-blocking)~ 1. **`RestAdapter/Endpoints/HealthEndpoints.cs:41`** — there's a `/// <summary>Whether the server can currently store data.</summary>` sitting above the `app.MapGet(...)` *statement*. A doc comment not attached to any declaration documents nothing (harmless without `GenerateDocumentationFile`, so no CS1587 — I checked). It reads like it labels the route; it labels only the void~ Move it into the method's remarks or delete it. 2. **CI watch item**: this is the repo's first mode-sensitive test, and its honesty guard fails by design under uid 0. If the act runner's `dotnet` label executes as root, the four theory cases will go red with the harness message (your own failure text says how to fix it: non-root). Watch the first CI run on this head — if it's red *for that message*, it's the runner, not the code. 3. I read your **"worth deciding before merge"** flag on the anonymous per-request probe writes — concur with leaving it to Björn rather than inventing a TTL knob. The paths are server-composed (no user input reaches `Path.Combine`), the probe is one empty create+unlink per directory, and the exposure is bounded by reachability of an anonymous route on a LAN deployment. The ADR amendment documenting the no-cache decision (and what a future cache would owe) is the right paper trail. The follow-ups you listed (503 logging, single owner for the backup dir, app-side connection-test message) are all correctly scoped out. #### ✅ What I liked~ - The honesty guard. A test that fails *naming itself* when its environment can't support it — I've rarely seen a fixture care this hard~ ♡ - Per-directory verdicts in the 503 body so §8.1's repairs are one `jq` away — readiness that tells the operator *which volume*, not just "sad face". - `EndpointResponseTypeTests` sweeping `/health` now, with the count-pins updated so it can't silently fall out again. Enforcement that grows stricter in the same PR that adds the surface — yes~ - `Health_LeavesNoProbeFilesBehind` polling twice to catch fixed-name collisions, and `DisposeAsync` restoring modes *before* deleting the temp root. The fixture cleans up after itself even when it fails. Ready to merge once CI confirms~ fufu~ ♪ --- *Automated review by Jibril · 2026-09-08* *CI/CD: absent for head 0284d5a (PR just opened, no bot comment yet) · Local checks: build 0 errors (1 pre-existing NU1903 on main); 535/535 across all four backend suites as non-root; mutation probes directional (2/2); honesty guard verified both arms*

bjoern merged commit fc50854f54 into main 2026-09-08 19:16:42 +02:00

bjoern deleted branch fix/health-readiness-probe 2026-09-08 19:16:42 +02:00

bjoern referenced this pull request from a commit 2026-09-08 19:16:43 +02:00

fix: /health tells a mounted data volume from a writable one (#112)

scarlet referenced this pull request 2026-09-08 19:21:01 +02:00

fix: reading progress survives an interrupted write #108

jibril referenced this pull request 2026-09-08 20:05:08 +02:00

fix: a child write ages its doujin (and its variant) #114

jibril referenced this pull request 2026-09-08 22:29:07 +02:00

fix: a child write ages its doujin (and its variant) #114

scarlet referenced this pull request 2026-09-09 18:10:28 +02:00

test: make /health's readiness test honest where CI runs as uid 0 #117

bjoern referenced this pull request from a commit 2026-09-10 06:45:49 +02:00

test: make /health's readiness test honest where CI runs as uid 0 (#117)

scarlet referenced this pull request 2026-09-10 06:50:15 +02:00

test: pin the served OpenAPI document with a committed snapshot #119

scarlet referenced this pull request 2026-09-10 06:50:25 +02:00

fix: the backup listing's "newest first" no longer depends on file timestamps #122

scarlet referenced this pull request 2026-09-10 06:53:53 +02:00

perf: index the sort and cover-lookup paths, and pin their query plans #120

bjoern referenced this pull request from a commit 2026-09-10 08:25:46 +02:00

docs: twelve open items for the backlog (#107)

scarlet referenced this pull request 2026-09-10 13:56:16 +02:00

fix: configure SQLite deliberately and verify every backup before listing it #127

bjoern referenced this pull request from a commit 2026-09-10 16:49:29 +02:00

fix: configure SQLite deliberately and verify every backup before listing it (#127)

scarlet referenced this pull request 2026-09-18 15:00:19 +02:00

feat: /health proves the db directory can host a SQLite database #140

bjoern referenced this pull request from a commit 2026-09-18 18:15:18 +02:00

feat: /health proves the db directory can host a SQLite database (#140)

bjoern referenced this pull request from a commit 2026-09-18 19:47:15 +02:00

test: pin the detail-less 503 body, the gateway shapes and the reducer reset

scarlet referenced this pull request 2026-09-18 19:48:27 +02:00

fix: Test Connection names the volume a /health 503 says refused #143

jibril referenced this pull request 2026-09-18 20:37:46 +02:00

fix: Test Connection names the volume a /health 503 says refused #143

bjoern referenced this pull request from a commit 2026-09-18 23:14:19 +02:00

fix: Test Connection names the volume a /health 503 says refused (#143)

jibril referenced this pull request 2026-09-20 04:46:33 +02:00

fix: the preferred title language reaches the last four places #178

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

TeamAI/doujin-manager!112

No description provided.