fix(migrations): defer v1.40.0.0 done-marker when jq is missing (#1581)#1600
Open
genisis0x wants to merge 1 commit into
Open
fix(migrations): defer v1.40.0.0 done-marker when jq is missing (#1581)#1600genisis0x wants to merge 1 commit into
genisis0x wants to merge 1 commit into
Conversation
…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.
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1581.
Summary
gstack-upgrade/migrations/v1.40.0.0.shwrites~/.gstack/.migrations/v1.40.0.0.doneunconditionally. When the host doesn't havejqinstalled, the privacy-map patch logs a singleWARN:line and gets skipped — but the done-marker is still written, so subsequent/gstack-upgraderuns short-circuit at[ -f "${DONE}" ] && exit 0and the privacy-map repair never lands. Federation sync keeps silently dropping/plan-eng-reviewtest plans.Fix
Track a
skipped_requiredflag. Set to1when:command -v jqreturns false (jq not installed), orSkip
touch ${DONE}whenskipped_required=1so the next/gstack-upgraderetries against an env with jq installed. The allowlist + gitattributes patches still run on the same pass (they don't need jq), and theirgrep -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 passapt install jqpatches the privacy-mapFollowed @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.