Skip to content

fix(migrations): defer v1.40.0.0 done-marker when jq is missing (#1581)#1600

Open
genisis0x wants to merge 1 commit into
garrytan:mainfrom
genisis0x:fix/1581-migration-jq-missing
Open

fix(migrations): defer v1.40.0.0 done-marker when jq is missing (#1581)#1600
genisis0x wants to merge 1 commit into
garrytan:mainfrom
genisis0x:fix/1581-migration-jq-missing

Conversation

@genisis0x
Copy link
Copy Markdown
Contributor

Closes #1581.

Summary

gstack-upgrade/migrations/v1.40.0.0.sh writes ~/.gstack/.migrations/v1.40.0.0.done unconditionally. When the host doesn't have jq installed, the privacy-map patch logs a single WARN: line and gets skipped — but the done-marker is still written, so subsequent /gstack-upgrade runs short-circuit at [ -f "${DONE}" ] && exit 0 and the privacy-map repair never lands. Federation sync keeps silently dropping /plan-eng-review test plans.

Fix

Track a skipped_required flag. Set to 1 when:

  • command -v jq returns false (jq not installed), or
  • jq is present but the patch invocation fails.

Skip touch ${DONE} when skipped_required=1 so the next /gstack-upgrade retries against an env with jq installed. The allowlist + gitattributes patches still run on the same pass (they don't need jq), and their grep -Fq 'already present' gates keep re-runs idempotent.

Also replace the buried one-line WARN with an action-required block listing the install commands for macOS / Debian / Fedora and an explicit note that the done-marker was withheld on purpose.

Tests

Two new regressions in test/artifacts-init-migration.test.ts (file as a whole: 14/14 green):

  • #1581: jq missing leaves done-marker unwritten and patches the rest — clamps PATH to a fake bin dir with coreutils symlinks but no jq, asserts allowlist + gitattributes were patched, privacy-map untouched, done-marker absent, ACTION REQUIRED block present in stderr.
  • #1581: retry after jq install completes the migration — first run with no-jq PATH (no marker), second run with real PATH lands the privacy-map entry and writes the marker.

Existing tests (allowlist patch, privacy-map patch, gitattributes patch, idempotent re-run, fresh-init no-op) unchanged.

Test plan

  • bun test test/artifacts-init-migration.test.ts — 14/14 pass
  • Manual: simulated Debian-without-jq box, confirmed done-marker withheld and second run after apt install jq patches the privacy-map

Followed @0xfandom-flagged option 1 from the issue (single-marker, deferred). The per-prerequisite marker (option 2) is a larger refactor; happy to follow up if you'd prefer that direction.

…ytan#1581)

The v1.40.0.0 migration writes its done-marker unconditionally, even
when the .brain-privacy-map.json patch was skipped because jq isn't
installed. Subsequent /gstack-upgrade runs short-circuit on the
marker, so the privacy-map repair silently never lands and federation
sync keeps dropping /plan-eng-review test plans. The user has to
notice the single WARN line, install jq, manually remove the marker,
and re-run.

Track a 'skipped_required' flag. Bump it to 1 when jq is missing or
when jq is present but the patch invocation fails. Skip 'touch DONE'
in that case so the next /gstack-upgrade retries the privacy-map
patch against an environment with jq installed. The allowlist and
gitattributes patches run independently (no jq needed) and the
'already present' gates keep re-runs idempotent.

Replace the one-line warning with an action-required block listing
the install commands for macOS / Debian / Fedora plus an explicit
note that the done-marker was withheld on purpose.

Tests: two new regressions in test/artifacts-init-migration.test.ts
use a PATH-clamped fake bin dir (coreutils symlinks, no jq) to
exercise both the jq-missing pass and the jq-installed retry. Full
file: 14/14 green.
@jbetala7
Copy link
Copy Markdown
Contributor

This solves the same #1581 failure already covered by older #1589. I think #1589 is the better canonical PR at the moment because it handles not only missing jq, but also malformed privacy-map JSON and write/mv/append failures, and its body calls out the separate remediation migration needed for users who already have the bad v1.40.0.0.done marker. If maintainers prefer this smaller patch, the action-required stderr copy is useful, but I would avoid landing both; they touch the same migration and test the same retry path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.40.0.0 migration writes done-marker even when jq is missing, leaving privacy-map unpatched

2 participants