Resolve previous artifacts from commit declarations

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
-}