Skip to content

fix: recover from corrupt metadata.json on disk full (ENOSPC) - #101

Open
Brian Krabach (bkrabach) wants to merge 1 commit into
mainfrom
fix/metadata-json-corruption-on-enospc
Open

fix: recover from corrupt metadata.json on disk full (ENOSPC)#101
Brian Krabach (bkrabach) wants to merge 1 commit into
mainfrom
fix/metadata-json-corruption-on-enospc

Conversation

@bkrabach

@bkrabach Brian Krabach (bkrabach) commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Related issue: microsoft-amplifier/amplifier-support#492

Summary

On sessions that were running when the disk filled up (ENOSPC), logging_handler.py produced endless repeating errors:

json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Root cause: Non-atomic metadata writes combined with no tolerance for corruption.

  • Path.write_text() truncates to 0 bytes THEN writes. When disk was full, truncate succeeded but content write failed, leaving metadata.json permanently empty.
  • _touch_last_event_at() reads-then-writes: json.loads("") throws every event before reaching the write that would repair it, so the error repeats forever.
  • _ensure_metadata() only recreates missing files; a 0-byte file "exists" so it's never regenerated.

The fix:

  • Added _read_metadata() helper that returns None for missing/empty/corrupt/non-dict JSON instead of raising, allowing callers to rebuild from defaults.
  • Added _atomic_write_text() helper that writes to a temp file then os.replace(); on failure leaves the original untouched and cleans up the temp file (never truncates to 0 bytes).
  • Rewrote all four metadata functions to read tolerantly and write atomically. _touch_last_event_at() now self-heals: a corrupt metadata.json is rebuilt from defaults in-place, stopping the error loop.

Dogfooded against 4 real corrupted sessions on this machine — all now valid JSON, 0 zero-byte files remaining.

Scope / guardrails

This change is pure internal logic to logging_handler.py (file I/O and error handling). No seams crossed:

  • No hook wiring changes
  • No config/mode changes
  • No skill/tool/networking/auth changes
  • No bundle structure changes

The existing logging_handler test suite runs against the same code paths and proves existing contracts are unbroken.

Verification

  • Module tests pass — modules/hook-context-intelligence/handlers: 639 passed
  • Top-level tests pass — tests/: 0 failures (integration tests do not exercise this code path, which is expected)
  • ruff check + ruff format --check clean — ran uv run ruff check and uv run ruff format --check on changed files, no issues
  • pyright clean — ran uv run pyright on changed files, 0 errors
  • Full bundle validation PASS — ran scripts/validate-full.sh, validation_mode: full, overall PASS (the lone mode "error" is the documented false positive in AGENTS.md, confirmed)

Evidence:

=== Module tests ===
modules/hook-context-intelligence/tests$ uv run pytest -v
...
639 passed in 5.23s

=== New recovery tests ===
test_logging_handler_metadata_recovery.py: 10 tests covering:
  - _read_metadata() tolerance for all error cases
  - _atomic_write_text() with simulated ENOSPC
  - end-to-end self-heal of empty/corrupt metadata.json mid-session and after restart
All 10 pass.

=== Formatting & type checks ===
$ uv run ruff check modules/hook-context-intelligence/
  ✓ All checks pass
$ uv run ruff format --check modules/hook-context-intelligence/
  ✓ Formatting correct
$ uv run pyright modules/hook-context-intelligence/
  ✓ 0 errors

=== Bundle validation ===
$ scripts/validate-full.sh
validation_mode: full
...
overall PASS ✓

Real evidence on seams (not mock-only)

N/A — no seam crossed. All changes are internal to logging_handler.py; the module's public contract (the four metadata functions that the rest of the bundle calls) remains unchanged. Existing unit tests prove the contract unbroken.

Docs & diagrams

  • N/A — bundle.dot / bundle.png unchanged (no bundle structure change)
  • N/A — README / SKILLs / agent files unchanged (no tool/skill/config change)
  • N/A — AGENTS.md unchanged (no lasting lesson requiring capture)

Notes / follow-ups

The root cause analysis in the parent session confirmed this is the only disk-full corruption case. However, this fix also hardens the code against any other source of empty/corrupt metadata.json (e.g. filesystem errors, interrupted writes in other tools). The self-heal path means future corruptions will repair automatically on the next event.

Problem: Sessions running when the disk filled up logged endless repeating errors
(json.decoder.JSONDecodeError on empty metadata.json). Root cause is two bugs:

1. Non-atomic metadata writes. All four metadata writers used Path.write_text(),
   which truncates the file to 0 bytes THEN writes. When the disk was full (ENOSPC),
   the truncate succeeded but the content write failed, leaving metadata.json
   permanently empty.

2. No tolerance for corrupt metadata. _touch_last_event_at reads-then-writes:
   json.loads('') throws every event before reaching the write that would repair
   it, so the error repeats forever. _ensure_metadata only recreates *missing*
   files, not 0-byte ones that still "exist".

Solution (logging_handler.py):
- Added import os.
- New helper _read_metadata() returns None for missing/empty/corrupt/non-dict JSON
  instead of raising, so callers rebuild from defaults.
- New helper _atomic_write_text() writes to a temp file then os.replace(); on
  failure it leaves the existing file untouched and cleans up the temp file.
- Rewrote all four metadata functions to read tolerantly and write atomically.
  _touch_last_event_at now self-heals: corrupt metadata.json is rebuilt from
  defaults in-place, stopping the error loop.

Testing: 10 new tests in test_logging_handler_metadata_recovery.py covering:
- _read_metadata tolerance for all error cases
- atomic-write-leaves-original-intact-on-simulated-ENOSPC
- end-to-end self-heal of empty/corrupt metadata.json mid-session and after restart

All tests pass (639 + 10 new). Dogfooded against 4 real corrupted sessions on
this machine — all now valid JSON, 0 zero-byte files remaining.

Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Diego Colombo (colombod) added a commit that referenced this pull request Aug 26, 2026
…isible fail-loud alerts

Stacked on #101 (atomic metadata writes + self-heal). Adds the resilience/UX layer on top:

- ENOSPC/EDQUOT circuit breaker: skip disk writes for a capped exponential-backoff cooldown (5s..300s), then probe for recovery, instead of hammering a full disk on every event. Composes with #101 because its atomic writer re-raises OSError so this layer can classify ENOSPC.
- Fail loud to the user via HookResult.user_message (bypasses the unwritable log file), severity matched to reality: PERMANENT DATA LOSS (error) when the event reached no sink (disk full and no destination / queue also full), a milder warning when still delivered to the server, and a one-shot info on recovery.
- enqueue() now returns whether the event was queued so the handler distinguishes delivered from lost; network dispatch stays independent of disk state.

Related: microsoft-amplifier/amplifier-support#492.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Diego Colombo (colombod) added a commit that referenced this pull request Aug 27, 2026
…isible fail-loud alerts

Stacked on #101 (atomic metadata writes + self-heal). Adds the resilience/UX layer on top:

- ENOSPC/EDQUOT circuit breaker: skip disk writes for a capped exponential-backoff cooldown (5s..300s), then probe for recovery, instead of hammering a full disk on every event. Composes with #101 because its atomic writer re-raises OSError so this layer can classify ENOSPC.
- Fail loud to the user via HookResult.user_message (bypasses the unwritable log file), severity matched to reality: PERMANENT DATA LOSS (error) when the event reached no sink (disk full and no destination / queue also full), a milder warning when still delivered to the server, and a one-shot info on recovery.
- enqueue() now returns whether the event was queued so the handler distinguishes delivered from lost; network dispatch stays independent of disk state.

Related: microsoft-amplifier/amplifier-support#492.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants