Skip to content

fix(run): purge local stores from the correct directory - #1404

Open
l2ysho wants to merge 2 commits into
masterfrom
claude/master-pipeline-check-failures-2dbef0
Open

fix(run): purge local stores from the correct directory#1404
l2ysho wants to merge 2 commits into
masterfrom
claude/master-pipeline-check-failures-2dbef0

Conversation

@l2ysho

@l2ysho l2ysho commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Note

After we merged apify/actor-templates#866 this popped up. Actually it is missing scenario in run --purge not covered by tests and also a bug. + one windows bug found in review.

Why master is red

apify/actor-templates #866 added .actor/input_schema.json to every template on 2026-09-08 15:15 UTC. The tests download the live template, so the change hit CI with no commit here — the run before it was green.

With a schema present, apify run --purge writes a merged temp input file, so it turns crawlee's purge-on-start off (that purge only preserves INPUT* and would delete the temp file) and purges by itself instead. That path was broken:

  • useModuleVersion ran the crawlee version probe without cwd, so under the test cwd mock it looked in the repo root instead of the project. Production is unaffected — execa defaults to process.cwd() — but nothing could exercise the branch.
  • purgeDefault* resolved storage paths against the OS working directory, so under the same mock they returned early and deleted nothing — a silent no-op that reads exactly like a pass.

Two user-facing fixes

Found while reviewing the code around the above, both pre-existing:

  • Windows: checkIfStorageIsEmpty interpolated storage\key_value_stores\default into glob patterns, which reads backslashes as escapes. The !…/INPUT.* negations never matched, so apify run without --purge always warned "the storage directory contains a previous state" — even with nothing but the input file. Confirmed empirically against the pinned tinyglobby.
  • Absolute APIFY_LOCAL_STORAGE_DIR: getLocalInput and the input paths in validateAndStoreInput used join(cwd, …), producing <cwd>/<abs>. Now resolve, matching the purge helpers.

Changes

File Change
src/lib/utils.ts Windows glob fix; purge helpers and getLocalInput resolve against the command's directory; redundant existsSync guards dropped
src/commands/run.ts Legacy-storage rename and input/temp-input paths resolve against the command's directory
src/lib/hooks/useModuleVersion.ts Pass cwd to the version probe
test/local/lib/utils-purge-storages.test.ts New — the purge helpers and checkIfStorageIsEmpty, directly
test/local/lib/useModuleVersion.test.ts New — the version probe resolves modules from the project
test/local/commands/run-without-crawlee.test.ts New — the purge branch taken when crawlee is absent
test/local/commands/run.test.ts Input tests own the schema instead of racing the template's

The last one fixes the Windows/macOS half of the failure. .actor/INPUT_SCHEMA.json is first in DEFAULT_INPUT_SCHEMA_PATHS, and on case-insensitive file systems it matched the template's .actor/input_schema.json, so an empty schema won over the test's own. The tests now delete the template's file, so exactly one schema exists on every platform.

Coverage

Every change here has a test that fails without it. checkIfStorageIsEmpty had no coverage at all before. The no-crawlee test asserts a marker file the Actor writes — without it, "the Actor never ran" and "the purge worked" both leave an empty storage folder, which is how the original bug stayed hidden.

pnpm run test:local: 550 passed, 4 skipped, 0 failed. Lint, format, tsc --noEmit, build clean. No dependency changes, so no install-size impact.

Merge order

Node 20 legs fail here on create.test.ts > should skip installing optional dependencies — red on master since 2026-09-04, four days before the templates change, and unrelated to this PR. #1403 drops those legs; its own legs fail on the bug this PR fixes. Merging #1403 first lets this one get a fully green run before it lands.

🤖 Generated with Claude Code

@l2ysho
l2ysho requested a review from DaveHanns as a code owner September 8, 2026 22:23
@github-actions github-actions Bot added this to the 149th sprint - Tooling team milestone Sep 8, 2026
@github-actions github-actions Bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Sep 8, 2026
@l2ysho l2ysho added t-builders Issues owned by the Builders team. and removed t-tooling Issues with this label are in the ownership of the tooling team. labels Sep 8, 2026
@l2ysho
l2ysho marked this pull request as draft September 8, 2026 22:24
l2ysho and others added 2 commits September 9, 2026 16:27
`apify run --purge` hands the purge to crawlee unless a temp input file is in
play; templates now ship an input schema, so that file is always written and the
CLI has to purge by itself. That path was broken in two places:

- `useModuleVersion` probed for crawlee without passing `cwd`, so the subprocess
  looked in whatever directory the CLI process sat in
- the `purgeDefault*` helpers resolved storage paths against the OS working
  directory, which silently made them no-ops under the test cwd mock

Both now resolve against `process.cwd()`. Adds unit tests for the helpers and a
test for the branch taken when crawlee is absent; the run tests write their input
schema to `.actor/`, where templates keep it, so it is not shadowed on
case-insensitive file systems.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up from review of the purge fix. Same class of defect in the code around
it, plus the coverage that was missing.

- `checkIfStorageIsEmpty` fed backslash paths to glob on Windows, which reads
  them as escapes, so the `!.../INPUT.*` negations never matched and every
  `apify run` without `--purge` warned about leftover state
- the legacy `apify_storage` rename and the input/temp-input paths in
  `validateAndStoreInput` resolve against the command's directory
- `getLocalInput` used `join`, which breaks on an absolute
  `APIFY_LOCAL_STORAGE_DIR`
- dropped the redundant storage-dir `existsSync` guards from the purge helpers

Adds `useModuleVersion` tests, `checkIfStorageIsEmpty` coverage, and drops the
template's input schema in the run input tests so one schema exists on every
file system.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@l2ysho
l2ysho force-pushed the claude/master-pipeline-check-failures-2dbef0 branch from 6c144be to f3ad9d8 Compare September 9, 2026 14:27
@l2ysho
l2ysho marked this pull request as ready for review September 9, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-builders Issues owned by the Builders team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants