Editing the body of an already-released installer migration drifts its computed checksum (it hashes plan.toString()). The integrity guard then hard-aborted every prior install on upgrade with "applied migration checksum changed" — a 100% reproducible blocker (v1.3.0, all platforms). Already-applied migrations are filtered out of `pending` and never re-run, so a drifted checksum is functionally inert. ADR-0008 anticipates checksum-mismatch state as something the install-state layer must handle gracefully (plan -> apply -> recover/report), not abort on. This supersedes the published-checksum allowlist merged in #674 (per-release maintenance debt — every historical checksum hand-pinned, still throws for any unregistered value) with a general, self-healing recovery: - Replace the throwing guard with non-fatal `collectAppliedChecksumDrift`, surfaced on `plan.checksumDrift`. - Reconcile drifted stored checksums durably on the next state write (`reconcileDriftedChecksums`), idempotently (no perpetual writes). - Relocate the "shipped migration bodies are immutable" rule to a CI baseline test that locks every shipped migration's checksum and fails on body drift — where #615 should have been caught, instead of blocking users. Removes #674's legacyChecksums field, per-migration checksum pins, published-checksums.json fixture, and compat test. Fixes #670 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -91,7 +91,6 @@ export function validateInstallerMigrationRecord(record: unknown, source?: strin
|
||||
throw new Error(`migration record must declare destructive as a boolean: ${displaySource}`);
|
||||
}
|
||||
validateStringArray(rec, 'runtimes', displaySource);
|
||||
validateStringArray(rec, 'legacyChecksums', displaySource);
|
||||
requireStringArray(rec, 'scopes', displaySource);
|
||||
if (typeof rec['plan'] !== 'function') {
|
||||
throw new Error(`migration record must include a plan function: ${displaySource}`);
|
||||
|
||||
@@ -208,27 +208,51 @@ function migrationChecksum(migration: MigrationRecord): string {
|
||||
return `sha256:${sha256Text(JSON.stringify(serializable))}`;
|
||||
}
|
||||
|
||||
function migrationLegacyChecksums(migration: MigrationRecord): string[] {
|
||||
return Array.isArray(migration.legacyChecksums)
|
||||
? migration.legacyChecksums.filter((checksum): checksum is string => typeof checksum === 'string' && checksum.length > 0)
|
||||
: [];
|
||||
// Rewrite the stored checksum of any already-applied entry whose id drifted, so the
|
||||
// drift is reconciled durably and not re-detected on every subsequent run (issue #670).
|
||||
// Returns the number of entries actually changed (so callers know whether a write is needed).
|
||||
function reconcileDriftedChecksums(
|
||||
appliedEntries: Array<Record<string, unknown>>,
|
||||
checksumDrift: Array<{ id: string; currentChecksum: string }> | undefined
|
||||
): number {
|
||||
if (!Array.isArray(checksumDrift) || checksumDrift.length === 0) return 0;
|
||||
const reconcile = new Map(checksumDrift.map((d) => [d.id, d.currentChecksum]));
|
||||
let changed = 0;
|
||||
for (let i = 0; i < appliedEntries.length; i++) {
|
||||
const existing = appliedEntries[i];
|
||||
if (existing && typeof existing.id === 'string' && reconcile.has(existing.id)) {
|
||||
const next = reconcile.get(existing.id) as string;
|
||||
if (existing.checksum !== next) {
|
||||
appliedEntries[i] = { ...existing, checksum: next };
|
||||
changed += 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
return changed;
|
||||
}
|
||||
|
||||
function migrationAcceptedChecksums(migration: MigrationRecord): Set<string> {
|
||||
return new Set([migrationChecksum(migration), ...migrationLegacyChecksums(migration)]);
|
||||
}
|
||||
|
||||
function assertAppliedMigrationChecksums(applied: Map<string, Record<string, unknown>>, migrations: MigrationRecord[]): void {
|
||||
function collectAppliedChecksumDrift(
|
||||
applied: Map<string, Record<string, unknown>>,
|
||||
migrations: MigrationRecord[]
|
||||
): Array<{ id: string; storedChecksum: string; currentChecksum: string }> {
|
||||
const drift: Array<{ id: string; storedChecksum: string; currentChecksum: string }> = [];
|
||||
for (const migration of migrations) {
|
||||
const entry = applied.get(migration.id as string);
|
||||
if (!entry || !entry.checksum) continue;
|
||||
const accepted = migrationAcceptedChecksums(migration);
|
||||
if (!accepted.has(entry.checksum as string)) {
|
||||
throw new Error(
|
||||
`applied migration checksum changed for ${migration.id as string}; create a new fix-forward migration id`
|
||||
);
|
||||
const currentChecksum = migrationChecksum(migration);
|
||||
if (entry.checksum !== currentChecksum) {
|
||||
// An already-applied migration is never re-run (it is filtered out of `pending`),
|
||||
// so a checksum drift here is functionally inert. A prior release may have edited a
|
||||
// shipped migration body (see issue #670). Surface it for reconciliation instead of
|
||||
// hard-aborting the user's upgrade.
|
||||
drift.push({
|
||||
id: migration.id as string,
|
||||
storedChecksum: entry.checksum as string,
|
||||
currentChecksum,
|
||||
});
|
||||
}
|
||||
}
|
||||
return drift;
|
||||
}
|
||||
|
||||
function migrationMatchesContext(migration: MigrationRecord, { runtime, scope }: { runtime: string | null; scope: string | null }): boolean {
|
||||
@@ -461,6 +485,7 @@ interface MigrationPlan {
|
||||
pendingMigrations: MigrationRecord[];
|
||||
actions: PlannedAction[];
|
||||
blocked: PlannedAction[];
|
||||
checksumDrift: Array<{ id: string; storedChecksum: string; currentChecksum: string }>;
|
||||
}
|
||||
|
||||
function planInstallerMigrations({
|
||||
@@ -490,7 +515,7 @@ function planInstallerMigrations({
|
||||
migrationMatchesContext(migration, { runtime, scope })
|
||||
);
|
||||
const applied = appliedMigrationEntries(state);
|
||||
assertAppliedMigrationChecksums(applied, scopedMigrations);
|
||||
const checksumDrift = collectAppliedChecksumDrift(applied, scopedMigrations);
|
||||
const pending = scopedMigrations.filter((migration) => !applied.has(migration.id as string));
|
||||
const actions: PlannedAction[] = [];
|
||||
const blocked: PlannedAction[] = [];
|
||||
@@ -578,6 +603,7 @@ function planInstallerMigrations({
|
||||
pendingMigrations: pending,
|
||||
actions,
|
||||
blocked,
|
||||
checksumDrift,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -757,6 +783,7 @@ function applyInstallerMigrationPlan({
|
||||
const state = readInstallState(configDir);
|
||||
const applied = appliedMigrationIds(state);
|
||||
const nextApplied = [...state.appliedMigrations];
|
||||
reconcileDriftedChecksums(nextApplied, plan.checksumDrift);
|
||||
const actionsByMigrationId = new Map<string, PlannedAction>();
|
||||
for (const action of plan.actions) {
|
||||
if (action.migrationId && !actionsByMigrationId.has(action.migrationId)) {
|
||||
@@ -819,29 +846,36 @@ function markPendingMigrationsApplied({
|
||||
plan: MigrationPlan;
|
||||
now?: () => string;
|
||||
}): string[] {
|
||||
if (!plan || !Array.isArray(plan.pendingMigrationIds) || plan.pendingMigrationIds.length === 0) {
|
||||
return [];
|
||||
}
|
||||
if (!plan) return [];
|
||||
const hasPending = Array.isArray(plan.pendingMigrationIds) && plan.pendingMigrationIds.length > 0;
|
||||
const hasDrift = Array.isArray(plan.checksumDrift) && plan.checksumDrift.length > 0;
|
||||
if (!hasPending && !hasDrift) return [];
|
||||
|
||||
const appliedAt = now();
|
||||
const state = readInstallState(configDir);
|
||||
const applied = appliedMigrationIds(state);
|
||||
const checksumsByMigrationId = new Map<string, string>();
|
||||
for (const migration of plan.pendingMigrations || []) {
|
||||
checksumsByMigrationId.set(migration.id as string, migrationChecksum(migration));
|
||||
}
|
||||
const nextApplied = [...state.appliedMigrations];
|
||||
const reconciledCount = reconcileDriftedChecksums(nextApplied, plan.checksumDrift);
|
||||
|
||||
const newlyApplied: string[] = [];
|
||||
for (const id of plan.pendingMigrationIds) {
|
||||
if (applied.has(id)) continue;
|
||||
nextApplied.push({
|
||||
id,
|
||||
appliedAt,
|
||||
journal: null,
|
||||
checksum: checksumsByMigrationId.get(id) || null,
|
||||
});
|
||||
newlyApplied.push(id);
|
||||
if (hasPending) {
|
||||
const checksumsByMigrationId = new Map<string, string>();
|
||||
for (const migration of plan.pendingMigrations || []) {
|
||||
checksumsByMigrationId.set(migration.id as string, migrationChecksum(migration));
|
||||
}
|
||||
for (const id of plan.pendingMigrationIds) {
|
||||
if (applied.has(id)) continue;
|
||||
nextApplied.push({
|
||||
id,
|
||||
appliedAt,
|
||||
journal: null,
|
||||
checksum: checksumsByMigrationId.get(id) || null,
|
||||
});
|
||||
newlyApplied.push(id);
|
||||
}
|
||||
}
|
||||
if (newlyApplied.length > 0) {
|
||||
|
||||
if (newlyApplied.length > 0 || reconciledCount > 0) {
|
||||
writeInstallState(configDir, {
|
||||
schemaVersion: 1,
|
||||
appliedMigrations: nextApplied,
|
||||
@@ -933,8 +967,8 @@ export = {
|
||||
acquireInstallMigrationLock,
|
||||
applyInstallerMigrationPlan,
|
||||
classifyArtifact,
|
||||
computeInstallerMigrationChecksum: migrationChecksum,
|
||||
discoverInstallerMigrations,
|
||||
migrationChecksum,
|
||||
planInstallerMigrations,
|
||||
readInstallManifest,
|
||||
readInstallState,
|
||||
|
||||
@@ -171,8 +171,6 @@ interface InstallerMigration {
|
||||
title: string;
|
||||
description: string;
|
||||
introducedIn: string;
|
||||
checksum: string;
|
||||
legacyChecksums?: string[];
|
||||
scopes: string[];
|
||||
destructive: boolean;
|
||||
plan: (ctx: PlanContext) => BaselineAction[];
|
||||
@@ -183,10 +181,6 @@ const migration: InstallerMigration = {
|
||||
title: 'Record first-time installer migration baseline',
|
||||
description: 'Classify existing install surfaces before destructive installer migrations run.',
|
||||
introducedIn: '1.50.0',
|
||||
checksum: 'sha256:4ec58d35b30dbf39cc56e3972146086d8d31861ecd800cf0b37a7aa94fe74c2a',
|
||||
legacyChecksums: [
|
||||
'sha256:34608ea4e2f4e1c53b069604892860e603600d8573cc6a5584e4194044b48e67',
|
||||
],
|
||||
scopes: ['global', 'local'],
|
||||
destructive: false,
|
||||
plan: ({ configDir, runtime, baselineScan, classifyArtifact }: PlanContext): BaselineAction[] => {
|
||||
|
||||
@@ -31,8 +31,6 @@ interface InstallerMigration {
|
||||
title: string;
|
||||
description: string;
|
||||
introducedIn: string;
|
||||
checksum: string;
|
||||
legacyChecksums?: string[];
|
||||
scopes: string[];
|
||||
destructive: boolean;
|
||||
plan: (ctx: MigrationPlanContext) => MigrationAction[];
|
||||
@@ -48,10 +46,6 @@ const migration: InstallerMigration = {
|
||||
title: 'Remove manifest-managed legacy orphan hook files',
|
||||
description: 'Remove legacy orphan hook files that are still manifest-managed.',
|
||||
introducedIn: '1.50.0',
|
||||
checksum: 'sha256:e492698748a2436a12a55f0940f539b9bf651d8ffcac6f60cd856a6dabd6788c',
|
||||
legacyChecksums: [
|
||||
'sha256:4488e38c127a5225b31016918bcbc85ba3fd3139291ad407b94e76c03c0b89d3',
|
||||
],
|
||||
scopes: ['global', 'local'],
|
||||
destructive: true,
|
||||
// Retired generated hook files are removed only with manifest-managed
|
||||
|
||||
@@ -48,8 +48,6 @@ interface InstallerMigration {
|
||||
title: string;
|
||||
description: string;
|
||||
introducedIn: string;
|
||||
checksum: string;
|
||||
legacyChecksums?: string[];
|
||||
runtimes: string[];
|
||||
scopes: string[];
|
||||
destructive: boolean;
|
||||
@@ -112,10 +110,6 @@ const migration: InstallerMigration = {
|
||||
title: 'Remove legacy Codex hooks.json GSD hook registrations',
|
||||
description: 'Remove legacy Codex hooks.json GSD hook registrations after config.toml migration.',
|
||||
introducedIn: '1.50.0',
|
||||
checksum: 'sha256:5ce55294aa02f25758f604a569c899a6d2d060299189f5f447f68d8033157058',
|
||||
legacyChecksums: [
|
||||
'sha256:41f1545704dc72dfc3ab019207677a8652e389b200e8c450d52317df5bc198da',
|
||||
],
|
||||
runtimes: ['codex'],
|
||||
scopes: ['global', 'local'],
|
||||
destructive: true,
|
||||
|
||||
@@ -48,7 +48,6 @@ interface InstallerMigration {
|
||||
title: string;
|
||||
description: string;
|
||||
introducedIn: string;
|
||||
checksum: string;
|
||||
scopes: string[];
|
||||
destructive: boolean;
|
||||
plan(ctx: MigrationPlanContext): MigrationAction[];
|
||||
@@ -81,7 +80,6 @@ const migration: InstallerMigration = {
|
||||
'After the config dir rename from get-shit-done/ to gsd-core/ (#604), remove prior-manifest-managed files ' + // gsd-allow-legacy-name
|
||||
'from the stale legacy directory during install (framework rollback restores them if install fails). User-added files are preserved.',
|
||||
introducedIn: '1.2.0',
|
||||
checksum: 'sha256:3a9f1d97f64097fb313203d19c6d93a187a38df61dd299afa5eef73e16124e95',
|
||||
scopes: ['global', 'local'],
|
||||
destructive: true,
|
||||
plan(ctx: MigrationPlanContext): MigrationAction[] {
|
||||
|
||||
Reference in New Issue
Block a user