fix: v1.40.0.0 migration done-marker leaves privacy-map half-patched on jq-less machines (#1581)

[stedfn]

Fix v1.40.0.0 migration done-marker (#1581)

Fixes #1581.

What this fixes

gstack-upgrade/migrations/v1.40.0.0.sh used to unconditionally touch its done-marker (~/.gstack/.migrations/v1.40.0.0.done) at the end of the script, even when the .brain-privacy-map.json patch was skipped because jq is missing from the user's PATH. On subsequent runs the script short-circuits at [ -f "${DONE}" ] && exit 0, so the privacy-map repair never lands unless the user spots the buried WARN, installs jq, manually removes the marker, and re-runs.

The silent failure path: .brain-allowlist and .gitattributes get patched on the first pass (they don't need jq), so /gstack-upgrade looks successful — but federation sync silently drops /plan-eng-review test plans because the privacy-map entry for projects/*/*-eng-review-test-plan-*.md was never added.

The fix

Defer touch "${DONE}" until every required repair either succeeded or was provably unnecessary. Track every failure mode via a single incomplete flag:

• jq missing when privacy-map exists
• malformed JSON (refuse to mutate corrupt JSON; leave for manual repair)
• jq mutation failure
• mktemp failure
• mv failure on the rewrite
• allowlist append failure (printf >> writing to a read-only file, etc.)
• gitattributes append failure

The marker is written only when incomplete=0, so the migration runner retries on the next /gstack-upgrade once the prerequisites are met. No manual marker-removal required.

Tests

8 cases in test/gstack-upgrade-migration-v1_40_0_0.test.ts:

#	Case	What it pins
1	happy path	jq present, fresh privacy-map → all three files patched, marker written
2	regression for #1581	jq missing, privacy-map present → marker NOT written
3	recovery	jq missing, then restored → patch lands on second run
4	idempotency	privacy-map already has correct entry → no mutation, marker written
5	fresh-init	privacy-map file absent → allowlist + gitattrs patched, marker written
6	malformed JSON	broken privacy-map → no marker, no mutation
7	jq mutation failure	fake jq returning 1 → no marker, tempfile cleaned up
8	allowlist append failure	read-only allowlist → no marker

Case 2 fails against the unmodified script (verified before applying the fix) and passes against the fix.

bun test test/gstack-upgrade-migration-v1_40_0_0.test.ts
# 8 pass, 0 fail, 33 expect() calls

Proposed remediation migration (for the next wave)

The in-place fix above helps fresh installs and users who haven't yet upgraded to v1.40.0.0. It does NOT help users who already ran the buggy version — their v1.40.0.0.done marker is set, and the migration runner's version check (gstack-upgrade/SKILL.md:206) won't re-invoke v1.40.0.0.sh for users whose installed version is already >= 1.40.0.0.

To remediate stuck users automatically, a new migration script needs to ship in the next wave. The script below is idempotent, well-tested (9 cases, all green locally), and ready to drop in at whatever version your next wave assigns. The filename and the DONE path inside need to match the wave version you pick (e.g. rename to v1.41.0.0.sh and update DONE="${MIGRATION_DIR}/v1.41.0.0.done").

I'm holding the remediation back from this PR because pre-claiming a migration version slot conflicts with how you batch community PRs into release waves. If you'd prefer I add it to this PR at a specific version, happy to push a follow-up commit — just tell me which slot you want.

Proposed gstack-upgrade/migrations/v1.40.0.1.sh (rename + update DONE name)

#!/usr/bin/env bash
# Migration: v1.40.0.1 — remediation for #1581.
#
# v1.40.0.0.sh used to unconditionally `touch` its done-marker even when jq
# was missing, leaving `.brain-privacy-map.json` half-patched. v1.40.0.0.sh
# was patched in a follow-up to defer the done-marker, but users who already
# ran the buggy version have a stale marker and the runner skips v1.40.0.0
# for them. This script re-patches the privacy-map idempotently and also
# repairs entries with wrong class metadata.
#
# Logic:
#   1. Short-circuit on this migration's own done-marker.
#   2. If jq is missing, log a WARN and exit 0 WITHOUT touching the marker —
#      we'll retry on the next /gstack-upgrade.
#   3. If the privacy-map file doesn't exist (fresh-init user), nothing to
#      repair → touch the marker and exit.
#   4. If the privacy-map is malformed JSON, log an ERROR and exit 0 WITHOUT
#      touching the marker. Corrupt JSON is a separate problem; we don't
#      mutate it.
#   5. If the eng-review entry is present AND has the expected class, touch
#      the marker and exit.
#   6. If the eng-review pattern exists but with the wrong class, repair the
#      entry in place (replace, don't duplicate).
#   7. Otherwise (pattern missing), append the canonical entry.
#   8. On any jq mutation failure, do NOT touch the marker — leave for next
#      run.

set -u

GSTACK_HOME="${HOME}/.gstack"
PRIVACY="${GSTACK_HOME}/.brain-privacy-map.json"

MIGRATION_DIR="${GSTACK_HOME}/.migrations"
DONE="${MIGRATION_DIR}/v1.40.0.1.done"   # ← rename to match your wave version

# Must match v1.40.0.0.sh exactly.
EXPECTED_PATTERN='projects/*/*-eng-review-test-plan-*.md'
EXPECTED_CLASS='artifact'

mkdir -p "${MIGRATION_DIR}" 2>/dev/null || true
if [ -f "${DONE}" ]; then
  exit 0
fi

if ! command -v jq >/dev/null 2>&1; then
  echo "  [v1.40.0.1] WARN: jq not found; skipping privacy-map remediation. Install jq and re-run gstack-upgrade." >&2
  exit 0
fi

if [ ! -f "${PRIVACY}" ]; then
  touch "${DONE}"
  exit 0
fi

if ! jq -e . "${PRIVACY}" >/dev/null 2>&1; then
  echo "  [v1.40.0.1] ERROR: ${PRIVACY} is malformed JSON; refusing to mutate. Please repair manually or run gstack-artifacts-init." >&2
  exit 0
fi

if jq -e --arg p "${EXPECTED_PATTERN}" --arg c "${EXPECTED_CLASS}" \
  '[.[] | select(.pattern == $p and .class == $c)] | length > 0' \
  "${PRIVACY}" >/dev/null 2>&1; then
  touch "${DONE}"
  exit 0
fi

pattern_present=0
if jq -e --arg p "${EXPECTED_PATTERN}" \
  '[.[] | select(.pattern == $p)] | length > 0' \
  "${PRIVACY}" >/dev/null 2>&1; then
  pattern_present=1
fi

tmp=$(mktemp "${PRIVACY}.tmp.XXXXXX" 2>/dev/null)
if [ -z "${tmp}" ] || [ ! -f "${tmp}" ]; then
  echo "  [v1.40.0.1] WARN: failed to create tempfile for ${PRIVACY}; will retry on next upgrade." >&2
  exit 0
fi

if [ "${pattern_present}" = "1" ]; then
  if ! jq --arg p "${EXPECTED_PATTERN}" --arg c "${EXPECTED_CLASS}" \
    'map(if .pattern == $p then .class = $c else . end)' \
    "${PRIVACY}" > "${tmp}" 2>/dev/null; then
    echo "  [v1.40.0.1] WARN: jq repair mutation failed for ${PRIVACY}; will retry on next upgrade." >&2
    rm -f "${tmp}"
    exit 0
  fi
else
  if ! jq --arg p "${EXPECTED_PATTERN}" --arg c "${EXPECTED_CLASS}" \
    '. += [{"pattern": $p, "class": $c}]' \
    "${PRIVACY}" > "${tmp}" 2>/dev/null; then
    echo "  [v1.40.0.1] WARN: jq append mutation failed for ${PRIVACY}; will retry on next upgrade." >&2
    rm -f "${tmp}"
    exit 0
  fi
fi

if ! mv "${tmp}" "${PRIVACY}" 2>/dev/null; then
  echo "  [v1.40.0.1] WARN: failed to rewrite ${PRIVACY}; will retry on next upgrade." >&2
  rm -f "${tmp}"
  exit 0
fi

touch "${DONE}"
echo "  [v1.40.0.1] privacy-map remediated for /plan-eng-review test plans (#1581)" >&2

exit 0

Proposed test/gstack-upgrade-migration-v1_40_0_1.test.ts (9 cases)

Available on request — full file is 200 lines. Cases:

1. jq missing → no marker.
2. privacy-map already has correct entry → no mutation, marker written.
3. privacy-map missing the entry → entry appended with correct class.
4. privacy-map has entry with WRONG class → repaired in place (no duplicate).
5. privacy-map has duplicate correct entries → preserved, no mutation.
6. malformed JSON → no marker, error logged.
7. privacy-map file doesn't exist → marker written (nothing to remediate).
8. re-run after success → short-circuits on done-marker, no-op.
9. stale v1.40.0.0.done + missing entry (the user-visible field flow) → entry added, new marker written.

Test plan

• Run bun test test/gstack-upgrade-migration-v1_40_0_0.test.ts — 8/8 pass
• Verify case 2 fails against unmodified v1.40.0.0.sh (regression test catches the bug)
• Verify full free test suite (bun test) still green

Out of scope

• VERSION bump and CHANGELOG entry — maintainer's call when the next wave ships.
• v1.38.1.0's same unconditional-touch pattern — latent (no jq-gated step), not user-impacting. Worth a separate refactor PR if you want a shared mark_done() helper across migrations.
• Repairing corrupt .brain-privacy-map.json content — out of scope; we log an error and bail rather than mutate corrupt JSON.