Address copilot review feedback

.github/workflows/path-validation-e2e.yml

@@ -24,10 +24,15 @@ permissions:
 #        - warn:  the malicious entry is extracted but a workflow warning is logged.
 #        - error: the malicious entry is rejected (no extraction).
 #
-# NOTE: The poisoned-cache phase requires a small Node.js helper script
-# (__tests__/e2e/generate-poisoned-archive.mjs) that the test workflow invokes.
-# We build the archive locally and upload it via the action under a strict-paths
-# label so the cache key namespacing remains consistent.
+# NOTE: The poisoned-cache phase relies on a small Node.js helper script
+# (__tests__/e2e/save-poisoned-cache.mjs) that the workflow invokes. Rather
+# than fabricating a tar archive by hand, the helper calls the toolkit's
+# `@actions/cache.saveCache()` with the declared `path` AND one or more extra
+# paths that escape it; the toolkit packs everything into a normal cache
+# archive. The action's later restore step declares only the legitimate
+# `path`, so the extra entries become "escape" entries that the client-side
+# validation should reject (or warn about) per the configured strict-paths
+# mode.
 
 jobs:
   good-cache:

__tests__/actionUtils.test.ts

@@ -1,7 +1,7 @@
 import * as cache from "@actions/cache";
 import * as core from "@actions/core";
 
-import { Events, RefKey } from "../src/constants";
+import { Events, Inputs, RefKey } from "../src/constants";
 import * as actionUtils from "../src/utils/actionUtils";
 import * as testUtils from "../src/utils/testUtils";
 
@@ -267,11 +267,11 @@ test("isGhes returns true when the GITHUB_SERVER_URL environment variable is set
 });
 
 describe("getPathValidationInput", () => {
-    const inputEnv = "INPUT_STRICT-PATHS";
+    const inputEnv = `INPUT_${Inputs.StrictPaths.toUpperCase()}`;
 
     beforeEach(() => {
         delete process.env[inputEnv];
-        // Re-mock getInput so the each-test environment reads INPUT_STRICT-PATHS
+        // Re-mock getInput so the each-test environment reads the input env var
         jest.spyOn(core, "getInput").mockImplementation((name, options) => {
             return jest.requireActual("@actions/core").getInput(name, options);
         });
@@ -297,24 +297,21 @@ describe("getPathValidationInput", () => {
         expect(actionUtils.getPathValidationInput()).toBe(expected);
     });
 
-    test("falls back to 'warn' for unrecognized values and logs a warning", () => {
+    test("falls back to 'warn' for unrecognized values and emits a workflow warning", () => {
         process.env[inputEnv] = "strict";
-        // getPathValidationInput() calls the same module's logWarning() via a
-        // local function reference (TypeScript compiles intra-module calls as
-        // direct references, not exports.X lookups), so a jest.spyOn on
-        // actionUtils.logWarning cannot intercept it. Spy on core.info — the
-        // only externally observable side effect — and suppress the real
-        // implementation so the warning does not pollute the Jest log.
-        const infoSpy = jest
-            .spyOn(core, "info")
+        // Suppress the real implementation so the warning does not pollute
+        // the Jest log, and assert it was emitted via core.warning so it
+        // surfaces as a real `::warning::` workflow annotation.
+        const warningSpy = jest
+            .spyOn(core, "warning")
             .mockImplementation(() => undefined);
         try {
             expect(actionUtils.getPathValidationInput()).toBe("warn");
-            expect(infoSpy).toHaveBeenCalledWith(
+            expect(warningSpy).toHaveBeenCalledWith(
                 expect.stringContaining("Unrecognized value for strict-paths")
             );
         } finally {
-            infoSpy.mockRestore();
+            warningSpy.mockRestore();
         }
     });
 

__tests__/restoreImpl.test.ts

@@ -546,12 +546,12 @@ test("restore falls back to 'warn' when strict-paths input is unrecognized", asy
     const restoreCacheMock = jest
         .spyOn(cache, "restoreCache")
         .mockResolvedValueOnce(key);
-    // getPathValidationInput()'s call to logWarning() is intra-module so a
-    // spy on actionUtils.logWarning would not intercept it. Spy on core.info
-    // (the underlying transport for logWarning) and suppress the real
-    // implementation so the warning does not print into the Jest log.
-    const infoMock = jest
-        .spyOn(core, "info")
+    // getPathValidationInput() emits the misconfiguration notice via
+    // core.warning() so it surfaces as a real `::warning::` workflow
+    // annotation. Suppress the real implementation to keep the Jest log
+    // clean while asserting it was called.
+    const warningMock = jest
+        .spyOn(core, "warning")
         .mockImplementation(() => undefined);
 
     await restoreImpl(new StateProvider());
@@ -563,7 +563,7 @@ test("restore falls back to 'warn' when strict-paths input is unrecognized", asy
         { lookupOnly: false, pathValidation: "warn" },
         false
     );
-    expect(infoMock).toHaveBeenCalledWith(
+    expect(warningMock).toHaveBeenCalledWith(
         expect.stringContaining("Unrecognized value for strict-paths")
     );
 });

docs/path-validation-test-plan.md

@@ -49,7 +49,7 @@ errors per the `fail-on-cache-invalid` input.
 | defaults strict-paths to `'warn'` and forwards it to `restoreCache`                              | Default option object contains `pathValidation: 'warn'`                                                                          |
 | `test.each(['off', 'warn', 'error'])` forwards each value to `restoreCache`                      | All three valid values reach the toolkit unchanged                                                                               |
 | falls back to `'warn'` when strict-paths input is unrecognized                                   | Unknown values are coerced to `'warn'` and a warning is logged                                                                   |
-| treats `CacheIntegrityError` as a cache miss by default                                          | When the toolkit throws `CacheIntegrityError` and `fail-on-cache-invalid: false`, action sets `cache-hit=false` and continues    |
+| treats `CacheIntegrityError` as a cache miss by default                                          | When the toolkit throws `CacheIntegrityError` and `fail-on-cache-invalid: false`, action logs the rejection and returns without setting the `cache-hit` output (intentionally unset to match regular cache-miss semantics — see issue #1466) |
 | fails when `CacheIntegrityError` is raised and `fail-on-cache-invalid: true`                     | When `fail-on-cache-invalid: true`, `core.setFailed()` is called with a message containing `integrity validation` and the code   |
 | propagates non-integrity errors normally                                                         | Network/auth errors still surface via `core.setFailed()` rather than being mis-classified as integrity failures                  |
 | `PARSE_ERROR` integrity failure also treated as miss by default                                  | Validation handles both `PATH_VIOLATION` and `PARSE_ERROR` codes identically                                                     |

src/restoreImpl.ts

@@ -64,11 +64,16 @@ export async function restoreImpl(
             if (err instanceof Error && err.name === "CacheIntegrityError") {
                 const code = (err as Error & { code?: string }).code;
                 if (failOnCacheInvalid) {
-                    throw new Error(
+                    // Preserve the toolkit's original error via `Error.cause`.
+                    // (Assigned after construction because this project's
+                    // tsconfig targets ES6.)
+                    const failure = new Error(
                         `Restored cache failed integrity validation (${
                             code ?? "unknown"
                         }): ${err.message}`
                     );
+                    (failure as Error & { cause?: unknown }).cause = err;
+                    throw failure;
                 }
                 // Treat as a cache miss. Intentionally do NOT set the
                 // `cache-hit` output here, to preserve the same downstream

src/utils/actionUtils.ts

@@ -1,7 +1,7 @@
 import * as cache from "@actions/cache";
 import * as core from "@actions/core";
 
-import { RefKey } from "../constants";
+import { Inputs, RefKey } from "../constants";
 
 export function isGhes(): boolean {
     const ghUrl = new URL(
@@ -70,15 +70,19 @@ export function getInputAsBool(
  * Read the `strict-paths` input and coerce it to a value the `@actions/cache`
  * toolkit understands. Unknown values default to `'warn'` (a best-effort
  * recovery — we don't want a typo to silently disable client-side validation)
- * and a warning is emitted so the user notices.
+ * and a workflow warning annotation is emitted so the user notices.
+ *
+ * Uses `core.warning()` directly (rather than this module's `logWarning()`
+ * helper, which routes through `core.info()`) so an input misconfiguration
+ * surfaces as a real `::warning::` annotation in the run summary.
  */
 export function getPathValidationInput(): "off" | "warn" | "error" {
-    const raw = core.getInput("strict-paths") || "warn";
+    const raw = core.getInput(Inputs.StrictPaths) || "warn";
     const normalized = raw.toLowerCase();
     if (normalized === "off" || normalized === "warn" || normalized === "error") {
         return normalized;
     }
-    logWarning(
+    core.warning(
         `Unrecognized value for strict-paths: "${raw}". Falling back to "warn". Valid values are: off, warn, error.`
     );
     return "warn";