From 62b1f9b0ae3ed2f13a6bef4b83fd0df995c1146b Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 2 Sep 2026 13:53:23 +0000 Subject: [PATCH] Resolve previous artifacts from commit declarations --- docs/internal/artifacts.md | 6 +- docs/internal/command-restore.md | 9 +- docs/internal/stage-prepare.md | 6 +- docs/operations.md | 3 +- docs/roadmap/implementation.md | 2 +- docs/troubleshooting.md | 4 +- internal/adapters/storage/fake.go | 15 +- internal/app/operator_helpers_test.go | 2 +- internal/app/restore_execution_test.go | 4 +- internal/app/restore_plan_test.go | 8 +- internal/previouscache/previouscache.go | 286 +++---------------- internal/previouscache/previouscache_test.go | 89 +++--- internal/stage/prepare_previous_test.go | 4 +- 13 files changed, 127 insertions(+), 311 deletions(-) diff --git a/docs/internal/artifacts.md b/docs/internal/artifacts.md index 0d04525..12b48ae 100644 --- a/docs/internal/artifacts.md +++ b/docs/internal/artifacts.md @@ -136,8 +136,10 @@ Extraction sources (`narratio.extraction.*`): Previous-session sources (`narratio.previous_session.artifact.*`): - resolve only from local `previous/` cache state; -- prefer manifest-backed previous-input paths; -- fallback to existing previous-cache filesystem paths. +- are hydrated only from the unique `published_output` declaration for their + canonical source in the selected remote commit; and +- never infer a path from a previous manifest, current configuration, basename, + or existing cache file. Source absence is evaluated by the consuming artifact input. An optional input is omitted from that invocation; a required input fails resolution. This is diff --git a/docs/internal/command-restore.md b/docs/internal/command-restore.md index 3d1dee3..640603d 100644 --- a/docs/internal/command-restore.md +++ b/docs/internal/command-restore.md @@ -34,7 +34,7 @@ Planner behavior: - a new-protocol restore uses only the selected commit's declared artifact set; each action carries that artifact's immutable key, checksum, size, and - generation. Coherent legacy state remains on the isolated compatibility path; + generation; - remote-to-local mapping is traversal-safe; - actions are sorted by local relative path and then remote key; - planning opens the selected session root and inspects every target by its @@ -48,8 +48,11 @@ For a non-dry-run restore, planning/classification happens only after acquiring the session lock. Runner manifest/reuse checks acquire that same lock first. Previous-cache readiness is resolved through `previouscache.Resolve` for restore, -prepare, status, and validation. A committed source is selected only by its -exact source identity; legacy fallback remains isolated and rejects ambiguity. +prepare, status, and validation. Each requirement selects the unique +`published_output` for its canonical source in the pointer-selected commit, +then strips only that run's immutable `published/` prefix for its safe local +cache path. It never probes candidate objects or derives paths from manifests, +configuration, or basenames. ## Execution Contract diff --git a/docs/internal/stage-prepare.md b/docs/internal/stage-prepare.md index 7f9c5d9..9eeff42 100644 --- a/docs/internal/stage-prepare.md +++ b/docs/internal/stage-prepare.md @@ -49,8 +49,10 @@ Materialize canonical current-session inputs before processing stages. - records previous inputs in `manifest.inputs`. Required previous-session inputs fail when unavailable; optional missing inputs -are typed skipped results. Committed sources use their exact source-to-destination -mapping, while the isolated legacy reader rejects ambiguous fallback matches. +are typed skipped results. Each requirement resolves only from its unique +`published_output` declaration in the pointer-selected remote commit. Prepare +strips that selected run's immutable `published/` prefix to derive the local +cache path, then validates the committed checksum, size, and generation. ## Invariants diff --git a/docs/operations.md b/docs/operations.md index 2767f6e..e3982b1 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -436,7 +436,8 @@ Default restore scope: - the committed session manifest and the committed transcript/artifact objects declared by the selected remote commit -- `previous/**` when needed by configured previous-session artifact inputs +- `previous/**` when needed by configured previous-session artifact inputs; + each is selected from its exact committed `published_output` declaration Optional: diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 10ecb55..1825709 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -278,7 +278,7 @@ and reuse. ## Stage 5 — Resolve Previous Artifacts From Commit Declarations -**Status: Pending** +**Status: Completed** ### Goal diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 4dd99e1..ebc03e3 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -494,7 +494,9 @@ Symptom: Likely causes: - missing `session.previous_session_id`; -- previous artifact not restored/published for source session. +- the selected previous-session commit does not declare the required canonical + source as a `published_output`; or +- the declared published object is unavailable or fails identity validation. Diagnostics: diff --git a/internal/adapters/storage/fake.go b/internal/adapters/storage/fake.go index 5f1cc25..b767380 100644 --- a/internal/adapters/storage/fake.go +++ b/internal/adapters/storage/fake.go @@ -19,10 +19,11 @@ import ( type FakeBackend struct { mu sync.RWMutex - Objects map[string]FakeObject - Uploads []FakeUploadCall - Downloads []FakeDownloadCall - Reads []FakeReadCall + Objects map[string]FakeObject + Uploads []FakeUploadCall + Downloads []FakeDownloadCall + Reads []FakeReadCall + ExistsKeys []string ListErr error DownloadErr error @@ -323,8 +324,12 @@ func (f *FakeBackend) Exists(ctx context.Context, key string) (bool, error) { if f.ExistsErr != nil { return false, f.ExistsErr } + normalizedKey := normalizeObjectKey(key) + f.mu.Lock() + f.ExistsKeys = append(f.ExistsKeys, normalizedKey) + f.mu.Unlock() f.mu.RLock() - _, ok := f.Objects[normalizeObjectKey(key)] + _, ok := f.Objects[normalizedKey] f.mu.RUnlock() return ok, nil } diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index 7c1cc84..92f7fe9 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -1230,7 +1230,7 @@ func TestExecuteStatusDetectsMissingRequiredPreviousArtifact(t *testing.T) { if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) } - if !strings.Contains(stdout.String(), `Previous-session artifacts: unavailable: remote required previous-session artifact "session_recap" object missing`) { + if !strings.Contains(stdout.String(), `Previous-session artifacts: unavailable: remote required previous-session artifact "session_recap" is not declared`) { t.Fatalf("stdout = %q, want missing required previous artifact", stdout.String()) } } diff --git a/internal/app/restore_execution_test.go b/internal/app/restore_execution_test.go index 65a86b6..a47b444 100644 --- a/internal/app/restore_execution_test.go +++ b/internal/app/restore_execution_test.go @@ -632,7 +632,9 @@ func seedRestorePreviousCurrent(t *testing.T, fake *storage.FakeBackend, cfg *co t.Helper() seedRestorePreviousCurrentManifestOnly(t, fake, cfg) previousPrefix := artifacts.S3SessionPrefix(cfg.Pipeline.Storage.S3.RootPrefix, cfg.Session.Campaign, cfg.Session.PreviousSessionID) - seedRestoreObject(fake, previousPrefix+"artifacts/session_recap.md", []byte(artifactBody)) + previousRunID := "20260426T010203Z-a1b2c3d4" + key := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(previousPrefix, previousRunID), "published/artifacts/session_recap.md") + seedRestoreObject(fake, key, []byte(artifactBody)) } func seedRestorePreviousCurrentManifestOnly(t *testing.T, fake *storage.FakeBackend, cfg *config.Config) { diff --git a/internal/app/restore_plan_test.go b/internal/app/restore_plan_test.go index 90bccc3..a99c9d4 100644 --- a/internal/app/restore_plan_test.go +++ b/internal/app/restore_plan_test.go @@ -380,15 +380,17 @@ func seedRestoreObject(store *storage.FakeBackend, key string, data []byte) { } destination := key if strings.HasPrefix(key, sessionPrefix) && !strings.HasPrefix(key, runPrefix) { - destination = runPrefix + strings.TrimPrefix(key, sessionPrefix) + relative := strings.TrimPrefix(key, sessionPrefix) + destination = runPrefix + relative store.SeedObject(storage.FakeObject{Key: destination, Data: data}) } typeID := artifacts.RemoteArtifactTypePublishedOutput source := "narratio.test" if destination == artifacts.S3RunSessionManifestKey(sessionPrefix, pointer.RunID) { typeID, source = artifacts.RemoteArtifactTypeSessionManifest, "session.manifest" - } else if relative := strings.TrimPrefix(destination, runPrefix); strings.HasPrefix(relative, "artifacts/") { - name := strings.TrimSuffix(strings.TrimPrefix(relative, "artifacts/"), filepath.Ext(relative)) + } else if relative := strings.TrimPrefix(destination, runPrefix); strings.HasPrefix(relative, "artifacts/") || strings.HasPrefix(relative, "published/artifacts/") { + name := strings.TrimPrefix(relative, "published/") + name = strings.TrimSuffix(strings.TrimPrefix(name, "artifacts/"), filepath.Ext(name)) source = artifacts.ConfiguredArtifactSourceID(name) } artifact := remoteRestoreArtifact(store, typeID, source, destination) diff --git a/internal/previouscache/previouscache.go b/internal/previouscache/previouscache.go index 9f57eed..4a5d4a9 100644 --- a/internal/previouscache/previouscache.go +++ b/internal/previouscache/previouscache.go @@ -12,7 +12,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" "gitea.maximumdirect.net/eric/narratio/internal/artifacts" "gitea.maximumdirect.net/eric/narratio/internal/config" - "gitea.maximumdirect.net/eric/narratio/internal/manifest" "gitea.maximumdirect.net/eric/narratio/internal/pathsafe" ) @@ -118,7 +117,9 @@ func Resolve( } result.PreviousRunID = strings.TrimSpace(current.RunID) currentManifestKey := current.CurrentManifestKey - previousManifest := current.Manifest + if current.Commit == nil { + return nil, fmt.Errorf("previous-session current state does not include a committed remote declaration") + } manifestRel, err := relativeToSession(paths, paths.PreviousManifestPath) if err != nil { @@ -131,64 +132,33 @@ func Resolve( RemoteKey: currentManifestKey, S3Bucket: bucket, } - if current.Commit != nil { - committedManifest, ok := current.Commit.Artifact(artifacts.RemoteArtifactTypeSessionManifest) - if !ok || committedManifest.DestinationKey != currentManifestKey { - return nil, fmt.Errorf("previous-session remote commit does not declare its session manifest") - } - manifestRecord.SHA256 = committedManifest.SHA256 - manifestRecord.Size = committedManifest.Size - manifestRecord.Generation = committedManifest.Generation - manifestRecord.VerifiedContent = append([]byte(nil), current.ManifestData...) + committedManifest, ok := current.Commit.Artifact(artifacts.RemoteArtifactTypeSessionManifest) + if !ok || committedManifest.DestinationKey != currentManifestKey { + return nil, fmt.Errorf("previous-session remote commit does not declare its session manifest") } + manifestRecord.SHA256 = committedManifest.SHA256 + manifestRecord.Size = committedManifest.Size + manifestRecord.Generation = committedManifest.Generation + manifestRecord.VerifiedContent = append([]byte(nil), current.ManifestData...) result.Records = append(result.Records, manifestRecord) for _, requirement := range orderedRequirements { - selectedRel := "" - selectedKey := "" - var selectedArtifact *artifacts.RemoteArtifact - if current.Commit != nil { - sourceID := previousArtifactSourceID(requirement.Name) - artifact, ok := committedPublishedArtifactBySource(current.Commit, sourceID) - if ok { - selectedRel, ok = sourceRelativePath(sourceID, requirement.Name, previousManifest, cfg) - if ok { - selectedKey = artifact.DestinationKey - selectedArtifact = &artifact - } - } - } else { - // Legacy current state predates immutable source-to-destination - // mappings. Keep its compatibility candidates isolated here. - matchedCandidates := make([]string, 0, 1) - for _, candidate := range legacyArtifactRelativePathCandidates(requirement.Name, previousManifest, cfg) { - remoteKey := artifacts.S3PublishedOutputKey(previousSessionPrefix, candidate) - exists, err := store.Exists(ctx, remoteKey) - if err != nil { - return nil, fmt.Errorf("check previous-session artifact object %q: %w", remoteKey, err) - } - if exists { - matchedCandidates = append(matchedCandidates, candidate) - if len(matchedCandidates) == 1 { - selectedRel = candidate - selectedKey = remoteKey - } - } - } - if len(matchedCandidates) > 1 { - return nil, fmt.Errorf("legacy previous-session artifact %q matches multiple published candidates: %s", requirement.Name, strings.Join(matchedCandidates, ", ")) - } - } - if selectedRel == "" { + sourceID := previousArtifactSourceID(requirement.Name) + selectedArtifact, ok := committedPublishedArtifactBySource(current.Commit, sourceID) + if !ok { if requirement.Required { return nil, fmt.Errorf( - "required previous-session artifact %q object missing from published candidate keys", + "required previous-session artifact %q is not declared by the selected remote commit", requirement.Name, ) } result.SkippedMissing = append(result.SkippedMissing, requirement.Name) continue } + selectedRel, err := publishedOutputRelativePath(previousSessionPrefix, current.RunID, selectedArtifact.DestinationKey) + if err != nil { + return nil, fmt.Errorf("resolve declared previous-session artifact %q: %w", requirement.Name, err) + } localPath, err := artifacts.SessionPreviousArtifactPath(paths, selectedRel) if err != nil { @@ -204,13 +174,11 @@ func Resolve( Required: requirement.Required, LocalRelativePath: localRel, LocalPath: localPath, - RemoteKey: selectedKey, + RemoteKey: selectedArtifact.DestinationKey, S3Bucket: bucket, - } - if selectedArtifact != nil { - record.SHA256 = selectedArtifact.SHA256 - record.Size = selectedArtifact.Size - record.Generation = selectedArtifact.Generation + SHA256: selectedArtifact.SHA256, + Size: selectedArtifact.Size, + Generation: selectedArtifact.Generation, } result.Records = append(result.Records, record) } @@ -225,17 +193,6 @@ func Resolve( return result, nil } -// BuildPlan is retained for callers that need the historical name. -func BuildPlan( - ctx context.Context, - cfg *config.Config, - paths artifacts.SessionPaths, - requirements []artifacts.PreviousArtifactRequirement, - store storage.ObjectStore, -) (*Plan, error) { - return Resolve(ctx, cfg, paths, requirements, store) -} - func committedPublishedArtifactBySource(commit *artifacts.RemoteCommitManifest, source string) (artifacts.RemoteArtifact, bool) { if commit == nil { return artifacts.RemoteArtifact{}, false @@ -252,6 +209,19 @@ func committedPublishedArtifactBySource(commit *artifacts.RemoteCommitManifest, return artifacts.RemoteArtifact{}, false } +func publishedOutputRelativePath(sessionPrefix, runID, destinationKey string) (string, error) { + prefix := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(sessionPrefix, runID), "published") + "/" + rel, ok := strings.CutPrefix(strings.TrimSpace(destinationKey), prefix) + if !ok || strings.TrimSpace(rel) == "" { + return "", fmt.Errorf("destination %q is outside selected run published-output prefix %q", destinationKey, prefix) + } + normalized, err := pathsafe.NormalizeRelativeDestination(rel) + if err != nil { + return "", fmt.Errorf("published output destination %q: %w", destinationKey, err) + } + return normalized, nil +} + func requiredPreviousArtifactNames(requirements []artifacts.PreviousArtifactRequirement) []string { names := make([]string, 0, len(requirements)) for _, requirement := range requirements { @@ -283,172 +253,6 @@ func previousArtifactSourceID(artifactName string) string { return sourceID } -func sourceRelativePath(sourceID, artifactName string, previousManifest *manifest.Manifest, cfg *config.Config) (string, bool) { - if rel, ok := manifestArtifactRelativePathBySourceID(previousManifest, sourceID); ok { - return rel, true - } - if cfg != nil && cfg.Pipeline != nil && cfg.Pipeline.Scriptorium != nil { - if artifactCfg, ok := cfg.Pipeline.Scriptorium.Artifacts[artifactName]; ok { - normalized, err := pathsafe.NormalizeRelativeDestination(artifactCfg.OutputPath) - if err == nil { - return normalized, true - } - } - } - return "", false -} - -func legacyArtifactRelativePathCandidates( - artifactName string, - previousManifest *manifest.Manifest, - cfg *config.Config, -) []string { - candidates := []string{} - appendCandidate := func(v string) { - normalized, err := pathsafe.NormalizeRelativeDestination(v) - if err != nil { - return - } - candidates = append(candidates, normalized) - } - - sourceID := previousArtifactSourceID(artifactName) - if rel, ok := manifestArtifactRelativePathBySourceID(previousManifest, sourceID); ok { - appendCandidate(rel) - base := filepath.Base(rel) - for _, published := range manifestPublishedPaths(previousManifest) { - if filepath.Base(published) == base { - appendCandidate(published) - } - } - } - - if cfg != nil && cfg.Pipeline != nil && cfg.Pipeline.Scriptorium != nil { - if artifactCfg, ok := cfg.Pipeline.Scriptorium.Artifacts[artifactName]; ok { - appendCandidate(artifactCfg.OutputPath) - } - } - - return dedupeOrderedStrings(candidates) -} - -func manifestArtifactRelativePathBySourceID(previousManifest *manifest.Manifest, sourceID string) (string, bool) { - if previousManifest == nil || len(previousManifest.Stages) == 0 { - return "", false - } - sourceID = strings.TrimSpace(sourceID) - if sourceID == "" { - return "", false - } - - stageNames := make([]string, 0, len(previousManifest.Stages)) - if _, ok := previousManifest.Stages["analyze"]; ok { - stageNames = append(stageNames, "analyze") - } - for stageName := range previousManifest.Stages { - if stageName == "analyze" { - continue - } - stageNames = append(stageNames, stageName) - } - start := 0 - if len(stageNames) > 0 && stageNames[0] == "analyze" { - start = 1 - } - sort.Strings(stageNames[start:]) - - for _, stageName := range stageNames { - sr := previousManifest.Stages[stageName] - if sr == nil { - continue - } - for _, out := range sr.Outputs { - if strings.TrimSpace(out.SourceID) != sourceID { - continue - } - rel, ok := deriveManifestRelativePath(previousManifest, out.LocalPath) - if ok { - return rel, true - } - } - } - return "", false -} - -func deriveManifestRelativePath(previousManifest *manifest.Manifest, localPath string) (string, bool) { - trimmed := strings.TrimSpace(localPath) - if trimmed == "" { - return "", false - } - if !filepath.IsAbs(trimmed) { - normalized, err := pathsafe.NormalizeRelativeDestination(filepath.ToSlash(trimmed)) - if err != nil { - return "", false - } - return normalized, true - } - - sessionRoot, ok := manifestSessionRoot(previousManifest) - if !ok { - return "", false - } - normalized, err := pathsafe.SlashRelativeFromRoot(sessionRoot, trimmed) - if err != nil { - return "", false - } - return normalized, true -} - -func manifestSessionRoot(previousManifest *manifest.Manifest) (string, bool) { - if previousManifest == nil { - return "", false - } - runRoot := filepath.Clean(strings.TrimSpace(previousManifest.LocalWorkDir)) - runID := strings.TrimSpace(previousManifest.RunID) - if runRoot == "" || runID == "" { - return "", false - } - if filepath.Base(runRoot) != runID { - return "", false - } - runsDir := filepath.Dir(runRoot) - if filepath.Base(runsDir) != config.PathRunsDirSegment { - return "", false - } - return filepath.Dir(runsDir), true -} - -func manifestPublishedPaths(previousManifest *manifest.Manifest) []string { - if previousManifest == nil || len(previousManifest.Stages) == 0 { - return nil - } - sr := previousManifest.Stages["publish"] - if sr == nil || sr.Metadata == nil { - return nil - } - raw, ok := sr.Metadata["published_paths"] - if !ok { - return nil - } - values, ok := raw.([]any) - if !ok { - return nil - } - out := make([]string, 0, len(values)) - for _, value := range values { - asString, ok := value.(string) - if !ok { - continue - } - normalized, err := pathsafe.NormalizeRelativeDestination(asString) - if err != nil { - continue - } - out = append(out, normalized) - } - return dedupeOrderedStrings(out) -} - func relativeToSession(paths artifacts.SessionPaths, localPath string) (string, error) { root := filepath.Clean(paths.Root) if strings.TrimSpace(root) == "" { @@ -460,23 +264,3 @@ func relativeToSession(paths artifacts.SessionPaths, localPath string) (string, } return normalized, nil } - -func dedupeOrderedStrings(values []string) []string { - if len(values) == 0 { - return nil - } - seen := map[string]struct{}{} - out := make([]string, 0, len(values)) - for _, value := range values { - trimmed := strings.TrimSpace(value) - if trimmed == "" { - continue - } - if _, ok := seen[trimmed]; ok { - continue - } - seen[trimmed] = struct{}{} - out = append(out, trimmed) - } - return out -} diff --git a/internal/previouscache/previouscache_test.go b/internal/previouscache/previouscache_test.go index 3a52e7e..a03dc50 100644 --- a/internal/previouscache/previouscache_test.go +++ b/internal/previouscache/previouscache_test.go @@ -16,18 +16,17 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/manifest" ) -func TestBuildPlanResolvesPublishedArtifactFromPreviousManifest(t *testing.T) { +func TestResolveUsesExactCommittedDeclaration(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} seedCommittedPreviousState(t, store, cfg, previousManifestWithOutput(t, cfg, "artifacts/session_recap.md", []string{"artifacts/session_recap.md"}), []artifacts.RemoteArtifact{committedPublishedArtifactForTest("narratio.artifact.session_recap", "artifacts/session_recap.md", []byte("# recap\n"), "recap-generation")}) - previousPrefix := artifacts.S3SessionPrefix("dnd", cfg.Session.Campaign, cfg.Session.PreviousSessionID) - store.SeedObject(storage.FakeObject{Key: previousPrefix + "artifacts/session_recap.md", Data: []byte("# recap\n")}) + store.ListErr = context.Canceled - plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + plan, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: true}, }, store) if err != nil { - t.Fatalf("BuildPlan() error = %v", err) + t.Fatalf("Resolve() error = %v", err) } if plan.PreviousRunID != "previous-run" { t.Fatalf("PreviousRunID = %q, want previous-run", plan.PreviousRunID) @@ -37,20 +36,22 @@ func TestBuildPlanResolvesPublishedArtifactFromPreviousManifest(t *testing.T) { if strings.Join(got, ",") != strings.Join(want, ",") { t.Fatalf("record rel paths = %#v, want %#v", got, want) } + pointerKey := artifacts.S3CurrentCommitPointerKey(artifacts.S3SessionPrefix("dnd", cfg.Session.Campaign, cfg.Session.PreviousSessionID)) + if strings.Join(store.ExistsKeys, ",") != pointerKey { + t.Fatalf("Exists keys = %#v, want only current pointer %q", store.ExistsKeys, pointerKey) + } } -func TestBuildPlanFallsBackToConfiguredOutputPath(t *testing.T) { +func TestResolveDerivesLocalPathFromCommittedDestination(t *testing.T) { cfg, paths := previousCacheTestConfig(t) + cfg.Pipeline.Scriptorium.Artifacts["session_recap"] = config.ScriptoriumArtifactConfig{Enabled: true, OutputPath: "configured/ignored.md"} store := &storage.FakeBackend{} seedCommittedPreviousState(t, store, cfg, previousManifestWithOutput(t, cfg, "", nil), []artifacts.RemoteArtifact{committedPublishedArtifactForTest("narratio.artifact.session_recap", "artifacts/session_recap.md", []byte("# recap\n"), "recap-generation")}) - previousPrefix := artifacts.S3SessionPrefix("dnd", cfg.Session.Campaign, cfg.Session.PreviousSessionID) - store.SeedObject(storage.FakeObject{Key: previousPrefix + "artifacts/session_recap.md", Data: []byte("# recap\n")}) - - plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + plan, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: true}, }, store) if err != nil { - t.Fatalf("BuildPlan() error = %v", err) + t.Fatalf("Resolve() error = %v", err) } if len(plan.Records) != 2 { t.Fatalf("records len = %d, want 2", len(plan.Records)) @@ -60,6 +61,18 @@ func TestBuildPlanFallsBackToConfiguredOutputPath(t *testing.T) { } } +func TestPublishedOutputRelativePathRejectsUnsafeDestination(t *testing.T) { + sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-04-26") + _, err := publishedOutputRelativePath(sessionPrefix, "previous-run", artifacts.S3RunPrefix(sessionPrefix, "previous-run")+"artifacts/session_recap.md") + if err == nil || !strings.Contains(err.Error(), "outside selected run published-output prefix") { + t.Fatalf("publishedOutputRelativePath() error = %v, want prefix rejection", err) + } + _, err = publishedOutputRelativePath(sessionPrefix, "previous-run", artifacts.S3RunPrefix(sessionPrefix, "previous-run")+"published/../session_recap.md") + if err == nil || !strings.Contains(err.Error(), "published output destination") { + t.Fatalf("publishedOutputRelativePath() error = %v, want unsafe destination rejection", err) + } +} + func TestResolveUsesCommittedSourceMappingForCustomDestination(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} @@ -86,8 +99,8 @@ func TestResolveUsesCommittedSourceMappingForCustomDestination(t *testing.T) { if artifact.RemoteKey != customKey { t.Fatalf("remote key = %q, want exact configured source destination %q", artifact.RemoteKey, customKey) } - if artifact.LocalRelativePath != "previous/artifacts/session_recap.md" { - t.Fatalf("local relative path = %q, want source-local cache path", artifact.LocalRelativePath) + if artifact.LocalRelativePath != "previous/artifacts/history/recap-v2.txt" { + t.Fatalf("local relative path = %q, want committed-destination cache path", artifact.LocalRelativePath) } } @@ -100,12 +113,12 @@ func TestResolveDoesNotUseCommittedBasenameFallback(t *testing.T) { }) _, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{{Name: "session_recap", Required: true}}, store) - if err == nil || !strings.Contains(err.Error(), `required previous-session artifact "session_recap" object missing`) { + if err == nil || !strings.Contains(err.Error(), `required previous-session artifact "session_recap" is not declared`) { t.Fatalf("Resolve() error = %v, want source-mapping absence", err) } } -func TestResolveRejectsUndeclaredPublishedCandidates(t *testing.T) { +func TestResolveRejectsUndeclaredPublishedCandidatesWithoutProbing(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} seedPreviousCurrent(t, store, cfg, previousManifestWithOutput(t, cfg, "artifacts/session_recap.md", []string{"archive/session_recap.md"})) @@ -114,7 +127,7 @@ func TestResolveRejectsUndeclaredPublishedCandidates(t *testing.T) { store.SeedObject(storage.FakeObject{Key: prefix + "archive/session_recap.md", Data: []byte("published candidate")}) _, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{{Name: "session_recap", Required: true}}, store) - if err == nil || !strings.Contains(err.Error(), "object missing from published candidate keys") { + if err == nil || !strings.Contains(err.Error(), "is not declared by the selected remote commit") { t.Fatalf("Resolve() error = %v, want undeclared-candidate error", err) } } @@ -130,16 +143,16 @@ func TestResolveHonorsCancellation(t *testing.T) { } } -func TestBuildPlanSkipsMissingOptionalPreviousArtifact(t *testing.T) { +func TestResolveSkipsMissingOptionalPreviousArtifact(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} seedPreviousCurrent(t, store, cfg, previousManifestWithOutput(t, cfg, "", nil)) - plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + plan, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: false}, }, store) if err != nil { - t.Fatalf("BuildPlan() error = %v", err) + t.Fatalf("Resolve() error = %v", err) } if strings.Join(plan.SkippedMissing, ",") != "session_recap" { t.Fatalf("SkippedMissing = %#v, want session_recap", plan.SkippedMissing) @@ -149,85 +162,85 @@ func TestBuildPlanSkipsMissingOptionalPreviousArtifact(t *testing.T) { } } -func TestBuildPlanMissingRequiredPreviousArtifactFails(t *testing.T) { +func TestResolveMissingRequiredPreviousArtifactFails(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} seedPreviousCurrent(t, store, cfg, previousManifestWithOutput(t, cfg, "", nil)) - _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + _, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: true}, }, store) - if err == nil || !strings.Contains(err.Error(), `required previous-session artifact "session_recap" object missing`) { - t.Fatalf("BuildPlan() error = %v, want required missing error", err) + if err == nil || !strings.Contains(err.Error(), `required previous-session artifact "session_recap" is not declared`) { + t.Fatalf("Resolve() error = %v, want required missing error", err) } } -func TestBuildPlanValidatesPreviousManifestIdentity(t *testing.T) { +func TestResolveValidatesPreviousManifestIdentity(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} manifest := previousManifestWithOutput(t, cfg, "artifacts/session_recap.md", nil) manifest.SessionID = "wrong-session" seedPreviousCurrent(t, store, cfg, manifest) - _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + _, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: true}, }, store) if err == nil || !strings.Contains(err.Error(), "identity does not match") { - t.Fatalf("BuildPlan() error = %v, want identity validation error", err) + t.Fatalf("Resolve() error = %v, want identity validation error", err) } } -func TestBuildPlanMissingCurrentRunPointerSkipsOptionalRequirements(t *testing.T) { +func TestResolveMissingCurrentRunPointerSkipsOptionalRequirements(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} - plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + plan, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: false}, }, store) if err != nil { - t.Fatalf("BuildPlan() error = %v", err) + t.Fatalf("Resolve() error = %v", err) } if strings.Join(plan.SkippedMissing, ",") != "session_recap" { t.Fatalf("SkippedMissing = %#v, want session_recap", plan.SkippedMissing) } } -func TestBuildPlanMissingCurrentRunPointerFailsRequiredRequirements(t *testing.T) { +func TestResolveMissingCurrentRunPointerFailsRequiredRequirements(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} - _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + _, err := Resolve(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 commit pointer missing`) { - t.Fatalf("BuildPlan() error = %v, want required missing commit pointer error", err) + t.Fatalf("Resolve() error = %v, want required missing commit pointer error", err) } } -func TestBuildPlanMissingCurrentCommitPointerSkipsOptionalRequirements(t *testing.T) { +func TestResolveMissingCurrentCommitPointerSkipsOptionalRequirements(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} - plan, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + plan, err := Resolve(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ {Name: "session_recap", Required: false}, }, store) if err != nil { - t.Fatalf("BuildPlan() error = %v", err) + t.Fatalf("Resolve() error = %v", err) } if strings.Join(plan.SkippedMissing, ",") != "session_recap" { t.Fatalf("SkippedMissing = %#v, want session_recap", plan.SkippedMissing) } } -func TestBuildPlanMissingCurrentCommitPointerFailsRequiredRequirements(t *testing.T) { +func TestResolveMissingCurrentCommitPointerFailsRequiredRequirements(t *testing.T) { cfg, paths := previousCacheTestConfig(t) store := &storage.FakeBackend{} - _, err := BuildPlan(context.Background(), cfg, paths, []artifacts.PreviousArtifactRequirement{ + _, err := Resolve(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 commit pointer missing`) { - t.Fatalf("BuildPlan() error = %v, want required missing commit pointer error", err) + t.Fatalf("Resolve() error = %v, want required missing commit pointer error", err) } } diff --git a/internal/stage/prepare_previous_test.go b/internal/stage/prepare_previous_test.go index 2f0fabd..0e83cac 100644 --- a/internal/stage/prepare_previous_test.go +++ b/internal/stage/prepare_previous_test.go @@ -197,7 +197,7 @@ func TestHydratePreviousSessionArtifactsDoesNotUseLocalPreviousWorkspaceState(t writeFile(t, filepath.Join(seed.PreviousSessionRoot, "artifacts", "session_recap.md"), "# local stale recap\n") _, err := hydratePreviousSessionArtifacts(context.Background(), env, sessionPaths, requirements) - if err == nil || !strings.Contains(err.Error(), "object missing from published candidate keys") { + if err == nil || !strings.Contains(err.Error(), "is not declared by the selected remote commit") { t.Fatalf("error = %v, want remote-object-missing failure", err) } } @@ -290,7 +290,7 @@ func seedPreviousCurrentState( }) } - artifactKey := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(previousSessionPrefix, previousRunID), "artifacts/session_recap.md") + artifactKey := artifacts.S3RunRelativeDestinationKey(artifacts.S3RunPrefix(previousSessionPrefix, previousRunID), "published/artifacts/session_recap.md") if options.includeArtifactObject { body := options.artifactBody if body == "" {