From 49232455162130b892358428f7e5ccadb203f161 Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Wed, 30 Sep 2026 11:15:07 -0400 Subject: [PATCH 1/2] fix(sync): confirm destructive watch actions --- internal/sync/prompt/plan.go | 11 ++++++ internal/sync/prompt/plan_test.go | 20 +++++++++++ internal/sync/prompt/runner.go | 17 ++++++---- internal/sync/prompt/runner_test.go | 15 ++++++++ internal/sync/prompt/terminal.go | 3 +- internal/sync/prompt/terminal_test.go | 49 +++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 7 deletions(-) diff --git a/internal/sync/prompt/plan.go b/internal/sync/prompt/plan.go index 907e2627..7594b35e 100644 --- a/internal/sync/prompt/plan.go +++ b/internal/sync/prompt/plan.go @@ -158,6 +158,17 @@ func (plan Plan) RequiresConfirmation() bool { return false } +// HasDestructiveActions reports whether applying the plan would remove a local +// resource or archive one in LaunchDarkly. +func (plan Plan) HasDestructiveActions() bool { + for _, resource := range plan.Resources { + if resource.Action == ActionArchiveServer || resource.Action == ActionDeleteLocal { + return true + } + } + return false +} + // BlockingError returns a readable error for conflicts or invalid resources. func (plan Plan) BlockingError() error { for _, resource := range plan.Resources { diff --git a/internal/sync/prompt/plan_test.go b/internal/sync/prompt/plan_test.go index 0b899b30..b82ec26a 100644 --- a/internal/sync/prompt/plan_test.go +++ b/internal/sync/prompt/plan_test.go @@ -97,6 +97,26 @@ func TestBuildPlanRejectsParentConfigModeMismatch(t *testing.T) { require.Contains(t, plan.Resources[0].Error, "does not match config mode") } +func TestPlanHasDestructiveActions(t *testing.T) { + tests := map[string]struct { + action Action + destructive bool + }{ + "create server": {action: ActionCreateServer}, + "update server": {action: ActionUpdateServer}, + "archive server": {action: ActionArchiveServer, destructive: true}, + "update local": {action: ActionUpdateLocal}, + "delete local": {action: ActionDeleteLocal, destructive: true}, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + plan := Plan{Resources: []PlannedResource{{Action: test.action}}} + require.Equal(t, test.destructive, plan.HasDestructiveActions()) + }) + } +} + func testResourceID() ResourceID { return ResourceID{Kind: syncdomain.KindVariation, ProjectKey: "production", LookupKey: "support/default"} } diff --git a/internal/sync/prompt/runner.go b/internal/sync/prompt/runner.go index 7f250acb..990de4b9 100644 --- a/internal/sync/prompt/runner.go +++ b/internal/sync/prompt/runner.go @@ -158,12 +158,7 @@ func (runner Runner) Run(options Options) error { ctx, stop := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) defer stop() - syncOptions := options - syncOptions.Add = false - syncOptions.Format = "" - syncOptions.Link = "" - syncOptions.Yes = true - syncOptions.Context = ctx + syncOptions := optionsForWatchSync(ctx, options) // Watch owns the retry loop. Each callback still runs the exact same // plan, review, revalidation, and execution pipeline as a normal sync. return runner.watch(ctx, workspace.root, watchDebounce, func(watcher *sourceWatcher) error { @@ -175,6 +170,16 @@ func (runner Runner) Run(options Options) error { return runner.runWorkspaceSync(options, workspace) } +// optionsForWatchSync clears one-time actions while preserving explicit user +// choices such as --yes for each sync triggered by the watcher. +func optionsForWatchSync(ctx context.Context, options Options) Options { + options.Add = false + options.Format = "" + options.Link = "" + options.Context = ctx + return options +} + // runWorkspaceSync plans, reviews, revalidates, and executes one workspace sync. func (runner Runner) runWorkspaceSync(options Options, workspace syncWorkspace) error { var watched *watchedSources diff --git a/internal/sync/prompt/runner_test.go b/internal/sync/prompt/runner_test.go index 5f1217b9..0579bcb3 100644 --- a/internal/sync/prompt/runner_test.go +++ b/internal/sync/prompt/runner_test.go @@ -128,6 +128,21 @@ func TestRunnerWatchDoesNotRunInitialSync(t *testing.T) { assert.True(t, watchCalled) } +func TestOptionsForWatchSyncPreservesYes(t *testing.T) { + ctx := context.Background() + for _, yes := range []bool{false, true} { + options := optionsForWatchSync(ctx, Options{ + Add: true, Format: syncreference.PlainMarkdown, Link: "prompt.md", Yes: yes, + }) + + assert.False(t, options.Add) + assert.Empty(t, options.Format) + assert.Empty(t, options.Link) + assert.Equal(t, yes, options.Yes) + assert.Equal(t, ctx, options.Context) + } +} + func TestRunnerLinksBeforeWatching(t *testing.T) { root := initGitRepository(t) runner := NewRunner(noopResourceClient{}) diff --git a/internal/sync/prompt/terminal.go b/internal/sync/prompt/terminal.go index c62bc9a6..e6b89f81 100644 --- a/internal/sync/prompt/terminal.go +++ b/internal/sync/prompt/terminal.go @@ -23,7 +23,8 @@ func reviewAndConfirmPlan(options Options, plan Plan, interactive bool) (bool, e } return false, nil } - if options.Yes || !plan.RequiresConfirmation() { + autoApply := options.Yes || (options.Watch && !plan.HasDestructiveActions()) + if autoApply || !plan.RequiresConfirmation() { return true, nil } diff --git a/internal/sync/prompt/terminal_test.go b/internal/sync/prompt/terminal_test.go index 16ee4063..6740e636 100644 --- a/internal/sync/prompt/terminal_test.go +++ b/internal/sync/prompt/terminal_test.go @@ -37,3 +37,52 @@ func TestConfirmApply(t *testing.T) { }) } } + +func TestReviewAndConfirmPlanWatchPolicy(t *testing.T) { + tests := []struct { + name string + action Action + input string + interactive bool + yes bool + wantContinue bool + wantError string + wantPrompt bool + }{ + {name: "updates apply automatically", action: ActionUpdateServer, wantContinue: true}, + { + name: "server archive requires confirmation", action: ActionArchiveServer, input: "yes\n", + interactive: true, wantContinue: true, wantPrompt: true, + }, + { + name: "local deletion can be declined", action: ActionDeleteLocal, input: "no\n", + interactive: true, wantPrompt: true, + }, + { + name: "non-terminal destructive action is rejected", action: ActionArchiveServer, + wantError: "rerun with --yes", + }, + { + name: "explicit yes applies destructive action", action: ActionArchiveServer, + yes: true, wantContinue: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + var output bytes.Buffer + plan := Plan{Resources: []PlannedResource{{ID: testResourceID(), Action: test.action}}} + continued, err := reviewAndConfirmPlan(Options{ + Watch: true, Yes: test.yes, Input: strings.NewReader(test.input), ErrorOutput: &output, + }, plan, test.interactive) + + assert.Equal(t, test.wantContinue, continued) + if test.wantError != "" { + require.ErrorContains(t, err, test.wantError) + } else { + require.NoError(t, err) + } + assert.Equal(t, test.wantPrompt, strings.Contains(output.String(), "Sync these changes?")) + }) + } +} From 70469942abdad5df50774e171b8a7737200bbbc7 Mon Sep 17 00:00:00 2001 From: Clifford Tawiah Date: Thu, 1 Oct 2026 15:34:29 +0000 Subject: [PATCH 2/2] fix(sync): cancel destructive watch prompts --- internal/sync/prompt/terminal.go | 36 ++++++++++++- internal/sync/prompt/terminal_test.go | 74 +++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/internal/sync/prompt/terminal.go b/internal/sync/prompt/terminal.go index e6b89f81..bced14a0 100644 --- a/internal/sync/prompt/terminal.go +++ b/internal/sync/prompt/terminal.go @@ -2,6 +2,7 @@ package prompt import ( "bufio" + "context" "fmt" "io" "strings" @@ -28,7 +29,7 @@ func reviewAndConfirmPlan(options Options, plan Plan, interactive bool) (bool, e return true, nil } - confirmed, err := confirmApply(options.Input, options.ErrorOutput, interactive) + confirmed, err := confirmApplyWithContext(options.Context, options.Input, options.ErrorOutput, interactive) if err != nil { return false, err } @@ -40,6 +41,39 @@ func reviewAndConfirmPlan(options Options, plan Plan, interactive bool) (bool, e type terminalCheck func(io.Reader, io.Writer) bool +type confirmationResult struct { + confirmed bool + err error +} + +func confirmApplyWithContext(ctx context.Context, input io.Reader, prompt io.Writer, interactive bool) (bool, error) { + if ctx == nil { + return confirmApply(input, prompt, interactive) + } + if err := ctx.Err(); err != nil { + return false, err + } + + // io.Reader has no context-aware read contract. Isolate the blocking read + // so cancellation can return immediately; the buffered channel lets the + // reader finish without waiting for a receiver after the caller exits. + result := make(chan confirmationResult, 1) + go func() { + confirmed, err := confirmApply(input, prompt, interactive) + result <- confirmationResult{confirmed: confirmed, err: err} + }() + + select { + case <-ctx.Done(): + return false, ctx.Err() + case confirmation := <-result: + if err := ctx.Err(); err != nil { + return false, err + } + return confirmation.confirmed, confirmation.err + } +} + // confirmApply asks an interactive user to approve planned changes. func confirmApply(input io.Reader, prompt io.Writer, interactive bool) (bool, error) { if !interactive { diff --git a/internal/sync/prompt/terminal_test.go b/internal/sync/prompt/terminal_test.go index 6740e636..65701990 100644 --- a/internal/sync/prompt/terminal_test.go +++ b/internal/sync/prompt/terminal_test.go @@ -2,13 +2,30 @@ package prompt import ( "bytes" + "context" + "io" "strings" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +type notifyingWriter struct { + io.Writer + once sync.Once + written chan struct{} +} + +func (writer *notifyingWriter) Write(data []byte) (int, error) { + if bytes.Contains(data, []byte("Sync these changes?")) { + writer.once.Do(func() { close(writer.written) }) + } + return writer.Writer.Write(data) +} + func TestConfirmApply(t *testing.T) { tests := []struct { name string @@ -38,6 +55,63 @@ func TestConfirmApply(t *testing.T) { } } +func TestReviewAndConfirmPlanStopsWhenWatchContextIsCanceled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + input, inputWriter := io.Pipe() + defer input.Close() + defer inputWriter.Close() + + promptStarted := make(chan struct{}) + output := ¬ifyingWriter{Writer: io.Discard, written: promptStarted} + results := make(chan struct { + confirmed bool + err error + }, 1) + go func() { + confirmed, err := reviewAndConfirmPlan(Options{ + Watch: true, Context: ctx, Input: input, ErrorOutput: output, + }, Plan{Resources: []PlannedResource{{ + ID: testResourceID(), Action: ActionArchiveServer, + }}}, true) + results <- struct { + confirmed bool + err error + }{confirmed: confirmed, err: err} + }() + + select { + case <-promptStarted: + case <-time.After(time.Second): + t.Fatal("confirmation prompt was not written") + } + cancel() + + select { + case result := <-results: + require.False(t, result.confirmed) + require.ErrorIs(t, result.err, context.Canceled) + case <-time.After(time.Second): + _, err := io.WriteString(inputWriter, "yes\n") + require.NoError(t, err) + result := <-results + t.Fatalf("confirmation remained blocked after cancellation and later returned confirmed=%v, err=%v", result.confirmed, result.err) + } +} + +func TestReviewAndConfirmPlanRejectsConfirmationWhenWatchContextIsAlreadyCanceled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + confirmed, err := reviewAndConfirmPlan(Options{ + Watch: true, Context: ctx, Input: strings.NewReader("yes\n"), ErrorOutput: io.Discard, + }, Plan{Resources: []PlannedResource{{ + ID: testResourceID(), Action: ActionArchiveServer, + }}}, true) + + require.False(t, confirmed) + require.ErrorIs(t, err, context.Canceled) +} + func TestReviewAndConfirmPlanWatchPolicy(t *testing.T) { tests := []struct { name string