ci: report the correct pnpmDeps hash when flake.nix is stale (#1817)

* ci: report the correct pnpmDeps hash when flake.nix is stale Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: scope the reported hash to the pnpmDeps block Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(flake): scope every hash rewrite to the pnpmDeps block alfred-openspec on #1817: the workflow read is scoped now, but the script it runs is not. update-flake.sh read CURRENT_HASH from the first hash assignment anywhere in flake.nix, and all three in-place rewrites matched every hash assignment. flake.nix holds one fixed-output derivation today, so that lands on the right line by luck; add a second and the script stamps the placeholder over both, reads back whichever mismatch Nix reported first, and writes pnpmDeps' hash into the other derivation. Scoping only the workflow left that path fragile, as the review says. The address range is declared once as PNPM_DEPS_BLOCK and used by the read and all three rewrites, so the scoping cannot drift between call sites. Also guards the read: an unmatched block previously left CURRENT_HASH empty, and the failure path would then restore hash = "". It now exits before touching the file. Verified against a three-derivation fixture with pnpmDeps in the middle, which catches both shapes of the bug: the scoped read returns the pnpmDeps hash while an unscoped read returns the first derivation's, the placeholder is written once rather than three times, and the neighbouring hashes survive the restore. That fixture is the new test, alongside a static check that no hash read or rewrite in the script is missing the range. Verified the static check fails when any one call site is unscoped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(flake): run the scoping fixture on its own volume The new test failed on windows-pwsh with 'sed: cannot rename ./sedKaAflu: Invalid cross-device link'. sed -i writes its temp file in the working directory and renames it over the target; on a GitHub Windows runner the repo is on D: and os.tmpdir() is on C:, so that rename crosses volumes. bash now runs with cwd set to the fixture directory and addresses the file by name, which keeps the temp file and its rename on one volume. The assertions are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-02 05:24:34 +08:00 · 2026-09-09 16:24:16 +00:00
parent e062b9572b
commit 63666c8bb2
3 changed files with 159 additions and 25 deletions
@@ -181,6 +181,31 @@ jobs:
      - name: Setup Nix cache
        uses: DeterminateSystems/magic-nix-cache-action@908b263ff629f4cc17666315b7fd3ec127c6244d # v14

+      # Run the update script before `nix build`, not after. The script recomputes
+      # the pnpmDeps hash from pnpm-lock.yaml and rewrites flake.nix in place, so a
+      # stale hash is reported here as the exact value to paste. Built first, the
+      # same staleness surfaces as pnpm's ERR_PNPM_NO_OFFLINE_TARBALL — which names
+      # a missing tarball, not the hash — and the script never runs to say otherwise.
+      # Every root lockfile change needs this value, and Dependabot cannot produce it.
+      - name: Verify pnpmDeps hash matches the lockfile
+        run: |
+          bash scripts/update-flake.sh
+          if git diff --quiet flake.nix; then
+            echo "✅ flake.nix pnpmDeps hash is up to date"
+            exit 0
+          fi
+          # Scoped to the pnpmDeps block: a bare first-match would report some other
+          # FOD's hash if one is ever added above it.
+          HASH=$(sed -n '/pnpmDeps = /,/};/p' flake.nix \
+            | sed -nE 's/.*hash = "(sha256-[^"]+)".*/\1/p' | head -1)
+          git diff flake.nix
+          echo "::error file=flake.nix::Stale pnpmDeps hash. Set pnpmDeps.hash to $HASH and push."
+          exit 1
+
+      - name: Restore flake.nix
+        if: always()
+        run: git checkout -- flake.nix || true
+
      - name: Build with Nix
        run: nix build

@@ -206,25 +231,6 @@ jobs:
          fi
          echo "✅ Binary execution successful"

-      - name: Validate update script
-        run: |
-          echo "Testing update-flake.sh script..."
-          bash scripts/update-flake.sh
-          echo "✅ Update script executed successfully"
-
-      - name: Check flake.nix modifications
-        run: |
-          if git diff --quiet flake.nix; then
-            echo "ℹ️  flake.nix unchanged (hash already up-to-date)"
-          else
-            echo "✅ flake.nix was updated by script"
-            git diff flake.nix
-          fi
-
-      - name: Restore flake.nix
-        if: always()
-        run: git checkout -- flake.nix || true
-
  validate-changesets:
    name: Validate Release Tracking
    runs-on: ubuntu-latest
@@ -10,6 +10,14 @@ PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
 FLAKE_FILE="$PROJECT_ROOT/flake.nix"
 PACKAGE_JSON="$PROJECT_ROOT/package.json"

+# Every hash read and every hash rewrite below is confined to this sed address
+# range. flake.nix holds one fixed-output derivation today, so an unscoped
+# `hash = "sha256-..."` happens to hit the right line; the moment a second FOD
+# is added, an unscoped script would stamp the placeholder over both, extract
+# whichever mismatch Nix reported first, and write pnpmDeps' hash into the
+# other derivation. Scoping is what keeps that from being a silent corruption.
+PNPM_DEPS_BLOCK='/pnpmDeps = /,/};/'
+
 # Colors for output
 RED='\033[0;31m'
 GREEN='\033[0;32m'
@@ -49,15 +57,21 @@ fi
 echo -e "${BLUE}🔧 Current pnpm-lock.yaml:${NC} $(stat -c%y "$PROJECT_ROOT/pnpm-lock.yaml" 2>/dev/null || stat -f%Sm "$PROJECT_ROOT/pnpm-lock.yaml")"
 echo ""

-# Get current hash from flake.nix
-CURRENT_HASH=$(sed -nE 's/.*hash = "(sha256-[^"]+)".*/\1/p' "$FLAKE_FILE" | head -1)
+# Get current pnpmDeps hash from flake.nix
+CURRENT_HASH=$(sed -nE "$PNPM_DEPS_BLOCK"' s/.*hash = "(sha256-[^"]+)".*/\1/p' "$FLAKE_FILE" | head -1)
+if [ -z "$CURRENT_HASH" ]; then
+  echo -e "${RED}❌ Error: no pnpmDeps hash found in flake.nix${NC}"
+  echo -e "   Looked for 'hash = \"sha256-...\"' inside the 'pnpmDeps = ... };' block."
+  echo -e "   Nothing was modified."
+  exit 1
+fi
 echo -e "${BLUE}📌 Current hash:${NC} $CURRENT_HASH"
 echo ""

 # Set placeholder hash to trigger error
 echo -e "${YELLOW}⏳ Setting placeholder hash to calculate correct value...${NC}"
 PLACEHOLDER="sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="
-sed "${SED_INPLACE[@]}" "s|hash = \"sha256-[^\"]*\"|hash = \"$PLACEHOLDER\"|" "$FLAKE_FILE"
+sed "${SED_INPLACE[@]}" "$PNPM_DEPS_BLOCK s|hash = \"sha256-[^\"]*\"|hash = \"$PLACEHOLDER\"|" "$FLAKE_FILE"

 # Try to build and capture the correct hash
 echo -e "${BLUE}🔨 Building to determine correct hash (expected to fail)...${NC}"
@@ -77,7 +91,7 @@ if [ -z "$CORRECT_HASH" ]; then
  echo "$BUILD_OUTPUT"
  echo ""
  echo -e "${YELLOW}Restoring original hash...${NC}"
-  sed "${SED_INPLACE[@]}" "s|hash = \"$PLACEHOLDER\"|hash = \"$CURRENT_HASH\"|" "$FLAKE_FILE"
+  sed "${SED_INPLACE[@]}" "$PNPM_DEPS_BLOCK s|hash = \"$PLACEHOLDER\"|hash = \"$CURRENT_HASH\"|" "$FLAKE_FILE"
  exit 1
 fi

@@ -87,14 +101,14 @@ echo ""
 # Check if hash changed
 if [ "$CURRENT_HASH" = "$CORRECT_HASH" ]; then
  echo -e "${GREEN}✓ Hash is already up-to-date!${NC}"
-  sed "${SED_INPLACE[@]}" "s|hash = \"$PLACEHOLDER\"|hash = \"$CORRECT_HASH\"|" "$FLAKE_FILE"
+  sed "${SED_INPLACE[@]}" "$PNPM_DEPS_BLOCK s|hash = \"$PLACEHOLDER\"|hash = \"$CORRECT_HASH\"|" "$FLAKE_FILE"
  echo ""
  echo -e "${BLUE}ℹ️  No changes needed. Your flake is in sync with pnpm-lock.yaml${NC}"
  exit 0
 fi

 echo -e "${YELLOW}🔄 Updating hash in flake.nix...${NC}"
-sed "${SED_INPLACE[@]}" "s|hash = \"$PLACEHOLDER\"|hash = \"$CORRECT_HASH\"|" "$FLAKE_FILE"
+sed "${SED_INPLACE[@]}" "$PNPM_DEPS_BLOCK s|hash = \"$PLACEHOLDER\"|hash = \"$CORRECT_HASH\"|" "$FLAKE_FILE"

 # Verify the build works
 echo -e "${BLUE}🔍 Verifying build with new hash...${NC}"
@@ -0,0 +1,114 @@
+import { execFileSync } from 'child_process';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { describe, expect, it } from 'vitest';
+
+const projectRoot = process.cwd();
+const scriptPath = path.join(projectRoot, 'scripts', 'update-flake.sh');
+const script = fs.readFileSync(scriptPath, 'utf8');
+
+/**
+ * `scripts/update-flake.sh` rewrites the pnpmDeps hash in flake.nix in place.
+ *
+ * flake.nix holds exactly one fixed-output derivation today, so an unscoped
+ * `hash = "sha256-..."` happens to land on the right line and the bug is
+ * invisible. Add a second FOD and an unscoped script stamps the placeholder
+ * over both, reads back whichever mismatch Nix reported first, and writes
+ * pnpmDeps' hash into the other derivation. That is a silent corruption of a
+ * supply-chain pin, so the scoping is pinned here rather than left to review.
+ */
+describe('update-flake.sh confines every hash rewrite to the pnpmDeps block', () => {
+  const BLOCK = "PNPM_DEPS_BLOCK='/pnpmDeps = /,/};/'";
+
+  it('declares the block address once, so the scoping cannot drift per call site', () => {
+    expect(script).toContain(BLOCK);
+  });
+
+  it('scopes every line that reads or rewrites a hash', () => {
+    const unscoped = script
+      .split('\n')
+      .map((line, index) => [index + 1, line.trim()] as const)
+      .filter(([, line]) => !line.startsWith('#'))
+      // Every line that extracts a hash or edits one in place.
+      .filter(([, line]) => /CURRENT_HASH=\$\(sed|sed "\$\{SED_INPLACE\[@\]\}"/.test(line))
+      .filter(([, line]) => !line.includes('PNPM_DEPS_BLOCK'));
+
+    expect(unscoped).toEqual([]);
+  });
+
+  // The static checks above say the range is spelled everywhere; this one says
+  // the range actually selects the right derivation. Runs the script's own
+  // three sed operations against a flake with three FODs, pnpmDeps in the
+  // middle, so a first-match bug and a global-replace bug both show up.
+  it('touches only the pnpmDeps hash in a flake with several derivations', () => {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-flake-scope-'));
+    const flake = path.join(dir, 'flake.nix');
+    const other = 'sha256-OTHEROTHEROTHEROTHEROTHEROTHEROTHEROTHEROT0=';
+    const pnpm = 'sha256-PNPMPNPMPNPMPNPMPNPMPNPMPNPMPNPMPNPMPNPMPN0=';
+    const another = 'sha256-ANOTHERANOTHERANOTHERANOTHERANOTHERANOTHE0=';
+    const fresh = 'sha256-NEWNEWNEWNEWNEWNEWNEWNEWNEWNEWNEWNEWNEWNE0=';
+
+    fs.writeFileSync(
+      flake,
+      [
+        '{',
+        '  other = pkgs.fetchFromGitHub {',
+        `    hash = "${other}";`,
+        '  };',
+        '  pnpmDeps = pkgs.fetchPnpmDeps {',
+        `    hash = "${pnpm}";`,
+        '  };',
+        '  another = pkgs.fetchurl {',
+        `    hash = "${another}";`,
+        '  };',
+        '}',
+        '',
+      ].join('\n')
+    );
+
+    // Mirrors the script: read the current hash, stamp the placeholder, write
+    // the calculated hash back.
+    // `bash` runs inside the fixture directory and addresses the file by name:
+    // `sed -i` writes its temp file in the working directory and renames it
+    // into place, which fails with "Invalid cross-device link" on Windows when
+    // the repo (D:) and os.tmpdir() (C:) are different volumes.
+    const inFixture = (command: string): string =>
+      execFileSync('bash', ['-c', `${BLOCK}\n${command}`, '_', 'flake.nix'], {
+        cwd: dir,
+        encoding: 'utf8',
+      });
+
+    const read = inFixture(
+      `sed -nE "$PNPM_DEPS_BLOCK"' s/.*hash = "(sha256-[^"]+)".*/\\1/p' "$1" | head -1`
+    ).trim();
+
+    // The whole point: an unscoped read returns the first derivation's hash.
+    expect(read).toBe(pnpm);
+    expect(read).not.toBe(other);
+
+    const placeholder = 'sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=';
+    inFixture(
+      `sed -i.bak "$PNPM_DEPS_BLOCK s|hash = \\"sha256-[^\\"]*\\"|hash = \\"${placeholder}\\"|" "$1"`
+    );
+    expect(fs.readFileSync(flake, 'utf8').split(placeholder).length - 1).toBe(1);
+
+    inFixture(
+      `sed -i.bak "$PNPM_DEPS_BLOCK s|hash = \\"${placeholder}\\"|hash = \\"${fresh}\\"|" "$1"`
+    );
+
+    const updated = fs.readFileSync(flake, 'utf8');
+    expect(updated).toContain(`hash = "${fresh}"`);
+    // The neighbours are untouched, which is what a global replace would break.
+    expect(updated).toContain(`hash = "${other}"`);
+    expect(updated).toContain(`hash = "${another}"`);
+    expect(updated).not.toContain(placeholder);
+
+    fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  it('refuses to touch the file when no pnpmDeps hash is found', () => {
+    expect(script).toContain('no pnpmDeps hash found in flake.nix');
+    expect(script).toContain('Nothing was modified.');
+  });
+});