From 449f59549bf326c9f7d08dcb37aff7db90664f0e Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 12:02:29 -0700 Subject: [PATCH] fix(upgrade): v1.27 migration no longer auto-proceeds without a TTY or records a failed rename as done (#1383) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent-failure shapes in one script. Non-interactive runs (Claude Code Bash tool, CI) blanket-auto-proceeded into a REMOTE repo rename — now they skip-for-now by default and ask again next upgrade; unattended runs opt in with GSTACK_MIGRATE_ASSUME_YES=1. And a failed gh rename was journaled as done and the done-touchfile written, permanently stranding a half-renamed install — the failed step now stays PENDING with the manual command printed, finalize refuses the done-marker while any step is unjournaled, and the migration exits 1 with a re-run pointer while completed steps still skip on retry. Harness updated to opt in explicitly; new tests pin the default-skip and failure-stays-pending-then-retry-succeeds contracts (13/13). Co-Authored-By: Claude Fable 5 --- gstack-upgrade/migrations/v1.27.0.0.sh | 33 +++++++++++++--- test/migrations-v1.27.0.0.test.ts | 55 +++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 6 deletions(-) diff --git a/gstack-upgrade/migrations/v1.27.0.0.sh b/gstack-upgrade/migrations/v1.27.0.0.sh index fb1ce73ce..2fbd273c0 100755 --- a/gstack-upgrade/migrations/v1.27.0.0.sh +++ b/gstack-upgrade/migrations/v1.27.0.0.sh @@ -130,8 +130,19 @@ EOF ;; esac else - # Non-interactive (CI, scripted upgrade): proceed automatically. - echo " (non-interactive: proceeding automatically)" >&2 + # Non-interactive (CI, Claude Code Bash tool, scripted upgrade). Step 1 + # renames a REMOTE repo — consent-shaped, and blanket auto-proceed once + # left an install half-migrated when that step failed mid-run (#1383). + # Skip for now by default (asked again next upgrade); explicit opt-in + # proceeds unattended. + if [ "${GSTACK_MIGRATE_ASSUME_YES:-0}" = "1" ]; then + echo " (non-interactive: proceeding — GSTACK_MIGRATE_ASSUME_YES=1)" >&2 + else + echo " Non-interactive session: skipping for now (will ask again next upgrade)." >&2 + echo " To proceed unattended: GSTACK_MIGRATE_ASSUME_YES=1 ./setup" >&2 + echo " To run interactively: /setup-gbrain --rerun-migration" >&2 + exit 0 + fi fi fi @@ -195,8 +206,8 @@ if ! journal_done "gh_repo_renamed"; then mark_done "gh_repo_renamed" else echo " WARNING: gh rename failed (repo may not exist or permission denied)" >&2 - echo " skipping step 1; subsequent steps still run" >&2 - mark_done "gh_repo_renamed" + echo " step 1 stays PENDING and will retry on re-run; later steps still run (#1383)" >&2 + echo " manual: gh repo rename $NEW_REPO_NAME --repo $OLD_REPO_NAME --yes" >&2 fi fi else @@ -335,8 +346,20 @@ EOF fi # --------------------------------------------------------------------------- -# Step 6: finalize (touchfile + clear journal) +# Step 6: finalize (touchfile + clear journal) — only when EVERY step is +# journaled. A failed step must leave the migration visibly incomplete and +# retryable, never silently recorded as done (#1383). # --------------------------------------------------------------------------- +INCOMPLETE="" +for _step in gh_repo_renamed remote_txt_renamed config_key_renamed claude_md_block_rewritten sources_swapped; do + journal_done "$_step" || INCOMPLETE="$INCOMPLETE $_step" +done +if [ -n "$INCOMPLETE" ]; then + echo " [v1.27.0.0] migration INCOMPLETE — pending step(s):$INCOMPLETE" >&2 + echo " Completed steps are journaled and will be skipped on re-run." >&2 + echo " Re-run via: /setup-gbrain --rerun-migration" >&2 + exit 1 +fi touch "$DONE" rm -f "$JOURNAL" diff --git a/test/migrations-v1.27.0.0.test.ts b/test/migrations-v1.27.0.0.test.ts index 7a1a9908c..fa2206ea8 100644 --- a/test/migrations-v1.27.0.0.test.ts +++ b/test/migrations-v1.27.0.0.test.ts @@ -71,7 +71,11 @@ function run(extraEnv: Record = {}, input = ''): { code: number; PATH: `${fakeBinDir}:${path.join(ROOT, 'bin')}:/usr/bin:/bin:/opt/homebrew/bin`, HOME: tmpHome, USER: 'testuser', - // Disable interactive prompt: empty stdin = treat as non-interactive. + // Empty stdin = non-interactive. Since #1383 the script SKIPS by + // default in that context (a remote repo rename is consent-shaped); + // the harness opts in explicitly, simulating a consenting unattended + // run. The default-skip contract has its own test below. + GSTACK_MIGRATE_ASSUME_YES: '1', ...extraEnv, }, encoding: 'utf-8', @@ -168,6 +172,55 @@ describe('v1.27.0.0 migration — GitHub host (non-interactive)', () => { }); }); +describe('v1.27.0.0 migration — #1383 consent + failure-stays-pending contract', () => { + beforeEach(() => { + fs.writeFileSync( + path.join(tmpHome, '.gstack-brain-remote.txt'), + 'https://github.com/testuser/gstack-brain-testuser\n' + ); + fs.writeFileSync( + path.join(tmpHome, '.gstack/config.yaml'), + 'gbrain_sync_mode: full\n' + ); + makeFakeGh({}); + }); + + test('non-interactive without opt-in: skips for now, touches NOTHING', () => { + const r = run({ GSTACK_MIGRATE_ASSUME_YES: '0' }); + expect(r.code).toBe(0); + expect(r.stderr).toContain('skipping for now'); + expect(r.stderr).toContain('GSTACK_MIGRATE_ASSUME_YES=1'); + // Old state untouched, nothing recorded as done. + expect(fs.existsSync(path.join(tmpHome, '.gstack-brain-remote.txt'))).toBe(true); + expect(fs.existsSync(path.join(tmpHome, '.gstack-artifacts-remote.txt'))).toBe(false); + expect(fs.existsSync(path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.done'))).toBe(false); + }); + + test('gh rename failure: step stays pending, migration exits INCOMPLETE, retry succeeds', () => { + makeFakeGh({ renameSucceeds: false }); + const r = run(); + // Failure must be loud and the migration visibly incomplete — the old + // behavior marked the failed step done and wrote the done touchfile, + // permanently stranding a half-renamed install (#1383). + expect(r.code).toBe(1); + expect(r.stderr).toContain('PENDING'); + expect(r.stderr).toContain('INCOMPLETE'); + expect(fs.existsSync(path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.done'))).toBe(false); + const journal = fs.readFileSync( + path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.journal'), + 'utf-8' + ); + expect(journal).not.toContain('gh_repo_renamed'); + // Later steps DID run and are journaled (independent of step 1)... + expect(journal).toContain('remote_txt_renamed'); + // ...so a retry with working gh only redoes step 1 and completes. + makeFakeGh({}); + const r2 = run(); + expect(r2.code).toBe(0); + expect(fs.existsSync(path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.done'))).toBe(true); + }); +}); + describe('v1.27.0.0 migration — interruption resume', () => { beforeEach(() => { fs.writeFileSync(