From 5c09721e34684bc06e5427e669151f3cb88e8a76 Mon Sep 17 00:00:00 2001 From: Jayesh Betala Date: Thu, 14 May 2026 17:25:52 +0530 Subject: [PATCH] fix(upgrade): gate artifact migration in agents --- gstack-upgrade/migrations/v1.27.0.0.sh | 40 ++++++++++++++-------- test/migrations-v1.27.0.0.test.ts | 46 +++++++++++++++++++++----- 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/gstack-upgrade/migrations/v1.27.0.0.sh b/gstack-upgrade/migrations/v1.27.0.0.sh index fb1ce73ce8..dd80768aff 100755 --- a/gstack-upgrade/migrations/v1.27.0.0.sh +++ b/gstack-upgrade/migrations/v1.27.0.0.sh @@ -44,6 +44,7 @@ MIGRATION_DIR="${GSTACK_HOME}/.migrations" JOURNAL="${MIGRATION_DIR}/v1.27.0.0.journal" DONE="${MIGRATION_DIR}/v1.27.0.0.done" SKIPPED="${MIGRATION_DIR}/v1.27.0.0.skipped-by-user" +PENDING="${MIGRATION_DIR}/v1.27.0.0.pending-user-confirmation" USER_NAME="${USER:-$(whoami 2>/dev/null || echo unknown)}" OLD_REPO_NAME="gstack-brain-${USER_NAME}" @@ -121,17 +122,26 @@ EOF n|N|no|No|NO) echo " Skipping migration. Re-run via /setup-gbrain --rerun-migration." >&2 touch "$SKIPPED" + rm -f "$PENDING" 2>/dev/null || true exit 0 ;; skip|skip-for-now|s) echo " Skipping for now. Will ask again next upgrade." >&2 + touch "$PENDING" # Don't write SKIPPED — leave both old + new state untouched, ask again next time. exit 0 ;; esac + rm -f "$PENDING" 2>/dev/null || true else - # Non-interactive (CI, scripted upgrade): proceed automatically. - echo " (non-interactive: proceeding automatically)" >&2 + if [ "${GSTACK_AUTO_MIGRATE:-}" = "1" ]; then + echo " (non-interactive: GSTACK_AUTO_MIGRATE=1, proceeding automatically)" >&2 + rm -f "$PENDING" 2>/dev/null || true + else + echo " (non-interactive: waiting for user confirmation; set GSTACK_AUTO_MIGRATE=1 to run automatically)" >&2 + touch "$PENDING" + exit 0 + fi fi fi @@ -194,14 +204,15 @@ if ! journal_done "gh_repo_renamed"; then echo " renamed on GitHub" >&2 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 " ERROR: gh rename failed (repo may not exist, permission denied, or confirmation unavailable)" >&2 + echo " Aborting before rewriting local artifact remote/config state." >&2 + exit 1 fi fi else - echo " gh CLI not available — skipping rename step (manual: gh repo rename ...)" >&2 - mark_done "gh_repo_renamed" + echo " ERROR: gh CLI not available — cannot safely rename GitHub repo." >&2 + echo " Aborting before rewriting local artifact remote/config state." >&2 + exit 1 fi ;; gitlab) @@ -211,19 +222,22 @@ if ! journal_done "gh_repo_renamed"; then mark_done "gh_repo_renamed" else # GitLab CLI doesn't have a direct rename; user has to do it via API. - echo " glab repo rename isn't a single command on GitLab." >&2 + echo " ERROR: glab repo rename isn't a single command on GitLab." >&2 echo " Manual: visit your GitLab project Settings → General → Advanced → Rename" >&2 echo " or use: glab api projects/:id -X PUT -f name=$NEW_REPO_NAME -f path=$NEW_REPO_NAME" >&2 - mark_done "gh_repo_renamed" + echo " Aborting before rewriting local artifact remote/config state." >&2 + exit 1 fi else - echo " glab not available — manual rename required" >&2 - mark_done "gh_repo_renamed" + echo " ERROR: glab not available — manual rename required" >&2 + echo " Aborting before rewriting local artifact remote/config state." >&2 + exit 1 fi ;; manual|*) - echo " unknown host (not github/gitlab) — manual rename required" >&2 - mark_done "gh_repo_renamed" + echo " ERROR: unknown host (not github/gitlab) — manual rename required" >&2 + echo " Aborting before rewriting local artifact remote/config state." >&2 + exit 1 ;; esac fi diff --git a/test/migrations-v1.27.0.0.test.ts b/test/migrations-v1.27.0.0.test.ts index 7a1a9908cc..60bf06afc7 100644 --- a/test/migrations-v1.27.0.0.test.ts +++ b/test/migrations-v1.27.0.0.test.ts @@ -81,6 +81,8 @@ function run(extraEnv: Record = {}, input = ''): { code: number; return { code: r.status ?? -1, stdout: r.stdout || '', stderr: r.stderr || '' }; } +const AUTO_MIGRATE = { GSTACK_AUTO_MIGRATE: '1' }; + beforeEach(() => { tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'mig-v1.27-')); fakeBinDir = fs.mkdtempSync(path.join(os.tmpdir(), 'mig-v1.27-fake-')); @@ -120,7 +122,7 @@ describe('v1.27.0.0 migration — nothing to migrate', () => { }); }); -describe('v1.27.0.0 migration — GitHub host (non-interactive)', () => { +describe('v1.27.0.0 migration — GitHub host', () => { beforeEach(() => { fs.writeFileSync( path.join(tmpHome, '.gstack-brain-remote.txt'), @@ -133,9 +135,21 @@ describe('v1.27.0.0 migration — GitHub host (non-interactive)', () => { makeFakeGh({}); }); - test('renames repo, mvs remote.txt, rewrites config key, writes done', () => { + test('non-interactive without explicit opt-in waits for user confirmation', () => { const r = run(); expect(r.code).toBe(0); + expect(r.stderr).toContain('waiting for user confirmation'); + expect(fs.existsSync(path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.pending-user-confirmation'))).toBe(true); + expect(fs.existsSync(path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.done'))).toBe(false); + expect(fs.existsSync(path.join(tmpHome, '.gstack-artifacts-remote.txt'))).toBe(false); + expect(fs.existsSync(path.join(fakeBinDir, 'gh-calls.log'))).toBe(false); + const cfg = fs.readFileSync(path.join(tmpHome, '.gstack/config.yaml'), 'utf-8'); + expect(cfg).toContain('gbrain_sync_mode: full'); + }); + + test('explicit auto-migrate renames repo, mvs remote.txt, rewrites config key, writes done', () => { + const r = run(AUTO_MIGRATE); + expect(r.code).toBe(0); // gh rename was called (or edit fallback). const ghLog = fs.readFileSync(path.join(fakeBinDir, 'gh-calls.log'), 'utf-8'); expect(ghLog).toMatch(/gh repo (rename|edit)/); @@ -154,7 +168,7 @@ describe('v1.27.0.0 migration — GitHub host (non-interactive)', () => { }); test('idempotent: re-run after success is a no-op', () => { - run(); + run(AUTO_MIGRATE); const r2 = run(); expect(r2.code).toBe(0); expect(r2.stderr).toBe(''); @@ -162,10 +176,24 @@ describe('v1.27.0.0 migration — GitHub host (non-interactive)', () => { test('repo already renamed (gh repo view succeeds with new name) → no rename attempt', () => { makeFakeGh({ alreadyRenamed: true }); - const r = run(); + const r = run(AUTO_MIGRATE); expect(r.code).toBe(0); expect(r.stderr).toContain('already named'); }); + + test('gh rename failure aborts before local state rewrite', () => { + makeFakeGh({ renameSucceeds: false }); + const r = run(AUTO_MIGRATE); + expect(r.code).toBe(1); + expect(r.stderr).toContain('gh rename failed'); + expect(r.stderr).toContain('Aborting before rewriting local artifact remote/config state'); + expect(fs.existsSync(path.join(tmpHome, '.gstack-brain-remote.txt'))).toBe(true); + expect(fs.existsSync(path.join(tmpHome, '.gstack-artifacts-remote.txt'))).toBe(false); + const cfg = fs.readFileSync(path.join(tmpHome, '.gstack/config.yaml'), 'utf-8'); + expect(cfg).toContain('gbrain_sync_mode: full'); + expect(cfg).not.toContain('artifacts_sync_mode: full'); + expect(fs.existsSync(path.join(tmpHome, '.gstack/.migrations/v1.27.0.0.done'))).toBe(false); + }); }); describe('v1.27.0.0 migration — interruption resume', () => { @@ -183,7 +211,7 @@ describe('v1.27.0.0 migration — interruption resume', () => { fs.mkdirSync(migDir, { recursive: true }); fs.writeFileSync(path.join(migDir, 'v1.27.0.0.journal'), 'gh_repo_renamed\nremote_txt_renamed\n'); - const r = run(); + const r = run(AUTO_MIGRATE); expect(r.code).toBe(0); // gh should NOT have been called (step 1 already done). if (fs.existsSync(path.join(fakeBinDir, 'gh-calls.log'))) { @@ -210,7 +238,7 @@ describe('v1.27.0.0 migration — remote-MCP mode (step 5 prints, never executes makeFakeGh({}); makeFakeGbrain({}); // installed, but should NOT be called for sources commands - const r = run(); + const r = run(AUTO_MIGRATE); expect(r.code).toBe(0); expect(r.stderr).toContain('Remote MCP detected'); expect(r.stderr).toContain('Send this to your brain admin'); @@ -235,7 +263,7 @@ describe('v1.27.0.0 migration — local CLI sources swap (codex Finding #6 order makeFakeGh({}); makeFakeGbrain({ hasOldSource: true }); - const r = run(); + const r = run(AUTO_MIGRATE); expect(r.code).toBe(0); const log = fs.readFileSync(path.join(fakeBinDir, 'gbrain-calls.log'), 'utf-8'); @@ -256,7 +284,7 @@ describe('v1.27.0.0 migration — local CLI sources swap (codex Finding #6 order makeFakeGh({}); makeFakeGbrain({ addSucceeds: false }); - const r = run(); + const r = run(AUTO_MIGRATE); expect(r.code).toBe(0); // step 5 warns, doesn't fail the migration expect(r.stderr).toContain('failed to add'); const log = fs.readFileSync(path.join(fakeBinDir, 'gbrain-calls.log'), 'utf-8'); @@ -281,7 +309,7 @@ describe('v1.27.0.0 migration — CLAUDE.md block field rewrite', () => { fs.writeFileSync(path.join(tmpHome, 'CLAUDE.md'), claudeMd); makeFakeGh({}); - const r = run(); + const r = run(AUTO_MIGRATE); expect(r.code).toBe(0); const updated = fs.readFileSync(path.join(tmpHome, 'CLAUDE.md'), 'utf-8'); expect(updated).toContain('- Artifacts sync: full');