fix: split read-only data volume to allow state file writes (#85) #86

Claude · 2026-03-17T18:35:03Z

Claude commented

2026-03-17 18:35:03 +00:00

Closes #85

Summary

The data/ volume was mounted :ro to protect Google Sheets credentials, but this also prevented all state trackers from persisting their JSON files. save_data() calls were silently failing (logged as errors), so scorecard removals, voice channel state, trade channel state, and soak data were never written to disk.

Changes

docker-compose.yml — split the single :ro volume into two:

Credentials file mounted read-only at file level: ${SHEETS_CREDENTIALS_HOST_PATH:-./data/major-domo-service-creds.json}:/app/data/major-domo-service-creds.json:ro
New writable state volume: ${STATE_HOST_PATH:-./storage}:/app/storage:rw

5 tracker files — default data_file path changed from data/ → storage/:

commands/gameplay/scorecard_tracker.py — storage/scorecards.json
commands/voice/tracker.py + cleanup_service.py — storage/voice_channels.json
commands/transactions/trade_channel_tracker.py — storage/trade_channels.json
commands/soak/tracker.py — storage/soak_data.json

.gitignore — added storage/ to ignore runtime state files

Deployment notes

On akamai, update container-data/major-domo/.env:

# Before (directory):
SHEETS_CREDENTIALS_HOST_PATH=./data

# After (full file path):
SHEETS_CREDENTIALS_HOST_PATH=./data/major-domo-service-creds.json
STATE_HOST_PATH=./storage

Also migrate existing state files if any were written locally:

mkdir -p ./storage
cp ./data/*.json ./storage/ 2>/dev/null || true

Test results

953 passed, 2 skipped — no regressions.

Other observations

commands/gameplay/CLAUDE.md and commands/voice/CLAUDE.md still reference data/ paths for state files — these should be updated separately.
.env.example also has the updated SHEETS_CREDENTIALS_HOST_PATH and new STATE_HOST_PATH docs, but it is gitignored so it was not included in this commit. The file was updated locally.
The linter auto-reformatted tracker.py, cleanup_service.py, trade_channel_tracker.py, and soak/tracker.py (single→double quotes, trailing commas) when those files were edited. The reformatting is cosmetic and all tests pass.

Closes #85 ## Summary The `data/` volume was mounted `:ro` to protect Google Sheets credentials, but this also prevented all state trackers from persisting their JSON files. `save_data()` calls were silently failing (logged as errors), so scorecard removals, voice channel state, trade channel state, and soak data were never written to disk. ## Changes **`docker-compose.yml`** — split the single `:ro` volume into two: - Credentials file mounted read-only at file level: `${SHEETS_CREDENTIALS_HOST_PATH:-./data/major-domo-service-creds.json}:/app/data/major-domo-service-creds.json:ro` - New writable state volume: `${STATE_HOST_PATH:-./storage}:/app/storage:rw` **5 tracker files** — default `data_file` path changed from `data/` → `storage/`: - `commands/gameplay/scorecard_tracker.py` — `storage/scorecards.json` - `commands/voice/tracker.py` + `cleanup_service.py` — `storage/voice_channels.json` - `commands/transactions/trade_channel_tracker.py` — `storage/trade_channels.json` - `commands/soak/tracker.py` — `storage/soak_data.json` **`.gitignore`** — added `storage/` to ignore runtime state files ## Deployment notes On `akamai`, update `container-data/major-domo/.env`: ``` # Before (directory): SHEETS_CREDENTIALS_HOST_PATH=./data # After (full file path): SHEETS_CREDENTIALS_HOST_PATH=./data/major-domo-service-creds.json STATE_HOST_PATH=./storage ``` Also migrate existing state files if any were written locally: ```bash mkdir -p ./storage cp ./data/*.json ./storage/ 2>/dev/null || true ``` ## Test results 953 passed, 2 skipped — no regressions. ## Other observations - `commands/gameplay/CLAUDE.md` and `commands/voice/CLAUDE.md` still reference `data/` paths for state files — these should be updated separately. - `.env.example` also has the updated `SHEETS_CREDENTIALS_HOST_PATH` and new `STATE_HOST_PATH` docs, but it is gitignored so it was not included in this commit. The file was updated locally. - The linter auto-reformatted `tracker.py`, `cleanup_service.py`, `trade_channel_tracker.py`, and `soak/tracker.py` (single→double quotes, trailing commas) when those files were edited. The reformatting is cosmetic and all tests pass.

Claude added 1 commit 2026-03-17 18:35:04 +00:00

fix: split read-only data volume to allow state file writes (#85 )

Build Docker Image / build (pull_request) Successful in 57s

Details

03dd449551

The data/ volume was mounted :ro to protect Google Sheets credentials,
but this also prevented all state trackers from persisting JSON files
(scorecards, voice channels, trade channels, soak data), causing silent
save failures and stale data accumulating across restarts.

- Mount only the credentials file as :ro (file-level mount)
- Add a separate :rw storage/ volume for runtime state files
- Move all tracker default paths from data/ to storage/
- Add STATE_HOST_PATH env var (defaults to ./storage)
- Update SHEETS_CREDENTIALS_HOST_PATH semantics: now a file path
  (e.g. ./data/major-domo-service-creds.json) instead of a directory
- Add storage/ to .gitignore

Closes #85

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-03-17 18:35:16 +00:00

bug: read-only volume mount prevents scorecard tracker from persisting state #85

Claude added the

ai-reviewing

label 2026-03-17 18:45:49 +00:00

Claude reviewed 2026-03-17 18:48:05 +00:00

Claude left a comment

AI Code Review

Files Reviewed

docker-compose.yml (modified)
commands/gameplay/scorecard_tracker.py (modified)
commands/voice/tracker.py (modified)
commands/voice/cleanup_service.py (modified)
commands/transactions/trade_channel_tracker.py (modified)
commands/soak/tracker.py (modified)
.gitignore (modified)

Findings

Correctness

All 5 trackers updated consistently: data/ → storage/ in default data_file param.
All 5 trackers already have self.data_file.parent.mkdir(exist_ok=True) in __init__ (unchanged from before). This means storage/ is created automatically on first write even without a volume mount, which is correct.
WORKDIR /app in the Dockerfile confirms relative path storage/scorecards.json resolves to /app/storage/scorecards.json — matching the volume mount target.
File-level credentials mount (/app/data/major-domo-service-creds.json:ro) is correct. Docker will create /app/data/ automatically if it doesn't exist in the container image.
mergeable: true — no conflicts with current main.

Security

Credentials file is still mounted :ro at the file level — no regression on the credentials protection goal.
State files go to a separate :rw volume, correctly isolating writable data from secrets.
No hardcoded credentials or secrets introduced.

Style & Conventions

Cosmetic linter reformatting (single→double quotes, trailing commas, line wrapping) is acknowledged in PR body and all tests pass. No convention violations.

Suggestions

Deployment risk (not a code issue): SHEETS_CREDENTIALS_HOST_PATH now expects a full file path, not a directory. If production .env still has SHEETS_CREDENTIALS_HOST_PATH=./data, Docker will bind-mount the ./data directory as a file at /app/data/major-domo-service-creds.json, making it a directory inside the container and breaking Google Sheets auth silently. The PR body documents the migration, but this is worth calling out as a high-impact step during deploy.
commands/gameplay/CLAUDE.md and commands/voice/CLAUDE.md still reference data/ paths — noted in PR body for a follow-up. Fine to defer.

Verdict: COMMENT

Clean, correct fix. Root cause (single :ro volume blocking all state writes) is accurately diagnosed and properly resolved by splitting at the file level. All trackers updated consistently, directory auto-creation works, and the deployment migration is documented. Posting as COMMENT — Gitea does not allow self-approval.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `docker-compose.yml` (modified) - `commands/gameplay/scorecard_tracker.py` (modified) - `commands/voice/tracker.py` (modified) - `commands/voice/cleanup_service.py` (modified) - `commands/transactions/trade_channel_tracker.py` (modified) - `commands/soak/tracker.py` (modified) - `.gitignore` (modified) ### Findings #### Correctness - All 5 trackers updated consistently: `data/` → `storage/` in default `data_file` param. - All 5 trackers already have `self.data_file.parent.mkdir(exist_ok=True)` in `__init__` (unchanged from before). This means `storage/` is created automatically on first write even without a volume mount, which is correct. - `WORKDIR /app` in the Dockerfile confirms relative path `storage/scorecards.json` resolves to `/app/storage/scorecards.json` — matching the volume mount target. - File-level credentials mount (`/app/data/major-domo-service-creds.json:ro`) is correct. Docker will create `/app/data/` automatically if it doesn't exist in the container image. - `mergeable: true` — no conflicts with current `main`. #### Security - Credentials file is still mounted `:ro` at the file level — no regression on the credentials protection goal. - State files go to a separate `:rw` volume, correctly isolating writable data from secrets. - No hardcoded credentials or secrets introduced. #### Style & Conventions - Cosmetic linter reformatting (single→double quotes, trailing commas, line wrapping) is acknowledged in PR body and all tests pass. No convention violations. #### Suggestions - **Deployment risk (not a code issue):** `SHEETS_CREDENTIALS_HOST_PATH` now expects a full file path, not a directory. If production `.env` still has `SHEETS_CREDENTIALS_HOST_PATH=./data`, Docker will bind-mount the `./data` _directory_ as a file at `/app/data/major-domo-service-creds.json`, making it a directory inside the container and breaking Google Sheets auth silently. The PR body documents the migration, but this is worth calling out as a high-impact step during deploy. - `commands/gameplay/CLAUDE.md` and `commands/voice/CLAUDE.md` still reference `data/` paths — noted in PR body for a follow-up. Fine to defer. ### Verdict: COMMENT Clean, correct fix. Root cause (single `:ro` volume blocking all state writes) is accurately diagnosed and properly resolved by splitting at the file level. All trackers updated consistently, directory auto-creation works, and the deployment migration is documented. Posting as COMMENT — Gitea does not allow self-approval. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-17 18:48:53 +00:00

cal changed target branch from main to next-release

2026-03-20 15:27:47 +00:00

cal merged commit 18ab1393c0 into next-release

2026-03-20 15:28:15 +00:00

cal deleted branch ai/major-domo-v2-85

2026-03-20 15:28:15 +00:00

cal referenced this issue from a commit