From 39714438317df9734af931ebb9af6c0d246f075c Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 23 May 2026 13:56:53 +0000 Subject: [PATCH] Centralize remote current-state loading and preserve caller policy --- internal/app/operator_helpers.go | 19 +- internal/app/operator_helpers_test.go | 69 +++++++ internal/app/restore_discovery.go | 77 +------ internal/app/restore_discovery_test.go | 4 +- internal/artifacts/current_state.go | 203 +++++++++++++++++++ internal/artifacts/current_state_test.go | 140 +++++++++++++ internal/previouscache/previouscache.go | 94 ++------- internal/previouscache/previouscache_test.go | 62 +++++- 8 files changed, 511 insertions(+), 157 deletions(-) create mode 100644 internal/artifacts/current_state.go create mode 100644 internal/artifacts/current_state_test.go diff --git a/internal/app/operator_helpers.go b/internal/app/operator_helpers.go index 0050942..37efb08 100644 --- a/internal/app/operator_helpers.go +++ b/internal/app/operator_helpers.go @@ -870,17 +870,14 @@ func validateRemoteAudioFinding(ctx context.Context, cfg *config.Config, store s func validatePreviousArtifactFindings(ctx context.Context, cfg *config.Config, store storage.ObjectStore, requirements []artifacts.PreviousArtifactRequirement) []finding { out := []finding{} prefix := artifacts.S3SessionPrefix(cfg.Pipeline.Storage.S3.RootPrefix, cfg.Session.Campaign, cfg.Session.PreviousSessionID) - manifestKey, runIDKey := artifacts.ResolveCurrentStateKeys(prefix) - for _, key := range []string{runIDKey, manifestKey} { - exists, err := store.Exists(ctx, key) - if err != nil { - out = append(out, errorFinding("previous", fmt.Sprintf("check %s: %v", key, err))) - return out - } - if !exists { - out = append(out, errorFinding("previous", "missing "+key)) - return out - } + _, err := artifacts.LoadCurrentState(ctx, store, prefix, artifacts.CurrentStateValidation{ + ExpectedSessionID: strings.TrimSpace(cfg.Session.PreviousSessionID), + ExpectedCampaign: strings.TrimSpace(cfg.Session.Campaign), + ValidateRunID: true, + }) + if err != nil { + out = append(out, errorFinding("previous", fmt.Sprintf("remote %v", err))) + return out } for _, req := range requirements { out = append(out, okFinding("previous", fmt.Sprintf("%s required=%t", req.Name, req.Required))) diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index 062d714..6dd254e 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -861,6 +861,60 @@ func TestExecuteStatusReportsRemoteArtifactCatalogErrorsWithoutFailing(t *testin } } +func TestExecuteStatusReportsMissingRemoteCurrentStateWithoutFailing(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) + fake := &storage.FakeBackend{} + var storeInitCalls int + restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "status", "2026-05-03", + "--config", pipelinePath, + "--campaign-file", campaignPath, + "--session", sessionPath, + }, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + if !strings.Contains(stdout.String(), "Remote publish: missing or unavailable: remote current run pointer missing") { + t.Fatalf("stdout = %q, want missing remote current-state line", stdout.String()) + } +} + +func TestExecuteSessionValidateReportsPreviousStateFindingAndReturnsFindingError(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, sessionPath := writeValidConfigFilesWithScriptoriumArtifacts(t, workspaceRoot) + replaceInFileOrFatal(t, pipelinePath, "source: narratio.artifact.session_recap", "source: narratio.previous_session.artifact.session_recap") + replaceInFileOrFatal(t, sessionPath, "session_id: 2026-05-03\n", "session_id: 2026-05-03\nprevious_session_id: 2026-04-26\n") + fake := &storage.FakeBackend{} + var storeInitCalls int + restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "session", "validate", "2026-05-03", + "--config", pipelinePath, + "--campaign-file", campaignPath, + "--session", sessionPath, + }, &stdout, &stderr) + if code == 0 { + t.Fatal("exit code = 0, want non-zero") + } + if !strings.Contains(stdout.String(), "ERROR previous") { + t.Fatalf("stdout = %q, want previous finding error", stdout.String()) + } + if !strings.Contains(stdout.String(), "remote current run pointer missing") { + t.Fatalf("stdout = %q, want missing run pointer finding", stdout.String()) + } + if !strings.Contains(stderr.String(), "validation error(s)") { + t.Fatalf("stderr = %q, want finding error summary", stderr.String()) + } +} + func TestExecutePublishLoadsRemoteLocks(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidPublishRunConfigFiles(t, workspaceRoot) @@ -904,6 +958,21 @@ func addPublishOutputsToPipeline(t *testing.T, pipelinePath, publishYAML string) } } +func replaceInFileOrFatal(t *testing.T, path, old, new string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + updated := strings.Replace(string(data), old, new, 1) + if updated == string(data) { + t.Fatalf("%s did not contain %q", path, old) + } + if err := os.WriteFile(path, []byte(updated), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + func writeValidPublishRunConfigFiles(t *testing.T, workspaceRoot string) (string, string, string) { t.Helper() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) diff --git a/internal/app/restore_discovery.go b/internal/app/restore_discovery.go index 4e590cd..b08d080 100644 --- a/internal/app/restore_discovery.go +++ b/internal/app/restore_discovery.go @@ -3,7 +3,6 @@ package app import ( "context" "fmt" - "os" "strings" "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" @@ -42,70 +41,14 @@ func discoverRemoteCurrentState(ctx context.Context, cfg *config.Config, store s } currentManifestKey, currentRunIDKey := artifacts.ResolveCurrentStateKeys(sessionPrefix) - exists, err := store.Exists(ctx, currentRunIDKey) - if err != nil { - return nil, fmt.Errorf("check remote current run pointer %q: %w", currentRunIDKey, err) - } - if !exists { - return nil, fmt.Errorf("remote current run pointer missing: %q", currentRunIDKey) - } - - runIDPath, err := storage.DownloadObjectToTemp(ctx, store, currentRunIDKey, "narratio-restore-current-run-id-*.txt") - if err != nil { - return nil, fmt.Errorf("download remote current run pointer %q: %w", currentRunIDKey, err) - } - defer func() { _ = os.Remove(runIDPath) }() - - runIDData, err := os.ReadFile(runIDPath) - if err != nil { - return nil, fmt.Errorf("read downloaded run pointer %q: %w", currentRunIDKey, err) - } - runID := strings.TrimSpace(string(runIDData)) - if runID == "" { - return nil, fmt.Errorf("remote current run pointer %q is empty", currentRunIDKey) - } - - exists, err = store.Exists(ctx, currentManifestKey) - if err != nil { - return nil, fmt.Errorf("check remote current manifest %q: %w", currentManifestKey, err) - } - if !exists { - return nil, fmt.Errorf("remote current manifest missing: %q", currentManifestKey) - } - - manifestPath, err := storage.DownloadObjectToTemp(ctx, store, currentManifestKey, "narratio-restore-current-manifest-*.json") - if err != nil { - return nil, fmt.Errorf("download remote current manifest %q: %w", currentManifestKey, err) - } - defer func() { _ = os.Remove(manifestPath) }() - - manifestStore := &manifest.LocalStore{} - remoteManifest, err := manifestStore.Load(ctx, manifestPath) - if err != nil { - return nil, fmt.Errorf("remote current manifest decode failed: %w", err) - } - requestedSession := strings.TrimSpace(cfg.Session.SessionID) requestedCampaign := strings.TrimSpace(cfg.Session.Campaign) - manifestSession := strings.TrimSpace(remoteManifest.SessionID) - manifestCampaign := strings.TrimSpace(remoteManifest.Campaign) - - if manifestSession != requestedSession { - return nil, fmt.Errorf( - "remote current manifest session_id %q does not match requested session_id %q", - manifestSession, - requestedSession, - ) - } - if manifestCampaign == "" { - return nil, fmt.Errorf("remote current manifest campaign is required") - } - if manifestCampaign != requestedCampaign { - return nil, fmt.Errorf( - "remote current manifest campaign %q does not match requested campaign %q", - manifestCampaign, - requestedCampaign, - ) + current, err := artifacts.LoadCurrentState(ctx, store, sessionPrefix, artifacts.CurrentStateValidation{ + ExpectedSessionID: requestedSession, + ExpectedCampaign: requestedCampaign, + }) + if err != nil { + return nil, fmt.Errorf("remote %w", err) } return &RemoteCurrentState{ @@ -113,9 +56,9 @@ func discoverRemoteCurrentState(ctx context.Context, cfg *config.Config, store s SessionPrefix: sessionPrefix, CurrentRunIDKey: currentRunIDKey, CurrentManifestKey: currentManifestKey, - RunID: runID, - SessionID: manifestSession, - Campaign: manifestCampaign, - Manifest: remoteManifest, + RunID: current.RunID, + SessionID: strings.TrimSpace(current.Manifest.SessionID), + Campaign: strings.TrimSpace(current.Manifest.Campaign), + Manifest: current.Manifest, }, nil } diff --git a/internal/app/restore_discovery_test.go b/internal/app/restore_discovery_test.go index 4ea32c9..1e36005 100644 --- a/internal/app/restore_discovery_test.go +++ b/internal/app/restore_discovery_test.go @@ -102,7 +102,7 @@ func TestDiscoverRemoteCurrentStateSessionMismatchFails(t *testing.T) { store.SeedObject(storage.FakeObject{Key: manifestKey, Data: restoreManifestJSON(t, "wrong-session", cfg.Session.Campaign)}) _, err := discoverRemoteCurrentState(context.Background(), cfg, store) - if err == nil || !strings.Contains(err.Error(), "does not match requested session_id") { + if err == nil || !strings.Contains(err.Error(), "does not match expected session_id") { t.Fatalf("error = %v, want session mismatch failure", err) } } @@ -116,7 +116,7 @@ func TestDiscoverRemoteCurrentStateCampaignMismatchFails(t *testing.T) { store.SeedObject(storage.FakeObject{Key: manifestKey, Data: restoreManifestJSON(t, cfg.Session.SessionID, "wrong-campaign")}) _, err := discoverRemoteCurrentState(context.Background(), cfg, store) - if err == nil || !strings.Contains(err.Error(), "does not match requested campaign") { + if err == nil || !strings.Contains(err.Error(), "does not match expected campaign") { t.Fatalf("error = %v, want campaign mismatch failure", err) } } diff --git a/internal/artifacts/current_state.go b/internal/artifacts/current_state.go new file mode 100644 index 0000000..3c81418 --- /dev/null +++ b/internal/artifacts/current_state.go @@ -0,0 +1,203 @@ +package artifacts + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + + "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" + "gitea.maximumdirect.net/eric/narratio/internal/manifest" +) + +var ( + ErrCurrentRunPointerMissing = errors.New("current run pointer missing") + ErrCurrentManifestMissing = errors.New("current manifest missing") +) + +type CurrentRunPointerMissingError struct { + Key string +} + +func (e *CurrentRunPointerMissingError) Error() string { + return fmt.Sprintf("%s: %q", ErrCurrentRunPointerMissing, e.Key) +} + +func (e *CurrentRunPointerMissingError) Unwrap() error { + return ErrCurrentRunPointerMissing +} + +type CurrentManifestMissingError struct { + Key string +} + +func (e *CurrentManifestMissingError) Error() string { + return fmt.Sprintf("%s: %q", ErrCurrentManifestMissing, e.Key) +} + +func (e *CurrentManifestMissingError) Unwrap() error { + return ErrCurrentManifestMissing +} + +type CurrentState struct { + SessionPrefix string + CurrentRunIDKey string + CurrentManifestKey string + RunID string + Manifest *manifest.Manifest +} + +type CurrentStateValidation struct { + ExpectedCampaign string + ExpectedSessionID string + ExpectedRunID string + ValidateRunID bool +} + +func LoadCurrentRunPointer(ctx context.Context, store storage.ObjectStore, currentRunIDKey string) (string, error) { + if store == nil { + return "", fmt.Errorf("object store is required") + } + key := strings.TrimSpace(currentRunIDKey) + if key == "" { + return "", fmt.Errorf("current run pointer key is required") + } + + exists, err := store.Exists(ctx, key) + if err != nil { + return "", fmt.Errorf("check current run pointer %q: %w", key, err) + } + if !exists { + return "", &CurrentRunPointerMissingError{Key: key} + } + + localPath, err := storage.DownloadObjectToTemp(ctx, store, key, "narratio-current-run-id-*.txt") + if err != nil { + return "", fmt.Errorf("download current run pointer %q: %w", key, err) + } + defer func() { _ = os.Remove(localPath) }() + + data, err := os.ReadFile(localPath) + if err != nil { + return "", fmt.Errorf("read downloaded current run pointer %q: %w", key, err) + } + runID := strings.TrimSpace(string(data)) + if runID == "" { + return "", fmt.Errorf("current run pointer %q is empty", key) + } + return runID, nil +} + +func LoadCurrentManifest(ctx context.Context, store storage.ObjectStore, currentManifestKey string) (*manifest.Manifest, error) { + if store == nil { + return nil, fmt.Errorf("object store is required") + } + key := strings.TrimSpace(currentManifestKey) + if key == "" { + return nil, fmt.Errorf("current manifest key is required") + } + + exists, err := store.Exists(ctx, key) + if err != nil { + return nil, fmt.Errorf("check current manifest %q: %w", key, err) + } + if !exists { + return nil, &CurrentManifestMissingError{Key: key} + } + + localPath, err := storage.DownloadObjectToTemp(ctx, store, key, "narratio-current-manifest-*.json") + if err != nil { + return nil, fmt.Errorf("download current manifest %q: %w", key, err) + } + defer func() { _ = os.Remove(localPath) }() + + manifestStore := &manifest.LocalStore{} + m, err := manifestStore.Load(ctx, localPath) + if err != nil { + return nil, fmt.Errorf("current manifest decode failed: %w", err) + } + return m, nil +} + +func LoadCurrentState( + ctx context.Context, + store storage.ObjectStore, + sessionPrefix string, + validation CurrentStateValidation, +) (*CurrentState, error) { + prefix := strings.TrimSpace(sessionPrefix) + if prefix == "" { + return nil, fmt.Errorf("session prefix is required") + } + currentManifestKey, currentRunIDKey := ResolveCurrentStateKeys(prefix) + runID, err := LoadCurrentRunPointer(ctx, store, currentRunIDKey) + if err != nil { + return nil, err + } + m, err := LoadCurrentManifest(ctx, store, currentManifestKey) + if err != nil { + return nil, err + } + + state := &CurrentState{ + SessionPrefix: prefix, + CurrentRunIDKey: currentRunIDKey, + CurrentManifestKey: currentManifestKey, + RunID: runID, + Manifest: m, + } + if err := ValidateCurrentStateIdentity(state, validation); err != nil { + return nil, err + } + return state, nil +} + +func ValidateCurrentStateIdentity(state *CurrentState, validation CurrentStateValidation) error { + if state == nil || state.Manifest == nil { + return fmt.Errorf("current state with manifest is required") + } + expectedSessionID := strings.TrimSpace(validation.ExpectedSessionID) + expectedCampaign := strings.TrimSpace(validation.ExpectedCampaign) + expectedRunID := strings.TrimSpace(validation.ExpectedRunID) + manifestSessionID := strings.TrimSpace(state.Manifest.SessionID) + manifestCampaign := strings.TrimSpace(state.Manifest.Campaign) + manifestRunID := strings.TrimSpace(state.Manifest.RunID) + + if expectedSessionID != "" && manifestSessionID != expectedSessionID { + return fmt.Errorf( + "current manifest session_id %q does not match expected session_id %q", + manifestSessionID, + expectedSessionID, + ) + } + if expectedCampaign != "" { + if manifestCampaign == "" { + return fmt.Errorf("current manifest campaign is required") + } + if manifestCampaign != expectedCampaign { + return fmt.Errorf( + "current manifest campaign %q does not match expected campaign %q", + manifestCampaign, + expectedCampaign, + ) + } + } + if expectedRunID == "" && validation.ValidateRunID { + expectedRunID = strings.TrimSpace(state.RunID) + } + if expectedRunID != "" { + if manifestRunID == "" { + return fmt.Errorf("current manifest run_id is required") + } + if manifestRunID != expectedRunID { + return fmt.Errorf( + "current run pointer %q references run %q but current manifest run_id is %q", + state.CurrentRunIDKey, + expectedRunID, + manifestRunID, + ) + } + } + return nil +} diff --git a/internal/artifacts/current_state_test.go b/internal/artifacts/current_state_test.go new file mode 100644 index 0000000..8eded76 --- /dev/null +++ b/internal/artifacts/current_state_test.go @@ -0,0 +1,140 @@ +package artifacts + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + "time" + + "gitea.maximumdirect.net/eric/narratio/internal/adapters/storage" +) + +func TestLoadCurrentStateMissingRunPointer(t *testing.T) { + store := &storage.FakeBackend{} + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{}) + if err == nil { + t.Fatal("LoadCurrentState() error = nil, want missing run pointer error") + } + var missing *CurrentRunPointerMissingError + if !errors.As(err, &missing) { + t.Fatalf("errors.As(err, *CurrentRunPointerMissingError) = false; err=%v", err) + } + if !errors.Is(err, ErrCurrentRunPointerMissing) { + t.Fatalf("errors.Is(err, ErrCurrentRunPointerMissing) = false; err=%v", err) + } +} + +func TestLoadCurrentStateMissingManifest(t *testing.T) { + store := &storage.FakeBackend{} + _, _, runIDKey := testCurrentStateKeys() + store.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("20260519T010203Z-a1b2c3d4\n")}) + + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{}) + if err == nil { + t.Fatal("LoadCurrentState() error = nil, want missing manifest error") + } + var missing *CurrentManifestMissingError + if !errors.As(err, &missing) { + t.Fatalf("errors.As(err, *CurrentManifestMissingError) = false; err=%v", err) + } + if !errors.Is(err, ErrCurrentManifestMissing) { + t.Fatalf("errors.Is(err, ErrCurrentManifestMissing) = false; err=%v", err) + } +} + +func TestLoadCurrentStateEmptyRunPointerFails(t *testing.T) { + store := &storage.FakeBackend{} + _, manifestKey, runIDKey := testCurrentStateKeys() + store.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte(" \n\t")}) + store.SeedObject(storage.FakeObject{Key: manifestKey, Data: testManifestJSON(t, "2026-05-03", "sample-campaign", "20260519T010203Z-a1b2c3d4")}) + + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{}) + if err == nil || !strings.Contains(err.Error(), "is empty") { + t.Fatalf("error = %v, want empty run pointer failure", err) + } +} + +func TestLoadCurrentStateMalformedManifestFails(t *testing.T) { + store := &storage.FakeBackend{} + _, manifestKey, runIDKey := testCurrentStateKeys() + store.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("20260519T010203Z-a1b2c3d4\n")}) + store.SeedObject(storage.FakeObject{Key: manifestKey, Data: []byte("{invalid json")}) + + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{}) + if err == nil || !strings.Contains(err.Error(), "current manifest decode failed") { + t.Fatalf("error = %v, want manifest decode failure", err) + } +} + +func TestLoadCurrentStateCampaignMismatchFails(t *testing.T) { + store := &storage.FakeBackend{} + seedCurrentState(t, store, "2026-05-03", "wrong-campaign", "20260519T010203Z-a1b2c3d4") + + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{ + ExpectedCampaign: "sample-campaign", + }) + if err == nil || !strings.Contains(err.Error(), "does not match expected campaign") { + t.Fatalf("error = %v, want campaign mismatch failure", err) + } +} + +func TestLoadCurrentStateSessionMismatchFails(t *testing.T) { + store := &storage.FakeBackend{} + seedCurrentState(t, store, "wrong-session", "sample-campaign", "20260519T010203Z-a1b2c3d4") + + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{ + ExpectedSessionID: "2026-05-03", + }) + if err == nil || !strings.Contains(err.Error(), "does not match expected session_id") { + t.Fatalf("error = %v, want session mismatch failure", err) + } +} + +func TestLoadCurrentStateRunIDMismatchFails(t *testing.T) { + store := &storage.FakeBackend{} + seedCurrentState(t, store, "2026-05-03", "sample-campaign", "different-run-id") + + _, err := LoadCurrentState(context.Background(), store, testCurrentSessionPrefix(), CurrentStateValidation{ + ValidateRunID: true, + }) + if err == nil || !strings.Contains(err.Error(), "current manifest run_id") { + t.Fatalf("error = %v, want run mismatch failure", err) + } +} + +func testCurrentSessionPrefix() string { + return S3SessionPrefix("dnd", "sample-campaign", "2026-05-03") +} + +func testCurrentStateKeys() (sessionPrefix, manifestKey, runIDKey string) { + sessionPrefix = testCurrentSessionPrefix() + manifestKey, runIDKey = ResolveCurrentStateKeys(sessionPrefix) + return sessionPrefix, manifestKey, runIDKey +} + +func seedCurrentState(t *testing.T, store *storage.FakeBackend, sessionID, campaign, manifestRunID string) { + t.Helper() + _, manifestKey, runIDKey := testCurrentStateKeys() + store.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("20260519T010203Z-a1b2c3d4\n")}) + store.SeedObject(storage.FakeObject{Key: manifestKey, Data: testManifestJSON(t, sessionID, campaign, manifestRunID)}) +} + +func testManifestJSON(t *testing.T, sessionID, campaign, runID string) []byte { + t.Helper() + now := time.Date(2026, 5, 19, 23, 0, 0, 0, time.UTC).Format(time.RFC3339Nano) + payload := map[string]any{ + "session_id": sessionID, + "campaign": campaign, + "run_id": runID, + "created_at": now, + "updated_at": now, + "stages": map[string]any{}, + } + data, err := json.Marshal(payload) + if err != nil { + t.Fatalf("marshal manifest payload: %v", err) + } + return append(data, '\n') +} diff --git a/internal/previouscache/previouscache.go b/internal/previouscache/previouscache.go index cba048f..bd0549a 100644 --- a/internal/previouscache/previouscache.go +++ b/internal/previouscache/previouscache.go @@ -2,8 +2,8 @@ package previouscache import ( "context" + "errors" "fmt" - "os" "path" "path/filepath" "sort" @@ -92,86 +92,28 @@ func BuildPlan( campaign, previousSessionID, ) - currentManifestKey, currentRunIDKey := artifacts.ResolveCurrentStateKeys(previousSessionPrefix) result := &Plan{} - - runPointerExists, err := store.Exists(ctx, currentRunIDKey) + current, err := artifacts.LoadCurrentState(ctx, store, previousSessionPrefix, artifacts.CurrentStateValidation{ + ExpectedSessionID: previousSessionID, + ExpectedCampaign: campaign, + ValidateRunID: true, + }) if err != nil { - return nil, fmt.Errorf("check previous-session current run pointer %q: %w", currentRunIDKey, err) - } - if !runPointerExists { - if len(requiredNames) > 0 { - return nil, fmt.Errorf("required previous-session artifacts unavailable: remote current run pointer missing: %q", currentRunIDKey) + var runPointerMissing *artifacts.CurrentRunPointerMissingError + var manifestMissing *artifacts.CurrentManifestMissingError + if errors.As(err, &runPointerMissing) || errors.As(err, &manifestMissing) { + if len(requiredNames) > 0 { + return nil, fmt.Errorf("required previous-session artifacts unavailable: remote %w", err) + } + result.SkippedMissing = optionalNames + return result, nil } - result.SkippedMissing = optionalNames - return result, nil - } - - runIDTemp, err := storage.DownloadObjectToTemp(ctx, store, currentRunIDKey, "narratio-previous-run-id-*.txt") - if err != nil { - return nil, fmt.Errorf("download previous-session current run pointer %q: %w", currentRunIDKey, err) - } - defer func() { _ = os.Remove(runIDTemp) }() - - runIDBytes, err := os.ReadFile(runIDTemp) - if err != nil { - return nil, fmt.Errorf("read previous-session current run pointer %q: %w", currentRunIDKey, err) - } - previousRunID := strings.TrimSpace(string(runIDBytes)) - if previousRunID == "" { - return nil, fmt.Errorf("previous-session current run pointer %q is empty", currentRunIDKey) - } - result.PreviousRunID = previousRunID - - manifestExists, err := store.Exists(ctx, currentManifestKey) - if err != nil { - return nil, fmt.Errorf("check previous-session current manifest %q: %w", currentManifestKey, err) - } - if !manifestExists { - if len(requiredNames) > 0 { - return nil, fmt.Errorf("required previous-session artifacts unavailable: remote current manifest missing: %q", currentManifestKey) - } - result.SkippedMissing = optionalNames - return result, nil - } - - manifestTemp, err := storage.DownloadObjectToTemp(ctx, store, currentManifestKey, "narratio-previous-manifest-*.json") - if err != nil { - return nil, fmt.Errorf("download previous-session current manifest %q: %w", currentManifestKey, err) - } - defer func() { _ = os.Remove(manifestTemp) }() - - manifestStore := &manifest.LocalStore{} - previousManifest, err := manifestStore.Load(ctx, manifestTemp) - if err != nil { - return nil, fmt.Errorf("decode downloaded previous-session manifest %q: %w", currentManifestKey, err) - } - if strings.TrimSpace(previousManifest.SessionID) != previousSessionID { - return nil, fmt.Errorf( - "previous-session manifest session_id %q does not match configured previous_session_id %q", - strings.TrimSpace(previousManifest.SessionID), - previousSessionID, - ) - } - if strings.TrimSpace(previousManifest.Campaign) != campaign { - return nil, fmt.Errorf( - "previous-session manifest campaign %q does not match current campaign %q", - strings.TrimSpace(previousManifest.Campaign), - campaign, - ) - } - if strings.TrimSpace(previousManifest.RunID) == "" { - return nil, fmt.Errorf("previous-session manifest run_id is required") - } - if strings.TrimSpace(previousManifest.RunID) != previousRunID { - return nil, fmt.Errorf( - "previous-session current run pointer %q references run %q but current manifest run_id is %q", - currentRunIDKey, - previousRunID, - strings.TrimSpace(previousManifest.RunID), - ) + return nil, fmt.Errorf("load previous-session current state: %w", err) } + result.PreviousRunID = strings.TrimSpace(current.RunID) + currentManifestKey := current.CurrentManifestKey + previousManifest := current.Manifest manifestRel, err := relativeToSession(paths, paths.PreviousManifestPath) if err != nil { diff --git a/internal/previouscache/previouscache_test.go b/internal/previouscache/previouscache_test.go index 0a44e17..fd504e1 100644 --- a/internal/previouscache/previouscache_test.go +++ b/internal/previouscache/previouscache_test.go @@ -100,11 +100,71 @@ func TestBuildPlanValidatesPreviousManifestIdentity(t *testing.T) { _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: true}, }, store) - if err == nil || !strings.Contains(err.Error(), "does not match configured previous_session_id") { + if err == nil || !strings.Contains(err.Error(), "does not match expected session_id") { t.Fatalf("BuildPlan() error = %v, want identity validation error", err) } } +func TestBuildPlanMissingCurrentRunPointerSkipsOptionalRequirements(t *testing.T) { + cfg, paths := previousCacheTestConfig(t) + store := &storage.FakeBackend{} + + plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + {Name: "session_recap", Required: false}, + }, store) + if err != nil { + t.Fatalf("BuildPlan() error = %v", err) + } + if strings.Join(plan.SkippedMissing, ",") != "session_recap" { + t.Fatalf("SkippedMissing = %#v, want session_recap", plan.SkippedMissing) + } +} + +func TestBuildPlanMissingCurrentRunPointerFailsRequiredRequirements(t *testing.T) { + cfg, paths := previousCacheTestConfig(t) + store := &storage.FakeBackend{} + + _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + {Name: "session_recap", Required: true}, + }, store) + if err == nil || !strings.Contains(err.Error(), `required previous-session artifacts unavailable: remote current run pointer missing`) { + t.Fatalf("BuildPlan() error = %v, want required missing run pointer error", err) + } +} + +func TestBuildPlanMissingCurrentManifestSkipsOptionalRequirements(t *testing.T) { + cfg, paths := previousCacheTestConfig(t) + store := &storage.FakeBackend{} + previousPrefix := artifacts.S3SessionPrefix("dnd", cfg.Session.Campaign, cfg.Session.PreviousSessionID) + _, runIDKey := artifacts.ResolveCurrentStateKeys(previousPrefix) + store.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("previous-run\n")}) + + plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + {Name: "session_recap", Required: false}, + }, store) + if err != nil { + t.Fatalf("BuildPlan() error = %v", err) + } + if strings.Join(plan.SkippedMissing, ",") != "session_recap" { + t.Fatalf("SkippedMissing = %#v, want session_recap", plan.SkippedMissing) + } +} + +func TestBuildPlanMissingCurrentManifestFailsRequiredRequirements(t *testing.T) { + cfg, paths := previousCacheTestConfig(t) + store := &storage.FakeBackend{} + previousPrefix := artifacts.S3SessionPrefix("dnd", cfg.Session.Campaign, cfg.Session.PreviousSessionID) + _, runIDKey := artifacts.ResolveCurrentStateKeys(previousPrefix) + store.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("previous-run\n")}) + + _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + {Name: "session_recap", Required: true}, + }, store) + if err == nil || !strings.Contains(err.Error(), `required previous-session artifacts unavailable: remote current manifest missing`) { + t.Fatalf("BuildPlan() error = %v, want required missing manifest error", err) + } +} + func previousCacheTestConfig(t *testing.T) (*config.Config, artifacts.SessionPaths) { t.Helper() workspaceRoot := t.TempDir()