From 078f661e3a01a9b9b970a10b5fe2a0ba036951fc Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 5 Sep 2026 15:30:31 +0000 Subject: [PATCH] Remove retired cleanup manifest state --- docs/internal/command-restore.md | 4 +- docs/internal/manifest.md | 11 ++++-- docs/operations.md | 8 ++++ docs/roadmap/implementation.md | 2 +- docs/troubleshooting.md | 5 +++ internal/app/restore_manifest_paths.go | 3 -- internal/app/restore_recovery_test.go | 4 ++ internal/manifest/manifest.go | 54 ++++++++------------------ internal/manifest/store.go | 39 ++++++------------- internal/manifest/store_test.go | 28 +++++++++++++ 10 files changed, 84 insertions(+), 74 deletions(-) diff --git a/docs/internal/command-restore.md b/docs/internal/command-restore.md index 640603d..a2eb4ef 100644 --- a/docs/internal/command-restore.md +++ b/docs/internal/command-restore.md @@ -78,7 +78,9 @@ Execution order and safety: - restored manifest local references are rebased beneath the selected local session root. Unsafe relative references and producer-machine absolute paths outside the manifest's producer session root are rejected; producer-local - spool/cache and cleanup locations are not restored as authority. + spool paths and run-local diagnostic references are not restored as + authority. Cleanup remains ordinary restored stage state and is not reset or + synthesized by restore. Audio restore path: diff --git a/docs/internal/manifest.md b/docs/internal/manifest.md index 21a9fe7..f0cfd03 100644 --- a/docs/internal/manifest.md +++ b/docs/internal/manifest.md @@ -16,16 +16,18 @@ manifest placement belongs in - remote identity metadata (`s3_bucket`, `s3_session_prefix`, `s3_run_prefix`) - `inputs` records - durable `artifacts` records -- per-stage `stages` map -- an optional `post_publish_cleanup` obligation, which binds a committed run, - remote commit identity, and each exact root-confined local target to its - completion evidence +- per-stage `stages` map, including the ordinary cleanup stage record Session, campaign, and run identities in local and downloaded manifests must be portable opaque segments. Unsafe legacy identities are rejected with migration guidance rather than being normalized into a different workspace or remote namespace. +Decoding is strict at every typed manifest field. Unknown fields are rejected; +the schema version remains `narratio.session-manifest.v1` and there is no +compatibility reader that silently drops retired state. Cleanup progress uses +only the cleanup entry in `stages`. + Every persisted session manifest has a non-nil `stages` map. Each entry is non-nil and its embedded `name` exactly matches its map key; loading never invents either marker. A missing or unsupported schema version, or incomplete @@ -145,6 +147,7 @@ establish a current immutable snapshot. `manifest.LocalStore`: - accepts only the exact supported session-manifest schema version; +- rejects unknown fields and multiple JSON values; - validates stage-map identity without synthesizing missing maps, records, or names; - retains only representation-preserving collection normalization; diff --git a/docs/operations.md b/docs/operations.md index 4077e67..3d222cc 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -610,6 +610,14 @@ also available when the manifest has valid publish evidence. A range ending at `notify` or earlier and the standalone `publish` command never delete local state. +Older local manifests that contain the retired cleanup-obligation field are +intentionally incompatible with strict manifest decoding. If such an +obligation is still outstanding, complete it with the pre-upgrade binary. +Otherwise verify that the remote publication is committed, move aside or +regenerate the incompatible local manifest (or restore the verified remote +snapshot), and then use the cleanup stage. Manual JSON editing is not the +normal migration path. + ## Operational Caveats - Local and S3 audio modes are mutually exclusive. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 8813ae2..b130778 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -246,7 +246,7 @@ post-loop deletion path. ## Stage 4 — Remove The Cleanup Obligation From Manifest State -**Status:** Pending +**Status:** Completed ### Goal diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 271cc65..191e591 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -73,6 +73,11 @@ Safe fix: regenerate it; restore a verified current committed snapshot or republish healthy regenerated state when appropriate. Do not add version or stage markers by hand. +- if an older manifest is rejected for its retired cleanup-obligation field, + finish outstanding cleanup with the pre-upgrade binary. Otherwise verify the + remote commit, move aside or regenerate the incompatible manifest, and use + the cleanup stage. Do not treat manual JSON editing as the normal migration + path. Relevant reference: [Configuration](./config.md). diff --git a/internal/app/restore_manifest_paths.go b/internal/app/restore_manifest_paths.go index b05d28f..1557aa5 100644 --- a/internal/app/restore_manifest_paths.go +++ b/internal/app/restore_manifest_paths.go @@ -122,9 +122,6 @@ func rebaseRestoredManifestPaths(cfg *config.Config, m *manifest.Manifest, desti m.LocalWorkDir = destinationRoot } m.LocalSpoolDir = "" - // A post-publish cleanup record is authority to delete producer-local - // directories. It must never cross a restore boundary. - m.PostPublishCleanup = nil return nil } diff --git a/internal/app/restore_recovery_test.go b/internal/app/restore_recovery_test.go index 73fe6a4..1ede465 100644 --- a/internal/app/restore_recovery_test.go +++ b/internal/app/restore_recovery_test.go @@ -168,6 +168,7 @@ func TestRebaseRestoredManifestPathsRejectsUnsafeReferencesAndRebasesPortablePat m.RunID = "20260519T010203Z-a1b2c3d4" m.LocalWorkDir = test.workDir m.Stages["analyze"] = &manifest.StageRecord{Outputs: []manifest.ArtifactRecord{{Kind: "recap", LocalPath: test.output}}} + m.MarkStageFailed("cleanup", nowUTC(), "retry local deletion") destination := artifacts.SessionWorkDirForCampaign(cfg.Pipeline.Workspace.Root, cfg.Session.Campaign, cfg.Session.SessionID) err := rebaseRestoredManifestPaths(cfg, m, destination) @@ -183,6 +184,9 @@ func TestRebaseRestoredManifestPathsRejectsUnsafeReferencesAndRebasesPortablePat if got, want := m.Stages["analyze"].Outputs[0].LocalPath, filepath.Join(destination, test.wantPath); got != want { t.Fatalf("rebased local path = %q, want %q", got, want) } + if got := m.Stages["cleanup"].Status; got != manifest.StatusFailed { + t.Fatalf("cleanup status = %q, want preserved failed state", got) + } }) } } diff --git a/internal/manifest/manifest.go b/internal/manifest/manifest.go index dad2033..e6dbebc 100644 --- a/internal/manifest/manifest.go +++ b/internal/manifest/manifest.go @@ -66,25 +66,6 @@ type StageRecord struct { AnalyzeArtifacts map[string]AnalyzeArtifactRecord `json:"analyze_artifacts,omitempty"` } -// CleanupTarget records one root-confined local deletion requested by a -// committed publication. -type CleanupTarget struct { - Policy string `json:"policy"` - Root string `json:"root"` - Path string `json:"path"` - Completed bool `json:"completed"` -} - -// PostPublishCleanup records the durable deletion work left by a committed -// publish. It remains after completion as evidence of the exact committed run -// and local paths involved. -type PostPublishCleanup struct { - CommittedRunID string `json:"committed_run_id"` - RemoteCommitKey string `json:"remote_commit_key"` - CurrentCommitPointerKey string `json:"current_commit_pointer_key"` - Targets []CleanupTarget `json:"targets"` -} - // EffectiveConfigProvenance records bounded non-secret configuration identity // for one resolved invocation. type EffectiveConfigProvenance struct { @@ -101,24 +82,23 @@ type SelectedProfileProvenance struct { // Manifest is the durable run-state record for a session execution. type Manifest struct { - SchemaVersion string `json:"schema_version"` - SessionID string `json:"session_id"` - Campaign string `json:"campaign,omitempty"` - RunID string `json:"run_id,omitempty"` - LocalWorkDir string `json:"local_workdir,omitempty"` - LocalSpoolDir string `json:"local_spool_dir,omitempty"` - S3Bucket string `json:"s3_bucket,omitempty"` - S3SessionPrefix string `json:"s3_session_prefix,omitempty"` - S3RunPrefix string `json:"s3_run_prefix,omitempty"` - PipelineVersion string `json:"pipeline_version,omitempty"` - EffectiveConfig *EffectiveConfigProvenance `json:"effective_config,omitempty"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - LastError *ErrorRecord `json:"last_error,omitempty"` - Inputs []InputRecord `json:"inputs,omitempty"` - Artifacts []ArtifactRecord `json:"artifacts,omitempty"` - Stages map[string]*StageRecord `json:"stages"` - PostPublishCleanup *PostPublishCleanup `json:"post_publish_cleanup,omitempty"` + SchemaVersion string `json:"schema_version"` + SessionID string `json:"session_id"` + Campaign string `json:"campaign,omitempty"` + RunID string `json:"run_id,omitempty"` + LocalWorkDir string `json:"local_workdir,omitempty"` + LocalSpoolDir string `json:"local_spool_dir,omitempty"` + S3Bucket string `json:"s3_bucket,omitempty"` + S3SessionPrefix string `json:"s3_session_prefix,omitempty"` + S3RunPrefix string `json:"s3_run_prefix,omitempty"` + PipelineVersion string `json:"pipeline_version,omitempty"` + EffectiveConfig *EffectiveConfigProvenance `json:"effective_config,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + LastError *ErrorRecord `json:"last_error,omitempty"` + Inputs []InputRecord `json:"inputs,omitempty"` + Artifacts []ArtifactRecord `json:"artifacts,omitempty"` + Stages map[string]*StageRecord `json:"stages"` } // New constructs a new manifest with deterministic timestamps. diff --git a/internal/manifest/store.go b/internal/manifest/store.go index 85e4077..5cc56e5 100644 --- a/internal/manifest/store.go +++ b/internal/manifest/store.go @@ -1,6 +1,7 @@ package manifest import ( + "bytes" "context" "encoding/json" "fmt" @@ -69,7 +70,16 @@ func (s *LocalStore) LoadReader(ctx context.Context, source io.Reader) (*Manifes } var m Manifest - if err := json.Unmarshal(data, &m); err != nil { + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&m); err != nil { + return nil, fmt.Errorf("decode manifest: %w", err) + } + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + if err == nil { + return nil, fmt.Errorf("decode manifest: multiple JSON values are not allowed") + } return nil, fmt.Errorf("decode manifest: %w", err) } @@ -104,9 +114,6 @@ func (s *LocalStore) Save(ctx context.Context, path string, m *Manifest) error { if err := validateManifestIdentities(m.SessionID, m.Campaign, m.RunID); err != nil { return fmt.Errorf("save manifest: %w", err) } - if err := validatePostPublishCleanup(m.PostPublishCleanup); err != nil { - return fmt.Errorf("save manifest: %w", err) - } if err := validateSessionStages(m.Stages); err != nil { return fmt.Errorf("save manifest: %w", err) } @@ -149,9 +156,6 @@ func validateLoadedManifest(m *Manifest) error { if err := validateManifestIdentities(m.SessionID, m.Campaign, m.RunID); err != nil { return err } - if err := validatePostPublishCleanup(m.PostPublishCleanup); err != nil { - return err - } if err := validateSessionStages(m.Stages); err != nil { return err } @@ -175,27 +179,6 @@ func normalizeManifest(m *Manifest) { } } -func validatePostPublishCleanup(cleanup *PostPublishCleanup) error { - if cleanup == nil { - return nil - } - if err := pathsafe.ValidateOpaqueSegment(cleanup.CommittedRunID); err != nil { - return fmt.Errorf("post_publish_cleanup.committed_run_id is not a portable opaque identifier: %w", err) - } - if strings.TrimSpace(cleanup.RemoteCommitKey) == "" || strings.TrimSpace(cleanup.CurrentCommitPointerKey) == "" { - return fmt.Errorf("post_publish_cleanup commit identity is required") - } - if len(cleanup.Targets) == 0 { - return fmt.Errorf("post_publish_cleanup.targets is required") - } - for index, target := range cleanup.Targets { - if strings.TrimSpace(target.Policy) == "" || strings.TrimSpace(target.Root) == "" || strings.TrimSpace(target.Path) == "" { - return fmt.Errorf("post_publish_cleanup.targets[%d] policy, root, and path are required", index) - } - } - return nil -} - func validateManifestIdentities(sessionID, campaign, runID string) error { identities := []struct { field string diff --git a/internal/manifest/store_test.go b/internal/manifest/store_test.go index 1d6a0ec..efed254 100644 --- a/internal/manifest/store_test.go +++ b/internal/manifest/store_test.go @@ -6,6 +6,7 @@ import ( "errors" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -256,6 +257,33 @@ func TestLoadRejectsInvalidManifest(t *testing.T) { } } +func TestLocalStoreRejectsRetiredCleanupFieldAndRoundTripsCanonicalV1(t *testing.T) { + store := &LocalStore{} + canonical := `{"schema_version":"narratio.session-manifest.v1","session_id":"2026-05-03","created_at":"2026-05-03T10:00:00Z","updated_at":"2026-05-03T10:01:00Z","stages":{}}` + + loaded, err := store.LoadReader(context.Background(), strings.NewReader(canonical)) + if err != nil { + t.Fatalf("load canonical manifest: %v", err) + } + encoded, err := json.Marshal(loaded) + if err != nil { + t.Fatalf("marshal canonical manifest: %v", err) + } + roundTripped, err := store.LoadReader(context.Background(), strings.NewReader(string(encoded))) + if err != nil { + t.Fatalf("reload canonical manifest: %v", err) + } + if !reflect.DeepEqual(roundTripped, loaded) { + t.Fatalf("round-tripped manifest = %#v, want %#v", roundTripped, loaded) + } + + retired := strings.TrimSuffix(canonical, "}") + `,"post_publish_cleanup":{"committed_run_id":"20260905T010203Z-a1b2c3d4"}}` + _, err = store.LoadReader(context.Background(), strings.NewReader(retired)) + if err == nil || !strings.Contains(err.Error(), `unknown field "post_publish_cleanup"`) { + t.Fatalf("load retired cleanup field error = %v, want strict unknown-field rejection", err) + } +} + func TestLocalStoreRejectsUnsupportedSchemaVersions(t *testing.T) { store := &LocalStore{} now := time.Date(2026, 5, 3, 10, 0, 0, 0, time.UTC)