diff --git a/gstack-upgrade/migrations/v1.40.0.0.sh b/gstack-upgrade/migrations/v1.40.0.0.sh index d21c18ba3a..400c1bd3c9 100755 --- a/gstack-upgrade/migrations/v1.40.0.0.sh +++ b/gstack-upgrade/migrations/v1.40.0.0.sh @@ -34,6 +34,13 @@ NEW_PATTERNS=( ) added_any=0 +# Set to 1 if a required step had to be skipped (e.g. jq missing for the +# privacy-map patch). We do NOT write the done-marker in that case — so the +# next /gstack-upgrade run will retry against an environment with jq +# installed. See #1581: previously the done-marker was written +# unconditionally, which silently dropped the privacy-map repair on boxes +# without jq and federation sync kept missing eng-review test plans. +skipped_required=0 # ----- .brain-allowlist --------------------------------------------------- if [ -f "${ALLOWLIST}" ]; then @@ -62,12 +69,23 @@ if [ -f "${PRIVACY}" ]; then added_any=1 else rm -f "${PRIVACY}.tmp" + skipped_required=1 echo " [v1.40.0.0] WARN: jq failed to patch ${PRIVACY}; skipping pattern ${PATTERN}." >&2 fi fi done else - echo " [v1.40.0.0] WARN: jq not found; skipping privacy-map repair. Install jq and re-run gstack-upgrade, or run gstack-artifacts-init manually." >&2 + skipped_required=1 + echo "" >&2 + echo " [v1.40.0.0] *** ACTION REQUIRED ***" >&2 + echo " [v1.40.0.0] jq not found; cannot patch .brain-privacy-map.json." >&2 + echo " [v1.40.0.0] Federation sync will keep dropping /plan-eng-review test plans" >&2 + echo " [v1.40.0.0] until this runs. Install jq and re-run /gstack-upgrade:" >&2 + echo " [v1.40.0.0] - macOS: brew install jq" >&2 + echo " [v1.40.0.0] - Debian/Ubuntu: sudo apt install jq" >&2 + echo " [v1.40.0.0] - Fedora: sudo dnf install jq" >&2 + echo " [v1.40.0.0] (Migration done-marker NOT written — next /gstack-upgrade retries.)" >&2 + echo "" >&2 fi fi @@ -82,10 +100,18 @@ if [ -f "${GITATTRS}" ]; then done fi -# Mark done even if no patches needed — a fresh-init user's +# Mark done only when nothing was skipped. A fresh-init user's # bin/gstack-artifacts-init now writes the pattern directly, so re-runs -# should no-op. The touchfile keeps the migration runner from looping. -touch "${DONE}" +# should no-op and the marker gets written immediately. The touchfile keeps +# the migration runner from looping on healthy installs. +# +# When skipped_required=1 (e.g. jq missing) we deliberately leave the +# done-marker unwritten so the next /gstack-upgrade run retries the +# privacy-map patch. The other files were patched on this pass and their +# "already present" gates make re-running idempotent. See #1581. +if [ "${skipped_required}" = "0" ]; then + touch "${DONE}" +fi if [ "${added_any}" = "1" ]; then echo " [v1.40.0.0] allowlist/privacy-map/gitattributes patched for /plan-eng-review test plans (idempotent)" >&2 diff --git a/test/artifacts-init-migration.test.ts b/test/artifacts-init-migration.test.ts index c09affffdf..db9024e600 100644 --- a/test/artifacts-init-migration.test.ts +++ b/test/artifacts-init-migration.test.ts @@ -3,10 +3,28 @@ // .brain-privacy-map.json, and .gitattributes. import { describe, expect, test, beforeEach } from 'bun:test'; -import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync } from 'fs'; +import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync, mkdirSync, symlinkSync } from 'fs'; import { tmpdir } from 'os'; import { join } from 'path'; +// Build a fake bin directory that holds symlinks to every coreutil the +// migration script invokes — but deliberately omits `jq`. Used by the +// #1581 regression tests to simulate a host that hasn't installed jq. +function makeNoJqBin(): string { + const dir = mkdtempSync(join(tmpdir(), 'mig-nojq-bin-')); + // Match the actual /usr/bin paths the migration script + bash need. + const utils = ['bash', 'sh', 'grep', 'sed', 'printf', 'touch', 'cat', + 'rm', 'mv', 'mkdir', 'ls', 'dirname', 'basename', 'echo', 'env', + 'command', 'date', 'awk', 'tr', 'find', 'tee', 'sort', 'head', 'tail']; + for (const util of utils) { + const src = `/usr/bin/${util}`; + if (existsSync(src)) { + try { symlinkSync(src, join(dir, util)); } catch { /* already there */ } + } + } + return dir; +} + const REPO_ROOT = new URL('..', import.meta.url).pathname; const MIGRATION = join(REPO_ROOT, 'gstack-upgrade', 'migrations', 'v1.38.1.0.sh'); @@ -330,4 +348,90 @@ describe('v1.40.0.0 migration', () => { rmSync(home, { recursive: true, force: true }); } }); + + // Regression for #1581: when jq is missing on the host, the privacy-map + // block is skipped. Previously the done-marker was written anyway, so the + // next /gstack-upgrade no-op'd and the privacy-map never landed. The + // marker must now stay unwritten so the next run retries. + test('#1581: jq missing leaves done-marker unwritten and patches the rest', () => { + const home = setupFakeHome(); + const fakeBinDir = makeNoJqBin(); + try { + writeFileSync(join(home, '.gstack', '.brain-allowlist'), 'existing-line\n'); + writeFileSync(join(home, '.gstack', '.brain-privacy-map.json'), JSON.stringify([ + { pattern: 'projects/*/*-design-*.md', class: 'artifact' }, + ], null, 2)); + writeFileSync(join(home, '.gstack', '.gitattributes'), 'projects/*/*-design-*.md merge=union\n'); + + // PATH points only at the no-jq bin dir (coreutils symlinks, no jq). + // Mirrors a Debian box that hasn't apt-installed jq yet. + const proc = Bun.spawnSync({ + cmd: ['/bin/bash', MIGRATION_V1_40], + env: { HOME: home, PATH: fakeBinDir }, + stdout: 'pipe', + stderr: 'pipe', + }); + const r = { + code: proc.exitCode ?? -1, + stderr: new TextDecoder().decode(proc.stderr), + }; + expect(r.code).toBe(0); + + // Allowlist and gitattributes were patchable without jq — both updated. + expect(readFileSync(join(home, '.gstack', '.brain-allowlist'), 'utf-8')) + .toContain('projects/*/*-eng-review-test-plan-*.md'); + expect(readFileSync(join(home, '.gstack', '.gitattributes'), 'utf-8')) + .toContain('projects/*/*-eng-review-test-plan-*.md merge=union'); + + // Privacy-map untouched (no jq) — the eng-review entry didn't land. + const privacy = JSON.parse(readFileSync(join(home, '.gstack', '.brain-privacy-map.json'), 'utf-8')); + expect(privacy.find((e: any) => e.pattern === 'projects/*/*-eng-review-test-plan-*.md')).toBeUndefined(); + + // Crucially: done-marker NOT written, so /gstack-upgrade retries on + // the next run once jq is installed. + expect(existsSync(join(home, '.gstack', '.migrations', 'v1.40.0.0.done'))).toBe(false); + + // User gets an actionable WARN block, not a single buried line. + expect(r.stderr).toContain('ACTION REQUIRED'); + expect(r.stderr).toContain('Install jq'); + expect(r.stderr).toContain('done-marker NOT written'); + } finally { + rmSync(home, { recursive: true, force: true }); + rmSync(fakeBinDir, { recursive: true, force: true }); + } + }); + + // Companion to the regression above — re-running after installing jq + // completes the migration: privacy-map patched, done-marker now written. + test('#1581: retry after jq install completes the migration', () => { + const home = setupFakeHome(); + const fakeBinDir = makeNoJqBin(); + try { + writeFileSync(join(home, '.gstack', '.brain-allowlist'), 'existing\n'); + writeFileSync(join(home, '.gstack', '.brain-privacy-map.json'), JSON.stringify([ + { pattern: 'projects/*/*-design-*.md', class: 'artifact' }, + ], null, 2)); + writeFileSync(join(home, '.gstack', '.gitattributes'), 'placeholder\n'); + + // Run 1: simulate jq missing. + Bun.spawnSync({ + cmd: ['/bin/bash', MIGRATION_V1_40], + env: { HOME: home, PATH: fakeBinDir }, + stdout: 'pipe', + stderr: 'pipe', + }); + expect(existsSync(join(home, '.gstack', '.migrations', 'v1.40.0.0.done'))).toBe(false); + + // Run 2: jq now available (real PATH). Should finish the job. + const r2 = runMigrationV140(home); + expect(r2.code).toBe(0); + expect(existsSync(join(home, '.gstack', '.migrations', 'v1.40.0.0.done'))).toBe(true); + + const privacy = JSON.parse(readFileSync(join(home, '.gstack', '.brain-privacy-map.json'), 'utf-8')); + expect(privacy.find((e: any) => e.pattern === 'projects/*/*-eng-review-test-plan-*.md')?.class).toBe('artifact'); + } finally { + rmSync(home, { recursive: true, force: true }); + rmSync(fakeBinDir, { recursive: true, force: true }); + } + }); });