Make configuration truthful and clean remote session files

This commit is contained in:
2026-08-10 21:41:00 +00:00
parent 72a200968a
commit 32653f54f9
32 changed files with 395 additions and 72 deletions

View File

@@ -40,10 +40,12 @@ func cleanSession(ctx context.Context, flags commonConfigFlags, dryRun, clearCac
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("clean: session_id is required unless --all is set")
}
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
return fmt.Errorf("clean: %w", err)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if cfg == nil || cfg.Pipeline == nil || cfg.Session == nil {
return fmt.Errorf("clean: resolved pipeline and session config are required")
}

View File

@@ -218,7 +218,7 @@ audita:
binary: ` + auditaBinary + `
llm_api_key_env: OPENROUTER_API_KEY
notification:
timeout: 10s
mode: noop
`
sessionYAML := `session_id: ` + sessionID + `
campaign: sample-campaign
@@ -283,7 +283,7 @@ seriatim:
audita:
binary: audita
notification:
timeout: 10s
mode: noop
`
sessionYAML := `session_id: 2026-05-03
campaign: sample-campaign
@@ -510,7 +510,7 @@ seriatim:
audita:
binary: ` + auditaBinary + `
notification:
timeout: 10s
mode: noop
`
sessionYAML := `session_id: 2026-05-03

View File

@@ -2,13 +2,16 @@ package app
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
"gitea.maximumdirect.net/eric/narratio/internal/artifacts"
"gitea.maximumdirect.net/eric/narratio/internal/config"
"gitea.maximumdirect.net/eric/narratio/internal/fileops"
)
type pipelineCampaignConfig struct {
@@ -18,14 +21,48 @@ type pipelineCampaignConfig struct {
Campaign *config.CampaignConfig
}
func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaignFileFlag, sessionFlag string, sessionOpts config.SessionLoadOptions) (*config.Config, error) {
var downloadObjectToTempFn = storage.DownloadObjectToTemp
type commandConfig struct {
Config *config.Config
cleanup func() error
}
func (c *commandConfig) Close() error {
if c == nil || c.cleanup == nil {
return nil
}
cleanup := c.cleanup
c.cleanup = nil
return cleanup()
}
func retainedCommandConfig(cfg *config.Config) *commandConfig {
return &commandConfig{Config: cfg}
}
func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaignFileFlag, sessionFlag string, sessionOpts config.SessionLoadOptions) (loaded *commandConfig, err error) {
var cleanup func() error
defer func() {
if err == nil || cleanup == nil {
return
}
if cleanupErr := cleanup(); cleanupErr != nil {
err = errors.Join(err, cleanupErr)
}
}()
base, err := loadPipelineCampaignConfig(pipelineFlag, campaignFlag, campaignFileFlag)
if err != nil {
return nil, err
}
if explicitSession := strings.TrimSpace(sessionFlag); explicitSession != "" {
return config.LoadWithSessionOptions(base.PipelinePath, base.CampaignPath, explicitSession, sessionOpts)
cfg, err := config.LoadWithSessionOptions(base.PipelinePath, base.CampaignPath, explicitSession, sessionOpts)
if err != nil {
return nil, err
}
return retainedCommandConfig(cfg), nil
}
discoveredSession, err := discoverSessionConfigPathWithCandidates(config.DefaultSessionConfigSearchPaths)
@@ -33,7 +70,11 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaign
return nil, err
}
if discoveredSession.Path != "" {
return config.LoadWithSessionOptions(base.PipelinePath, base.CampaignPath, discoveredSession.Path, sessionOpts)
cfg, err := config.LoadWithSessionOptions(base.PipelinePath, base.CampaignPath, discoveredSession.Path, sessionOpts)
if err != nil {
return nil, err
}
return retainedCommandConfig(cfg), nil
}
sessionID := strings.TrimSpace(sessionOpts.SessionID)
@@ -62,20 +103,26 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaign
if err != nil {
return nil, missingSessionConfigError(discoveredSession.Searched, err.Error())
}
sessionTempPath, err := storage.DownloadObjectToTemp(ctx, store, remoteKey, "narratio-session-*.yml")
sessionTempPath, err := downloadObjectToTempFn(ctx, store, remoteKey, "narratio-session-*.yml")
if err != nil {
return nil, missingSessionConfigError(discoveredSession.Searched, fmt.Sprintf("remote session %q download failed: %v", remoteKey, err))
}
cleanup = func() error {
if err := fileops.RemoveAllUnderRoot(filepath.Dir(sessionTempPath), sessionTempPath); err != nil {
return fmt.Errorf("remove downloaded remote session config: %w", err)
}
return nil
}
sessionBytes, err := os.ReadFile(sessionTempPath)
if err != nil {
return nil, fmt.Errorf("read downloaded remote session %q: %w", sessionTempPath, err)
return nil, fmt.Errorf("read downloaded remote session config: %w", err)
}
sessionCfg, err := config.LoadSessionBytesWithOptions("s3://"+s3BucketName(base.Pipeline)+"/"+remoteKey, sessionBytes, sessionOpts)
if err != nil {
return nil, err
}
return config.Resolve(
cfg, err := config.Resolve(
base.PipelinePath,
base.Pipeline,
base.CampaignPath,
@@ -89,9 +136,14 @@ func loadCommandConfig(ctx context.Context, pipelineFlag, campaignFlag, campaign
S3Key: remoteKey,
S3Size: sessionInfo.Size,
S3ETag: sessionInfo.ETag,
SpoolPath: sessionTempPath,
},
)
if err != nil {
return nil, err
}
loaded = &commandConfig{Config: cfg, cleanup: cleanup}
cleanup = nil
return loaded, nil
}
func loadPipelineCampaignConfig(pipelineFlag, campaignFlag, campaignFileFlag string) (*pipelineCampaignConfig, error) {

View File

@@ -22,10 +22,11 @@ func ArtifactsList(ctx context.Context, args []string, out io.Writer) error {
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("artifacts list: session_id is required")
}
cfg, store, locks, m, err := loadHelperContext(ctx, flags, remote)
cfg, store, locks, m, cleanup, err := loadHelperContext(ctx, flags, remote)
if err != nil {
return fmt.Errorf("artifacts list: %w", err)
}
defer cleanup()
catalog, err := buildHelperArtifactCatalog(cfg, m)
if err != nil {
return fmt.Errorf("artifacts list: %w", err)

View File

@@ -90,33 +90,41 @@ func Artifacts(ctx context.Context, args []string, out io.Writer) error {
}
}
func loadHelperContext(ctx context.Context, flags commonConfigFlags, needStore bool) (*config.Config, storage.ObjectStore, *effectiveLocks, *manifest.Manifest, error) {
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
func loadHelperContext(ctx context.Context, flags commonConfigFlags, needStore bool) (*config.Config, storage.ObjectStore, *effectiveLocks, *manifest.Manifest, func(), error) {
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
return nil, nil, nil, nil, err
return nil, nil, nil, nil, nil, err
}
release := true
defer func() {
if release {
_ = loaded.Close()
}
}()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
return nil, nil, nil, nil, err
return nil, nil, nil, nil, nil, err
}
var store storage.ObjectStore
if needStore {
store, err = newCommandObjectStore(ctx, cfg, nil)
if err != nil {
return nil, nil, nil, nil, err
return nil, nil, nil, nil, nil, err
}
} else {
store, _ = objectStoreIfConfigured(ctx, cfg)
}
locks, err := loadEffectiveLocks(ctx, cfg, store)
if err != nil {
return nil, nil, nil, nil, err
return nil, nil, nil, nil, nil, err
}
paths := artifacts.NewLocalStore(cfg.Pipeline.Workspace.Root).SessionPathsFor(cfg.Session.Campaign, cfg.Session.SessionID)
m, err := loadLocalManifest(ctx, paths.ManifestPath)
if err != nil {
return nil, nil, nil, nil, err
return nil, nil, nil, nil, nil, err
}
return cfg, store, locks, m, nil
release = false
return cfg, store, locks, m, func() { _ = loaded.Close() }, nil
}
func objectStoreIfConfigured(ctx context.Context, cfg *config.Config) (storage.ObjectStore, error) {

View File

@@ -38,10 +38,11 @@ func LocksList(ctx context.Context, args []string, out io.Writer) error {
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("locks: session_id is required")
}
cfg, _, locks, _, err := loadHelperContext(ctx, flags, true)
cfg, _, locks, _, cleanup, err := loadHelperContext(ctx, flags, true)
if err != nil {
return fmt.Errorf("locks: %w", err)
}
defer cleanup()
writeLocks(out, cfg, locks)
return nil
}
@@ -63,10 +64,11 @@ func LocksAdd(ctx context.Context, args []string, out io.Writer) error {
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("locks add: session_id is required")
}
cfg, store, locks, _, err := loadHelperContext(ctx, flags, true)
cfg, store, locks, _, cleanup, err := loadHelperContext(ctx, flags, true)
if err != nil {
return fmt.Errorf("locks add: %w", err)
}
defer cleanup()
if _, err := config.ValidatePublishLockRules([]config.PublishLockRule{{Source: source}}, cfg.Pipeline.Scriptorium, cfg.Pipeline.Notarius, "locks add"); err != nil {
return fmt.Errorf("locks add: %w", err)
}
@@ -106,10 +108,11 @@ func LocksRemove(ctx context.Context, args []string, out io.Writer) error {
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("locks remove: session_id is required")
}
cfg, store, locks, _, err := loadHelperContext(ctx, flags, true)
cfg, store, locks, _, cleanup, err := loadHelperContext(ctx, flags, true)
if err != nil {
return fmt.Errorf("locks remove: %w", err)
}
defer cleanup()
if _, err := config.ValidatePublishLockRules([]config.PublishLockRule{{Source: source}}, cfg.Pipeline.Scriptorium, cfg.Pipeline.Notarius, "locks remove"); err != nil {
return fmt.Errorf("locks remove: %w", err)
}

View File

@@ -25,11 +25,13 @@ func SessionValidate(ctx context.Context, args []string, out io.Writer) error {
}
findings := []finding{}
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
findings = append(findings, errorFinding("config", err.Error()))
return renderFindings(out, "", "", findings)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
findings = append(findings, errorFinding("config", err.Error()))
} else {

View File

@@ -26,10 +26,12 @@ func Status(ctx context.Context, args []string, out io.Writer) error {
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("status: session_id is required")
}
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
return fmt.Errorf("status: %w", err)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
return fmt.Errorf("status: %w", err)
}

View File

@@ -30,10 +30,12 @@ func Plan(ctx context.Context, args []string, out io.Writer) error {
if flags.sessionID == "" {
return fmt.Errorf("plan: session_id is required")
}
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
return fmt.Errorf("plan: %w", err)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
return fmt.Errorf("plan: %w", err)
}

View File

@@ -109,7 +109,7 @@ seriatim:
audita:
binary: audita
notification:
timeout: 10s
mode: noop
`
sessionYAML := `session_id: 2026-05-03
campaign: sample-campaign

View File

@@ -13,6 +13,7 @@ import (
"gitea.maximumdirect.net/eric/narratio/internal/adapters/storage"
"gitea.maximumdirect.net/eric/narratio/internal/artifacts"
"gitea.maximumdirect.net/eric/narratio/internal/config"
"gitea.maximumdirect.net/eric/narratio/internal/stage"
)
func TestExecuteRemoteSessionFallbackLoadsFromObjectStore(t *testing.T) {
@@ -44,6 +45,146 @@ inputs:
}
}
func TestRemoteSessionConfigIsRemovedAfterEveryCommandExit(t *testing.T) {
tests := []struct {
name string
sessionYAML string
command []string
configureRun func()
wantSuccessful bool
}{
{
name: "success",
sessionYAML: `session_id: 2026-05-03
inputs:
audio_s3:
prefix: audio/
`,
command: []string{"session", "plan", "2026-05-03"},
wantSuccessful: true,
},
{
name: "validation failure",
sessionYAML: `session_id: 2026-05-03
`,
command: []string{"session", "plan", "2026-05-03"},
},
{
name: "load failure",
sessionYAML: `session_id: 2026-05-03
unknown: true
`,
command: []string{"session", "plan", "2026-05-03"},
},
{
name: "adapter failure",
sessionYAML: `session_id: 2026-05-03
inputs:
audio_s3:
prefix: audio/
`,
command: []string{"run", "2026-05-03"},
configureRun: func() {
executeStagesFn = func(context.Context, *config.Config, []stage.Stage, RunOptions) (*RunSummary, error) {
return nil, errors.New("adapter failed")
}
},
},
{
name: "cancellation",
sessionYAML: `session_id: 2026-05-03
inputs:
audio_s3:
prefix: audio/
`,
command: []string{"run", "2026-05-03"},
configureRun: func() {
executeStagesFn = func(context.Context, *config.Config, []stage.Stage, RunOptions) (*RunSummary, error) {
return nil, context.Canceled
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
workspaceRoot := t.TempDir()
pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot)
fake := &storage.FakeBackend{}
seedRemoteSessionConfig(t, fake, "2026-05-03", tt.sessionYAML)
var storeInitCalls int
restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{filepath.Join(t.TempDir(), "session.yml")})
var downloadedPath string
captureRemoteSessionTempPath(t, &downloadedPath)
if tt.configureRun != nil {
origExecuteStagesFn := executeStagesFn
t.Cleanup(func() { executeStagesFn = origExecuteStagesFn })
tt.configureRun()
}
args := append(append([]string(nil), tt.command...), "--config", pipelinePath, "--campaign-file", campaignPath)
var stdout bytes.Buffer
var stderr bytes.Buffer
code := Execute(args, &stdout, &stderr)
if tt.wantSuccessful && code != 0 {
t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String())
}
if !tt.wantSuccessful && code == 0 {
t.Fatal("exit code = 0, want non-zero")
}
if downloadedPath == "" {
t.Fatal("remote session download path was not captured")
}
if _, err := os.Stat(downloadedPath); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("downloaded remote session path still exists or could not be inspected: %q, err=%v", downloadedPath, err)
}
})
}
}
func TestRemoteSessionConfigCloseIsIdempotent(t *testing.T) {
workspaceRoot := t.TempDir()
pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot)
fake := &storage.FakeBackend{}
seedRemoteSessionConfig(t, fake, "2026-05-03", `session_id: 2026-05-03
inputs:
audio_s3:
prefix: audio/
`)
var storeInitCalls int
restoreAppConfigTestGlobals(t, fake, &storeInitCalls, []string{filepath.Join(t.TempDir(), "session.yml")})
var downloadedPath string
captureRemoteSessionTempPath(t, &downloadedPath)
loaded, err := loadCommandConfig(context.Background(), pipelinePath, "", campaignPath, "", config.SessionLoadOptions{SessionID: "2026-05-03"})
if err != nil {
t.Fatalf("loadCommandConfig() error = %v", err)
}
if err := loaded.Close(); err != nil {
t.Fatalf("first Close() error = %v", err)
}
if err := loaded.Close(); err != nil {
t.Fatalf("second Close() error = %v", err)
}
if _, err := os.Stat(downloadedPath); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("downloaded remote session path still exists or could not be inspected: %q, err=%v", downloadedPath, err)
}
}
func captureRemoteSessionTempPath(t *testing.T, destination *string) {
t.Helper()
original := downloadObjectToTempFn
downloadObjectToTempFn = func(ctx context.Context, store storage.ObjectStore, key, pattern string) (string, error) {
path, err := original(ctx, store, key, pattern)
if err == nil {
*destination = path
}
return path, err
}
t.Cleanup(func() { downloadObjectToTempFn = original })
}
func TestExecuteRemoteSessionFallbackLoadsSecretsBeforeObjectStoreInit(t *testing.T) {
workspaceRoot := t.TempDir()
pipelinePath, campaignPath, _ := writeValidConfigFiles(t, workspaceRoot)

View File

@@ -54,10 +54,12 @@ func Restore(ctx context.Context, args []string, out io.Writer) (resultErr error
if strings.TrimSpace(flags.sessionID) == "" {
return fmt.Errorf("restore: session_id is required")
}
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
return fmt.Errorf("restore: %w", err)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
return fmt.Errorf("restore: %w", err)
}

View File

@@ -27,10 +27,12 @@ func Run(ctx context.Context, args []string, out io.Writer) error {
if flags.sessionID == "" {
return fmt.Errorf("run: session_id is required")
}
cfg, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
loaded, err := loadCommandConfig(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions())
if err != nil {
return fmt.Errorf("run: %w", err)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
return fmt.Errorf("run: %w", err)
}

View File

@@ -204,13 +204,15 @@ func runSingleStageCommand(ctx context.Context, req singleStageCommand) (*RunSum
return nil, fmt.Errorf("%s: %w", req.CommandName, err)
}
cfg, err := loadCommandConfig(ctx, req.PipelinePath, req.CampaignPath, req.CampaignFilePath, req.SessionPath, config.SessionLoadOptions{
loaded, err := loadCommandConfig(ctx, req.PipelinePath, req.CampaignPath, req.CampaignFilePath, req.SessionPath, config.SessionLoadOptions{
SessionID: req.SessionID,
PreviousSessionID: req.PreviousSessionID,
})
if err != nil {
return nil, fmt.Errorf("%s: %w", req.CommandName, err)
}
defer func() { _ = loaded.Close() }()
cfg := loaded.Config
if err := config.Validate(cfg); err != nil {
return nil, fmt.Errorf("%s: %w", req.CommandName, err)
}

View File

@@ -1603,7 +1603,7 @@ func TestBuildDefaultRunnersWithOmittedToolSections(t *testing.T) {
whisperx:
transcribe_url: https://example.com/transcribe
notification:
timeout: 10s
mode: noop
`
campaignYAML := `campaign_id: sample-campaign
inputs: