From ca1ded1821b0b669c1aa9346a1e5b4e3359ca6a2 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Thu, 21 May 2026 21:00:51 -0500 Subject: [PATCH] Bugfix for commands that list artifacts in the S3 backend --- docs/cli.md | 3 +- docs/operations.md | 2 +- internal/app/operator_helpers.go | 96 +++++++++++++++++++++++++-- internal/app/operator_helpers_test.go | 71 ++++++++++++++++++++ 4 files changed, 166 insertions(+), 6 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 467a78a..1e5c0e3 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -132,7 +132,7 @@ Valid stage names: - `--session ` - `--session-id ` - `--previous-session-id ` -- `--remote`: check promoted remote object availability. +- `--remote`: check remote object availability. Catalog sections use canonical source paths; the `Promoted` section checks configured archive destinations. ### `locks` @@ -289,6 +289,7 @@ narratio artifacts list [--config ] [--campaign ] [- ``` `--remote` checks promoted top-level object availability through the storage adapter. +Catalog sections report canonical source-path availability. The `Promoted` section reports each configured archive promotion destination and includes `dest=` when that destination differs from the source's canonical path. ### `locks` diff --git a/docs/operations.md b/docs/operations.md index ef4e349..6587f38 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -253,7 +253,7 @@ Dry-run does not write restore report files. - `status` with no config/session flags still requires explicit `--manifest`. - `status --session-id ` uses normal config/session loading, including remote session fallback. -- `status --session-id ` includes the same source-based remote output availability view as `artifacts list --remote` when storage is configured. +- `status --session-id ` includes the same remote output availability view as `artifacts list --remote` when storage is configured: catalog sections use canonical source paths, and promoted outputs use configured archive destinations. - local and S3 audio input modes are mutually exclusive. - archive publish requires upstream stages through `analyze` to be `succeeded`. - required promotion rules can fail when selected analyze artifacts did not generate a required file path. diff --git a/internal/app/operator_helpers.go b/internal/app/operator_helpers.go index bb0a46c..c436899 100644 --- a/internal/app/operator_helpers.go +++ b/internal/app/operator_helpers.go @@ -219,11 +219,13 @@ func Status(ctx context.Context, args []string, out io.Writer) error { } } remoteState := map[string]string{} + promotedRemoteState := map[string]string{} if store != nil { remoteState = remoteArtifactAvailability(ctx, cfg, store, catalog) + promotedRemoteState = remotePromotionAvailability(ctx, cfg, store, catalog) } fmt.Fprintln(out, "Remote outputs:") - writeArtifactList(out, cfg, catalog, catalogLocks, remoteState) + writeArtifactList(out, cfg, catalog, catalogLocks, remoteState, promotedRemoteState) } if err != nil { fmt.Fprintf(out, "Archive locks: error: %v\n", err) @@ -389,10 +391,12 @@ func ArtifactsList(ctx context.Context, args []string, out io.Writer) error { return fmt.Errorf("artifacts list: %w", err) } remoteState := map[string]string{} + promotedRemoteState := map[string]string{} if remote && store != nil { remoteState = remoteArtifactAvailability(ctx, cfg, store, catalog) + promotedRemoteState = remotePromotionAvailability(ctx, cfg, store, catalog) } - writeArtifactList(out, cfg, catalog, locks, remoteState) + writeArtifactList(out, cfg, catalog, locks, remoteState, promotedRemoteState) return nil } @@ -793,7 +797,7 @@ func buildHelperArtifactCatalog(cfg *config.Config) (*artifacts.ArtifactCatalog, return catalog, nil } -func writeArtifactList(out io.Writer, cfg *config.Config, catalog *artifacts.ArtifactCatalog, locks *effectiveLocks, remoteState map[string]string) { +func writeArtifactList(out io.Writer, cfg *config.Config, catalog *artifacts.ArtifactCatalog, locks *effectiveLocks, remoteState map[string]string, promotedRemoteState map[string]string) { lockSet := lockSourceSet(locks.All) fmt.Fprintln(out, "Built-in:") for _, id := range []string{ @@ -815,7 +819,7 @@ func writeArtifactList(out io.Writer, cfg *config.Config, catalog *artifacts.Art } fmt.Fprintln(out, "Promoted:") for _, rule := range cfg.Pipeline.Archive.PromoteArtifacts { - writeArtifactLine(out, rule.Source, lockSet, remoteState) + writePromotedArtifactLine(out, rule, catalog, lockSet, promotedRemoteState) } } @@ -830,6 +834,27 @@ func writeArtifactLine(out io.Writer, source string, lockSet map[string]config.A fmt.Fprintf(out, "- %s\n", strings.Join(parts, " ")) } +func writePromotedArtifactLine(out io.Writer, rule config.ArchivePromotionRule, catalog *artifacts.ArtifactCatalog, lockSet map[string]config.ArchiveLockRule, remoteState map[string]string) { + source := strings.TrimSpace(rule.Source) + parts := []string{source} + if _, ok := lockSet[source]; ok { + parts = append(parts, "locked") + } + dest, showDest, err := helperPromotionDest(rule, catalog) + if err != nil { + parts = append(parts, "remote=error") + fmt.Fprintf(out, "- %s\n", strings.Join(parts, " ")) + return + } + if showDest { + parts = append(parts, "dest="+dest) + } + if state := remoteState[promotionRemoteStateKey(source, dest)]; state != "" { + parts = append(parts, state) + } + fmt.Fprintf(out, "- %s\n", strings.Join(parts, " ")) +} + func remoteArtifactAvailability(ctx context.Context, cfg *config.Config, store storage.ObjectStore, catalog *artifacts.ArtifactCatalog) map[string]string { out := map[string]string{} sessionPrefix := artifacts.S3SessionPrefix(cfg.Pipeline.Storage.S3.RootPrefix, cfg.Session.Campaign, cfg.Session.SessionID) @@ -850,6 +875,69 @@ func remoteArtifactAvailability(ctx context.Context, cfg *config.Config, store s return out } +func remotePromotionAvailability(ctx context.Context, cfg *config.Config, store storage.ObjectStore, catalog *artifacts.ArtifactCatalog) map[string]string { + out := map[string]string{} + sessionPrefix := artifacts.S3SessionPrefix(cfg.Pipeline.Storage.S3.RootPrefix, cfg.Session.Campaign, cfg.Session.SessionID) + for _, rule := range cfg.Pipeline.Archive.PromoteArtifacts { + source := strings.TrimSpace(rule.Source) + dest, _, err := helperPromotionDest(rule, catalog) + if err != nil { + out[promotionRemoteStateKey(source, "")] = "remote=error" + continue + } + key := artifacts.S3PromotedArtifactKey(sessionPrefix, dest) + if exists, err := store.Exists(ctx, key); err == nil && exists { + out[promotionRemoteStateKey(source, dest)] = "remote=promoted" + } else if err != nil { + out[promotionRemoteStateKey(source, dest)] = "remote=error" + } else { + out[promotionRemoteStateKey(source, dest)] = "remote=missing" + } + } + return out +} + +func helperPromotionDest(rule config.ArchivePromotionRule, catalog *artifacts.ArtifactCatalog) (string, bool, error) { + source := strings.TrimSpace(rule.Source) + dest := strings.TrimSpace(rule.Dest) + if dest == "" { + entry, ok := catalog.Lookup(source) + if !ok { + return "", false, fmt.Errorf("destination omitted and source is unknown") + } + dest = strings.TrimSpace(entry.CanonicalRelPath) + if dest == "" { + return "", false, fmt.Errorf("destination omitted and no canonical destination is available") + } + } + normalized, err := normalizeHelperArchiveRelativePath(dest) + if err != nil { + return "", false, err + } + entry, ok := catalog.Lookup(source) + showDest := !ok || strings.TrimSpace(entry.CanonicalRelPath) != normalized + return normalized, showDest, nil +} + +func normalizeHelperArchiveRelativePath(rel string) (string, error) { + trimmed := strings.TrimSpace(rel) + if trimmed == "" { + return "", fmt.Errorf("relative path is required") + } + cleaned := filepath.ToSlash(filepath.Clean(filepath.FromSlash(trimmed))) + if cleaned == "." || cleaned == "" { + return "", fmt.Errorf("relative path is required") + } + if filepath.IsAbs(trimmed) || strings.HasPrefix(cleaned, "/") || cleaned == ".." || strings.HasPrefix(cleaned, "../") { + return "", fmt.Errorf("path must be a clean relative path") + } + return cleaned, nil +} + +func promotionRemoteStateKey(source, dest string) string { + return strings.TrimSpace(source) + "\x00" + strings.TrimSpace(dest) +} + func allCatalogSources(catalog *artifacts.ArtifactCatalog) []string { out := []string{ artifacts.ArtifactTranscriptMerged, diff --git a/internal/app/operator_helpers_test.go b/internal/app/operator_helpers_test.go index 467e127..1e8f59c 100644 --- a/internal/app/operator_helpers_test.go +++ b/internal/app/operator_helpers_test.go @@ -337,17 +337,72 @@ func TestExecuteArtifactsListRemoteReportsPromotedAvailability(t *testing.T) { } } +func TestExecuteArtifactsListRemoteUsesPromotionDestinations(t *testing.T) { + workspaceRoot := t.TempDir() + pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) + addArchivePromotionsToPipeline(t, pipelinePath, ` + promote_artifacts: + - source: narratio.transcript.full + dest: transcripts/full.json + required: true + - source: narratio.bounds.session + dest: transcripts/bounds.json + required: true +`) + fake := &storage.FakeBackend{} + sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03") + fake.SeedObject(storage.FakeObject{Key: artifacts.S3PromotedArtifactKey(sessionPrefix, "transcripts/full.json"), Data: []byte(`{"segments":[]}`)}) + fake.SeedObject(storage.FakeObject{Key: artifacts.S3PromotedArtifactKey(sessionPrefix, "transcripts/bounds.json"), Data: []byte(`{}`)}) + var storeInitCalls int + restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) + + var stdout bytes.Buffer + var stderr bytes.Buffer + code := Execute([]string{ + "artifacts", "list", + "--config", pipelinePath, + "--campaign", campaignPath, + "--session", sessionPath, + "--remote", + }, &stdout, &stderr) + if code != 0 { + t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) + } + out := stdout.String() + for _, want := range []string{ + "narratio.transcript.full remote=missing", + "narratio.bounds.session remote=missing", + "narratio.transcript.full dest=transcripts/full.json remote=promoted", + "narratio.bounds.session dest=transcripts/bounds.json remote=promoted", + } { + if !strings.Contains(out, want) { + t.Fatalf("stdout = %q, want %q", out, want) + } + } +} + func TestExecuteStatusReportsRemoteArtifactCatalog(t *testing.T) { workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) + addArchivePromotionsToPipeline(t, pipelinePath, ` + promote_artifacts: + - source: narratio.transcript.trimmed + dest: transcripts/trimmed.json + required: true + - source: narratio.transcript.full + dest: transcripts/full.json + required: true +`) fake := &storage.FakeBackend{} sessionPrefix := artifacts.S3SessionPrefix("dnd", "sample-campaign", "2026-05-03") manifestKey, runIDKey := artifacts.ResolveArchiveCurrentStateKeys(sessionPrefix) trimmedKey := artifacts.S3PromotedArtifactKey(sessionPrefix, "transcripts/trimmed.json") + fullKey := artifacts.S3PromotedArtifactKey(sessionPrefix, "transcripts/full.json") lockKey := artifacts.S3SessionLocksKey(sessionPrefix) fake.SeedObject(storage.FakeObject{Key: runIDKey, Data: []byte("20260519T010203Z-a1b2c3d4\n")}) fake.SeedObject(storage.FakeObject{Key: manifestKey, Data: restoreManifestJSON(t, "2026-05-03", "sample-campaign")}) fake.SeedObject(storage.FakeObject{Key: trimmedKey, Data: []byte(`{"segments":[]}`)}) + fake.SeedObject(storage.FakeObject{Key: fullKey, Data: []byte(`{"segments":[]}`)}) fake.SeedObject(storage.FakeObject{Key: lockKey, Data: []byte("locks:\n - source: narratio.transcript.trimmed\n reason: remote review\n")}) var storeInitCalls int restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{sessionPath}) @@ -373,6 +428,7 @@ func TestExecuteStatusReportsRemoteArtifactCatalog(t *testing.T) { "Promoted:", "narratio.transcript.trimmed locked remote=promoted", "narratio.transcript.merged remote=missing", + "narratio.transcript.full dest=transcripts/full.json remote=promoted", } { if !strings.Contains(out, want) { t.Fatalf("stdout = %q, want %q", out, want) @@ -439,6 +495,21 @@ func TestExecuteArchiveLoadsRemoteLocks(t *testing.T) { } } +func addArchivePromotionsToPipeline(t *testing.T, pipelinePath, archiveYAML string) { + t.Helper() + data, err := os.ReadFile(pipelinePath) + if err != nil { + t.Fatalf("read pipeline: %v", err) + } + updated := strings.Replace(string(data), " upload_run: false\n", " upload_run: false\n"+archiveYAML, 1) + if updated == string(data) { + t.Fatalf("pipeline %q did not contain archive upload_run marker", pipelinePath) + } + if err := os.WriteFile(pipelinePath, []byte(updated), 0o644); err != nil { + t.Fatalf("write pipeline: %v", err) + } +} + func writeValidArchiveConfigFiles(t *testing.T, workspaceRoot string) (string, string, string) { t.Helper() pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot)