Skip to content

refactor: move legacy-logs migration into local storage adapter#53

Merged
lis186 merged 1 commit into
mainfrom
refactor/legacy-migration-into-storage
Jun 7, 2026
Merged

refactor: move legacy-logs migration into local storage adapter#53
lis186 merged 1 commit into
mainfrom
refactor/legacy-migration-into-storage

Conversation

@lis186

@lis186 lis186 commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Addresses the require-time migration hygiene item raised during the #50/#51/#52 CCXRAY_HOME work (Codex review of the merged state flagged it as should-fix).

Problem

server/config.js ran the one-time legacy-logs migration as a top-level side effect at require time:

  1. Fires on every import — 9 modules require('./config'), so the mkdir + file-move ran far beyond startup.
  2. Ignores STORAGE_BACKEND — under STORAGE_BACKEND=s3, it created a local logs dir and moved legacy logs into a directory the server never reads.

Approach (chosen from a 5-way spike + Codex comparison: B > C > A > D > E)

Fold the migration into the local storage adapter's init() (server/storage/local.js), where the logs dir is already created:

  • Backend-gated by construction — only the local adapter has this logic; storage/index.js passes legacyDir solely on the local branch, so S3/R2 can never run it (no flag to forget).
  • Guard preservedinit() snapshots whether the logs dir existed before mkdir, so migration only ever runs into a freshly-created dir (mirrors the original if (!existsSync(LOGS_DIR)), never clobbers populated logs).
  • Best-effort — a failed rename is caught and logged, never crashing startServer()/init() (matches the original catch-and-log).
  • config.js becomes pure — drops fs, path, and LEGACY_LOGS_DIR; zero filesystem side effects at import.
  • No new startup call siteindex.js already await config.storage.init().

Why not the alternatives

  • E (remove auto-migration + warn) was a risky behavior change pointing at an unwired command.
  • A/C/D keep an existsSync(logsDir) guard that is only valid because they run before storage.init() — fragile ordering. B runs inside init() via the existed-snapshot, so the guard is robust.

Tests

Previously untested. Adds 6 focused tests (test/legacy-migration.test.js): require-time purity, fresh migrate, no-sentinel skip, existing-dir no-clobber, no-legacyDir (S3 path), and best-effort error handling.

Verification

  • node -c on all changed files.
  • Full suite: 778/778 (was 772; +6 new).

Out of scope

ratelimit-log.js:78 dir-ensure nit — intentionally left as-is (it's an intentional fire-and-forget design; documented in the #51 discussion).

The one-time legacy-logs migration ran as a top-level side effect in
config.js at require time. Because 9 modules import config.js, it fired far
beyond startup; and it ran regardless of STORAGE_BACKEND, so an S3/R2 setup
would create a local logs dir and move legacy logs into a directory the
server never reads.

Fold the migration into the local storage adapter's init() (server/storage/
local.js), where the logs dir is already created. Only the local adapter has
this logic, so non-local backends are excluded by construction — no backend
flag to forget. The migration is gated to a freshly-created logs dir via an
existed-before-mkdir snapshot (mirrors the original guard) and is best-effort:
a failed rename is caught and logged, never crashing startup. config.js loses
its fs/path imports and LEGACY_LOGS_DIR and is now side-effect-free at import.

The previously-untested migration gains 6 focused tests (require purity, fresh
migrate, no-sentinel skip, existing-dir no-clobber, no-legacyDir/S3 path,
best-effort error handling).

Closes #51 (the require-time migration item; the ratelimit-log dir-ensure nit
is intentionally left as-is, documented in the issue discussion).
@lis186 lis186 merged commit f065c91 into main Jun 7, 2026
2 checks passed
@lis186 lis186 deleted the refactor/legacy-migration-into-storage branch June 7, 2026 05:20
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.

1 participant