diff --git a/.github/workflows/cicd.yml b/.github/workflows/cicd.yml index 9d5dce58c4..126771fbb5 100644 --- a/.github/workflows/cicd.yml +++ b/.github/workflows/cicd.yml @@ -36,7 +36,7 @@ jobs: matrix: check: [format, lint, typecheck] steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno - name: Run ${{ matrix.check }} run: | @@ -56,12 +56,18 @@ jobs: timeout-minutes: 15 name: tests (integration) steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno with: warm-cache: "true" warm-redis-cache: "true" - - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 + with: + node-version: "24" + package-manager-cache: false + - name: Run Node cache-link compatibility tests + run: node ./tests/node/run-tests.mjs 'src/utils/cache-dir.test.ts' + - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 with: timeout_minutes: 10 max_attempts: 3 @@ -78,7 +84,7 @@ jobs: matrix: shard: [1, 2, 3, 4, 5, 6, 7, 8] steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno with: warm-cache: "true" @@ -91,7 +97,7 @@ jobs: echo "duration=$(($(date +%s) - START))s" >> "$GITHUB_OUTPUT" - name: Upload unit coverage lcov if: ${{ !cancelled() }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: coverage-shard-${{ matrix.shard }} path: coverage-shard-${{ matrix.shard }}/lcov.info @@ -130,10 +136,10 @@ jobs: echo "::error::Coverage shards finished with $COVERAGE_SHARDS_RESULT" exit 1 fi - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno - name: Download unit coverage lcov files - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: coverage-profiles pattern: coverage-shard-* @@ -152,7 +158,7 @@ jobs: timeout-minutes: 25 name: tests (rsc browser e2e) steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno with: warm-cache: "true" @@ -194,7 +200,7 @@ jobs: sudo rm -rf /var/lib/apt/lists/* || true sleep $((attempt * 20)) done - - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 + - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 with: timeout_minutes: 10 max_attempts: 2 @@ -211,7 +217,7 @@ jobs: timeout-minutes: 60 name: tests (binary e2e) steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno with: warm-cache: "true" @@ -253,7 +259,7 @@ jobs: sudo rm -rf /var/lib/apt/lists/* || true sleep $((attempt * 20)) done - - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 + - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 with: timeout_minutes: 30 max_attempts: 2 @@ -274,11 +280,11 @@ jobs: timeout-minutes: 30 name: tests (npm install smoke) steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno with: warm-cache: "true" - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 with: node-version: "24" package-manager-cache: false @@ -295,11 +301,11 @@ jobs: timeout-minutes: 15 name: tests (sentry runtime packages) steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno with: warm-cache: "true" - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 with: node-version: "24" package-manager-cache: false @@ -317,7 +323,7 @@ jobs: name: tests (split mode) continue-on-error: true steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno - name: Compile binary @@ -375,7 +381,7 @@ jobs: is_stable: ${{ steps.check.outputs.is_stable }} stable_release_requested: ${{ steps.check.outputs.stable_release_requested }} steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 - name: Detect release type @@ -411,7 +417,7 @@ jobs: if: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && github.event_name == 'pull_request' }} runs-on: ubuntu-latest steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno - run: deno task build:prepare - name: Verify proxy dependency lock is current @@ -474,7 +480,7 @@ jobs: entrypoint: cli/main.ts profile: full steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno - run: deno task build:prepare @@ -505,7 +511,7 @@ jobs: shell: bash run: bash scripts/build/smoke-proxy-memory.sh ./veryfront-proxy-linux-x64 - - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: ${{ matrix.name }} path: ${{ matrix.name }} @@ -519,18 +525,27 @@ jobs: prerelease: if: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && needs.version-check.outputs.is_stable == 'false' }} - needs: [ci, tests, coverage, tests-binary-e2e, tests-npm-install-smoke, tests-sentry-runtime-packages, build-binaries, version-check] + needs: [ + ci, + tests, + coverage, + tests-binary-e2e, + tests-npm-install-smoke, + tests-sentry-runtime-packages, + build-binaries, + version-check, + ] runs-on: ubuntu-latest environment: production permissions: contents: write id-token: write steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: ./.github/actions/setup-deno - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 with: node-version: "24" registry-url: "https://registry.npmjs.org" @@ -551,7 +566,7 @@ jobs: deno task build:npm scripts/ci/publish-npm-packages.sh rc-publish - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: binaries @@ -564,7 +579,7 @@ jobs: - name: Create release GitHub App token id: release-app-token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: client-id: ${{ vars.VERYFRONT_RELEASE_APP_CLIENT_ID }} private-key: ${{ secrets.VERYFRONT_RELEASE_APP_PRIVATE_KEY }} @@ -609,7 +624,7 @@ jobs: $SBOMS - name: Trigger server deploy - uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 + uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 with: token: ${{ steps.release-app-token.outputs.token }} repository: veryfront/veryfront-server @@ -617,7 +632,7 @@ jobs: client-payload: '{"version": "${{ steps.version.outputs.version }}"}' - name: Trigger job-runner deploy - uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 + uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 with: token: ${{ steps.release-app-token.outputs.token }} repository: veryfront/veryfront-job-runner @@ -625,7 +640,7 @@ jobs: client-payload: '{"version": "${{ steps.version.outputs.version }}"}' - name: Trigger sandbox deploy - uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 + uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 with: token: ${{ steps.release-app-token.outputs.token }} repository: veryfront/veryfront-sandbox @@ -640,14 +655,23 @@ jobs: release: if: ${{ (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) && needs.version-check.outputs.is_stable == 'true' && needs.version-check.outputs.stable_release_requested == 'true' }} - needs: [ci, tests, coverage, tests-binary-e2e, tests-npm-install-smoke, tests-sentry-runtime-packages, build-binaries, version-check] + needs: [ + ci, + tests, + coverage, + tests-binary-e2e, + tests-npm-install-smoke, + tests-sentry-runtime-packages, + build-binaries, + version-check, + ] runs-on: ubuntu-latest environment: production permissions: contents: write id-token: write steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Read version id: version @@ -657,7 +681,7 @@ jobs: - name: Create release GitHub App token id: release-app-token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: client-id: ${{ vars.VERYFRONT_RELEASE_APP_CLIENT_ID }} private-key: ${{ secrets.VERYFRONT_RELEASE_APP_PRIVATE_KEY }} @@ -692,7 +716,7 @@ jobs: - uses: ./.github/actions/setup-deno - - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 + - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v6 with: node-version: "24" registry-url: "https://registry.npmjs.org" @@ -705,7 +729,7 @@ jobs: deno task build:npm scripts/ci/publish-npm-packages.sh release-publish - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: path: binaries @@ -776,7 +800,7 @@ jobs: --notes "Binaries available at: https://github.com/veryfront/veryfront/releases/tag/v${VERSION}" - name: Trigger server deploy - uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 + uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 with: token: ${{ steps.release-app-token.outputs.token }} repository: veryfront/veryfront-server @@ -784,7 +808,7 @@ jobs: client-payload: '{"version": "${{ steps.version.outputs.version }}"}' - name: Trigger job-runner deploy - uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 + uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 with: token: ${{ steps.release-app-token.outputs.token }} repository: veryfront/veryfront-job-runner @@ -792,7 +816,7 @@ jobs: client-payload: '{"version": "${{ steps.version.outputs.version }}"}' - name: Trigger sandbox deploy - uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 + uses: peter-evans/repository-dispatch@28959ce8df70de7be546dd1250a005dd32156697 # v4.0.1 with: token: ${{ steps.release-app-token.outputs.token }} repository: veryfront/veryfront-sandbox @@ -809,11 +833,11 @@ jobs: runs-on: ubuntu-latest environment: production steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Create Homebrew tap GitHub App token id: homebrew-app-token - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: client-id: ${{ vars.HOMEBREW_TAP_APP_CLIENT_ID }} private-key: ${{ secrets.HOMEBREW_TAP_APP_PRIVATE_KEY }} @@ -823,7 +847,7 @@ jobs: permission-pull-requests: write - name: Update Homebrew tap - uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 + uses: nick-fields/retry@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 env: HOMEBREW_TAP_TOKEN: ${{ steps.homebrew-app-token.outputs.token }} with: diff --git a/deno.lock b/deno.lock index 3db661d016..9122020026 100644 --- a/deno.lock +++ b/deno.lock @@ -61,7 +61,7 @@ "npm:ajv@8.18.0": "8.18.0", "npm:bash-tool@1.3.18": "1.3.18_ai@7.0.41__zod@3.25.76_just-bash@3.0.1", "npm:better-sqlite3@9.6.0": "9.6.0", - "npm:brace-expansion@5.0.8": "5.0.8", + "npm:brace-expansion@5.0.9": "5.0.9", "npm:browserslist@4.28.7": "4.28.7", "npm:daisyui@5.5.14": "5.5.14", "npm:es-module-lexer@2.3.1": "2.3.1", @@ -2961,8 +2961,8 @@ "bowser@2.14.1": { "integrity": "sha512-tzPjzCxygAKWFOJP011oxFHs57HzIhOEracIgAePE4pqB3LikALKnSzUyU4MGs9/iCEUuHlAJTjTc5M+u7YEGg==" }, - "brace-expansion@5.0.8": { - "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", + "brace-expansion@5.0.9": { + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dependencies": [ "balanced-match" ] @@ -6545,7 +6545,7 @@ "jsr:@std/testing@1.0.17", "npm:ai@7.0.41", "npm:bash-tool@1.3.18", - "npm:brace-expansion@5.0.8", + "npm:brace-expansion@5.0.9", "npm:just-bash@3.0.1" ] }, diff --git a/docs/guides/configuration.md b/docs/guides/configuration.md index edc7233c9f..aa1960a870 100644 --- a/docs/guides/configuration.md +++ b/docs/guides/configuration.md @@ -187,6 +187,9 @@ Notes: - `paths` are relative to your project root. - Defaults are `tools`, `agents`, `skills`, `prompts`, `resources`, `workflows`, and `tasks`. - Set `enabled: false` to disable discovery for that primitive. +- Eval, task, trigger, and workflow definitions with filenames containing + `.test.` or `.spec.` are ignored during discovery. Rename production + definitions that use those filename segments before upgrading. ### AI providers and MCP diff --git a/extensions/ext-sandbox-shell-tools/deno.json b/extensions/ext-sandbox-shell-tools/deno.json index a3e0f55a05..39abf1754d 100644 --- a/extensions/ext-sandbox-shell-tools/deno.json +++ b/extensions/ext-sandbox-shell-tools/deno.json @@ -14,7 +14,7 @@ "imports": { "ai": "npm:ai@7.0.41", "bash-tool": "npm:bash-tool@1.3.18", - "brace-expansion": "npm:brace-expansion@5.0.8", + "brace-expansion": "npm:brace-expansion@5.0.9", "just-bash": "npm:just-bash@3.0.1", "@std/assert": "jsr:@std/assert@1.0.19", "@std/testing/bdd": "jsr:@std/testing@1.0.17/bdd", diff --git a/scripts/build/npm-package-metadata.test.ts b/scripts/build/npm-package-metadata.test.ts index 614bda6aa8..2a2d9c20c1 100644 --- a/scripts/build/npm-package-metadata.test.ts +++ b/scripts/build/npm-package-metadata.test.ts @@ -404,7 +404,7 @@ describe("normalizeNpmPackageMetadata", () => { "@opentelemetry/sdk-metrics": "2.8.0", "@opentelemetry/sdk-node": "0.218.0", "@sentry/deno": "10.68.0", - "brace-expansion": "5.0.8", + "brace-expansion": "5.0.9", "gaxios": "7.2.0", "gcp-metadata": "8.1.2", "protobufjs": "7.6.5", diff --git a/scripts/build/proxy-deno.lock b/scripts/build/proxy-deno.lock index e9d40ce6b6..9adb8b0cfa 100644 --- a/scripts/build/proxy-deno.lock +++ b/scripts/build/proxy-deno.lock @@ -1690,7 +1690,7 @@ "jsr:@std/testing@1.0.17", "npm:ai@7.0.41", "npm:bash-tool@1.3.18", - "npm:brace-expansion@5.0.8", + "npm:brace-expansion@5.0.9", "npm:just-bash@3.0.1" ] }, diff --git a/scripts/lint/test-typecheck-baseline.json b/scripts/lint/test-typecheck-baseline.json index 65e43b2529..9671836a3e 100644 --- a/scripts/lint/test-typecheck-baseline.json +++ b/scripts/lint/test-typecheck-baseline.json @@ -49,6 +49,5 @@ "src/transforms/md/compiler/md-compiler.test.ts", "src/transforms/mdx/compiler/index.test.ts", "src/transforms/mdx/esm-module-loader/loader.test.ts", - "src/utils/cache/stores/memory/lru-list-manager.test.ts", "src/workflow/api/workflow-client.test.ts" ] diff --git a/src/modules/react-loader/ssr-module-loader/loader.test.ts b/src/modules/react-loader/ssr-module-loader/loader.test.ts index f6689f6a4f..f723900a0b 100644 --- a/src/modules/react-loader/ssr-module-loader/loader.test.ts +++ b/src/modules/react-loader/ssr-module-loader/loader.test.ts @@ -27,6 +27,7 @@ import { buildMdxEsmPathCacheKey, } from "#veryfront/transforms/mdx/esm-module-loader/cache-format.ts"; import type { RuntimeAdapter } from "#veryfront/platform/adapters/base.ts"; +import type { FileSystem } from "#veryfront/platform/compat/fs.ts"; import type { ModuleCacheEntry } from "./types.ts"; import { clearModulePathCache, @@ -378,6 +379,130 @@ describe("SSRModuleLoader", { sanitizeResources: false, sanitizeOps: false }, () } }); + it("invalidates stale cache indexes when cached output cannot be inspected", async () => { + clearSSRModuleCache(); + + const projectDir = await makeTempDir({ prefix: "vf-ssr-loader-unreadable-output-" }); + const filePath = join(projectDir, "UnreadableCachedOutput.tsx"); + const projectId = "project-unreadable-cached-output-test"; + const contentSourceId = "preview-main"; + const source = "export default function UnreadableCachedOutput() { return null; }"; + const contentHash = hashAsLoader(source, filePath, projectDir); + const configHash = computeConfigHashSync({ dev: true }); + const reactVersion = "default"; + const filePathCacheKey = buildSSRModuleCacheKey( + RUNTIME_VERSION, + projectId, + `${contentSourceId}:${reactVersion}:${configHash}:${filePath}`, + ); + const contentCacheKey = buildSSRModuleCacheKey( + RUNTIME_VERSION, + projectId, + `${contentSourceId}:${reactVersion}:${configHash}:${filePath}:${contentHash}`, + ); + const staleEntry = { + tempPath: join(projectDir, "unreadable-cache-output.mjs"), + contentHash, + }; + globalModuleCache.set(contentCacheKey, staleEntry); + globalModuleCache.set(filePathCacheKey, staleEntry); + verifiedHttpBundlePaths.set(`${staleEntry.tempPath}:${contentHash}`, true); + + const loader = new SSRModuleLoader({ + projectDir, + projectId, + contentSourceId, + adapter: denoAdapter, + dev: true, + }); + const cacheManager = (loader as unknown as { + cache: { fs: FileSystem; getFs(): FileSystem }; + }).cache; + const originalFs = cacheManager.getFs(); + cacheManager.fs = { + stat: () => Promise.reject(Object.assign(new Error("permission denied"), { code: "EACCES" })), + } as unknown as FileSystem; + + try { + await assertRejects( + () => loader.loadModule(filePath, source), + Error, + "permission denied", + ); + assertEquals(globalModuleCache.get(contentCacheKey), undefined); + assertEquals(globalModuleCache.get(filePathCacheKey), undefined); + assertEquals(verifiedHttpBundlePaths.get(`${staleEntry.tempPath}:${contentHash}`), undefined); + } finally { + cacheManager.fs = originalFs; + await remove(projectDir, { recursive: true }); + } + }); + + it("preserves an operational cache error when MDX invalidation also fails", async () => { + clearSSRModuleCache(); + + const projectDir = await makeTempDir({ prefix: "vf-ssr-loader-invalidation-failure-" }); + const filePath = join(projectDir, "InvalidationFailure.tsx"); + const projectId = "project-invalidation-failure-test"; + const contentSourceId = "preview-main"; + const source = "export default function InvalidationFailure() { return null; }"; + const contentHash = hashAsLoader(source, filePath, projectDir); + const configHash = computeConfigHashSync({ dev: true }); + const reactVersion = "default"; + const filePathCacheKey = buildSSRModuleCacheKey( + RUNTIME_VERSION, + projectId, + `${contentSourceId}:${reactVersion}:${configHash}:${filePath}`, + ); + const contentCacheKey = buildSSRModuleCacheKey( + RUNTIME_VERSION, + projectId, + `${contentSourceId}:${reactVersion}:${configHash}:${filePath}:${contentHash}`, + ); + const staleEntry = { + tempPath: join(projectDir, "unreadable-cache-output.mjs"), + contentHash, + }; + globalModuleCache.set(contentCacheKey, staleEntry); + globalModuleCache.set(filePathCacheKey, staleEntry); + verifiedHttpBundlePaths.set(`${staleEntry.tempPath}:${contentHash}`, true); + + const loader = new SSRModuleLoader({ + projectDir, + projectId, + contentSourceId, + adapter: denoAdapter, + dev: true, + }); + const mutableLoader = loader as unknown as { + cache: { fs: FileSystem; getFs(): FileSystem }; + invalidateMdxEsmCacheEntry( + filePath: string, + cacheEntry: ModuleCacheEntry, + ): Promise; + }; + const originalFs = mutableLoader.cache.getFs(); + mutableLoader.cache.fs = { + stat: () => Promise.reject(Object.assign(new Error("permission denied"), { code: "EACCES" })), + } as unknown as FileSystem; + mutableLoader.invalidateMdxEsmCacheEntry = () => + Promise.reject(new Error("invalidation failed")); + + try { + await assertRejects( + () => loader.loadModule(filePath, source), + Error, + "permission denied", + ); + assertEquals(globalModuleCache.get(filePathCacheKey), undefined); + assertEquals(globalModuleCache.get(contentCacheKey), undefined); + assertEquals(verifiedHttpBundlePaths.get(`${staleEntry.tempPath}:${contentHash}`), undefined); + } finally { + mutableLoader.cache.fs = originalFs; + await remove(projectDir, { recursive: true }); + } + }); + it("clears verified MDX-ESM path cache before retrying stale local dependencies", async () => { clearSSRModuleCache(); clearModulePathCache(); diff --git a/src/modules/react-loader/ssr-module-loader/loader.ts b/src/modules/react-loader/ssr-module-loader/loader.ts index b4f65de055..a050e9806b 100644 --- a/src/modules/react-loader/ssr-module-loader/loader.ts +++ b/src/modules/react-loader/ssr-module-loader/loader.ts @@ -251,11 +251,35 @@ export class SSRModuleLoader { cacheEntry: ModuleCacheEntry, ): Promise> { // Verify the cache file exists before attempting dynamic import - const fileExists = await verifyCacheFileExists( - this.cache.getFs(), - cacheEntry.tempPath, - "SSR-MODULE-LOADER", - ); + let fileExists: boolean; + try { + fileExists = await verifyCacheFileExists( + this.cache.getFs(), + cacheEntry.tempPath, + "SSR-MODULE-LOADER", + ); + } catch (error) { + // An unreadable cache entry cannot be trusted on a later attempt. Keep + // the original operational error, but remove both indexes so a repaired + // filesystem does not keep routing requests back to stale metadata. + try { + await this.invalidateMdxEsmCacheEntry(filePath, cacheEntry); + } catch (invalidationError) { + logger.warn("Failed to invalidate unreadable MDX cache entry", { + file: filePath.slice(-40), + error: invalidationError, + }); + } + try { + this.cache.invalidateFilePathCacheEntry(filePath, cacheEntry); + } catch (invalidationError) { + logger.warn("Failed to invalidate unreadable file-path cache entry", { + file: filePath.slice(-40), + error: invalidationError, + }); + } + throw error; + } if (!fileExists) { logger.debug("Cache file missing before import, invalidating", { file: filePath.slice(-40), diff --git a/src/security/sandbox/worker-pool.test.ts b/src/security/sandbox/worker-pool.test.ts index eea1a4a7cf..53515f1b9c 100644 --- a/src/security/sandbox/worker-pool.test.ts +++ b/src/security/sandbox/worker-pool.test.ts @@ -30,7 +30,7 @@ import type { import { DEFAULT_WORKER_POOL_CONFIG, MAX_WORKER_BODY_BYTES } from "./worker-types.ts"; import { WORKER_INTERNAL_EGRESS_OVERRIDE_ENV } from "./worker-egress-guard.ts"; import { resolveWorkerGeneration, snapshotWorkerGenerationIdentity } from "./worker-generation.ts"; -import { fromFileUrl } from "#veryfront/compat/path"; +import { fromFileUrl, join } from "#veryfront/compat/path"; // Worker isolation only works in Deno (requires Deno Worker permissions API) const testSuite = isDeno ? describe : describe.skip; @@ -447,7 +447,7 @@ testSuite("WorkerPool", () => { "ssr-permissions", [projectRoot], makeSSRRequest("ssr-permissions-request", { - pageModulePath: `${projectRoot}/page.tsx`, + pageModulePath: join(projectRoot, "page.tsx"), }), ); const worker = latestWorker(controlled.workers, "ssr-permissions"); diff --git a/src/transforms/mdx/esm-module-loader/module-writer.test.ts b/src/transforms/mdx/esm-module-loader/module-writer.test.ts index b2cc144781..538142d41f 100644 --- a/src/transforms/mdx/esm-module-loader/module-writer.test.ts +++ b/src/transforms/mdx/esm-module-loader/module-writer.test.ts @@ -1,11 +1,16 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals } from "#veryfront/testing/assert.ts"; +import { assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; -import { buildMdxModuleCacheIdentity } from "./module-writer.ts"; +import { __moduleWriterInternals, buildMdxModuleCacheIdentity } from "./module-writer.ts"; import { mdxRenderer } from "../index.ts"; import { denoAdapter } from "#veryfront/platform/adapters/deno.ts"; import { hashString } from "#veryfront/cache/hash.ts"; import { withMockFetch } from "#veryfront/testing/mock-fetch.ts"; +import type { FileInfo } from "#veryfront/platform/adapters/base.ts"; +import type { FileSystem } from "#veryfront/platform/compat/fs.ts"; +import { VeryfrontError } from "#veryfront/errors"; +import { LRUCache } from "#veryfront/utils/lru-wrapper.ts"; +import type { MDXModule } from "../types.ts"; function cacheKeyForDependencies( dependencies: Readonly>, @@ -144,3 +149,89 @@ describe("MDX root module cache identity", () => { } }); }); + +describe("verifyMdxCacheFile", () => { + const { verifyMdxCacheFile } = __moduleWriterInternals; + + const FILE_STAT: FileInfo = { + isFile: true, + isDirectory: false, + isSymlink: false, + size: 100, + mtime: null, + }; + + function filesystemError(message: string, code: string): Error & { code: string } { + return Object.assign(new Error(message), { code }); + } + + function createStatFs(stat: FileSystem["stat"]): FileSystem { + return { stat } as FileSystem; + } + + function createContext(): { moduleCache: LRUCache } { + return { moduleCache: new LRUCache({ maxEntries: 10 }) }; + } + + it("returns true when the cache file exists", async () => { + const result = await verifyMdxCacheFile( + createStatFs(() => Promise.resolve(FILE_STAT)), + "/cache/module.mjs", + createContext(), + "ns:hash", + ); + + assertEquals(result, true); + }); + + it("returns false when the cache file is genuinely absent", async () => { + const result = await verifyMdxCacheFile( + createStatFs(() => Promise.reject(filesystemError("not found", "ENOENT"))), + "/cache/module.mjs", + createContext(), + "ns:hash", + ); + + assertEquals(result, false); + }); + + it("wraps operational stat failures in CACHE_ERROR with the original cause", async () => { + const original = filesystemError("permission denied", "EACCES"); + + const error = await assertRejects( + () => + verifyMdxCacheFile( + createStatFs(() => Promise.reject(original)), + "/cache/module.mjs", + createContext(), + "ns:hash", + ), + VeryfrontError, + "MDX module cache file inspection failed", + ) as VeryfrontError; + + assertEquals(error.slug, "cache-error"); + assertEquals(error.cause, original); + assertEquals(error.message.includes("/cache/module.mjs"), false); + }); + + it("invalidates the stale module index entry on operational stat failures", async () => { + const context = createContext(); + context.moduleCache.set("ns:hash", {} as MDXModule); + context.moduleCache.set("ns:other", {} as MDXModule); + + await assertRejects( + () => + verifyMdxCacheFile( + createStatFs(() => Promise.reject(filesystemError("io error", "EIO"))), + "/cache/module.mjs", + context, + "ns:hash", + ), + VeryfrontError, + ); + + assertEquals(context.moduleCache.has("ns:hash"), false); + assertEquals(context.moduleCache.has("ns:other"), true); + }); +}); diff --git a/src/transforms/mdx/esm-module-loader/module-writer.ts b/src/transforms/mdx/esm-module-loader/module-writer.ts index d3e93fcc52..de43b573ec 100644 --- a/src/transforms/mdx/esm-module-loader/module-writer.ts +++ b/src/transforms/mdx/esm-module-loader/module-writer.ts @@ -134,6 +134,41 @@ async function cacheHttpImports( return result.code; } +/** + * Verify an MDX module cache file, mirroring the SSR loader's handling of the + * throwing `verifyCacheFileExists`: a genuinely absent file returns `false`, + * while an operational stat failure (EACCES, EIO, ...) drops the stale + * in-memory module index entry — so a repaired filesystem cannot keep routing + * requests to untrusted metadata — and is rethrown inside the CACHE_ERROR + * taxonomy with the original error attached as `cause`. + */ +async function verifyMdxCacheFile( + localFs: Parameters[0], + filePath: string, + context: Pick, + compositeKey: string, +): Promise { + try { + return await verifyCacheFileExists(localFs, filePath, "MDX-ESM-LOADER"); + } catch (error) { + try { + context.moduleCache.delete(compositeKey); + } catch (invalidationError) { + logger.warn(`${LOG_PREFIX_MDX_LOADER} Failed to invalidate module cache entry`, { + compositeKey, + error: invalidationError, + }); + } + throw CACHE_ERROR.create({ + detail: "MDX module cache file inspection failed", + cause: error, + }); + } +} + +/** Internal test seam for cache verification error handling. */ +export const __moduleWriterInternals = { verifyMdxCacheFile }; + export async function doLoadModuleESM( compiledProgramCode: string, context: ESMLoaderContext, @@ -280,7 +315,7 @@ export async function doLoadModuleESM( logger.debug(`${LOG_PREFIX_MDX_LOADER} Step: mdxWriteFlight START`, { projectSlug, filePath }); await mdxWriteFlight.do(filePath, async () => { // Check if file already exists (written by another request) - if (await verifyCacheFileExists(localFs, filePath, "MDX-ESM-LOADER")) { + if (await verifyMdxCacheFile(localFs, filePath, effectiveContext, compositeKey)) { logger.debug(`${LOG_PREFIX_MDX_LOADER} File exists, skipping write`, { projectSlug, filePath, @@ -474,7 +509,7 @@ export async function doLoadModuleESM( } // Verify the cache file exists before attempting dynamic import - const fileExists = await verifyCacheFileExists(localFs, filePath, "MDX-ESM-LOADER"); + const fileExists = await verifyMdxCacheFile(localFs, filePath, effectiveContext, compositeKey); if (!fileExists) { throw CACHE_ERROR.create({ detail: `MDX module cache file missing before import: ${filePath}`, diff --git a/src/utils/bundle-manifest.test.ts b/src/utils/bundle-manifest.test.ts index 2efbe52822..2bca8edc3d 100644 --- a/src/utils/bundle-manifest.test.ts +++ b/src/utils/bundle-manifest.test.ts @@ -2,8 +2,10 @@ import "#veryfront/schemas/_test-setup.ts"; import { assertEquals, assertExists } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { delay } from "#std/async.ts"; +import { FakeTime } from "#std/testing/time"; import { scaleMs } from "#veryfront/testing/timing.ts"; import { + BUNDLE_MANIFEST_SWEEP_INTERVAL_MS, type BundleCode, type BundleMetadata, computeCodeHash, @@ -44,6 +46,29 @@ describe("InMemoryBundleManifestStore", () => { assertEquals(await store.isAvailable(), true); }); + it("snapshots bundle code on write and read", async () => { + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { + code: "export default 'original'", + sourceMap: "original-map", + css: ".original {}", + }; + + await store.setBundleCode("code-hash", code); + code.code = "export default 'mutated input'"; + + const firstRead = await store.getBundleCode("code-hash"); + assertExists(firstRead); + assertEquals(firstRead.code, "export default 'original'"); + firstRead.code = "export default 'mutated output'"; + + assertEquals(await store.getBundleCode("code-hash"), { + code: "export default 'original'", + sourceMap: "original-map", + css: ".original {}", + }); + }); + it("TTL expiration", async () => { const store = new InMemoryBundleManifestStore(); @@ -95,6 +120,380 @@ describe("InMemoryBundleManifestStore", () => { assertEquals(await store.getBundleMetadata("key-2"), undefined); }); + it("removes a replaced key from its previous source index", async () => { + const store = new InMemoryBundleManifestStore(); + const original: BundleMetadata = { + hash: "hash-original", + codeHash: "code-original", + size: 10, + compiledAt: Date.now(), + source: "original.mdx", + mode: "development", + }; + const replacement: BundleMetadata = { + ...original, + hash: "hash-replacement", + codeHash: "code-replacement", + source: "replacement.mdx", + }; + + await store.setBundleMetadata("shared-key", original); + await store.setBundleMetadata("shared-key", replacement); + + assertEquals(await store.invalidateSource("original.mdx"), 0); + assertEquals(await store.getBundleMetadata("shared-key"), replacement); + }); + + it("does not delete code that is still referenced by another bundle", async () => { + const store = new InMemoryBundleManifestStore(); + const sharedCode: BundleCode = { code: "export default 1" }; + const first: BundleMetadata = { + hash: "hash-first", + codeHash: "shared-code", + size: 10, + compiledAt: Date.now(), + source: "first.mdx", + mode: "development", + }; + const second: BundleMetadata = { + ...first, + hash: "hash-second", + source: "second.mdx", + }; + + await store.setBundleMetadata("first", first); + await store.setBundleMetadata("second", second); + await store.setBundleCode("shared-code", sharedCode); + await store.deleteBundle("first"); + + assertEquals(await store.getBundleMetadata("second"), second); + assertEquals(await store.getBundleCode("shared-code"), sharedCode); + + await store.deleteBundle("second"); + assertEquals(await store.getBundleCode("shared-code"), undefined); + }); + + it("retains shared code across partial source invalidation", async () => { + const store = new InMemoryBundleManifestStore(); + const sharedCode: BundleCode = { code: "export default 1" }; + const metadata: BundleMetadata = { + hash: "hash", + codeHash: "shared-code", + size: 10, + compiledAt: Date.now(), + source: "first.mdx", + mode: "development", + }; + + await store.setBundleCode(metadata.codeHash, sharedCode); + await store.setBundleMetadata("first-a", metadata); + await store.setBundleMetadata("first-b", { ...metadata, hash: "hash-b" }); + await store.setBundleMetadata("second", { ...metadata, source: "second.mdx" }); + + assertEquals(await store.invalidateSource("first.mdx"), 2); + assertEquals(await store.getBundleCode(metadata.codeHash), sharedCode); + + assertEquals(await store.invalidateSource("second.mdx"), 1); + assertEquals(await store.getBundleCode(metadata.codeHash), undefined); + }); + + it("transfers code references when metadata is replaced", async () => { + const store = new InMemoryBundleManifestStore(); + const originalCode: BundleCode = { code: "export default 'original'" }; + const replacementCode: BundleCode = { code: "export default 'replacement'" }; + const original: BundleMetadata = { + hash: "hash-original", + codeHash: "code-original", + size: 10, + compiledAt: Date.now(), + source: "original.mdx", + mode: "development", + }; + const replacement: BundleMetadata = { + ...original, + hash: "hash-replacement", + codeHash: "code-replacement", + source: "replacement.mdx", + }; + + await store.setBundleCode(original.codeHash, originalCode); + await store.setBundleCode(replacement.codeHash, replacementCode); + await store.setBundleMetadata("replaced", original); + await store.setBundleMetadata("remaining-original", { + ...original, + source: "remaining.mdx", + }); + await store.setBundleMetadata("replaced", replacement); + + assertEquals(await store.getBundleCode(original.codeHash), originalCode); + assertEquals(await store.getBundleCode(replacement.codeHash), replacementCode); + + await store.deleteBundle("remaining-original"); + assertEquals(await store.getBundleCode(original.codeHash), undefined); + assertEquals(await store.getBundleCode(replacement.codeHash), replacementCode); + + await store.deleteBundle("replaced"); + assertEquals(await store.getBundleCode(replacement.codeHash), undefined); + }); + + it("does not double-count an unchanged key and code hash", async () => { + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { code: "export default true" }; + const metadata: BundleMetadata = { + hash: "hash", + codeHash: "code", + size: 10, + compiledAt: Date.now(), + source: "source.mdx", + mode: "development", + }; + + await store.setBundleCode(metadata.codeHash, code); + await store.setBundleMetadata("key", metadata); + await store.setBundleMetadata("key", { ...metadata, hash: "updated-hash" }); + await store.deleteBundle("key"); + + assertEquals(await store.getBundleCode(metadata.codeHash), undefined); + }); + + it("snapshots metadata supplied by callers", async () => { + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { code: "export default true" }; + const metadata: BundleMetadata = { + hash: "original-hash", + codeHash: "original-code", + size: 10, + compiledAt: Date.now(), + source: "original.mdx", + mode: "development", + meta: { + type: "mdx", + headings: [{ id: "original", text: "Original", level: 1 }], + }, + }; + + await store.setBundleCode(metadata.codeHash, code); + await store.setBundleMetadata("key", metadata); + + metadata.codeHash = "mutated-code"; + metadata.source = "mutated.mdx"; + const suppliedHeading = metadata.meta?.headings?.[0]; + assertExists(suppliedHeading); + suppliedHeading.text = "Mutated"; + + const stored = await store.getBundleMetadata("key"); + assertEquals(stored?.codeHash, "original-code"); + assertEquals(stored?.source, "original.mdx"); + assertEquals(stored?.meta?.headings?.[0]?.text, "Original"); + + await store.deleteBundle("key"); + assertEquals(await store.getBundleCode("original-code"), undefined); + assertEquals(await store.invalidateSource("original.mdx"), 0); + }); + + it("does not expose indexed metadata records to callers", async () => { + const store = new InMemoryBundleManifestStore(); + const first: BundleMetadata = { + hash: "first-hash", + codeHash: "first-code", + size: 10, + compiledAt: Date.now(), + source: "first.mdx", + mode: "development", + meta: { + type: "mdx", + headings: [{ id: "first", text: "First", level: 1 }], + }, + }; + const second: BundleMetadata = { + ...first, + hash: "second-hash", + codeHash: "second-code", + source: "second.mdx", + }; + + await store.setBundleCode(first.codeHash, { code: "export default 1" }); + await store.setBundleCode(second.codeHash, { code: "export default 2" }); + await store.setBundleMetadata("first", first); + await store.setBundleMetadata("second", second); + + const exposed = await store.getBundleMetadata("first"); + assertExists(exposed); + exposed.codeHash = second.codeHash; + exposed.source = second.source; + const exposedHeading = exposed.meta?.headings?.[0]; + assertExists(exposedHeading); + exposedHeading.text = "Mutated"; + + const stored = await store.getBundleMetadata("first"); + assertEquals(stored?.codeHash, first.codeHash); + assertEquals(stored?.source, first.source); + assertEquals(stored?.meta?.headings?.[0]?.text, "First"); + + await store.deleteBundle("first"); + assertEquals(await store.getBundleCode(first.codeHash), undefined); + assertEquals(await store.getBundleCode(second.codeHash), { code: "export default 2" }); + assertEquals(await store.invalidateSource(first.source), 0); + assertEquals(await store.getBundleMetadata("second"), second); + }); + + it("releases code and source references when metadata expires", async () => { + using time = new FakeTime(); + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { code: "export default true" }; + const metadata: BundleMetadata = { + hash: "hash", + codeHash: "code", + size: 10, + compiledAt: Date.now(), + source: "source.mdx", + mode: "development", + }; + + await store.setBundleCode(metadata.codeHash, code); + await store.setBundleMetadata("key", metadata, 10); + await time.tickAsync(11); + + assertEquals(await store.getBundleMetadata("key"), undefined); + assertEquals(await store.getBundleCode(metadata.codeHash), undefined); + assertEquals(await store.invalidateSource(metadata.source), 0); + }); + + it("keeps referenced code alive past a shorter code ttl", async () => { + using time = new FakeTime(); + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { code: "export default true" }; + const metadata: BundleMetadata = { + hash: "hash", + codeHash: "code", + size: 10, + compiledAt: Date.now(), + source: "source.mdx", + mode: "development", + }; + + await store.setBundleCode(metadata.codeHash, code, 10); + await store.setBundleMetadata("key", metadata, 100); + await time.tickAsync(10); + + assertEquals(await store.getBundleCode(metadata.codeHash), code); + await store.deleteBundle("key"); + assertEquals(await store.getBundleCode(metadata.codeHash), undefined); + }); + + it("does not expose stored bundle code records to caller mutation", async () => { + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { + code: "export default 'original'", + css: ".original { color: red; }", + sourceMap: "{}", + }; + + await store.setBundleCode("code", code); + code.code = "export default 'mutated after set'"; + code.css = ".mutated { color: blue; }"; + + const firstRead = await store.getBundleCode("code"); + assertEquals(firstRead, { + code: "export default 'original'", + css: ".original { color: red; }", + sourceMap: "{}", + }); + + assertExists(firstRead); + firstRead.code = "export default 'mutated after read'"; + firstRead.sourceMap = "mutated"; + + assertEquals(await store.getBundleCode("code"), { + code: "export default 'original'", + css: ".original { color: red; }", + sourceMap: "{}", + }); + }); + + it("prunes unread expired metadata before resolving referenced code", async () => { + using time = new FakeTime(); + const store = new InMemoryBundleManifestStore(); + const metadata: BundleMetadata = { + hash: "hash", + codeHash: "code", + size: 10, + compiledAt: Date.now(), + source: "source.mdx", + mode: "development", + }; + + await store.setBundleCode(metadata.codeHash, { code: "export default true" }); + await store.setBundleMetadata("key", metadata, 10); + await time.tickAsync(10); + + assertEquals(await store.getBundleCode(metadata.codeHash), undefined); + assertEquals(await store.invalidateSource(metadata.source), 0); + assertEquals((await store.getStats()).totalBundles, 0); + }); + + it("excludes expired metadata from stats without a pruning sweep", async () => { + using time = new FakeTime(); + const store = new InMemoryBundleManifestStore(); + const now = Date.now(); + const expiring: BundleMetadata = { + hash: "expiring-hash", + codeHash: "expiring-code", + size: 100, + compiledAt: now - 1000, + source: "expiring.mdx", + mode: "development", + }; + const durable: BundleMetadata = { + hash: "durable-hash", + codeHash: "durable-code", + size: 50, + compiledAt: now, + source: "durable.mdx", + mode: "development", + }; + + await store.setBundleCode(expiring.codeHash, { code: "export default 1" }, 10); + await store.setBundleMetadata("expiring", expiring, 10); + await store.setBundleMetadata("durable", durable); + await time.tickAsync(10); + + const stats = await store.getStats(); + assertEquals(stats.totalBundles, 1); + assertEquals(stats.totalSize, durable.size); + assertEquals(stats.oldestBundle, durable.compiledAt); + assertEquals(stats.newestBundle, durable.compiledAt); + + // The expired entry was excluded from the view, never served, and its + // code is not resolvable either. + assertEquals(await store.getBundleMetadata("expiring"), undefined); + assertEquals(await store.getBundleCode(expiring.codeHash), undefined); + assertEquals(await store.getBundleMetadata("durable"), durable); + }); + + it("sweeps unread expired metadata on writes after the sweep interval", async () => { + using time = new FakeTime(); + const store = new InMemoryBundleManifestStore(); + const stale: BundleMetadata = { + hash: "stale-hash", + codeHash: "stale-code", + size: 10, + compiledAt: Date.now(), + source: "stale.mdx", + mode: "development", + }; + + await store.setBundleMetadata("stale", stale, 10); + await time.tickAsync(BUNDLE_MANIFEST_SWEEP_INTERVAL_MS + 1); + + // An unrelated write amortizes the full sweep; the expired entry must be + // gone from the source index without ever having been read. + await store.setBundleMetadata("fresh", { ...stale, hash: "fresh-hash", source: "fresh.mdx" }); + + assertEquals(await store.invalidateSource(stale.source), 0); + assertEquals((await store.getStats()).totalBundles, 1); + }); + it("delete bundle", async () => { const store = new InMemoryBundleManifestStore(); @@ -141,6 +540,29 @@ describe("InMemoryBundleManifestStore", () => { assertEquals(await store.getBundleCode(metadata.codeHash), undefined); }); + it("clear resets code reference counts", async () => { + const store = new InMemoryBundleManifestStore(); + const code: BundleCode = { code: "export default true" }; + const metadata: BundleMetadata = { + hash: "hash", + codeHash: "shared-code", + size: 10, + compiledAt: Date.now(), + source: "source.mdx", + mode: "development", + }; + + await store.setBundleMetadata("first", metadata); + await store.setBundleMetadata("second", { ...metadata, source: "second.mdx" }); + await store.clear(); + + await store.setBundleCode(metadata.codeHash, code); + await store.setBundleMetadata("after-clear", metadata); + await store.deleteBundle("after-clear"); + + assertEquals(await store.getBundleCode(metadata.codeHash), undefined); + }); + it("statistics", async () => { const store = new InMemoryBundleManifestStore(); const now = Date.now(); diff --git a/src/utils/bundle-manifest.ts b/src/utils/bundle-manifest.ts index 12130ae699..6a5cd2eb6a 100644 --- a/src/utils/bundle-manifest.ts +++ b/src/utils/bundle-manifest.ts @@ -45,61 +45,85 @@ export interface BundleManifestStore { getStats(): Promise; } +/** Minimum interval between full expired-metadata sweeps (amortized on writes). */ +export const BUNDLE_MANIFEST_SWEEP_INTERVAL_MS = 30_000; + export class InMemoryBundleManifestStore implements BundleManifestStore { private metadata = new Map(); private code = new Map(); private sourceIndex = new Map>(); + // codeHash → metadata keys holding a reference. Each key holds at most one + // reference per hash, so set size is the reference count and the members + // let reads prune only the metadata entries relevant to one hash. + private codeReferences = new Map>(); + private lastExpiredSweepAt = Date.now(); - private getIfNotExpired( - map: Map, - key: string, - ): T | undefined { - const entry = map.get(key); + async getBundleMetadata(key: string): Promise { + const entry = this.metadata.get(key); if (!entry) return undefined; - if (entry.expiry != null && Date.now() > entry.expiry) { - map.delete(key); + if (entry.expiry != null && Date.now() >= entry.expiry) { + this.removeMetadata(key); return undefined; } - return entry.value; - } - - async getBundleMetadata(key: string): Promise { - return this.getIfNotExpired(this.metadata, key); + return structuredClone(entry.value); } async setBundleMetadata(key: string, metadata: BundleMetadata, ttlMs?: number): Promise { + this.sweepExpiredMetadataIfDue(); const expiry = ttlMs != null ? Date.now() + ttlMs : undefined; - this.metadata.set(key, { value: metadata, expiry }); + const snapshot = structuredClone(metadata); + const previous = this.metadata.get(key)?.value; + + if (!previous) { + this.addCodeReference(snapshot.codeHash, key); + } else if (previous.codeHash !== snapshot.codeHash) { + this.removeCodeReference(previous.codeHash, key); + this.addCodeReference(snapshot.codeHash, key); + } + + if (previous && previous.source !== snapshot.source) { + this.removeSourceReference(key, previous.source); + } - const keys = this.sourceIndex.get(metadata.source) ?? new Set(); + this.metadata.set(key, { value: snapshot, expiry }); + + const keys = this.sourceIndex.get(snapshot.source) ?? new Set(); keys.add(key); - this.sourceIndex.set(metadata.source, keys); + this.sourceIndex.set(snapshot.source, keys); } async getBundleCode(hash: string): Promise { - return this.getIfNotExpired(this.code, hash); + // Only the metadata entries referencing this hash decide its liveness, so + // prune those lazily instead of sweeping the entire metadata map on every + // per-render read. Full sweeps are amortized onto writes. + this.pruneExpiredReferencesTo(hash); + const entry = this.code.get(hash); + if (!entry) return undefined; + + // Metadata references own code liveness. A shorter code TTL must not turn + // a still-valid manifest into a pointer to a missing content blob. + if ( + entry.expiry != null && + Date.now() >= entry.expiry && + !this.codeReferences.has(hash) + ) { + this.code.delete(hash); + return undefined; + } + + return structuredClone(entry.value); } async setBundleCode(hash: string, code: BundleCode, ttlMs?: number): Promise { + this.sweepExpiredMetadataIfDue(); const expiry = ttlMs != null ? Date.now() + ttlMs : undefined; - this.code.set(hash, { value: code, expiry }); + this.code.set(hash, { value: structuredClone(code), expiry }); } async deleteBundle(key: string): Promise { - const metadata = await this.getBundleMetadata(key); - - this.metadata.delete(key); - if (!metadata) return; - - this.code.delete(metadata.codeHash); - - const sourceKeys = this.sourceIndex.get(metadata.source); - if (!sourceKeys) return; - - sourceKeys.delete(key); - if (sourceKeys.size === 0) this.sourceIndex.delete(metadata.source); + this.removeMetadata(key); } async invalidateSource(source: string): Promise { @@ -107,7 +131,7 @@ export class InMemoryBundleManifestStore implements BundleManifestStore { if (!keys) return 0; const keysArray = [...keys]; - await Promise.all(keysArray.map((key) => this.deleteBundle(key))); + for (const key of keysArray) this.removeMetadata(key); this.sourceIndex.delete(source); return keysArray.length; @@ -117,6 +141,7 @@ export class InMemoryBundleManifestStore implements BundleManifestStore { this.metadata.clear(); this.code.clear(); this.sourceIndex.clear(); + this.codeReferences.clear(); } async isAvailable(): Promise { @@ -124,11 +149,17 @@ export class InMemoryBundleManifestStore implements BundleManifestStore { } async getStats(): Promise { + // Non-mutating expiry view: expired entries are excluded from the + // aggregate without paying a pruning sweep on this read path. + const now = Date.now(); + let totalBundles = 0; let totalSize = 0; let oldestBundle: number | undefined; let newestBundle: number | undefined; - for (const { value } of this.metadata.values()) { + for (const { value, expiry } of this.metadata.values()) { + if (expiry != null && now >= expiry) continue; + totalBundles++; totalSize += value.size; oldestBundle = oldestBundle == null ? value.compiledAt @@ -139,12 +170,80 @@ export class InMemoryBundleManifestStore implements BundleManifestStore { } return { - totalBundles: this.metadata.size, + totalBundles, totalSize, oldestBundle, newestBundle, }; } + + private addCodeReference(codeHash: string, key: string): void { + const refs = this.codeReferences.get(codeHash) ?? new Set(); + refs.add(key); + this.codeReferences.set(codeHash, refs); + } + + /** + * Drop expired metadata entries referencing one code hash so the reference + * check in getBundleCode stays trustworthy without a full-map sweep. + */ + private pruneExpiredReferencesTo(hash: string, now = Date.now()): void { + const refs = this.codeReferences.get(hash); + if (!refs) return; + + for (const key of [...refs]) { + const entry = this.metadata.get(key); + if (!entry) { + // Defensive: a reference without metadata is stale bookkeeping. + this.removeCodeReference(hash, key); + continue; + } + if (entry.expiry != null && now >= entry.expiry) this.removeMetadata(key); + } + } + + /** + * Full expired-metadata sweep, amortized: runs on writes at most once per + * BUNDLE_MANIFEST_SWEEP_INTERVAL_MS so unread expired entries cannot pin + * memory forever while per-render reads stay off the O(all-metadata) path. + */ + private sweepExpiredMetadataIfDue(now = Date.now()): void { + if (now - this.lastExpiredSweepAt < BUNDLE_MANIFEST_SWEEP_INTERVAL_MS) return; + this.lastExpiredSweepAt = now; + + for (const [key, entry] of this.metadata) { + if (entry.expiry != null && now >= entry.expiry) this.removeMetadata(key); + } + } + + private removeCodeReference(codeHash: string, key: string): void { + const refs = this.codeReferences.get(codeHash); + if (refs) { + refs.delete(key); + if (refs.size > 0) return; + this.codeReferences.delete(codeHash); + } + + this.code.delete(codeHash); + } + + private removeMetadata(key: string): BundleMetadata | undefined { + const metadata = this.metadata.get(key)?.value; + if (!metadata) return undefined; + + this.metadata.delete(key); + this.removeSourceReference(key, metadata.source); + this.removeCodeReference(metadata.codeHash, key); + return metadata; + } + + private removeSourceReference(key: string, source: string): void { + const sourceKeys = this.sourceIndex.get(source); + if (!sourceKeys) return; + + sourceKeys.delete(key); + if (sourceKeys.size === 0) this.sourceIndex.delete(source); + } } let manifestStore: BundleManifestStore = new InMemoryBundleManifestStore(); diff --git a/src/utils/cache-dir.test.ts b/src/utils/cache-dir.test.ts index e7870ed13c..aa474e51ad 100644 --- a/src/utils/cache-dir.test.ts +++ b/src/utils/cache-dir.test.ts @@ -1,8 +1,28 @@ import "#veryfront/schemas/_test-setup.ts"; +import { + closeSync, + fstatSync, + lstatSync, + mkdirSync, + mkdtempSync, + openSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, + unlinkSync, + writeFileSync, +} from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; import { deleteEnv, getEnv, setEnv } from "#veryfront/platform/compat/process.ts"; +import { isNode } from "#veryfront/platform/compat/runtime.ts"; import { assert, assertEquals } from "#veryfront/testing/assert.ts"; import { afterEach, describe, it } from "#veryfront/testing/bdd.ts"; import { + __cacheDirInternals, + ensureCacheNodeModules, getCacheBaseDir, getCacheDirFromContext, getHttpBundleCacheDir, @@ -23,6 +43,23 @@ const originalEnv = new Map( MANAGED_ENV_KEYS.map((key) => [key, getEnv(key)]), ); +const nodeCacheRoots = new Set(); + +function makeNodeCacheRoot(): string { + const root = mkdtempSync(join(tmpdir(), "veryfront-cache-node-modules-")); + nodeCacheRoots.add(root); + return root; +} + +function assertFrameworkNodeModulesLink(cacheRoot: string): void { + const link = join(cacheRoot, "node_modules"); + const require = createRequire(import.meta.url); + const expectedReactDir = realpathSync(dirname(require.resolve("react"))); + + assert(lstatSync(link).isSymbolicLink()); + assertEquals(realpathSync(join(link, "react")), expectedReactDir); +} + function restoreManagedEnv(): void { for (const [key, value] of originalEnv) { if (value === undefined) { @@ -36,6 +73,11 @@ function restoreManagedEnv(): void { describe("cache-dir", () => { afterEach(() => { restoreManagedEnv(); + __cacheDirInternals.resetNodeModulesLinkState(); + for (const root of nodeCacheRoots) { + rmSync(root, { recursive: true, force: true }); + } + nodeCacheRoots.clear(); }); describe("getCacheDirFromContext", () => { @@ -44,6 +86,67 @@ describe("cache-dir", () => { }); }); + describe("resolved React module paths", () => { + it("should locate node_modules on POSIX and Windows", () => { + assertEquals( + __cacheDirInternals.getReactNodeModulesDir( + "/repo/node_modules/react/index.js", + ), + "/repo/node_modules", + ); + assertEquals( + __cacheDirInternals.getReactNodeModulesDir( + "C:\\repo\\node_modules\\react\\index.js", + ), + "C:\\repo\\node_modules", + ); + assertEquals( + __cacheDirInternals.getReactNodeModulesDir("/repo/vendor/react.js"), + undefined, + ); + }); + }); + + describe("link failure diagnostics", () => { + it("should describe cache roots without exposing absolute paths", () => { + const cacheRoot = "/tmp/veryfront-cache-node-modules/private"; + const reason = `ENOTDIR: not a directory, mkdir '${cacheRoot}/node_modules'`; + + const context = { + cacheRoot: __cacheDirInternals.describeCacheRoot(cacheRoot), + reason: __cacheDirInternals.redactCachePathDetails(reason, cacheRoot), + }; + + assert(context.cacheRoot.startsWith("cache:")); + assertEquals(context.cacheRoot.includes(cacheRoot), false); + assertEquals(context.reason.includes(cacheRoot), false); + assertEquals(context.reason.includes("/tmp/veryfront-cache-node-modules"), false); + assertEquals(context.reason.includes("ENOTDIR"), true); + }); + + it("redacts both quoted symlink operands when POSIX paths contain spaces", () => { + const cacheRoot = "/workspace/private person/cache root"; + const frameworkRoot = "/workspace/private person/framework/node_modules"; + const reason = `EEXIST: symlink '${frameworkRoot}' -> '${cacheRoot}/node_modules'`; + + const redacted = __cacheDirInternals.redactCachePathDetails(reason, cacheRoot); + + assertEquals(redacted, "EEXIST: symlink '[path]' -> '[path]'"); + assertEquals(redacted.includes("private person"), false); + }); + + it("redacts both quoted symlink operands when Windows paths contain spaces", () => { + const cacheRoot = "C:\\workspace\\private person\\cache root"; + const frameworkRoot = "C:\\workspace\\private person\\framework\\node_modules"; + const reason = `EPERM: symlink '${frameworkRoot}' -> '${cacheRoot}\\node_modules'`; + + const redacted = __cacheDirInternals.redactCachePathDetails(reason, cacheRoot); + + assertEquals(redacted, "EPERM: symlink '[path]' -> '[path]'"); + assertEquals(redacted.includes("private person"), false); + }); + }); + describe("runWithCacheDir", () => { it("should make cache dir available within the callback", () => { const result = runWithCacheDir("/tmp/test-cache", getCacheDirFromContext); @@ -158,4 +261,104 @@ describe("cache-dir", () => { assert(result.endsWith("veryfront-http-bundle")); }); }); + + describe({ name: "ensureCacheNodeModules on Node", ignore: !isNode }, () => { + it("should link distinct cache roots independently", async () => { + const firstRoot = makeNodeCacheRoot(); + const secondRoot = makeNodeCacheRoot(); + + await Promise.all([ + runWithCacheDir(firstRoot, ensureCacheNodeModules), + runWithCacheDir(secondRoot, ensureCacheNodeModules), + ]); + + assertFrameworkNodeModulesLink(firstRoot); + assertFrameworkNodeModulesLink(secondRoot); + }); + + it("should deduplicate concurrent callers and revalidate verified roots", async () => { + const cacheRoot = makeNodeCacheRoot(); + + const results = await runWithCacheDir( + cacheRoot, + () => Promise.all(Array.from({ length: 20 }, () => ensureCacheNodeModules())), + ); + assertEquals(results, Array.from({ length: 20 }, () => true)); + assertFrameworkNodeModulesLink(cacheRoot); + + // A remembered root must still be checked: another process can clear a + // cache directory after the first successful call. + unlinkSync(join(cacheRoot, "node_modules")); + assertEquals(await runWithCacheDir(cacheRoot, ensureCacheNodeModules), true); + assertFrameworkNodeModulesLink(cacheRoot); + }); + + it("should report failure when the link cannot be created", async () => { + const cacheRoot = makeNodeCacheRoot(); + const blocker = join(cacheRoot, "blocker"); + writeFileSync(blocker, "not a directory"); + const impossibleCacheBase = join(blocker, "nested"); + + assertEquals( + await runWithCacheDir(impossibleCacheBase, ensureCacheNodeModules), + false, + ); + + // A failed root is not memoized as verified; once the obstruction is + // gone the next call self-heals and reports success. + unlinkSync(blocker); + assertEquals( + await runWithCacheDir(impossibleCacheBase, ensureCacheNodeModules), + true, + ); + assertFrameworkNodeModulesLink(impossibleCacheBase); + }); + + it("should replace wrong and dangling symlinks", async () => { + const cacheRoot = makeNodeCacheRoot(); + const wrongTarget = join(cacheRoot, "wrong-node-modules"); + const link = join(cacheRoot, "node_modules"); + mkdirSync(wrongTarget); + symlinkSync(wrongTarget, link, "dir"); + + await runWithCacheDir(cacheRoot, ensureCacheNodeModules); + assertFrameworkNodeModulesLink(cacheRoot); + + unlinkSync(link); + symlinkSync(join(cacheRoot, "missing-node-modules"), link, "dir"); + + await runWithCacheDir(cacheRoot, ensureCacheNodeModules); + assertFrameworkNodeModulesLink(cacheRoot); + }); + + it("should preserve a real node_modules directory", async () => { + const cacheRoot = makeNodeCacheRoot(); + const nodeModulesDir = join(cacheRoot, "node_modules"); + const marker = join(nodeModulesDir, "keep.txt"); + mkdirSync(nodeModulesDir); + writeFileSync(marker, "keep"); + + await runWithCacheDir(cacheRoot, ensureCacheNodeModules); + + assert(lstatSync(nodeModulesDir).isDirectory()); + assertEquals(lstatSync(nodeModulesDir).isSymbolicLink(), false); + assertEquals(readFileSync(marker, "utf8"), "keep"); + }); + + it("should preserve a non-directory node_modules entry", async () => { + const cacheRoot = makeNodeCacheRoot(); + const nodeModulesEntry = join(cacheRoot, "node_modules"); + writeFileSync(nodeModulesEntry, "keep"); + + await runWithCacheDir(cacheRoot, ensureCacheNodeModules); + + const entry = openSync(nodeModulesEntry, "r"); + try { + assert(fstatSync(entry).isFile()); + assertEquals(readFileSync(entry, "utf8"), "keep"); + } finally { + closeSync(entry); + } + }); + }); }); diff --git a/src/utils/cache-dir.ts b/src/utils/cache-dir.ts index 1499544951..982da37452 100644 --- a/src/utils/cache-dir.ts +++ b/src/utils/cache-dir.ts @@ -2,9 +2,72 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { join } from "#veryfront/compat/path/index.ts"; import { cwd, getHostEnv } from "#veryfront/platform/compat/process.ts"; import { isNode } from "#veryfront/platform/compat/runtime.ts"; +import { hashString } from "#veryfront/cache/hash.ts"; +import { serverLogger } from "./logger/index.ts"; + +const logger = serverLogger.component("cache-dir"); const cacheStorage = new AsyncLocalStorage(); -let nodeModulesLinked = false; +const nodeModulesLinkOperations = new Map>(); + +// Bounded memo of cache roots and their expected framework dependency root. +// A cache hit still validates the target entry: cache directories can be +// cleared or replaced by another process, so remembered success must never +// turn a missing or wrong link into a false-positive `true` result. +const MAX_SETTLED_CACHE_ROOTS = 128; +const verifiedCacheRoots = new Map(); +// Roots whose link creation failed, kept only to log the failure once. +const warnedLinkFailureRoots = new Set(); + +function rememberBounded(set: Set, value: string): void { + if (set.size >= MAX_SETTLED_CACHE_ROOTS) { + const oldest = set.values().next().value; + if (oldest !== undefined) set.delete(oldest); + } + set.add(value); +} + +function rememberVerifiedRoot(cacheBase: string, nodeModulesDir: string): void { + if (!verifiedCacheRoots.has(cacheBase) && verifiedCacheRoots.size >= MAX_SETTLED_CACHE_ROOTS) { + const oldest = verifiedCacheRoots.keys().next().value; + if (oldest !== undefined) verifiedCacheRoots.delete(oldest); + } + verifiedCacheRoots.set(cacheBase, nodeModulesDir); +} + +function getReactNodeModulesDir(reactEntry: string): string | undefined { + const normalizedReactEntry = reactEntry.replaceAll("\\", "/"); + const marker = "/node_modules/react"; + const markerIndex = normalizedReactEntry.lastIndexOf(marker); + if (markerIndex === -1) return undefined; + return reactEntry.slice(0, markerIndex + "/node_modules".length); +} + +function describeCacheRoot(cacheBase: string): string { + return `cache:${hashString(cacheBase)}`; +} + +function redactCachePathDetails(reason: string, cacheBase: string): string { + return reason + .replace(/(["'`])(?:(?:[A-Za-z]:)?[\\/])[^"'`]*\1/g, "$1[path]$1") + .replaceAll(cacheBase, "[cache-dir]") + .replace(/(?:[A-Za-z]:)?[\\/][^\s'"`]+/g, "[path]"); +} + +/** Reset memoized link state (test seam). */ +function resetNodeModulesLinkState(): void { + nodeModulesLinkOperations.clear(); + verifiedCacheRoots.clear(); + warnedLinkFailureRoots.clear(); +} + +/** Internal test seam for platform-specific resolved module paths. */ +export const __cacheDirInternals = { + describeCacheRoot, + getReactNodeModulesDir, + redactCachePathDetails, + resetNodeModulesLinkState, +}; export function runWithCacheDir(cacheDir: string, fn: () => T): T { return cacheStorage.run(cacheDir, fn); @@ -56,37 +119,132 @@ export function getHttpBundleCacheDir(): string { * * so Node.js module resolution finds the same packages the framework itself uses, * guaranteeing a single React instance (no "Invalid hook call" errors). + * + * Returns `true` when a usable framework dependency root is in place for the + * cache dir (link created, correct link already present, or an equivalent + * real directory), and `false` when it could not be ensured — so callers can + * distinguish total link failure from success. Failures are logged once per + * cache dir and are retried on later calls (self-healing); only verified + * roots retain their expected framework dependency root for a cheaper + * revalidation on later calls. */ -export async function ensureCacheNodeModules(): Promise { - if (!isNode || nodeModulesLinked) return; - nodeModulesLinked = true; +export async function ensureCacheNodeModules(): Promise { + if (!isNode) return true; + + // Key the memoized link operation by the resolved cache base dir: + // getCacheBaseDir() is AsyncLocalStorage-scoped, so different requests can + // resolve different cache dirs. A single global done-flag would let the + // first cache dir claim the link forever and leave every other cache dir + // without a node_modules symlink (second React copy → "Invalid hook call"). + // Storing the in-flight promise also makes concurrent callers wait for the + // link to actually exist instead of returning before the async work is done. + const cacheBase = getCacheBaseDir(); + const verifiedRoot = verifiedCacheRoots.get(cacheBase); + if (verifiedRoot !== undefined) { + if (await isCacheNodeModulesUsable(cacheBase, verifiedRoot)) return true; + verifiedCacheRoots.delete(cacheBase); + } + let operation = nodeModulesLinkOperations.get(cacheBase); + if (!operation) { + operation = linkCacheNodeModules(cacheBase); + nodeModulesLinkOperations.set(cacheBase, operation); + } try { - const { createRequire } = await import("node:module"); - const { lstatSync, symlinkSync, mkdirSync } = await import("node:fs"); + const nodeModulesDir = await operation; + if (nodeModulesDir === undefined) return false; + rememberVerifiedRoot(cacheBase, nodeModulesDir); + warnedLinkFailureRoots.delete(cacheBase); + return true; + } finally { + // The in-flight map deduplicates only concurrent work. The identity check + // prevents an older waiter from deleting a replacement operation. + if (nodeModulesLinkOperations.get(cacheBase) === operation) { + nodeModulesLinkOperations.delete(cacheBase); + } + } +} - const cacheBase = getCacheBaseDir(); +async function isCacheNodeModulesUsable( + cacheBase: string, + nodeModulesDir: string, +): Promise { + try { + const { lstatSync, realpathSync } = await import("node:fs"); const targetLink = join(cacheBase, "node_modules"); - - try { - lstatSync(targetLink); - return; - } catch (_) { - /* expected: symlink doesn't exist yet */ + const existing = lstatSync(targetLink); + if (existing.isSymbolicLink()) { + return realpathSync(targetLink) === realpathSync(nodeModulesDir); } + if (!existing.isDirectory()) return false; + return realpathSync(join(targetLink, "react")) === + realpathSync(join(nodeModulesDir, "react")); + } catch { + return false; + } +} + +async function linkCacheNodeModules(cacheBase: string): Promise { + try { + const { createRequire } = await import("node:module"); + const { lstatSync, mkdirSync, realpathSync, symlinkSync, unlinkSync } = await import("node:fs"); + + const targetLink = join(cacheBase, "node_modules"); const require = createRequire(import.meta.url); const reactEntry = require.resolve("react"); + const nodeModulesDir = getReactNodeModulesDir(reactEntry); + if (!nodeModulesDir) return warnLinkFailure(cacheBase, "framework node_modules not found"); - const marker = "/node_modules/react"; - const idx = reactEntry.lastIndexOf(marker); - if (idx === -1) return; - - const nodeModulesDir = reactEntry.substring(0, idx + "/node_modules".length); + try { + const existing = lstatSync(targetLink); + if (existing.isSymbolicLink()) { + try { + if (realpathSync(targetLink) === realpathSync(nodeModulesDir)) return nodeModulesDir; + } catch { + // A dangling link is safe to replace without touching its target. + } + unlinkSync(targetLink); + } else if (existing.isDirectory()) { + // Preserve a real directory only when it resolves React to the same + // framework-owned package. Never remove user-created directories. + try { + if ( + realpathSync(join(targetLink, "react")) === + realpathSync(join(nodeModulesDir, "react")) + ) return nodeModulesDir; + } catch { + // The existing directory is not a usable framework dependency root. + } + return warnLinkFailure(cacheBase, "existing node_modules directory preserved"); + } else { + // Do not overwrite a non-directory entry in a best-effort helper. + return warnLinkFailure(cacheBase, "existing non-directory node_modules entry preserved"); + } + } catch (_) { + // No entry exists yet. mkdir/symlink below owns creation. + } mkdirSync(cacheBase, { recursive: true }); symlinkSync(nodeModulesDir, targetLink, "dir"); - } catch (_) { - /* expected: best-effort symlink may fail due to permissions or platform */ + return nodeModulesDir; + } catch (error) { + // Best-effort: symlink creation may fail due to permissions or platform, + // but total failure must stay observable instead of looking like success. + return warnLinkFailure( + cacheBase, + error instanceof Error ? error.message : String(error), + ); + } +} + +function warnLinkFailure(cacheBase: string, reason: string): undefined { + if (!warnedLinkFailureRoots.has(cacheBase)) { + rememberBounded(warnedLinkFailureRoots, cacheBase); + logger.warn("Cache node_modules link not established", { + cacheRoot: describeCacheRoot(cacheBase), + reason: redactCachePathDetails(reason, cacheBase), + }); } + return undefined; } diff --git a/src/utils/cache-file-ops.test.ts b/src/utils/cache-file-ops.test.ts index f1b3afb8c0..159f242fa2 100644 --- a/src/utils/cache-file-ops.test.ts +++ b/src/utils/cache-file-ops.test.ts @@ -2,6 +2,14 @@ import "#veryfront/schemas/_test-setup.ts"; import { assertEquals, assertRejects } from "#veryfront/testing/assert"; import { describe, it } from "#veryfront/testing/bdd"; import { isCacheWriteRaceError, verifyCacheFileExists, writeCacheFile } from "./cache-file-ops.ts"; +import { + __resetLoggerConfigForTests, + __resetLogRecordEmitterForTests, + __subscribeLogRecordEmitter, + type LogEntry, + LogLevel, + setLogLevel, +} from "./logger/logger.ts"; import type { FileSystem } from "#veryfront/platform/compat/fs.ts"; import type { FileInfo } from "#veryfront/platform/adapters/base.ts"; @@ -21,6 +29,10 @@ const DIR_STAT: FileInfo = { mtime: null, }; +function filesystemError(message: string, code: string): Error & { code: string } { + return Object.assign(new Error(message), { code }); +} + function createMockFs(overrides: Partial = {}): FileSystem { return { readTextFile: () => Promise.resolve(""), @@ -101,15 +113,27 @@ describe("cache-file-ops", () => { ); }); - it("returns false when post-write verification fails", async () => { + it("returns false when post-write verification finds a missing file", async () => { const fs = createMockFs({ - stat: () => Promise.reject(new Error("file gone")), + stat: () => Promise.reject(filesystemError("file gone", "ENOENT")), }); const result = await writeCacheFile(fs, "/cache/dir/file.js", "content", "TEST"); assertEquals(result, false); }); + it("propagates operational post-write verification failures", async () => { + const fs = createMockFs({ + stat: () => Promise.reject(filesystemError("permission denied", "EACCES")), + }); + + await assertRejects( + () => writeCacheFile(fs, "/cache/dir/file.js", "content", "TEST"), + Error, + "permission denied", + ); + }); + it("returns false when stat says not a file", async () => { const fs = createMockFs({ stat: () => Promise.resolve(DIR_STAT), @@ -132,13 +156,83 @@ describe("cache-file-ops", () => { it("returns false when file does not exist", async () => { const fs = createMockFs({ - stat: () => Promise.reject(new Error("not found")), + stat: () => Promise.reject(filesystemError("not found", "ENOENT")), }); const result = await verifyCacheFileExists(fs, "/cache/file.js", "TEST"); assertEquals(result, false); }); + it("propagates operational existence-check failures", async () => { + const fs = createMockFs({ + stat: () => Promise.reject(filesystemError("permission denied", "EACCES")), + }); + + await assertRejects( + () => verifyCacheFileExists(fs, "/cache/file.js", "TEST"), + Error, + "permission denied", + ); + }); + + it("redacts the cache path from operational failure logs", async () => { + const originalDebug = console.debug; + const records: LogEntry[] = []; + const path = "/srv/private workspace/cache/file.js"; + const fs = createMockFs({ + stat: () => + Promise.reject(filesystemError(`EACCES: permission denied, stat '${path}'`, "EACCES")), + }); + + console.debug = () => {}; + __resetLogRecordEmitterForTests(); + setLogLevel(LogLevel.DEBUG); + const unsubscribe = __subscribeLogRecordEmitter((entry) => { + records.push(entry); + }); + + try { + await assertRejects( + () => verifyCacheFileExists(fs, path, "TEST"), + Error, + "permission denied", + ); + } finally { + unsubscribe(); + __resetLogRecordEmitterForTests(); + __resetLoggerConfigForTests(); + console.debug = originalDebug; + } + + assertEquals(records.length, 1); + assertEquals(records[0]?.context?.error, "EACCES: permission denied, stat '[path]'"); + assertEquals(String(records[0]?.context?.error).includes("private workspace"), false); + }); + + it("propagates I/O failures instead of reporting a cache miss", async () => { + const fs = createMockFs({ + stat: () => Promise.reject(filesystemError("input/output error", "EIO")), + }); + + await assertRejects( + () => verifyCacheFileExists(fs, "/cache/file.js", "TEST"), + Error, + "input/output error", + ); + }); + + it("requires custom adapters to classify missing paths", async () => { + const fs = createMockFs({ + stat: () => Promise.reject(new Error("not found")), + }); + + await assertRejects( + () => verifyCacheFileExists(fs, "/cache/file.js", "TEST"), + Error, + "not found", + ); + }); + it("returns false when path is a directory", async () => { const fs = createMockFs({ stat: () => Promise.resolve(DIR_STAT), diff --git a/src/utils/cache-file-ops.ts b/src/utils/cache-file-ops.ts index 43ead95b9a..c941a6010b 100644 --- a/src/utils/cache-file-ops.ts +++ b/src/utils/cache-file-ops.ts @@ -5,9 +5,18 @@ * to ensure consistent, robust file handling across all cache code paths. */ -import type { FileSystem } from "#veryfront/platform/compat/fs.ts"; +import { type FileSystem, isNotFoundError } from "#veryfront/platform/compat/fs.ts"; +import { redactPathFromText } from "#veryfront/utils/logger/redact.ts"; import { rendererLogger as logger } from "#veryfront/utils"; +function describeCacheError(error: unknown, ...paths: string[]): string { + const raw = error instanceof Error ? error.message : String(error); + return paths.reduce( + (message, path) => redactPathFromText(message, path, "[path]"), + raw, + ); +} + /** * Safely write a cache file: mkdir parent dir → write file → verify file exists. * @@ -29,7 +38,7 @@ export async function writeCacheFile( logger.debug(`[${label}] mkdir failed for cache file parent`, { path: path.slice(-80), dir: parentDir.slice(-80), - error: mkdirError instanceof Error ? mkdirError.message : String(mkdirError), + error: describeCacheError(mkdirError, path, parentDir), }); throw mkdirError; } @@ -46,7 +55,7 @@ export async function writeCacheFile( } logger.debug(`[${label}] Failed to write cache file`, { path: path.slice(-80), - error: writeError instanceof Error ? writeError.message : String(writeError), + error: describeCacheError(writeError, path, parentDir), }); throw writeError; } @@ -63,9 +72,10 @@ export async function writeCacheFile( } catch (verifyError) { logger.debug(`[${label}] Cache file verification failed: cannot stat after write`, { path: path.slice(-80), - error: verifyError instanceof Error ? verifyError.message : String(verifyError), + error: describeCacheError(verifyError, path, parentDir), }); - return false; + if (isNotFoundError(verifyError)) return false; + throw verifyError; } return true; @@ -73,19 +83,27 @@ export async function writeCacheFile( /** * Verify a cache file exists before attempting dynamic import. - * Returns true if file exists and is a regular file, false otherwise. + * Returns true if the file exists and is a regular file, false when the path + * is genuinely absent or is not a regular file. Non-absence stat failures + * (EACCES, EIO, ...) are + * rethrown so callers do not misreport an unreadable cache as a cache miss + * and loop forever re-transforming the same module. */ export async function verifyCacheFileExists( fs: FileSystem, path: string, - _label = "cache", + label = "cache", ): Promise { try { const stat = await fs.stat(path); return !!stat?.isFile; - } catch (_) { - /* expected: file may not exist */ - return false; + } catch (error) { + if (isNotFoundError(error)) return false; + logger.debug(`[${label}] Cache file existence check failed`, { + path: path.slice(-80), + error: describeCacheError(error, path), + }); + throw error; } } diff --git a/src/utils/cache/eviction/eviction-manager.test.ts b/src/utils/cache/eviction/eviction-manager.test.ts index c0f8958539..569eb4a679 100644 --- a/src/utils/cache/eviction/eviction-manager.test.ts +++ b/src/utils/cache/eviction/eviction-manager.test.ts @@ -33,12 +33,22 @@ describe("EvictionManager", () => { assertEquals(em.isExpired({ size: 1, expiry: 3000 }, undefined, 2000), false); }); + it("should expire an entry exactly at its expiry timestamp", () => { + const em = new EvictionManager(); + assertEquals(em.isExpired({ size: 1, expiry: 2000 }, undefined, 2000), true); + }); + it("should use timestamp + ttl when no expiry", () => { const em = new EvictionManager(); assertEquals(em.isExpired({ size: 1, timestamp: 1000 }, 500, 2000), true); assertEquals(em.isExpired({ size: 1, timestamp: 1000 }, 5000, 2000), false); }); + it("should expire a timestamp-based entry exactly at its ttl boundary", () => { + const em = new EvictionManager(); + assertEquals(em.isExpired({ size: 1, timestamp: 1000 }, 1000, 2000), true); + }); + it("should return false when no expiry info available", () => { const em = new EvictionManager(); assertEquals(em.isExpired({ size: 1 }), false); diff --git a/src/utils/cache/eviction/eviction-manager.ts b/src/utils/cache/eviction/eviction-manager.ts index 10882e83ed..4c02cf607f 100644 --- a/src/utils/cache/eviction/eviction-manager.ts +++ b/src/utils/cache/eviction/eviction-manager.ts @@ -155,12 +155,12 @@ export class EvictionManager { isExpired(entry: TEntry, ttl?: number, now: number = Date.now()): boolean { const expiry = entry.expiry; if (typeof expiry === "number") { - return now > expiry; + return now >= expiry; } const timestamp = entry.timestamp; if (typeof timestamp === "number" && typeof ttl === "number") { - return now - timestamp > ttl; + return now - timestamp >= ttl; } return false; diff --git a/src/utils/cache/stores/memory/entry-manager.ts b/src/utils/cache/stores/memory/entry-manager.ts index 522066faa1..a839fa4255 100644 --- a/src/utils/cache/stores/memory/entry-manager.ts +++ b/src/utils/cache/stores/memory/entry-manager.ts @@ -3,7 +3,10 @@ import { LRUNode } from "./lru-node.ts"; import type { LRUListManager } from "./lru-list-manager.ts"; export class EntryManager { - constructor(private readonly estimateSizeOf: (value: unknown) => number) {} + constructor( + private readonly estimateSizeOf: (value: unknown) => number, + private readonly now: () => number = Date.now, + ) {} updateExistingEntry( node: LRUNode, @@ -26,7 +29,7 @@ export class EntryManager { size: newSize, expiry, tags, - lastAccessed: Date.now(), + lastAccessed: this.now(), }; listManager.moveToFront(node); @@ -51,7 +54,7 @@ export class EntryManager { size, expiry, tags, - lastAccessed: Date.now(), + lastAccessed: this.now(), }; const node = new LRUNode(key, entry); @@ -91,8 +94,8 @@ export class EntryManager { ttlMs: number | undefined, defaultTtlMs: number | undefined, ): number | undefined { - if (typeof ttlMs === "number") return Date.now() + ttlMs; - if (typeof defaultTtlMs === "number") return Date.now() + defaultTtlMs; + if (typeof ttlMs === "number") return this.now() + ttlMs; + if (typeof defaultTtlMs === "number") return this.now() + defaultTtlMs; return undefined; } } diff --git a/src/utils/cache/stores/memory/lru-cache-adapter.test.ts b/src/utils/cache/stores/memory/lru-cache-adapter.test.ts index 2cc17ca29d..8c68313f94 100644 --- a/src/utils/cache/stores/memory/lru-cache-adapter.test.ts +++ b/src/utils/cache/stores/memory/lru-cache-adapter.test.ts @@ -33,6 +33,13 @@ describe("LRUCacheAdapter", () => { expect(cache.has("nonexistent")).toBe(false); }); + it("should distinguish a stored undefined value from a missing key", () => { + cache.set("present", undefined); + + expect(cache.has("present")).toBe(true); + expect(cache.has("missing")).toBe(false); + }); + it("should clear all entries", () => { cache.set("key1", "value1"); cache.set("key2", "value2"); @@ -51,6 +58,20 @@ describe("LRUCacheAdapter", () => { expect(keys).toContain("key1"); expect(keys).toContain("key2"); }); + + it("does not expose expired entries through key iteration", () => { + let now = Date.now(); + const cacheWithClock = new LRUCacheAdapter({ + maxEntries: 5, + maxSizeBytes: 1024, + now: () => now, + }); + cacheWithClock.set("expired", "value", 10); + cacheWithClock.set("fresh", "value", 100); + now += 10; + + expect([...cacheWithClock.keys()]).toEqual(["fresh"]); + }); }); describe("LRU eviction", () => { @@ -83,6 +104,24 @@ describe("LRUCacheAdapter", () => { expect(smallCache.get("c")).toBe("3"); expect(smallCache.get("d")).toBe("4"); }); + + it("uses the injected clock for access timestamps", () => { + let now = 100; + const cacheWithClock = new LRUCacheAdapter({ now: () => now }); + const inspectHead = () => + (cacheWithClock as unknown as { + listManager: { + getHead(): { entry: { lastAccessed: number } } | null; + }; + }).listManager.getHead(); + + cacheWithClock.set("key", "value"); + expect(inspectHead()?.entry.lastAccessed).toBe(100); + + now = 250; + cacheWithClock.get("key"); + expect(inspectHead()?.entry.lastAccessed).toBe(250); + }); }); describe("TTL expiration", () => { @@ -112,6 +151,18 @@ describe("LRUCacheAdapter", () => { expect(cache.cleanupExpired()).toBe(2); expect(cache.get("keep")).toBe("value3"); }); + + it("expires entries exactly at their expiry timestamp", () => { + let now = Date.now(); + const cacheWithClock = new LRUCacheAdapter({ + maxEntries: 5, + maxSizeBytes: 1024, + now: () => now, + }); + cacheWithClock.set("boundary", "value", 10); + now += 10; + expect(cacheWithClock.has("boundary")).toBe(false); + }); }); describe("tag-based invalidation", () => { @@ -139,6 +190,16 @@ describe("LRUCacheAdapter", () => { it("should return 0 when invalidating non-existent tag", () => { expect(cache.invalidateTag("nonexistent")).toBe(0); }); + + it("snapshots tags so caller mutation cannot leak index entries", () => { + const tags = ["tag-a"]; + cache.set("key", "value", undefined, tags); + tags[0] = "mutated"; + + cache.delete("key"); + + expect(cache.getStats().tags).toBe(0); + }); }); describe("size management", () => { @@ -175,6 +236,53 @@ describe("LRUCacheAdapter", () => { expect(stats.maxSizeBytes).toBe(1024); expect(stats.tags).toBe(2); }); + + it("excludes expired entries without requiring a cleanup sweep", () => { + let now = 100; + const cacheWithClock = new LRUCacheAdapter({ + maxEntries: 5, + maxSizeBytes: 1024, + now: () => now, + }); + cacheWithClock.set("expired", "value", 10, ["expired-tag"]); + cacheWithClock.set("fresh", "value", 100, ["fresh-tag"]); + const retainedSize = cacheWithClock.getStats().sizeBytes; + now = 110; + + expect([...cacheWithClock.keys()]).toEqual(["fresh"]); + const stats = cacheWithClock.getStats(); + expect(stats.entries).toBe(1); + expect(stats.sizeBytes).toBeLessThan(retainedSize); + expect(stats.sizeBytes).toBeGreaterThan(0); + expect(stats.maxEntries).toBe(5); + expect(stats.maxSizeBytes).toBe(1024); + expect(stats.tags).toBe(1); + expect(retainedSize).toBeGreaterThan(0); + }); + + it("does not mutate the cache or fire onEvict from a stats read", () => { + let now = 100; + const evicted: string[] = []; + const cacheWithClock = new LRUCacheAdapter({ + maxEntries: 5, + maxSizeBytes: 1024, + now: () => now, + onEvict: (key) => { + evicted.push(key); + }, + }); + cacheWithClock.set("expired", "value", 10); + cacheWithClock.set("fresh", "value", 100); + now = 110; + + expect(cacheWithClock.getStats().entries).toBe(1); + + // The stats read is pure: no observer fired, and the expired entry is + // still present internally for the mutating cleanup path to reclaim. + expect(evicted).toEqual([]); + expect(cacheWithClock.cleanupExpired()).toBe(1); + expect(evicted).toEqual(["expired"]); + }); }); describe("onEvict callback", () => { @@ -228,6 +336,25 @@ describe("LRUCacheAdapter", () => { expect(evicted).toContain("a"); expect(evicted).toContain("b"); }); + + it("should clear every entry when an onEvict callback throws", () => { + const evicted: string[] = []; + const callbackCache = new LRUCacheAdapter({ + maxEntries: 10, + onEvict: (key) => { + evicted.push(key); + if (key === "a") throw new Error("onEvict error"); + }, + }); + + callbackCache.set("a", "1"); + callbackCache.set("b", "2"); + callbackCache.clear(); + + expect(evicted).toEqual(["a", "b"]); + expect(callbackCache.getStats().entries).toBe(0); + expect(callbackCache.getStats().sizeBytes).toBe(0); + }); }); describe("update existing entry", () => { diff --git a/src/utils/cache/stores/memory/lru-cache-adapter.ts b/src/utils/cache/stores/memory/lru-cache-adapter.ts index 410edf7ee9..8e088b88e1 100644 --- a/src/utils/cache/stores/memory/lru-cache-adapter.ts +++ b/src/utils/cache/stores/memory/lru-cache-adapter.ts @@ -56,7 +56,7 @@ function defaultSizeEstimator(value: unknown): number { export class LRUCacheAdapter implements CacheAdapter { private readonly store = new Map>(); private readonly tagIndex = new Map>(); - private readonly listManager = new LRUListManager(); + private readonly listManager: LRUListManager; private readonly evictionManager: EvictionManager>; private readonly entryManager: EntryManager; private currentSize = 0; @@ -64,12 +64,15 @@ export class LRUCacheAdapter implements CacheAdapter { private readonly maxSizeBytes: number; private readonly defaultTtlMs?: number; private readonly onEvict?: (key: string, value: unknown) => void; + private readonly now: () => number; constructor(options: LRUCacheOptions = {}) { this.maxEntries = options.maxEntries || 1000; this.maxSizeBytes = options.maxSizeBytes || 50 * 1024 * 1024; this.defaultTtlMs = options.ttlMs; this.onEvict = options.onEvict; + this.now = options.now ?? Date.now; + this.listManager = new LRUListManager(this.now); const estimateSizeOf = options.estimateSizeOf ?? defaultSizeEstimator; @@ -77,14 +80,23 @@ export class LRUCacheAdapter implements CacheAdapter { onEvict: this.onEvict, loggerContext: "MemoryCache", }); - this.entryManager = new EntryManager(estimateSizeOf); + this.entryManager = new EntryManager(estimateSizeOf, this.now); + } + + /** + * Entries expire exactly at their expiry timestamp. Delegates to + * EvictionManager.isExpired so the boundary semantics have one source of + * truth across the cache subsystem. + */ + private isExpired(entry: LRUEntry, now: number): boolean { + return this.evictionManager.isExpired(entry, undefined, now); } get(key: string): T | undefined { const node = this.store.get(key); if (!node) return undefined; - if (this.evictionManager.isExpired(node.entry)) { + if (this.isExpired(node.entry, this.now())) { this.delete(key); return undefined; } @@ -94,6 +106,9 @@ export class LRUCacheAdapter implements CacheAdapter { } set(key: string, value: T, ttlMs?: number, tags?: string[]): void { + // Snapshot tags so a caller mutating the array after set() cannot desync + // the tag index from the tags retained on the entry. + const storedTags = tags ? [...tags] : undefined; const existingNode = this.store.get(key); if (existingNode) { @@ -101,7 +116,7 @@ export class LRUCacheAdapter implements CacheAdapter { existingNode, value, ttlMs, - tags, + storedTags, this.defaultTtlMs, this.listManager, this.tagIndex, @@ -112,7 +127,7 @@ export class LRUCacheAdapter implements CacheAdapter { key, value, ttlMs, - tags, + storedTags, this.defaultTtlMs, this.listManager, this.store, @@ -120,7 +135,7 @@ export class LRUCacheAdapter implements CacheAdapter { this.currentSize += size; } - if (tags?.length) this.entryManager.updateTagIndex(tags, key, this.tagIndex); + if (storedTags?.length) this.entryManager.updateTagIndex(storedTags, key, this.tagIndex); this.currentSize = this.evictionManager.enforceMemoryLimits( this.listManager, @@ -169,7 +184,16 @@ export class LRUCacheAdapter implements CacheAdapter { clear(): void { if (this.onEvict) { for (const [key, node] of this.store) { - this.onEvict(key, node.entry.value); + try { + this.onEvict(key, node.entry.value); + } catch (error) { + // A throwing observer must not abort clear() and leave the cache + // populated; every entry is still notified and then removed. + logger.warn("onEvict callback threw during clear", { + key, + error: error instanceof Error ? error.message : String(error), + }); + } } } @@ -180,21 +204,39 @@ export class LRUCacheAdapter implements CacheAdapter { } getStats(): LRUCacheStats { + // Pure expiry view: a stats read must not delete entries or fire onEvict + // observers. Expired entries are excluded from every reported figure + // (matching keys()/entries()); reclamation stays with cleanupExpired() + // and the mutating paths. + const now = this.now(); + let entries = 0; + let sizeBytes = 0; + const liveTags = new Set(); + + for (const node of this.store.values()) { + if (this.isExpired(node.entry, now)) continue; + entries++; + sizeBytes += node.entry.size; + if (node.entry.tags) { + for (const tag of node.entry.tags) liveTags.add(tag); + } + } + return { - entries: this.store.size, - sizeBytes: this.currentSize, + entries, + sizeBytes, maxEntries: this.maxEntries, maxSizeBytes: this.maxSizeBytes, - tags: this.tagIndex.size, + tags: liveTags.size, }; } cleanupExpired(): number { - const now = Date.now(); + const now = this.now(); let cleaned = 0; for (const [key, node] of this.store) { - if (typeof node.entry.expiry !== "number" || now <= node.entry.expiry) continue; + if (!this.isExpired(node.entry, now)) continue; this.delete(key); cleaned++; } @@ -202,19 +244,34 @@ export class LRUCacheAdapter implements CacheAdapter { return cleaned; } - keys(): IterableIterator { - return this.store.keys(); + *keys(): IterableIterator { + const now = this.now(); + for (const [key, node] of this.store) { + if (!this.isExpired(node.entry, now)) yield key; + } } *entries(): IterableIterator<[string, T]> { + const now = this.now(); for (const [key, node] of this.store) { - if (!this.evictionManager.isExpired(node.entry)) { + if (!this.isExpired(node.entry, now)) { yield [key, node.entry.value as T]; } } } has(key: string): boolean { - return this.get(key) !== undefined; + const node = this.store.get(key); + if (!node) return false; + + if (this.isExpired(node.entry, this.now())) { + this.delete(key); + return false; + } + + // A stored `undefined` value is still a present entry; membership must + // not be derived from get()'s undefined sentinel. + this.listManager.moveToFront(node); + return true; } } diff --git a/src/utils/cache/stores/memory/lru-list-manager.test.ts b/src/utils/cache/stores/memory/lru-list-manager.test.ts index ce793dfae9..bbd92f2ffe 100644 --- a/src/utils/cache/stores/memory/lru-list-manager.test.ts +++ b/src/utils/cache/stores/memory/lru-list-manager.test.ts @@ -1,5 +1,5 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals } from "#veryfront/testing/assert.ts"; +import { assertEquals, assertExists } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { LRUListManager } from "./lru-list-manager.ts"; import { LRUNode } from "./lru-node.ts"; @@ -24,6 +24,12 @@ function createListWithNodes(...keys: string[]): { return { list, nodes }; } +function getNode(nodes: Record>, key: string): LRUNode { + const node = nodes[key]; + assertExists(node); + return node; +} + describe("LRUListManager", () => { describe("addToFront", () => { it("should set single node as both head and tail", () => { @@ -56,7 +62,7 @@ describe("LRUListManager", () => { it("should be no-op for head node", () => { const { list, nodes } = createListWithNodes("a", "b"); - list.moveToFront(nodes.b); + list.moveToFront(getNode(nodes, "b")); assertEquals(list.getHead()?.key, "b"); assertEquals(list.getTail()?.key, "a"); @@ -65,7 +71,7 @@ describe("LRUListManager", () => { it("should move tail to front", () => { const { list, nodes } = createListWithNodes("a", "b", "c"); - list.moveToFront(nodes.a); + list.moveToFront(getNode(nodes, "a")); assertEquals(list.getHead()?.key, "a"); assertEquals(list.getTail()?.key, "b"); @@ -74,7 +80,7 @@ describe("LRUListManager", () => { it("should move middle node to front", () => { const { list, nodes } = createListWithNodes("a", "b", "c"); - list.moveToFront(nodes.b); + list.moveToFront(getNode(nodes, "b")); assertEquals(list.getHead()?.key, "b"); assertEquals(list.getHead()?.next?.key, "c"); @@ -86,7 +92,7 @@ describe("LRUListManager", () => { it("should remove head node", () => { const { list, nodes } = createListWithNodes("a", "b"); - list.removeNode(nodes.b); + list.removeNode(getNode(nodes, "b")); assertEquals(list.getHead()?.key, "a"); assertEquals(list.getTail()?.key, "a"); @@ -95,7 +101,7 @@ describe("LRUListManager", () => { it("should remove tail node", () => { const { list, nodes } = createListWithNodes("a", "b"); - list.removeNode(nodes.a); + list.removeNode(getNode(nodes, "a")); assertEquals(list.getHead()?.key, "b"); assertEquals(list.getTail()?.key, "b"); @@ -104,7 +110,7 @@ describe("LRUListManager", () => { it("should remove middle node", () => { const { list, nodes } = createListWithNodes("a", "b", "c"); - list.removeNode(nodes.b); + list.removeNode(getNode(nodes, "b")); assertEquals(list.getHead()?.key, "c"); assertEquals(list.getHead()?.next?.key, "a"); diff --git a/src/utils/cache/stores/memory/lru-list-manager.ts b/src/utils/cache/stores/memory/lru-list-manager.ts index 23aa8a13ca..22960a804e 100644 --- a/src/utils/cache/stores/memory/lru-list-manager.ts +++ b/src/utils/cache/stores/memory/lru-list-manager.ts @@ -4,6 +4,8 @@ export class LRUListManager { private head: LRUNode | null = null; private tail: LRUNode | null = null; + constructor(private readonly now: () => number = Date.now) {} + getHead(): LRUNode | null { return this.head; } @@ -13,9 +15,8 @@ export class LRUListManager { } moveToFront(node: LRUNode): void { - node.entry.lastAccessed = Date.now(); - if (node === this.head) { + node.entry.lastAccessed = this.now(); return; } @@ -34,7 +35,7 @@ export class LRUListManager { } this.head = node; - node.entry.lastAccessed = Date.now(); + node.entry.lastAccessed = this.now(); } removeNode(node: LRUNode): void { diff --git a/src/utils/cache/stores/memory/types.ts b/src/utils/cache/stores/memory/types.ts index 106f564e7a..7f9eb5df20 100644 --- a/src/utils/cache/stores/memory/types.ts +++ b/src/utils/cache/stores/memory/types.ts @@ -12,6 +12,8 @@ export interface LRUCacheOptions { ttlMs?: number; onEvict?: (key: string, value: unknown) => void; estimateSizeOf?: (value: unknown) => number; + /** Internal clock injection for deterministic expiry tests. */ + now?: () => number; } export interface LRUEntry { diff --git a/src/utils/file-discovery.test.ts b/src/utils/file-discovery.test.ts index d38a83b47c..60bda91e32 100644 --- a/src/utils/file-discovery.test.ts +++ b/src/utils/file-discovery.test.ts @@ -1,5 +1,7 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals, assertExists } from "#veryfront/testing/assert.ts"; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { assertEquals, assertExists, assertThrows } from "#veryfront/testing/assert.ts"; import { describe, it } from "#veryfront/testing/bdd.ts"; import { join } from "#veryfront/compat/path"; import { collectFiles, countFiles, discoverFiles, hasMatchingFiles } from "./file-discovery.ts"; @@ -7,7 +9,37 @@ import { cwd } from "../platform/compat/process.ts"; const TEST_DIR = join(cwd(), "src/utils"); +function withFixtureTree(build: (root: string) => void, run: (root: string) => Promise) { + const root = mkdtempSync(join(tmpdir(), "veryfront-file-discovery-")); + try { + build(root); + } catch (error) { + rmSync(root, { recursive: true, force: true }); + throw error; + } + return run(root).finally(() => rmSync(root, { recursive: true, force: true })); +} + describe("file-discovery", () => { + it("cleans up fixture trees when setup throws", () => { + let fixtureRoot = ""; + + assertThrows( + () => + withFixtureTree( + (root) => { + fixtureRoot = root; + throw new Error("setup failed"); + }, + async () => undefined, + ), + Error, + "setup failed", + ); + + assertEquals(existsSync(fixtureRoot), false); + }); + it("discovers files with extension filter", async () => { const files = await collectFiles({ baseDir: TEST_DIR, @@ -45,6 +77,33 @@ describe("file-discovery", () => { assertEquals(files.every((f) => f.name.includes("test")), true); }); + it("filters by glob pattern", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + patterns: ["file-*.test.ts"], + recursive: false, + }); + + assertEquals(files.some((f) => f.name === "file-discovery.test.ts"), true); + assertEquals( + files.every((f) => f.name.startsWith("file-") && f.name.endsWith(".test.ts")), + true, + ); + }); + + it("filters by single-character glob pattern", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + patterns: ["file-discover?.ts"], + recursive: false, + }); + + assertEquals(files.some((f) => f.name === "file-discovery.ts"), true); + assertEquals(files.some((f) => f.name === "file-discovery.test.ts"), false); + }); + it("respects maxDepth", async () => { const files = await collectFiles({ baseDir: join(cwd(), "src"), @@ -69,6 +128,161 @@ describe("file-discovery", () => { assertEquals(files.every((f) => !f.name.includes("test")), true); }); + it("ignores glob patterns", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + ignorePatterns: ["*.test.*"], + recursive: true, + }); + + assertExists(files); + assertEquals(files.length > 0, true); + assertEquals(files.some((f) => f.name === "file-discovery.ts"), true); + assertEquals(files.every((f) => !f.name.includes(".test.")), true); + }); + + it("ignores single-character glob patterns", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + ignorePatterns: ["file-discover?.ts"], + recursive: false, + }); + + assertExists(files); + assertEquals(files.some((f) => f.name === "file-discovery.ts"), false); + assertEquals(files.some((f) => f.name === "file-discovery.test.ts"), true); + }); + + it("treats a leading **/ include glob as any-depth entry matching", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + patterns: ["**/file-*.test.ts"], + recursive: false, + }); + + assertEquals(files.some((f) => f.name === "file-discovery.test.ts"), true); + assertEquals( + files.every((f) => f.name.startsWith("file-") && f.name.endsWith(".test.ts")), + true, + ); + }); + + it("treats a leading Windows **\\ include glob as any-depth entry matching", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + patterns: ["**\\file-*.test.ts"], + recursive: false, + }); + + assertEquals(files.some((f) => f.name === "file-discovery.test.ts"), true); + assertEquals( + files.every((f) => f.name.startsWith("file-") && f.name.endsWith(".test.ts")), + true, + ); + }); + + it("does not let an empty any-depth include pattern match every entry", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + patterns: ["**/"], + recursive: false, + }); + + assertEquals(files, []); + }); + + it("disables path-shaped include patterns instead of silently matching everything", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + patterns: ["utils/*.ts"], + recursive: false, + }); + + assertEquals(files.length, 0); + }); + + it("treats a leading **/ ignore glob as any-depth entry matching", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + ignorePatterns: ["**/*.test.*"], + recursive: true, + }); + + assertEquals(files.length > 0, true); + assertEquals(files.every((f) => !f.name.includes(".test.")), true); + }); + + it("does not let path-shaped ignore patterns hide files", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + ignorePatterns: ["utils/file-discovery.ts"], + recursive: false, + }); + + assertEquals(files.some((f) => f.name === "file-discovery.ts"), true); + }); + + it("does not let empty or Windows path-shaped ignores hide files", async () => { + const files = await collectFiles({ + baseDir: TEST_DIR, + extensions: [".ts"], + ignorePatterns: ["**/", "utils\\file-discovery.ts"], + recursive: false, + }); + + assertEquals(files.some((f) => f.name === "file-discovery.ts"), true); + }); + + it("does not prune directories with file-glob ignore patterns", async () => { + await withFixtureTree( + (root) => { + mkdirSync(join(root, "fixtures.test.data")); + writeFileSync(join(root, "fixtures.test.data", "inner.ts"), "export {};"); + writeFileSync(join(root, "keep.ts"), "export {};"); + writeFileSync(join(root, "skip.test.ts"), "export {};"); + }, + async (root) => { + const files = await collectFiles({ + baseDir: root, + extensions: [".ts"], + ignorePatterns: ["*.test.*"], + recursive: true, + }); + const names = files.map((f) => f.name).sort(); + + assertEquals(names, ["inner.ts", "keep.ts"]); + }, + ); + }); + + it("still prunes whole subtrees for literal directory-name ignores", async () => { + await withFixtureTree( + (root) => { + mkdirSync(join(root, "__ignored__")); + writeFileSync(join(root, "__ignored__", "nested.ts"), "export {};"); + writeFileSync(join(root, "keep.ts"), "export {};"); + }, + async (root) => { + const files = await collectFiles({ + baseDir: root, + extensions: [".ts"], + ignorePatterns: ["__ignored__"], + recursive: true, + }); + + assertEquals(files.map((f) => f.name), ["keep.ts"]); + }, + ); + }); + it("includes directories when requested", async () => { const results = await collectFiles({ baseDir: TEST_DIR, diff --git a/src/utils/file-discovery.ts b/src/utils/file-discovery.ts index ff14ea235c..75b694e4fa 100644 --- a/src/utils/file-discovery.ts +++ b/src/utils/file-discovery.ts @@ -9,6 +9,9 @@ import { join } from "#veryfront/compat/path/index.ts"; import type { RuntimeAdapter } from "#veryfront/platform/adapters/base.ts"; import { isBun, isDeno } from "#veryfront/platform/compat/runtime.ts"; import { withSpan } from "#veryfront/observability/tracing/otlp-setup.ts"; +import { serverLogger } from "./logger/index.ts"; + +const logger = serverLogger.component("file-discovery"); async function getDefaultAdapter(): Promise { if (isDeno) { @@ -52,12 +55,110 @@ function matchesExtensions(fileName: string, extensions: string[] | undefined): function matchesPatterns(fileName: string, patterns: string[] | undefined): boolean { if (!patterns?.length) return true; - return patterns.some((pattern) => fileName.includes(pattern)); + return patterns.some((pattern) => matchesEntryPattern(fileName, pattern)); } -function shouldIgnore(name: string, ignorePatterns: string[] | undefined): boolean { +/** + * Match a glob against one directory-entry name without compiling caller input + * as a regular expression. `*` matches zero or more characters and `?` + * matches exactly one character; every other character is literal. + */ +function matchesEntryGlob(name: string, pattern: string): boolean { + const nameTokens = [...name]; + const patternTokens = [...pattern]; + let nameIndex = 0; + let patternIndex = 0; + let lastStarIndex = -1; + let lastStarMatchIndex = -1; + + while (nameIndex < nameTokens.length) { + const token = patternTokens[patternIndex]; + if (token === "?" || token === nameTokens[nameIndex]) { + nameIndex++; + patternIndex++; + continue; + } + + if (token === "*") { + lastStarIndex = patternIndex++; + lastStarMatchIndex = nameIndex; + continue; + } + + if (lastStarIndex === -1) return false; + patternIndex = lastStarIndex + 1; + nameIndex = ++lastStarMatchIndex; + } + + while (patternTokens[patternIndex] === "*") patternIndex++; + return patternIndex === patternTokens.length; +} + +const warnedPathShapedPatterns = new Set(); + +// Normalize a caller pattern to entry-name form. Patterns are matched against +// one directory-entry name, so a leading "**/" (match at any depth) is +// redundant and stripped: "**/*.ts" means "*.ts at any depth", which is +// exactly what entry-name matching during the recursive walk provides. Any +// other path-shaped pattern (containing "/") can never match a bare entry +// name; returning undefined disables it, after warning once so the +// misconfiguration is visible instead of silently matching nothing. +function toEntryPattern(pattern: string): string | undefined { + let entryPattern = pattern; + while (entryPattern.startsWith("**/") || entryPattern.startsWith("**\\")) { + entryPattern = entryPattern.slice(3); + } + + if ( + entryPattern.length > 0 && + !entryPattern.includes("/") && + !entryPattern.includes("\\") + ) { + return entryPattern; + } + + if (!warnedPathShapedPatterns.has(pattern) && warnedPathShapedPatterns.size < 1000) { + warnedPathShapedPatterns.add(pattern); + logger.warn( + "File discovery patterns match single directory-entry names; an empty or path-shaped pattern is ignored", + { pattern }, + ); + } + return undefined; +} + +function isGlobPattern(pattern: string): boolean { + return pattern.includes("*") || pattern.includes("?"); +} + +function matchesNormalizedEntryPattern(name: string, entryPattern: string): boolean { + return isGlobPattern(entryPattern) + ? matchesEntryGlob(name, entryPattern) + : name.includes(entryPattern); +} + +function matchesEntryPattern(name: string, pattern: string): boolean { + const entryPattern = toEntryPattern(pattern); + if (entryPattern === undefined) return false; + return matchesNormalizedEntryPattern(name, entryPattern); +} + +function shouldIgnore( + name: string, + ignorePatterns: string[] | undefined, + isDirectory: boolean, +): boolean { if (!ignorePatterns?.length) return false; - return ignorePatterns.some((pattern) => name.includes(pattern)); + return ignorePatterns.some((pattern) => { + const entryPattern = toEntryPattern(pattern); + if (entryPattern === undefined) return false; + // Glob ignores (e.g. `*.test.*`) describe file names; matching them + // against directory names would silently prune entire subtrees (a + // directory named `fixtures.test.data` would vanish). Subtree pruning is + // reserved for directory-name patterns like `node_modules` or `.git`. + if (isDirectory && isGlobPattern(entryPattern)) return false; + return matchesNormalizedEntryPattern(name, entryPattern); + }); } function matchesFile( @@ -132,7 +233,7 @@ async function* walkDirectory(options: WalkDirectoryOptions): AsyncGenerator { assertEquals(cache.delete("exists"), false); }); + it("tracks membership for an undefined value", (): void => { + const cache = createCache({ maxEntries: 3 }); + + cache.set("present", undefined); + + assertEquals(cache.get("present"), undefined); + assertEquals(cache.has("present"), true); + assertEquals(cache.delete("present"), true); + assertEquals(cache.has("present"), false); + assertEquals(cache.delete("present"), false); + }); + it("clear and size", (): void => { const cache = createCache({ maxEntries: 3, ttlMs: 1000 }); diff --git a/src/utils/lru-wrapper.ts b/src/utils/lru-wrapper.ts index c969c5028d..48ffceb44e 100644 --- a/src/utils/lru-wrapper.ts +++ b/src/utils/lru-wrapper.ts @@ -72,7 +72,7 @@ export class LRUCache { } has(key: K): boolean { - return this.adapter.get(this.toStringKey(key)) !== undefined; + return this.adapter.has(this.toStringKey(key)); } get(key: K): V | undefined { @@ -85,7 +85,7 @@ export class LRUCache { delete(key: K): boolean { const stringKey = this.toStringKey(key); - const had = this.adapter.get(stringKey) !== undefined; + const had = this.adapter.has(stringKey); this.adapter.delete(stringKey); return had; }