Skip to content

resolve the data directory per call instead of capturing it at import #1477

Description

@bendrucker

Four modules capture getPlannotatorDataDir() at import time and derive their paths from that value, so they use whatever the environment variable was set to when the module first loaded. Every other consumer resolves it per call.

  • packages/shared/improvement-hooks.ts: DATA_DIR, plus HOOKS_BASE_DIR and LEGACY_BASE_DIR
  • packages/server/codex-review.ts: DATA_DIR, DEBUG_LOG_PATH, SCHEMA_DIR, SCHEMA_FILE
  • packages/server/tour/tour-review.ts: TOUR_SCHEMA_DIR, TOUR_SCHEMA_FILE
  • packages/server/guide/guide-review.ts: GUIDE_SCHEMA_DIR, GUIDE_SCHEMA_FILE

These constants are module-private, so no signatures change. vendor.sh copies improvement-hooks.ts into the Pi extension. Nothing outside tests sets PLANNOTATOR_DATA_DIR after startup, so production behavior is unchanged.

Motivation

storage.ts captured the value the same way until #1437. There it broke annotate's submission-record tests on Linux runners, and the initial diagnosis attributed the failure to path normalization in an unrelated feature.

embedded.ts loads @plannotator/server with await import(...). That call ran during a test that had set PLANNOTATOR_DATA_DIR to its own temp directory, so storage.ts captured that directory and used it for the remainder of the process.

packages/server/index.ts imports improvement-hooks.ts, so the same dynamic import loads it under the same conditions. The other three modules are imported by review.ts, which that dynamic import does not reach. 38 test files set the variable at runtime, and Bun's test-file order is filesystem-dependent, so the captured value varies as test files are added.

Approach

Replacing DATA_DIR alone is insufficient. DEBUG_LOG_PATH, SCHEMA_FILE, TOUR_SCHEMA_FILE, and GUIDE_SCHEMA_FILE are derived at module scope and stay fixed.

A regression test must import the module before setting the environment variable, because the reverse order passes with or without the fix. packages/shared/storage.test.ts is the model. improvement-hooks.test.ts currently spawns a subprocess with a modified HOME for the same reason, and can set the variable in-process after this change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions