diff --git a/pkg/keg/archive.go b/pkg/keg/archive.go index a9481be..f16751b 100644 --- a/pkg/keg/archive.go +++ b/pkg/keg/archive.go @@ -18,6 +18,12 @@ import ( // Archive format identifiers. v3 adds optional keg settings, optional keg schemas, // and stores file attachments under assets/ to match the on-disk/web node layout. +// +// v3 has two manifest spellings for the keg document. It was written as +// `with_config` before the settings rename and `with_settings` after, and the +// format identifier was not bumped between them, so both are accepted on read +// and normalized in importNodes. The entry itself has always been +// keg-archive/keg.yaml. const ( kegArchiveFormatV3 = "keg-archive/v3" ) @@ -28,9 +34,15 @@ type archiveManifest struct { ExportedAt time.Time `json:"exported_at"` WithHistory bool `json:"with_history,omitempty"` WithSettings bool `json:"with_settings,omitempty"` - WithSchemas bool `json:"with_schemas,omitempty"` - Schemas []string `json:"schemas,omitempty"` - Nodes []archiveManifestNode `json:"nodes"` + // WithConfig is the pre-rename spelling of WithSettings. An archive is a + // stored artifact that outlives the code that wrote it, and v3 tarballs + // carrying this spelling are already in users' hands, so it stays readable. + // Read-only: export never writes it, and importNodes folds it into + // WithSettings immediately after unmarshalling. + WithConfig bool `json:"with_config,omitempty"` + WithSchemas bool `json:"with_schemas,omitempty"` + Schemas []string `json:"schemas,omitempty"` + Nodes []archiveManifestNode `json:"nodes"` } type archiveManifestNode struct { @@ -357,6 +369,14 @@ func (k *LocalKeg) importNodes(ctx context.Context, r io.Reader, opts ImportNode if manifest.Format != kegArchiveFormatV3 { return nil, fmt.Errorf("unsupported archive format %q: %w", manifest.Format, ErrInvalid) } + // Fold the pre-rename spelling forward before anything reads WithSettings. + // Every later decision -- validating the document, restoring it, and the + // inverted branch that stamps settings-updated when an archive carried no + // document -- reads this one value, so normalizing here is what keeps a + // legacy archive from importing as though it had no keg settings at all. + if manifest.WithConfig { + manifest.WithSettings = true + } archivedSchemas, err := readArchiveSchemas(entries, manifest) if err != nil { diff --git a/pkg/keg/archive_test.go b/pkg/keg/archive_test.go index 9cf0be6..827f278 100644 --- a/pkg/keg/archive_test.go +++ b/pkg/keg/archive_test.go @@ -200,6 +200,10 @@ func TestArchiveExportUsesAssetsDirectoryAndIncludesConfigForFullBackup(t *testi require.NoError(t, json.Unmarshal(entries["keg-archive/manifest.json"], &manifest)) require.Equal(t, "keg-archive/v3", manifest.Format) require.True(t, manifest.WithSettings) + // The importer accepts the pre-rename `with_config` spelling; the writer + // must never emit it, or the legacy alias would outlive the archives that + // justify it. + require.NotContains(t, string(entries["keg-archive/manifest.json"]), "with_config") require.Contains(t, entries, "keg-archive/keg.yaml") require.Contains(t, entries, "keg-archive/nodes/"+id.ID.Path()+"/assets/doc.txt") require.Contains(t, entries, "keg-archive/nodes/"+id.ID.Path()+"/images/diagram.png") @@ -287,6 +291,82 @@ func TestArchiveImportRestoresKegSettingsForFullBackup(t *testing.T) { require.Contains(t, string(rawIndex), "indexed") } +// legacyConfigManifest rewrites a v3 manifest to the pre-rename spelling, +// producing the archive an older Tapper would have written. The format +// identifier is deliberately left alone: it was never bumped for the rename, +// which is exactly why these archives still reach the importer. +func legacyConfigManifest(t *testing.T, rawManifest []byte) []byte { + t.Helper() + var manifest map[string]any + require.NoError(t, json.Unmarshal(rawManifest, &manifest)) + withSettings, ok := manifest["with_settings"] + require.True(t, ok, "fixture expects a manifest that carries keg settings") + delete(manifest, "with_settings") + manifest["with_config"] = withSettings + rewritten, err := json.MarshalIndent(manifest, "", " ") + require.NoError(t, err) + return rewritten +} + +// A keg archive is a stored artifact that outlives the code that wrote it. The +// settings rename changed the manifest key from `with_config` to +// `with_settings` without bumping the format identifier, so a pre-rename +// archive still passes the version gate. Before the importer normalized the +// two spellings, it read WithSettings as false and silently dropped the keg +// document -- title, summary, instructions, and custom indexes -- while +// reporting a successful restore. +func TestArchiveImportRestoresKegSettingsFromLegacyConfigManifest(t *testing.T) { + t.Parallel() + fx := NewSandbox(t) + ctx := fx.Context() + + src := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) + initNonStrictTestKeg(t, src, ctx) + _, err := src.Create(ctx, &keg.CreateOptions{Title: "indexed", Body: []byte("# indexed\n"), Tags: []string{"restored"}}) + require.NoError(t, err) + require.NoError(t, src.UpdateSettings(ctx, func(cfg *keg.Settings) { + cfg.Title = "Legacy Title" + cfg.Summary = "Legacy summary" + cfg.Instructions = "Legacy instructions." + cfg.Timezone = "America/Chicago" + cfg.Indexes = append(cfg.UserIndexEntries(), keg.IndexEntry{File: "restored.md", Summary: "Restored nodes", Query: "restored"}) + })) + + archive := mustExportArchive(t, src, keg.ExportNodesOptions{WithAssets: true}) + entries := readArchiveEntriesForTest(t, archive) + legacy := legacyConfigManifest(t, entries["keg-archive/manifest.json"]) + require.Contains(t, string(legacy), "with_config") + require.NotContains(t, string(legacy), "with_settings") + archive = replaceArchiveEntry(t, archive, "keg-archive/manifest.json", legacy) + + dst := keg.NewLocalKeg(newTestMemoryRepo(fx.Runtime()), fx.Runtime()) + initNonStrictTestKeg(t, dst, ctx) + require.NoError(t, dst.UpdateSettings(ctx, func(cfg *keg.Settings) { + cfg.Title = "Target Title" + cfg.Summary = "Target summary" + cfg.Instructions = "Target instructions." + cfg.Timezone = "UTC" + })) + + _, err = dst.ImportNodes(ctx, bytes.NewReader(archive), keg.ImportNodesOptions{}) + require.NoError(t, err) + + // Each of these differs from what the destination was seeded with, so + // matching the source proves the archived document was applied rather than + // the import taking the "carried no settings" branch and leaving the + // target's own values in place. + got, err := dst.Settings(ctx) + require.NoError(t, err) + require.Equal(t, "Legacy Title", got.Title) + require.Equal(t, "Legacy summary", got.Summary) + require.Equal(t, "Legacy instructions.", got.Instructions) + require.Equal(t, "America/Chicago", got.Timezone) + + rawIndex, err := dst.ReadIndex(ctx, "restored.md") + require.NoError(t, err) + require.Contains(t, string(rawIndex), "indexed") +} + func TestArchiveImportNodeSubsetDoesNotRestoreKegSettings(t *testing.T) { t.Parallel() fx := NewSandbox(t)