mirror of https://github.com/garrytan/gstack.git
fix(upgrade): v1.27 migration no longer auto-proceeds without a TTY or records a failed rename as done (#1383)
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 <noreply@anthropic.com>
This commit is contained in:
parent
cd5433add1
commit
449f59549b
|
|
@ -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"
|
||||
|
||||
|
|
|
|||
|
|
@ -71,7 +71,11 @@ function run(extraEnv: Record<string, string> = {}, 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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue