feat(logging): report file-sink health on control.status

[MichaelTaylor3d]

Tracked upstream as DIG-Network/dig_ecosystem#553 (a super-repo issue, so no Closes keyword here -- a submodule PR cannot close it; the orchestrator closes it).

Status: rebased onto current main, squashed to one commit, awaiting its gate round. Still a DRAFT deliberately — do not merge until the gate returns.

What changed, and why the title moved

The original title said "adopt dig-logging 0.2.0". That half already shipped: origin/main has declared dig-logging = "0.2" since 2f0f148 (#395, merged while this PR sat idle), and Cargo.lock already resolves dig-logging 0.2.0. So the dependency bump is now a no-op and the lock is untouched by this PR.

What remains — and what this PR is actually for — is wiring the signal that 0.2.0 exposes.

The defect

Under dig-logging 0.1.4, dig_logging::init was all-or-nothing. When the rolling file appender could not be built it returned Err, and the stderr layer was never installed either, so the process ran with no tracing subscriber at all. A user running dig-node run interactively on a host where C:\ProgramData\DigNetwork\logs\dig-node belongs to the service account got silence — which reads as a dead subsystem rather than a broken one.

dig-logging 0.2.0 degrades to console-only logging instead, and reports the reason through LogGuard::file_error(). Nothing consumed that yet, so the node could serve happily while writing nothing to disk and no surface would say so.

The change

• logging::{health, file_error, log_dir, initialized} expose the guard state. health is pure in its inputs, so both arms are testable without a process-global subscriber.
• control.status gains a logging object: initialized, dir, file_logging, file_error. A node serving while writing nothing to disk no longer reports healthy file logging.
• tests/logging_degraded.rs proves the property end to end.
• SPEC.md records the degraded contract (section 20.1) and corrects the sentence that still listed "unwritable dir" as an init failure.

The fixture, and why it is the one that can fail

The nearest wrong implementation reports file_logging: true whenever logging initialised — precisely the lie a degraded file sink makes possible. So the unit tests carry an honest control: health(true, dir, None) must report file_logging: true, while health(true, dir, Some(err)) must report false. A constant in either direction fails one of the two.

The integration test's fixture is a path whose parent is a regular file. create_dir_all cannot succeed under a file on any platform, so the fixture does not depend on running unprivileged, on ACLs, or on a read-only mount — the three things that quietly make a permission fixture pass for the wrong reason, or not fail at all under a runner elevated to Administrator. It also asserts LevelFilter::current() is not OFF, which is the assertion that would have failed on 0.1.x for the right reason.

Blast radius

Checked by grep + direct read, not by gitnexus: the only index available is the superproject's, which CLAUDE.md 2.0 forbids querying from a lane, and a per-worktree analyze was not worth the ~10-minute bound for a diff this size. Stating the fallback per bound (2).

• logging::health, file_error, log_dir, initialized are all new. Their only callers are control.rs:984-987 and the new test. No existing caller to break.
• logging::init's signature is unchanged; its two callers (entrypoint.rs:1291, win_service.rs:74) are untouched. The only behaviour change inside it is 0.2.0's degradation, which is the fix.
• control.status gains a field and changes none. grep finds no consumer of a logging key anywhere in the tree, so the addition is purely additive on the wire.

Version

0.164.0 -> 0.165.0 — minor. A compatible new capability: a new field on an existing control method, no removal, no rename, no changed default.

Verification

• cargo test -p dig-node-service --lib --test logging_degraded
• Full required check set on CI.

Rebase note

This branch was 73 commits behind main and could not merge under strict=true. It was rebased, not merged. Two conflicts, both resolved by keeping both sides: control.rs (main had added wallet_mtls, this branch adds logging — adjacent fields in the same object, both wanted) and the workspace version (main at 0.164.0, superseding this branch's stale 0.125.0). The three original commits are squashed into one; the old third commit's message claimed a 0.125.0 bump that no longer exists.

[MichaelTaylor3d]

Lane progress — resume-ready

Branch: loop/dig-logging-0.2.0
HEAD: bafb54a8ccb0d0316f196f301c1d49f57f2c8aa4
Worktree: C:\tmp\worktrees\dn237
State: DRAFT, deliberately. Gate round has NOT run.

Done

1. Rebased onto origin/main (26e640d). The branch was 73 commits behind and could not merge under strict=true.
2. Two conflicts, both resolved by keeping both sides, no markers left (git grep for markers is clean):
   • control.rs — main added wallet_mtls to the control.status object while this branch added logging. Adjacent fields, both wanted.
   • Cargo.toml version — main at 0.164.0 superseded this branch's stale 0.125.0.
3. Discovered the dep bump had already shipped. origin/main has declared dig-logging = "0.2" since 2f0f148 (feat(collateral): serve mirror-coin collateral control methods and spends.list #395), and Cargo.lock already resolved dig-logging 0.2.0. This PR's Cargo.lock is therefore identical to main's except the workspace version line. The original title was wrong and has been corrected; the body explains it.
4. Verified dig-logging 0.2.0 is the latest published version (index.crates.io, with the required User-Agent). No newer line exists.
5. Squashed the three commits (one of them a (WIP), one claiming a 0.125.0 bump that no longer exists) into one Conventional Commit.
6. Bumped 0.164.0 -> 0.165.0 in Cargo.toml and the matching dig-node-service entry in Cargo.lock. Minor — a new field on an existing control method, nothing removed or renamed.
7. Rewrote the PR body; the DO NOT MERGE - gate round not yet run opener is gone and replaced with the real state.

Section 2.4b dependency audit

Every dig-* dep in the touched manifest is already at its latest published version: dig-node-control-interface 0.24, dig-mirror-collateral 0.3, dig-rpc-protocol 0.10 (0.10.2 latest, caret-covered), dig-cert 0.1.2, dig-urn-resolver 0.5.3, dig-constants 0.13.0, dig-logging 0.2. Nothing stale, nothing bumped.

One deliberate non-bump, reported rather than done: chia-bls is pinned at 0.36.1 and the latest published is 0.48.0. chia-* crates move as a SET, so bumping chia-bls alone would ship this crate internally split across two chia lines — the exact defect section 2.4b exists to prevent. That is an ecosystem-wide cascade, not this PR's scope. Flagging it for a dedicated family.

Remains

1. CI to finish on bafb54a. Already SUCCESS: Rustfmt, Clippy, Lint commit messages, Check version increment, Release-script tests, Analyze (actions), Analyze (javascript-typescript). Pending: Test + coverage, Analyze (rust), the four installer builds.
2. The gate round — this is the reason the PR stays a draft. It has never been gated.

Exact next action

Run the gate round against bafb54a. Tier: one correctness gate. The diff is additive (four new functions whose only callers are the new call site and the new test, plus one new JSON field on control.status that nothing in the tree consumes), it touches no custody, crypto or peer-facing input, and logging::init's signature and both its callers are unchanged.

Then, and only then, check-merge-preconditions.sh --repo DIG-Network/dig-node --pr 237 unpiped, read the exit code from a file, and undraft only on exit 0.

[MichaelTaylor3d]

CI note — Test + coverage is running long, and it is not yet proven benign

At ~85 minutes, Test + coverage on bafb54a is still in_progress. The baseline for this workflow on main is 18-23 minutes across the last five runs, so this is roughly 3-4x normal.

Job 99013632129, step 7 (cargo llvm-cov nextest). The four other required checks — Lint commit messages, Check version increment, Rustfmt, Clippy — are all SUCCESS.

Most likely cause, stated as a hypothesis rather than a finding: the version bump changes Cargo.toml, which is part of the Cache cargo + target key, so this run rebuilds the whole workspace cold under coverage instrumentation. That is exactly the shape that turns a 20-minute job into an 80-minute one.

What would distinguish that from a hung test: a cold-cache rebuild finishes; a hang does not. If this job passes, the hypothesis stands and nothing needs doing. If it times out, the next thing to check is whether any test in the new tests/logging_degraded.rs can block — and the answer looks like no, because it runs in its own process, takes no lock, opens no socket, and completed locally in 0.00s.

Local evidence, for what it is worth ahead of CI — cargo test -p dig-node-service --lib --test logging_degraded on this exact tree:

test result: ok. 506 passed; 0 failed  (lib)
test result: ok. 1 passed;   0 failed  (logging_degraded)

The integration test genuinely exercised the degraded path rather than merely asserting it. Its own captured output shows both halves of the property:

WARN dig-logging: file logging is DISABLED for ...\dig-node-logtest-71628\root\dig-node
  (could not create or write the log directory ...: os error 183).
  Console logging continues; set DIG_LOG_DIR to a writable directory to restore JSONL log files.
INFO logging_degraded: node still speaks on the console test="degraded"

The WARN proves the file sink was really refused; the INFO proves the console sink stayed live. Under dig-logging 0.1.x that INFO line could not have been emitted at all, because no subscriber would have been installed.

This does not change the merge posture. The PR stays a DRAFT, the gate round has still not run, and check-merge-preconditions.sh --allow-draft returns EXIT 1 pending this check.

[MichaelTaylor3d]

Correction to the CI-anomaly note above: there is no anomaly.

The earlier note reported Test + coverage as running ~85-90 minutes against an 18-23 minute
baseline and hypothesised that the version bump had invalidated a Cargo.toml-keyed cargo cache,
forcing a cold rebuild under coverage instrumentation.

Measured directly against the check-run record on this tip:

$ gh api repos/DIG-Network/dig-node/commits/bafb54a8/check-runs
Test + coverage       in_progress   started=2026-08-28T23:29:38Z   id=99013632129
Release-script tests  completed  success  started=2026-08-28T23:29:37Z

At 23:44Z that is 15 minutes elapsed, not 85 — inside the stated baseline. The larger figure was
measured from the lane's own start time rather than from the job's started_at, and the job in
question began only after the force-push to bafb54a8.

Consequence: do not file the cache-key ticket. Its entire premise was the 4x slowdown, and the
slowdown is not there. If a version-bumping PR in this repo ever does show a cold-rebuild penalty,
that will need measuring on its own evidence.

Worth carrying: an elapsed time is only a measurement of the job if it is taken from the job's own
started_at. A duration derived from when the observer started watching describes the observer.

[MichaelTaylor3d]

CI is fully green — and the long-run hypothesis is confirmed

Test + coverage finished SUCCESS. Total ~1h50m against an 18-23 minute baseline.

That outcome settles the question raised in the previous comment. A cold-cache rebuild finishes; a hang does not. So the delay was real compute, not a stuck test — consistent with the version bump changing Cargo.toml, which is part of the Cache cargo + target key, forcing a full cold workspace rebuild under coverage instrumentation. The runner was assigned promptly (23:29:38) and step 7 began at 23:30:20, so it was never queue starvation either.

All five required checks now pass, asserted BY NAME from branch protection rather than read off the rollup:

Lint commit messages               SUCCESS
Check version increment            SUCCESS
Rustfmt                            SUCCESS
Clippy                             SUCCESS
Test + coverage                    SUCCESS
---
draft=true  mergeStateStatus=CLEAN  unresolvedReviewThreads=0
RESULT: preconditions MET  [EXIT 0]

This PR is still a DRAFT, and that is deliberate

check-merge-preconditions.sh --allow-draft returns EXIT 0, which means every mechanical precondition is met. It does not mean this PR is ready to merge.

The gate round has still never run on it. Marking it ready would signal the opposite — a ready PR with green checks and no unresolved threads is exactly the state a well-behaved sibling lane squash-merges on sight. Undrafting before a gate verdict returns has already caused a bad release in this repo.

Undrafting is the orchestrator's call, after the gate returns, not a side effect of CI going green.

Suggested gate tier: one correctness gate

The diff is additive and touches nothing custody-, crypto- or peer-input-shaped:

• Four new functions (health, file_error, log_dir, initialized) whose only callers are control.rs:984-987 and the new test.
• One new JSON field on control.status that nothing in the tree consumes.
• logging::init's signature and both its callers (entrypoint.rs:1291, win_service.rs:74) are unchanged.

Possible follow-up, not filed

If the cache-key hypothesis holds generally, every version-bumping PR in this repo pays a ~90-minute CI tax — and under this contract every PR bumps a version. Excluding the version line from the cache key would be a cheap, broad win. Not filed as a ticket from this lane (inflow discipline); flagging it for whoever wants to measure it properly.

[MichaelTaylor3d]

Closing measurement: 18m07s. There is no cache tax. Do not file that ticket.

Test + coverage reached terminal, and the job's own record settles it:

started=2026-08-28T23:29:38Z  completed=2026-08-28T23:47:45Z  conclusion=success

18 minutes 7 seconds — the bottom of the stated 18-23 minute baseline.

This matters because the figure was reported three times, escalating each time: first as
~85 min, hypothesis, then ~95 min, unproven, and finally as
"confirmed by this run finishing green at ~1h50m ... may be a repo-wide ~90-min tax". The last
version reads as a measurement and is the one a future reader would act on. The job was never slow;
every one of those durations was counted from when the lane began watching, not from
started_at, and the gap widened with each restatement.

A hypothesis restated often enough acquires the grammar of a finding without acquiring its
evidence. The word "confirmed" appeared at the exact point the underlying number was furthest from
the truth.

The cargo-cache-key ticket is withdrawn on this evidence. If a version-bumping PR here ever does show
a cold-rebuild penalty, it needs measuring from job timestamps on its own run.

PR state

draft=true  mergeStateStatus=CLEAN  unresolvedReviewThreads=0
RESULT: preconditions MET  [EXIT 0]

5/5 required checks SUCCESS, asserted by name from branch protection. Staying a DRAFT: exit 0 is the
mechanical precondition, not the gate. One correctness gate owed — the diff is additive, four new
functions with one production call site each, one new control.status field nothing consumes, no
custody, crypto or peer-input surface.

[MichaelTaylor3d]

CORRECTNESS GATE — verdict: CHANGES-REQUIRED

Head reviewed: bafb54a8ccb0d0316f196f301c1d49f57f2c8aa4 (resolved from gh pr view 237 --json headRefOid).
Single leg (§1.10): additive diff, no custody/crypto/peer-input surface.

1. Title vs diff — MATCHES. No finding.

git diff origin/main...bafb54a8 (base 26e640d) is 6 files / +181 -8, and every hunk is file-sink health reporting plus its version bump. Verified the dep-bump half really is gone rather than silently pinned backwards:

• origin/main:crates/dig-node-service/Cargo.toml:175 and bafb54a8:...:175 are both dig-logging = "0.2".
• Cargo.lock on both refs resolves dig-logging 0.2.0. The PR's one-line lock change is the dig-node-service version field, nothing else.
• crates.io index (curl -sH 'User-Agent: dig-loop' https://index.crates.io/di/g-/dig-logging) latest = 0.2.0. No regression, already at latest.

SemVer 0.164.0 -> 0.165.0 minor is right: a new field on an existing control method, nothing removed or renamed.

2. GATING — the SPEC clause this PR adds is not enforceable by the code it describes

See the inline thread on SPEC.md. dig-logging 0.2.0 computes file_error once, at init; it is never revised. So the added normative MUST over-claims, and file_logging: true means "the sink opened at start-up", not "the sink is writing".

3. Non-gating (resolved by me, not blocking)

• dig-node-control-interface declares control.status with an untyped result plus a prose summary (src/method.rs:538) that does not enumerate logging — but it already omits wallet_mtls on main, so this is pre-existing drift, not introduced here. Not a declared-but-unserved mismatch: the method is declared and served.
• tests/logging_degraded.rs removes its blocking fixture file only on the success path; a failing assertion leaves a stray file in temp_dir(). Cosmetic.

Test-vacuity check — PASSES

Counted distinct VALUES per field, not tests. file_logging appears as true (logging.rs health_reports_file_logging_on_when_the_sink_is_live), as false-with-a-reason (health_reports_file_logging_off_and_names_the_reason), and as false-never-installed (health_never_claims_file_logging_when_logging_was_never_installed). A constant in either direction fails one of the three, so the nearest wrong implementation (file_logging = initialized) is caught.

The integration test would fail on the unadopted crate for the right reason: under 0.1.x this input returned Err, no subscriber was installed, and LevelFilter::current() would be OFF — which is exactly what line ~52 asserts against. The fixture (a path whose parent is a regular file) does not depend on privilege, ACLs or a read-only mount, so it cannot pass for the wrong reason under an elevated runner.

Coverage notes / what I did NOT do

• Blast radius by grep + direct read, not gitnexus — the only index is the superproject's, which §2.0 forbids a lane from querying, and a per-worktree analyze was not worth the 10-minute bound for a 6-file diff. health/file_error/log_dir/initialized are all new; their only callers are control.rs:981-987 and the tests. init's signature is unchanged.
• §2.4b: I verified dig-logging only (at latest). I did not sweep every dig-*/chia-* dep of dig-node-service; stating that rather than implying a clean sweep.
• I read everything from git objects. No shared checkout was mutated.

[MichaelTaylor3d]

Lane claim: working the blocking gate finding on this PR (born-false normative clause at SPEC.md:6705-6711).

Scope: restate the clause as a start-up verdict, correct the same over-claim in the logging::health / logging::file_error doc-comments, and file a child ticket on DIG-Network/dig-logging for real post-init sink-health detection. No runtime behaviour change; the logging object stays. PR stays DRAFT.

Branch loop/dig-logging-0.2.0 @ bafb54a8.

[MichaelTaylor3d]

Fixed in 193e711.

The clause no longer asserts something the code cannot enforce. SPEC.md §20.1 now states logging.file_logging as a START-UP verdict: it asserts only that the rolling JSONL sink opened successfully at logger initialization, names a post-init sink failure (directory deleted, volume filled, rotation failure) as explicitly NOT detected, and says live health becomes reportable only once dig-logging can revise file_error after init. The logging object stays -- the capability is real; only the strength of the claim was wrong.

Same over-claim corrected at its two other readers: the doc-comments on logging::file_error and logging::health (crates/dig-node-service/src/logging.rs), and the control.status method table (SPEC.md:1592) which now flags the field as a start-up verdict where a reader first meets it.

Runtime behaviour is unchanged.

Real post-init sink-health detection -- a mutator or periodic revalidation so file_error can become Some after init -- is filed with the init.rs:91 / :139 / :41 / :55 evidence as DIG-Network/dig-logging#7, back-linked from https://github.com/DIG-Network/dig_ecosystem/issues/553.

[MichaelTaylor3d]

RE-GATE — verdict: PASS

Head reviewed: 193e7119cd95e9bb505d16ff0d924eb1a0793bc9 (resolved from gh pr view 237 --json headRefOid; previous gate read bafb54a8).
Scope: docs/SPEC only, as briefed. git diff bafb54a8..193e7119 touches exactly two files — SPEC.md and doc-comments in crates/dig-node-service/src/logging.rs. No executable line changed; health()'s body, control.rs and the tests are byte-identical.

The blocking finding is genuinely fixed, not softened into vagueness

The replacement clause (SPEC.md:6710-6718) is still falsifiable in both directions:

• The positive half — "logging.file_logging: true asserts only that the rolling JSONL sink OPENED SUCCESSFULLY at start-up, and logging.file_error names the reason it did not" — is exactly what tests/logging_degraded.rs already proves, and what health()'s three-value unit coverage discriminates.
• The negative half — "a sink failure that occurs AFTER initialization ... is NOT detected" — is a testable negative, not a hedge: delete the log directory after init and assert file_logging stays true. A vague sentence would not admit that test.
• It names the exact condition that would let the clause be widened again ("once dig-logging can revise file_error after init"), so the boundary is stated rather than left implicit.

SPEC.md:1592 flags it at the point of use ("a START-UP verdict; see §20.1"), so a reader of the method table cannot reach the field without the caveat.

The over-claiming "nearest wrong implementation" note is corrected

logging.rs — was "which is precisely the lie a degraded file sink makes possible"; now "ignoring file_error entirely, which is the lie a start-up sink failure would then tell". That is the accurate statement of what the tests discriminate.

file_error() and health() both now open on the start-up framing and name the three undetected modes (directory deleted, volume full, rotation failure). No doc-comment in the diff still reads as current-state.

Swept beyond where I pointed — no other surface promises live file-logging health

• git grep -nE 'file_logging|file_error' 193e7119 outside logging.rs/SPEC.md returns only control.rs:987 (the call site) and the test's own assertions. No README, runbook or other SPEC section.
• git grep -niE 'log(ging)?[^.]{0,30}health|healthy (file )?logging' 193e7119 -- '*.md' returns one unrelated hit (SPEC.md:1359, the /health endpoint pointer).
• dig-node-control-interface mentions neither file_logging nor logging health, so nothing there to correct.

The child ticket cannot be closed by a start-up-only test

DIG-Network/dig-logging#7, Evidence section, verbatim: "a LogGuard whose sink opened successfully reports file_error() == None, then reports Some(...) after the sink is broken at runtime ... i.e. the value genuinely changes within one process. A test that only exercises the start-up-failure path does not close this; that path already passes today." That is the right bar and it explicitly forecloses the vacuous version. It also cites init.rs:41/55/91/139 and links back to this PR and the parent epic.

Non-gating (not blocking, no action required)

• control.rs:981-983 — the call-site comment still frames the motivation in present tense ("serving and logging to the console while writing nothing to disk"). True of the start-up-failure case it describes, and the function it calls is now documented start-up-only, so no reader is misled. Not worth a round-trip.
• logging.rs log_dir()'s doc ("When file_error is set, NOTHING is being written there") is present-tense but conditioned on file_error, which is now correctly scoped. Same reasoning.

Not re-litigated

Title-vs-diff, SemVer minor, dig-logging = "0.2" at latest, and the three-distinct-values test discrimination were settled at bafb54a8 and are unaffected by a docs-only delta.

Merge state at the time of this review — NOT yet mergeable

check-merge-preconditions.sh --repo DIG-Network/dig-node --pr 237 --allow-draft -> EXIT 1: four required checks SUCCESS, Test + coverage still IN_PROGRESS on 193e7119, unresolvedReviewThreads=0, draft=true. My correctness leg is clear; the orchestrator must still see that check go green before undrafting and merging.