diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index f17653f..5406d58 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -244,7 +244,7 @@ no factory global, and remote behavior remains deterministic and offline. ## Stage 4 — Replace Restore Phase Globals With Explicit Dependencies -**Status:** Pending +**Status:** Completed ### Goal diff --git a/internal/app/operator_helpers.go b/internal/app/operator_helpers.go index f691c59..f5d179f 100644 --- a/internal/app/operator_helpers.go +++ b/internal/app/operator_helpers.go @@ -65,7 +65,7 @@ func sessionWithDependencies(ctx context.Context, args []string, out io.Writer, case "plan": return planWithDependencies(ctx, args[1:], out, deps) case "restore": - return Restore(ctx, args[1:], out) + return restoreWithDependencies(ctx, args[1:], out, deps) case "artifacts": return artifactsListWithDependencies(ctx, args[1:], out, deps) case "locks": diff --git a/internal/app/restore.go b/internal/app/restore.go index 6fd9b39..311cf1f 100644 --- a/internal/app/restore.go +++ b/internal/app/restore.go @@ -10,19 +10,18 @@ import ( "os" "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/logging" ) -var newObjectStoreFromConfigFn = storage.NewObjectStoreFromConfig -var discoverRemoteCurrentStateFn = discoverRemoteCurrentState -var buildRestorePlanFn = buildRestorePlan -var executeRestorePlanFn = executeRestorePlan - // Restore validates restore CLI/config inputs and storage preflight for future restore phases. func Restore(ctx context.Context, args []string, out io.Writer) (resultErr error) { + return restoreWithDependencies(ctx, args, out, commandDependencies{}) +} + +func restoreWithDependencies(ctx context.Context, args []string, out io.Writer, deps commandDependencies) (resultErr error) { + deps = normalizeCommandDependencies(deps) positionalSessionID, args := pullLeadingSessionID(args) fs := flag.NewFlagSet("restore", flag.ContinueOnError) fs.SetOutput(out) @@ -54,8 +53,7 @@ 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") } - restoreDeps := commandDependencies{newObjectStore: newObjectStoreFromConfigFn} - loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), restoreDeps) + loaded, err := loadCommandConfigWithDependencies(ctx, flags.pipelinePath, flags.campaignPath, flags.campaignFilePath, flags.sessionPath, flags.sessionOptions(), deps) if err != nil { return fmt.Errorf("restore: %w", err) } @@ -64,16 +62,16 @@ func Restore(ctx context.Context, args []string, out io.Writer) (resultErr error if err := config.Validate(cfg); err != nil { return fmt.Errorf("restore: %w", err) } - objectStore, err := newCommandObjectStoreWithDependencies(ctx, cfg, logging.NewLogger(os.Stderr, slog.LevelInfo), restoreDeps) + objectStore, err := newCommandObjectStoreWithDependencies(ctx, cfg, logging.NewLogger(os.Stderr, slog.LevelInfo), deps) if err != nil { return fmt.Errorf("restore: %w", err) } - current, err := discoverRemoteCurrentStateFn(ctx, cfg, objectStore) + current, err := deps.discoverRemoteCurrentState(ctx, cfg, objectStore) if err != nil { return fmt.Errorf("restore: %w", err) } if dryRun { - plan, err := buildRestorePlanFn(ctx, cfg, current, objectStore, RestorePlanOptions{ + plan, err := deps.buildRestorePlan(ctx, cfg, current, objectStore, RestorePlanOptions{ IncludeAudio: includeAudio, Force: force, DryRun: true, @@ -116,7 +114,7 @@ func Restore(ctx context.Context, args []string, out io.Writer) (resultErr error // Classification and installation share the same transition lock as a // runner. This prevents a runner from making reuse decisions against state // that restore is about to replace. - plan, err := buildRestorePlanFn(ctx, cfg, current, objectStore, RestorePlanOptions{ + plan, err := deps.buildRestorePlan(ctx, cfg, current, objectStore, RestorePlanOptions{ IncludeAudio: includeAudio, Force: force, }) @@ -148,7 +146,7 @@ func Restore(ctx context.Context, args []string, out io.Writer) (resultErr error return fmt.Errorf("restore: mark incomplete restore: %w", err) } - result, err := executeRestorePlanFn(ctx, cfg, current, plan, report, objectStore) + result, err := deps.executeRestorePlan(ctx, cfg, current, plan, report, objectStore) if err != nil { report.setFailed(err) if _, reportErr := persistRestoreReport(artifactStore, cfg, report); reportErr != nil { diff --git a/internal/app/restore_execution_test.go b/internal/app/restore_execution_test.go index e895489..453c2c2 100644 --- a/internal/app/restore_execution_test.go +++ b/internal/app/restore_execution_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "io" "os" "path/filepath" "strings" @@ -34,11 +35,11 @@ func TestExecuteRestoreNonDryRunRestoresDurableFiles(t *testing.T) { "audio/alice.flac": []byte("remote-audio"), }) - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "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()) } @@ -99,11 +100,11 @@ func TestExecuteRestoreRoundTripsPublishedExtractionAndManifestMetadata(t *testi t.Fatal(err) } seedRestoreObject(fake, manifestKey, manifestBody) - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "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()) } @@ -130,11 +131,11 @@ func TestExecuteRestoreIncludeAudioRestoresAudio(t *testing.T) { cfg, sessionPrefix, _, _ := seedRestoreCommittedState(t, fake, pipelinePath, campaignPath, sessionPath) seedRestoreObject(fake, sessionPrefix+"audio/alice.flac", []byte("remote-audio")) - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--include-audio"}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--include-audio"}, &stdout, &stderr) if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) } @@ -156,11 +157,11 @@ func TestExecuteRestoreIncludeAudioUsesCacheAfterWorkspaceDeletion(t *testing.T) audioKey := sessionPrefix + "audio/alice.flac" seedRestoreObject(fake, audioKey, []byte("remote-audio")) - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--include-audio"}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--include-audio"}, &stdout, &stderr) if code != 0 { t.Fatalf("first restore exit code = %d, want 0; stderr=%q", code, stderr.String()) } @@ -176,7 +177,7 @@ func TestExecuteRestoreIncludeAudioUsesCacheAfterWorkspaceDeletion(t *testing.T) } stdout.Reset() stderr.Reset() - code = Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--include-audio"}, &stdout, &stderr) + code = executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--include-audio"}, &stdout, &stderr) if code != 0 { t.Fatalf("second restore exit code = %d, want 0; stderr=%q", code, stderr.String()) } @@ -195,11 +196,11 @@ func TestExecuteRestoreRestoresPreviousCacheWhenPresent(t *testing.T) { cfg, _, _, _ := seedRestoreCommittedState(t, fake, pipelinePath, campaignPath, sessionPath) seedRestorePreviousCurrent(t, fake, cfg, "# previous recap\n") - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "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()) } @@ -228,11 +229,11 @@ func TestExecuteRestoreDryRunReportsPreviousCacheWithoutWriting(t *testing.T) { cfg, _, _, _ := seedRestoreCommittedState(t, fake, pipelinePath, campaignPath, sessionPath) seedRestorePreviousCurrent(t, fake, cfg, "# previous recap\n") - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--dry-run"}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--dry-run"}, &stdout, &stderr) if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) } @@ -260,11 +261,11 @@ func TestExecuteRestoreConflictWithoutForceDoesNotOverwrite(t *testing.T) { sessionRoot := artifacts.SessionWorkDirForCampaign(workspaceRoot, cfg.Session.Campaign, cfg.Session.SessionID) mustWriteTestFile(t, filepath.Join(sessionRoot, "transcripts", "full.json"), "local-transcript") - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) if code == 0 { t.Fatal("exit code = 0, want non-zero") } @@ -292,11 +293,11 @@ func TestExecuteRestoreForceOverwritesDifferingFile(t *testing.T) { sessionRoot := artifacts.SessionWorkDirForCampaign(workspaceRoot, cfg.Session.Campaign, cfg.Session.SessionID) mustWriteTestFile(t, filepath.Join(sessionRoot, "transcripts", "full.json"), "local-transcript") - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) } @@ -322,11 +323,11 @@ func TestExecuteRestoreForceOverwritesDifferingPreviousCacheFile(t *testing.T) { sessionRoot := artifacts.SessionWorkDirForCampaign(workspaceRoot, cfg.Session.Campaign, cfg.Session.SessionID) mustWriteTestFile(t, filepath.Join(sessionRoot, "previous", "artifacts", "session_recap.md"), "# local previous recap\n") - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) + code := executeRestore([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) } @@ -348,11 +349,11 @@ func TestExecuteRestoreLockConflictFailsAndWritesNothing(t *testing.T) { } defer func() { _ = store.ReleaseSessionLock(lock) }() - restoreWithStoreAndRealPhases(t, fake) + deps := restoreCommandDependencies(fake) ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) defer cancel() - err = Restore(ctx, []string{"2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &bytes.Buffer{}) + err = restoreWithDependencies(ctx, []string{"2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &bytes.Buffer{}, deps) if err == nil || !strings.Contains(err.Error(), "acquire session lock") { t.Fatalf("Restore() error = %v, want lock failure", err) } @@ -448,10 +449,9 @@ func TestExecuteRestoreInvalidManifestReportsFailureAndRetainsRecoveryMarker(t * t.Fatalf("read existing local manifest: %v", err) } - restoreWithStoreAndRealPhases(t, store) - realBuildRestorePlan := buildRestorePlanFn - buildRestorePlanFn = func(ctx context.Context, cfg *config.Config, current *RemoteCurrentState, objectStore storage.ObjectStore, opts RestorePlanOptions) (*RestorePlan, error) { - plan, err := realBuildRestorePlan(ctx, cfg, current, objectStore, opts) + deps := restoreCommandDependencies(store) + deps.buildRestorePlan = func(ctx context.Context, cfg *config.Config, current *RemoteCurrentState, objectStore storage.ObjectStore, opts RestorePlanOptions) (*RestorePlan, error) { + plan, err := buildRestorePlan(ctx, cfg, current, objectStore, opts) if err != nil { return nil, err } @@ -476,7 +476,7 @@ func TestExecuteRestoreInvalidManifestReportsFailureAndRetainsRecoveryMarker(t * var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", cfg.Session.SessionID, "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) + code := executeWithDependencies([]string{"session", "restore", cfg.Session.SessionID, "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr, deps, executePlan) if code == 0 { t.Fatal("exit code = 0, want non-zero") } @@ -573,25 +573,17 @@ func mustReadRestoreReport(t *testing.T, path string) *RestoreReport { return &report } -func restoreWithStoreAndRealPhases(t *testing.T, objectStore storage.ObjectStore) { - t.Helper() - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - origExecuteFn := executeRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - executeRestorePlanFn = origExecuteFn - }) - - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { +func restoreCommandDependencies(objectStore storage.ObjectStore) commandDependencies { + return commandDependencies{newObjectStore: func(context.Context, *config.Config) (storage.ObjectStore, error) { return objectStore, nil + }} +} + +func restoreCommandExecutor(objectStore storage.ObjectStore) func([]string, io.Writer, io.Writer) int { + deps := restoreCommandDependencies(objectStore) + return func(args []string, stdout, stderr io.Writer) int { + return executeWithDependencies(args, stdout, stderr, deps, executePlan) } - discoverRemoteCurrentStateFn = discoverRemoteCurrentState - buildRestorePlanFn = buildRestorePlan - executeRestorePlanFn = executeRestorePlan } func seedRestoreCommittedState(t *testing.T, fake *storage.FakeBackend, pipelinePath, campaignPath, sessionPath string) (*config.Config, string, string, string) { diff --git a/internal/app/restore_recovery_test.go b/internal/app/restore_recovery_test.go index 1ede465..0ee2f27 100644 --- a/internal/app/restore_recovery_test.go +++ b/internal/app/restore_recovery_test.go @@ -21,26 +21,17 @@ func TestRestoreHoldsTransitionLockBeforePlanning(t *testing.T) { fake := &storage.FakeBackend{} cfg, _, _, _ := seedRestoreCommittedState(t, fake, pipelinePath, campaignPath, sessionPath) - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - }) - current, err := discoverRemoteCurrentState(context.Background(), cfg, fake) if err != nil { t.Fatalf("discoverRemoteCurrentState() error = %v", err) } planning := make(chan struct{}) releasePlanning := make(chan struct{}) - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { return fake, nil } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { + deps := restoreCommandDependencies(fake) + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return current, nil } - buildRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { + deps.buildRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { close(planning) <-releasePlanning return &RestorePlan{}, nil @@ -48,12 +39,12 @@ func TestRestoreHoldsTransitionLockBeforePlanning(t *testing.T) { restoreDone := make(chan error, 1) go func() { - restoreDone <- Restore(context.Background(), []string{ + restoreDone <- restoreWithDependencies(context.Background(), []string{ cfg.Session.SessionID, "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, - }, &bytes.Buffer{}) + }, &bytes.Buffer{}, deps) }() <-planning @@ -102,12 +93,12 @@ func TestForcedRestoreRetryClearsIncompleteMarker(t *testing.T) { } return nil } - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) var stdout bytes.Buffer var stderr bytes.Buffer args := []string{"session", "restore", cfg.Session.SessionID, "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"} - if code := Execute(args, &stdout, &stderr); code == 0 { + if code := executeRestore(args, &stdout, &stderr); code == 0 { t.Fatal("forced restore exit code = 0, want partial-install failure") } markerPath := artifacts.SessionRestoreMarkerPathForCampaign(workspaceRoot, cfg.Session.Campaign, cfg.Session.SessionID) @@ -119,7 +110,7 @@ func TestForcedRestoreRetryClearsIncompleteMarker(t *testing.T) { fake.DownloadHook = nil stdout.Reset() stderr.Reset() - if code := Execute(args, &stdout, &stderr); code != 0 { + if code := executeRestore(args, &stdout, &stderr); code != 0 { t.Fatalf("restore retry exit code = %d, want 0; stderr=%q", code, stderr.String()) } if _, err := os.Stat(markerPath); !os.IsNotExist(err) { diff --git a/internal/app/restore_test.go b/internal/app/restore_test.go index 6bac65e..fa71e63 100644 --- a/internal/app/restore_test.go +++ b/internal/app/restore_test.go @@ -37,28 +37,16 @@ func TestExecuteRestoreHelp(t *testing.T) { } } -func TestExecuteRestoreRecognizedAndReturnsNYI(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - origExecuteFn := executeRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - executeRestorePlanFn = origExecuteFn - }) - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { - return &storage.FakeBackend{}, nil - } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { +func TestExecuteRestoreDryRunPrintsPlanWithoutWriting(t *testing.T) { + deps := restoreCommandDependencies(&storage.FakeBackend{}) + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return &RemoteCurrentState{ SessionID: "2026-05-03", Campaign: "sample-campaign", RunID: "20260519T010203Z-a1b2c3d4", }, nil } - buildRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { + deps.buildRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { return &RestorePlan{ Actions: []RestoreAction{ { @@ -77,7 +65,7 @@ func TestExecuteRestoreRecognizedAndReturnsNYI(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute( + code := executeWithDependencies( []string{ "session", "restore", "2026-05-03", "--config", pipelinePath, @@ -89,6 +77,8 @@ func TestExecuteRestoreRecognizedAndReturnsNYI(t *testing.T) { }, &stdout, &stderr, + deps, + executePlan, ) if code != 0 { t.Fatalf("exit code = %d, want 0 for --dry-run restore planning; stderr=%q", code, stderr.String()) @@ -109,7 +99,7 @@ func TestExecuteRestoreRecognizedAndReturnsNYI(t *testing.T) { manifestPath := artifacts.SessionManifestPathForCampaign(workspaceRoot, "sample-campaign", "2026-05-03") if _, err := os.Stat(manifestPath); !os.IsNotExist(err) { - t.Fatalf("manifest should not be created during phase-4 restore planning; stat err=%v", err) + t.Fatalf("manifest should not be created during dry-run restore planning; stat err=%v", err) } reportPath := filepath.Join(workspaceRoot, "work", "sample-campaign", "2026-05-03", "reports", "restore-latest.json") if _, err := os.Stat(reportPath); !os.IsNotExist(err) { @@ -133,13 +123,6 @@ func TestExecuteRestoreRejectsUnexpectedPositionalArguments(t *testing.T) { } func TestExecuteRestoreFailsWhenStorageBackendNotConfigured(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - }) - workspaceRoot := t.TempDir() pipelinePath, campaignPath, sessionPath := writeRestoreConfigWithoutStorage(t, workspaceRoot) @@ -155,16 +138,8 @@ func TestExecuteRestoreFailsWhenStorageBackendNotConfigured(t *testing.T) { } func TestExecuteRestoreDiscoveryErrorSurfaced(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - }) - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { - return &storage.FakeBackend{}, nil - } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { + deps := restoreCommandDependencies(&storage.FakeBackend{}) + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return nil, fmt.Errorf("remote current commit pointer missing: %q", "dnd/campaigns/sample-campaign/sessions/2026-05-03/current/commit-pointer.json") } @@ -173,7 +148,7 @@ func TestExecuteRestoreDiscoveryErrorSurfaced(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := executeWithDependencies([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr, deps, executePlan) if code == 0 { t.Fatal("exit code = 0, want non-zero") } @@ -183,17 +158,6 @@ func TestExecuteRestoreDiscoveryErrorSurfaced(t *testing.T) { } func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - origExecuteFn := executeRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - executeRestorePlanFn = origExecuteFn - }) - const accessKeyEnv = "OBJECT_STORAGE_KEY_ID" const secretKeyEnv = "OBJECT_STORAGE_KEY" restoreEnv := func(name string) { @@ -225,7 +189,7 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { } storeInitCalled := false - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { + deps := commandDependencies{newObjectStore: func(context.Context, *config.Config) (storage.ObjectStore, error) { storeInitCalled = true gotID, okID := os.LookupEnv(accessKeyEnv) if !okID || gotID != "test-access-key-id" { @@ -236,15 +200,15 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { return nil, fmt.Errorf("missing or unexpected %s: %q (set=%t)", secretKeyEnv, gotSecret, okSecret) } return &storage.FakeBackend{}, nil - } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { + }} + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return &RemoteCurrentState{ SessionID: "2026-05-03", Campaign: "sample-campaign", RunID: "20260519T010203Z-a1b2c3d4", }, nil } - buildRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { + deps.buildRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { return &RestorePlan{ Actions: []RestoreAction{ { @@ -260,7 +224,7 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute( + code := executeWithDependencies( []string{ "session", "restore", "2026-05-03", "--config", pipelinePath, @@ -270,6 +234,8 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { }, &stdout, &stderr, + deps, + executePlan, ) if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) @@ -279,28 +245,16 @@ func TestExecuteRestoreLoadsSecretsBeforeObjectStoreInit(t *testing.T) { } } -func TestExecuteRestoreNonDryRunConflictFailsBeforeNYI(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - origExecuteFn := executeRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - executeRestorePlanFn = origExecuteFn - }) - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { - return &storage.FakeBackend{}, nil - } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { +func TestExecuteRestoreNonDryRunConflictDoesNotExecute(t *testing.T) { + deps := restoreCommandDependencies(&storage.FakeBackend{}) + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return &RemoteCurrentState{ SessionID: "2026-05-03", Campaign: "sample-campaign", RunID: "20260519T010203Z-a1b2c3d4", }, nil } - buildRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { + deps.buildRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { return &RestorePlan{ Actions: []RestoreAction{ {Kind: RestoreActionConflict, LocalRelativePath: "transcripts/full.json", RemoteKey: "k", Reason: "local file differs"}, @@ -313,7 +267,7 @@ func TestExecuteRestoreNonDryRunConflictFailsBeforeNYI(t *testing.T) { pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr) + code := executeWithDependencies([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath}, &stdout, &stderr, deps, executePlan) if code == 0 { t.Fatal("exit code = 0, want non-zero") } @@ -323,33 +277,18 @@ func TestExecuteRestoreNonDryRunConflictFailsBeforeNYI(t *testing.T) { if !strings.Contains(stderr.String(), "restore conflict: 1 conflicting path(s)") { t.Fatalf("stderr = %q, want conflict failure", stderr.String()) } - if strings.Contains(stderr.String(), "phase 4: restore execution") { - t.Fatalf("stderr = %q, should fail before phase-4 NYI boundary", stderr.String()) - } } func TestExecuteRestoreForceStillBlocksUnresolvedConflict(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - origExecuteFn := executeRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - executeRestorePlanFn = origExecuteFn - }) - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { - return &storage.FakeBackend{}, nil - } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { + deps := restoreCommandDependencies(&storage.FakeBackend{}) + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return &RemoteCurrentState{SessionID: "2026-05-03", Campaign: "sample-campaign", RunID: "20260519T010203Z-a1b2c3d4"}, nil } - buildRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { + deps.buildRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { return &RestorePlan{Actions: []RestoreAction{{Kind: RestoreActionConflict, LocalRelativePath: "artifacts/session_recap.md", Reason: "local path is a directory"}}, ConflictCount: 1}, nil } executed := false - executeRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, *RestorePlan, *RestoreReport, storage.ObjectStore) (*RestoreExecutionResult, error) { + deps.executeRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, *RestorePlan, *RestoreReport, storage.ObjectStore) (*RestoreExecutionResult, error) { executed = true return &RestoreExecutionResult{}, nil } @@ -358,7 +297,7 @@ func TestExecuteRestoreForceStillBlocksUnresolvedConflict(t *testing.T) { pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) + code := executeWithDependencies([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr, deps, executePlan) if code == 0 { t.Fatal("exit code = 0, want conflict failure") } @@ -368,27 +307,15 @@ func TestExecuteRestoreForceStillBlocksUnresolvedConflict(t *testing.T) { } func TestExecuteRestoreNonDryRunForceExecutesPlan(t *testing.T) { - origStoreFn := newObjectStoreFromConfigFn - origDiscoverFn := discoverRemoteCurrentStateFn - origPlanFn := buildRestorePlanFn - origExecuteFn := executeRestorePlanFn - t.Cleanup(func() { - newObjectStoreFromConfigFn = origStoreFn - discoverRemoteCurrentStateFn = origDiscoverFn - buildRestorePlanFn = origPlanFn - executeRestorePlanFn = origExecuteFn - }) - newObjectStoreFromConfigFn = func(context.Context, *config.Config) (storage.ObjectStore, error) { - return &storage.FakeBackend{}, nil - } - discoverRemoteCurrentStateFn = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { + deps := restoreCommandDependencies(&storage.FakeBackend{}) + deps.discoverRemoteCurrentState = func(context.Context, *config.Config, storage.ObjectStore) (*RemoteCurrentState, error) { return &RemoteCurrentState{ SessionID: "2026-05-03", Campaign: "sample-campaign", RunID: "20260519T010203Z-a1b2c3d4", }, nil } - buildRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { + deps.buildRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, storage.ObjectStore, RestorePlanOptions) (*RestorePlan, error) { return &RestorePlan{ Actions: []RestoreAction{ {Kind: RestoreActionDownload, LocalRelativePath: "transcripts/full.json", RemoteKey: "k", Reason: "local file differs; overwrite with --force"}, @@ -396,7 +323,7 @@ func TestExecuteRestoreNonDryRunForceExecutesPlan(t *testing.T) { DownloadCount: 1, }, nil } - executeRestorePlanFn = func(context.Context, *config.Config, *RemoteCurrentState, *RestorePlan, *RestoreReport, storage.ObjectStore) (*RestoreExecutionResult, error) { + deps.executeRestorePlan = func(context.Context, *config.Config, *RemoteCurrentState, *RestorePlan, *RestoreReport, storage.ObjectStore) (*RestoreExecutionResult, error) { return &RestoreExecutionResult{DownloadedCount: 1}, nil } @@ -404,7 +331,7 @@ func TestExecuteRestoreNonDryRunForceExecutesPlan(t *testing.T) { pipelinePath, campaignPath, sessionPath := writeValidConfigFiles(t, workspaceRoot) var stdout bytes.Buffer var stderr bytes.Buffer - code := Execute([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr) + code := executeWithDependencies([]string{"session", "restore", "2026-05-03", "--config", pipelinePath, "--campaign-file", campaignPath, "--session", sessionPath, "--force"}, &stdout, &stderr, deps, executePlan) if code != 0 { t.Fatalf("exit code = %d, want 0; stderr=%q", code, stderr.String()) } diff --git a/internal/app/restore_workflow_test.go b/internal/app/restore_workflow_test.go index 782af15..050d1eb 100644 --- a/internal/app/restore_workflow_test.go +++ b/internal/app/restore_workflow_test.go @@ -27,7 +27,7 @@ func TestRestoreThenRunStageForceAnalyzeUsesRestoredDurableState(t *testing.T) { seedRestoreObject(fake, sessionPrefix+"transcripts/full.json", []byte(`{"segments":[1,2,3]}`+"\n")) seedRestoreObject(fake, sessionPrefix+"artifacts/session_recap.md", []byte("# restored recap\n")) - restoreWithStoreAndRealPhases(t, fake) + executeRestore := restoreCommandExecutor(fake) executor := func(ctx context.Context, cfg *config.Config, plan BoundedPlan, opts RunOptions) (*RunSummary, error) { if opts.Env == nil { @@ -39,7 +39,7 @@ func TestRestoreThenRunStageForceAnalyzeUsesRestoredDurableState(t *testing.T) { var stdout bytes.Buffer var stderr bytes.Buffer - restoreCode := Execute( + restoreCode := executeRestore( []string{ "session", "restore", cfg.Session.SessionID, "--config", pipelinePath, @@ -164,11 +164,11 @@ previous_session_id: 2026-04-26 seedRestoreObject(fakeStore, sessionPrefix+"transcripts/final.trimmed.json", []byte(`{"segments":[]}`+"\n")) seedRestorePreviousCurrent(t, fakeStore, cfg, "# previous recap\n") - restoreWithStoreAndRealPhases(t, fakeStore) + executeRestore := restoreCommandExecutor(fakeStore) var stdout bytes.Buffer var stderr bytes.Buffer - restoreCode := Execute( + restoreCode := executeRestore( []string{ "session", "restore", cfg.Session.SessionID, "--config", pipelinePath,