From 3db12127b6cdedd3236a9f014901a521a2e76339 Mon Sep 17 00:00:00 2001 From: Jared Rickert Date: Sun, 30 Aug 2026 23:36:35 -0500 Subject: [PATCH] fix(keg): restore keg settings from pre-rename archives Renaming the keg document from config to settings renamed the archive manifest key with it -- `with_config` became `with_settings` -- but left the format identifier at `keg-archive/v3` on both sides of the change. An archive is a stored artifact that outlives the code that wrote it, so v3 tarballs carrying the old spelling are already in users' hands. They still pass the version gate, then unmarshal with WithSettings false because `with_config` matches no field. Three decisions read that one value: validation skips the document, replaceSettings never runs, and the inverted branch that stamps settings-updated for a settings-free archive now fires. The restore reports success while silently dropping the keg's title, summary, instructions, and custom indexes, and refreshes the updated stamp so it looks handled. Nodes, schemas, attachments, and history all restore correctly, which is what makes it easy to miss. Accept the pre-rename spelling and fold it forward immediately after the manifest is parsed, before anything reads WithSettings. Export is unchanged and still writes only `with_settings`; the alias is read-only, and a test asserts a freshly written manifest never contains `with_config` so it cannot leak back into the writer. The entry itself was always keg-archive/keg.yaml, so the document was present in every affected archive and merely unread -- nothing is unrecoverable. No released version is affected: v0.38.0 both writes and reads `with_config`, self-consistently. The break existed only between that rename and this commit. Covered by an import test built from a real export whose manifest is rewritten to the old spelling. Confirmed it fails without the fix, with the destination keeping its own title instead of the archived one. --- pkg/keg/archive.go | 26 ++++++++++++-- pkg/keg/archive_test.go | 80 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 3 deletions(-) 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)